[PATCH v2] usbcore: Add quirk for 255-bytes initial config read

[PATCH v2] usbcore: Add quirk for 255-bytes initial config read

[Nikhil Solanke]

Certain third-party USB game controllers exposing (or spoofing) an Xbox
360-compatible interface (VID:PID 045e:028e) fail to enumerate under Linux.
The device disconnects from the bus without responding to the initial
GET_DESCRIPTOR(CONFIGURATION) request, and the kernel logs 'unable to read
config index 0 descriptor/start: -71'.

The device then falls back to a secondary Android HID mode (with a
different VID:PID), losing XInput functionality including rumble support.
The failure reproduces across multiple machines, host controller types, and
kernel versions including current mainline and LTS. The device enumerates
correctly and remains in XInput mode under Windows. Notably, the device
enumerates correctly in Android mode when the same 9-byte request
is issued for that mode's configuration descriptor, confirming the firmware
bug is specific to the XInput mode.

usbmon traces from Linux and Wireshark/USBPcap traces from Windows are
identical up to the point of failure, with no visible protocol-level
difference explaining the divergence. The root cause was identified when
Michal Pecio discovered via a QEMU bus-level capture that Windows does not
use wLength=9 for the initial config descriptor request; it uses
wLength=255. Alan Stern subsequently confirmed this with a bus
analyzer on a different USB 2.0 device, and Michal verified the behavior
goes back to Windows 95 OSR2.1.

So, add a new quirk flag USB_QUIRK_CONFIG_SIZE which causes
usb_get_configuration() to issue a 255 byte sized configuration request
instead of USB_DT_CONFIG_SIZE (9) for the initial
GET_DESCRIPTOR(CONFIGURATION) request, mimicking long-standing Windows
behavior.

Suggested-by: Alan Stern <stern@rowland.harvard.edu>
Suggested-by: Michal Pecio <michal.pecio@gmail.com>
Closes: https://lore.kernel.org/linux-usb/CAFgddh+JWdT4LLwMc5qjM8q_pBu-fRo2qADR5ovAKoGHWMQrRw@mail.gmail.com/
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org

Signed-off-by: Nikhil Solanke <nikhilsolanke5@gmail.com>
---
Changes in v2:
- Add Documentation
- Naming changes
- Refactored to have a better flow with existing code.

 .../admin-guide/kernel-parameters.txt         |  9 +++
 drivers/usb/core/config.c                     | 61 ++++++++++++++-----
 drivers/usb/core/hub.c                        |  6 +-
 drivers/usb/core/quirks.c                     |  4 ++
 include/linux/usb/quirks.h                    |  3 +
 5 files changed, 67 insertions(+), 16 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 97007f4f69d4..af4bf0ef2c7b 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -8158,6 +8158,15 @@ Kernel parameters
 				q = USB_QUIRK_FORCE_ONE_CONFIG (Device
 					claims zero configurations,
 					forcing to 1);
+                r = USB_QUIRK_WINDOWS_CONFIG_REQ_SIZE (Device
+                    fails during initialization when asked for
+                    9-bytes configuration desciptor request. Ask
+                    for 255-bytes request instead to mirror
+                    Windows' behavior. This quirk is originally
+                    meant to fix some quirky gamepads that refuse
+                    to connect in their XInput mode. But it can also
+                    potentially fix issues with other USB devices
+                    that work on Windows but not on Linux)
 			Example: quirks=0781:5580:bk,0a5c:5834:gij
 
 	usbhid.mousepoll=
diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
index 45e20c6d76c0..4fc3145404d6 100644
--- a/drivers/usb/core/config.c
+++ b/drivers/usb/core/config.c
@@ -19,6 +19,9 @@
 
 #define USB_MAXCONFIG			8	/* Arbitrary limit */
 
+/* config req size if USB_QUIRK_WINDOWS_CONFIG_REQ_SIZE is set */
+#define USB_CONFIG_WINDOWS_REQ_SIZE	255
+
 static int find_next_descriptor(unsigned char *buffer, int size,
     int dt1, int dt2, int *num_skipped)
 {
@@ -912,6 +915,13 @@ int usb_get_configuration(struct usb_device *dev)
 	unsigned char *bigbuffer;
 	struct usb_config_descriptor *desc;
 	int result;
+	/*
+	 * Devices with quirky firmware will stall or reset when asked only for
+	 * the configuration header. This variable decides which size to use in
+	 * that case, if the quirk for that device was set.
+	 */
+	size_t usb_config_req_size = (dev->quirks & USB_QUIRK_WINDOWS_CONFIG_REQ_SIZE)
+		? USB_CONFIG_WINDOWS_REQ_SIZE : USB_DT_CONFIG_SIZE;
 
 	if (ncfg > USB_MAXCONFIG) {
 		dev_notice(ddev, "too many configurations: %d, "
@@ -938,18 +948,27 @@ int usb_get_configuration(struct usb_device *dev)
 	if (!dev->rawdescriptors)
 		return -ENOMEM;
 
-	desc = kmalloc(USB_DT_CONFIG_SIZE, GFP_KERNEL);
+	desc = kmalloc(usb_config_req_size, GFP_KERNEL);
+
 	if (!desc)
 		return -ENOMEM;
 
 	for (cfgno = 0; cfgno < ncfg; cfgno++) {
-		/* We grab just the first descriptor so we know how long
-		 * the whole configuration is */
+
+		if (dev->quirks & USB_QUIRK_DELAY_INIT)
+			msleep(200);
+
+		/*
+		 * Grab just the first descriptor so we know how long the whole
+		 * configuration is. In case of quirky firmware, try to grab the
+		 * whole thing in one go by asking for a 255-bytes sized buffer
+		 * mirroring Windows behavior.
+		 */
 		result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno,
-		    desc, USB_DT_CONFIG_SIZE);
+						desc, usb_config_req_size);
 		if (result < 0) {
 			dev_err(ddev, "unable to read config index %d "
-			    "descriptor/%s: %d\n", cfgno, "start", result);
+				"descriptor/%s: %d\n", cfgno, "start", result);
 			if (result != -EPIPE)
 				goto err;
 			dev_notice(ddev, "chopping to %d config(s)\n", cfgno);
@@ -957,13 +976,25 @@ int usb_get_configuration(struct usb_device *dev)
 			break;
 		} else if (result < 4) {
 			dev_err(ddev, "config index %d descriptor too short "
-			    "(expected %i, got %i)\n", cfgno,
-			    USB_DT_CONFIG_SIZE, result);
+				"(asked for %zu, got %i, expected at least %i)\n",
+				cfgno, usb_config_req_size, result, 4);
 			result = -EINVAL;
 			goto err;
 		}
+
 		length = max_t(int, le16_to_cpu(desc->wTotalLength),
-		    USB_DT_CONFIG_SIZE);
+				USB_DT_CONFIG_SIZE);
+
+		/*
+		 * If the device returns the full length configuration
+		 * descriptor, skip the second read. Otherwise, send a second
+		 * request asking for the full length.
+		 */
+		if (result >= le16_to_cpu(desc->wTotalLength)) {
+			bigbuffer = (unsigned char *) desc;
+			desc = NULL;
+			goto store_and_parse;
+		}
 
 		/* Now that we know the length, get the whole thing */
 		bigbuffer = kmalloc(length, GFP_KERNEL);
@@ -972,23 +1003,25 @@ int usb_get_configuration(struct usb_device *dev)
 			goto err;
 		}
 
-		if (dev->quirks & USB_QUIRK_DELAY_INIT)
-			msleep(200);
-
 		result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno,
-		    bigbuffer, length);
+						bigbuffer, length);
+
 		if (result < 0) {
 			dev_err(ddev, "unable to read config index %d "
-			    "descriptor/%s\n", cfgno, "all");
+				"descriptor/%s\n", cfgno, "all");
 			kfree(bigbuffer);
 			goto err;
 		}
+
 		if (result < length) {
 			dev_notice(ddev, "config index %d descriptor too short "
-			    "(expected %i, got %i)\n", cfgno, length, result);
+				"(asked for %i, got %i)\n",
+				cfgno, length, result);
 			length = result;
 		}
 
+store_and_parse:
+		krealloc(bigbuffer, length, GFP_KERNEL);
 		dev->rawdescriptors[cfgno] = bigbuffer;
 
 		result = usb_parse_configuration(dev, cfgno,
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 24960ba9caa9..9acd278666fc 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2527,8 +2527,10 @@ static int usb_enumerate_device(struct usb_device *udev)
 		err = usb_get_configuration(udev);
 		if (err < 0) {
 			if (err != -ENODEV)
-				dev_err(&udev->dev, "can't read configurations, error %d\n",
-						err);
+				dev_err(&udev->dev, "can't read configurations, "
+					"for device %04x:%04x, error %d\n",
+					le16_to_cpu(udev->descriptor.idVendor),
+					le16_to_cpu(udev->descriptor.idProduct), err);
 			return err;
 		}
 	}
diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index 87810eff974e..df670b0b66fe 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -142,6 +142,10 @@ static int quirks_param_set(const char *value, const struct kernel_param *kp)
 				break;
 			case 'q':
 				flags |= USB_QUIRK_FORCE_ONE_CONFIG;
+				break;
+			case 'r':
+				flags |= USB_QUIRK_WINDOWS_CONFIG_REQ_SIZE;
+				break;
 			/* Ignore unrecognized flag characters */
 			}
 		}
diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h
index b3cc7beab4a3..a4043b33c2c2 100644
--- a/include/linux/usb/quirks.h
+++ b/include/linux/usb/quirks.h
@@ -81,4 +81,7 @@
 /* Device claims zero configurations, forcing to 1 */
 #define USB_QUIRK_FORCE_ONE_CONFIG		BIT(18)
 
+/* Use a 255 bytes config descriptor request mirroring windows behavior */
+#define USB_QUIRK_WINDOWS_CONFIG_REQ_SIZE	BIT(19)
+
 #endif /* __LINUX_USB_QUIRKS_H */
-- 
2.54.0

Re: [PATCH v2] usbcore: Add quirk for 255-bytes initial config read

[Randy Dunlap]

On 6/23/26 9:10 AM, Nikhil Solanke wrote:
> Certain third-party USB game controllers exposing (or spoofing) an Xbox
> 360-compatible interface (VID:PID 045e:028e) fail to enumerate under Linux.
> The device disconnects from the bus without responding to the initial
> GET_DESCRIPTOR(CONFIGURATION) request, and the kernel logs 'unable to read
> config index 0 descriptor/start: -71'.
> 
> The device then falls back to a secondary Android HID mode (with a
> different VID:PID), losing XInput functionality including rumble support.
> The failure reproduces across multiple machines, host controller types, and
> kernel versions including current mainline and LTS. The device enumerates
> correctly and remains in XInput mode under Windows. Notably, the device
> enumerates correctly in Android mode when the same 9-byte request
> is issued for that mode's configuration descriptor, confirming the firmware
> bug is specific to the XInput mode.
> 
> usbmon traces from Linux and Wireshark/USBPcap traces from Windows are
> identical up to the point of failure, with no visible protocol-level
> difference explaining the divergence. The root cause was identified when
> Michal Pecio discovered via a QEMU bus-level capture that Windows does not
> use wLength=9 for the initial config descriptor request; it uses
> wLength=255. Alan Stern subsequently confirmed this with a bus
> analyzer on a different USB 2.0 device, and Michal verified the behavior
> goes back to Windows 95 OSR2.1.
> 
> So, add a new quirk flag USB_QUIRK_CONFIG_SIZE which causes
> usb_get_configuration() to issue a 255 byte sized configuration request
> instead of USB_DT_CONFIG_SIZE (9) for the initial
> GET_DESCRIPTOR(CONFIGURATION) request, mimicking long-standing Windows
> behavior.
> 
> Suggested-by: Alan Stern <stern@rowland.harvard.edu>
> Suggested-by: Michal Pecio <michal.pecio@gmail.com>
> Closes: https://lore.kernel.org/linux-usb/CAFgddh+JWdT4LLwMc5qjM8q_pBu-fRo2qADR5ovAKoGHWMQrRw@mail.gmail.com/
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> 
> Signed-off-by: Nikhil Solanke <nikhilsolanke5@gmail.com>
> ---
> Changes in v2:
> - Add Documentation
> - Naming changes
> - Refactored to have a better flow with existing code.
> 
>  .../admin-guide/kernel-parameters.txt         |  9 +++
>  drivers/usb/core/config.c                     | 61 ++++++++++++++-----
>  drivers/usb/core/hub.c                        |  6 +-
>  drivers/usb/core/quirks.c                     |  4 ++
>  include/linux/usb/quirks.h                    |  3 +
>  5 files changed, 67 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 97007f4f69d4..af4bf0ef2c7b 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -8158,6 +8158,15 @@ Kernel parameters
>  				q = USB_QUIRK_FORCE_ONE_CONFIG (Device
>  					claims zero configurations,
>  					forcing to 1);
> +                r = USB_QUIRK_WINDOWS_CONFIG_REQ_SIZE (Device
> +                    fails during initialization when asked for
> +                    9-bytes configuration desciptor request. Ask

		                             descriptor

> +                    for 255-bytes request instead to mirror
> +                    Windows' behavior. This quirk is originally
> +                    meant to fix some quirky gamepads that refuse
> +                    to connect in their XInput mode. But it can also
> +                    potentially fix issues with other USB devices
> +                    that work on Windows but not on Linux)

add ending '.'

For all lines added here, use tabs instead of spaces for indentation.


>  			Example: quirks=0781:5580:bk,0a5c:5834:gij
>  
>  	usbhid.mousepoll=


-- 
~Randy

Re: [PATCH v2] usbcore: Add quirk for 255-bytes initial config read

[Nikhil Solanke]

> add ending '.'
>
> For all lines added here, use tabs instead of spaces for indentation.

Done! Waiting for any other changes before submitting v3

Re: [PATCH v2] usbcore: Add quirk for 255-bytes initial config read

[Alan Stern]

This is looking better.  A few more things to mention below, but 
otherwise okay.

On Tue, Jun 23, 2026 at 09:40:35PM +0530, Nikhil Solanke wrote:
> Certain third-party USB game controllers exposing (or spoofing) an Xbox
> 360-compatible interface (VID:PID 045e:028e) fail to enumerate under Linux.
> The device disconnects from the bus without responding to the initial
> GET_DESCRIPTOR(CONFIGURATION) request, and the kernel logs 'unable to read
> config index 0 descriptor/start: -71'.
> 
> The device then falls back to a secondary Android HID mode (with a
> different VID:PID), losing XInput functionality including rumble support.
> The failure reproduces across multiple machines, host controller types, and
> kernel versions including current mainline and LTS. The device enumerates
> correctly and remains in XInput mode under Windows. Notably, the device
> enumerates correctly in Android mode when the same 9-byte request
> is issued for that mode's configuration descriptor, confirming the firmware
> bug is specific to the XInput mode.
> 
> usbmon traces from Linux and Wireshark/USBPcap traces from Windows are
> identical up to the point of failure, with no visible protocol-level
> difference explaining the divergence. The root cause was identified when
> Michal Pecio discovered via a QEMU bus-level capture that Windows does not
> use wLength=9 for the initial config descriptor request; it uses
> wLength=255. Alan Stern subsequently confirmed this with a bus
> analyzer on a different USB 2.0 device, and Michal verified the behavior
> goes back to Windows 95 OSR2.1.
> 
> So, add a new quirk flag USB_QUIRK_CONFIG_SIZE which causes
> usb_get_configuration() to issue a 255 byte sized configuration request
> instead of USB_DT_CONFIG_SIZE (9) for the initial
> GET_DESCRIPTOR(CONFIGURATION) request, mimicking long-standing Windows
> behavior.
> 
> Suggested-by: Alan Stern <stern@rowland.harvard.edu>
> Suggested-by: Michal Pecio <michal.pecio@gmail.com>
> Closes: https://lore.kernel.org/linux-usb/CAFgddh+JWdT4LLwMc5qjM8q_pBu-fRo2qADR5ovAKoGHWMQrRw@mail.gmail.com/
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> 
> Signed-off-by: Nikhil Solanke <nikhilsolanke5@gmail.com>

