[U-Boot] twister: usb host support

[U-Boot] twister: usb host support

[Jeroen Hofstee]

Hello Stefano,

The twister always had a fragile usb support in u-boot
in the past. Since some floating patches mentioned
alternating success after usb hub resets, I had a look
if these patches fixed the twister as well, they don't.

Some fiddling around [1] makes the usb work however,
by not attempting to reset the USB hub with gpio_25.
I cannot find any document mentioning gpio_25 though.
Any reason for this usage? It seems better to get rid
of it completely.. Anything against that?

---
Regards,
Jeroen

p.s.
I found by sheer accidence that the usb is behaving
completely normal (after the patch) on rev B1. It is
broken on rev B without the patch and working buggy
with it. On rev B1 + patch the USB to SATA converter
is discovered as well, which I have never seen before.


=== [1] patch to disable the usb reset

The change to port_mode[1] is not needed, but since it is unused,
MODE_UNUSED seems appropriate.


diff --git a/board/technexion/twister/twister.c 
b/board/technexion/twister/twister.c
index a28c704..31cf6c0 100644
--- a/board/technexion/twister/twister.c
+++ b/board/technexion/twister/twister.c
@@ -63,7 +63,7 @@ static const u32 gpmc_XR16L2751[] = {
  #ifdef CONFIG_USB_EHCI
  static struct omap_usbhs_board_data usbhs_bdata = {
      .port_mode[0] = OMAP_EHCI_PORT_MODE_PHY,
-    .port_mode[1] = OMAP_EHCI_PORT_MODE_PHY,
+    .port_mode[1] = OMAP_USBHS_PORT_MODE_UNUSED,
      .port_mode[2] = OMAP_USBHS_PORT_MODE_UNUSED,
  };

@@ -92,9 +92,6 @@ int board_init(void)
      enable_gpmc_cs_config(gpmc_XR16L2751, &gpmc_cfg->cs[3],
          XR16L2751_UART2_BASE, GPMC_SIZE_16M);

-    gpio_request(CONFIG_OMAP_EHCI_PHY1_RESET_GPIO, "USB_PHY1_RESET");
-    gpio_direction_output(CONFIG_OMAP_EHCI_PHY1_RESET_GPIO, 1);
-

=== [2] current usb master (rev B1) ===

twister => usb start
(Re)start USB...
USB0:   USB EHCI 1.00
scanning bus 0 for devices... 3 USB Device(s) found
        scanning usb for storage devices... 1 Storage Device(s) found
twister => usb start
(Re)start USB...
USB0:   USB EHCI 1.00
scanning bus 0 for devices... cannot reset port 1!?
1 USB Device(s) found
        scanning usb for storage devices... 0 Storage Device(s) found
twister => usb start
(Re)start USB...
USB0:   USB EHCI 1.00
scanning bus 0 for devices... 3 USB Device(s) found
        scanning usb for storage devices... 1 Storage Device(s) found
twister => usb start
(Re)start USB...
USB0:   USB EHCI 1.00
scanning bus 0 for devices... cannot reset port 1!?
1 USB Device(s) found
        scanning usb for storage devices... 0 Storage Device(s) found

=== [3] after the patch (rev B1) ===

twister => usb start
(Re)start USB...
USB0:   USB EHCI 1.00
scanning bus 0 for devices... 4 USB Device(s) found
        scanning usb for storage devices... Device NOT ready
    Request Sense returned 02 3A 00
1 Storage Device(s) found

twister => usb tree
USB device tree:
   1  Hub (480 Mb/s, 0mA)
   |  u-boot EHCI Host Controller
   |
   +-2  Hub (480 Mb/s, 100mA)
     |   USB2.0 Hub
     |
     +-3  Mass Storage (480 Mb/s, 96mA)
     |     USB Storage 000000000033
     |
     +-4  Mass Storage (480 Mb/s, 498mA)
          Philips USB Flash Drive 070825AEA237D121


=== [4] after the patch (rev B) ===
Re)start USB...
USB0:   USB EHCI 1.00
scanning bus 0 for devices... 1 USB Device(s) found
        scanning usb for storage devices... 0 Storage Device(s) found
