On Monday, June 19, 2017 02:32:57 PM Alan Stern wrote: > On Mon, 19 Jun 2017, Bjorn Helgaas wrote: > > > > > Have you tested it with system suspend? That is, if you suspend the > > > > whole computer, does plugging or unplugging a USB device cause the > > > > system to wake up? > > > > > > No, the system will not wake up when plugging or unplugging. > > > Tried several times, nether runtime PM enabled nor runtime PM disabled > > > will wake up the system under S3, when (un)plugging USB devices. > > > > Alan, I don't know what this test means for the patch > > (http://marc.info/?l=linux-pci&m=149760607914628&w=2). > > > > pci_target_state() is documented as "return the deepest state from > > which the device can generate wake events." For this device, I guess > > that means D2, and the patch should accomplish that. > > > > I don't know what's supposed to happen to this device when the system > > is in S3. I assume that if the system is in S3, most devices are in > > D3. If this device is in D3, we won't get PMEs, which I guess is what > > Kai-Heng is seeing. Is that the desired behavior? Or do we want the > > PMEs enough that we should leave the device in D2 (if that's even > > possible)? > > It's possible that the test was invalid. Kai-Heng did not say whether > /sys/.../power/wakeup was set to "enabled" for both the EHCI controller > and the USB root hub beneath it, before the test was started. If > either of them was set to "disabled" then we would not expect a plug or > unplug event to wake up the system. > > In any case, the controller should be set to the lowest power setting > that is consistent with the desired wakeup behavior. If wakeup is set > to "enabled" then the state should be D2 -- if possible. That's the > theory, anyway. If the system supports putting devices only into D3 > during S3 sleep then there's no choice, but if we do have a choice then > we should take it. > > BTW, I just noticed that pci_target_state() uses device_may_wakeup() to > get the desired wakeup behavior. That is correct for system sleep, but > it is wrong for runtime PM. For runtime PM, wakeup should be enabled > whenever the hardware allows it, so the test should be > device_can_wakeup(). > > This means that pci_target_state() should behave differently depending > on whether it is called from pci_prepare_to_sleep() or from > pci_finish_runtime_suspend(). Probably nobody noticed this before > because it usually doesn't make any difference. Right, this is a bug. Let me cut a fix for it. Thanks, Rafael