Normally people don't leave a blank line before their Signed-off-by: 
(unless it's the first tag to appear).  But I don't think it makes any 
difference, especially if the checkpatch.pl script doesn't object.

> ---
> Changes in v2:
> - Add Documentation
> - Naming changes
> - Refactored to have a better flow with existing code.
> 
>  .../admin-guide/kernel-parameters.txt         |  9 +++
>  drivers/usb/core/config.c                     | 61 ++++++++++++++-----
>  drivers/usb/core/hub.c                        |  6 +-
>  drivers/usb/core/quirks.c                     |  4 ++
>  include/linux/usb/quirks.h                    |  3 +
>  5 files changed, 67 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 97007f4f69d4..af4bf0ef2c7b 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -8158,6 +8158,15 @@ Kernel parameters
>  				q = USB_QUIRK_FORCE_ONE_CONFIG (Device
>  					claims zero configurations,
>  					forcing to 1);
> +                r = USB_QUIRK_WINDOWS_CONFIG_REQ_SIZE (Device
> +                    fails during initialization when asked for
> +                    9-bytes configuration desciptor request. Ask
> +                    for 255-bytes request instead to mirror
> +                    Windows' behavior. This quirk is originally
> +                    meant to fix some quirky gamepads that refuse
> +                    to connect in their XInput mode. But it can also
> +                    potentially fix issues with other USB devices
> +                    that work on Windows but not on Linux)
>  			Example: quirks=0781:5580:bk,0a5c:5834:gij

As Randy said, use tabs instead of spaces.  And this new entry should be 
aligned with all the preceding entries, and it should end with a ';' 
like they do.

I would leave out the two sentences of explanation, but that's more of a 
personal choice.  We'll see if Greg KH objects.

>  
>  	usbhid.mousepoll=
> diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
> index 45e20c6d76c0..4fc3145404d6 100644
> --- a/drivers/usb/core/config.c
> +++ b/drivers/usb/core/config.c
> @@ -19,6 +19,9 @@
>  
>  #define USB_MAXCONFIG			8	/* Arbitrary limit */
>  
> +/* config req size if USB_QUIRK_WINDOWS_CONFIG_REQ_SIZE is set */
> +#define USB_CONFIG_WINDOWS_REQ_SIZE	255
> +
>  static int find_next_descriptor(unsigned char *buffer, int size,
>      int dt1, int dt2, int *num_skipped)
>  {
> @@ -912,6 +915,13 @@ int usb_get_configuration(struct usb_device *dev)
>  	unsigned char *bigbuffer;
>  	struct usb_config_descriptor *desc;
>  	int result;
> +	/*
> +	 * Devices with quirky firmware will stall or reset when asked only for
> +	 * the configuration header. This variable decides which size to use in
> +	 * that case, if the quirk for that device was set.
> +	 */
> +	size_t usb_config_req_size = (dev->quirks & USB_QUIRK_WINDOWS_CONFIG_REQ_SIZE)
> +		? USB_CONFIG_WINDOWS_REQ_SIZE : USB_DT_CONFIG_SIZE;

It's a little unusual to have a multiline comment in the middle of a 
bunch of variable definitions.  The alternative is to do the assignment 
later on and move the comment there.

>  
>  	if (ncfg > USB_MAXCONFIG) {
>  		dev_notice(ddev, "too many configurations: %d, "
> @@ -938,18 +948,27 @@ int usb_get_configuration(struct usb_device *dev)
>  	if (!dev->rawdescriptors)
>  		return -ENOMEM;
>  
> -	desc = kmalloc(USB_DT_CONFIG_SIZE, GFP_KERNEL);
> +	desc = kmalloc(usb_config_req_size, GFP_KERNEL);
> +
>  	if (!desc)
>  		return -ENOMEM;
>  
>  	for (cfgno = 0; cfgno < ncfg; cfgno++) {
> -		/* We grab just the first descriptor so we know how long
> -		 * the whole configuration is */
> +
> +		if (dev->quirks & USB_QUIRK_DELAY_INIT)
> +			msleep(200);

Moving this delay up here changes the behavior when the quirk flag isn't 
set.  While it agrees with the intention of the USB_QUIRK_DELAY_INIT 
flag, such a change should be mentioned in the patch description.

> +
> +		/*
> +		 * Grab just the first descriptor so we know how long the whole
> +		 * configuration is. In case of quirky firmware, try to grab the
> +		 * whole thing in one go by asking for a 255-bytes sized buffer
> +		 * mirroring Windows behavior.
> +		 */

This needs to be rewritten, as it is self-contradictory.  When the quirk 
flag is set we issue a 255-byte request to mimic the Windows behavior, 
and only when the flag isn't set do we grab just the first descriptor.

>  		result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno,
> -		    desc, USB_DT_CONFIG_SIZE);
> +						desc, usb_config_req_size);

Don't make extraneous changes to the existing indentation (or whitespace 
in general), here and below.

>  		if (result < 0) {
>  			dev_err(ddev, "unable to read config index %d "
> -			    "descriptor/%s: %d\n", cfgno, "start", result);
> +				"descriptor/%s: %d\n", cfgno, "start", result);

At the time this code was originally written, the policy for kernel code 
was to break lines before 80 columns.  Since then the policy has changed 
to avoid splitting long literal strings into pieces, even when that 
means exceeding 80 columns.  So your new string here should all go on 
one line.

>  			if (result != -EPIPE)
>  				goto err;
>  			dev_notice(ddev, "chopping to %d config(s)\n", cfgno);
> @@ -957,13 +976,25 @@ int usb_get_configuration(struct usb_device *dev)
>  			break;
>  		} else if (result < 4) {
>  			dev_err(ddev, "config index %d descriptor too short "
> -			    "(expected %i, got %i)\n", cfgno,
> -			    USB_DT_CONFIG_SIZE, result);
> +				"(asked for %zu, got %i, expected at least %i)\n",
> +				cfgno, usb_config_req_size, result, 4);
>  			result = -EINVAL;
>  			goto err;
>  		}
> +
>  		length = max_t(int, le16_to_cpu(desc->wTotalLength),
> -		    USB_DT_CONFIG_SIZE);
> +				USB_DT_CONFIG_SIZE);

This is another example of a change that has nothing to do with the 
purpose of the patch.

> +
> +		/*
> +		 * If the device returns the full length configuration
> +		 * descriptor, skip the second read. Otherwise, send a second

Strictly speaking, the configuration descriptor is only 9 bytes long.  
What you mean here is the entire configuration descriptor set.

> +		 * request asking for the full length.
> +		 */
> +		if (result >= le16_to_cpu(desc->wTotalLength)) {

Shouldn't this be: result >= length?  No point in repeating the 
le16_to_cpu calculation.

> +			bigbuffer = (unsigned char *) desc;
> +			desc = NULL;
> +			goto store_and_parse;
> +		}
>  
>  		/* Now that we know the length, get the whole thing */
>  		bigbuffer = kmalloc(length, GFP_KERNEL);
> @@ -972,23 +1003,25 @@ int usb_get_configuration(struct usb_device *dev)
>  			goto err;
>  		}
>  
> -		if (dev->quirks & USB_QUIRK_DELAY_INIT)
> -			msleep(200);
> -
>  		result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno,
> -		    bigbuffer, length);
> +						bigbuffer, length);
> +
>  		if (result < 0) {
>  			dev_err(ddev, "unable to read config index %d "
> -			    "descriptor/%s\n", cfgno, "all");
> +				"descriptor/%s\n", cfgno, "all");
>  			kfree(bigbuffer);
>  			goto err;
>  		}
> +

More examples of unnecessary whitespace changes.

>  		if (result < length) {
>  			dev_notice(ddev, "config index %d descriptor too short "
> -			    "(expected %i, got %i)\n", cfgno, length, result);
> +				"(asked for %i, got %i)\n",
> +				cfgno, length, result);
>  			length = result;
>  		}
>  
> +store_and_parse:
> +		krealloc(bigbuffer, length, GFP_KERNEL);
>  		dev->rawdescriptors[cfgno] = bigbuffer;
>  
>  		result = usb_parse_configuration(dev, cfgno,
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 24960ba9caa9..9acd278666fc 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -2527,8 +2527,10 @@ static int usb_enumerate_device(struct usb_device *udev)
>  		err = usb_get_configuration(udev);
>  		if (err < 0) {
>  			if (err != -ENODEV)
> -				dev_err(&udev->dev, "can't read configurations, error %d\n",
> -						err);
> +				dev_err(&udev->dev, "can't read configurations, "
> +					"for device %04x:%04x, error %d\n",

Like above, this string should all be on one line.

Alan Stern

> +					le16_to_cpu(udev->descriptor.idVendor),
> +					le16_to_cpu(udev->descriptor.idProduct), err);
>  			return err;
>  		}
>  	}
> diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
> index 87810eff974e..df670b0b66fe 100644
> --- a/drivers/usb/core/quirks.c
> +++ b/drivers/usb/core/quirks.c
> @@ -142,6 +142,10 @@ static int quirks_param_set(const char *value, const struct kernel_param *kp)
>  				break;
>  			case 'q':
>  				flags |= USB_QUIRK_FORCE_ONE_CONFIG;
> +				break;
> +			case 'r':
> +				flags |= USB_QUIRK_WINDOWS_CONFIG_REQ_SIZE;
> +				break;
>  			/* Ignore unrecognized flag characters */
>  			}
>  		}
> diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h
> index b3cc7beab4a3..a4043b33c2c2 100644
> --- a/include/linux/usb/quirks.h
> +++ b/include/linux/usb/quirks.h
> @@ -81,4 +81,7 @@
>  /* Device claims zero configurations, forcing to 1 */
>  #define USB_QUIRK_FORCE_ONE_CONFIG		BIT(18)
>  
> +/* Use a 255 bytes config descriptor request mirroring windows behavior */
> +#define USB_QUIRK_WINDOWS_CONFIG_REQ_SIZE	BIT(19)
> +
>  #endif /* __LINUX_USB_QUIRKS_H */
> -- 
> 2.54.0
> 

Re: [PATCH v2] usbcore: Add quirk for 255-bytes initial config read

[Nikhil Solanke]

> Moving this delay up here changes the behavior when the quirk flag isn't
> set.  While it agrees with the intention of the USB_QUIRK_DELAY_INIT
> flag, such a change should be mentioned in the patch description.

How should I mention it then? Nothing comes to mind besides the
obvious: "Also move the USB_QUIRK_DELAY_INIT sleep to before the
initial descriptor read, so the delay applies consistently regardless
of whether USB_QUIRK_CONFIG_SIZE is set.". Or should i revert it back
to original position?

> > +
> > +             /*
> > +              * Grab just the first descriptor so we know how long the whole
> > +              * configuration is. In case of quirky firmware, try to grab the
> > +              * whole thing in one go by asking for a 255-bytes sized buffer
> > +              * mirroring Windows behavior.
> > +              */
>
> This needs to be rewritten, as it is self-contradictory.  When the quirk
> flag is set we issue a 255-byte request to mimic the Windows behavior,
> and only when the flag isn't set do we grab just the first descriptor.

I am sorry I didn't understand how it is self contradictory. The
comment does say, "in case of quirky firmware..."? Am i missing
something?

> >               result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno,
> > -                 desc, USB_DT_CONFIG_SIZE);
> > +                                             desc, usb_config_req_size);
>
> Don't make extraneous changes to the existing indentation (or whitespace
> in general), here and below.

Well the linux coding style guidelines mention that those descendants
should preferably be aligned with the function open parenthesis. Since
i did "touch" that line/part of code I though might as well indent it
a bit accordingly. Should i revert the indent then (in this and the
other place)?

> >                       if (result != -EPIPE)
> >                               goto err;
> >                       dev_notice(ddev, "chopping to %d config(s)\n", cfgno);
> > @@ -957,13 +976,25 @@ int usb_get_configuration(struct usb_device *dev)
> >                       break;
> >               } else if (result < 4) {
> >                       dev_err(ddev, "config index %d descriptor too short "
> > -                         "(expected %i, got %i)\n", cfgno,
> > -                         USB_DT_CONFIG_SIZE, result);
> > +                             "(asked for %zu, got %i, expected at least %i)\n",
> > +                             cfgno, usb_config_req_size, result, 4);
> >                       result = -EINVAL;
> >                       goto err;
> >               }
> > +
> >               length = max_t(int, le16_to_cpu(desc->wTotalLength),
> > -                 USB_DT_CONFIG_SIZE);
> > +                             USB_DT_CONFIG_SIZE);
>
> This is another example of a change that has nothing to do with the
> purpose of the patch.

Isn't that what you told me to change? So the logs are accurate? I
made that change because you suggested it. :')

> > +
> > +             /*
> > +              * If the device returns the full length configuration
> > +              * descriptor, skip the second read. Otherwise, send a second
>
> Strictly speaking, the configuration descriptor is only 9 bytes long.
> What you mean here is the entire configuration descriptor set.

Alright i'll reword it.

> > +              * request asking for the full length.
> > +              */
> > +             if (result >= le16_to_cpu(desc->wTotalLength)) {
>
> Shouldn't this be: result >= length?  No point in repeating the
> le16_to_cpu calculation.

Yess. initially the length assignment was happening afterwards in my
patch. then i decided to move it before the "if" statement since the
outcome of length was going to be similar in any case (within if and
after if). but then i forgot to modify the if too. Will fix it.

> Like above, this string should all be on one line.

Will fix all the strings as well

Nikhil Solanke

Re: [PATCH v2] usbcore: Add quirk for 255-bytes initial config read

[Alan Stern]

On Wed, Jun 24, 2026 at 02:44:07AM +0530, Nikhil Solanke wrote:
> > Moving this delay up here changes the behavior when the quirk flag isn't
> > set.  While it agrees with the intention of the USB_QUIRK_DELAY_INIT
> > flag, such a change should be mentioned in the patch description.
> 
> How should I mention it then? Nothing comes to mind besides the
> obvious: "Also move the USB_QUIRK_DELAY_INIT sleep to before the
> initial descriptor read, so the delay applies consistently regardless
> of whether USB_QUIRK_CONFIG_SIZE is set.". Or should i revert it back
> to original position?

Actually, the best approach here would be to put this single change into 
a separate patch that comes before the current one.  That removes issues 
of making more than one functional change in one patch and improves 
bisectability.

But to answer your question: In general, a patch's description should 
explain the reasons for the changes that the patch makes.  Especially 
when a particular change doesn't appear, at first glance, to be related 
to the patch's primary purpose.  (On the other hand, it doesn't need to 
explain in detail what the patch does; we can see that for ourselves 
just by reading the patch's contents.)

> > > +             /*
> > > +              * Grab just the first descriptor so we know how long the whole
> > > +              * configuration is. In case of quirky firmware, try to grab the
> > > +              * whole thing in one go by asking for a 255-bytes sized buffer
> > > +              * mirroring Windows behavior.
> > > +              */
> >
> > This needs to be rewritten, as it is self-contradictory.  When the quirk
> > flag is set we issue a 255-byte request to mimic the Windows behavior,
> > and only when the flag isn't set do we grab just the first descriptor.
> 
> I am sorry I didn't understand how it is self contradictory. The
> comment does say, "in case of quirky firmware..."? Am i missing
> something?

Literally, what the comment says is: Grab just the first descriptor, 
and if the quirk flag is set, get all the descriptors.  That's a 
contradiction -- you can get just the first, or you can get all of 
them, but you can't do both at the same time!

> > >               result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno,
> > > -                 desc, USB_DT_CONFIG_SIZE);
> > > +                                             desc, usb_config_req_size);
> >
> > Don't make extraneous changes to the existing indentation (or whitespace
> > in general), here and below.
> 
> Well the linux coding style guidelines mention that those descendants
> should preferably be aligned with the function open parenthesis. Since
> i did "touch" that line/part of code I though might as well indent it
> a bit accordingly. Should i revert the indent then (in this and the
> other place)?

The style used in this file is to indent continuation lines by 4 spaces, 
because some of the continued statements are extremely long.  If you 
want to align new continuation lines with an open paren, you can -- but 
you didn't even do that in the example above; you aligned it with the 
space following the first comma.

And while you did change some nearby code, you did not change the code 
in this line.  So reformatting it is not justified.

> > >                       if (result != -EPIPE)
> > >                               goto err;
> > >                       dev_notice(ddev, "chopping to %d config(s)\n", cfgno);
> > > @@ -957,13 +976,25 @@ int usb_get_configuration(struct usb_device *dev)
> > >                       break;
> > >               } else if (result < 4) {
> > >                       dev_err(ddev, "config index %d descriptor too short "
> > > -                         "(expected %i, got %i)\n", cfgno,
> > > -                         USB_DT_CONFIG_SIZE, result);
> > > +                             "(asked for %zu, got %i, expected at least %i)\n",
> > > +                             cfgno, usb_config_req_size, result, 4);
> > >                       result = -EINVAL;
> > >                       goto err;
> > >               }
> > > +
> > >               length = max_t(int, le16_to_cpu(desc->wTotalLength),
> > > -                 USB_DT_CONFIG_SIZE);
> > > +                             USB_DT_CONFIG_SIZE);
> >
> > This is another example of a change that has nothing to do with the
> > purpose of the patch.
> 
> Isn't that what you told me to change? So the logs are accurate? I
> made that change because you suggested it. :')

My comment referred to the two lines directly above it, and I did not 
suggest leaving the code exactly the same except for indenting it 
farther.  Or inserting an extra blank line just before the assignment to 
length.

Alan Stern

Re: [PATCH v2] usbcore: Add quirk for 255-bytes initial config read

[Nikhil Solanke]

> Actually, the best approach here would be to put this single change into
> a separate patch that comes before the current one.  That removes issues
> of making more than one functional change in one patch and improves
> bisectability.

Before? Shouldn't it be after my changes? That would make it easier to
justify the changes. And just to be sure, you did mention it does
align with what the intention of USB_QUIRK_DELAY_INIT, but it does
change its behavior when the quirk is not set. Atleast from what I
understood from the documentation and an LLM's summary, the device
needs time to prepare the full configuration set. So, does delaying
before the first header read really work? I can't test this since I
don't have a device that requires the quirk to be set.

I personally think adding a condition to check if the quirk is set and
then delaying before sending the first request would be appropriate.
What are your opinions on this.

> The style used in this file is to indent continuation lines by 4 spaces,
> because some of the continued statements are extremely long.  If you
> want to align new continuation lines with an open paren, you can -- but
> you didn't even do that in the example above; you aligned it with the
> space following the first comma.

I will make my changes more consistent with the existing file, i.e.
continuation with 4 spaces.

Also is it fine if the string lines exceed 100 columns?

Also, is there a need to check for krealloc()'s return value? Since we
are only shrinking the buffer, there won't be any moves or completely
new blocks (at least as per my understanding). Do I still need to
check its return value for completeness' sake?

Re: [PATCH v2] usbcore: Add quirk for 255-bytes initial config read

[Alan Stern]

