usb: new quirk to reduce the SET_ADDRESS request timeout
authorHardik Gajjar <hgajjar@de.adit-jv.com>
Fri, 27 Oct 2023 15:20:29 +0000 (17:20 +0200)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Thu, 23 Nov 2023 12:32:44 +0000 (12:32 +0000)
This patch introduces a new USB quirk,
USB_QUIRK_SHORT_SET_ADDRESS_REQ_TIMEOUT, which modifies the timeout value
for the SET_ADDRESS request. The standard timeout for USB request/command
is 5000 ms, as recommended in the USB 3.2 specification (section 9.2.6.1).

However, certain scenarios, such as connecting devices through an APTIV
hub, can lead to timeout errors when the device enumerates as full speed
initially and later switches to high speed during chirp negotiation.

In such cases, USB analyzer logs reveal that the bus suspends for
5 seconds due to incorrect chirp parsing and resumes only after two
consecutive timeout errors trigger a hub driver reset.

Packet(54) Dir(?) Full Speed J(997.100 us) Idle(  2.850 us)
_______| Time Stamp(28 . 105 910 682)
_______|_____________________________________________________________Ch0
Packet(55) Dir(?) Full Speed J(997.118 us) Idle(  2.850 us)
_______| Time Stamp(28 . 106 910 632)
_______|_____________________________________________________________Ch0
Packet(56) Dir(?) Full Speed J(399.650 us) Idle(222.582 us)
_______| Time Stamp(28 . 107 910 600)
_______|_____________________________________________________________Ch0
Packet(57) Dir Chirp J( 23.955 ms) Idle(115.169 ms)
_______| Time Stamp(28 . 108 532 832)
_______|_____________________________________________________________Ch0
Packet(58) Dir(?) Full Speed J (Suspend)( 5.347 sec) Idle(  5.366 us)
_______| Time Stamp(28 . 247 657 600)
_______|_____________________________________________________________Ch0

This 5-second delay in device enumeration is undesirable, particularly
in automotive applications where quick enumeration is crucial
(ideally within 3 seconds).

The newly introduced quirks provide the flexibility to align with a
3-second time limit, as required in specific contexts like automotive
applications.

By reducing the SET_ADDRESS request timeout to 500 ms, the
system can respond more swiftly to errors, initiate rapid recovery, and
ensure efficient device enumeration. This change is vital for scenarios
where rapid smartphone enumeration and screen projection are essential.

To use the quirk, please write "vendor_id:product_id:p" to
/sys/bus/usb/drivers/hub/module/parameter/quirks

For example,
echo "0x2c48:0x0132:p" > /sys/bus/usb/drivers/hub/module/parameters/quirks"

Signed-off-by: Hardik Gajjar <hgajjar@de.adit-jv.com>
Reviewed-by: Alan Stern <stern@rowland.harvard.edu>
Link: https://lore.kernel.org/r/20231027152029.104363-2-hgajjar@de.adit-jv.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Documentation/admin-guide/kernel-parameters.txt
drivers/usb/core/hub.c
drivers/usb/core/quirks.c
include/linux/usb/quirks.h

index 65731b060e3fef98cb97f03695ca8757ee197700..0a6a4b7f7a3b9f11df0ced5cdf642b3d7021d6ce 100644 (file)
                                        pause after every control message);
                                o = USB_QUIRK_HUB_SLOW_RESET (Hub needs extra
                                        delay after resetting its port);
+                               p = USB_QUIRK_SHORT_SET_ADDRESS_REQ_TIMEOUT
+                                       (Reduce timeout of the SET_ADDRESS
+                                       request from 5000 ms to 500 ms);
                        Example: quirks=0781:5580:bk,0a5c:5834:gij
 
        usbhid.mousepoll=
index 0490582643678be5f2ae50f0c67936ed6a14a7eb..756fbd7cac435df53b727a485c7e312c2f7290cd 100644 (file)
 #define USB_TP_TRANSMISSION_DELAY_MAX  65535   /* ns */
 #define USB_PING_RESPONSE_TIME         400     /* ns */
 
