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