On Wed, Jun 24, 2026 at 01:36:28PM +0530, Nikhil Solanke wrote:
> > Actually, the best approach here would be to put this single change into
> > a separate patch that comes before the current one.  That removes issues
> > of making more than one functional change in one patch and improves
> > bisectability.
> 
> Before? Shouldn't it be after my changes? That would make it easier to
> justify the changes. And just to be sure, you did mention it does
> align with what the intention of USB_QUIRK_DELAY_INIT, but it does
> change its behavior when the quirk is not set. Atleast from what I
> understood from the documentation and an LLM's summary, the device
> needs time to prepare the full configuration set. So, does delaying
> before the first header read really work? I can't test this since I
> don't have a device that requires the quirk to be set.
> 
> I personally think adding a condition to check if the quirk is set and
> then delaying before sending the first request would be appropriate.
> What are your opinions on this.

Well, put it this way: If you change the existing behavior, that change 
belongs in a separate patch.  If you want to redo this patch so that it 
doesn't change anything when the quirk flag isn't set, that's fine.

> Also is it fine if the string lines exceed 100 columns?

In lines containing long strings, it's okay for the string to extend 
well beyond 80 columns.  But then you should break the line at some 
point closely following the end of the string.  I'm sure you can find 
examples of this if you look through some of the other source files.

> Also, is there a need to check for krealloc()'s return value? Since we
> are only shrinking the buffer, there won't be any moves or completely
> new blocks (at least as per my understanding). Do I still need to
> check its return value for completeness' sake?

It's a little tricky to track this down, but if you look in 
include/linux/slab.h you'll see that krealloc() is defined as 
krealloc_node(), which is defined as krealloc_node_align(), which is 
defined as krealloc_node_align_noprof(), which is declared with 
__must_check.  So yes, you need to check the return value from 
krealloc().

Of course, you could simply try not checking the return value and seeing 
if that provokes a warning or error from the compiler.

Alan Stern

Re: [PATCH v2] usbcore: Add quirk for 255-bytes initial config read

[Greg KH]

On Tue, Jun 23, 2026 at 09:40:35PM +0530, Nikhil Solanke wrote:
> @@ -912,6 +915,13 @@ int usb_get_configuration(struct usb_device *dev)
>  	unsigned char *bigbuffer;
>  	struct usb_config_descriptor *desc;
>  	int result;
> +	/*
> +	 * Devices with quirky firmware will stall or reset when asked only for
> +	 * the configuration header. This variable decides which size to use in
> +	 * that case, if the quirk for that device was set.
> +	 */
> +	size_t usb_config_req_size = (dev->quirks & USB_QUIRK_WINDOWS_CONFIG_REQ_SIZE)
> +		? USB_CONFIG_WINDOWS_REQ_SIZE : USB_DT_CONFIG_SIZE;

Please just use if () lines for code logic like this.  Don't abuse ?:
stuff as it's not needed.  Remember, we write code for people first,
compilers second, and in this case the compiler doesn't care either way
at all, but an if () line makes people much happier.

thanks,

greg k-h

Re: [PATCH v2] usbcore: Add quirk for 255-bytes initial config read

[Nikhil Solanke]

I need some help with the USB_QUIRK_DELAY_INIT part. I can't figure
out how to make it properly work with my patch because of the
following reasons:

1. I don't want to move it to the top because, from my pov, there must
have been some reason for placing that quirk where it is now. so i
don't want to mess with it.

2. Regarding my idea of adding a condition — so that it doesn't change
the behavior when the quirk isn't set — if the full configuration set
exceeds 255 bytes, we would have to issue a 2nd request. In this case
the existing behavior would be more justified.

So, I'm a bit confused about how to implement this properly. Adding
yet another condition to fix the second case doesn't feel right to me.
It would look unnecessarily complicated. I would appreciate a bit of
help and advice.

Thanks,
Nikhil Solanke

Re: [PATCH v2] usbcore: Add quirk for 255-bytes initial config read

[Alan Stern]

On Sun, Jun 28, 2026 at 11:53:09AM +0530, Nikhil Solanke wrote:
> I need some help with the USB_QUIRK_DELAY_INIT part. I can't figure
> out how to make it properly work with my patch because of the
> following reasons:
> 
> 1. I don't want to move it to the top because, from my pov, there must
> have been some reason for placing that quirk where it is now. so i
> don't want to mess with it.
> 
> 2. Regarding my idea of adding a condition — so that it doesn't change
> the behavior when the quirk isn't set — if the full configuration set
> exceeds 255 bytes, we would have to issue a 2nd request. In this case
> the existing behavior would be more justified.
> 
> So, I'm a bit confused about how to implement this properly. Adding
> yet another condition to fix the second case doesn't feel right to me.
> It would look unnecessarily complicated. I would appreciate a bit of
> help and advice.

If the 255-byte quirk flag isn't set, do the delay before the second 
transfer just as it is now.

If the 255-byte quirk flag is set, do the delay before the first 
transfer.  If a second transfer is needed, you can do a second delay 
before it or not -- I suspect it doesn't matter.  If you want to be 
safe, add the second delay.

Alan Stern

Re: [PATCH v2] usbcore: Add quirk for 255-bytes initial config read

[Michal Pecio]

On Sun, 28 Jun 2026 09:55:07 -0400, Alan Stern wrote:
> On Sun, Jun 28, 2026 at 11:53:09AM +0530, Nikhil Solanke wrote:
> > I need some help with the USB_QUIRK_DELAY_INIT part. I can't figure
> > out how to make it properly work with my patch because of the
> > following reasons:
> > 
> > 1. I don't want to move it to the top because, from my pov, there
> > must have been some reason for placing that quirk where it is now.
> > so i don't want to mess with it.

git blame is your friend:

    The DELAY_INIT quirk only reduces the frequency of enumeration
    failures with the Logitech HD Pro C920 and C930e webcams, but does
    not quite eliminate them. We have found that adding a delay of 100ms
    between the first and second Get Configuration request makes the
    device enumerate perfectly reliable even after several weeks of
    extensive testing. The reasons for that are anyone's guess,

> > 
> > 2. Regarding my idea of adding a condition — so that it doesn't
> > change the behavior when the quirk isn't set — if the full
> > configuration set exceeds 255 bytes, we would have to issue a 2nd
> > request. In this case the existing behavior would be more justified.
> > 
> > So, I'm a bit confused about how to implement this properly. Adding
> > yet another condition to fix the second case doesn't feel right to
> > me. It would look unnecessarily complicated. I would appreciate a
> > bit of help and advice.  
> 
> If the 255-byte quirk flag isn't set, do the delay before the second 
> transfer just as it is now.
> 
> If the 255-byte quirk flag is set, do the delay before the first 
> transfer.  If a second transfer is needed, you can do a second delay 
> before it or not -- I suspect it doesn't matter.  If you want to be 
> safe, add the second delay.

How about "keep unrelated changes out of a stable patch", i.e. always
do the delay (if any) after the first request, regardless of size?

Regards,
Michal

Re: [PATCH v2] usbcore: Add quirk for 255-bytes initial config read

[Alan Stern]

On Sun, Jun 28, 2026 at 04:50:40PM +0200, Michal Pecio wrote:
> On Sun, 28 Jun 2026 09:55:07 -0400, Alan Stern wrote:
> > On Sun, Jun 28, 2026 at 11:53:09AM +0530, Nikhil Solanke wrote:
> > > I need some help with the USB_QUIRK_DELAY_INIT part. I can't figure
> > > out how to make it properly work with my patch because of the
> > > following reasons:
> > > 
> > > 1. I don't want to move it to the top because, from my pov, there
> > > must have been some reason for placing that quirk where it is now.
> > > so i don't want to mess with it.
> 
> git blame is your friend:
> 
>     The DELAY_INIT quirk only reduces the frequency of enumeration
>     failures with the Logitech HD Pro C920 and C930e webcams, but does
>     not quite eliminate them. We have found that adding a delay of 100ms
>     between the first and second Get Configuration request makes the
>     device enumerate perfectly reliable even after several weeks of
>     extensive testing. The reasons for that are anyone's guess,
> 
> > > 
> > > 2. Regarding my idea of adding a condition — so that it doesn't
> > > change the behavior when the quirk isn't set — if the full
> > > configuration set exceeds 255 bytes, we would have to issue a 2nd
> > > request. In this case the existing behavior would be more justified.
> > > 
> > > So, I'm a bit confused about how to implement this properly. Adding
> > > yet another condition to fix the second case doesn't feel right to
> > > me. It would look unnecessarily complicated. I would appreciate a
> > > bit of help and advice.  
> > 
> > If the 255-byte quirk flag isn't set, do the delay before the second 
> > transfer just as it is now.
> > 
> > If the 255-byte quirk flag is set, do the delay before the first 
> > transfer.  If a second transfer is needed, you can do a second delay 
> > before it or not -- I suspect it doesn't matter.  If you want to be 
> > safe, add the second delay.
> 
> How about "keep unrelated changes out of a stable patch", i.e. always
> do the delay (if any) after the first request, regardless of size?

