Re: Regression due to 59cf44575456 ("USB: core: Fix oversight in SuperSpeed initialization")

Takashi Iwai
2024-04-24T08:56:04Z

On Tue, 23 Apr 2024 21:29:27 +0200,
Alan Stern wrote:
>
> On Mon, Apr 22, 2024 at 09:24:24PM +0200, Takashi Iwai wrote:
> > On Mon, 22 Apr 2024 20:03:46 +0200,
> > Alan Stern wrote:
> > >
> > > On Mon, Apr 22, 2024 at 07:33:21PM +0200, Linux regression tracking (Thorsten Leemhuis) wrote:
> > > > Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
> > > > for once, to make this easily accessible to everyone.
> > > >
> > > > Is anyone still working on fixing below regression? From here it looks
> > > > stalled, but I might have missed something.
> > >
> > > I've been waiting to hear back from Oliver or Takashi. A revised patch
> > > taking my comments into account would be welcome; it should be a very
> > > small change (just one or two lines of code).
> >
> > As posted in another mail, it's a virtualized environment.
> > Details are found in the original bug report
> > https://bugzilla.suse.com/show_bug.cgi?id=1220569
>
> Hmmm. If this is a virtualized device, isn't the best solution to fix the
> emulation code for the device so that it presents a valid descriptor?

Honestly speaking, I don't know, but it smells like a hard way. After
all, we brought some regression of the previously (even casually /
unintended) working "device"...

> > About the patch change: I appreciate if you cook it rather by
> > yourself since I'm not 100% sure what you suggested. I can
> > provide the reporter a test kernel with the patch for confirmation, of
> > course.
>
> Here's a condensed version of the patch you wrote. But I would prefer not
> to add this to the kernel if the problem can be fixed somewhere else.

Thanks, I asked the report for testing the patch now.

thanks,

Takashi

>
> Alan Stern
>
>
>
> Index: usb-devel/drivers/usb/core/hub.c
> ===================================================================
> --- usb-devel.orig/drivers/usb/core/hub.c
> +++ usb-devel/drivers/usb/core/hub.c
> @@ -5110,9 +5110,10 @@ hub_port_init(struct usb_hub *hub, struc
> }
> if (usb_endpoint_maxp(&udev->ep0.desc) == i) {
> ; /* Initial ep0 maxpacket guess is right */
> - } else if ((udev->speed == USB_SPEED_FULL ||
> + } else if (((udev->speed == USB_SPEED_FULL ||
> udev->speed == USB_SPEED_HIGH) &&
> - (i == 8 || i == 16 || i == 32 || i == 64)) {
> + (i == 8 || i == 16 || i == 32 || i == 64)) ||
> + (udev->speed >= USB_SPEED_SUPER && i > 0)) {
> /* Initial guess is wrong; use the descriptor's value */
> if (udev->speed == USB_SPEED_FULL)
> dev_dbg(&udev->dev, "ep0 maxpacket = %d\n", i);
>