[U-Boot] testing: [PATCH v7 0/3] common: usb_storage: Implement logic to calculate optimal usb maximum trasfer blocks
Rajesh Bhagat
rajesh.bhagat at nxp.com
Thu Jul 21 15:09:25 CEST 2016
> -----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 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.
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.
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.
IMHO, we should work on it to close it. Please share your opinion.
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