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 <ma...@denx.de>
>> Cc: Chin Liang See <cl...@altera.com>
>> Cc: Dinh Nguyen <dingu...@opensource.altera.com>
>> Cc: Hans de Goede <hdego...@redhat.com>
>> Cc: Stefan Roese <s...@denx.de>
>> Cc: Stephen Warren <swar...@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
# Total Phase Data Center(tm) v6.71
# (c) 2005-2016 Total Phase, Inc.
# www.totalphase.com
#
# 
#
# Level,Sp,Index,m:s.ms.us,Dur,Len,Err,Dev,Ep,Record,Summary
0,,0,0:00.000.000,,,,,,Capture started (Aggregate),[05/04/16 13:22:38]
0,,1,0:00.000.000,,,,,,<Host disconnected>,
0,,2,0:11.250.028,,,,,,<Host connected>,
0,FS,3,0:12.627.507,,,,,,<Full-speed>,
0,FS,4,0:12.630.507,99.017.400 ms,,,,,<Suspend>,
0,FS,5,0:12.729.525,132.316 us,,,,,<Reset> / <Chirp J> / <Tiny J>,
0,FS,6,0:12.729.657,2.214.633 ms,,,,,<Chirp K>,
0,FS,7,0:12.731.872,5.250 us,,,,,<Reset> / <Chirp J> / <Tiny J>,
0,FS,8,0:12.731.877,21.617.500 ms,,,,,[216 Chirp K-J pairs] ,
0,FS,442,0:12.753.494,325.316 us,,,,,<Reset>,
0,HS,443,0:12.753.820,,,,,,<High-speed>,
0,HS,444,0:12.753.820,204.654.333 ms,,,,,[1638 SOF],[Frames: 0.x - 204.5] 
0,HS,445,0:12.957.186,1.292.816 ms,18 B,,00,00,Get Device Descriptor,Index=0 Length=64
0,HS,459,0:12.958.498,35.816 us,0 B,,00,00,Set Address,Address=02
0,HS,469,0:12.958.599,11.251.683 ms,,,,,[91 SOF],[Frames: 204.6 - 216.0] 
0,HS,470,0:12.968.589,1.293.416 ms,18 B,,02,00,Get Device Descriptor,Index=0 Length=18
0,HS,484,0:12.969.976,66 ns,,,,,[1 SOF],[Frame: 216.1] 
0,HS,485,0:12.969.901,128.300 us,9 B,,02,00,Get Configuration Descriptor,Index=0 Length=9
0,HS,499,0:12.970.101,125.100 us,,,,,[2 SOF],[Frames: 216.2 - 216.3] 
0,HS,500,0:12.970.056,276.100 us,32 B,,02,00,Get Configuration Descriptor,Index=0 Length=32
0,HS,514,0:12.970.351,83 ns,,,,,[1 SOF],[Frame: 216.4] 
0,HS,515,0:12.970.368,45.166 us,0 B,,02,00,Set Configuration,Configuration=1
0,HS,525,0:12.970.476,1.125.233 ms,,,,,[10 SOF],[Frames: 216.5 - 217.6] 
0,HS,526,0:12.970.437,1.206.883 ms,4 B,,02,00,Get String Descriptor,Index=0 Length=255
0,HS,540,0:12.971.726,1.125.233 ms,,,,,[10 SOF],[Frames: 217.7 - 219.0] 
0,HS,541,0:12.971.662,1.239.450 ms,10 B,,02,00,Get String Descriptor,Index=1 Length=255
0,HS,555,0:12.972.976,1.250.250 ms,,,,,[11 SOF],[Frames: 219.1 - 220.3] 
0,HS,556,0:12.972.922,1.384.250 ms,34 B,,02,00,Get String Descriptor,Index=2 Length=255
0,HS,570,0:12.974.351,1.250.266 ms,,,,,[11 SOF],[Frames: 220.4 - 221.6] 
0,HS,571,0:12.974.330,1.286.366 ms,18 B,,02,00,Get String Descriptor,Index=3 Length=255
0,HS,585,0:12.975.726,125.100 us,,,,,[2 SOF],[Frames: 221.7 - 222.0] 
0,HS,586,0:12.975.843,52.600 us,1 B,,02,00,Get Max LUN,Max LUN = 0
0,HS,600,0:12.975.976,5.000.800 ms,,,,,[41 SOF],[Frames: 222.1 - 227.1] 
0,HS,601,0:12.975.925,5.070.783 ms,36 B,,02,01,Inquiry [0],Passed
0,HS,618,0:12.981.102,4.875.783 ms,,,,,[40 SOF],[Frames: 227.2 - 232.1] 
0,HS,619,0:12.981.029,5.031.616 ms,,,02,01,Test Unit Ready [0],Failed
0,HS,630,0:12.986.103,5.000.800 ms,,,,,[41 SOF],[Frames: 232.2 - 237.2] 
0,HS,631,0:12.986.088,5.071.300 ms,18 B,,02,01,Request Sense [0],Sense Key = Unit Attention (Passed)
0,HS,648,0:12.991.229,105.265.133 ms,,,,,[843 SOF],[Frames: 237.3 - 342.5] 
0,HS,649,0:13.091.514,5.033.783 ms,,,02,01,Test Unit Ready [0],Passed
0,HS,660,0:13.096.619,125.100 us,,,,,[2 SOF],[Frames: 342.6 - 342.7] 
0,HS,661,0:13.096.575,185.200 us,8 B,,02,01,Read Capacity [0],Passed
0,HS,679,0:13.096.869,5.000.800 ms,,,,,[41 SOF],[Frames: 343.0 - 348.0] 
0,HS,680,0:13.096.794,5.107.100 ms,512 B,,02,01,Read [0],LBA = 0 Length = 1 block (Passed)
0,HS,697,0:13.101.994,1.999.910.983 s,,,,,[15998 SOF],[Frames: 348.1 - 299.6] [Periodic Timeout]
0,HS,698,0:15.102.030,1.999.910.983 s,,,,,[15998 SOF],[Frames: 299.7 - 251.4] [Periodic Timeout]
0,HS,699,0:17.102.066,1.999.910.983 s,,,,,[15998 SOF],[Frames: 251.5 - 203.2] [Periodic Timeout]
0,HS,700,0:19.102.102,1.999.910.966 s,,,,,[15998 SOF],[Frames: 203.3 - 155.0] [Periodic Timeout]
0,HS,701,0:21.102.138,1.999.910.983 s,,,,,[15998 SOF],[Frames: 155.1 - 106.6] [Periodic Timeout]
0,HS,702,0:23.102.174,1.999.910.983 s,,,,,[15998 SOF],[Frames: 106.7 - 58.4] [Periodic Timeout]
0,HS,703,0:25.102.210,1.999.910.983 s,,,,,[15998 SOF],[Frames: 58.5 - 10.2] [Periodic Timeout]
0,HS,704,0:27.102.246,1.999.910.966 s,,,,,[15998 SOF],[Frames: 10.3 - 2010.0] [Periodic Timeout]
0,HS,705,0:29.102.282,1.999.910.966 s,,,,,[15998 SOF],[Frames: 2010.1 - 1961.6] [Periodic Timeout]
0,HS,706,0:31.102.318,1.999.910.966 s,,,,,[15998 SOF],[Frames: 1961.7 - 1913.4] [Periodic Timeout]
0,HS,707,0:33.102.354,1.999.910.966 s,,,,,[15998 SOF],[Frames: 1913.5 - 1865.2] [Periodic Timeout]
0,HS,708,0:35.102.389,1.999.910.950 s,,,,,[15998 SOF],[Frames: 1865.3 - 1817.0] [Periodic Timeout]
0,HS,709,0:37.102.425,1.999.910.966 s,,,,,[15998 SOF],[Frames: 1817.1 - 1768.6] [Periodic Timeout]
0,HS,710,0:39.102.461,1.641.484.733 s,,,,,[13131 SOF],[Frames: 1768.7 - 1362.1] 
0,,711,0:40.743.946,,,,,,Capture stopped,[05/04/16 13:23:19]
# Total Phase Data Center(tm) v6.71
# (c) 2005-2016 Total Phase, Inc.
# www.totalphase.com
#
# 
#
# Level,Sp,Index,m:s.ms.us,Dur,Len,Err,Dev,Ep,Record,Summary
0,,0,0:00.000.000,,,,,,Capture started (Aggregate),[05/04/16 13:21:26]
0,,1,0:00.000.000,,,,,,<Host disconnected>,
0,,2,0:03.317.745,,,,,,<Host connected>,
0,FS,3,0:04.692.784,,,,,,<Full-speed>,
0,FS,4,0:04.695.784,99.017.400 ms,,,,,<Suspend>,
0,FS,5,0:04.794.801,131.433 us,,,,,<Reset> / <Chirp J> / <Tiny J>,
0,FS,6,0:04.794.933,2.213.166 ms,,,,,<Chirp K>,
0,FS,7,0:04.797.146,5.250 us,,,,,<Reset> / <Chirp J> / <Tiny J>,
0,FS,8,0:04.797.151,2.000.018.333 s,,,,,[19984 Chirp K-J pairs] ,
0,FS,39978,0:06.797.169,2.000.018.400 s,,,,,[19984 Chirp K-J pairs] ,
0,FS,79948,0:08.797.188,2.000.018.383 s,,,,,[19984 Chirp K-J pairs] ,
0,FS,119918,0:10.797.206,2.000.018.383 s,,,,,[19984 Chirp K-J pairs] ,
0,FS,159888,0:12.797.225,2.000.018.383 s,,,,,[19984 Chirp K-J pairs] ,
0,FS,199858,0:14.797.243,1.665.948.050 s,,,,,[16646 Chirp K-J pairs] ,
0,,233152,0:16.463.141,,,,,,Capture stopped,[05/04/16 13:21:43]
# Total Phase Data Center(tm) v6.71
# (c) 2005-2016 Total Phase, Inc.
# www.totalphase.com
#
# 
#
# Level,Sp,Index,m:s.ms.us,Dur,Len,Err,Dev,Ep,Record,Summary
0,,0,0:00.000.000,,,,,,Capture started (Aggregate),[05/04/16 13:34:04]
0,,1,0:00.000.000,,,,,,<Host connected>,
0,,2,0:00.000.000,6.456.266.433 s,,,,,<Reset> / <Chirp J> / <Tiny J>,
0,,3,0:06.456.266,2.210.033 ms,,,,,<Chirp K>,
0,,4,0:06.458.476,1.183 us,,,,,<Reset> / <Chirp J> / <Tiny J>,
0,,5,0:06.458.477,47.703.383 ms,,,,,[476 Chirp K-J pairs] ,
0,,959,0:06.506.181,50 ns,,U,,,<Reset> / <Chirp K> / <Tiny K>,
0,,960,0:06.506.181,327.900 us,,,,,<Reset>,
0,HS,961,0:06.506.508,,,,,,<High-speed>,
0,HS,962,0:06.506.508,24.379.283 ms,,,,,[196 SOF],[Frames: 183.x - 207.4] 
0,HS,963,0:06.529.640,1.273.300 ms,18 B,,00,00,Get Device Descriptor,Index=0 Length=64
0,HS,977,0:06.531.013,83 ns,,,,,[1 SOF],[Frame: 207.5] 
0,HS,978,0:06.531.015,44.316 us,0 B,,00,00,Set Address,Address=02
0,HS,988,0:06.531.138,11.252.016 ms,,,,,[91 SOF],[Frames: 207.6 - 219.0] 
0,HS,989,0:06.541.142,1.276.100 ms,18 B,,02,00,Get Device Descriptor,Index=0 Length=18
0,HS,1003,0:06.542.515,125.100 us,,,,,[2 SOF],[Frames: 219.1 - 219.2] 
0,HS,1004,0:06.542.517,130.916 us,9 B,,02,00,Get Configuration Descriptor,Index=0 Length=9
0,HS,1018,0:06.542.765,250.116 us,,,,,[3 SOF],[Frames: 219.3 - 219.5] 
0,HS,1019,0:06.542.767,272.066 us,32 B,,02,00,Get Configuration Descriptor,Index=0 Length=32
0,HS,1033,0:06.543.140,66 ns,,,,,[1 SOF],[Frame: 219.6] 
0,HS,1034,0:06.543.142,41.983 us,0 B,,02,00,Set Configuration,Configuration=1
0,HS,1044,0:06.543.265,1.125.266 ms,,,,,[10 SOF],[Frames: 219.7 - 221.0] 
0,HS,1045,0:06.543.268,1.202.116 ms,4 B,,02,00,Get String Descriptor,Index=0 Length=255
0,HS,1059,0:06.544.515,1.125.266 ms,,,,,[10 SOF],[Frames: 221.1 - 222.2] 
0,HS,1060,0:06.544.517,1.225.550 ms,10 B,,02,00,Get String Descriptor,Index=1 Length=255
0,HS,1074,0:06.545.765,1.375.316 ms,,,,,[12 SOF],[Frames: 222.3 - 223.6] 
0,HS,1075,0:06.545.768,1.380.216 ms,34 B,,02,00,Get String Descriptor,Index=2 Length=255
0,HS,1089,0:06.547.266,1.250.300 ms,,,,,[11 SOF],[Frames: 223.7 - 225.1] 
0,HS,1090,0:06.547.268,1.291.800 ms,18 B,,02,00,Get String Descriptor,Index=3 Length=255
0,HS,1104,0:06.548.641,5.626.033 ms,,,,,[46 SOF],[Frames: 225.2 - 230.7] 
0,HS,1105,0:06.554.269,47.483 us,1 B,,02,00,Get Max LUN,Max LUN = 0
0,HS,1119,0:06.554.392,5.250.983 ms,,,,,[43 SOF],[Frames: 231.0 - 236.2] 
0,HS,1120,0:06.554.395,5.252.150 ms,36 B,,02,01,Inquiry [0],Passed
0,HS,1136,0:06.559.768,5.125.950 ms,,,,,[42 SOF],[Frames: 236.3 - 241.4] 
0,HS,1137,0:06.559.771,5.126.050 ms,,,02,01,Test Unit Ready [0],Failed
0,HS,1148,0:06.565.019,5.250.983 ms,,,,,[43 SOF],[Frames: 241.5 - 246.7] 
0,HS,1149,0:06.565.023,5.251.116 ms,18 B,,02,01,Request Sense [0],Sense Key = Unit Attention (Passed)
0,HS,1165,0:06.570.395,105.143.216 ms,,,,,[842 SOF],[Frames: 247.0 - 352.1] 
0,HS,1166,0:06.670.415,5.127.733 ms,,,02,01,Test Unit Ready [0],Passed
0,HS,1177,0:06.675.663,250.116 us,,,,,[3 SOF],[Frames: 352.2 - 352.4] 
0,HS,1178,0:06.675.666,250.083 us,8 B,,02,01,Read Capacity [0],Passed
0,HS,1195,0:06.676.038,5.250.966 ms,,,,,[43 SOF],[Frames: 352.5 - 357.7] 
0,HS,1196,0:06.676.041,5.251.100 ms,512 B,,02,01,Read [0],LBA = 0 Length = 1 block (Passed)
0,HS,1212,0:06.681.414,5.250.983 ms,,,,,[43 SOF],[Frames: 358.0 - 363.2] 
0,HS,1213,0:06.681.417,5.251.083 ms,512 B,,02,01,Read [0],LBA = 0 Length = 1 block (Passed)
0,HS,1229,0:06.686.790,1.999.970.183 s,,,,,[15998 SOF],[Frames: 363.3 - 315.0] [Periodic Timeout]
0,HS,1230,0:08.686.885,1.999.970.200 s,,,,,[15998 SOF],[Frames: 315.1 - 266.6] [Periodic Timeout]
0,HS,1231,0:10.686.980,1.999.970.216 s,,,,,[15998 SOF],[Frames: 266.7 - 218.4] [Periodic Timeout]
0,HS,1232,0:12.687.075,1.999.970.233 s,,,,,[15998 SOF],[Frames: 218.5 - 170.2] [Periodic Timeout]
0,HS,1233,0:14.687.170,1.999.970.233 s,,,,,[15998 SOF],[Frames: 170.3 - 122.0] [Periodic Timeout]
0,HS,1234,0:16.687.265,1.999.970.266 s,,,,,[15998 SOF],[Frames: 122.1 - 73.6] [Periodic Timeout]
0,HS,1235,0:18.687.361,1.999.970.266 s,,,,,[15998 SOF],[Frames: 73.7 - 25.4] [Periodic Timeout]
0,HS,1236,0:20.687.456,1.999.970.283 s,,,,,[15998 SOF],[Frames: 25.5 - 2025.2] [Periodic Timeout]
0,HS,1237,0:22.687.551,208.661.100 ms,,,,,[1670 SOF],[Frames: 2025.3 - 186.0] 
0,,1238,0:22.896.212,,,,,,Capture stopped,[05/04/16 13:34:28]
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to