Bug 37952
Summary: | ehci_hcd runtime suspend breaks shutdown - Thinkpad T420s | ||
---|---|---|---|
Product: | Drivers | Reporter: | Alex Zhavnerchik (alex.vizor) |
Component: | PCI | Assignee: | Rafael J. Wysocki (rjw) |
Status: | CLOSED CODE_FIX | ||
Severity: | high | CC: | alan, alex.vizor, disabled_rt69273, dries, florian, jardiamj, kamil.54002, r.schtz, rjw, roberto.velasquez, rui.zhang, stern, the.ridikulus.rat |
Priority: | P1 | ||
Hardware: | All | ||
OS: | Linux | ||
Kernel Version: | 3.0-rc3 | Subsystem: | |
Regression: | No | Bisected commit-id: | |
Bug Depends on: | |||
Bug Blocks: | 56331 | ||
Attachments: |
Turn of PME-enable during PCI shutdown
lspci -vv output from Thinkpad x220 Patch to drivers/pci/pci-driver.c PCI / PM: Disable wakeup during shutdown for devices not enabled to wake up PCI / PM: Disable wakeup during shutdown for devices not enabled to wake up PCI / PM: Disable wakeup during shutdown for devices not enabled to wake up /proc/acpi wakeup from Thinkpad x220 |
Description
Alex Zhavnerchik
2011-06-19 22:46:35 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. This happens to me on an HP 2415nr laptop. Same issue, computer shutsdown but after a couple of seconds it turns on again. I have this problem on kernels >2.6.37. Laptop lenovo x220i. It's great that the kernel bugzilla is back. Can you please verify if the problem still exists in the latest upstream kernel? *** Bug 41912 has been marked as a duplicate of this bug. *** 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). 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?
The patch fixes the problem for me on Thinkpad x220 with kernel 3.2.1. Great job! However, it would prevent Wake-on-Lan, for example, from working on those machines from the "power off" state. 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? 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.
This is the first time I can remember seeing a USB controller that supported PME# in D3cold. Maybe that's relevant... 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? 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. (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. 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).
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.
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.
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? Wakeup settings are controlled via the /sys/bus/pci/devices/*/power/wakeup file. If the file contains "enabled", echo "disabled" to it. 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? 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. (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? (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? (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. 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. 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 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. |