public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] testing: [PATCH v7 0/3] common: usb_storage: Implement logic to calculate optimal usb maximum trasfer blocks
@ 2016-06-22  6:12 Matthew Bright
  2016-06-22  6:36 ` Rajesh Bhagat
  0 siblings, 1 reply; 10+ messages in thread
From: Matthew Bright @ 2016-06-22  6:12 UTC (permalink / raw)
  To: u-boot

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?

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.

The following debug output demonstrates this behavior:

(note: the retry attempts has been set to 4)

----------------------------------------------------------------------

=> fatload usb 0:auto 0x1000000 test.rel

------
(where test.rel is dd if=/dev/urandom of=test.rel bs=32M count=2)
------

usb_stor_read_write: udev 0

usb_stor_read_write: dev 0 startblk 0, blccnt 1 buffer 7fb0d980
usb_stor_read_write: retry #4, cur_xfer_blks 4095, smallblks 1
usb_read_write_10: start 0 blocks 1
usb_stor_read_write: end startblk 1, blccnt 1 buffer 7fb0db80

------
(40 further transfers occur with blccnt bewteen 1 and 8 blocks)
------

usb_stor_read_write: udev 0

usb_stor_read_write: dev 0 startblk 169a0, blccnt 20000 buffer 1000000
usb_stor_read_write: retry #4, cur_xfer_blks 4095, smallblks 4095
usb_read_write_10: start 169a0 blocks 4095
usb_stor_read_write: retry #4, cur_xfer_blks 8191, smallblks 8191
usb_read_write_10: start 1799f blocks 8191
usb_stor_read_write: retry #4, cur_xfer_blks 16383, smallblks 16383
usb_read_write_10: start 1999e blocks 16383
usb_stor_read_write: retry #4, cur_xfer_blks 32767, smallblks 32767
usb_read_write_10: start 1d99d blocks 32767
usb_stor_read_write: retry #4, cur_xfer_blks 65535, smallblks 65535
usb_read_write_10: start 2599c blocks 65535
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
usb_stor_read_write: retry #3, cur_xfer_blks 32767, smallblks 32767
usb_read_write_10: start 2599c blocks 32767
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
usb_stor_read_write: retry #2, cur_xfer_blks 16383, smallblks 16383
usb_read_write_10: start 2599c blocks 16383
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
usb_stor_read_write: retry #1, cur_xfer_blks 8191, smallblks 8191
usb_read_write_10: start 2599c blocks 8191
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
usb_stor_read_write: retry #0, cur_xfer_blks 4095, smallblks 4095
usb_read_write_10: start 2599c blocks 4095
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
usb_stor_read_write: end startblk 2599c, blccnt fff buffer 2dff800
Error reading cluster
** Unable to read file test.rel **
=> fatload usb 0:auto 0x1000000 test.rel

usb_stor_read_write: udev 0

usb_stor_read_write: dev 0 startblk 0, blccnt 1 buffer 7fb0d980
usb_stor_read_write: retry #4, cur_xfer_blks 4095, smallblks 1
usb_read_write_10: start 0 blocks 1
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
usb_stor_read_write: retry #3, cur_xfer_blks 4095, smallblks 1
usb_read_write_10: start 0 blocks 1
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
usb_stor_read_write: retry #2, cur_xfer_blks 4095, smallblks 1
usb_read_write_10: start 0 blocks 1
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
usb_stor_read_write: retry #1, cur_xfer_blks 4095, smallblks 1
usb_read_write_10: start 0 blocks 1
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
usb_stor_read_write: retry #0, cur_xfer_blks 4095, smallblks 1
usb_read_write_10: start 0 blocks 1
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
usb_stor_read_write: end startblk 0, blccnt 1 buffer 7fb0d980
** Can't read partition table on 0:0 **

------
(usb_read_write_10 is repeated 15 more times producing the same errors)
------

** No valid partitions found **
=> usb reset
resetting USB...
USB0:   USB EHCI 1.00
scanning bus 0 for devices... 2 USB Device(s) found
       scanning usb for storage devices...
usb_stor_read_write: udev 0

usb_stor_read_write: dev 0 startblk 0, blccnt 1 buffer 7fb0d980
usb_stor_read_write: retry #4, cur_xfer_blks 4095, smallblks 1
usb_read_write_10: start 0 blocks 1
usb_stor_read_write: end startblk 1, blccnt 1 buffer 7fb0db80
1 Storage Device(s) found
=> fatload usb 0:auto 0x1000000 test.rel

usb_stor_read_write: udev 0

usb_stor_read_write: dev 0 startblk 0, blccnt 1 buffer 7fb0d980
usb_stor_read_write: retry #4, cur_xfer_blks 4095, smallblks 1
usb_read_write_10: start 0 blocks 1
usb_stor_read_write: end startblk 1, blccnt 1 buffer 7fb0db80

------
(transfers continues on happily until 65535 blocks is hit again)
------

----------------------------------------------------------------------

This debug output shows the ehci timeout error occur for a transfer
with 65535 blocks (as expected) however the timeout error continues
for each proceeding transfer. This occurs even though the number of
blocks are reduced back to what should be an acceptable number.

I am currently unsure if this "lock up" behavior is simply a quirk of
the specific usb device that I am using. However, if such behavior is
common, then it could render this fix moot. This is assuming that the
ehci cannot be reset part way through a transfer (i.e. after the echi
time out error occurs).

I have been able to consistently reproduce this "lock up" behavior on
a single 4-GB Apacer usb flash device (details below). However I have
been unable to reproduce this behavior on several 8GB counterparts as
they appear to work fine with the maximum possible block size.

----------------------------------------------------------------------

=> usb info
1: Hub,  USB Revision 2.0
 - u-boot EHCI Host Controller
 - Class: Hub
 - PacketSize: 64  Configurations: 1
 - Vendor: 0x0000  Product 0x0000 Version 1.0
   Configuration: 1
   - Interfaces: 1 Self Powered 0mA
     Interface: 0
     - Alternate Setting 0, Endpoints: 1
     - Class Hub
     - Endpoint 1 In Interrupt MaxPacket 2048 Interval 255ms

2: Mass Storage,  USB Revision 2.0
 - USB FLASH DRIVE AP9BDE1B0918CBDB
 - Class: (from Interface) Mass Storage
 - PacketSize: 64  Configurations: 1
 - Vendor: 0x1005  Product 0xb113 Version 1.0
   Configuration: 1
   - Interfaces: 1 Bus Powered 500mA
     Interface: 0
     - Alternate Setting 0, Endpoints: 2
     - Class Mass Storage, Transp. SCSI, Bulk only
     - Endpoint 2 In Bulk MaxPacket 512
     - Endpoint 1 Out Bulk MaxPacket 512

----------------------------------------------------------------------

I think this patch is a good solution to the original problem. Please
let me know if there is anything I can do to help.

Cheers.
- Matt Bright

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] testing: [PATCH v7 0/3] common: usb_storage: Implement logic to calculate optimal usb maximum trasfer blocks
  2016-06-22  6:12 [U-Boot] testing: [PATCH v7 0/3] common: usb_storage: Implement logic to calculate optimal usb maximum trasfer blocks Matthew Bright
@ 2016-06-22  6:36 ` Rajesh Bhagat
  2016-06-22 23:02   ` Marek Vasut
  0 siblings, 1 reply; 10+ messages in thread
From: Rajesh Bhagat @ 2016-06-22  6:36 UTC (permalink / raw)
  To: u-boot



From: Matthew Bright [mailto:Matthew.Bright at alliedtelesis.co.nz] 
Sent: Wednesday, June 22, 2016 11:42 AM
To: Rajesh Bhagat <rajesh.bhagat@nxp.com>; marex at denx.de
Cc: u-boot at lists.denx.de; Chris Packham <Chris.Packham@alliedtelesis.co.nz>; Mark Tomlinson <Mark.Tomlinson@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): 

USB write: device 0 block # 0, count 524288 ... 
usb_stor_read_write: retry #2, cur_xfer_blks 4095, smallblks 4095
usb_stor_read_write: retry #2, cur_xfer_blks 8191, smallblks 8191
usb_stor_read_write: retry #2, cur_xfer_blks 16383, smallblks 16383
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 65535, smallblks 65535
EHCI timed out on TD - token=0xa6008c80
usb_stor_read_write: retry #1, cur_xfer_blks 32767, smallblks 32767 <== 
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 4114
524288 blocks write: OK

USB read: device 0 block # 0, count 524288 ... 
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 65535, smallblks 65535
EHCI timed out on TD - token=0x2c008d80
usb_stor_read_write: retry #1, cur_xfer_blks 32767, smallblks 32767 <== 
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 32767
usb_stor_read_write: retry #2, cur_xfer_blks 32767, smallblks 16
524288 blocks read: OK

Best Regards,
Rajesh Bhagat 

> The following debug output demonstrates this behavior:
>
> (note: the retry attempts has been set to 4)
>
> ----------------------------------------------------------------------
>
> => fatload usb 0:auto 0x1000000 test.rel
>
> ------
> (where test.rel is dd if=/dev/urandom of=test.rel bs=32M count=2)
> ------
>
> usb_stor_read_write: udev 0
>
> usb_stor_read_write: dev 0 startblk 0, blccnt 1 buffer 7fb0d980
> usb_stor_read_write: retry #4, cur_xfer_blks 4095, smallblks 1
> usb_read_write_10: start 0 blocks 1
> usb_stor_read_write: end startblk 1, blccnt 1 buffer 7fb0db80
>
> ------
> (40 further transfers occur with blccnt bewteen 1 and 8 blocks)
> ------
>
> usb_stor_read_write: udev 0
>
> usb_stor_read_write: dev 0 startblk 169a0, blccnt 20000 buffer 1000000
> usb_stor_read_write: retry #4, cur_xfer_blks 4095, smallblks 4095
> usb_read_write_10: start 169a0 blocks 4095
> usb_stor_read_write: retry #4, cur_xfer_blks 8191, smallblks 8191
> usb_read_write_10: start 1799f blocks 8191
> usb_stor_read_write: retry #4, cur_xfer_blks 16383, smallblks 16383
> usb_read_write_10: start 1999e blocks 16383
> usb_stor_read_write: retry #4, cur_xfer_blks 32767, smallblks 32767
> usb_read_write_10: start 1d99d blocks 32767
> usb_stor_read_write: retry #4, cur_xfer_blks 65535, smallblks 65535
> usb_read_write_10: start 2599c blocks 65535
> 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
> usb_stor_read_write: retry #3, cur_xfer_blks 32767, smallblks 32767
> usb_read_write_10: start 2599c blocks 32767
> 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
> usb_stor_read_write: retry #2, cur_xfer_blks 16383, smallblks 16383
> usb_read_write_10: start 2599c blocks 16383
> 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
> usb_stor_read_write: retry #1, cur_xfer_blks 8191, smallblks 8191
> usb_read_write_10: start 2599c blocks 8191
> 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
> usb_stor_read_write: retry #0, cur_xfer_blks 4095, smallblks 4095
> usb_read_write_10: start 2599c blocks 4095
> 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
> usb_stor_read_write: end startblk 2599c, blccnt fff buffer 2dff800
> Error reading cluster
> ** Unable to read file test.rel **
> => fatload usb 0:auto 0x1000000 test.rel
>
> usb_stor_read_write: udev 0
>
> sb_stor_read_write: dev 0 startblk 0, blccnt 1 buffer 7fb0d980
> usb_stor_read_write: retry #4, cur_xfer_blks 4095, smallblks 1
> usb_read_write_10: start 0 blocks 1
> 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
> usb_stor_read_write: retry #3, cur_xfer_blks 4095, smallblks 1
> usb_read_write_10: start 0 blocks 1
> 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
> usb_stor_read_write: retry #2, cur_xfer_blks 4095, smallblks 1
> usb_read_write_10: start 0 blocks 1
> 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
> usb_stor_read_write: retry #1, cur_xfer_blks 4095, smallblks 1
> usb_read_write_10: start 0 blocks 1
> 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
> usb_stor_read_write: retry #0, cur_xfer_blks 4095, smallblks 1
> usb_read_write_10: start 0 blocks 1
> 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
> usb_stor_read_write: end startblk 0, blccnt 1 buffer 7fb0d980
> ** Can't read partition table on 0:0 **
>
> ------
> (usb_read_write_10 is repeated 15 more times producing the same errors)
> ------
>
> ** No valid partitions found **
> => usb reset
> resetting USB...
> USB0: ? USB EHCI 1.00
> scanning bus 0 for devices... 2 USB Device(s) found
>? ? ? ?scanning usb for storage devices...?
> usb_stor_read_write: udev 0
>
> usb_stor_read_write: dev 0 startblk 0, blccnt 1 buffer 7fb0d980
> usb_stor_read_write: retry #4, cur_xfer_blks 4095, smallblks 1
> usb_read_write_10: start 0 blocks 1
> usb_stor_read_write: end startblk 1, blccnt 1 buffer 7fb0db80
> 1 Storage Device(s) found
> => fatload usb 0:auto 0x1000000 test.rel
>
> usb_stor_read_write: udev 0
>
> usb_stor_read_write: dev 0 startblk 0, blccnt 1 buffer 7fb0d980
> usb_stor_read_write: retry #4, cur_xfer_blks 4095, smallblks 1
> usb_read_write_10: start 0 blocks 1
> usb_stor_read_write: end startblk 1, blccnt 1 buffer 7fb0db80
>
> ------
> (transfers continues on happily until 65535 blocks is hit again)
> ------
>
> ----------------------------------------------------------------------
>
> This debug output shows the ehci timeout error occur for a transfer
> with 65535 blocks (as expected) however the timeout error continues
> for each proceeding transfer. This occurs even though the number of
> blocks are reduced back to what should be an acceptable number.?
>
> I am currently unsure if this "lock up" behavior is simply a quirk of
> the specific usb device that I am using. However, if such behavior is
> common, then it could render this fix moot. This is assuming that the
> ehci cannot be reset part way through a transfer (i.e. after the echi
> time out error occurs).
>
> I have been able to consistently reproduce this "lock up" behavior on
> a single 4-GB Apacer usb flash device (details below). However I have
> been unable to reproduce this behavior on several 8GB counterparts as
> they appear to work fine with the maximum possible block size.
>
> ----------------------------------------------------------------------
>
> => usb info
> 1: Hub, ?USB Revision 2.0
>?- u-boot EHCI Host Controller?
>?- Class: Hub
>?- PacketSize: 64 ?Configurations: 1
>?- Vendor: 0x0000 ?Product 0x0000 Version 1.0
>? ?Configuration: 1
>? ?- Interfaces: 1 Self Powered 0mA
>? ? ?Interface: 0
>? ? ?- Alternate Setting 0, Endpoints: 1
>? ? ?- Class Hub
>? ? ?- Endpoint 1 In Interrupt MaxPacket 2048 Interval 255ms
>
> 2: Mass Storage, ?USB Revision 2.0
>?- USB FLASH DRIVE AP9BDE1B0918CBDB
>?- Class: (from Interface) Mass Storage
>?- PacketSize: 64 ?Configurations: 1
>?- Vendor: 0x1005 ?Product 0xb113 Version 1.0
>? ?Configuration: 1
>? ?- Interfaces: 1 Bus Powered 500mA
>? ? ?Interface: 0
>? ? ?- Alternate Setting 0, Endpoints: 2
>? ? ?- Class Mass Storage, Transp. SCSI, Bulk only
>? ? ?- Endpoint 2 In Bulk MaxPacket 512
>? ? ?- Endpoint 1 Out Bulk MaxPacket 512
>
> ----------------------------------------------------------------------
>
> I think this patch is a good solution to the original problem. Please
> let me know if there is anything I can do to help.
>
> Cheers.
> - Matt Bright

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] testing: [PATCH v7 0/3] common: usb_storage: Implement logic to calculate optimal usb maximum trasfer blocks
  2016-06-22  6:36 ` Rajesh Bhagat
