[U-Boot] [PATCH v2 1/4] Add Atmel USBA UDC

[Marcel Janssen]

Hi Remy,

> Continuing producing some remarks:
> 
> 2011/2/13 Marcel Janssen <korgull at home.nl>:
> > From: Marcel <korgull at home.nl>
> > 
> > Atmel USBA UDC cleanup
> > 
> > Atmel USBA UDC cleanup
> > 
> > more cleanup of Atmel USBA UDC
> > 
> > Some more cleaning of Atmel USBA UDC
> > 
> > further cleaning of Atmel USBA UDC
> 
> Strange header.
> 
> > Signed-off-by: Marcel <korgull at home.nl>
> > ---
> >  drivers/usb/gadget/Makefile         |    1 +
> >  drivers/usb/gadget/atmel_usba_udc.c | 1438
> > +++++++++++++++++++++++++++++++++++ include/usb/atmel_usba_udc.h      
> >  |  398 ++++++++++
> >  3 files changed, 1837 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/usb/gadget/atmel_usba_udc.c
> >  create mode 100644 include/usb/atmel_usba_udc.h
> > 
> > diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
> > index 0846233..024844d 100644
> > --- a/drivers/usb/gadget/Makefile
> > +++ b/drivers/usb/gadget/Makefile
> > @@ -29,6 +29,7 @@ LIB   := $(obj)libusb_gadget.o
> >  ifdef CONFIG_USB_ETHER
> >  COBJS-y += ether.o epautoconf.o config.o usbstring.o
> >  COBJS-$(CONFIG_USB_GADGET_AT91) += at91_udc.o
> > +COBJS-$(CONFIG_USB_GADGET_ATMEL_USBA) += atmel_usba_udc.o
> >  else
> >  # Devices not related to the new gadget layer depend on
> > CONFIG_USB_DEVICE ifdef CONFIG_USB_DEVICE
> > diff --git a/drivers/usb/gadget/atmel_usba_udc.c
> > b/drivers/usb/gadget/atmel_usba_udc.c new file mode 100644
> > index 0000000..6d02de6
> > --- /dev/null
> > +++ b/drivers/usb/gadget/atmel_usba_udc.c
> > @@ -0,0 +1,1438 @@
> > +/*
> > + * Driver for the Atmel USBA high speed USB device controller
> > + *
> > + * Copyright (C) 2011 Marcel Janssen, Admesy B.V.
> > + * Copyright (C) 2005-2007 Atmel Corporation
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> 
> Is this GPLv2 only from its origin? I thought only GPLv2+ code was
> acceptable. See
> http://www.denx.de/wiki/view/U-Boot/Patches#Attributing_Code_Copyrights_Si
> gn

Wow, there's something I completely missed.
I would need to check on that. The original code comes from kernel 2.6.33 and 
I didn't change that part.

> > +               if (!(ptr->in_use))
> > +                       debug("%s: ptr not marked as \"in_use\"\n",
> > __func__); +
> > +               hang();
> 
> Is hang required? Is there no recovery to the terminal possible?

I guess not.

> > +       if (!ptr)
> > +               DBG("panic: no more free req buffers\n");
> 
> Cool. A panic in a debug printf that is removed by the preprocessor.
> Is it really panic?

Actually I think this should never be hit unless someone calls it wrong.

> > +       udc->regs = (unsigned *) AT91SAM9G45_BASE_UDPHS;
> > +       udc->fifo = (unsigned *) AT91SAM9G45_UDPHS_FIFO;
> 
> Is at91sam9g45 the only SoC that has this UDP-USBA controller?
> Make it a generic define. Globally.

So far I haven't seen others than the at91sam9g45 and at91sam9g45m10

> Please fix the double empty lines globally as well.
>
> This patch looks relatively clean compared to the reset of the series.
> Please rework comments.

I see what I can do.

Best regards,
Marcel