Re: [PATCH v7 1/2] usb: xhci: Add timeout argument in address_device USB HCD callback

Re: [PATCH v7 1/2] usb: xhci: Add timeout argument in address_device USB HCD callback

[Mathias Nyman]

On 27.10.2023 18.20, Hardik Gajjar wrote:
> - The HCD address_device callback now accepts a user-defined timeout value
>    in milliseconds, providing better control over command execution times.
> - The default timeout value for the address_device command has been set
>    to 5000 ms, aligning with the USB 3.2 specification. However, this
>    timeout can be adjusted as needed.
> - The xhci_setup_device function has been updated to accept the timeout
>    value, allowing it to specify the maximum wait time for the command
>    operation to complete.
> - The hub driver has also been updated to accommodate the newly added
>    timeout parameter during the SET_ADDRESS request.
> 
> Signed-off-by: Hardik Gajjar <hgajjar@de.adit-jv.com>

For the xhci parts

Reviewed-by: Mathias Nyman <mathias.nyman@linux.intel.com>

Re: [PATCH v7 1/2] usb: xhci: Add timeout argument in address_device USB HCD callback

[Hardik Gajjar]

On Mon, Oct 30, 2023 at 12:45:54PM +0200, Mathias Nyman wrote:
> On 27.10.2023 18.20, Hardik Gajjar wrote:
> > - The HCD address_device callback now accepts a user-defined timeout value
> >    in milliseconds, providing better control over command execution times.
> > - The default timeout value for the address_device command has been set
> >    to 5000 ms, aligning with the USB 3.2 specification. However, this
> >    timeout can be adjusted as needed.
> > - The xhci_setup_device function has been updated to accept the timeout
> >    value, allowing it to specify the maximum wait time for the command
> >    operation to complete.
> > - The hub driver has also been updated to accommodate the newly added
> >    timeout parameter during the SET_ADDRESS request.
> > 
> > Signed-off-by: Hardik Gajjar <hgajjar@de.adit-jv.com>
> 
> For the xhci parts
> 
> Reviewed-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> 
>

@Greg KH, Friendly reminder.

Thanks,
Hardik

Re: [PATCH v7 1/2] usb: xhci: Add timeout argument in address_device USB HCD callback

[Greg KH]

On Fri, Nov 03, 2023 at 04:18:22PM +0100, Hardik Gajjar wrote:
> On Mon, Oct 30, 2023 at 12:45:54PM +0200, Mathias Nyman wrote:
> > On 27.10.2023 18.20, Hardik Gajjar wrote:
> > > - The HCD address_device callback now accepts a user-defined timeout value
> > >    in milliseconds, providing better control over command execution times.
> > > - The default timeout value for the address_device command has been set
> > >    to 5000 ms, aligning with the USB 3.2 specification. However, this
> > >    timeout can be adjusted as needed.
> > > - The xhci_setup_device function has been updated to accept the timeout
> > >    value, allowing it to specify the maximum wait time for the command
> > >    operation to complete.
> > > - The hub driver has also been updated to accommodate the newly added
> > >    timeout parameter during the SET_ADDRESS request.
> > > 
> > > Signed-off-by: Hardik Gajjar <hgajjar@de.adit-jv.com>
> > 
> > For the xhci parts
> > 
> > Reviewed-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> > 
> >
> 
> @Greg KH, Friendly reminder.

It is the m iddle of the merge window, my branches are closed for
obvious reasons until after -rc1 is out.  Please relax and wait for a
week or so _after_ -rc1 is out before worrying about anything.

thanks,

greg k-h

Re: [PATCH v7 1/2] usb: xhci: Add timeout argument in address_device USB HCD callback

[Greg KH]

