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

[Hardik Gajjar]

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@xxxxxxxxxxxxxx>
> > ---
> > 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