Message ID | 1464199129-7639-1-git-send-email-tim.gardner@canonical.com |
---|---|
State | New |
Headers | show |
On Wed, May 25, 2016 at 11:58:49AM -0600, Tim Gardner wrote: > From: Alan Stern <stern@rowland.harvard.edu> > > BugLink: http://bugs.launchpad.net/bugs/1577024 > > When a USB driver is bound to an interface (either through probing or > by claiming it) or is unbound from an interface, the USB core always > disables Link Power Management during the transition and then > re-enables it afterward. The reason is because the driver might want > to prevent hub-initiated link power transitions, in which case the HCD > would have to recalculate the various LPM parameters. This > recalculation takes place when LPM is re-enabled and the new > parameters are sent to the device and its parent hub. > > However, if the driver does not want to prevent hub-initiated link > power transitions then none of this work is necessary. The parameters > don't need to be recalculated, and LPM doesn't need to be disabled and > re-enabled. > > It turns out that disabling and enabling LPM can be time-consuming, > enough so that it interferes with user programs that want to claim and > release interfaces rapidly via usbfs. Since the usbfs kernel driver > doesn't set the disable_hub_initiated_lpm flag, we can speed things up > and get the user programs to work by leaving LPM alone whenever the > flag isn't set. > > And while we're improving the way disable_hub_initiated_lpm gets used, > let's also fix its kerneldoc. > > Signed-off-by: Alan Stern <stern@rowland.harvard.edu> > Tested-by: Matthew Giassa <matthew@giassa.net> > CC: Mathias Nyman <mathias.nyman@intel.com> > CC: <stable@vger.kernel.org> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > (cherry picked from commit 6fb650d43da3e7054984dc548eaa88765a94d49f) > Signed-off-by: Tim Gardner <tim.gardner@canonical.com> > --- > drivers/usb/core/driver.c | 40 +++++++++++++++++++++++----------------- > include/linux/usb.h | 2 +- > 2 files changed, 24 insertions(+), 18 deletions(-) > > diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c > index 2057d91..dadd1e8d 100644 > --- a/drivers/usb/core/driver.c > +++ b/drivers/usb/core/driver.c > @@ -284,7 +284,7 @@ static int usb_probe_interface(struct device *dev) > struct usb_device *udev = interface_to_usbdev(intf); > const struct usb_device_id *id; > int error = -ENODEV; > - int lpm_disable_error; > + int lpm_disable_error = -ENODEV; > > dev_dbg(dev, "%s\n", __func__); > > @@ -336,12 +336,14 @@ static int usb_probe_interface(struct device *dev) > * setting during probe, that should also be fine. usb_set_interface() > * will attempt to disable LPM, and fail if it can't disable it. > */ > - lpm_disable_error = usb_unlocked_disable_lpm(udev); > - if (lpm_disable_error && driver->disable_hub_initiated_lpm) { > - dev_err(&intf->dev, "%s Failed to disable LPM for driver %s\n.", > - __func__, driver->name); > - error = lpm_disable_error; > - goto err; > + if (driver->disable_hub_initiated_lpm) { > + lpm_disable_error = usb_unlocked_disable_lpm(udev); > + if (lpm_disable_error) { > + dev_err(&intf->dev, "%s Failed to disable LPM for driver %s\n.", > + __func__, driver->name); > + error = lpm_disable_error; > + goto err; > + } > } > > /* Carry out a deferred switch to altsetting 0 */ > @@ -391,7 +393,8 @@ static int usb_unbind_interface(struct device *dev) > struct usb_interface *intf = to_usb_interface(dev); > struct usb_host_endpoint *ep, **eps = NULL; > struct usb_device *udev; > - int i, j, error, r, lpm_disable_error; > + int i, j, error, r; > + int lpm_disable_error = -ENODEV; > > intf->condition = USB_INTERFACE_UNBINDING; > > @@ -399,12 +402,13 @@ static int usb_unbind_interface(struct device *dev) > udev = interface_to_usbdev(intf); > error = usb_autoresume_device(udev); > > - /* Hub-initiated LPM policy may change, so attempt to disable LPM until > + /* If hub-initiated LPM policy may change, attempt to disable LPM until > * the driver is unbound. If LPM isn't disabled, that's fine because it > * wouldn't be enabled unless all the bound interfaces supported > * hub-initiated LPM. > */ > - lpm_disable_error = usb_unlocked_disable_lpm(udev); > + if (driver->disable_hub_initiated_lpm) > + lpm_disable_error = usb_unlocked_disable_lpm(udev); > > /* > * Terminate all URBs for this interface unless the driver > @@ -505,7 +509,7 @@ int usb_driver_claim_interface(struct usb_driver *driver, > struct device *dev; > struct usb_device *udev; > int retval = 0; > - int lpm_disable_error; > + int lpm_disable_error = -ENODEV; > > if (!iface) > return -ENODEV; > @@ -526,12 +530,14 @@ int usb_driver_claim_interface(struct usb_driver *driver, > > iface->condition = USB_INTERFACE_BOUND; > > - /* Disable LPM until this driver is bound. */ > - lpm_disable_error = usb_unlocked_disable_lpm(udev); > - if (lpm_disable_error && driver->disable_hub_initiated_lpm) { > - dev_err(&iface->dev, "%s Failed to disable LPM for driver %s\n.", > - __func__, driver->name); > - return -ENOMEM; > + /* See the comment about disabling LPM in usb_probe_interface(). */ > + if (driver->disable_hub_initiated_lpm) { > + lpm_disable_error = usb_unlocked_disable_lpm(udev); > + if (lpm_disable_error) { > + dev_err(&iface->dev, "%s Failed to disable LPM for driver %s\n.", > + __func__, driver->name); > + return -ENOMEM; > + } > } > > /* Claimed interfaces are initially inactive (suspended) and > diff --git a/include/linux/usb.h b/include/linux/usb.h > index 22b226d..1b1692a 100644 > --- a/include/linux/usb.h > +++ b/include/linux/usb.h > @@ -1070,7 +1070,7 @@ struct usbdrv_wrap { > * for interfaces bound to this driver. > * @soft_unbind: if set to 1, the USB core will not kill URBs and disable > * endpoints before calling the driver's disconnect method. > - * @disable_hub_initiated_lpm: if set to 0, the USB core will not allow hubs > + * @disable_hub_initiated_lpm: if set to 1, the USB core will not allow hubs > * to initiate lower power link state transitions when an idle timeout > * occurs. Device-initiated USB 3.0 link PM will still be allowed. > * Looks to do what is claimed. Is a clean cherry-pick. Acked-by: Andy Whitcroft <apw@canonical.com> -apw
Applied to V, W, X. -Kamal
diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index 2057d91..dadd1e8d 100644 --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c @@ -284,7 +284,7 @@ static int usb_probe_interface(struct device *dev) struct usb_device *udev = interface_to_usbdev(intf); const struct usb_device_id *id; int error = -ENODEV; - int lpm_disable_error; + int lpm_disable_error = -ENODEV; dev_dbg(dev, "%s\n", __func__); @@ -336,12 +336,14 @@ static int usb_probe_interface(struct device *dev) * setting during probe, that should also be fine. usb_set_interface() * will attempt to disable LPM, and fail if it can't disable it. */ - lpm_disable_error = usb_unlocked_disable_lpm(udev); - if (lpm_disable_error && driver->disable_hub_initiated_lpm) { - dev_err(&intf->dev, "%s Failed to disable LPM for driver %s\n.", - __func__, driver->name); - error = lpm_disable_error; - goto err; + if (driver->disable_hub_initiated_lpm) { + lpm_disable_error = usb_unlocked_disable_lpm(udev); + if (lpm_disable_error) { + dev_err(&intf->dev, "%s Failed to disable LPM for driver %s\n.", + __func__, driver->name); + error = lpm_disable_error; + goto err; + } } /* Carry out a deferred switch to altsetting 0 */ @@ -391,7 +393,8 @@ static int usb_unbind_interface(struct device *dev) struct usb_interface *intf = to_usb_interface(dev); struct usb_host_endpoint *ep, **eps = NULL; struct usb_device *udev; - int i, j, error, r, lpm_disable_error; + int i, j, error, r; + int lpm_disable_error = -ENODEV; intf->condition = USB_INTERFACE_UNBINDING; @@ -399,12 +402,13 @@ static int usb_unbind_interface(struct device *dev) udev = interface_to_usbdev(intf); error = usb_autoresume_device(udev); - /* Hub-initiated LPM policy may change, so attempt to disable LPM until + /* If hub-initiated LPM policy may change, attempt to disable LPM until * the driver is unbound. If LPM isn't disabled, that's fine because it * wouldn't be enabled unless all the bound interfaces supported * hub-initiated LPM. */ - lpm_disable_error = usb_unlocked_disable_lpm(udev); + if (driver->disable_hub_initiated_lpm) + lpm_disable_error = usb_unlocked_disable_lpm(udev); /* * Terminate all URBs for this interface unless the driver @@ -505,7 +509,7 @@ int usb_driver_claim_interface(struct usb_driver *driver, struct device *dev; struct usb_device *udev; int retval = 0; - int lpm_disable_error; + int lpm_disable_error = -ENODEV; if (!iface) return -ENODEV; @@ -526,12 +530,14 @@ int usb_driver_claim_interface(struct usb_driver *driver, iface->condition = USB_INTERFACE_BOUND; - /* Disable LPM until this driver is bound. */ - lpm_disable_error = usb_unlocked_disable_lpm(udev); - if (lpm_disable_error && driver->disable_hub_initiated_lpm) { - dev_err(&iface->dev, "%s Failed to disable LPM for driver %s\n.", - __func__, driver->name); - return -ENOMEM; + /* See the comment about disabling LPM in usb_probe_interface(). */ + if (driver->disable_hub_initiated_lpm) { + lpm_disable_error = usb_unlocked_disable_lpm(udev); + if (lpm_disable_error) { + dev_err(&iface->dev, "%s Failed to disable LPM for driver %s\n.", + __func__, driver->name); + return -ENOMEM; + } } /* Claimed interfaces are initially inactive (suspended) and diff --git a/include/linux/usb.h b/include/linux/usb.h index 22b226d..1b1692a 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -1070,7 +1070,7 @@ struct usbdrv_wrap { * for interfaces bound to this driver. * @soft_unbind: if set to 1, the USB core will not kill URBs and disable * endpoints before calling the driver's disconnect method. - * @disable_hub_initiated_lpm: if set to 0, the USB core will not allow hubs + * @disable_hub_initiated_lpm: if set to 1, the USB core will not allow hubs * to initiate lower power link state transitions when an idle timeout * occurs. Device-initiated USB 3.0 link PM will still be allowed. *