@ 2016-06-22 23:02   ` Marek Vasut
  2016-06-23  2:52     ` Matthew Bright
  0 siblings, 1 reply; 10+ messages in thread
From: Marek Vasut @ 2016-06-22 23:02 UTC (permalink / raw)
  To: u-boot

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@nxp.com>; marex at denx.de
> Cc: u-boot at lists.denx.de; Chris Packham <Chris.Packham@alliedtelesis.co.nz>; Mark Tomlinson <Mark.Tomlinson@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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] testing: [PATCH v7 0/3] common: usb_storage: Implement logic to calculate optimal usb maximum trasfer blocks
  2016-06-22 23:02   ` Marek Vasut
@ 2016-06-23  2:52     ` Matthew Bright
  2016-06-28  6:44       ` Rajesh Bhagat
  0 siblings, 1 reply; 10+ messages in thread
From: Matthew Bright @ 2016-06-23  2:52 UTC (permalink / raw)
  To: u-boot

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@nxp.com>; marex at denx.de
>> Cc: u-boot at lists.denx.de; Chris Packham <Chris.Packham@alliedtelesis.co.nz>; Mark Tomlinson <Mark.Tomlinson@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

Cheers.
- Matt Bright

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] testing: [PATCH v7 0/3] common: usb_storage: Implement logic to calculate optimal usb maximum trasfer blocks
  2016-06-23  2:52     ` Matthew Bright
@ 2016-06-28  6:44       ` Rajesh Bhagat
  2016-07-21  8:08         ` Rajesh Bhagat
  0 siblings, 1 reply; 10+ messages in thread
From: Rajesh Bhagat @ 2016-06-28  6:44 UTC (permalink / raw)
  To: u-boot



> -----Original Message-----
> From: Matthew Bright [mailto:Matthew.Bright at alliedtelesis.co.nz]
> Sent: Thursday, June 23, 2016 8:23 AM
> To: Marek Vasut <marex@denx.de>; Rajesh Bhagat <rajesh.bhagat@nxp.com>
> Cc: u-boot at lists.denx.de; Chris Packham <Chris.Packham@alliedtelesis.co.nz>; Mark
> Tomlinson <Mark.Tomlinson@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@nxp.com>; marex at denx.de
> >> Cc: u-boot at lists.denx.de; Chris Packham
> >> <Chris.Packham@alliedtelesis.co.nz>; Mark Tomlinson
> >> <Mark.Tomlinson@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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] testing: [PATCH v7 0/3] common: usb_storage: Implement logic to calculate optimal usb maximum trasfer blocks
  2016-06-28  6:44       ` Rajesh Bhagat
@ 2016-07-21  8:08         ` Rajesh Bhagat
  2016-07-21 11:43           ` Marek Vasut
  0 siblings, 1 reply; 10+ messages in thread
From: Rajesh Bhagat @ 2016-07-21  8:08 UTC (permalink / raw)
  To: u-boot

Hello Marek, 

Any comments? 

> -----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@alliedtelesis.co.nz>; Marek Vasut
> <marex@denx.de>
> Cc: u-boot at lists.denx.de; Chris Packham <Chris.Packham@alliedtelesis.co.nz>; Mark
> Tomlinson <Mark.Tomlinson@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@denx.de>; Rajesh Bhagat <rajesh.bhagat@nxp.com>
> > Cc: u-boot at lists.denx.de; Chris Packham
> > <Chris.Packham@alliedtelesis.co.nz>; Mark Tomlinson
> > <Mark.Tomlinson@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@nxp.com>; marex at denx.de
> > >> Cc: u-boot at lists.denx.de; Chris Packham
> > >> <Chris.Packham@alliedtelesis.co.nz>; Mark Tomlinson
> > >> <Mark.Tomlinson@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 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] testing: [PATCH v7 0/3] common: usb_storage: Implement logic to calculate optimal usb maximum trasfer blocks
  2016-07-21  8:08         ` Rajesh Bhagat
@ 2016-07-21 11:43           ` Marek Vasut
  2016-07-21 13:09             ` Rajesh Bhagat
  0 siblings, 1 reply; 10+ messages in thread
From: Marek Vasut @ 2016-07-21 11:43 UTC (permalink / raw)
  To: u-boot

On 07/21/2016 10:08 AM, Rajesh Bhagat wrote:
> Hello Marek, 
> 
> Any comments? 

If I recall correctly, this broke things for Matthew.
Is this resolved ?

>> -----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@alliedtelesis.co.nz>; Marek Vasut
>> <marex@denx.de>
>> Cc: u-boot at lists.denx.de; Chris Packham <Chris.Packham@alliedtelesis.co.nz>; Mark
>> Tomlinson <Mark.Tomlinson@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@denx.de>; Rajesh Bhagat <rajesh.bhagat@nxp.com>
>>> Cc: u-boot at lists.denx.de; Chris Packham
>>> <Chris.Packham@alliedtelesis.co.nz>; Mark Tomlinson
>>> <Mark.Tomlinson@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@nxp.com>; marex at denx.de
>>>>> Cc: u-boot at lists.denx.de; Chris Packham
>>>>> <Chris.Packham@alliedtelesis.co.nz>; Mark Tomlinson
>>>>> <Mark.Tomlinson@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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] testing: [PATCH v7 0/3] common: usb_storage: Implement logic to calculate optimal usb maximum trasfer blocks
  2016-07-21 11:43           ` Marek Vasut
@ 2016-07-21 13:09             ` Rajesh Bhagat
  2016-07-21 23:59               ` Matthew Bright
  0 siblings, 1 reply; 10+ messages in thread
From: Rajesh Bhagat @ 2016-07-21 13:09 UTC (permalink / raw)
  To: u-boot



> -----Original Message-----
> From: Marek Vasut [mailto:marex at denx.de]
> Sent: Thursday, July 21, 2016 5:13 PM
> To: Rajesh Bhagat <rajesh.bhagat@nxp.com>; Matthew Bright
> <Matthew.Bright@alliedtelesis.co.nz>
> Cc: u-boot at lists.denx.de; Chris Packham <Chris.Packham@alliedtelesis.co.nz>; Mark
> Tomlinson <Mark.Tomlinson@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@alliedtelesis.co.nz>; Marek Vasut
> >> <marex@denx.de>
> >> Cc: u-boot at lists.denx.de; Chris Packham
> >> <Chris.Packham@alliedtelesis.co.nz>; Mark Tomlinson
> >> <Mark.Tomlinson@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@denx.de>; Rajesh Bhagat
> >>> <rajesh.bhagat@nxp.com>
> >>> Cc: u-boot at lists.denx.de; Chris Packham
> >>> <Chris.Packham@alliedtelesis.co.nz>; Mark Tomlinson
> >>> <Mark.Tomlinson@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@nxp.com>; marex at denx.de
> >>>>> Cc: u-boot at lists.denx.de; Chris Packham
> >>>>> <Chris.Packham@alliedtelesis.co.nz>; Mark Tomlinson
> >>>>> <Mark.Tomlinson@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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] testing: [PATCH v7 0/3] common: usb_storage: Implement logic to calculate optimal usb maximum trasfer blocks
  2016-07-21 13:09             ` Rajesh Bhagat
