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

[Sven Schwermer]

Hi Peng,

Yes, this works.

Best regards,
Sven

> On 24 Apr 2019, at 10:59, Peng Fan <peng.fan at nxp.com> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: U-Boot [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Peng Fan
>> Sent: 2019年4月24日 16:57
>> To: Sven Schwermer <sven at svenschwermer.de>; Marcel Ziswiler
>> <marcel at ziswiler.com>
>> Cc: Marek Vasut <marex at denx.de>; Marcel Ziswiler
>> <marcel.ziswiler at toradex.com>; u-boot at lists.denx.de
>> Subject: Re: [U-Boot] [RFC PATCH v3] usb: limit USB_MAX_XFER_BLK to 256
>> 
>> 
>>> Subject: Re: [RFC PATCH v3] usb: limit USB_MAX_XFER_BLK to 256
>>> 
>>> Hi,
>>> 
>>> I am using an iMX7 with a DataTraveler 3.0 and I’m seeing the same
>>> timeout error. However, I have CONFIG_DM_USB=y configured so your
>>> patch doesn’t fix my issue. The USB class queries the EHCI driver for
>>> its maximum transfer size which is SIZE_MAX. I believe a solution that works
>> with DM is necessary.
>> 
>> Does the patch works for you with non DM usb?
> 
> Just a quick hack with DM usb, please help verify whether it could fix your issue.
> 
> diff --git a/common/usb_storage.c b/common/usb_storage.c
> index 8c889bb1a6..0eb63c21f4 100644
> --- a/common/usb_storage.c
> +++ b/common/usb_storage.c
> @@ -965,7 +965,7 @@ static void usb_stor_set_max_xfer_blk(struct usb_device *udev,
>        }
> #endif
> 
> -       us->max_xfer_blk = blk;
> +       us->max_xfer_blk = 256;
> }
> 
> static int usb_inquiry(struct scsi_cmd *srb, struct us_data *ss)
> 
> Regards,
> Peng.
> 
>> 
>> Regards,
>> Peng
>> 
>>> 
>>> => usb tree
>>> USB device tree:
>>>  1  Hub (480 Mb/s, 0mA)
>>>  |  u-boot EHCI Host Controller
>>>  |
>>>  +-2  Mass Storage (480 Mb/s, 300mA)
>>>       Kingston DataTraveler 3.0 50E54942694AE23017A55884
>>> 
>>> => usb storage
>>>  Device 0: Vendor: Kingston Rev: PMAP Prod: DataTraveler 3.0
>>>            Type: Removable Hard Disk
>>>            Capacity: 59184.0 MB = 57.7 GB (121208832 x 512)
>>> 
>>> Best regards,
>>> Sven
>>> 
>>>> On 18 Apr 2019, at 01:45, Marcel Ziswiler <marcel at ziswiler.com> 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;
>>>> #else
>>>> 	blk = 20;
>>>> #endif
>>>> --
>>>> 2.20.1
>>>> 
>> 
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.d
>> enx.de%2Flistinfo%2Fu-boot&data=02%7C01%7CPeng.Fan%40nxp.com
>> %7C92e70f2854ce4eda6cf908d6c892e747%7C686ea1d3bc2b4c6fa92cd99c5
>> c301635%7C0%7C0%7C636916930623480866&sdata=5DyOZrU55x3Y9
>> 1wuaK%2BmKZsFu4PtUMwd81qNC91MshE%3D&reserved=0