| Message ID | 20221030223835.422976-1-marex@denx.de |
|---|---|
| State | Deferred |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | usb: Add 1ms delay after first Get Descriptor request | expand |
Hi Marek, On Sun, 30 Oct 2022 at 16:38, Marek Vasut <marex@denx.de> wrote: > > Logitech Unifying Receiver 046d:c52b bcdDevice 12.10 seems > sensitive about the first Get Descriptor request. If there > are any other requests in the same microframe, the device > reports bogus data, first of the descriptor parts is not > sent to the host. Wait over one microframe duration before > issuing subsequent requests to avoid probe failure with > this device, since it can be used to connect USB keyboards. > > Signed-off-by: Marek Vasut <marex@denx.de> > --- > Cc: Janne Grunau <j@jannau.net> > Cc: Mark Kettenis <kettenis@openbsd.org> > --- > common/usb.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) Is this device complying with the spec or is it broken? In any case we need a way to enable/disable this as it will slow down unaffected platforms. I've also been wondering if (not with this patch) USB can move to using the cyclic stuff to do its work, so we don't have to wait. > > diff --git a/common/usb.c b/common/usb.c > index 6fcf1e8428e..ae9253dfc0e 100644 > --- a/common/usb.c > +++ b/common/usb.c > @@ -999,6 +999,17 @@ static int usb_setup_descriptor(struct usb_device *dev, bool do_read) > err = get_descriptor_len(dev, 64, 8); > if (err) > return err; > + > + /* > + * Logitech Unifying Receiver 046d:c52b bcdDevice 12.10 seems > + * sensitive about the first Get Descriptor request. If there > + * are any other requests in the same microframe, the device > + * reports bogus data, first of the descriptor parts is not > + * sent to the host. Wait over one microframe duration here > + * (1mS for USB 1.x , 125uS for USB 2.0) to avoid triggering > + * the issue. > + */ > + mdelay(1); > } > > dev->epmaxpacketin[0] = dev->descriptor.bMaxPacketSize0; > -- > 2.35.1 > Regards, Simon
On 10/31/22 20:27, Simon Glass wrote: > Hi Marek, Hi, > On Sun, 30 Oct 2022 at 16:38, Marek Vasut <marex@denx.de> wrote: >> >> Logitech Unifying Receiver 046d:c52b bcdDevice 12.10 seems >> sensitive about the first Get Descriptor request. If there >> are any other requests in the same microframe, the device >> reports bogus data, first of the descriptor parts is not >> sent to the host. Wait over one microframe duration before >> issuing subsequent requests to avoid probe failure with >> this device, since it can be used to connect USB keyboards. >> >> Signed-off-by: Marek Vasut <marex@denx.de> >> --- >> Cc: Janne Grunau <j@jannau.net> >> Cc: Mark Kettenis <kettenis@openbsd.org> >> --- >> common/usb.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) > > Is this device complying with the spec or is it broken? > > In any case we need a way to enable/disable this as it will slow down > unaffected platforms. This makes little difference, since anyone can plug such device into a port and suddenly the platform is affected. We cannot really predict what users have on their desks. Specifically for this case, the logitech receiver seems to be a rather common device. > I've also been wondering if (not with this patch) USB can move to > using the cyclic stuff to do its work, so we don't have to wait. EHCI HCD already partly does, at least for storage, so yes, patches welcome.
On 2022-10-31 21:57:39 +0100, Marek Vasut wrote: > On 10/31/22 20:27, Simon Glass wrote: > > Hi Marek, > > Hi, > > > On Sun, 30 Oct 2022 at 16:38, Marek Vasut <marex@denx.de> wrote: > > > > > > Logitech Unifying Receiver 046d:c52b bcdDevice 12.10 seems > > > sensitive about the first Get Descriptor request. If there > > > are any other requests in the same microframe, the device > > > reports bogus data, first of the descriptor parts is not > > > sent to the host. Wait over one microframe duration before > > > issuing subsequent requests to avoid probe failure with > > > this device, since it can be used to connect USB keyboards. > > > > > > Signed-off-by: Marek Vasut <marex@denx.de> > > > --- > > > Cc: Janne Grunau <j@jannau.net> > > > Cc: Mark Kettenis <kettenis@openbsd.org> > > > --- > > > common/usb.c | 11 +++++++++++ > > > 1 file changed, 11 insertions(+) > > > > Is this device complying with the spec or is it broken? This is not just this device. We have seen this reported with a few keyboards on apple silicon devices. Keychron devices are one group I can remember. > > > > In any case we need a way to enable/disable this as it will slow down > > unaffected platforms. > > This makes little difference, since anyone can plug such device into a port > and suddenly the platform is affected. We cannot really predict what users > have on their desks. Would it make sense to limit it to dwc3 host controllers or do you fear the same problem could happen with other controllers on fast boards? In any case patch fixes the problem here as well. Tested-by: Janne Grunau <j@jannau.net> Thanks, Janne
On 11/3/22 22:31, Janne Grunau wrote: Hi, [...] >>> In any case we need a way to enable/disable this as it will slow down >>> unaffected platforms. >> >> This makes little difference, since anyone can plug such device into a port >> and suddenly the platform is affected. We cannot really predict what users >> have on their desks. > > Would it make sense to limit it to dwc3 host controllers or do you fear > the same problem could happen with other controllers on fast boards? I believe the DWC3 triggers it on "fast" devices now, but this is a generic issue. The U-Boot USB stack is just way too efficient (well, more efficient than any OS anyway, because it is basically just a tight loop), no OS triggers this because the OSes take their time. > In any case patch fixes the problem here as well. > > Tested-by: Janne Grunau <j@jannau.net> Thanks! (and sorry for the delay)
diff --git a/common/usb.c b/common/usb.c index 6fcf1e8428e..ae9253dfc0e 100644 --- a/common/usb.c +++ b/common/usb.c @@ -999,6 +999,17 @@ static int usb_setup_descriptor(struct usb_device *dev, bool do_read) err = get_descriptor_len(dev, 64, 8); if (err) return err; + + /* + * Logitech Unifying Receiver 046d:c52b bcdDevice 12.10 seems + * sensitive about the first Get Descriptor request. If there + * are any other requests in the same microframe, the device + * reports bogus data, first of the descriptor parts is not + * sent to the host. Wait over one microframe duration here + * (1mS for USB 1.x , 125uS for USB 2.0) to avoid triggering + * the issue. + */ + mdelay(1); } dev->epmaxpacketin[0] = dev->descriptor.bMaxPacketSize0;
Logitech Unifying Receiver 046d:c52b bcdDevice 12.10 seems sensitive about the first Get Descriptor request. If there are any other requests in the same microframe, the device reports bogus data, first of the descriptor parts is not sent to the host. Wait over one microframe duration before issuing subsequent requests to avoid probe failure with this device, since it can be used to connect USB keyboards. Signed-off-by: Marek Vasut <marex@denx.de> --- Cc: Janne Grunau <j@jannau.net> Cc: Mark Kettenis <kettenis@openbsd.org> --- common/usb.c | 11 +++++++++++ 1 file changed, 11 insertions(+)