Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751317AbdFSSdA (ORCPT ); Mon, 19 Jun 2017 14:33:00 -0400 Received: from iolanthe.rowland.org ([192.131.102.54]:48640 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1751116AbdFSSc6 (ORCPT ); Mon, 19 Jun 2017 14:32:58 -0400 Date: Mon, 19 Jun 2017 14:32:57 -0400 (EDT) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: Bjorn Helgaas cc: Kai-Heng Feng , Bjorn Helgaas , , USB list , , LKML , "Rafael J. Wysocki" , Subject: Re: [PATCH] usb: host: ehci: workaround PME bug on AMD EHCI controller In-Reply-To: <20170619174533.GA20550@bhelgaas-glaptop.roam.corp.google.com> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2362 Lines: 49 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. Alan Stern