On Wed, Feb 22, 2012 at 03:49:39PM -0500, Alan Stern wrote: > On Tue, 21 Feb 2012, Sarah Sharp wrote: > > > Given that I shouldn't enable the timeouts if a driver sets the > > disable_hub_initiated_lpm flag, I need to make sure the timeouts (and > > probably device-initiated LPM as well) are disabled before we bind any > > new drivers. The timeouts also need to be evaluated when a driver > > unbinds. > > > > It seems like there are several places where I should disable LPM, let > > some change occur, and then re-evaluate what the U1/U2 timeouts should > > be. Some of those places I've identified are: > > This depends on what your criteria are going to be. For example, with > communications class devices, you might want to leave the timeouts > permanently disabled. > > > - usb_disable_device > > Why? This routine gets called only when the config is changed or the > device is unplugged. I'm just interested in when the configuration or alt setting changes, so I guess usb_disable_device doesn't make much sense. > > - usb_set_interface > > Only if you use a criterion based on the enabled endpoints. That was my original plan. If a new alternate interface setting adds a periodic endpoint, then the timeouts might have to change in order to make sure the device doesn't go into a lower power link state between service intervals. Calculating the timeouts when the drivers bind and unbind doesn't help me catch the case where a driver changes alternate interface settings. > > - usb_set_configuration > > Okay, maybe -- see below. > > > - usb_reset_configuration > > This is like usb_set_interface; all it does is possibly change the set > of enabled endpoints. > > > - usb_reset_device > > I suppose you might want to disable the timeouts during the reset and > re-enable them afterward. It probably doesn't matter. USB 3.0 devices won't accept a request to enable or disable device-initiated U1 or U2 if they aren't in the configured state. So it sort of makes sense to disable device-initiated LPM before we reset the device. > > - where ever a new driver is bound > > > > Is there any function I'm missing? I also don't know exactly where to > > handle checking the timeouts when a new driver binds. There's a note > > at the end of usb_set_configuration that adding each device interface > > causes the driver to be bound, and I could evaluate the timeouts there, > > but I'm not sure where I need to handle drivers that are loaded some > > time later after the interface is installed (say through a user running > > modprobe for a blacklisted module). > > 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? > 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. 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? 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. 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
