diff mbox series

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

Message ID 20190325161944.8044-1-marcel@ziswiler.com
State Changes Requested
Delegated to: Lukasz Majewski
Headers show
Series [U-Boot,RESEND,RFC,v1] MLK-12883 usb: limit USB_MAX_XFER_BLK to 256 | expand

Commit Message

Marcel Ziswiler March 25, 2019, 4:19 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.

 common/usb_storage.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Igor Opaniuk March 28, 2019, 2:38 p.m. UTC | #1 
There is a typo in the comment: USB Massstorage

Also regarding refering to a specific commit ("(cherry picked from
commit df0052575b2bc9d66ae73584768e1a457ed5d914)" line):

> If you want to refer to a specific commit, don’t just refer to the SHA-1 ID of the commit.
> Please also include the oneline summary of the commit, to make it easier for reviewers to know what it is about. Example:
> Commit e21d2170f36602ae2708 ("video: remove unnecessary platform_set_drvdata()")

[1]: https://www.kernel.org/doc/html/v5.1-rc2/process/submitting-patches.html

On Mon, Mar 25, 2019 at 6:20 PM Marcel Ziswiler <marcel@ziswiler.com> 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.
>
>  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..7420438363 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 Massstorage devices have issues, limiting this to 256
> +        * fixes this.
> +        */
> +       blk = 256;
>  #else
>         blk = 20;
>  #endif
> --
> 2.20.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
Marcel Ziswiler April 9, 2019, 1:45 p.m. UTC | #2 
On Thu, 2019-03-28 at 14:38 +0000, Igor Opaniuk wrote:
> There is a typo in the comment: USB Massstorage

I believe it is just everywhere called USB mass storage really.

> Also regarding refering to a specific commit ("(cherry picked from
> commit df0052575b2bc9d66ae73584768e1a457ed5d914)" line):

That is really just git's default cherry pick message as added by its
-x command line argument. None of my doing really.

> > If you want to refer to a specific commit, don’t just refer to the
> > SHA-1 ID of the commit.
> > Please also include the oneline summary of the commit, to make it
> > easier for reviewers to know what it is about. Example:
> > Commit e21d2170f36602ae2708 ("video: remove unnecessary
> > platform_set_drvdata()")

I am fully aware of this, however, I believe usually the hash would be
shortened to just 12 characters.

> [1]: 
> https://www.kernel.org/doc/html/v5.1-rc2/process/submitting-patches.html
> 
> On Mon, Mar 25, 2019 at 6:20 PM Marcel Ziswiler <marcel@ziswiler.com>
> 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.
> > 
> >  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..7420438363 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 Massstorage devices have issues, limiting this
> > to 256
> > +        * fixes this.
> > +        */
> > +       blk = 256;
> >  #else
> >         blk = 20;
> >  #endif
> > --
> > 2.20.1
> > 
> > _______________________________________________
> > 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..7420438363 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 Massstorage devices have issues, limiting this to 256
+	 * fixes this.
+	 */
+	blk = 256;
 #else
 	blk = 20;
 #endif