[U-Boot] testing: [PATCH v7 0/3] common: usb_storage: Implement logic to calculate optimal usb maximum trasfer blocks

Marek Vasut marex at denx.de
Fri Jul 22 12:27:26 CEST 2016


On 07/22/2016 01:59 AM, Matthew Bright wrote:
> On 07/22/2016 01:24 AM, Rajesh Bhagat wrote:

Hi,

>>> -----Original Message-----
>>> From: Marek Vasut [mailto:marex at denx.de]
>>> Sent: Thursday, July 21, 2016 5:13 PM
>>> To: Rajesh Bhagat <rajesh.bhagat at nxp.com>; Matthew Bright
>>> <Matthew.Bright at alliedtelesis.co.nz>
>>> Cc: u-boot at lists.denx.de; Chris Packham <Chris.Packham at alliedtelesis.co.nz>; Mark
>>> Tomlinson <Mark.Tomlinson at alliedtelesis.co.nz>
>>> Subject: Re: testing: [PATCH v7 0/3] common: usb_storage: Implement logic to
>>> calculate optimal usb maximum trasfer blocks
>>>
>>> On 07/21/2016 10:08 AM, Rajesh Bhagat wrote:
>>>> Hello Marek,
>>>>
>>>> Any comments?
>>
>> Hello Marek, 
>>
>>>
>>> If I recall correctly, this broke things for Matthew.
>>> Is this resolved ?
>>>
>>
>> Let me try to explain the exact scenario, Matthew Please correct me if this is not the case:
>>
>> 1. Matthew performed some testing on some pen drives. 
>> 2. Matthew shared some review comments on the patch. 
>>
>> Regarding item#1, He is facing issue when some of the pen drives are __not__ recovering once
>> Y number blocks are sent, which is increased when X number of blocks passed (where Y > X). 
> 
> This is correct.
> 
> It appears that some usb storage devices can only be recovered by resetting the
> ehci interface. Unfortunately resetting the ehci interface part way through a
> usb transfer is not a practical solution.

So what is the solution here ?

> http://lists.denx.de/pipermail/u-boot/2016-June/258838.html
> 
>> This is true for some of the pen drives, though a large number of pen drives (which I also tested) 

Last time this large number was 3 or so, did that change ?

>> are passing which were not passing when MAX_XFER_BLKS was hardcoded. To add to it, the pen drives failing 
>> i.e. not recovering were not passing previously (prior to this patch) also. 
> 
> This is correct.
> 
> However I suggest that we can extent our test coverage to obtain a more
> representative sample. There are several other people who have reported
> being able to reproduce the same echi timeout issue.
> 
> http://lists.denx.de/pipermail/u-boot/2016-June/258905.html
> 
>> Hence, the new logic is making many pen drives better, and some pen drives which were not working
>> with previous logic are failing due to no recovery of HW with new logic also. 
> 
> This is correct.
> 
> This patch does not *NOT* introduce any regressions and provides a recovery
> mechanism that appears to work with the majority of usb storage devices.
> 
>> Regarding item#2, He provided some review comments which are valid, and suggested it to be applied on 
>> XHCI also which I tested after enabling CONFIG_DM_USB. Please refer below (my last comments). 
>>
>>>>> When both EHCI and XHCI are defined using CONFIG_DM_USB, the value of
>>>>> USB_MAX_XFER_BLK becomes 65535 according to current implementation.
>>>>> And is causing read/write issues in XHCI, though EHCI is working fine.
> 
> I was expecting to see a v8 patch based on the review comments.
> Not all comments were specific to the xhci interface.
> 
> http://lists.denx.de/pipermail/u-boot/2016-June/258839.html

OK, then V8 please.

