[REGRESSION] Re: [PATCH 1/3] USB: core: Unite old scheme and new scheme descriptor reads

[Christian Eggers <ceggers@arri.de>]

> In preparation for reworking the usb_get_device_descriptor() routine,
> it is desirable to unite the two different code paths responsible for
> initially determining endpoint 0's maximum packet size in a newly
> discovered USB device.  Making this determination presents a
> chicken-and-egg sort of problem, in that the only way to learn the
> maxpacket value is to get it from the device descriptor retrieved from
> the device, but communicating with the device to retrieve a descriptor
> requires us to know beforehand the ep0 maxpacket size.
> 
> In practice this problem is solved in two different ways, referred to
> in hub.c as the "old scheme" and the "new scheme".  The old scheme
> (which is the approach recommended by the USB-2 spec) involves asking
> the device to send just the first eight bytes of its device
> descriptor.  Such a transfer uses packets containing no more than
> eight bytes each, and every USB device must have an ep0 maxpacket size
> 
> >= 8, so this should succeed.  Since the bMaxPacketSize0 field of the
> 
> device descriptor lies within the first eight bytes, this is all we
> need.
> 
> The new scheme is an imitation of the technique used in an early
> Windows USB implementation, giving it the happy advantage of working
> with a wide variety of devices (some of them at the time would not
> work with the old scheme, although that's probably less true now).  It
> involves making an initial guess of the ep0 maxpacket size, asking the
> device to send up to 64 bytes worth of its device descriptor (which is
> only 18 bytes long), and then resetting the device to clear any error
> condition that might have resulted from the guess being wrong.  The
> initial guess is determined by the connection speed; it should be
> correct in all cases other than full speed, for which the allowed
> values are 8, 16, 32, and 64 (in this case the initial guess is 64).
> 
> The reason for this patch is that the old- and new-scheme parts of
> hub_port_init() use different code paths, one involving
> usb_get_device_descriptor() and one not, for their initial reads of
> the device descriptor.  Since these reads have essentially the same
> purpose and are made under essentially the same circumstances, this is
> illogical.  It makes more sense to have both of them use a common
> subroutine.
> 
> This subroutine does basically what the new scheme's code did, because
> that approach is more general than the one used by the old scheme.  It
> only needs to know how many bytes to transfer and whether or not it is
> being called for the first iteration of a retry loop (in case of
> certain time-out errors).  There are two main differences from the
> 
> former code:
> 	We initialize the bDescriptorType field of the transfer buffer
> 	to 0 before performing the transfer, to avoid possibly
> 	accessing an uninitialized value afterward.
> 	
> 	We read the device descriptor into a temporary buffer rather
> 	than storing it directly into udev->descriptor, which the old
> 	scheme implementation used to do.
> 
> Since the whole point of this first read of the device descriptor is
> to determine the bMaxPacketSize0 value, that is what the new routine
> returns (or an error code).  The value is stored in a local variable
> rather than in udev->descriptor.  As a side effect, this necessitates
> moving a section of code that checks the bcdUSB field for SuperSpeed
> devices until after the full device descriptor has been retrieved.
> 
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> Cc: Oliver Neukum <oneukum@suse.com>

Last week I upgraded within the 5.15-stable branch. Since upstream commit
85d07c556216 ("USB: core: Unite old scheme and new scheme descriptor reads"),
there are problems detecting a directly attached USB hub. I identified this
commit by bisecting and get the same result during upgrading within the 6.1-stable
branch.

Hardware: ARMv7 NXP i.MX6ULL with directly attached USB hub (Microchip USB4916).
Log messages:

[    6.150881] usb 1-1: new high-speed USB device number 2 using ci_hdrc
[    6.215484] usb 1-1: device descriptor read/8, error -71
[    6.377532] usb 1-1: device descriptor read/8, error -71
[    6.581934] usb 1-1: new high-speed USB device number 3 using ci_hdrc
[    6.642853] usb 1-1: device descriptor read/8, error -71
[    6.803355] usb 1-1: device descriptor read/8, error -71
[    6.920418] usb usb1-port1: attempt power cycle
[    7.051419] usb 1-1: new high-speed USB device number 4 using ci_hdrc
[    7.192320] usb 1-1: New USB device found, idVendor=0424, idProduct=4916, bcdDevice= 8.02
[    7.192348] usb 1-1: New USB device strings: Mfr=1, Product=2, SerialNumber=0
[    7.192363] usb 1-1: Product: Orbiter USB hub
[    7.192376] usb 1-1: Manufacturer: ARRI
[    7.193263] hub 1-1:1.0: USB hub found
[    7.193951] hub 1-1:1.0: 7 ports detected

The "device descriptor read" and "attempt power cycle" error messages definitely
haven't been there before the commit mentioned above. Disregarding the additional
log messages, the USB hub (and its devices) seem to work.

I didn't try reverting this single commit as it seems that later changes depends
on it.

regards
Christian