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

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

[Oliver Neukum]

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

Hi,

with the following device:

Bus 002 Device 002: ID fb5d:0001 BHYVE HID Tablet
Device Descriptor:
   bLength                18
   bDescriptorType         1
   bcdUSB               3.00
   bDeviceClass            0
   bDeviceSubClass         0
   bDeviceProtocol         0
   bMaxPacketSize0         8
   idVendor           0xfb5d
   idProduct          0x0001
   bcdDevice            0.00
   iManufacturer           1 BHYVE
   iProduct                2 HID Tablet
   iSerial                 3 01
   bNumConfigurations      1
   Configuration Descriptor:
     bLength                 9
     bDescriptorType         2
     wTotalLength       0x0028
     bNumInterfaces          1
     bConfigurationValue     1
     iConfiguration          4 HID Tablet Device
     bmAttributes         0xa0
       (Bus Powered)
       Remote Wakeup
     MaxPower                0mA
     Interface Descriptor:
       bLength                 9
       bDescriptorType         4
       bInterfaceNumber        0
       bAlternateSetting       0
       bNumEndpoints           1
       bInterfaceClass         3 Human Interface Device
       bInterfaceSubClass      1 Boot Interface Subclass
       bInterfaceProtocol      2 Mouse
       iInterface              0
         HID Device Descriptor:
           bLength                 9
           bDescriptorType        33
           bcdHID              10.01
           bCountryCode            0 Not supported
           bNumDescriptors         1
           bDescriptorType        34 Report
           wDescriptorLength      74
          Report Descriptors:
            ** UNAVAILABLE **
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x81  EP 1 IN
         bmAttributes            3
           Transfer Type            Interrupt
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0008  1x 8 bytes
         bInterval              10
         bMaxBurst               0
Binary Object Store Descriptor:
   bLength                 5
   bDescriptorType        15
   wTotalLength       0x000f
   bNumDeviceCaps          1
   SuperSpeed USB Device Capability:
     bLength                10
     bDescriptorType        16
     bDevCapabilityType      3
     bmAttributes         0x00
     wSpeedsSupported   0x0008
       Device can operate at SuperSpeed (5Gbps)
     bFunctionalitySupport   3
       Lowest fully-functional device speed is SuperSpeed (5Gbps)
     bU1DevExitLat          10 micro seconds
     bU2DevExitLat          32 micro seconds
Device Status:     0x0000
   (Bus Powered)

we are getting a regression on enumeration. It used to work with the
code prior to your patch. Takashi is proposing the attached fixed.
It looks like we are a bit too restrictive and should just try.

	Regards
		Oliver

[-- Attachment #2: 0001-USB-hub-Workaround-for-buggy-max-packet-size-with-su.patch --]
[-- Type: text/x-patch, Size: 1982 bytes --]

From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] USB: hub: Workaround for buggy max packet size with super
 speed
Patch-mainline: Not yet, testing
References: bsc#1220569

Signed-off-by: Takashi Iwai <tiwai@suse.de>

---
 drivers/usb/core/hub.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index e38a4124f610..64193effc456 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4830,7 +4830,7 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
 	const char		*driver_name;
 	bool			do_new_scheme;
 	const bool		initial = !dev_descr;
-	int			maxp0;
+	int			maxp0, ep0_maxp;
 	struct usb_device_descriptor	*buf, *descr;
 
 	buf = kmalloc(GET_DESCRIPTOR_BUFSIZE, GFP_NOIO);
@@ -5070,7 +5070,8 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
 		else
 			i = 0;		/* Invalid */
 	}
