On Thu, May 17, 2012 at 03:19:20PM -0400, Alan Stern wrote: > On Wed, 16 May 2012, Sarah Sharp wrote: > > > There are various functions within the USB core that will need to > > disable USB 3.0 link power states. For example, when a USB device > > driver is being bound to an interface, we need to disable USB 3.0 LPM > > until we know if the driver will allow hub-initiated LPM transitions. > > Another example is when the USB core is switching alternate interface > > settings. The USB 3.0 timeout values are dependent on what endpoints > > are enabled, so we want to ensure that LPM is disabled until the new alt > > setting is fully installed. > > > > Multiple functions need to disable LPM, and those functions can even be > > nested. For example, usb_bind_interface() could disable LPM, and then > > call into the driver probe function, which may attempt to switch to a > > different alt setting. Therefore, we need to keep a count of the number > > of functions that require LPM to be disabled at any point in time. > > > > Introduce two new USB core API calls, usb_disable_lpm() and > > usb_enable_lpm(). These functions increment and decrement a new > > variable in the usb_device, lpm_disable_count. These two function calls > > should always be paired. Even if usb_disable_lpm() fails, > > usb_enable_lpm() must be called to balance the lpm_disable_count. The > > bandwidth_mutex must be held when these new functions are called. > > This is awkward. For one thing, all our other functions clean up after > themselves if they fail; usb_disable_lpm() should do the same. I'll have to look carefully at the callers to see if this makes sense. > Also, it's not natural for many of the callers to hold the > bandwidth_mutex. Why not create locking and non-locking versions of > these routines? The locking versions would grab the mutex and then > call the non-locking versions. Is it wise to hide the bandwidth_mutex lock? I would rather it be obvious in the callers that they can deadlock. Otherwise I have no qualms with creating locking and non-locking versions of these routines. > On an unrelated note, hub.c is already by far the largest source file > in drivers/usb/core -- and you're adding another thousand lines or so. > Maybe it's time to consider splitting it into two or more files. I can look at splitting out the link PM code into a separate file, sure. But I have no idea how best to split up the rest of the hub.c code, and no real desire to do so as part of this particular patchset. 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