>> IMHO, we should work on it to close it. Please share your opinion. 
> 
> Agreed.
> 
>> Best Regards,
>> Rajesh Bhagat 
>>>>> -----Original Message-----
>>>>> From: U-Boot [mailto:u-boot-bounces at lists.denx.de] On Behalf Of
>>>>> Rajesh Bhagat
>>>>> Sent: Tuesday, June 28, 2016 12:14 PM
>>>>> To: Matthew Bright <Matthew.Bright at alliedtelesis.co.nz>; Marek Vasut
>>>>> <marex at denx.de>
>>>>> Cc: u-boot at lists.denx.de; Chris Packham
>>>>> <Chris.Packham at alliedtelesis.co.nz>; Mark Tomlinson
>>>>> <Mark.Tomlinson at alliedtelesis.co.nz>
>>>>> Subject: Re: [U-Boot] testing: [PATCH v7 0/3] common: usb_storage:
>>>>> Implement logic to calculate optimal usb maximum trasfer blocks
>>>>>
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Matthew Bright [mailto:Matthew.Bright at alliedtelesis.co.nz]
>>>>>> Sent: Thursday, June 23, 2016 8:23 AM
>>>>>> To: Marek Vasut <marex at denx.de>; Rajesh Bhagat
>>>>>> <rajesh.bhagat at nxp.com>
>>>>>> Cc: u-boot at lists.denx.de; Chris Packham
>>>>>> <Chris.Packham at alliedtelesis.co.nz>; Mark Tomlinson
>>>>>> <Mark.Tomlinson at alliedtelesis.co.nz>
>>>>>> Subject: Re: testing: [PATCH v7 0/3] common: usb_storage: Implement
>>>>>> logic to calculate optimal usb maximum trasfer blocks
>>>>>>
>>>>>> On 06/23/2016 01:02 AM, Marek Vasut wrote:
>>>>>>>
>>>>>>> On 06/22/2016 08:36 AM, Rajesh Bhagat wrote:
>>>>>>>>
>>>>>>>> From: Matthew Bright [mailto:Matthew.Bright at alliedtelesis.co.nz]
>>>>>>>> Sent: Wednesday, June 22, 2016 11:42 AM
>>>>>>>> To: Rajesh Bhagat <rajesh.bhagat at nxp.com>; marex at denx.de
>>>>>>>> Cc: u-boot at lists.denx.de; Chris Packham
>>>>>>>> <Chris.Packham at alliedtelesis.co.nz>; Mark Tomlinson
>>>>>>>> <Mark.Tomlinson at alliedtelesis.co.nz>
>>>>>>>> Subject: testing: [PATCH v7 0/3] common: usb_storage: Implement
>>>>>>>> logic to calculate optimal usb maximum trasfer blocks
>>>>>>>>
>>>>>>>> On 06/16/2016 12:35 PM, Rajesh Bhagat wrote:
>>>>>>>>> Performs code cleanup by making common function for
>>>>>>>>> usb_stor_read/write and implements the logic to calculate the
>>>>>>>>> optimal usb maximum trasfer blocks instead of sending
>>>>>>>>> USB_MAX_XFER_BLK blocks which is 65535 and 20 in case of EHCI and
>>>>>>>>> other USB
>>>>>> protocols respectively.
>>>>>>>>>
>>>>>>>>> Rajesh Bhagat (3):
>>>>>>>>>   common: usb_storage: Make common function for
>>>>> usb_read_10/usb_write_10
>>>>>>>>>   common: usb_storage: Make common function for
>>>>>>>>>     usb_stor_read/usb_stor_write
>>>>>>>>>   common: usb_storage : Implement logic to calculate optimal usb
>>> maximum
>>>>>>>>>     trasfer blocks
>>>>>>>>>
>>>>>>>>>  common/usb_storage.c | 213 +++++++++++++++++++++++--------
>>> ---
>>>>> ---
>>>>>> --------------
>>>>>>>>>  include/usb.h        |   1 +
>>>>>>>>>  2 files changed, 98 insertions(+), 116 deletions(-)
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Hi Rajesh & Marek
>>>>>>>>>
>>>>>>>>> I have spend the last couple of days testing these patches on the
>>>>>>>>> v2016.05 release, with an usb mass storage device that is able to
>>>>>>>>> consistently reproduce the USB_MAX_XFER_BLK issue as described in
>>>>>>>>> the "Issue with USB mass storage (thumb drives)" u-boot thread.
>>>>>>>>>
>>>>>>>>> http://lists.denx.de/pipermail/u-boot/2016-February/244464.html​
>>>>>>>>>
>>>>>>>>
>>>>>>>> Hello Matt,
>>>>>>>>
>>>>>>>>> I can confirm the patch correctly increases the max transfer
>>>>>>>>> bocks after a successful read, and decreases the max transfer
>>>>>>>>> bocks after a read failure. However, I have noticed that once the
>>>>>>>>> ehci time out error occurs, the usb device appears to lock up.
>>>>>>>>> When in this state the usb device will stop responding to any
>>>>>>>>> further transfers. This behavior is independent of the number of
>>>>>>>>> blocks, and will continue until the ehci has been reset.
>>>>>>>>>
>>>>>>>>
>>>>>>>> I believe the lockup behavior mentioned by you to be device specific quirk.
>>>>>>>> I tested 3 pen drives, which recovered from EHCI timeout behavior
>>>>>>>> by reducing the number of blocks (check below output):
>>>>>>>>
>>>>>>>
>>>>>>> 3 devices is not a representative sample.
>>>>>>>
>>>>>>> --
>>>>>>> Best regards,
>>>>>>> Marek Vasut
>>>>>>
>>>>>> I agree.
>>>>>>
>>>>>> Several others on the u-boot threads have also reported the same
>>>>>> ehci time out issue related to the max transfer blocks. Perhaps we
>>>>>> could kindly ask if they could also test the patch ...
>>>>>>
>>>>>> Schrempf Frieder
>>>>>> http://lists.denx.de/pipermail/u-boot/2016-February/244464.html
>>>>>> http://lists.denx.de/pipermail/u-boot/2016-February/245893.html
>>>>>>
>>>>>> Hannes Schmelzer (hannes at schmelzer.or.at)
>>>>>> http://lists.denx.de/pipermail/u- boot/2016-February/244621.html
>>>>>>
>>>>>> Maxime Jayat
>>>>>> http://lists.denx.de/pipermail/u-boot/2016-February/246267.html
>>>>>>
>>>>>> Diego
>>>>>> http://lists.denx.de/pipermail/u-boot/2016-April/251799.html
>>>>>>
>>>>>> Nicolas Chauvet
>>>>>> http://lists.denx.de/pipermail/u-boot/2016-May/256117.html
>>>>>>
>>>>>> As a side note, there appears to be a subtle a difference in the
>>>>>> output when the usb device locks up:
>>>>>>
>>>>>> CASE 1: EHCI Time Out - Device Remains Responsive:
>>>>>>
>>>>>> => fatload usb 0 0x18000000 test.file EHCI timed out on TD -
>>>>>> token=0x2c008d80 EHCI timed out on TD -
>>>>>> token=0xac008d80 EHCI timed out on TD - token=0xac008d80 Error
>>>>>> reading cluster
>>>>>> ** Unable to read file test.file **
>>>>>> =>
>>>>>>
>>>>>> The three time out errors are caused by three attempts to send the
>>>>>> over-sized transfer before giving up.
>>>>>>
>>>>>> CASE 2: EHCI Time Out - Device Locks Up:
>>>>>>
>>>>>> => fatload usb 0:auto 0x1000000 test.rel reading test.rel EHCI timed
>>>>>> out on TD -
>>>>>> token=0x26008d80 EHCI timed out on TD - token=0x80008d80 EHCI timed
>>>>>> out on TD
>>>>>> - token=0x80008d80 EHCI timed out on TD - token=0x80008d80 EHCI
>>>>>> timed out on TD - token=0x80008d80 EHCI timed out on TD -
>>>>>> token=0x80008d80 EHCI timed out on TD - token=0x80008d80 EHCI timed
>>>>>> out on TD -
>>>>>> token=0x80008d80 EHCI timed out on TD - token=0x80008d80 EHCI timed
>>>>>> out on TD - token=0x801f8c80 EHCI timed out on TD - token=0x80008d80
>>>>>> EHCI timed out on TD - token=0x80008d80 EHCI timed out on TD -
>>>>>> token=0x80008d80 EHCI timed out on TD - token=0x801f8c80 EHCI timed
>>>>>> out on TD - token=0x80008d80 EHCI timed out on TD - token=0x80008d80
>>>>>> EHCI timed out on TD - token=0x80008d80 EHCI timed out on TD -
>>>>>> token=0x801f8c80 EHCI timed out on TD - token=0x80008d80 EHCI timed
>>>>>> out on TD -
>>>>>> token=0x80008d80 EHCI timed out on TD - token=0x80008d80 EHCI timed
>>>>>> out on TD
>>>>>> - token=0x801f8c80 EHCI timed out on TD - token=0x80008d80 EHCI
>>>>>> timed out on TD
>>>>>> - token=0x80008d80 EHCI timed out on TD - token=0x80008d80 Error
>>>>>> reading cluster
>>>>>> ** Unable to read file test.rel **
>>>>>> =>
>>>>>>
>>>>>> The additional time out errors are caused because the usb device
>>>>>> also fails to respond to several reset messages after the initial
>>>>>> time out; therefore providing a clear indication that the device has locked up.
>>>>>>
>>>>>> The usb device in the initial thread appears to exhibit such behavior:
>>>>>> http://lists.denx.de/pipermail/u-boot/2016-February/244464.html
>>>>>>
>>>>>
>>>>> Hello Matt/Marek,
>>>>>
>>>>> I'm working on enabling CONFIG_DM_USB for EHCI and XHCI controllers
>>>>> and found one more issue which this patch is solving.
>>>>>
>>>>> When both EHCI and XHCI are defined using CONFIG_DM_USB, the value of
>>>>> USB_MAX_XFER_BLK becomes 65535 according to current implementation.
>>>>> And is causing read/write issues in XHCI, though EHCI is working fine.
>>>>> (Please check below logs).
>>>>>
>>>>> Hence, extending this patch from EHCI to XHCI is very much required
>>>>> as pointed out by Matt. When I apply this patch after extending it
>>>>> for XHCI also, below problem is solved (check observations below).
>>>>>
>>>>>
>>>>> Observations:
>>>>> 1. "usb start" correctly detected two devices one in high speed and
>>>>> one in super speed on EHCI and XHCI controller respectively .
>>>>>
>>>>> => usb start
>>>>> starting USB...
>>>>> USB0:   USB EHCI 1.00
>>>>> USB1:   Register 200017f NbrPorts 2
>>>>> Starting the controller
>>>>> USB XHCI 1.00
>>>>> scanning bus 0 for devices... 2 USB Device(s) found scanning bus 1
>>>>> for devices... 2 USB Device(s) found
>>>>>
>>>>> 2. Read/write on high speed device on EHCI port worked fine.
>>>>>
>>>>> => usb dev 0
>>>>>
>>>>> USB device 0:
>>>>>     Device 0: Vendor: SanDisk  Rev: 1.26 Prod: Cruzer Blade
>>>>>             Type: Hard Disk
>>>>>             Capacity: 7633.5 MB = 7.4 GB (15633408 x 512) ... is now
>>>>> current device => mw 81000000 55555555 800000; mw a0000000 aaaaaaaa
>>>>> 800000; usb write
>>>>> a0000000 0 10000; usb read 81000000 0 10000; cmp.b a0000000 81000000
>>>>> 2000000;
>>>>>
>>>>> USB write: device 0 block # 0, count 65536 ... 65536 blocks write: OK
>>>>>
>>>>> USB read: device 0 block # 0, count 65536 ... 65536 blocks read: OK
>>>>> Total of
>>>>> 33554432 byte(s) were the same
>>>>>
>>>>> 3. Read/write on super speed device on XHCI port, I'm getting below
>>>>> error
>>>>>
>>>>> => usb dev 1
>>>>> USB device 1:
>>>>>     Device 1: Vendor: JetFlash Rev: 1100 Prod: Transcend 16GB
>>>>>             Type: Removable Hard Disk
>>>>>             Capacity: 15360.0 MB = 15.0 GB (31457280 x 512) ... is
>>>>> now current device => mw 81000000 55555555 800000; mw a0000000
>>>>> aaaaaaaa 800000; usb write
>>>>> a0000000 0 10000; usb read 81000000 0 10000; cmp.b a0000000 81000000
>>>>> 2000000;
>>>>>
>>>>> USB write: device 1 block # 0, count 65536 ... WARN halted endpoint,
>>>>> queueing URB anyway.
>>>>> Unexpected XHCI event TRB, skipping... (fef27250 00000000 13000000
>>>>> 01008401)
>>>>> BUG: failure at drivers/usb/host/xhci-ring.c:489/abort_td()!
>>>>> BUG!
>>>>>
>>>>>
>>>>> Best Regards,
>>>>> Rajesh Bhagat
>>>>>
>>>>>> Cheers.
>>>>>> - Matt Bright
>>>>> _______________________________________________
>>>>> U-Boot mailing list
>>>>> U-Boot at lists.denx.de
>>>>> http://lists.denx.de/mailman/listinfo/u-boot
>>>>
>>>> Best Regards,
>>>> Rajesh Bhagat
>>>>
>>>
>>>
>>> --
>>> Best regards,
>>> Marek Vasut


-- 
Best regards,
Marek Vasut


More information about the U-Boot mailing list