Re: [RFC 06/13] USB: Add support to enable/disable USB3 link states.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


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


B and H Foto and Electronics Corp.

[Linux Media]     [Video for Linux]     [Linux Input]     [Linux Audio Users]     [Photo]     [Yosemite News]    [Yosemite Photos]    [Free Online Dating]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]     [More Archives]

Add to Google Powered by Linux