diff mbox series

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

Message ID 20190409152045.10553-1-marcel@ziswiler.com
State Not Applicable
Delegated to: Stefano Babic
Headers show
Series [U-Boot,RFC,v2] MLK-12883 usb: limit USB_MAX_XFER_BLK to 256 | expand

Commit Message

Marcel Ziswiler April 9, 2019, 3:20 p.m. UTC 
From: Peng Fan <peng.fan@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@nxp.com>
Acked-by: Marcel Ziswiler <marcel.ziswiler@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(-)

Comments

Tom Rini April 9, 2019, 9:14 p.m. UTC | #1 
On Tue, Apr 09, 2019 at 05:20:45PM +0200, Marcel Ziswiler wrote:

> From: Peng Fan <peng.fan@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@nxp.com>
> Acked-by: Marcel Ziswiler <marcel.ziswiler@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
> +	 * 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!
Lukasz Majewski April 10, 2019, 5:11 a.m. UTC | #2 
On Tue, 9 Apr 2019 17:14:26 -0400
Tom Rini <trini@konsulko.com> wrote:

> On Tue, Apr 09, 2019 at 05:20:45PM +0200, Marcel Ziswiler wrote:
> 
> > From: Peng Fan <peng.fan@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@nxp.com>
> > Acked-by: Marcel Ziswiler <marcel.ziswiler@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?

> > +	 * 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@denx.de
Marcel Ziswiler April 10, 2019, 6:25 a.m. UTC | #3 
Hi Lukasz

On April 10, 2019 7:11:11 AM GMT+02:00, Lukasz Majewski <lukma@denx.de> wrote:
>On Tue, 9 Apr 2019 17:14:26 -0400
>Tom Rini <trini@konsulko.com> wrote:
>
>> On Tue, Apr 09, 2019 at 05:20:45PM +0200, Marcel Ziswiler wrote:
>> 
>> > From: Peng Fan <peng.fan@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@nxp.com>
>> > Acked-by: Marcel Ziswiler <marcel.ziswiler@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.

>> > +	 * 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@denx.de

Cheers

Marcel
Frieder Schrempf April 10, 2019, 7:05 a.m. UTC | #4 
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@denx.de> wrote:
>> On Tue, 9 Apr 2019 17:14:26 -0400
>> Tom Rini <trini@konsulko.com> wrote:
>>
>>> On Tue, Apr 09, 2019 at 05:20:45PM +0200, Marcel Ziswiler wrote:
>>>
>>>> From: Peng Fan <peng.fan@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@nxp.com>
>>>> Acked-by: Marcel Ziswiler <marcel.ziswiler@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@denx.de
> 
> Cheers
> 
> Marcel
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
>
diff mbox series

Patch

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