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

Hardik Gajjar
2023-10-10T10:50:08Z

On Mon, Oct 09, 2023 at 03:16:25PM -0400, Alan Stern wrote:
> 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?

This issue occurs when the device first enumerates as full speed and then
as a high-speed device.

It appears that the set_address request is issued as soon as the device is
detected as full speed. However, during the progress of the set_address
request, the device changes its state from full speed to high speed, causing
the set_address request to become stuck until it times out.

Stress testing of USB devices indicates that the problem is specific to the
set_address request. Other requests, such as device descriptor requests,
consistently fail immediately with a protocol error in almost all cases.

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

Good Point, Thanks, I will update in patch v3

>
> Alan Stern