diff mbox

v3.16-rc1 regression? unexpected usb_autopm_get_interface error

Message ID 8738f426ma.fsf@nemi.mork.no (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Bjørn Mork June 16, 2014, 3:53 p.m. UTC 
Bjørn Mork <bjorn@mork.no> writes:
> Alan Stern <stern@rowland.harvard.edu> writes:
>> On Mon, 16 Jun 2014, Bjørn Mork wrote:
>>> Bjørn Mork <bjorn@mork.no> writes:
>>> 
>>> > So the problem is related to runtime suspend before first use. I
>>> > strongly suspect 
>>> >
>>> >  aae4518b3124 PM / sleep: Mechanism to avoid resuming runtime-suspended devices unnecessarily
>>> 
>>> Nope, that was not it. So if blind guessing isn't going to work, then I
>>> guess there is no way around a bisect :-)
>>
>> You could simply wait for someone who knows the code to answer the 
>> question.  :-)
>
> Wait? Do I look like I'm patient :-)
>
> Besides, it was actually relieving to bisect a reliably reproducible
> non-crashing bug for once ;-)
>
>> I'm pretty sure this resulted from one of Dan Williams's changes to USB 
>> port runtime PM.  A whole bunch of them were added in 3.15-rc1.
>
> You are *so* much better at guessing than me:
>
> 9262c19d14c433a6a1ba25c3ff897cb89e412309 is the first bad commit

And for completeness, I just tried the following partial revert on top
of v3.16-rc1 and can confirm that it works fine for me:
But I'm not submitting that patch as I assume Dan wants to fix this up
properly.  Whatever that is... I don't understand any of the port magic.


Bjørn

Comments

Alan Stern June 16, 2014, 5:21 p.m. UTC | #1 
On Mon, 16 Jun 2014, Bjørn Mork wrote:

> >> I'm pretty sure this resulted from one of Dan Williams's changes to USB 
> >> port runtime PM.  A whole bunch of them were added in 3.15-rc1.
> >
> > You are *so* much better at guessing than me:
> >
> > 9262c19d14c433a6a1ba25c3ff897cb89e412309 is the first bad commit
> 
> And for completeness, I just tried the following partial revert on top
> of v3.16-rc1 and can confirm that it works fine for me:

I'm not up-to-date on the most recent few releases, so I'll let Dan
take care of this.  In general, it looks like the problem stems from
the fact that when a device is disabled for runtime PM, a
pm_runtime_get_sync() or pm_runtime_resume() call won't succeed, even
if the device is currently at full power.  Maybe we should change this 
in the PM core.

Dan, the warning message in hub_suspend() should mention that the child 
device isn't suspended yet.  Currently it looks like it's saying that 
the port isn't suspended, which gives the wrong impression.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

From f8cba987220ae6cca98d662704256839968c6a61 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B8rn=20Mork?= <bjorn@mork.no>
Date: Mon, 16 Jun 2014 17:26:05 +0200
Subject: [PATCH] usb: fix port runtime pm regression
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is a partial revert of commit 9262c19d14c4 ("usb: disable port
power control if not supported in wHubCharacteristics")

Fixes: 9262c19d14c4 ("usb: disable port power control if not supported in wHubCharacteristics")
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/usb/core/port.c |   13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 62036faf56c0..13a2ffb4b18a 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -411,15 +411,12 @@  int usb_hub_create_port_device(struct usb_hub *hub, int port1)
 
 	pm_runtime_set_active(&port_dev->dev);
 
-	/*
-	 * Do not enable port runtime pm if the hub does not support
-	 * power switching.  Also, userspace must have final say of
-	 * whether a port is permitted to power-off.  Do not enable
-	 * runtime pm if we fail to expose pm_qos_no_power_off.
+	/* It would be dangerous if user space couldn't
+	 * prevent usb device from being powered off. So don't
+	 * enable port runtime pm if failed to expose port's pm qos.
 	 */
-	if (hub_is_port_power_switchable(hub)
-			&& dev_pm_qos_expose_flags(&port_dev->dev,
-			PM_QOS_FLAG_NO_POWER_OFF) == 0)
+	if (!dev_pm_qos_expose_flags(&port_dev->dev,
+			PM_QOS_FLAG_NO_POWER_OFF))
 		pm_runtime_enable(&port_dev->dev);
 
 	device_enable_async_suspend(&port_dev->dev);
-- 
1.7.10.4