bpp3=> usb start
(Re)start USB...
USB0:   USB EHCI 1.00
scanning bus 0 for devices... 3 USB Device(s) found
        scanning usb for storage devices... 1 Storage Device(s) found
bpp3=> usb start
(Re)start USB...
USB0:   USB EHCI 1.00
scanning bus 0 for devices... cannot reset port 1!?
1 USB Device(s) found
        scanning usb for storage devices... 0 Storage Device(s) found

=== [5]
twister => usb start
(Re)start USB...
USB0:   USB EHCI 1.00
scanning bus 0 for devices... port(0) reset error
cannot reset port 1!?
1 USB Device(s) found
        scanning usb for storage devices... 0 Storage Device(s) found
twister => usb start
(Re)start USB...
USB0:   USB EHCI 1.00
scanning bus 0 for devices... cannot reset port 1!?
1 USB Device(s) found
        scanning usb for storage devices... 0 Storage Device(s) found




      return 0;
  }

diff --git a/include/configs/tam3517-common.h 
b/include/configs/tam3517-common.h
index 2af504b..852bdd7 100644
--- a/include/configs/tam3517-common.h
+++ b/include/configs/tam3517-common.h
@@ -102,7 +102,6 @@
  #define CONFIG_USB_EHCI_OMAP
  #define CONFIG_USB_ULPI
  #define CONFIG_USB_ULPI_VIEWPORT_OMAP
-#define CONFIG_OMAP_EHCI_PHY1_RESET_GPIO    25
  #define CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS 3
  #define CONFIG_USB_STORAGE

[U-Boot] twister: usb host support

[Stefano Babic]

Hi Jeroen,


On 09/07/2013 21:46, Jeroen Hofstee wrote:
> Hello Stefano,
> 
> The twister always had a fragile usb support in u-boot
> in the past. Since some floating patches mentioned
> alternating success after usb hub resets, I had a look
> if these patches fixed the twister as well, they don't.
> 
> Some fiddling around [1] makes the usb work however,
> by not attempting to reset the USB hub with gpio_25.
> I cannot find any document mentioning gpio_25 though.
> Any reason for this usage?

That is right. There is no written documentation by Technexion about
this pin. I have added in CC Tapani, maybe he can tell us something more.
The GPIO is used internally on the TAM3517 SOM, and we have no
schematics about it. We have to guess about its usage.

The reason to have it is that the sources for the kernel provided by
Technexion (an ancient 2.6.32 Version) set this pin to reset the EHCI.
As I said, there is no further documentation and rather we can only make
some supposition. The usage in U-Boot then reflects the usage made by
Technexion.


> p.s.
> I found by sheer accidence that the usb is behaving
> completely normal (after the patch) on rev B1. It is
> broken on rev B without the patch and working buggy
> with it. On rev B1 + patch the USB to SATA converter
> is discovered as well, which I have never seen before.
> 

I can confirm issues with version B of the TAM3517-SOM. I am testing a
B1 SOM with a B twister. I do not know the details about changelog
between B and B1 version of the SOM, but I got both USB and Ethernet
problems with TAM3517-B. The same image runs without the same problems
on a B1.

There is also this pending patch by Michael:

http://patchwork.ozlabs.org/patch/250290/

It is applied to u-boot-ti, not yet to mainline.

I have tested current mainline with Mihael's patch applied, and the
recognition of the USB storage seens reliable to me. No case with "0
device found".


> 
> === [1] patch to disable the usb reset
> 
> The change to port_mode[1] is not needed, but since it is unused,
> MODE_UNUSED seems appropriate.

Agree - as we do not use it, we can simply disable.