-	if (usb_endpoint_maxp(&udev->ep0.desc) == i) {
+	ep0_maxp = usb_endpoint_maxp(&udev->ep0.desc);
+	if (ep0_maxp == i) {
 		;	/* Initial ep0 maxpacket guess is right */
 	} else if ((udev->speed == USB_SPEED_FULL ||
 				udev->speed == USB_SPEED_HIGH) &&
@@ -5082,9 +5083,15 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
 			dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
 		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
 		usb_ep0_reinit(udev);
+	} else if (udev->speed >= USB_SPEED_SUPER && initial) {
+		/* FIXME: should be more restrictive? */
+		/* Initial guess is wrong; use the descriptor's value */
+		dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
+		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
+		usb_ep0_reinit(udev);
 	} else {
 		/* Initial guess is wrong and descriptor's value is invalid */
-		dev_err(&udev->dev, "Invalid ep0 maxpacket: %d\n", maxp0);
+		dev_err(&udev->dev, "Invalid ep0 maxpacket: %d, expected %d\n", maxp0, ep0_maxp);
 		retval = -EMSGSIZE;
 		goto fail;
 	}
-- 
2.35.3

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

[Alan Stern]

On Tue, Apr 09, 2024 at 03:49:01PM +0200, Oliver Neukum wrote:
> Hi,
> 
> with the following device:
> 
> Bus 002 Device 002: ID fb5d:0001 BHYVE HID Tablet
> Device Descriptor:
>   bLength                18
>   bDescriptorType         1
>   bcdUSB               3.00
>   bDeviceClass            0
>   bDeviceSubClass         0
>   bDeviceProtocol         0
>   bMaxPacketSize0         8

A USB-3 device, running at SuperSpeed with its bMaxPacketSize0 set to 8 
instead of 9?  Presumably this thing never received a USB certification.  
Does the packaging use the USB logo?

>   idVendor           0xfb5d
>   idProduct          0x0001
>   bcdDevice            0.00
>   iManufacturer           1 BHYVE
>   iProduct                2 HID Tablet
>   iSerial                 3 01
>   bNumConfigurations      1

Why on earth would an HID tablet need to run at SuperSpeed?

> Binary Object Store Descriptor:
>   bLength                 5
>   bDescriptorType        15
>   wTotalLength       0x000f
>   bNumDeviceCaps          1
>   SuperSpeed USB Device Capability:
>     bLength                10
>     bDescriptorType        16
>     bDevCapabilityType      3
>     bmAttributes         0x00
>     wSpeedsSupported   0x0008
>       Device can operate at SuperSpeed (5Gbps)
>     bFunctionalitySupport   3
>       Lowest fully-functional device speed is SuperSpeed (5Gbps)
>     bU1DevExitLat          10 micro seconds
>     bU2DevExitLat          32 micro seconds

A tablet not capable of running at any speed below 5 Gbps?

> we are getting a regression on enumeration. It used to work with the
> code prior to your patch. Takashi is proposing the attached fixed.
> It looks like we are a bit too restrictive and should just try.
> 
> 	Regards
> 		Oliver

> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH] USB: hub: Workaround for buggy max packet size with super
>  speed
> Patch-mainline: Not yet, testing
> References: bsc#1220569
> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> 
> ---
>  drivers/usb/core/hub.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index e38a4124f610..64193effc456 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -4830,7 +4830,7 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
>  	const char		*driver_name;
>  	bool			do_new_scheme;
>  	const bool		initial = !dev_descr;
> -	int			maxp0;
> +	int			maxp0, ep0_maxp;
>  	struct usb_device_descriptor	*buf, *descr;
>  
>  	buf = kmalloc(GET_DESCRIPTOR_BUFSIZE, GFP_NOIO);
> @@ -5070,7 +5070,8 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
>  		else
>  			i = 0;		/* Invalid */
>  	}
> -	if (usb_endpoint_maxp(&udev->ep0.desc) == i) {
> +	ep0_maxp = usb_endpoint_maxp(&udev->ep0.desc);
> +	if (ep0_maxp == i) {

This variable looks like it was left over from earlier testing.  It's 
not really needed.

>  		;	/* Initial ep0 maxpacket guess is right */
>  	} else if ((udev->speed == USB_SPEED_FULL ||
>  				udev->speed == USB_SPEED_HIGH) &&
> @@ -5082,9 +5083,15 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
>  			dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
>  		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
>  		usb_ep0_reinit(udev);
> +	} else if (udev->speed >= USB_SPEED_SUPER && initial) {
> +		/* FIXME: should be more restrictive? */
> +		/* Initial guess is wrong; use the descriptor's value */
> +		dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
> +		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
> +		usb_ep0_reinit(udev);

This could be merged with the previous case fairly easily.

>  	} else {
>  		/* Initial guess is wrong and descriptor's value is invalid */
> -		dev_err(&udev->dev, "Invalid ep0 maxpacket: %d\n", maxp0);
> +		dev_err(&udev->dev, "Invalid ep0 maxpacket: %d, expected %d\n", maxp0, ep0_maxp);

This also looks like a remnant from earlier testing.

Alan Stern

>  		retval = -EMSGSIZE;
>  		goto fail;
>  	}
> -- 
> 2.35.3
> 

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

[Linux regression tracking (Thorsten Leemhuis)]

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.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

#regzbot poke

On 09.04.24 16:56, Alan Stern wrote:
> On Tue, Apr 09, 2024 at 03:49:01PM +0200, Oliver Neukum wrote:
>> Hi,
>>
>> with the following device:
>>
>> Bus 002 Device 002: ID fb5d:0001 BHYVE HID Tablet
>> Device Descriptor:
>>   bLength                18
>>   bDescriptorType         1
>>   bcdUSB               3.00
>>   bDeviceClass            0
>>   bDeviceSubClass         0
>>   bDeviceProtocol         0
>>   bMaxPacketSize0         8
> 
> A USB-3 device, running at SuperSpeed with its bMaxPacketSize0 set to 8 
> instead of 9?  Presumably this thing never received a USB certification.  
> Does the packaging use the USB logo?
> 
>>   idVendor           0xfb5d
>>   idProduct          0x0001
>>   bcdDevice            0.00
>>   iManufacturer           1 BHYVE
>>   iProduct                2 HID Tablet
>>   iSerial                 3 01
>>   bNumConfigurations      1
> 
> Why on earth would an HID tablet need to run at SuperSpeed?
> 
>> Binary Object Store Descriptor:
>>   bLength                 5
>>   bDescriptorType        15
>>   wTotalLength       0x000f
>>   bNumDeviceCaps          1
>>   SuperSpeed USB Device Capability:
>>     bLength                10
>>     bDescriptorType        16
>>     bDevCapabilityType      3
>>     bmAttributes         0x00
>>     wSpeedsSupported   0x0008
>>       Device can operate at SuperSpeed (5Gbps)
>>     bFunctionalitySupport   3
>>       Lowest fully-functional device speed is SuperSpeed (5Gbps)
>>     bU1DevExitLat          10 micro seconds
>>     bU2DevExitLat          32 micro seconds
> 
> A tablet not capable of running at any speed below 5 Gbps?
> 
>> we are getting a regression on enumeration. It used to work with the
>> code prior to your patch. Takashi is proposing the attached fixed.
>> It looks like we are a bit too restrictive and should just try.
>>
>> 	Regards
>> 		Oliver
> 
>> From: Takashi Iwai <tiwai@suse.de>
>> Subject: [PATCH] USB: hub: Workaround for buggy max packet size with super
>>  speed
>> Patch-mainline: Not yet, testing
>> References: bsc#1220569
>>
>> Signed-off-by: Takashi Iwai <tiwai@suse.de>
>>
>> ---
>>  drivers/usb/core/hub.c | 13 ++++++++++---
>>  1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
>> index e38a4124f610..64193effc456 100644
>> --- a/drivers/usb/core/hub.c
>> +++ b/drivers/usb/core/hub.c
>> @@ -4830,7 +4830,7 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
>>  	const char		*driver_name;
>>  	bool			do_new_scheme;
>>  	const bool		initial = !dev_descr;
>> -	int			maxp0;
>> +	int			maxp0, ep0_maxp;
>>  	struct usb_device_descriptor	*buf, *descr;
>>  
>>  	buf = kmalloc(GET_DESCRIPTOR_BUFSIZE, GFP_NOIO);
>> @@ -5070,7 +5070,8 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
>>  		else
>>  			i = 0;		/* Invalid */
>>  	}
>> -	if (usb_endpoint_maxp(&udev->ep0.desc) == i) {
>> +	ep0_maxp = usb_endpoint_maxp(&udev->ep0.desc);
>> +	if (ep0_maxp == i) {
> 
> This variable looks like it was left over from earlier testing.  It's 
> not really needed.
> 
>>  		;	/* Initial ep0 maxpacket guess is right */
>>  	} else if ((udev->speed == USB_SPEED_FULL ||
>>  				udev->speed == USB_SPEED_HIGH) &&
>> @@ -5082,9 +5083,15 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
>>  			dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
>>  		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
>>  		usb_ep0_reinit(udev);
>> +	} else if (udev->speed >= USB_SPEED_SUPER && initial) {
>> +		/* FIXME: should be more restrictive? */
>> +		/* Initial guess is wrong; use the descriptor's value */
>> +		dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
>> +		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
>> +		usb_ep0_reinit(udev);
> 
> This could be merged with the previous case fairly easily.
> 
>>  	} else {
>>  		/* Initial guess is wrong and descriptor's value is invalid */
>> -		dev_err(&udev->dev, "Invalid ep0 maxpacket: %d\n", maxp0);
>> +		dev_err(&udev->dev, "Invalid ep0 maxpacket: %d, expected %d\n", maxp0, ep0_maxp);
> 
> This also looks like a remnant from earlier testing.
> 
> Alan Stern
> 
>>  		retval = -EMSGSIZE;
>>  		goto fail;
>>  	}
>> -- 
>> 2.35.3
>>
> 

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

[Alan Stern]

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).