+/*
+ * The SET_ADDRESS request timeout will be 500 ms when
+ * USB_QUIRK_SHORT_SET_ADDRESS_REQ_TIMEOUT quirk flag is set.
+ */
+#define USB_SHORT_SET_ADDRESS_REQ_TIMEOUT      500  /* ms */
+
 /* Protect struct usb_device->state and ->children members
  * Note: Both are also protected by ->dev.sem, except that ->state can
  * change to USB_STATE_NOTATTACHED even when the semaphore isn't held. */
@@ -4649,7 +4655,12 @@ EXPORT_SYMBOL_GPL(usb_ep0_reinit);
 static int hub_set_address(struct usb_device *udev, int devnum)
 {
        int retval;
+       unsigned int timeout_ms = USB_CTRL_SET_TIMEOUT;
        struct usb_hcd *hcd = bus_to_hcd(udev->bus);
+       struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent);
+
+       if (hub->hdev->quirks & USB_QUIRK_SHORT_SET_ADDRESS_REQ_TIMEOUT)
+               timeout_ms = USB_SHORT_SET_ADDRESS_REQ_TIMEOUT;
 
        /*
         * The host controller will choose the device address,
@@ -4662,11 +4673,11 @@ static int hub_set_address(struct usb_device *udev, int devnum)
        if (udev->state != USB_STATE_DEFAULT)
                return -EINVAL;
        if (hcd->driver->address_device)
-               retval = hcd->driver->address_device(hcd, udev, USB_CTRL_SET_TIMEOUT);
+               retval = hcd->driver->address_device(hcd, udev, timeout_ms);
        else
                retval = usb_control_msg(udev, usb_sndaddr0pipe(),
                                USB_REQ_SET_ADDRESS, 0, devnum, 0,
-                               NULL, 0, USB_CTRL_SET_TIMEOUT);
+                               NULL, 0, timeout_ms);
        if (retval == 0) {
                update_devnum(udev, devnum);
                /* Device now using proper address. */
index 15e9bd180a1d253f3b3cd2eebb3de68bcae9fb5e..b4783574b8e6603b7c521accdbc4cc52518d61ec 100644 (file)
@@ -138,6 +138,9 @@ static int quirks_param_set(const char *value, const struct kernel_param *kp)
                        case 'o':
                                flags |= USB_QUIRK_HUB_SLOW_RESET;
                                break;
+                       case 'p':
+                               flags |= USB_QUIRK_SHORT_SET_ADDRESS_REQ_TIMEOUT;
+                               break;
                        /* Ignore unrecognized flag characters */
                        }
                }
@@ -527,6 +530,10 @@ static const struct usb_device_id usb_quirk_list[] = {
 
        { USB_DEVICE(0x2386, 0x350e), .driver_info = USB_QUIRK_NO_LPM },
 
+       /* APTIV AUTOMOTIVE HUB */
+       { USB_DEVICE(0x2c48, 0x0132), .driver_info =
+                       USB_QUIRK_SHORT_SET_ADDRESS_REQ_TIMEOUT },
+
        /* DJI CineSSD */
        { USB_DEVICE(0x2ca3, 0x0031), .driver_info = USB_QUIRK_NO_LPM },
 
index eeb7c2157c72fba0e54f926cec863bb9e50ff9f5..59409c1fc3dee7b20112867615f9a7a92b65fa17 100644 (file)
@@ -72,4 +72,7 @@
 /* device has endpoints that should be ignored */
 #define USB_QUIRK_ENDPOINT_IGNORE              BIT(15)
 
+/* short SET_ADDRESS request timeout */
+#define USB_QUIRK_SHORT_SET_ADDRESS_REQ_TIMEOUT        BIT(16)
+
 #endif /* __LINUX_USB_QUIRKS_H */