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

Remy Bohmer linux at bohmer.net
Tue Feb 15 21:04:53 CET 2011


Hi,

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_Sign

> +               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?

> +       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?

> +       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.

Please fix the double empty lines globally as well.

This patch looks relatively clean compared to the reset of the series.
Please rework comments.

Kind regards,

Remy


More information about the U-Boot mailing list