This is not an unrelated change.  Rather, it's deciding on how to behave 
in an entirely new control pathway -- the one where the 255-byte quirk 
flag is set.  The old pathway is completely unaffected.

I suspect no devices will have both this quirk flag and the DELAY_INIT 
flag set, which means the location of any delays in the new pathway 
won't matter at all since they will never be used.  But even if some 
such devices do turn up, adding an extra unecessary 200 ms to an 
initialization that is already at least 2200 ms long won't make much 
difference.

Alan Stern

Re: [PATCH v2] usbcore: Add quirk for 255-bytes initial config read

[Nikhil Solanke]

On Sun, 28 Jun 2026 at 19:25, Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Sun, Jun 28, 2026 at 11:53:09AM +0530, Nikhil Solanke wrote:
> > I need some help with the USB_QUIRK_DELAY_INIT part. I can't figure
> > out how to make it properly work with my patch because of the
> > following reasons:
> >
> > 1. I don't want to move it to the top because, from my pov, there must
> > have been some reason for placing that quirk where it is now. so i
> > don't want to mess with it.
> >
> > 2. Regarding my idea of adding a condition — so that it doesn't change
> > the behavior when the quirk isn't set — if the full configuration set
> > exceeds 255 bytes, we would have to issue a 2nd request. In this case
> > the existing behavior would be more justified.
> >
> > So, I'm a bit confused about how to implement this properly. Adding
> > yet another condition to fix the second case doesn't feel right to me.
> > It would look unnecessarily complicated. I would appreciate a bit of
> > help and advice.
>
> If the 255-byte quirk flag isn't set, do the delay before the second
> transfer just as it is now.
>
> If the 255-byte quirk flag is set, do the delay before the first
> transfer.  If a second transfer is needed, you can do a second delay
> before it or not -- I suspect it doesn't matter.  If you want to be
> safe, add the second delay.
>
> Alan Stern

Ok thanks! Just to make sure, because the change I will introduce
won't affect any existing behavior, these changes (relating to
DELAY_INIT quirk) won't belong in a new patch, right?

Thanks,
Nikhil Solanke

Re: [PATCH v2] usbcore: Add quirk for 255-bytes initial config read

[Nikhil Solanke]

On Sun, 28 Jun 2026 at 20:20, Michal Pecio <michal.pecio@gmail.com> wrote:
>
> On Sun, 28 Jun 2026 09:55:07 -0400, Alan Stern wrote:
> > On Sun, Jun 28, 2026 at 11:53:09AM +0530, Nikhil Solanke wrote:
> > > I need some help with the USB_QUIRK_DELAY_INIT part. I can't figure
> > > out how to make it properly work with my patch because of the
> > > following reasons:
> > >
> > > 1. I don't want to move it to the top because, from my pov, there
> > > must have been some reason for placing that quirk where it is now.
> > > so i don't want to mess with it.
>
> git blame is your friend:

I'll keep in mind to use git blame in future. I haven't worked
extensively in a large, collaborative codebase, so using git blame
didn't occur to me in this case. Sorry about that!

Thanks,
Nikhil Solanke

Re: [PATCH v2] usbcore: Add quirk for 255-bytes initial config read

[Michal Pecio]

On Sun, 28 Jun 2026 11:48:36 -0400, Alan Stern wrote:
> > How about "keep unrelated changes out of a stable patch", i.e. always
> > do the delay (if any) after the first request, regardless of size?  
> 
> This is not an unrelated change.  Rather, it's deciding on how to behave 
> in an entirely new control pathway -- the one where the 255-byte quirk 
> flag is set.  The old pathway is completely unaffected.
> 
> I suspect no devices will have both this quirk flag and the DELAY_INIT 
> flag set, which means the location of any delays in the new pathway 
> won't matter at all since they will never be used.

If no devices will have both quirks then new delay added before the
first configuration request will never execute.

If such devices will exist, then it probably won't matter whether the
delay comes after or before the first request. Purpose isn't known,
but it appears to be rate limiting configuration descriptor requests
or delaying other requests after this function returns.

Either way, no known need exists to add another delay before the first
request or alter the existing delay (or its conditions) in any way.

In general, I always object to code which serves no purpose because
such code is easy to add but very hard to remove when it gets in the
way. There are no known users, no test cases, only paranoia.

So I would keep the delay code completely unchaged.

And skip other random changes like error string nitpicking. Reliable
and up to date information about how many bytes are requested,
"expected" (what does it even mean, to somebody reading dmesg?),
received or verified to exist can be gained from source and usbmon.

A stable patch is supposedly supposed to be 100 lines with context ;)

Regards,
Michal

Re: [PATCH v2] usbcore: Add quirk for 255-bytes initial config read

[Michal Pecio]

I really think it could (and should) be a simple patch.

This is what I wrote a few weeks ago. It's an unconditional change
for all devices, but it would be easy to turn it into a quirk.


--- a/drivers/usb/core/config.c
+++ b/drivers/usb/core/config.c
@@ -938,15 +938,14 @@ int usb_get_configuration(struct usb_device *dev)
 	if (!dev->rawdescriptors)
 		return -ENOMEM;
 
-	desc = kmalloc(USB_DT_CONFIG_SIZE, GFP_KERNEL);
+	desc = kmalloc(255, GFP_KERNEL);
 	if (!desc)
 		return -ENOMEM;
 
 	for (cfgno = 0; cfgno < ncfg; cfgno++) {
-		/* We grab just the first descriptor so we know how long
-		 * the whole configuration is */
+		/* Try 255 bytes first because that's what Windows does */
 		result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno,
-		    desc, USB_DT_CONFIG_SIZE);
+		    desc, 255);
 		if (result < 0) {
 			dev_err(ddev, "unable to read config index %d "
 			    "descriptor/%s: %d\n", cfgno, "start", result);
@@ -975,8 +974,12 @@ int usb_get_configuration(struct usb_device *dev)
 		if (dev->quirks & USB_QUIRK_DELAY_INIT)
 			msleep(200);
 
-		result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno,
-		    bigbuffer, length);
+		/* Don't bother if we already have it all */
+		if (length <= result)
+			memcpy(bigbuffer, desc, length);
+		else
+			result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno,
+					bigbuffer, length);
 		if (result < 0) {
 			dev_err(ddev, "unable to read config index %d "
 			    "descriptor/%s\n", cfgno, "all");

Re: [PATCH v2] usbcore: Add quirk for 255-bytes initial config read

[Alan Stern]

On Sun, Jun 28, 2026 at 07:02:01PM +0200, Michal Pecio wrote:
> On Sun, 28 Jun 2026 11:48:36 -0400, Alan Stern wrote:
> > > How about "keep unrelated changes out of a stable patch", i.e. always
> > > do the delay (if any) after the first request, regardless of size?  
> > 
> > This is not an unrelated change.  Rather, it's deciding on how to behave 
> > in an entirely new control pathway -- the one where the 255-byte quirk 
> > flag is set.  The old pathway is completely unaffected.
> > 
> > I suspect no devices will have both this quirk flag and the DELAY_INIT 
> > flag set, which means the location of any delays in the new pathway 
> > won't matter at all since they will never be used.
> 
> If no devices will have both quirks then new delay added before the
> first configuration request will never execute.

Correct.

> If such devices will exist, then it probably won't matter whether the
> delay comes after or before the first request. Purpose isn't known,
> but it appears to be rate limiting configuration descriptor requests
> or delaying other requests after this function returns.

In fact, the commit that talks about the Logitech webcams does describe 
their buggy behavior to some extent.  It says that they seem to reply 
with stale video data instead of the real config information, and from 
there it's a short guess that adding a delay gives time for the video 
pipeline to drain or time out.

In addition, the fact that the delay is needed after the first request 
but before the second suggests that the data corruption only affects 
transfers longer than 9 bytes -- which the new first request would be.  
Therefore it would be appropriate to have the delay before the new first 
request.  Whether another delay would be needed before the second 
request (if there is one) is unknown.

> Either way, no known need exists to add another delay before the first
> request or alter the existing delay (or its conditions) in any way.

See the reasoning above.

> In general, I always object to code which serves no purpose because
> such code is easy to add but very hard to remove when it gets in the
> way. There are no known users, no test cases, only paranoia.
> 
> So I would keep the delay code completely unchaged.

At this point it's entirely theoretical anyway, since the devices that 
would get the new 255-byte quirk flag don't also have the DELAY_INIT 
quirk.

> And skip other random changes like error string nitpicking. Reliable
> and up to date information about how many bytes are requested,
> "expected" (what does it even mean, to somebody reading dmesg?),
> received or verified to exist can be gained from source and usbmon.

Good point.  But I dislike messages that actively produce wrong 
information.  Nikhil could get rid of the parts of the log messages you 
don't like, but he shouldn't leave them as they are.  He could even do 
that in a second patch, separate from this one.

> A stable patch is supposedly supposed to be 100 lines with context ;)

Nonsense.  An excellent stable patch is 7 lines, including context.  :-)

