[U-Boot] testing: [PATCH v7 0/3] common: usb_storage: Implement logic to calculate optimal usb maximum trasfer blocks
Matthew Bright
Matthew.Bright at alliedtelesis.co.nz
Fri Jul 22 01:59:45 CEST 2016
On 07/22/2016 01:24 AM, Rajesh Bhagat wrote:
>
>> -----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.
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)
> 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
> 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
More information about the U-Boot
mailing list