[U-Boot] twister: usb host support

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.

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

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

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

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