Alan Stern

> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> --
> Everything you wanna know about Linux kernel regression tracking:
> https://linux-regtracking.leemhuis.info/about/#tldr
> If I did something stupid, please tell me, as explained on that page.
> 
> #regzbot poke
> 
> On 09.04.24 16:56, Alan Stern wrote:
> > On Tue, Apr 09, 2024 at 03:49:01PM +0200, Oliver Neukum wrote:
> >> Hi,
> >>
> >> with the following device:
> >>
> >> Bus 002 Device 002: ID fb5d:0001 BHYVE HID Tablet
> >> Device Descriptor:
> >>   bLength                18
> >>   bDescriptorType         1
> >>   bcdUSB               3.00
> >>   bDeviceClass            0
> >>   bDeviceSubClass         0
> >>   bDeviceProtocol         0
> >>   bMaxPacketSize0         8
> > 
> > A USB-3 device, running at SuperSpeed with its bMaxPacketSize0 set to 8 
> > instead of 9?  Presumably this thing never received a USB certification.  
> > Does the packaging use the USB logo?
> > 
> >>   idVendor           0xfb5d
> >>   idProduct          0x0001
> >>   bcdDevice            0.00
> >>   iManufacturer           1 BHYVE
> >>   iProduct                2 HID Tablet
> >>   iSerial                 3 01
> >>   bNumConfigurations      1
> > 
> > Why on earth would an HID tablet need to run at SuperSpeed?
> > 
> >> Binary Object Store Descriptor:
> >>   bLength                 5
> >>   bDescriptorType        15
> >>   wTotalLength       0x000f
> >>   bNumDeviceCaps          1
> >>   SuperSpeed USB Device Capability:
> >>     bLength                10
> >>     bDescriptorType        16
> >>     bDevCapabilityType      3
> >>     bmAttributes         0x00
> >>     wSpeedsSupported   0x0008
> >>       Device can operate at SuperSpeed (5Gbps)
> >>     bFunctionalitySupport   3
> >>       Lowest fully-functional device speed is SuperSpeed (5Gbps)
> >>     bU1DevExitLat          10 micro seconds
> >>     bU2DevExitLat          32 micro seconds
> > 
> > A tablet not capable of running at any speed below 5 Gbps?
> > 
> >> we are getting a regression on enumeration. It used to work with the
> >> code prior to your patch. Takashi is proposing the attached fixed.
> >> It looks like we are a bit too restrictive and should just try.
> >>
> >> 	Regards
> >> 		Oliver
> > 
> >> From: Takashi Iwai <tiwai@suse.de>
> >> Subject: [PATCH] USB: hub: Workaround for buggy max packet size with super
> >>  speed
> >> Patch-mainline: Not yet, testing
> >> References: bsc#1220569
> >>
> >> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> >>
> >> ---
> >>  drivers/usb/core/hub.c | 13 ++++++++++---
> >>  1 file changed, 10 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> >> index e38a4124f610..64193effc456 100644
> >> --- a/drivers/usb/core/hub.c
> >> +++ b/drivers/usb/core/hub.c
> >> @@ -4830,7 +4830,7 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
> >>  	const char		*driver_name;
> >>  	bool			do_new_scheme;
> >>  	const bool		initial = !dev_descr;
> >> -	int			maxp0;
> >> +	int			maxp0, ep0_maxp;
> >>  	struct usb_device_descriptor	*buf, *descr;
> >>  
> >>  	buf = kmalloc(GET_DESCRIPTOR_BUFSIZE, GFP_NOIO);
> >> @@ -5070,7 +5070,8 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
> >>  		else
> >>  			i = 0;		/* Invalid */
> >>  	}
> >> -	if (usb_endpoint_maxp(&udev->ep0.desc) == i) {
> >> +	ep0_maxp = usb_endpoint_maxp(&udev->ep0.desc);
> >> +	if (ep0_maxp == i) {
> > 
> > This variable looks like it was left over from earlier testing.  It's 
> > not really needed.
> > 
> >>  		;	/* Initial ep0 maxpacket guess is right */
> >>  	} else if ((udev->speed == USB_SPEED_FULL ||
> >>  				udev->speed == USB_SPEED_HIGH) &&
> >> @@ -5082,9 +5083,15 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
> >>  			dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
> >>  		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
> >>  		usb_ep0_reinit(udev);
> >> +	} else if (udev->speed >= USB_SPEED_SUPER && initial) {
> >> +		/* FIXME: should be more restrictive? */
> >> +		/* Initial guess is wrong; use the descriptor's value */
> >> +		dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
> >> +		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
> >> +		usb_ep0_reinit(udev);
> > 
> > This could be merged with the previous case fairly easily.
> > 
> >>  	} else {
> >>  		/* Initial guess is wrong and descriptor's value is invalid */
> >> -		dev_err(&udev->dev, "Invalid ep0 maxpacket: %d\n", maxp0);
> >> +		dev_err(&udev->dev, "Invalid ep0 maxpacket: %d, expected %d\n", maxp0, ep0_maxp);
> > 
> > This also looks like a remnant from earlier testing.
> > 
> > Alan Stern
> > 
> >>  		retval = -EMSGSIZE;
> >>  		goto fail;
> >>  	}
> >> -- 
> >> 2.35.3
> >>
> > 
> 

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

[Takashi Iwai]

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

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.


thanks,

Takashi

> 
> Alan Stern
> 
> > Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> > --
> > Everything you wanna know about Linux kernel regression tracking:
> > https://linux-regtracking.leemhuis.info/about/#tldr
> > If I did something stupid, please tell me, as explained on that page.
> > 
> > #regzbot poke
> > 
> > On 09.04.24 16:56, Alan Stern wrote:
> > > On Tue, Apr 09, 2024 at 03:49:01PM +0200, Oliver Neukum wrote:
> > >> Hi,
> > >>
> > >> with the following device:
> > >>
> > >> Bus 002 Device 002: ID fb5d:0001 BHYVE HID Tablet
> > >> Device Descriptor:
> > >>   bLength                18
> > >>   bDescriptorType         1
> > >>   bcdUSB               3.00
> > >>   bDeviceClass            0
> > >>   bDeviceSubClass         0
> > >>   bDeviceProtocol         0
> > >>   bMaxPacketSize0         8
> > > 
> > > A USB-3 device, running at SuperSpeed with its bMaxPacketSize0 set to 8 
> > > instead of 9?  Presumably this thing never received a USB certification.  
> > > Does the packaging use the USB logo?
> > > 
> > >>   idVendor           0xfb5d
> > >>   idProduct          0x0001
> > >>   bcdDevice            0.00
> > >>   iManufacturer           1 BHYVE
> > >>   iProduct                2 HID Tablet
> > >>   iSerial                 3 01
> > >>   bNumConfigurations      1
> > > 
> > > Why on earth would an HID tablet need to run at SuperSpeed?
> > > 
> > >> Binary Object Store Descriptor:
> > >>   bLength                 5
> > >>   bDescriptorType        15
> > >>   wTotalLength       0x000f
> > >>   bNumDeviceCaps          1
> > >>   SuperSpeed USB Device Capability:
> > >>     bLength                10
> > >>     bDescriptorType        16
> > >>     bDevCapabilityType      3
> > >>     bmAttributes         0x00
> > >>     wSpeedsSupported   0x0008
> > >>       Device can operate at SuperSpeed (5Gbps)
> > >>     bFunctionalitySupport   3
> > >>       Lowest fully-functional device speed is SuperSpeed (5Gbps)
> > >>     bU1DevExitLat          10 micro seconds
> > >>     bU2DevExitLat          32 micro seconds
> > > 
> > > A tablet not capable of running at any speed below 5 Gbps?
> > > 
> > >> we are getting a regression on enumeration. It used to work with the
> > >> code prior to your patch. Takashi is proposing the attached fixed.
> > >> It looks like we are a bit too restrictive and should just try.
> > >>
> > >> 	Regards
> > >> 		Oliver
> > > 
> > >> From: Takashi Iwai <tiwai@suse.de>
> > >> Subject: [PATCH] USB: hub: Workaround for buggy max packet size with super
> > >>  speed
> > >> Patch-mainline: Not yet, testing
> > >> References: bsc#1220569
> > >>
> > >> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > >>
> > >> ---
> > >>  drivers/usb/core/hub.c | 13 ++++++++++---
> > >>  1 file changed, 10 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > >> index e38a4124f610..64193effc456 100644
> > >> --- a/drivers/usb/core/hub.c
> > >> +++ b/drivers/usb/core/hub.c
> > >> @@ -4830,7 +4830,7 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
> > >>  	const char		*driver_name;
> > >>  	bool			do_new_scheme;
> > >>  	const bool		initial = !dev_descr;
> > >> -	int			maxp0;
> > >> +	int			maxp0, ep0_maxp;
> > >>  	struct usb_device_descriptor	*buf, *descr;
> > >>  
> > >>  	buf = kmalloc(GET_DESCRIPTOR_BUFSIZE, GFP_NOIO);
> > >> @@ -5070,7 +5070,8 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
> > >>  		else
> > >>  			i = 0;		/* Invalid */
> > >>  	}
> > >> -	if (usb_endpoint_maxp(&udev->ep0.desc) == i) {
> > >> +	ep0_maxp = usb_endpoint_maxp(&udev->ep0.desc);
> > >> +	if (ep0_maxp == i) {
> > > 
> > > This variable looks like it was left over from earlier testing.  It's 
> > > not really needed.
> > > 
> > >>  		;	/* Initial ep0 maxpacket guess is right */
> > >>  	} else if ((udev->speed == USB_SPEED_FULL ||
> > >>  				udev->speed == USB_SPEED_HIGH) &&
> > >> @@ -5082,9 +5083,15 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
> > >>  			dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
> > >>  		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
> > >>  		usb_ep0_reinit(udev);
> > >> +	} else if (udev->speed >= USB_SPEED_SUPER && initial) {
> > >> +		/* FIXME: should be more restrictive? */
> > >> +		/* Initial guess is wrong; use the descriptor's value */
> > >> +		dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
> > >> +		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
> > >> +		usb_ep0_reinit(udev);
> > > 
> > > This could be merged with the previous case fairly easily.
> > > 
> > >>  	} else {
> > >>  		/* Initial guess is wrong and descriptor's value is invalid */
> > >> -		dev_err(&udev->dev, "Invalid ep0 maxpacket: %d\n", maxp0);
> > >> +		dev_err(&udev->dev, "Invalid ep0 maxpacket: %d, expected %d\n", maxp0, ep0_maxp);
> > > 
> > > This also looks like a remnant from earlier testing.
> > > 
> > > Alan Stern
> > > 
> > >>  		retval = -EMSGSIZE;
> > >>  		goto fail;
> > >>  	}
> > >> -- 
> > >> 2.35.3
> > >>
> > > 
> > 

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

[Alan Stern]

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?

> 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.

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);

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

[Takashi Iwai]

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);
> 

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

[Alan Stern]

On Wed, Apr 24, 2024 at 10:56:11AM +0200, Takashi Iwai wrote:
> 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:
> > > 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"...

Still, it would be good to report the incorrect descriptor to the package's 
maintainer.

Alan Stern

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

[Takashi Iwai]

On Wed, 24 Apr 2024 16:14:23 +0200,
Alan Stern wrote:
> 
> On Wed, Apr 24, 2024 at 10:56:11AM +0200, Takashi Iwai wrote:
> > 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:
> > > > 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"...
> 
> Still, it would be good to report the incorrect descriptor to the package's 
> maintainer.

Well, it's no Linux package.


Takashi

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

[Alan Stern]

On Wed, Apr 24, 2024 at 04:22:44PM +0200, Takashi Iwai wrote:
> On Wed, 24 Apr 2024 16:14:23 +0200,
> Alan Stern wrote:
> > 
> > On Wed, Apr 24, 2024 at 10:56:11AM +0200, Takashi Iwai wrote:
> > > 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:
> > > > > 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"...
> > 
> > Still, it would be good to report the incorrect descriptor to the package's 
> > maintainer.
> 
> Well, it's no Linux package.

Doesn't matter.  Bugs should always be reported when possible, for any kind 
of package.  Especially after you spent so much time and effort to track 
this one down.

The fact that xz isn't specifically a Windows package didn't prevent Andres 
Freund from reporting the recent backdoor problem.

Alan Stern

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

[Takashi Iwai]

On Wed, 24 Apr 2024 16:56:53 +0200,
Alan Stern wrote:
> 
> On Wed, Apr 24, 2024 at 04:22:44PM +0200, Takashi Iwai wrote:
> > On Wed, 24 Apr 2024 16:14:23 +0200,
> > Alan Stern wrote:
> > > 
> > > On Wed, Apr 24, 2024 at 10:56:11AM +0200, Takashi Iwai wrote:
> > > > 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:
> > > > > > 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"...
> > > 
> > > Still, it would be good to report the incorrect descriptor to the package's 
> > > maintainer.
> > 
> > Well, it's no Linux package.
> 
> Doesn't matter.  Bugs should always be reported when possible, for any kind 
> of package.  Especially after you spent so much time and effort to track 
> this one down.
> 
> The fact that xz isn't specifically a Windows package didn't prevent Andres 
> Freund from reporting the recent backdoor problem.

Yeah, sure.  Any people can report to https://bhyve.npulse.net/


Takashi

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

[Takashi Iwai]

On Wed, 24 Apr 2024 10:56:11 +0200,
Takashi Iwai wrote:
> 
> 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:
> > > 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.

The reporter confirmed that it's still working.


thanks,

Takashi

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

[Alan Stern]

On Sun, Apr 28, 2024 at 09:57:42AM +0200, Takashi Iwai wrote:
> On Wed, 24 Apr 2024 10:56:11 +0200,
> Takashi Iwai wrote:
> > 
> > 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:
> > > > 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.
> 
> The reporter confirmed that it's still working.

Can you get a Reported-and-Tested-by: from the reporter?

Alan Stern

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

[Takashi Iwai]

On Mon, 29 Apr 2024 15:28:59 +0200,
Alan Stern wrote:
> 
> On Sun, Apr 28, 2024 at 09:57:42AM +0200, Takashi Iwai wrote:
> > On Wed, 24 Apr 2024 10:56:11 +0200,
> > Takashi Iwai wrote:
> > > 
> > > 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:
> > > > > 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.
> > 
> > The reporter confirmed that it's still working.
> 
> Can you get a Reported-and-Tested-by: from the reporter?

Yes, I got it from bugzilla entry, please put

  Reported-and-tested-by: Roger Whittaker <roger.whittaker@suse.com>
  Link: https://bugzilla.suse.com/show_bug.cgi?id=1220569


Thanks!

Takashi

[PATCH] usb: Fix regression caused by invalid ep0 maxpacket in virtual SuperSpeed device

[Alan Stern]

A virtual SuperSpeed device in the FreeBSD BVCP package
(https://bhyve.npulse.net/) presents an invalid ep0 maxpacket size of 256.
It stopped working with Linux following a recent commit because now we
check these sizes more carefully than before.

Fix this regression by using the bMaxpacketSize0 value in the device
descriptor for SuperSpeed or faster devices, even if it is invalid.  This
is a very simple-minded change; we might want to check more carefully for
values that actually make some sense (for instance, no smaller than 64).

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-and-tested-by: Roger Whittaker <roger.whittaker@suse.com>
Closes: https://bugzilla.suse.com/show_bug.cgi?id=1220569
Link: https://lore.kernel.org/linux-usb/9efbd569-7059-4575-983f-0ea30df41871@suse.com/
Fixes: 59cf44575456 ("USB: core: Fix oversight in SuperSpeed initialization")
Cc: stable@vger.kernel.org

---

 drivers/usb/core/hub.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

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);

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

[Takashi Iwai]

On Tue, 09 Apr 2024 16:56:53 +0200,
Alan Stern wrote:
> 
> On Tue, Apr 09, 2024 at 03:49:01PM +0200, Oliver Neukum wrote:
> > Hi,
> > 
> > with the following device:
> > 
> > Bus 002 Device 002: ID fb5d:0001 BHYVE HID Tablet
> > Device Descriptor:
> >   bLength                18
> >   bDescriptorType         1
> >   bcdUSB               3.00
> >   bDeviceClass            0
> >   bDeviceSubClass         0
> >   bDeviceProtocol         0
> >   bMaxPacketSize0         8
> 
> A USB-3 device, running at SuperSpeed with its bMaxPacketSize0 set to 8 
> instead of 9?  Presumably this thing never received a USB certification.  
> Does the packaging use the USB logo?

IIUC from the report, this is a virtualization environment, no real
device:
  https://bhyve.npulse.net/


Takashi

> >   idVendor           0xfb5d
> >   idProduct          0x0001
> >   bcdDevice            0.00
> >   iManufacturer           1 BHYVE
> >   iProduct                2 HID Tablet
> >   iSerial                 3 01
> >   bNumConfigurations      1
> 
> Why on earth would an HID tablet need to run at SuperSpeed?
> 
> > Binary Object Store Descriptor:
> >   bLength                 5
> >   bDescriptorType        15
> >   wTotalLength       0x000f
> >   bNumDeviceCaps          1
> >   SuperSpeed USB Device Capability:
> >     bLength                10
> >     bDescriptorType        16
> >     bDevCapabilityType      3
> >     bmAttributes         0x00
> >     wSpeedsSupported   0x0008
> >       Device can operate at SuperSpeed (5Gbps)
> >     bFunctionalitySupport   3
> >       Lowest fully-functional device speed is SuperSpeed (5Gbps)
> >     bU1DevExitLat          10 micro seconds
> >     bU2DevExitLat          32 micro seconds
> 
> A tablet not capable of running at any speed below 5 Gbps?
> 
> > we are getting a regression on enumeration. It used to work with the
> > code prior to your patch. Takashi is proposing the attached fixed.
> > It looks like we are a bit too restrictive and should just try.
> > 
> > 	Regards
> > 		Oliver
> 
> > From: Takashi Iwai <tiwai@suse.de>
> > Subject: [PATCH] USB: hub: Workaround for buggy max packet size with super
> >  speed
> > Patch-mainline: Not yet, testing
> > References: bsc#1220569
> > 
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > 
> > ---
> >  drivers/usb/core/hub.c | 13 ++++++++++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index e38a4124f610..64193effc456 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -4830,7 +4830,7 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
> >  	const char		*driver_name;
> >  	bool			do_new_scheme;
> >  	const bool		initial = !dev_descr;
> > -	int			maxp0;
> > +	int			maxp0, ep0_maxp;
> >  	struct usb_device_descriptor	*buf, *descr;
> >  
> >  	buf = kmalloc(GET_DESCRIPTOR_BUFSIZE, GFP_NOIO);
> > @@ -5070,7 +5070,8 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
> >  		else
> >  			i = 0;		/* Invalid */
> >  	}
> > -	if (usb_endpoint_maxp(&udev->ep0.desc) == i) {
> > +	ep0_maxp = usb_endpoint_maxp(&udev->ep0.desc);
> > +	if (ep0_maxp == i) {
> 
> This variable looks like it was left over from earlier testing.  It's 
> not really needed.
> 
> >  		;	/* Initial ep0 maxpacket guess is right */
> >  	} else if ((udev->speed == USB_SPEED_FULL ||
> >  				udev->speed == USB_SPEED_HIGH) &&
> > @@ -5082,9 +5083,15 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
> >  			dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
> >  		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
> >  		usb_ep0_reinit(udev);
> > +	} else if (udev->speed >= USB_SPEED_SUPER && initial) {
> > +		/* FIXME: should be more restrictive? */
> > +		/* Initial guess is wrong; use the descriptor's value */
> > +		dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
> > +		udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
> > +		usb_ep0_reinit(udev);
> 
> This could be merged with the previous case fairly easily.
> 
> >  	} else {
> >  		/* Initial guess is wrong and descriptor's value is invalid */
> > -		dev_err(&udev->dev, "Invalid ep0 maxpacket: %d\n", maxp0);
> > +		dev_err(&udev->dev, "Invalid ep0 maxpacket: %d, expected %d\n", maxp0, ep0_maxp);
> 
> This also looks like a remnant from earlier testing.
> 
> Alan Stern
> 
> >  		retval = -EMSGSIZE;
> >  		goto fail;
> >  	}
> > -- 
> > 2.35.3
> > 
>