Rafael J. Wysocki wrote:
On Monday, February 04, 2013 03:56:12 PM Konstantin Khlebnikov wrote:Documentation/power/pci.txt says: | It is expected that the device driver's pm->runtime_suspend() callback will | not attempt to prepare the device for signaling wakeup or to put it into a | low-power state. The driver ought to leave these tasks to the PCI subsystem | that has all of the information necessary to perform them. After commit 42eca2302146fed51335b95128e949ee6f54478f ("PCI: Don't touch card regs after runtime suspend D3") | If the driver takes care of state saving, don't touch any registers on it. pci_pm_runtime_suspend() thinks if state has been saved by ->runtime_suspend() that means device alredy prepared for wakeup and probably no longer accessible. Thus driver must either do all actions or leave all these tasks to PCI subsystem.I don't like this one, because it will generate noise for drivers that legitimately use pci_save_state() in their runtime suspend callbacks and know what they are doing.
Ok, you're right. Both these warnings can be false-positive. For example some of wifi cards easily might be weird enough for this.
Thanks, RafaelSigned-off-by: Konstantin Khlebnikov<khlebnikov@xxxxxxxxxx> Cc: Rafael J. Wysocki<rafael.j.wysocki@xxxxxxxxx> Cc: Bjorn Helgaas<bhelgaas@xxxxxxxxxx> Cc: linux-pci@xxxxxxxxxxxxxxx --- drivers/pci/pci-driver.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index f9aa311..2b0ff9a 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -1009,12 +1009,19 @@ static int pci_pm_runtime_suspend(struct device *dev) return 0; } - if (!pci_dev->state_saved) { - pci_save_state(pci_dev); - pci_finish_runtime_suspend(pci_dev); + if (pci_dev->state_saved) { + WARN_ONCE(pci_dev->current_state == prev, + "PCI PM: Power state of device not changed by %pF\n", + pm->runtime_suspend); + WARN_ONCE(pci_dev_run_wake(pci_dev)&& + !pci_dev->wakeup_prepared, + "PCI PM: Waking of device not configured by %pF\n", + pm->runtime_suspend); + return 0; } - return 0; + pci_save_state(pci_dev); + return pci_finish_runtime_suspend(pci_dev); } static int pci_pm_runtime_resume(struct device *dev) -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
-- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
