[Acked] [PATCH SRU V/W/X/Y] USB: leave LPM alone if possible when binding/unbinding interface drivers

[Andy Whitcroft]

On Wed, May 25, 2016 at 11:58:49AM -0600, Tim Gardner wrote:
> From: Alan Stern <stern at 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 at rowland.harvard.edu>
> Tested-by: Matthew Giassa <matthew at giassa.net>
> CC: Mathias Nyman <mathias.nyman at intel.com>
> CC: <stable at vger.kernel.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> (cherry picked from commit 6fb650d43da3e7054984dc548eaa88765a94d49f)
> Signed-off-by: Tim Gardner <tim.gardner at 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 at canonical.com>

-apw