Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

USB: leave LPM alone if possible when binding/unbinding interface drivers

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>

authored by

Alan Stern and committed by
Greg Kroah-Hartman
6fb650d4 e66fa8b0

+24 -18
+23 -17
drivers/usb/core/driver.c
··· 284 284 struct usb_device *udev = interface_to_usbdev(intf); 285 285 const struct usb_device_id *id; 286 286 int error = -ENODEV; 287 - int lpm_disable_error; 287 + int lpm_disable_error = -ENODEV; 288 288 289 289 dev_dbg(dev, "%s\n", __func__); 290 290 ··· 336 336 * setting during probe, that should also be fine. usb_set_interface() 337 337 * will attempt to disable LPM, and fail if it can't disable it. 338 338 */ 339 - lpm_disable_error = usb_unlocked_disable_lpm(udev); 340 - if (lpm_disable_error && driver->disable_hub_initiated_lpm) { 341 - dev_err(&intf->dev, "%s Failed to disable LPM for driver %s\n.", 342 - __func__, driver->name); 343 - error = lpm_disable_error; 344 - goto err; 339 + if (driver->disable_hub_initiated_lpm) { 340 + lpm_disable_error = usb_unlocked_disable_lpm(udev); 341 + if (lpm_disable_error) { 342 + dev_err(&intf->dev, "%s Failed to disable LPM for driver %s\n.", 343 + __func__, driver->name); 344 + error = lpm_disable_error; 345 + goto err; 346 + } 345 347 } 346 348 347 349 /* Carry out a deferred switch to altsetting 0 */ ··· 393 391 struct usb_interface *intf = to_usb_interface(dev); 394 392 struct usb_host_endpoint *ep, **eps = NULL; 395 393 struct usb_device *udev; 396 - int i, j, error, r, lpm_disable_error; 394 + int i, j, error, r; 395 + int lpm_disable_error = -ENODEV; 397 396 398 397 intf->condition = USB_INTERFACE_UNBINDING; 399 398 ··· 402 399 udev = interface_to_usbdev(intf); 403 400 error = usb_autoresume_device(udev); 404 401 405 - /* Hub-initiated LPM policy may change, so attempt to disable LPM until 402 + /* If hub-initiated LPM policy may change, attempt to disable LPM until 406 403 * the driver is unbound. If LPM isn't disabled, that's fine because it 407 404 * wouldn't be enabled unless all the bound interfaces supported 408 405 * hub-initiated LPM. 409 406 */ 410 - lpm_disable_error = usb_unlocked_disable_lpm(udev); 407 + if (driver->disable_hub_initiated_lpm) 408 + lpm_disable_error = usb_unlocked_disable_lpm(udev); 411 409 412 410 /* 413 411 * Terminate all URBs for this interface unless the driver ··· 509 505 struct device *dev; 510 506 struct usb_device *udev; 511 507 int retval = 0; 512 - int lpm_disable_error; 508 + int lpm_disable_error = -ENODEV; 513 509 514 510 if (!iface) 515 511 return -ENODEV; ··· 530 526 531 527 iface->condition = USB_INTERFACE_BOUND; 532 528 533 - /* Disable LPM until this driver is bound. */ 534 - lpm_disable_error = usb_unlocked_disable_lpm(udev); 535 - if (lpm_disable_error && driver->disable_hub_initiated_lpm) { 536 - dev_err(&iface->dev, "%s Failed to disable LPM for driver %s\n.", 537 - __func__, driver->name); 538 - return -ENOMEM; 529 + /* See the comment about disabling LPM in usb_probe_interface(). */ 530 + if (driver->disable_hub_initiated_lpm) { 531 + lpm_disable_error = usb_unlocked_disable_lpm(udev); 532 + if (lpm_disable_error) { 533 + dev_err(&iface->dev, "%s Failed to disable LPM for driver %s\n.", 534 + __func__, driver->name); 535 + return -ENOMEM; 536 + } 539 537 } 540 538 541 539 /* Claimed interfaces are initially inactive (suspended) and
+1 -1
include/linux/usb.h
··· 1068 1068 * for interfaces bound to this driver. 1069 1069 * @soft_unbind: if set to 1, the USB core will not kill URBs and disable 1070 1070 * endpoints before calling the driver's disconnect method. 1071 - * @disable_hub_initiated_lpm: if set to 0, the USB core will not allow hubs 1071 + * @disable_hub_initiated_lpm: if set to 1, the USB core will not allow hubs 1072 1072 * to initiate lower power link state transitions when an idle timeout 1073 1073 * occurs. Device-initiated USB 3.0 link PM will still be allowed. 1074 1074 *