Re: [U-Boot] [PATCH v4 2/2] usb: gadget: add Faraday FOTG210 USB gadget support

[Marek Vasut]

Dear Kuo-Jung Su,

> From: Kuo-Jung Su <dant...@faraday-tech.com>
> 
> The Faraday FOTG210 is an OTG chip which could operate
> as either an EHCI Host or a USB Device as a time.
> 
> Signed-off-by: Kuo-Jung Su <dant...@faraday-tech.com>
> CC: Marek Vasut <ma...@denx.de>
> ---
> Changes for v4:
>    - Use only macro constants and named bit/mask
>    - Remove dcache_enable() from usb_gadget_register_driver()
> 
> Changes for v3:
>    - Coding Style cleanup.
>    - Drop bit fields from c struct.
>    - Drop macros for wirtel()/readl(), call them directly.
>    - Always insert a blank line between declarations and code.
>    - Replace all the infinite wait loop with a timeout.
>    - Add '__iomem' to all the declaration of HW register pointers.
> 
> Changes for v2:
>    - Coding Style cleanup.
>    - Use readl(), writel(), clrsetbits_le32() to replace REG() macros.
>    - Use structure based hardware registers to replace the macro constants.
>    - Replace BIT() with BIT_MASK().
>    - echi-faraday: Remove debug codes.
> 
>  drivers/usb/gadget/Makefile       |    1 +
>  drivers/usb/gadget/fotg210.c      |  961
> +++++++++++++++++++++++++++++++++++++ drivers/usb/gadget/gadget_chips.h | 
>   8 +
>  3 files changed, 970 insertions(+)
>  create mode 100644 drivers/usb/gadget/fotg210.c
> 
> diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
> index e545b6b..432cf17 100644
> --- a/drivers/usb/gadget/Makefile
> +++ b/drivers/usb/gadget/Makefile
> @@ -35,6 +35,7 @@ endif
>  # new USB gadget layer dependencies
>  ifdef CONFIG_USB_GADGET
>  COBJS-$(CONFIG_USB_GADGET_S3C_UDC_OTG) += s3c_udc_otg.o
> +COBJS-$(CONFIG_USB_GADGET_FOTG210) += fotg210.o
>  COBJS-$(CONFIG_USBDOWNLOAD_GADGET) += g_dnl.o
>  COBJS-$(CONFIG_DFU_FUNCTION) += f_dfu.o
>  endif
> diff --git a/drivers/usb/gadget/fotg210.c b/drivers/usb/gadget/fotg210.c
> new file mode 100644
> index 0000000..70797ae
> --- /dev/null
> +++ b/drivers/usb/gadget/fotg210.c
> @@ -0,0 +1,961 @@
> +/*
> + * Faraday USB 2.0 OTG Controller
> + *
> + * (C) Copyright 2010 Faraday Technology
> + * Dante Su <dant...@faraday-tech.com>
> + *
> + * This file is released under the terms of GPL v2 and any later version.
> + * See the file COPYING in the root directory of the source tree for
> details. + */
> +
> +#include <common.h>
> +#include <command.h>
> +#include <config.h>
> +#include <net.h>
> +#include <malloc.h>
> +#include <asm/io.h>
> +#include <asm/errno.h>
> +#include <linux/types.h>
> +#include <linux/usb/ch9.h>
> +#include <linux/usb/gadget.h>
> +
> +#include <usb/fotg210.h>
> +
> +#define CFG_NUM_ENDPOINTS            4
> +#define CFG_EP0_MAX_PACKET_SIZE      64
> +#define CFG_EPX_MAX_PACKET_SIZE      512
> +
> +#define CFG_CMD_TIMEOUT (CONFIG_SYS_HZ >> 2) /* 250 ms */
> +#define CFG_RST_TIMEOUT (CONFIG_SYS_HZ >> 2) /* 250 ms */

Uh, how's this supposed to work? Why don't you just use mdelay(250) with those?

[...]

> +static inline int ep_reset(struct fotg210_chip *chip, uint8_t ep_addr)
> +{
> +     int ep = ep_addr & USB_ENDPOINT_NUMBER_MASK;
> +     struct fotg210_regs __iomem *regs = chip->regs;

No need for __iomem here.

[...]

> +     /* 3. DMA target setup */
> +#ifndef CONFIG_SYS_DCACHE_OFF
> +     if (ep->desc->bEndpointAddress & USB_DIR_IN)
> +             flush_dcache_range((uint32_t)buf, (uint32_t)buf + len);
> +     else
> +             invalidate_dcache_range((uint32_t)buf, (uint32_t)buf + len);
> +#endif

The cache stuff will be optimized out anyway, no ?

[...]

> +                     if (readl(&regs->dev_ctrl) & DEVCTRL_HS) {
> +                             puts("fotg210: HS\n");
> +                             chip->gadget.speed = USB_SPEED_HIGH;
> +                             writel(0x044c, &regs->sof_mtr);
> +                     } else {
> +                             puts("fotg210: FS\n");
> +                             chip->gadget.speed = USB_SPEED_FULL;
> +                             writel(0x2710, &regs->sof_mtr);

Magic values, please fix globally as previously asked.

[...]

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot