[U-Boot] [RFC PATCH v3] usb: limit USB_MAX_XFER_BLK to 256

[Marek Vasut]

On 4/18/19 1:45 AM, Marcel Ziswiler wrote:
> From: Peng Fan <peng.fan at nxp.com>
> 
> For Some USB mass storage devices, such as:
> "
>  - Kingston DataTraveler 2.0 001D7D06CF09B04199C7B3EA
>  - Class: (from Interface) Mass Storage
>  - PacketSize: 64  Configurations: 1
>  - Vendor: 0x0930  Product 0x6545 Version 1.16
> "
> When `usb read 0x80000000 0 0x2000`, we met
> "EHCI timed out on TD - token=0x80008d80".
> 
> The devices does not support scsi VPD page, we are not able
> to get the maximum transfer length for READ(10)/WRITE(10).
> 
> So we limit this to 256 blocks as READ(6).
> 
> Signed-off-by: Peng Fan <peng.fan at nxp.com>
> Acked-by: Marcel Ziswiler <marcel.ziswiler at toradex.com>
> (cherry picked from commit df0052575b2bc9d66ae73584768e1a457ed5d914)
> 
> Signed-off-by: Marcel Ziswiler <marcel at ziswiler.com>
> 
> ---
> This comes from NXP's downstream and has proven to tremendously improve
> the situation with those odd USB mass storage aka memory sticks. This is
> why I post it here asking whether or not this may be something
> benefiting more people. Any feedback and suggestions are welcome.
> 
> Changes in v3:
> - Drop the reference to the NXP internal MLK-xxx tracking number as
>   suggested by Peng.
> 
> Changes in v2:
> - Fixed spelling in comment as suggested by Igor.
> 
>  common/usb_storage.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/common/usb_storage.c b/common/usb_storage.c
> index 8c889bb1a6..4e284645f5 100644
> --- a/common/usb_storage.c
> +++ b/common/usb_storage.c
> @@ -949,7 +949,11 @@ static void usb_stor_set_max_xfer_blk(struct usb_device *udev,
>  	 * there is enough free heap space left, but the SCSI READ(10) and
>  	 * WRITE(10) commands are limited to 65535 blocks.
>  	 */
> -	blk = USHRT_MAX;
> +	/*
> +	 * Some USB mass storage devices have issues, limiting this to 256
> +	 * fixes this.
> +	 */
> +	blk = 256;

Seems like the previous comment, right above this new one, contradicts
this claim. Please update the original comment.

That said, there was an attempt to fix this properly by using adaptive
block length implementation, which would not impact devices that are not
broken. This hack above will degrade performance of such good devices.
Maybe you can resuscitate that adaptive approach instead ?

>  #else
>  	blk = 20;
>  #endif
> 


-- 
Best regards,
Marek Vasut