[U-Boot] [RFC PATCH v2] MLK-12883 usb: limit USB_MAX_XFER_BLK to 256

[Schrempf Frieder]

On 10.04.19 08:25, Marcel Ziswiler wrote:
> Hi Lukasz
> 
> On April 10, 2019 7:11:11 AM GMT+02:00, Lukasz Majewski <lukma at denx.de> wrote:
>> On Tue, 9 Apr 2019 17:14:26 -0400
>> Tom Rini <trini at konsulko.com> wrote:
>>
>>> On Tue, Apr 09, 2019 at 05:20:45PM +0200, 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)
>>>>
>>>> ---
>>>> 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 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
>>
>> Could you name those devices?
> 
> You mean all of them? The Kingston DataTraveler 2.0 is already mentioned in the commit message.

Only for reference I want to add a link to a discussion from some years 
ago, that also names some devices that one of our customers found that 
didn't work with USB_MAX_XFER_BLK = 65536, but only with lower values: [1].

There's also a post with some benchmarking to get an idea of the impact 
on transfer speed when lowering USB_MAX_XFER_BLK: [2].

Regards,
Frieder

[1] https://lists.denx.de/pipermail/u-boot/2016-February/245893.html
[2] https://lists.denx.de/pipermail/u-boot/2016-February/246267.html

> 
>>>> +	 * fixes this.
>>>> +	 */
>>>> +	blk = 256;
>>>>   #else
>>>>   	blk = 20;
>>>>   #endif
>>>> -- 
>>>> 2.20.1
>>>
>>> Adding in Lukasz now that get_maintainers.pl shows this should be
>>> Cc'd to him as well, thanks!
>>>
>>
>>
>>
>>
>> Best regards,
>>
>> Lukasz Majewski
>>
>> --
>>
>> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
>> lukma at denx.de
> 
> Cheers
> 
> Marcel
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
>