Bug 37952 - ehci_hcd runtime suspend breaks shutdown - Thinkpad T420s
Summary: ehci_hcd runtime suspend breaks shutdown - Thinkpad T420s
Status: CLOSED CODE_FIX
Alias: None
Product: Drivers
Classification: Unclassified
Component: PCI (show other bugs)
Hardware: All Linux
: P1 high
Assignee: Rafael J. Wysocki
URL:
Keywords:
: 41912 (view as bug list)
Depends on:
Blocks: 56331
  Show dependency tree
 
Reported: 2011-06-19 22:46 UTC by Alex Zhavnerchik
Modified: 2013-04-09 06:23 UTC (History)
13 users (show)

See Also:
Kernel Version: 3.0-rc3
Subsystem:
Regression: No
Bisected commit-id:


Attachments
Turn of PME-enable during PCI shutdown (583 bytes, patch)
2012-02-03 18:07 UTC, Alan Stern
Details | Diff
lspci -vv output from Thinkpad x220 (29.68 KB, text/plain)
2012-02-03 21:18 UTC, Kamil Iskra
Details
Patch to drivers/pci/pci-driver.c (309 bytes, patch)
2012-02-04 04:15 UTC, Kamil Iskra
Details | Diff
PCI / PM: Disable wakeup during shutdown for devices not enabled to wake up (851 bytes, patch)
2012-02-04 22:07 UTC, Rafael J. Wysocki
Details | Diff
PCI / PM: Disable wakeup during shutdown for devices not enabled to wake up (859 bytes, patch)
2012-02-04 22:11 UTC, Rafael J. Wysocki
Details | Diff
PCI / PM: Disable wakeup during shutdown for devices not enabled to wake up (855 bytes, patch)
2012-02-04 22:15 UTC, Rafael J. Wysocki
Details | Diff
/proc/acpi wakeup from Thinkpad x220 (308 bytes, text/plain)
2012-02-06 04:19 UTC, Kamil Iskra
Details

Description Alex Zhavnerchik 2011-06-19 22:46:35 UTC
I have thinkpad T420s and almost every time when I try to shout it dow it hangs or reboot. Actually it hangs on reboot. Please let me know how I can help in investigation.
Comment 1 Kamil Iskra 2011-08-16 20:47:34 UTC
I'm having probably the same problem with my Thinkpad X220, where shutdown results in a reboot instead.  A workaround I found to work is to "rmmod ehci_hcd" in the shutdown sequence.
Comment 2 Roberto Velasquez 2011-08-23 19:46:39 UTC
This happens to me on an HP 2415nr laptop. Same issue, computer shutsdown but after a couple of seconds it turns on again.
Comment 3 Account Disabled 2011-08-28 21:17:11 UTC
I have this problem on kernels >2.6.37. Laptop lenovo x220i.
Comment 4 Zhang Rui 2012-01-18 05:21:06 UTC
It's great that the kernel bugzilla is back.

Can you please verify if the problem still exists in the latest upstream
kernel?
Comment 5 Zhang Rui 2012-01-18 05:50:48 UTC
*** Bug 41912 has been marked as a duplicate of this bug. ***
Comment 6 Kamil Iskra 2012-01-18 16:25:25 UTC
I can confirm that the problem still exists in 3.2.1.

Please see https://bugzilla.kernel.org/show_bug.cgi?id=36132#c9 for a discussion of a related bug.  The problem there is caused by e1000e, the problem here by ehci_hcd.  The bug is triggered by the same thing: setting /sys/bus/pci/devices/*/power/control to "auto" (as powertop does), and setting the values back to "on" on shutdown fixes it.

Note that the problem with ehci_hcd only seems to take place with 2011-generation Thinkpads.  My 2010-generation Thinkpad x201 does not exhibit it (it does exhibit the other problem with e1000e, though, as do 2011-generation Thinkpads).
Comment 7 Alan Stern 2012-02-03 18:07:35 UTC
Created attachment 72263 [details]
Turn of PME-enable during PCI shutdown

It sounds like these computers don't like to have PME# enabled when they are turned off.  Does this patch help?
Comment 8 Kamil Iskra 2012-02-03 18:46:33 UTC
The patch fixes the problem for me on Thinkpad x220 with kernel 3.2.1.

Great job!
Comment 9 Rafael J. Wysocki 2012-02-03 19:05:44 UTC
However, it would prevent Wake-on-Lan, for example, from working on those machines from the "power off" state.
Comment 10 Alan Stern 2012-02-03 20:30:11 UTC
The real problem is that it would prevent Wake-on-Lan from working on _any_ machine in the power-off state!

It's possible that these devices can use ACPI for wakeup in a way that doesn't involve PME#, and they actively malfunction when PME# is enabled.  Kamil, can you provide the output of "sudo lspci -vv" for your EHCI controllers?
Comment 11 Kamil Iskra 2012-02-03 21:18:41 UTC
Created attachment 72265 [details]
lspci -vv output from Thinkpad x220

lspci -vv output is attached (plugged-in system; no fancy power management going on).

I will test Wake-on-Lan later today and report.
Comment 12 Alan Stern 2012-02-04 02:01:27 UTC
This is the first time I can remember seeing a USB controller that supported PME# in D3cold.  Maybe that's relevant...
Comment 13 Kamil Iskra 2012-02-04 04:15:40 UTC
Created attachment 72275 [details]
Patch to drivers/pci/pci-driver.c

I can confirm that Alan's patch from comment #7 breaks Wake-on-Lan.  So, no cigar.

With respect to PME support in D3cold.  I checked lspci -vv output on an older Thinkpad model (x201) that does not exhibit the ehci problem, and it reports PME support in D3cold too.  So I guess that's not it, is it?

Alan, your patch inspired me to try something.  As I wrote in comment #6, I only see this problem if userspace sets /sys/bus/pci/devices/*/power/control to "auto".  Setting those values back to "on" on shutdown prevents the problem from occurring.  So I replaced your pci_pme_active() call with pm_runtime_forbid(), which is the function that gets called when userspace does "echo on" to the /sys file.  See the attachment.

This fixed the ehci problem for me and Wake-on-Lan still works.  Is this as simple as that or am I missing something?
Comment 14 Alan Stern 2012-02-04 16:10:15 UTC
At this point Rafael should answer.  I'm not at all clear on the details of the Wake-on-Lan implementations, and I'm not even sure about the differences between normal shutdown and transition into S4.

One drawback to calling pm_runtime_forbid() is that it will cause devices which have been put into low-power mode (because they weren't being used) to go back to full power just before the system shuts down.  What we really want to do is disable PME# for devices that shouldn't be allowed to wake up the system, without changing the devices' power states.

For a normal shutdown, this includes everything (unless Wake-on-Lan is supposed to be enabled even during normal shutdown -- I don't how this is determined or where in the code Wake-on-Lan is enabled; likewise for Wake-on-Modem or other mechanisms).  For S4 hibernation, this includes all devices whose wakeup setting is "disabled"; presumably the existing PM routines already take care of this case.
Comment 15 Rafael J. Wysocki 2012-02-04 21:54:27 UTC
(In reply to comment #14)
> At this point Rafael should answer.  I'm not at all clear on the details of
> the
> Wake-on-Lan implementations, and I'm not even sure about the differences
> between normal shutdown and transition into S4.

If Wake-on-Lan is supported, it should be treated in the same way during transitions into S4 and S5 (which is "shutdown" in ACPI).  However, that may depend on the BIOS setup settings too.

> One drawback to calling pm_runtime_forbid() is that it will cause devices
> which
> have been put into low-power mode (because they weren't being used) to go
> back
> to full power just before the system shuts down.  What we really want to do
> is
> disable PME# for devices that shouldn't be allowed to wake up the system,
> without changing the devices' power states.
> 
> For a normal shutdown, this includes everything (unless Wake-on-Lan is
> supposed
> to be enabled even during normal shutdown -- I don't how this is determined
> or
> where in the code Wake-on-Lan is enabled; likewise for Wake-on-Modem or other
> mechanisms).

On a PC, this is done through the PCI layer.  The LAN driver is supposed to call pci_wake_from_d3() and that should do what's needed (ie. set up WoL if the device is enabled to wake up). 

> For S4 hibernation, this includes all devices whose wakeup setting is
> "disabled"; presumably the existing PM routines already take care of
> this case.

Yes, they should.

Hmm.

So the problem is that the device has been enabled to wake up by runtime PM and we don't change the settings during shutdown, which causes the system to wake up immediately after power off, right?

In that case, we can do something similar to the Alan's patch, although a bit more elaborate.  I'll attach a patch shortly.
Comment 16 Rafael J. Wysocki 2012-02-04 22:07:26 UTC
Created attachment 72284 [details]
PCI / PM: Disable wakeup during shutdown for devices not enabled to wake up

Kamil. please check if this patch fixes the issue for you and whether or not it breaks Wake-on-Lan (please make sure that the problematic device is not enabled to wake up the system during the tests).
Comment 17 Rafael J. Wysocki 2012-02-04 22:11:49 UTC
Created attachment 72285 [details]
PCI / PM: Disable wakeup during shutdown for devices not enabled to wake up

Sorry, I made a mistake in the previous patch.  The PCI functions should be called for pci_dev, not for dev.  Attached is a fixed patch.
Comment 18 Rafael J. Wysocki 2012-02-04 22:15:10 UTC
Created attachment 72286 [details]
PCI / PM: Disable wakeup during shutdown for devices not enabled to wake up

Well, it's broken in a different way: device_may_wakeup() still requires a (struct device *) argument.  This time it should be fixed for real.
Comment 19 Kamil Iskra 2012-02-05 02:59:23 UTC
I just tested the patch from comment #18.  Unfortunately, it has no effect.

With respect to the comment #16 ("please make sure that the problematic device is not enabled to wake up the system during the tests") -- how can I make sure of that?
Comment 20 Alan Stern 2012-02-05 03:34:27 UTC
Wakeup settings are controlled via the /sys/bus/pci/devices/*/power/wakeup file.  If the file contains "enabled", echo "disabled" to it.
Comment 21 Rafael J. Wysocki 2012-02-05 10:49:28 UTC
Alternatively, /sys/devices/pci0000\:00/*/power/wakeup .

You need to find the "power" directory corresponding to the EHCI controller and echo "disabled" into its "wakeup" file.

Also, can you post the contents of /proc/acpi/wakeup from your system?
Comment 22 Kamil Iskra 2012-02-06 04:19:52 UTC
Created attachment 72300 [details]
/proc/acpi wakeup from Thinkpad x220

/proc/acpi wakeup from Thinkpad x220 is attached.

You were right, wakeup was enabled on ehci controller(s).  Once I disabled it, the patch from comment #18 had the expected effect.  And no, the patch does not break Wake-on-Lan (I did keep the wakeup enabled on the Ethernet device).

So while the patch works, it doesn't solve the problem "out of the box"; a userspace intervention is still required.  Is that how it should be?

Also, for what it's worth, my older Thinkpad x201 shows the same enabled wakeup status for ehci controller(s), but shutdown works as expected without the patch.
Comment 23 Rafael J. Wysocki 2012-02-06 11:50:43 UTC
(In reply to comment #22)
> Created an attachment (id=72300) [details]
> /proc/acpi wakeup from Thinkpad x220
> 
> /proc/acpi wakeup from Thinkpad x220 is attached.
> 
> You were right, wakeup was enabled on ehci controller(s).  Once I disabled
> it,
> the patch from comment #18 had the expected effect.  And no, the patch does
> not break Wake-on-Lan (I did keep the wakeup enabled on the Ethernet device).

Good, thanks for the confirmation.

Does disabling wakeup on the EHCI controller help without the patch?

> So while the patch works, it doesn't solve the problem "out of the box"; a
> userspace intervention is still required.  Is that how it should be?

Yes, it is.

We can't do anything more here, because there likely are users who actually _want_ some of their devices to wake up systems from "power off" (e.g. USB keyboards).

> Also, for what it's worth, my older Thinkpad x201 shows the same enabled
> wakeup status for ehci controller(s), but shutdown works as expected without
> the patch.

That's because the wakeup doesn't work as expected on the newer machine, i.e. the controller generates wakeup signals as soon as the system is powered off.

Are there any USB devices attached to it?  Mice or Bluetooth dongles in particular?
Comment 24 Kamil Iskra 2012-02-06 23:00:42 UTC
(In reply to comment #23)
> Does disabling wakeup on the EHCI controller help without the patch?

Suspicious fella that I am, I have actually checked that.  Disabling wakeup without the patch did not help; the patch was required.

> That's because the wakeup doesn't work as expected on the newer machine, i.e.
> the controller generates wakeup signals as soon as the system is powered off.

Curiously though, only when the controller is in a low power state.  When fully powered, it does no such thing.

> Are there any USB devices attached to it?  Mice or Bluetooth dongles in
> particular?

Nothing.  Well, integrated webcam and BT are internally attached to the USB bus, but the webcam was inactive at the time and BT was completely off via thinkpad_acpi so it wouldn't even show under /sys/bus/usb/devices.

Anyway, so what would be your recommendation?  Assuming that your patch gets in, should people affected by this bug modify their system shutdown scripts to disable ehci wakeups?
Comment 25 Rafael J. Wysocki 2012-02-06 23:24:18 UTC
(In reply to comment #24)
> (In reply to comment #23)
> > Does disabling wakeup on the EHCI controller help without the patch?
> 
> Suspicious fella that I am, I have actually checked that.  Disabling wakeup
> without the patch did not help; the patch was required.

OK, so I'm going to submit it for inclusion.

> > That's because the wakeup doesn't work as expected on the newer machine,
> > i.e. the controller generates wakeup signals as soon as the system is
> > powered off.
> 
> Curiously though, only when the controller is in a low power state.  When
> fully powered, it does no such thing.

That's because it's only configured to generate wakeup events (by runtime PM)
while being put into a low-power state.

> > Are there any USB devices attached to it?  Mice or Bluetooth dongles in
> > particular?
> 
> Nothing.  Well, integrated webcam and BT are internally attached to the USB
> bus, but the webcam was inactive at the time and BT was completely off via
> thinkpad_acpi so it wouldn't even show under /sys/bus/usb/devices.
> 
> Anyway, so what would be your recommendation?  Assuming that your patch gets
> in, should people affected by this bug modify their system shutdown scripts
> to
> disable ehci wakeups?

Yes, I would recommend that if the devices on the controller (if any) are not supposed to actually wake up the system.
Comment 26 Alan Stern 2012-02-07 15:17:48 UTC
Or modify their startup scripts instead, so that wakeup gets disabled right away.

It would be interesting to know why these systems wake up for no apparent reason.  Perhaps some misconfiguration of the motherboard.
Comment 27 Florian Mickler 2012-04-04 14:55:49 UTC
A patch referencing this bug report has been merged in Linux v3.4-rc1:

commit 5b415f1e79e0c09366f26e3eabe751642059285a
Author: Rafael J. Wysocki <rjw@sisk.pl>
Date:   Tue Feb 7 00:50:35 2012 +0100

    PCI / PM: Disable wakeup during shutdown for devices not enabled to wake up
Comment 28 Jardiamj 2012-06-13 04:37:20 UTC
I have had this problem for a long time. I suspect that in my laptop the fingerprint reader wakes it up right after shutting down.
The interesting thing is that it only happens when it's running on battery when on power it shuts down just fine.
One thing I've noticed is that in Linux at first everything shuts down including my fingerprint reader, then the fingerprint reader comes back on and the laptop boots right away. When shut down from W7, the fingerprint reader never goes off, its little green light stays on all the time.
I don't know if any of this is relevant, but it might be.

Note You need to log in before you can comment on or make changes to this bug.