> I really think it could (and should) be a simple patch.
> 
> This is what I wrote a few weeks ago. It's an unconditional change
> for all devices, but it would be easy to turn it into a quirk.
> 
> 
> --- a/drivers/usb/core/config.c
> +++ b/drivers/usb/core/config.c
> @@ -938,15 +938,14 @@ int usb_get_configuration(struct usb_device *dev)
>  	if (!dev->rawdescriptors)
>  		return -ENOMEM;
>  
> -	desc = kmalloc(USB_DT_CONFIG_SIZE, GFP_KERNEL);
> +	desc = kmalloc(255, GFP_KERNEL);
>  	if (!desc)
>  		return -ENOMEM;
>  
>  	for (cfgno = 0; cfgno < ncfg; cfgno++) {
> -		/* We grab just the first descriptor so we know how long
> -		 * the whole configuration is */
> +		/* Try 255 bytes first because that's what Windows does */
>  		result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno,
> -		    desc, USB_DT_CONFIG_SIZE);
> +		    desc, 255);
>  		if (result < 0) {
>  			dev_err(ddev, "unable to read config index %d "
>  			    "descriptor/%s: %d\n", cfgno, "start", result);
> @@ -975,8 +974,12 @@ int usb_get_configuration(struct usb_device *dev)
>  		if (dev->quirks & USB_QUIRK_DELAY_INIT)
>  			msleep(200);
>  
> -		result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno,
> -		    bigbuffer, length);
> +		/* Don't bother if we already have it all */
> +		if (length <= result)
> +			memcpy(bigbuffer, desc, length);
> +		else
> +			result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno,
> +					bigbuffer, length);
>  		if (result < 0) {
>  			dev_err(ddev, "unable to read config index %d "
>  			    "descriptor/%s\n", cfgno, "all");

Making this depend on a quirk flag instead of being unconditional would 
yield a patch very similar to what Nikhil has already posted.

Alan Stern

Re: [PATCH v2] usbcore: Add quirk for 255-bytes initial config read

[Alan Stern]

On Sun, Jun 28, 2026 at 10:01:32PM +0530, Nikhil Solanke wrote:
> On Sun, 28 Jun 2026 at 19:25, Alan Stern <stern@rowland.harvard.edu> wrote:
> >
> > On Sun, Jun 28, 2026 at 11:53:09AM +0530, Nikhil Solanke wrote:
> > > I need some help with the USB_QUIRK_DELAY_INIT part. I can't figure
> > > out how to make it properly work with my patch because of the
> > > following reasons:
> > >
> > > 1. I don't want to move it to the top because, from my pov, there must
> > > have been some reason for placing that quirk where it is now. so i
> > > don't want to mess with it.
> > >
> > > 2. Regarding my idea of adding a condition — so that it doesn't change
> > > the behavior when the quirk isn't set — if the full configuration set
> > > exceeds 255 bytes, we would have to issue a 2nd request. In this case
> > > the existing behavior would be more justified.
> > >
> > > So, I'm a bit confused about how to implement this properly. Adding
> > > yet another condition to fix the second case doesn't feel right to me.
> > > It would look unnecessarily complicated. I would appreciate a bit of
> > > help and advice.
> >
> > If the 255-byte quirk flag isn't set, do the delay before the second
> > transfer just as it is now.
> >
> > If the 255-byte quirk flag is set, do the delay before the first
> > transfer.  If a second transfer is needed, you can do a second delay
> > before it or not -- I suspect it doesn't matter.  If you want to be
> > safe, add the second delay.
> >
> > Alan Stern
> 
> Ok thanks! Just to make sure, because the change I will introduce
> won't affect any existing behavior, these changes (relating to
> DELAY_INIT quirk) won't belong in a new patch, right?

Maybe the best thing to do at this point is to assume that both quirk 
flags will never be set for the same device.  Under that assumption 
there's no need to change the delay code in any way.  Just add a comment 
mentioning this assumption to avoid confusing people in the future.

Alan Stern

Re: [PATCH v2] usbcore: Add quirk for 255-bytes initial config read

[Michal Pecio]

On Sun, 28 Jun 2026 15:18:02 -0400, Alan Stern wrote:
> On Sun, Jun 28, 2026 at 07:02:01PM +0200, Michal Pecio wrote:
> > If such devices will exist, then it probably won't matter whether
> > the delay comes after or before the first request. Purpose isn't
> > known, but it appears to be rate limiting configuration descriptor
> > requests or delaying other requests after this function returns.  
> 
> In fact, the commit that talks about the Logitech webcams does
> describe their buggy behavior to some extent.  It says that they seem
> to reply with stale video data instead of the real config
> information, and from there it's a short guess that adding a delay
> gives time for the video pipeline to drain or time out.
> 
> In addition, the fact that the delay is needed after the first
> request but before the second suggests that the data corruption only
> affects transfers longer than 9 bytes -- which the new first request
> would be. Therefore it would be appropriate to have the delay before
> the new first request.  Whether another delay would be needed before
> the second request (if there is one) is unknown.

Fair enough, that's possible, but even in these specific webcams it's
still unclear what delays would be necessary with both quirks. We wait
for the HW to complete something, but we don't know what starts it. If
it's device reset, then b2a542bbb308 has already doubled all delays
so d86db25e53fa6 isn't even necessary anymore. OTOH, if it's the first
config request, then a delay between the requests is mandatory, and
a delay before the first request is useless. If it's something in
between then your approach could be the only viable choice.

I would worry about it when a device is known that uses both quirks.

> Good point.  But I dislike messages that actively produce wrong 
> information.  Nikhil could get rid of the parts of the log messages
> you don't like, but he shouldn't leave them as they are.  He could
> even do that in a second patch, separate from this one.

I can agree that the first "descriptor too short" message becomes
misleading, because we no longer expect 9 bytes exactly, but anything
between 9 and 255. So this could be changed to "requested".

But I see no need to change the second message. Regardless of initial
request length, the next request (if any) asks for the exact size and
we do expect it to produce 'length' bytes.

Using different words in these messages has a beneficial side effect
of making it possible to tell them apart when wTotalLength == 9.

Regards,
Michal

Re: [PATCH v2] usbcore: Add quirk for 255-bytes initial config read

[Michal Pecio]

On Tue, 23 Jun 2026 21:40:35 +0530, Nikhil Solanke wrote:
> Certain third-party USB game controllers exposing (or spoofing) an Xbox
> 360-compatible interface (VID:PID 045e:028e) fail to enumerate under Linux.
> The device disconnects from the bus without responding to the initial
> GET_DESCRIPTOR(CONFIGURATION) request, and the kernel logs 'unable to read
> config index 0 descriptor/start: -71'.
> 
> The device then falls back to a secondary Android HID mode (with a
> different VID:PID), losing XInput functionality including rumble support.
> The failure reproduces across multiple machines, host controller types, and
> kernel versions including current mainline and LTS. The device enumerates
> correctly and remains in XInput mode under Windows. Notably, the device
> enumerates correctly in Android mode when the same 9-byte request
> is issued for that mode's configuration descriptor, confirming the firmware
> bug is specific to the XInput mode.
> 
> usbmon traces from Linux and Wireshark/USBPcap traces from Windows are
> identical up to the point of failure, with no visible protocol-level
> difference explaining the divergence. The root cause was identified when
> Michal Pecio discovered via a QEMU bus-level capture that Windows does not
> use wLength=9 for the initial config descriptor request; it uses
> wLength=255. Alan Stern subsequently confirmed this with a bus
> analyzer on a different USB 2.0 device, and Michal verified the behavior
> goes back to Windows 95 OSR2.1.
> 
> So, add a new quirk flag USB_QUIRK_CONFIG_SIZE which causes
> usb_get_configuration() to issue a 255 byte sized configuration request
> instead of USB_DT_CONFIG_SIZE (9) for the initial
> GET_DESCRIPTOR(CONFIGURATION) request, mimicking long-standing Windows
> behavior.
> 
> Suggested-by: Alan Stern <stern@rowland.harvard.edu>
> Suggested-by: Michal Pecio <michal.pecio@gmail.com>
> Closes: https://lore.kernel.org/linux-usb/CAFgddh+JWdT4LLwMc5qjM8q_pBu-fRo2qADR5ovAKoGHWMQrRw@mail.gmail.com/
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> 
> Signed-off-by: Nikhil Solanke <nikhilsolanke5@gmail.com>
> ---
> Changes in v2:
> - Add Documentation
> - Naming changes
> - Refactored to have a better flow with existing code.
> 
>  .../admin-guide/kernel-parameters.txt         |  9 +++
>  drivers/usb/core/config.c                     | 61 ++++++++++++++-----
>  drivers/usb/core/hub.c                        |  6 +-
>  drivers/usb/core/quirks.c                     |  4 ++
>  include/linux/usb/quirks.h                    |  3 +
>  5 files changed, 67 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 97007f4f69d4..af4bf0ef2c7b 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -8158,6 +8158,15 @@ Kernel parameters
>  				q = USB_QUIRK_FORCE_ONE_CONFIG (Device
>  					claims zero configurations,
>  					forcing to 1);
> +                r = USB_QUIRK_WINDOWS_CONFIG_REQ_SIZE (Device
> +                    fails during initialization when asked for
> +                    9-bytes configuration desciptor request. Ask
> +                    for 255-bytes request instead to mirror
> +                    Windows' behavior. This quirk is originally
> +                    meant to fix some quirky gamepads that refuse
> +                    to connect in their XInput mode. But it can also
> +                    potentially fix issues with other USB devices
> +                    that work on Windows but not on Linux)

