[U-Boot] [PATCH v4 1/2] usb: ehci: add Faraday USB 2.0 EHCI support

Kuo-Jung Su
2013-05-08T02:18:17Z

2013/5/8 Marek Vasut <marex@denx.de>:
> Dear Kuo-Jung Su,
>
>> From: Kuo-Jung Su <dantesu@faraday-tech.com>
>>
>> This patch add supports to both Faraday FUSBH200 and FOTG210,
>> the differences between Faraday EHCI and standard EHCI as
>> listed bellow:
>>
>> 1. The PORTSC starts at 0x30 instead of 0x44.
>> 2. The CONFIGFLAG(0x40) is not only un-implemented, and
>> also has its address removed.
>> 3. Faraday EHCI is a TDI design, but it doesn't
>> compatible with the general TDI implementation
>> found at both U-Boot and Linux.
>> 4. The ISOC descriptors differs from standard EHCI in
>> several ways. Since U-boot doesn't support ISOC,
>> we don't have to worry that.
>>
>> Signed-off-by: Kuo-Jung Su <dantesu@faraday-tech.com>
>> CC: Marek Vasut <marex@denx.de>
>> ---
>> Changes for v4:
>> - Use only macro constants and named bit/mask
>> - Use weak-aliased functions for tdi implementation and
>> also portsc registers to avoid poluting ehci.h with ifdefs
>>
>> 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.
>>
>> common/usb_hub.c | 13 +-
>> drivers/usb/host/Makefile | 1 +
>> drivers/usb/host/ehci-faraday.c | 146 ++++++++++++++++
>> drivers/usb/host/ehci-hcd.c | 104 ++++++++----
>> include/usb/fotg210.h | 358
>> +++++++++++++++++++++++++++++++++++++++ include/usb/fusbh200.h |
>> 61 +++++++
>> 6 files changed, 646 insertions(+), 37 deletions(-)
>> create mode 100644 drivers/usb/host/ehci-faraday.c
>> create mode 100644 include/usb/fotg210.h
>> create mode 100644 include/usb/fusbh200.h
>>
>> diff --git a/common/usb_hub.c b/common/usb_hub.c
>> index b5eeb62..3c6cb69 100644
>
> Please split the EHCI changes from this patch.
>

Got it, thanks

>> --- a/common/usb_hub.c
>> +++ b/common/usb_hub.c
>> @@ -419,6 +419,14 @@ static int usb_hub_configure(struct usb_device *dev)
>> portstatus = le16_to_cpu(portsts->wPortStatus);
>> portchange = le16_to_cpu(portsts->wPortChange);
>>
>> +#ifdef CONFIG_USB_EHCI_FARADAY
>> + /* Faraday EHCI needs a long long delay here */
>> + if (!portchange && !portstatus) {
>> + if (get_timer(start) < 250)
>> + continue;
>> + }
>> +#endif
>
> I'd say just call a weak function here, in case some other non-EHCI compliant
> controller happened to need this too. btw. does it need to be 250 ms or can you
> poll for readiness somehow ?
>

Got it, thanks. I'll add a weak function later,
and about the 250 ms is actually an estimated value.
The delay time is actually board specific, it looks to me
that it's somehow related to the number of usb host controllers
and the attached usb flash drivers.

For example:

1. A369 - FUSBH200: a usb flash driver attached
A369 - FOTG210: nothing attached
=> no extra delay required.

2. A369 - FUSBH200: nothing attached
A369 - FOTG210: a usb flash driver attached
=> no extra delay required.

3. A369 - FUSBH200: a usb flash driver attached
A369 - FOTG210: a usb flash driver attached
=> The 2nd ehci host requires 200 ms extra delay to detect the attached device.
So I put a 250ms here for safe.

>> if ((portchange & USB_PORT_STAT_C_CONNECTION) ==
>> (portstatus & USB_PORT_STAT_CONNECTION))
>> break;
>> @@ -441,7 +449,9 @@ static int usb_hub_configure(struct usb_device *dev)
>> i + 1, portstatus);
>> usb_clear_port_feature(dev, i + 1,
>> USB_PORT_FEAT_C_ENABLE);
>> -
>> + /* The following hack causes a ghost device problem
>> + * to Faraday EHCI */
>
> Invalid multiline comment style (not that the one right below is a shining
> example of a valid one :-( ).
>
> /*
> * This is how you
> * do multiline comments.
> */
>

Got it, thanks!

>> +#ifndef CONFIG_USB_EHCI_FARADAY
>> /* EM interference sometimes causes bad shielded USB
>> * devices to be shutdown by the hub, this hack enables
>> * them again. Works at least with mouse driver */
>> @@ -453,6 +463,7 @@ static int usb_hub_configure(struct usb_device *dev)
>> "re-enabling...\n", i + 1);
>> usb_hub_port_connect_change(dev, i);
>> }
>> +#endif
>> }
>> if (portstatus & USB_PORT_STAT_SUSPEND) {
>> USB_HUB_PRINTF("port %d suspend change\n", i + 1);
>
> [...]

--
Best wishes,
Kuo-Jung Su