Re: [PATCH v3] usb: core: hub: Add quirks for reducing device address timeout

From: Alan Stern
Date: Wed Oct 11 2023 - 10:15:59 EST


On Wed, Oct 11, 2023 at 10:50:11AM +0200, Hardik Gajjar wrote:

> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -54,6 +54,9 @@
> #define USB_TP_TRANSMISSION_DELAY_MAX 65535 /* ns */
> #define USB_PING_RESPONSE_TIME 400 /* ns */
>
> +#define USB_DEFAULT_ADDR_DEVICE_TIMEOUT_MS 5000 /* 5000ms */
> +#define USB_SHORT_ADDR_DEVICE_TIMEOUT_MS 500 /* 500ms */

You don't have to repeat the numbers in the comments. You can just
write /* ms */ -- just like in the comments immediately above.

> return -EINVAL;
> if (hcd->driver->address_device)
> - retval = hcd->driver->address_device(hcd, udev);
> + retval = hcd->driver->address_device(hcd, udev, timeout_ms);
> else
> retval = usb_control_msg(udev, usb_sndaddr0pipe(),
> --- a/drivers/usb/core/quirks.c
> +++ b/drivers/usb/core/quirks.c
> @@ -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_DEVICE_ADDR_TIMEOUT;
> + break;
> /* Ignore unrecognized flag characters */
> }
> }
> @@ -542,6 +545,9 @@ static const struct usb_device_id usb_quirk_list[] = {
> /* INTEL VALUE SSD */
> { USB_DEVICE(0x8086, 0xf1a5), .driver_info = USB_QUIRK_RESET_RESUME },
>
> + /* APTIV AUTOMOTIVE HUB */
> + { USB_DEVICE(0x2c48, 0x0132), .driver_info = USB_QUIRK_SHORT_DEVICE_ADDR_TIMEOUT },

This table is sorted by Vendor ID, then Product ID, then Class ID, as
stated in the comment at the beginning of the definition. Your new
entry is in the wrong position.

Alan Stern