@ 2016-07-21 23:59               ` Matthew Bright
  2016-07-22 10:27                 ` Marek Vasut
  0 siblings, 1 reply; 10+ messages in thread
From: Matthew Bright @ 2016-07-21 23:59 UTC (permalink / raw)
  To: u-boot

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@nxp.com>; Matthew Bright
>> <Matthew.Bright@alliedtelesis.co.nz>
>> Cc: u-boot at lists.denx.de; Chris Packham <Chris.Packham@alliedtelesis.co.nz>; Mark
>> Tomlinson <Mark.Tomlinson@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@alliedtelesis.co.nz>; Marek Vasut
>> >> <marex@denx.de>
>> >> Cc: u-boot at lists.denx.de; Chris Packham
>> >> <Chris.Packham@alliedtelesis.co.nz>; Mark Tomlinson
>> >> <Mark.Tomlinson@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@denx.de>; Rajesh Bhagat
>> >>> <rajesh.bhagat@nxp.com>
>> >>> Cc: u-boot at lists.denx.de; Chris Packham
>> >>> <Chris.Packham@alliedtelesis.co.nz>; Mark Tomlinson
>> >>> <Mark.Tomlinson@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@nxp.com>; marex at denx.de
>> >>>>> Cc: u-boot at lists.denx.de; Chris Packham
>> >>>>> <Chris.Packham@alliedtelesis.co.nz>; Mark Tomlinson
>> >>>>> <Mark.Tomlinson@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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [U-Boot] testing: [PATCH v7 0/3] common: usb_storage: Implement logic to calculate optimal usb maximum trasfer blocks
  2016-07-21 23:59               ` Matthew Bright
@ 2016-07-22 10:27                 ` Marek Vasut
  0 siblings, 0 replies; 10+ messages in thread
