RE: [PATCH 1/1] usb: lpm: add boot flag to disable lpm

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

 



Please remember to use Reply-To-All so that your messages get sent to 
the mailing list as well as to me.

On Thu, 21 Apr 2016, Matthew Giassa wrote:

> Hi Alan,
> 
> I've tested your latest patch, and here is a subset of the output in
> `dmesg':

...
> [   44.975704] enable LPM
> [   44.975707] CPU: 4 PID: 2491 Comm: FlyCap2 Tainted: G            E  
> 4.5.0-astern-logging+ #11
> [   44.975708] Hardware name: ASUS All Series/Z97-PRO(Wi-Fi ac), BIOS
> 2012 09/30/2014
> [   44.975708]  0000000000000000 ffff880217c9bd50 ffffffff813c2ccc
> ffff8802362c5000
> [   44.975710]  ffff88023308a000 ffff880217c9bd78 ffffffff815ceadf
> ffff88023308a000
> [   44.975711]  ffff8802362c5000 ffffffff81ce5860 ffff880217c9bd98
> ffffffff815ceb1c
> [   44.975713] Call Trace:
> [   44.975715]  [<ffffffff813c2ccc>] dump_stack+0x63/0x87
> [   44.975717]  [<ffffffff815ceadf>] usb_enable_lpm+0x10f/0x120
> [   44.975718]  [<ffffffff815ceb1c>] usb_unlocked_enable_lpm+0x2c/0x40
> [   44.975719]  [<ffffffff815dd10e>]
> usb_driver_claim_interface+0xbe/0x110
> [   44.975720]  [<ffffffff815e386b>] claimintf+0x5b/0x80
> [   44.975721]  [<ffffffff815e6960>] usbdev_do_ioctl+0xd10/0x1180
> [   44.975723]  [<ffffffff815e6dfe>] usbdev_ioctl+0xe/0x20
> [   44.975724]  [<ffffffff812174a6>] do_vfs_ioctl+0x96/0x590
> [   44.975725]  [<ffffffff810fb491>] ? SyS_futex+0x71/0x150
> [   44.975727]  [<ffffffff81217a19>] SyS_ioctl+0x79/0x90
> [   44.975729]  [<ffffffff817e2036>] entry_SYSCALL_64_fastpath+0x16/0x75
...

Now I see the problem.  The patch changed usb_probe_interface, but I
forgot to change usb_driver_claim_interface also.  Try this version of
the patch instead.  It also has the extra debugging output, but now it
shouldn't ever get triggered.

Alan Stern



Index: usb-4.x/include/linux/usb.h
===================================================================
--- usb-4.x.orig/include/linux/usb.h
+++ usb-4.x/include/linux/usb.h
@@ -1069,7 +1069,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.
  *
Index: usb-4.x/drivers/usb/core/driver.c
===================================================================
--- usb-4.x.orig/drivers/usb/core/driver.c
+++ usb-4.x/drivers/usb/core/driver.c
@@ -284,7 +284,7 @@ static int usb_probe_interface(struct de
 	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 de
 	 * 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 d
 	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 d
 	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 us
 	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 us
 
 	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
Index: usb-4.x/drivers/usb/core/devio.c
===================================================================
--- usb-4.x.orig/drivers/usb/core/devio.c
+++ usb-4.x/drivers/usb/core/devio.c
@@ -738,6 +738,8 @@ struct usb_driver usbfs_driver = {
 	.resume =	driver_resume,
 };
 
+int alantest;
+
 static int claimintf(struct usb_dev_state *ps, unsigned int ifnum)
 {
 	struct usb_device *dev = ps->dev;
@@ -757,8 +759,11 @@ static int claimintf(struct usb_dev_stat
 	intf = usb_ifnum_to_if(dev, ifnum);
 	if (!intf)
 		err = -ENOENT;
-	else
+	else {
+		alantest = 1;
 		err = usb_driver_claim_interface(&usbfs_driver, intf, ps);
+		alantest = 0;
+	}
 	if (err == 0)
 		set_bit(ifnum, &ps->ifclaimed);
 	return err;
@@ -778,7 +783,9 @@ static int releaseintf(struct usb_dev_st
 	if (!intf)
 		err = -ENOENT;
 	else if (test_and_clear_bit(ifnum, &ps->ifclaimed)) {
+		alantest = 1;
 		usb_driver_release_interface(&usbfs_driver, intf);
+		alantest = 0;
 		err = 0;
 	}
 	return err;
Index: usb-4.x/drivers/usb/core/hub.c
===================================================================
--- usb-4.x.orig/drivers/usb/core/hub.c
+++ usb-4.x/drivers/usb/core/hub.c
@@ -33,6 +33,8 @@
 #include "hub.h"
 #include "otg_whitelist.h"
 
+extern int alantest;
+
 #define USB_VENDOR_GENESYS_LOGIC		0x05e3
 #define HUB_QUIRK_CHECK_PORT_AUTOSUSPEND	0x01
 
@@ -4054,6 +4056,11 @@ int usb_disable_lpm(struct usb_device *u
 	if ((udev->u1_params.timeout == 0 && udev->u2_params.timeout == 0))
 		return 0;
 
+	if (alantest) {
+		pr_info("disable LPM\n");
+		dump_stack();
+	}
+
 	/* If LPM is enabled, attempt to disable it. */
 	if (usb_disable_link_state(hcd, udev, USB3_LPM_U1))
 		goto enable_lpm;
@@ -4121,6 +4128,11 @@ void usb_enable_lpm(struct usb_device *u
 	if (!hub)
 		return;
 
+	if (alantest) {
+		pr_info("enable LPM\n");
+		dump_stack();
+	}
+
 	port_dev = hub->ports[udev->portnum - 1];
 
 	if (port_dev->usb3_lpm_u1_permit)

--
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



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux