[PATCH] usb: Add delay for control messages to reach usb stick

[PATCH] usb: Add delay for control messages to reach usb stick

[Ashok Reddy Soma]

We are seeing timing issues with transcend usb sticks. These devices
seems to require more time than regular devices for the control messages
to reach device. Add 1ms delay before sending control message to fix
trancend device detection issue.

Signed-off-by: Ashok Reddy Soma <ashok.reddy.soma@amd.com>
---

 common/usb.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/common/usb.c b/common/usb.c
index 6fcf1e8428..3fae32b048 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -241,6 +241,12 @@ int usb_control_msg(struct usb_device *dev, unsigned int pipe,
 	      request, requesttype, value, index, size);
 	dev->status = USB_ST_NOT_PROC; /*not yet processed */
 
+	/* Timing issues are observed with transcend usb sticks such as
+	 * “Transcend Jetflash 350 USB2.0". Add 1ms delay for the usb
+	 * device to get detected.
+	 */
+	mdelay(1);
+
 	err = submit_control_msg(dev, pipe, data, size, setup_packet);
 	if (err < 0)
 		return err;
-- 
2.17.1

Re: [PATCH] usb: Add delay for control messages to reach usb stick

[Janne Grunau]

[-- Attachment #1: Type: text/plain, Size: 2216 bytes --]

Hej,

On 2022-08-25 23:35:33 -0600, Ashok Reddy Soma wrote:
> We are seeing timing issues with transcend usb sticks. These devices
> seems to require more time than regular devices for the control messages
> to reach device. Add 1ms delay before sending control message to fix
> trancend device detection issue.

I suspect I see something similar with the DWC3 controller on Apple 
M1/M2 devices. It seems to be related to USB full speed devices with 
bMaxPacketSize0 of 8. Failing devices are so only keyboards since that 
is a device everyone will connect when using a Mac Mini as desktop.
I can reproduce the issue with older Logitech Unifying Receiver wireless 
keyboard/mouse dongles (bcdDevice 12.03 or 12.10). I could also resolve 
the issue with random 'mdelay(1);'. I chased the cause of the issue down 
to the initial USB descriptor read to parse 'bMaxPacketSize0' in 
usb_setup_descriptor(). Please test if adding the delay after the 
get_descriptor_len() call in usb_setup_descriptor() is enough.
On the Apple silicon devices reducing the read size from 64 byte to 8 
resolves the issue as well. Please try attached work-in-progress patch 
(comment and commit message are not finalized).

HTH Janne
 
> Signed-off-by: Ashok Reddy Soma <ashok.reddy.soma@amd.com>
> ---
> 
>  common/usb.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/common/usb.c b/common/usb.c
> index 6fcf1e8428..3fae32b048 100644
> --- a/common/usb.c
> +++ b/common/usb.c
> @@ -241,6 +241,12 @@ int usb_control_msg(struct usb_device *dev, unsigned int pipe,
>  	      request, requesttype, value, index, size);
>  	dev->status = USB_ST_NOT_PROC; /*not yet processed */
>  
> +	/* Timing issues are observed with transcend usb sticks such as
> +	 * “Transcend Jetflash 350 USB2.0". Add 1ms delay for the usb
> +	 * device to get detected.
> +	 */
> +	mdelay(1);

Please let's try to avoid this. I noticed a slowdown of USB probing with 
an hub with 4 devices connected. Since Apple silicon devices are desktop 
style machines I expect it's not uncommon to see systems with many USB 
devices.

> +
>  	err = submit_control_msg(dev, pipe, data, size, setup_packet);
>  	if (err < 0)
>  		return err;
> -- 
> 2.17.1
> 

[-- Attachment #2: 0001-usb-request-on-8-bytes-for-USB_SPEED_FULL-bMaxPacket.patch --]
[-- Type: text/x-diff, Size: 1454 bytes --]

From df9e5b78687fc4eed4988f99d3f195cba8b227e1 Mon Sep 17 00:00:00 2001
From: Janne Grunau <j@jannau.net>
Date: Fri, 26 Aug 2022 00:01:15 +0200
Subject: [PATCH 1/1] usb: request on 8 bytes for USB_SPEED_FULL
 bMaxPacketSize0 probing

Fixes probing of Logitech Unifying receivers (bcdDevice 12.03 or 12.10)
with bMaxPacketSize0 == 8 on Apple silicon SoCs using DWC3 controllers.

Signed-off-by: Janne Grunau <j@jannau.net>
---
 common/usb.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/common/usb.c b/common/usb.c
index aad13fd9c557..2d6d782f9103 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -999,8 +999,17 @@ static int usb_setup_descriptor(struct usb_device *dev, bool do_read)
 		 * the number of packets in addition to the number of bytes.
 		 * A request for 64 bytes of data with the maxpacket guessed
 		 * as 64 (above) yields a request for 1 packet.
+		 *
+		 * The DWC3 controller integrated into Apple silicon SoCs like
+		 * the M1 and M2 does not like to read 64 bytes for devices with
+		 * bMaxPacketSize0 == 8. request only 8 bytes which should also
+		 * result in a single packet.
+		 * Fixes probing errors with Logitech Unifying receivers with
+		 * bcdDevice 12.03 or 12.10.
+		 * A delay of 1 ms after this get_descriptor_len() call fixes
+		 * the issue as well.
 		 */
-		err = get_descriptor_len(dev, 64, 8);
+		err = get_descriptor_len(dev, 8, 8);
 		if (err)
 			return err;
 	}
-- 
2.35.1

Re: [PATCH] usb: Add delay for control messages to reach usb stick

[Simon Glass]

Hi,

On Fri, 26 Aug 2022 at 13:37, Janne Grunau <j@jannau.net> wrote:
>
> Hej,
>
> On 2022-08-25 23:35:33 -0600, Ashok Reddy Soma wrote:
> > We are seeing timing issues with transcend usb sticks. These devices
> > seems to require more time than regular devices for the control messages
> > to reach device. Add 1ms delay before sending control message to fix
> > trancend device detection issue.
>
> I suspect I see something similar with the DWC3 controller on Apple
> M1/M2 devices. It seems to be related to USB full speed devices with
> bMaxPacketSize0 of 8. Failing devices are so only keyboards since that
> is a device everyone will connect when using a Mac Mini as desktop.
> I can reproduce the issue with older Logitech Unifying Receiver wireless
> keyboard/mouse dongles (bcdDevice 12.03 or 12.10). I could also resolve
> the issue with random 'mdelay(1);'. I chased the cause of the issue down
> to the initial USB descriptor read to parse 'bMaxPacketSize0' in
> usb_setup_descriptor(). Please test if adding the delay after the
> get_descriptor_len() call in usb_setup_descriptor() is enough.
> On the Apple silicon devices reducing the read size from 64 byte to 8
> resolves the issue as well. Please try attached work-in-progress patch
> (comment and commit message are not finalized).
>
> HTH Janne
>
> > Signed-off-by: Ashok Reddy Soma <ashok.reddy.soma@amd.com>
> > ---
> >
> >  common/usb.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/common/usb.c b/common/usb.c
> > index 6fcf1e8428..3fae32b048 100644
> > --- a/common/usb.c
> > +++ b/common/usb.c
> > @@ -241,6 +241,12 @@ int usb_control_msg(struct usb_device *dev, unsigned int pipe,
> >             request, requesttype, value, index, size);
> >       dev->status = USB_ST_NOT_PROC; /*not yet processed */
> >
> > +     /* Timing issues are observed with transcend usb sticks such as
> > +      * “Transcend Jetflash 350 USB2.0". Add 1ms delay for the usb
> > +      * device to get detected.
> > +      */
> > +     mdelay(1);
>
> Please let's try to avoid this. I noticed a slowdown of USB probing with
> an hub with 4 devices connected. Since Apple silicon devices are desktop
> style machines I expect it's not uncommon to see systems with many USB
> devices.

Can we add a CONFIG for this, or put a setting in the device tree? Or
is there a way to detect the problem and retry?

>
> > +
> >       err = submit_control_msg(dev, pipe, data, size, setup_packet);
> >       if (err < 0)
> >               return err;
> > --
> > 2.17.1
> >

Regards,
Simon

Re: [PATCH] usb: Add delay for control messages to reach usb stick

[Janne Grunau]

On 2022-08-26 18:20:57 -0600, Simon Glass wrote:
> Hi,
> 
> On Fri, 26 Aug 2022 at 13:37, Janne Grunau <j@jannau.net> wrote:
> >
> > Hej,
> >
> > On 2022-08-25 23:35:33 -0600, Ashok Reddy Soma wrote:
> > > We are seeing timing issues with transcend usb sticks. These devices
> > > seems to require more time than regular devices for the control messages
> > > to reach device. Add 1ms delay before sending control message to fix
> > > trancend device detection issue.
> >
> > I suspect I see something similar with the DWC3 controller on Apple
> > M1/M2 devices. It seems to be related to USB full speed devices with
> > bMaxPacketSize0 of 8. Failing devices are so only keyboards since that
> > is a device everyone will connect when using a Mac Mini as desktop.
> > I can reproduce the issue with older Logitech Unifying Receiver wireless
> > keyboard/mouse dongles (bcdDevice 12.03 or 12.10). I could also resolve
> > the issue with random 'mdelay(1);'. I chased the cause of the issue down
> > to the initial USB descriptor read to parse 'bMaxPacketSize0' in
> > usb_setup_descriptor(). Please test if adding the delay after the
> > get_descriptor_len() call in usb_setup_descriptor() is enough.
> > On the Apple silicon devices reducing the read size from 64 byte to 8
> > resolves the issue as well. Please try attached work-in-progress patch
> > (comment and commit message are not finalized).
> >
> > HTH Janne
> >
> > > Signed-off-by: Ashok Reddy Soma <ashok.reddy.soma@amd.com>
> > > ---
> > >
> > >  common/usb.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/common/usb.c b/common/usb.c
> > > index 6fcf1e8428..3fae32b048 100644
> > > --- a/common/usb.c
> > > +++ b/common/usb.c
> > > @@ -241,6 +241,12 @@ int usb_control_msg(struct usb_device *dev, unsigned int pipe,
> > >             request, requesttype, value, index, size);
> > >       dev->status = USB_ST_NOT_PROC; /*not yet processed */
> > >
> > > +     /* Timing issues are observed with transcend usb sticks such as
> > > +      * “Transcend Jetflash 350 USB2.0". Add 1ms delay for the usb
> > > +      * device to get detected.
> > > +      */
> > > +     mdelay(1);
> >
> > Please let's try to avoid this. I noticed a slowdown of USB probing with
> > an hub with 4 devices connected. Since Apple silicon devices are desktop
> > style machines I expect it's not uncommon to see systems with many USB
> > devices.
> 
> Can we add a CONFIG for this, or put a setting in the device tree? Or
> is there a way to detect the problem and retry?

A short search suggests that Transcend USB1.0 flash drives have a 
bMaxPacketSize0 of 64 so it's unlikely that my change to reduce the 
initial read of the usb descriptor will fix this but please try first.  
Have you verified that this delay is required for every USB control 
message and not just for a single one? Compare the mdelay for "Kingston 
DT Ultimate" in usb_select_config().

If this is an issue of this specific USB devices I don't see how a 
config option or device tree entry is going to help. Since it is an 
issue in the device detection it might not be even possible to add a 
device specific quirk for this.

Have you tried if shorter delays than 1 ms fix the issue as well?

I'll clean up the comment in my commit and send it later as patch.

Best regards

Janne

Re: [PATCH] usb: Add delay for control messages to reach usb stick

[Marek Vasut]

On 8/26/22 07:35, Ashok Reddy Soma wrote:
> We are seeing timing issues with transcend usb sticks. These devices
> seems to require more time than regular devices for the control messages
> to reach device. Add 1ms delay before sending control message to fix
> trancend device detection issue.

In case you set 'usb_pgood_delay=2000' in U-Boot environment and re-test 
the USB stick, does the issue go away too ? This would increase the time 
from power Vbus enable to USB enumeration, it usually tends to fix such 
odd hardware too.