On Fri, Nov 03, 2023 at 04:26:37PM +0100, Greg KH wrote:
> On Fri, Nov 03, 2023 at 04:18:22PM +0100, Hardik Gajjar wrote:
> > On Mon, Oct 30, 2023 at 12:45:54PM +0200, Mathias Nyman wrote:
> > > On 27.10.2023 18.20, Hardik Gajjar wrote:
> > > > - The HCD address_device callback now accepts a user-defined timeout value
> > > >    in milliseconds, providing better control over command execution times.
> > > > - The default timeout value for the address_device command has been set
> > > >    to 5000 ms, aligning with the USB 3.2 specification. However, this
> > > >    timeout can be adjusted as needed.
> > > > - The xhci_setup_device function has been updated to accept the timeout
> > > >    value, allowing it to specify the maximum wait time for the command
> > > >    operation to complete.
> > > > - The hub driver has also been updated to accommodate the newly added
> > > >    timeout parameter during the SET_ADDRESS request.
> > > > 
> > > > Signed-off-by: Hardik Gajjar <hgajjar@de.adit-jv.com>
> > > 
> > > For the xhci parts
> > > 
> > > Reviewed-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> > > 
> > >
> > 
> > @Greg KH, Friendly reminder.
> 
> It is the m iddle of the merge window, my branches are closed for
> obvious reasons until after -rc1 is out.  Please relax and wait for a
> week or so _after_ -rc1 is out before worrying about anything.

In the meantime, to make things go faster, please help review patches
from others on the mailing list so that when the merge window does open
back up, my queue will be much smaller and lighter and yours will be
closer to the top.

thanks,

greg k-h

Re: [PATCH v7 2/2] usb: new quirk to reduce the SET_ADDRESS request timeout

[Hardik Gajjar]

On Fri, Oct 27, 2023 at 11:49:13AM -0400, Alan Stern wrote:
> On Fri, Oct 27, 2023 at 05:20:29PM +0200, Hardik Gajjar wrote:
> > This patch introduces a new USB quirk,
> > USB_QUIRK_SHORT_SET_ADDRESS_REQ_TIMEOUT, which modifies the timeout value
> > for the SET_ADDRESS request. The standard timeout for USB request/command
> > is 5000 ms, as recommended in the USB 3.2 specification (section 9.2.6.1).
> > 
> > However, certain scenarios, such as connecting devices through an APTIV
> > hub, can lead to timeout errors when the device enumerates as full speed
> > initially and later switches to high speed during chirp negotiation.
> > 
> > In such cases, USB analyzer logs reveal that the bus suspends for
> > 5 seconds due to incorrect chirp parsing and resumes only after two
> > consecutive timeout errors trigger a hub driver reset.
> > 
> > Packet(54) Dir(?) Full Speed J(997.100 us) Idle(  2.850 us)
> > _______| Time Stamp(28 . 105 910 682)
> > _______|_____________________________________________________________Ch0
> > Packet(55) Dir(?) Full Speed J(997.118 us) Idle(  2.850 us)
> > _______| Time Stamp(28 . 106 910 632)
> > _______|_____________________________________________________________Ch0
> > Packet(56) Dir(?) Full Speed J(399.650 us) Idle(222.582 us)
> > _______| Time Stamp(28 . 107 910 600)
> > _______|_____________________________________________________________Ch0
> > Packet(57) Dir Chirp J( 23.955 ms) Idle(115.169 ms)
> > _______| Time Stamp(28 . 108 532 832)
> > _______|_____________________________________________________________Ch0
> > Packet(58) Dir(?) Full Speed J (Suspend)( 5.347 sec) Idle(  5.366 us)
> > _______| Time Stamp(28 . 247 657 600)
> > _______|_____________________________________________________________Ch0
> > 
> > This 5-second delay in device enumeration is undesirable, particularly
> > in automotive applications where quick enumeration is crucial
> > (ideally within 3 seconds).
> > 
> > The newly introduced quirks provide the flexibility to align with a
> > 3-second time limit, as required in specific contexts like automotive
> > applications.
> > 
> > By reducing the SET_ADDRESS request timeout to 500 ms, the
> > system can respond more swiftly to errors, initiate rapid recovery, and
> > ensure efficient device enumeration. This change is vital for scenarios
> > where rapid smartphone enumeration and screen projection are essential.
> > 
> > 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/parameters/quirks"
> > 
> > Signed-off-by: Hardik Gajjar <hgajjar@de.adit-jv.com>
> > ---
> 
> Reviewed-by: Alan Stern <stern@rowland.harvard.edu>

@Greg KH, Friendly reminder.

Thanks,
Hardik