Re: [PATCH] usb: hcd: xhci: Add set command timer delay API

[Mathias Nyman <mathias.nyman@linux.intel.com>]

On 18.8.2023 12.23, Hardik Gajjar wrote:
> xHCI driver starts the response timer after sending each
> command to the device. The default value of this timer is
> 5 seconds (XHCI_CMD_DEFAULT_TIMEOUT = HZ*5). This seems
> too high in time crtical use case.
> 
> This patch provides an API to change the default value of
> the timer from the vendor USB driver.
> 
> The default value will be XHCI_CMD_DEFAULT_TIMEOUT (5 sec)
> 
> Use case:
> According to the Smartphone integration certification
> requirement in the automotive, the phone connected via USB
> should complete enumeration and user space handshake
> within 3 sec.

The above incorrectly makes it sound as if the command timeout
timer causes the delay.

> 
> Reducing the response waiting time by setting the smaller
> command timer delay helps to speed up overall re-enumeration
> process of the USB device in case of device is not responding
> properly in first enumeration iteration.

So is this a case where addressing a usb device behind xHC always
fail on the first attempt, i.e. address device command in xhci
never completes. Solution proposed here is to fail faster and
retry?

Is the rootcause known why first enumeration fails?

Does setting old_scheme_first module parameter help?

> 
> Signed-off-by: Hardik Gajjar <hgajjar@de.adit-jv.com>
> ---
>   drivers/usb/core/hcd.c       | 23 +++++++++++++++++++++++
>   drivers/usb/host/xhci-ring.c | 10 +++++-----
>   drivers/usb/host/xhci.c      | 15 +++++++++++++++
>   drivers/usb/host/xhci.h      |  1 +
>   include/linux/usb/hcd.h      |  2 ++
>   5 files changed, 46 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 8300baedafd2..e392e90e918c 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -3157,6 +3157,29 @@ int usb_hcd_setup_local_mem(struct usb_hcd *hcd, phys_addr_t phys_addr,
>   }
>   EXPORT_SYMBOL_GPL(usb_hcd_setup_local_mem);
>   
> +/**
> + * usb_hcd_set_cmd_timer_delay Set the delay of the command timer.
> + * @hcd - pointer to the HCD representing the controller
> + * @delay - Delay value to be used in command timer.
> + *
> + * wrapper function to call the set_cmd_timer_delay API of the host
> + * diver.
> + *
> + * return 0 on success; otherwise -ENODEV means the feature not
> + * supported by host driver.
> + */
> +
> +int usb_hcd_set_cmd_timer_delay(struct usb_hcd *hcd, int delay)
> +{
> +	int ret = -ENODEV;
> +
> +	if (hcd->driver->set_cmd_timer_delay)
> +		ret = hcd->driver->set_cmd_timer_delay(hcd, delay);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(usb_hcd_set_cmd_timer_delay);
> +

The xhci command timeout is more of a xhci internal thing, not sure it's a good
idea to add this to hcd.

Would it make sense to add a timeout parameter to hcd->driver->address_device(hcd, udev)
instead?

First priority should of course be finding out why the first enumeration fails,
and solve that.

Thanks
Mathias