> diff --git a/board/technexion/twister/twister.c
> b/board/technexion/twister/twister.c
> index a28c704..31cf6c0 100644
> --- a/board/technexion/twister/twister.c
> +++ b/board/technexion/twister/twister.c
> @@ -63,7 +63,7 @@ static const u32 gpmc_XR16L2751[] = {
>  #ifdef CONFIG_USB_EHCI
>  static struct omap_usbhs_board_data usbhs_bdata = {
>      .port_mode[0] = OMAP_EHCI_PORT_MODE_PHY,
> -    .port_mode[1] = OMAP_EHCI_PORT_MODE_PHY,
> +    .port_mode[1] = OMAP_USBHS_PORT_MODE_UNUSED,
>      .port_mode[2] = OMAP_USBHS_PORT_MODE_UNUSED,
>  };
> 
> @@ -92,9 +92,6 @@ int board_init(void)
>      enable_gpmc_cs_config(gpmc_XR16L2751, &gpmc_cfg->cs[3],
>          XR16L2751_UART2_BASE, GPMC_SIZE_16M);
> 
> -    gpio_request(CONFIG_OMAP_EHCI_PHY1_RESET_GPIO, "USB_PHY1_RESET");
> -    gpio_direction_output(CONFIG_OMAP_EHCI_PHY1_RESET_GPIO, 1);
> -

See my concerns above. Do not reset the hub in the kernel ?

Best regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] twister: usb host support

[Jeroen Hofstee]

Hello Stefano / Tapani,

On 07/10/2013 04:27 PM, Stefano Babic wrote:
> On 09/07/2013 21:46, Jeroen Hofstee wrote:
>> I cannot find any document mentioning gpio_25 though.
>> Any reason for this usage?
> That is right. There is no written documentation by Technexion about
> this pin. I have added in CC Tapani, maybe he can tell us something more.
> The GPIO is used internally on the TAM3517 SOM, and we have no
> schematics about it. We have to guess about its usage.

Yes, and some clarification of the pin 35 would be
appreciated as well, since it is marked as ball X, I guess
it is controlled by hardware and the hub cannot be reset
from software, but that is just guessing...

> The reason to have it is that the sources for the kernel provided by
> Technexion (an ancient 2.6.32 Version) set this pin to reset the EHCI.
> As I said, there is no further documentation and rather we can only make
> some supposition. The usage in U-Boot then reflects the usage made by
> Technexion.
>

Fyi my linux copy (3.7) with this gpio set as the phy_reset is also
unable to properly reset it and times out. The usb is working
though, also without a proper reset..

>> p.s.
>> I found by sheer accidence that the usb is behaving
>> completely normal (after the patch) on rev B1. It is
>> broken on rev B without the patch and working buggy
>> with it. On rev B1 + patch the USB to SATA converter
>> is discovered as well, which I have never seen before.
>>
> I can confirm issues with version B of the TAM3517-SOM. I am testing a
> B1 SOM with a B twister. I do not know the details about changelog
> between B and B1 version of the SOM, but I got both USB and Ethernet
> problems with TAM3517-B. The same image runs without the same problems
> on a B1.

ok, good to know.

> There is also this pending patch by Michael:
>
> http://patchwork.ozlabs.org/patch/250290/
>
> It is applied to u-boot-ti, not yet to mainline.
>

on top of usb master this patch leads to:
USB0:    ULPI: ulpi_reset: failed writing reset bit
ULPI:    ulpi_reset: failed writing reset bit

It does always find the stick though and never the SATA converter.

without the gpio_25 reset, the second error goes away and
the SATA is back.

>
> @@ -92,9 +92,6 @@ int board_init(void)
>       enable_gpmc_cs_config(gpmc_XR16L2751, &gpmc_cfg->cs[3],
>           XR16L2751_UART2_BASE, GPMC_SIZE_16M);
>
> -    gpio_request(CONFIG_OMAP_EHCI_PHY1_RESET_GPIO, "USB_PHY1_RESET");
> -    gpio_direction_output(CONFIG_OMAP_EHCI_PHY1_RESET_GPIO, 1);
> -
> See my concerns above. Do not reset the hub in the kernel ?

I don't get the last part, but feedback from Technexion is
needed first to remove all the guess, maybe etc. If it has a
valid function, not setting it's value might not be such a
good idea...

Regards,
Jeroen

[U-Boot] twister: usb host support

[Tapani]

Stefano, Jeroen,


On Wed, 10 Jul 2013 18:04:04 +0200
Jeroen Hofstee <jeroen@myspectrum.nl> wrote:

