Re: [PATCH 1/1] usb: request only 8 bytes for USB_SPEED_FULL bMaxPacketSize0 discovery

[Janne Grunau <j@jannau.net>]

On 2022-09-26 01:52:37 +0200, Marek Vasut wrote:
> On 8/29/22 08:31, Janne Grunau wrote:
> > Fixes probing of various keyboards with DWC3 as integrated into Apple
> > silicon SoCs. The problem appears to be requesting more data than the
> > devices's bMaxPacketSize0 of 8. Older Logitech unifying receivers
> > (bcdDevice 12.03 or 12.10) are for eaxample affected.
> > 
> > Signed-off-by: Janne Grunau <j@jannau.net>
> > Tested-by: Thomas Glanzmann <thomas@glanzmann.de>
> > ---
> >   common/usb.c | 8 +++++---
> >   1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/common/usb.c b/common/usb.c
> > index 6fcf1e8428e9..48a310e8599d 100644
> > --- a/common/usb.c
> > +++ b/common/usb.c
> > @@ -993,10 +993,12 @@ static int usb_setup_descriptor(struct usb_device *dev, bool do_read)
> >   		 *
> >   		 * At least the DWC2 controller needs to be programmed with
> >   		 * 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.
> > +		 * Requesting more than 8 bytes causes probing errors on the
> > +		 * DWC3 controller integrated into Apple silicon SoCs with
> > +		 * devices with bMaxPacketSize0 of 8. So limit the read request
> > +		 * to the used size of 8 bytes.
> >   		 */
> > -		err = get_descriptor_len(dev, 64, 8);
> > +		err = get_descriptor_len(dev, 8, 8);
> >   		if (err)
> >   			return err;
> >   	}
> 
> Sorry for the delay.
> 
> Can you be more specific about those logitech receivers ?

In my case it's a device a little bit larger than a USB-A plug. ~7mm 
heigh black plastic case with "logitech" written on the end and and 
their unifying logo in orange on a side (idVendor: 0x046d, idProduct: 
0xc52b, bcdDevice: 12.03). Russell has one with bcdDevice: 12.10

The problem is not limited to the logitech receivers. It reproduces for 
other keyboards with bMaxPacketSize0 == 8 as well. I suspect it affects 
all devices with bMaxPacketSize0 == 8. Keyboards are simply the type of 
device those breakage is noticed inside u-boot / efi bootloaders on 
desktop class devices and don't transfer that much data so that 
bMaxPacketSize0 == 8 doesn't hurt.

> I might have one
> of those devices, and I have DWC3 in i.MX8MP and i.MX8MQ, as well as ZynqMP,
> so I should be able to try and trigger the problem. Can you share the
> reproducer test case for this problem ?

The device is not detected. It is not listed as detected usb device 
during u-boot's USB probing. If the device is a keyboard or you have a 
matching logitech wireless keyboard it will not work to interrupt the 
auto boot or on the u-boot prompt.

> If the problem is specific to the Apple instance of the controller, 
> maybe we need some sort of quirk instead ?

The code looks evidently broken to me. The comment says that we can only 
expect to receive a single packet. We request 64 bytes but devices might 
have a bMaxPacketSize0 of 8. Requesting more than 8 bytes looks also 
unnecessary as we are only interested in bMaxPacketSize0 which is within 
the first 8 bytes of the device descriptor.

Thanks

Janne