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

[Alan Stern]

On Tue, Oct 17, 2023 at 06:53:44PM +0200, Greg KH wrote:
> On Tue, Oct 17, 2023 at 06:10:21PM +0200, Hardik Gajjar wrote:
> > More logs and detailed in patch V1:
> > https://lore.kernel.org/linux-usb/20230818092353.124658-1-hgajjar@xxxxxxxxxxxxxx/T/#m452ec9dad94e8181fdb050cd29483dd89437f7c1
> > > 
> > > > Achieving this is impossible in scenarios where the set_address is
> > > > not successful and waits for a timeout.
> > > 
> > > Agreed, broken hardware is a pain, but if your device is allowed to take
> > > longer, it can, and will, so you have to support that.
> > > 
> > The problem is not caused by the device taking an extended amount of time to
> > process the 'set_address' request. Instead, the issue lies in the absence of
> > any activity on the upstream bus until a timeout occurs.
> 
> So, a broken device.  Why are you then adding the hub to the quirk list
> and not the broken device?  We are used to adding broken devices to
> qurik lists all the time, this shouldn't be new.

Adding a quirk for the device isn't feasible, because the problem occurs 
before the device has been initialized and enumerated.  The kernel 
doesn't know anything about the device at this point; only that it has 
just connected.

> > This situation arises when the host has already transmitted the 'set_address' command to the hub,
> > assuming that the device operates at full speed. However, the device connected
> > to the hub undergoes a state change from full speed to high-speed during this process.
> 
> During which process?  While the set-address happens?  That feels like a
> hub bug then.
> 
> > > > 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.
> > > 
> > > Screen projection is a requirement that you should not be relying on USB
> > > for as USB has a different set of required timeouts, right?  This sounds
> > > like a bad hardware design, if not an impossible one.
> > > 
> > 
> > Screen projection for us means displaying the connected phone on the screen and
> > launching Carplay and Android Auto for the user. This works perfectly in nearly all
> > cases, except in scenarios like this one where a combination of a special hub and
> > a specific phone model is causing the issue
> 
> So which is broken, the hub or phone?

It sounds like both of them are broken to some extent, although we can't 
tell for sure without seeing what's actually happening on the USB bus 
(i.e., bus analyzer output):

	The phone seems to take too long to activate its high-speed
	terminations and deactivate the full-speed terminations.

	The hub doesn't seem to realize that the phone has disconnected 
	its full-speed connection and switched to high-speed.

But without real data, these are just best guesses.

> > The issue arises when the host has already issued the 'set_address' command to the hub,
> > assuming the device is operating at full speed. However, in such cases, when the device
> > connected to the hub changes its state from full speed to high-speed, the 'set_address'
> > request becomes blocked, waiting for the full 5-second timeout. This patch reduces the
> > timeout from 5 seconds to 500 milliseconds, allowing for quicker failure and re-enumeration
> > of the device as high-speed
> 
> Changing speed is under hub control, not device control, right?  Are you

Changing speed is under device control.  But of course, the hub has to 
detect the change and react to it properly.

Alan Stern

> sure the firmware is correct in that hub?  Has the hub passed all of the
> USB-IF testing requirements?
> 
> thanks,
> 
> greg k-h