[U-Boot] [PATCH 7/7] usb: hub: Increase the query delay

[Marek Vasut <marex@denx.de>]

On 05/04/2016 10:07 AM, Stefan Roese wrote:
> On 03.05.2016 22:51, Marek Vasut wrote:
>> Increase the query delay, otherwise some sticks are not detected.
>> The problem shows up on the USB bus analyzer such that the stick
>> takes longer time to switch from FS mode to HS mode than the code
>> allows.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Chin Liang See <clsee@altera.com>
>> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
>> Cc: Hans de Goede <hdegoede@redhat.com>
>> Cc: Stefan Roese <sr@denx.de>
>> Cc: Stephen Warren <swarren@nvidia.com>
>> ---
>>   common/usb_hub.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/common/usb_hub.c b/common/usb_hub.c
>> index 0f39c9f..6cd274a 100644
>> --- a/common/usb_hub.c
>> +++ b/common/usb_hub.c
>> @@ -145,7 +145,7 @@ static void usb_hub_power_on(struct usb_hub_device
>> *hub)
>>        * Do a minimum delay of the larger value of 100ms or pgood_delay
>>        * so that the power can stablize before the devices are queried
>>        */
>> -    hub->query_delay = get_timer(0) + max(100, (int)pgood_delay);
>> +    hub->query_delay = get_timer(0) + max(1000, (int)pgood_delay);
>>
>>       /*
>>        * Record the power-on timeout here. The max. delay (timeout)
>>
> 
> We have touched this part of the delay a number of times in my
> USB scanning patch series. I've integrated all very valuable
> suggestions from Stephen and Hans and I'm pretty sure that the
> current implementation is aligned with the USB spec.

Sadly, not everyone goes exactly be the USB spec it seems.
Especially the cheap sticks which are the majority in the market.

> And tested
> successfully one multiple platforms without regression.
> Stephen did some tests on Tegra and Hans on sunxi. I tested
> mainly on x86. But I now also tested on Armada XP (see
> below).

All of which use the same EHCI or XHCI controller, correct ?

> As mentioned before, the current version causes no problems with
> all my USB sticks on the congatec x86 board. Even the 2 ones that
> are problematic when connected to the SoCFPGA. They are detected
> without any issues on this board. Thats why I assume, that the
> real problem here is the DWC driver and not the generic USB
> handling code.

The logic here is flawed. If the code works against EHCI controller and
does not work against DWC2 controller, you cannot infer from this that
the DWC2 controller is the problem.

This can also be specific behavior of the EHCI controller, and I in fact
suspect it is, but you cannot decide this without checking the
bus itself.

> My feeling is that increasing this initial delay
> (before the scanning starts) just papers over the real problem
> most likely hidden somewhere in the DWC driver. This is just
> a feeling though which I can't prove somehow other than testing
> the common USB code on different platforms. And I really have
> no deeper knowledge of the DWC driver to manifest this feeling
> or even fix this potential problem there.

The bus analyzer tells me that the stick just takes longer to come out
of FS mode and switch to HS mode when the USB started from reset state
of the controller, I decide to trust what the analyzer shows me instead.

See attached logs.

> I've already mentioned this in another mail. My suggestion for
> SoCFPGA boards that need to use these problematic USB keys is
> to use the already available solution to set "usb_pgood_delay"
> to 1000. This effectively does the same as this patch. Without
> implying this general 1 second delay per hub (!!!) to all other
> platforms that use USB in U-Boot.
> 
> To test my suspicions about this being a DWC (SoCFPGA?) only
> problem, I've also tested all my current USB sticks including
> the 2 problematic ones (on SoCrates) on another ARM platform
> (additionally to all my test on x86). I've used the Marvell
> Armada XP development board (db-mv784mp-gp) for this. And all
> USB sticks are detected without any problems on this platform.

I see, it would be interesting to know what happens on the bus on the
marvell board compared to the socfpga board. For the socfpga, the bus
starts from complete cold state, could it be the marvell does have the
EHCI running when you do your tests ? That would explain why the stick
might be ready much faster on your platform.

> As a result of all this, I would like to have this patch not
> applied. As it negatively touches the common USB code to fix
> (paper over?) a problem only seen on one platform (AFAIK). And
> we already have the solution of this "usb_pgood_delay" that
> can be used on SoCFPGA. To manifest this, here again the
> numbers for the USB scanning time on x86, without and with
> this patch:
> 
> Without this patch:
> starting USB...
> USB0:   USB EHCI 1.00
> scanning bus 0 for devices... 9 USB Device(s) found
> 
> time: 1.940 seconds
> 
> With this patch:
> => time usb start
> starting USB...
> USB0:   USB EHCI 1.00
> scanning bus 0 for devices... 9 USB Device(s) found
> 
> time: 5.421 seconds

Well that's great, removing some delays makes the code run faster and
breaks some platform. Stefan, I need a solution for this release and
the release is coming very quickly. So far, I see no other proposals
how to fix this issue and it's been reported for well over a month.

As I don't want to have regressions in this release, here are the two
options:
a) I revert "usb: Change power-on / scanning timeout handling"
b) I apply these 7 patches. I am willing to isolate this increase
   of delay with an ifdef to DWC2 controller, but that does NOT
   seem correct according to the analyzer. I would just wait for
   an EHCI controller on which this breaks.

> Thanks,
> Stefan


-- 
Best regards,
Marek Vasut
-------------- next part --------------
A non-text attachment was scrubbed...
Name: trace-dwc2-with-delay.csv
Type: text/csv
Size: 4657 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160504/74350c99/attachment.csv>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: trace-dwc2-without-delay.csv
Type: text/csv
Size: 1066 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160504/74350c99/attachment-0001.csv>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: trace-ehci.csv
Type: text/csv
Size: 4362 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160504/74350c99/attachment-0002.csv>