On Thu, Feb 23, 2012 at 11:25:13AM -0500, Alan Stern wrote:
> On Wed, 22 Feb 2012, Sarah Sharp wrote:
>
> > > New drivers are bound in usb_probe_interface. There's also
> > > usb_driver_claim_interface, but with the exception of usbfs, it gets
> > > called only by drivers that are already bound to another interface on
> > > the same device.
> >
> > So usb_probe_device is called when a new device is connected? And then
> > it will call the driver probe function, which happens to be
> > usb_probe_interface? I think I'll have to go look more closely at this
> > code. Is there any documentation about the USB device probe flow?
>
> I think I'm the only documentation. :-)
Thanks for being the documentation. :)
> First, you have to realize the probing and binding can happen in
> response to several different events. Registering a new interface
> (which usb_set_configuration does) will do it. So will the driver core
> when userspace writes to the "bind" file in /sys/bus/usb/drivers/.../.
> usbcore itself initiates probing following a device reset or resume, if
> one of the interface drivers didn't include the appopriate methods and
> thus had to be unbound. And drivers can request binding on their own
> by calling usb_claim_interface. (This list may not be complete. The
> point is that binding happens in lots of different ways.)
>
> Furthermore, although binding usually involves calling a driver's probe
> routine, it doesn't have to. If dev->driver is set when dev is
> registered with the driver core, the binding will be created
> automatically without any probing. In the USB subsystem this can
> happen when a driver calls usb_claim_interface for an interface that
> hasn't been registered yet.
>
> To understand the usual control flow, keep in mind the distinction
> between USB devices (described as "functions" in the USB-2.0 spec
> although not in the USB-3.0 spec) and USB interfaces. Confusingly, we
> tend to refer to them both as "devices".
USB 3.0 has "functions" but those are specifically a set of interfaces
bound by an Interface Association Descriptor (IAD). It sounds like the
USB core would view each of those interfaces as possibly having
different drivers.
> As you know, when a new USB device is detected, khubd enumerates and
> registers it. Registration causes the usb_generic driver to be probed.
> generic_probe, among other things, chooses a default configuration and
> calls usb_set_configuration. (Note: The pathway involves
> usb_probe_device, but this is pretty much just a formality.
> usb_generic is the _only_ driver in the kernel for USB devices.)
>
> This in turn causes the config's interfaces to be registered, which
> causes the various drivers for those interfaces to be loaded and
> probed. Interface probing always passes through usb_probe_interface.
>
> Just like binding, unbinding can happen in several different ways.
> It's a little simpler because unbinding a driver from an interface
> always involves usb_unbind_interface, even if the interface hasn't been
> registered yet -- that's the only place where the driver's disconnect
> method gets called.
So it seems like usb_probe_interface, usb_driver_claim_interface, and
usb_unbind_interface would be the interesting places to do timeout
enabling/disabling.
There's this note in usb_unbind_interface that's a bit concerning:
/* Reset other interface state.
* We cannot do a Set-Interface if the device is suspended or
* if it is prepared for a system sleep (since installing a new
* altsetting means creating new endpoint device entries).
* When either of these happens, defer the Set-Interface.
*/
Why are we trying to avoid creating new endpoint device entries?
Are we trying to avoid memory allocation during driver unbind when the
system is going to sleep? If so, that's important to the xHCI driver
design for setting/clearing timeouts.
> > > But of course, now you face the problem of what to do when
> > > usb_set_configuration changes a whole bunch of bindings at once. The
> > > easiest answer is to disable/enable the timeouts around each new
> > > binding. Then you wouldn't need to touch usb_set_configuration.
> >
> > I think I have to disable the timeouts around unbindings as well,
> > correct? If I unbind a driver that's no longer using an isochronous
> > endpoint, then the timeout might change as well.
> >
> > The probe functions look like a good place to put this. However, I'm
> > going to have to take the bandwidth_mutex in order to change some xHCI
> > parameters, and I'm a bit concerned about deadlocking with the
> > usb_set_configuration, usb_reset_configuration, and usb_set_interface.
>
> Indeed, it looks like you may need nested enables and disables, which
> implies some sort of counting mechanism. Does it make sense always to
> update the link-state stuff along with the bandwidth calculations?
Yes, I think it does make sense to update the timeouts and max exit
latencies whenever there's a bandwidth change. I do think I should
submit the bandwidth changes due to config/alt setting changes before
attempting to change the timeouts. That way a new config/alt setting
will have a better chance of succeeding, but we may not get the best
power out of it.
All the information I need about what's changing is really in the places
where the bandwidth calculation changes happen. Putting aside the
question of drivers disabling hub-initiated timeouts, here's the design
I have in mind (sorry in advance for the length):
1. For each usb_device, add two new fields to track the maximum exit
latency for U1 and U2. This would be for the entire path from the
device to the roothub, including host controller U1/U2 exit latency.
Also add two new fields to track the current U1 and U2 timeouts that are
programmed into the parent hub (possibly this belongs in the parent hub
structure instead as a per port array). The usb_device also gets two
new bit fields that track whether device-initiated U1 or U2 is enabled.
2. Reuse the lpm_capable field to indicate that a device is LPM capable.
All certified USB 3.0 devices are *supposed* to support LPM, but they
need to indicate their U1 and U2 max exit latencies in the SuperSpeed
Extended Capabilities BOS descriptor, and we need to handle the odd case
where that's not present (or a non-certified hub made it out to the
market without LPM support).
3. When a new configuration is selected, save the old U1 and U2
timeouts, and then disable the timeouts and device-initiated U1/U2 (if
the device is configured). When the USB core calls into the xHCI driver
to add a new periodic endpoint for a bandwidth change, add the service
interval and endpoint type to a small linked list of endpoints, sorted
by configuration and interface numbers. If the USB core removes an
endpoint, delete it from the list. We might even be able to reuse some
of those awful bandwidth tables I had to create for the Panther Point
xHCI host controller.
4. Once the bandwidth change has successfully (but before the interfaces
are bound), call into the xHCI driver twice to calculate the new U1 and
U2 timeouts. It will attempt to submit an Evaluate Context command to
change the max exit latency for the device. If the xHC host agrees to
that latency, the xHCI driver returns a timeout. Otherwise the call
returns an error. (We have to increase the max exit latency stored by
the xHC before we change any timeouts in parent hubs, so that the xHC
will have added extra latency in its schedule before we enable U1/U2.)
4b. If the configuration change fails due to the SetConfig control
transfer failing, and we fall back to a previously enabled
configuration, then call into the xHCI reset bandwidth function. That
will roll back any changes to the endpoints, and issue xHC commands to
put the MEL back to what it was before the timeouts where disabled. If
those fail, the timeouts remain disabled. If they succeed, then the USB
core can proceed to step 5 with the old U1/U2 timeouts that it saved in
step 3.
5. The USB core sends the request to the device to enable
device-initiated U1 or U2. If that fails, just clear the
device-initiated U1 or U2 flag in the usb_device. Then attempt to send
the request to the parent hub to set the U1 or U2 timeout. We have to
set a non-zero timeout (0xFF) to tell the parent hub to accept U1 or U2
requests from the device if we are only doing device-initiated U1 and
U2. If we're setting parent hub U1 and U2 timeouts for hub-initiate U1
or U2, we'll set it to a non-zero (and non-0xFF) value that the xHCI
driver calculated.
6a. If the request to the parent hub to set the timeout fails three
times, disable LPM completely: send a request to the device to disable
device-initiated U1/U2, and call into the xHCI driver to set the max
exit latency for the device to zero. Possibly mark the hub as not being
lpm_capable.
6b. If the request succeeds, update the device with the system exit
latency (total max exit latency, plus hub delays for hub header
decoding). If it rejects that, disable LPM, and mark the device as not
lpm_capable.
Now, with all that in place, what is the best way to accommodate drivers
that don't want the hub U1/U2 timeouts to be set? One way I thought of
doing it was to look at all the device drivers in step 4 and leave the
U1/U2 timeouts disabled if any of them wanted to disable hub-initiated
LPM. That has issues though, because we might be changing to alt
setting 0 just before we unbind a driver, and drivers can bind to an
interface anytime later when they are finally loaded. That solution
might be necessary, but it's not sufficient to fix the problem.
It seems like on top the described system, I need to do some work during
the bind and unbind of drivers. In bind, if we need to change to alt
setting 0, do that. Before the driver's probe function is called, check
whether any of the drivers for the currently installed interfaces need
to have the hub U1/U2 timeouts disabled. If none of them do, repeat
steps 4-6, otherwise disable the timeouts. The probe function might
install a different alt setting, but I think the usb_set_interface call
will be able to see the driver that was being bound to the interface.
On a call to usb_driver_claim_interface, check for any interface drivers
that need the hub U1/U2 timeouts disabled (including the driver that's
claiming the interface). If so, disable the timeouts.
On a call to usb_unbind_interface, if we need to switch to alt setting
0, attempt that. Once the driver is unbound, check whether any driver
in the new set of bound drivers needs to have hub U1/U2 timeouts
disabled. If so, disable the timeouts. If not, repeat steps 4-6 to see
if we can we the timeouts.
> > I think I'll be ok, since the bandwidth_mutex is dropped before the
> > driver bind takes place. I think that's necessary since some drivers
> > set alternate interfaces in their probe functions?
>
> Yes, they do.
>
> > I think even if I set the timeouts around the driver bind/unbind, I
> > still need to handle the case where a driver changes the alternate
> > interface setting, and periodic endpoints get added or removed. So I
> > think the timeouts need to be recalculated in usb_set_interface as
> > well. And usb_reset_device needs to disable the timeouts before the
> > device is reset.
>
> How careful are you about keeping separate track of the parent hub's
> U1/U2 port-timeout settings and the device's settings? From what
> you've written so far, it isn't clear.
That's because I haven't written much code so far. :) See step 1 above
for a description of what variables I'm storing.
Sarah Sharp
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