From: Marek Vasut @ 2016-07-22 10:27 UTC (permalink / raw)
  To: u-boot

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@nxp.com>; Matthew Bright
>>> <Matthew.Bright@alliedtelesis.co.nz>
>>> Cc: u-boot at lists.denx.de; Chris Packham <Chris.Packham@alliedtelesis.co.nz>; Mark
>>> Tomlinson <Mark.Tomlinson@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@alliedtelesis.co.nz>; Marek Vasut
>>>>> <marex@denx.de>
>>>>> Cc: u-boot at lists.denx.de; Chris Packham
>>>>> <Chris.Packham@alliedtelesis.co.nz>; Mark Tomlinson
>>>>> <Mark.Tomlinson@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@denx.de>; Rajesh Bhagat
>>>>>> <rajesh.bhagat@nxp.com>
>>>>>> Cc: u-boot at lists.denx.de; Chris Packham
>>>>>> <Chris.Packham@alliedtelesis.co.nz>; Mark Tomlinson
>>>>>> <Mark.Tomlinson@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@nxp.com>; marex at denx.de
>>>>>>>> Cc: u-boot at lists.denx.de; Chris Packham
>>>>>>>> <Chris.Packham@alliedtelesis.co.nz>; Mark Tomlinson
>>>>>>>> <Mark.Tomlinson@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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-07-22 10:27 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-22  6:12 [U-Boot] testing: [PATCH v7 0/3] common: usb_storage: Implement logic to calculate optimal usb maximum trasfer blocks Matthew Bright
2016-06-22  6:36 ` Rajesh Bhagat
2016-06-22 23:02   ` Marek Vasut
2016-06-23  2:52     ` Matthew Bright
2016-06-28  6:44       ` Rajesh Bhagat
2016-07-21  8:08         ` Rajesh Bhagat
2016-07-21 11:43           ` Marek Vasut
2016-07-21 13:09             ` Rajesh Bhagat
2016-07-21 23:59               ` Matthew Bright
2016-07-22 10:27                 ` Marek Vasut

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox