Re: Autosuspend and unbound interfaces

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

 



On Fri, 7 May 2010, Rob Duncan wrote:

> > I don't understand.  The disable_depth value should be 0 -- the patch
> > sets it to 0 by means of the call to pm_runtime_enable() added in
> > usb_probe_interface().  Can you check at that spot to see if it really
> > does what it's supposed to do?
> 
> I added a trace immediately after the call to pm_runtime_enable() in
> usb_probe_interface().  It dumps the value of disable_depth of the device
> (the interface), and also the interface's parent device (the USB device
> itself).  Here's what I see:
> 
> May  7 08:42:03 localhost kernel: usb 3-3: adding 3-3:1.1 (config #1, interface 1)
> May  7 08:42:03 localhost kernel: usbserial_generic 3-3:1.1: usb_probe_interface
> May  7 08:42:03 localhost kernel: usbserial_generic 3-3:1.1: usb_probe_interface - got id
> May  7 08:42:03 localhost kernel: usb 3-3: usb_autoresume_device: cnt 5 -> 1
> May  7 08:42:03 localhost kernel: usbserial_generic 3-3:1.1: usb_probe_interface dev->power.disable_depth=1
> May  7 08:42:03 localhost kernel: usb 3-3: usb_probe_interface dev->parent->power.disable_depth=0
> May  7 08:42:03 localhost kernel: usb 3-3: usb_autosuspend_device: cnt 4 -> 0
> May  7 08:42:03 localhost kernel: exar 3-3:1.1: usb_probe_interface
> May  7 08:42:03 localhost kernel: exar 3-3:1.1: usb_probe_interface - got id
> May  7 08:42:03 localhost kernel: usb 3-3: usb_autoresume_device: cnt 5 -> 1
> May  7 08:42:03 localhost kernel: exar 3-3:1.1: usb_probe_interface dev->power.disable_depth=1
> May  7 08:42:03 localhost kernel: usb 3-3: usb_probe_interface dev->parent->power.disable_depth=0
> 
> It looks like the interface is being probed twice, which I don't understand.

The first time it is probed by the usbserial_generic driver, the second 
time by your exar driver.

>  But in
> each case the interface has disable_depth set and the parent doesn't.

Ah -- this is because my patch is buggy.  When a probe fails it always 
calls pm_runtime_disable(), even if it didn't call pm_runtime_enable() 
earlier.  Thus the disable count gets incremented, and it never works 
right thereafter.  In fact, this bug is present in the current code as 
well as in the patch.

Try this revised version of the patch instead.

Alan Stern



Index: usb-2.6/drivers/usb/core/driver.c
===================================================================
--- usb-2.6.orig/drivers/usb/core/driver.c
+++ usb-2.6/drivers/usb/core/driver.c
@@ -302,13 +302,15 @@ static int usb_probe_interface(struct de
 
 	intf->condition = USB_INTERFACE_BINDING;
 
-	/* Probed interfaces are initially active.  They are
-	 * runtime-PM-enabled only if the driver has autosuspend support.
-	 * They are sensitive to their children's power states.
+	/* If the driver does not have autosuspend support then we suppress
+	 * autosuspends by making its interfaces active and leaving them
+	 * runtime-PM-disabled, otherwise we do the reverse.  In either case,
+	 * the interfaces are sensitive to their children's power states.
 	 */
-	pm_runtime_set_active(dev);
 	pm_suspend_ignore_children(dev, false);
-	if (driver->supports_autosuspend)
+	if (!driver->supports_autosuspend)
+		pm_runtime_set_active(dev);
+	else
 		pm_runtime_enable(dev);
 
 	/* Carry out a deferred switch to altsetting 0 */
@@ -334,7 +336,8 @@ static int usb_probe_interface(struct de
 	usb_cancel_queued_reset(intf);
 
 	/* Unbound interfaces are always runtime-PM-disabled and -suspended */
-	pm_runtime_disable(dev);
+	if (driver->supports_autosuspend)
+		pm_runtime_disable(dev);
 	pm_runtime_set_suspended(dev);
 
 	usb_autosuspend_device(udev);
@@ -389,7 +392,8 @@ static int usb_unbind_interface(struct d
 	intf->needs_remote_wakeup = 0;
 
 	/* Unbound interfaces are always runtime-PM-disabled and -suspended */
-	pm_runtime_disable(dev);
+	if (driver->supports_autosuspend)
+		pm_runtime_disable(dev);
 	pm_runtime_set_suspended(dev);
 
 	/* Undo any residual pm_autopm_get_interface_* calls */
@@ -438,13 +442,15 @@ int usb_driver_claim_interface(struct us
 
 	iface->condition = USB_INTERFACE_BOUND;
 
-	/* Claimed interfaces are initially inactive (suspended).  They are
-	 * runtime-PM-enabled only if the driver has autosuspend support.
-	 * They are sensitive to their children's power states.
+	/* If the driver does not have autosuspend support then we suppress
+	 * autosuspends by making its interfaces active and leaving them
+	 * runtime-PM-disabled, otherwise we do the reverse.  In either case,
+	 * the interfaces are sensitive to their children's power states.
 	 */
-	pm_runtime_set_suspended(dev);
 	pm_suspend_ignore_children(dev, false);
-	if (driver->supports_autosuspend)
+	if (!driver->supports_autosuspend)
+		pm_runtime_set_active(dev);
+	else
 		pm_runtime_enable(dev);
 
 	/* if interface was already added, bind now; else let

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