> Hello Stefano / Tapani,
> 
> On 07/10/2013 04:27 PM, Stefano Babic wrote:
> > On 09/07/2013 21:46, Jeroen Hofstee wrote:
> >> I cannot find any document mentioning gpio_25 though.
> >> Any reason for this usage?
> > That is right. There is no written documentation by Technexion about
> > this pin. I have added in CC Tapani, maybe he can tell us something more.
> > The GPIO is used internally on the TAM3517 SOM, and we have no
> > schematics about it. We have to guess about its usage.
> 
> Yes, and some clarification of the pin 35 would be
> appreciated as well, since it is marked as ball X, I guess
> it is controlled by hardware and the hub cannot be reset
> from software, but that is just guessing...
> 
> > The reason to have it is that the sources for the kernel provided by
> > Technexion (an ancient 2.6.32 Version) set this pin to reset the EHCI.
> > As I said, there is no further documentation and rather we can only make
> > some supposition. The usage in U-Boot then reflects the usage made by
> > Technexion.
> >
> 
> Fyi my linux copy (3.7) with this gpio set as the phy_reset is also
> unable to properly reset it and times out. The usb is working
> though, also without a proper reset..
> 
> >> p.s.
> >> I found by sheer accidence that the usb is behaving
> >> completely normal (after the patch) on rev B1. It is
> >> broken on rev B without the patch and working buggy
> >> with it. On rev B1 + patch the USB to SATA converter
> >> is discovered as well, which I have never seen before.
> >>
> > I can confirm issues with version B of the TAM3517-SOM. I am testing a
> > B1 SOM with a B twister. I do not know the details about changelog
> > between B and B1 version of the SOM, but I got both USB and Ethernet
> > problems with TAM3517-B. The same image runs without the same problems
> > on a B1.
> 
> ok, good to know.
> 
> > There is also this pending patch by Michael:
> >
> > http://patchwork.ozlabs.org/patch/250290/
> >
> > It is applied to u-boot-ti, not yet to mainline.
> >
> 
> on top of usb master this patch leads to:
> USB0:    ULPI: ulpi_reset: failed writing reset bit
> ULPI:    ulpi_reset: failed writing reset bit
> 
> It does always find the stick though and never the SATA converter.
> 
> without the gpio_25 reset, the second error goes away and
> the SATA is back.
> 
> >
> > @@ -92,9 +92,6 @@ int board_init(void)
> >       enable_gpmc_cs_config(gpmc_XR16L2751, &gpmc_cfg->cs[3],
> >           XR16L2751_UART2_BASE, GPMC_SIZE_16M);
> >
> > -    gpio_request(CONFIG_OMAP_EHCI_PHY1_RESET_GPIO, "USB_PHY1_RESET");
> > -    gpio_direction_output(CONFIG_OMAP_EHCI_PHY1_RESET_GPIO, 1);
> > -
> > See my concerns above. Do not reset the hub in the kernel ?
> 
> I don't get the last part, but feedback from Technexion is
> needed first to remove all the guess, maybe etc. If it has a
> valid function, not setting it's value might not be such a
> good idea...
>

GPIO_25 is indeed USB PHY reset. (Pull low to reset). 

regards,
 
//Tapani

[U-Boot] twister: usb host support

[Stefano Babic]

Hi Jeroen,

On 10/07/2013 18:04, Jeroen Hofstee wrote:

>> @@ -92,9 +92,6 @@ int board_init(void)
>>       enable_gpmc_cs_config(gpmc_XR16L2751, &gpmc_cfg->cs[3],
>>           XR16L2751_UART2_BASE, GPMC_SIZE_16M);
>>
>> -    gpio_request(CONFIG_OMAP_EHCI_PHY1_RESET_GPIO, "USB_PHY1_RESET");
>> -    gpio_direction_output(CONFIG_OMAP_EHCI_PHY1_RESET_GPIO, 1);
>> -
>> See my concerns above. Do not reset the hub in the kernel ?
> 
> I don't get the last part, but feedback from Technexion is
> needed first to remove all the guess, maybe etc. If it has a
> valid function, not setting it's value might not be such a
> good idea...

I think we could safe drop the two lines. In fact, this code is
redundant and it is already called inside the omap_ehci_phy_reset()
function. No changes in behavior if we remove this lines from board_init().

Regards,
Stefano


-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================