I too think it's too long. This file explains what the parametrs do,
not their history.

And here it's USB_QUIRK_WINDOWS_CONFIG_REQ_SIZE, but in the commit
message it was USB_QUIRK_CONFIG_SIZE.

Honestly, I would suggest a third option: something with "255" instead
of "Windows", because not everybody knows how windows queries
descriptors, but everybody knows what 255 is.

>  			Example: quirks=0781:5580:bk,0a5c:5834:gij
>  
>  	usbhid.mousepoll=
> diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
> index 45e20c6d76c0..4fc3145404d6 100644
> --- a/drivers/usb/core/config.c
> +++ b/drivers/usb/core/config.c
> @@ -19,6 +19,9 @@
>  
>  #define USB_MAXCONFIG			8	/* Arbitrary limit */
>  
> +/* config req size if USB_QUIRK_WINDOWS_CONFIG_REQ_SIZE is set */
> +#define USB_CONFIG_WINDOWS_REQ_SIZE	255
> +
>  static int find_next_descriptor(unsigned char *buffer, int size,
>      int dt1, int dt2, int *num_skipped)
>  {
> @@ -912,6 +915,13 @@ int usb_get_configuration(struct usb_device *dev)
>  	unsigned char *bigbuffer;
>  	struct usb_config_descriptor *desc;
>  	int result;
> +	/*
> +	 * Devices with quirky firmware will stall or reset when asked only for
> +	 * the configuration header. This variable decides which size to use in
> +	 * that case, if the quirk for that device was set.
> +	 */
> +	size_t usb_config_req_size = (dev->quirks & USB_QUIRK_WINDOWS_CONFIG_REQ_SIZE)
> +		? USB_CONFIG_WINDOWS_REQ_SIZE : USB_DT_CONFIG_SIZE;

That's a lot of capital letters, USBCONFIG_WINDOWS_REQ_SIZE never
appears outside this function and personally I would just spell it out
as 255 here with appropriate comment.

The whole function is all about "usb" and "config" so maybe these words
don't need to appear here. OTOH, "first" would add useful information.

>  
>  	if (ncfg > USB_MAXCONFIG) {
>  		dev_notice(ddev, "too many configurations: %d, "
> @@ -938,18 +948,27 @@ int usb_get_configuration(struct usb_device *dev)
>  	if (!dev->rawdescriptors)
>  		return -ENOMEM;
>  
> -	desc = kmalloc(USB_DT_CONFIG_SIZE, GFP_KERNEL);
> +	desc = kmalloc(usb_config_req_size, GFP_KERNEL);
> +
>  	if (!desc)
>  		return -ENOMEM;
>  
>  	for (cfgno = 0; cfgno < ncfg; cfgno++) {
> -		/* We grab just the first descriptor so we know how long
> -		 * the whole configuration is */
> +
> +		if (dev->quirks & USB_QUIRK_DELAY_INIT)
> +			msleep(200);
> +
> +		/*
> +		 * Grab just the first descriptor so we know how long the whole
> +		 * configuration is. In case of quirky firmware, try to grab the
> +		 * whole thing in one go by asking for a 255-bytes sized buffer
> +		 * mirroring Windows behavior.
> +		 */
>  		result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno,
> -		    desc, USB_DT_CONFIG_SIZE);
> +						desc, usb_config_req_size);
>  		if (result < 0) {
>  			dev_err(ddev, "unable to read config index %d "
> -			    "descriptor/%s: %d\n", cfgno, "start", result);
> +				"descriptor/%s: %d\n", cfgno, "start", result);
>  			if (result != -EPIPE)
>  				goto err;
>  			dev_notice(ddev, "chopping to %d config(s)\n", cfgno);
> @@ -957,13 +976,25 @@ int usb_get_configuration(struct usb_device *dev)
>  			break;
>  		} else if (result < 4) {
>  			dev_err(ddev, "config index %d descriptor too short "
> -			    "(expected %i, got %i)\n", cfgno,
> -			    USB_DT_CONFIG_SIZE, result);
> +				"(asked for %zu, got %i, expected at least %i)\n",
> +				cfgno, usb_config_req_size, result, 4);
>  			result = -EINVAL;
>  			goto err;
>  		}
> +
>  		length = max_t(int, le16_to_cpu(desc->wTotalLength),
> -		    USB_DT_CONFIG_SIZE);
> +				USB_DT_CONFIG_SIZE);
> +
> +		/*
> +		 * If the device returns the full length configuration
> +		 * descriptor, skip the second read. Otherwise, send a second
> +		 * request asking for the full length.
> +		 */
> +		if (result >= le16_to_cpu(desc->wTotalLength)) {

Technically, this is a behavior change for devices without the quirk,
if their wTotalLength is 9. Not sure if this is worth worrynig about,
it would mean that the configuration has no interfaces or endpoints.

> +			bigbuffer = (unsigned char *) desc;
> +			desc = NULL;

What happens in the next iteration of the loop?

> +			goto store_and_parse;
> +		}
>  
>  		/* Now that we know the length, get the whole thing */
>  		bigbuffer = kmalloc(length, GFP_KERNEL);
> @@ -972,23 +1003,25 @@ int usb_get_configuration(struct usb_device *dev)
>  			goto err;
>  		}
>  
> -		if (dev->quirks & USB_QUIRK_DELAY_INIT)
> -			msleep(200);
> -
>  		result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno,
> -		    bigbuffer, length);
> +						bigbuffer, length);
> +
>  		if (result < 0) {
>  			dev_err(ddev, "unable to read config index %d "
> -			    "descriptor/%s\n", cfgno, "all");
> +				"descriptor/%s\n", cfgno, "all");
>  			kfree(bigbuffer);
>  			goto err;
>  		}
> +
>  		if (result < length) {
>  			dev_notice(ddev, "config index %d descriptor too short "
> -			    "(expected %i, got %i)\n", cfgno, length, result);
> +				"(asked for %i, got %i)\n",
> +				cfgno, length, result);
>  			length = result;
>  		}
>  
> +store_and_parse:
> +		krealloc(bigbuffer, length, GFP_KERNEL);
>  		dev->rawdescriptors[cfgno] = bigbuffer;
>  
>  		result = usb_parse_configuration(dev, cfgno,
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 24960ba9caa9..9acd278666fc 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -2527,8 +2527,10 @@ static int usb_enumerate_device(struct usb_device *udev)
>  		err = usb_get_configuration(udev);
>  		if (err < 0) {
>  			if (err != -ENODEV)
> -				dev_err(&udev->dev, "can't read configurations, error %d\n",
> -						err);
> +				dev_err(&udev->dev, "can't read configurations, "
> +					"for device %04x:%04x, error %d\n",
> +					le16_to_cpu(udev->descriptor.idVendor),
> +					le16_to_cpu(udev->descriptor.idProduct), err);

I wonder if it wouldn't make sense to split announce_device() so that
the first line is printed as soon as usb_new_device() starts, before
enumeration is attempted and possibly fails.

That would be a separate patch, of course.

>  			return err;
>  		}
>  	}
> diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
> index 87810eff974e..df670b0b66fe 100644
> --- a/drivers/usb/core/quirks.c
> +++ b/drivers/usb/core/quirks.c
> @@ -142,6 +142,10 @@ static int quirks_param_set(const char *value, const struct kernel_param *kp)
>  				break;
>  			case 'q':
>  				flags |= USB_QUIRK_FORCE_ONE_CONFIG;
> +				break;
> +			case 'r':
> +				flags |= USB_QUIRK_WINDOWS_CONFIG_REQ_SIZE;
> +				break;
>  			/* Ignore unrecognized flag characters */
>  			}
>  		}
> diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h
> index b3cc7beab4a3..a4043b33c2c2 100644
> --- a/include/linux/usb/quirks.h
> +++ b/include/linux/usb/quirks.h
> @@ -81,4 +81,7 @@
>  /* Device claims zero configurations, forcing to 1 */
>  #define USB_QUIRK_FORCE_ONE_CONFIG		BIT(18)
>  
> +/* Use a 255 bytes config descriptor request mirroring windows behavior */
> +#define USB_QUIRK_WINDOWS_CONFIG_REQ_SIZE	BIT(19)
> +
>  #endif /* __LINUX_USB_QUIRKS_H */
> -- 
> 2.54.0
>