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

Alan Stern
2023-10-09T19:16:34Z

On Mon, Oct 09, 2023 at 06:14:02PM +0200, Hardik Gajjar wrote:
> Currently, the timeout for the set address command is fixed at
> 5 seconds in the xhci driver. This means the host waits up to 5
> seconds to receive a response for the set_address command from
> the device.
>
> In the automotive context, most smartphone enumerations, including
> screen projection, should ideally complete within 3 seconds.
> Achieving this is impossible in scenarios where the set_address is
> not successful and waits for a timeout.

What will you do about scenarios where the Set-Address completes very
quickly but the following Get-Device-Descriptor times out after 5
seconds? Or any of the other transfers involved in device
initialization and enumeration?

> The shortened address device timeout quirks provide the flexibility
> to align with a 3-second time limit in the event of errors.
> By swiftly triggering a failure response and swiftly initiating
> retry procedures, these quirks ensure efficient and rapid recovery,
> particularly in automotive contexts where rapid smartphone enumeration
> and screen projection are vital.
>
> The quirk will set the timeout to 500 ms from 5 seconds.
>
> 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/parameter/quirks"
>
> Signed-off-by: Hardik Gajjar <hgajjar@de.adit-jv.com>
> ---
> changes since version 1:
> - implement quirk instead of new API in xhci driver
> ---
> drivers/usb/core/hub.c | 15 +++++++++++++--
> drivers/usb/core/quirks.c | 3 +++
> drivers/usb/host/xhci-mem.c | 1 +
> drivers/usb/host/xhci-ring.c | 3 ++-
> drivers/usb/host/xhci.c | 9 +++++----
> drivers/usb/host/xhci.h | 1 +
> include/linux/usb/hcd.h | 3 ++-
> include/linux/usb/quirks.h | 3 +++
> 8 files changed, 30 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 3c54b218301c..975449b03426 100644
> --- 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 (HZ * 5) /* 5000ms */
> +#define USB_SHORT_ADDR_DEVICE_TIMEOUT 125 /* ~500ms */

That number, 125, is meaningless. It's in units of jiffies, which vary
from one system to another. If you want the timeout to be about 500 ms,
you should write it as (HZ / 2).

Alan Stern