From 9a51c6b1f9e0239a9435db036b212498a2a3b75c Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 16 May 2019 12:42:20 +0200 Subject: ACPI/PCI: PM: Add missing wakeup.flags.valid checks Both acpi_pci_need_resume() and acpi_dev_needs_resume() check if the current ACPI wakeup configuration of the device matches what is expected as far as system wakeup from sleep states is concerned, as reflected by the device_may_wakeup() return value for the device. However, they only should do that if wakeup.flags.valid is set for the device's ACPI companion, because otherwise the wakeup.prepare_count value for it is meaningless. Add the missing wakeup.flags.valid checks to these functions. Signed-off-by: Rafael J. Wysocki Reviewed-by: Mika Westerberg --- drivers/acpi/device_pm.c | 4 ++-- drivers/pci/pci-acpi.c | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c index b859d75eaf9f..1f9058b86908 100644 --- a/drivers/acpi/device_pm.c +++ b/drivers/acpi/device_pm.c @@ -952,8 +952,8 @@ static bool acpi_dev_needs_resume(struct device *dev, struct acpi_device *adev) u32 sys_target = acpi_target_system_state(); int ret, state; - if (!pm_runtime_suspended(dev) || !adev || - device_may_wakeup(dev) != !!adev->wakeup.prepare_count) + if (!pm_runtime_suspended(dev) || !adev || (adev->wakeup.flags.valid && + device_may_wakeup(dev) != !!adev->wakeup.prepare_count)) return true; if (sys_target == ACPI_STATE_S0) diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c index c5e1a097d7e3..1897847ceb0c 100644 --- a/drivers/pci/pci-acpi.c +++ b/drivers/pci/pci-acpi.c @@ -733,7 +733,8 @@ static bool acpi_pci_need_resume(struct pci_dev *dev) if (!adev || !acpi_device_power_manageable(adev)) return false; - if (device_may_wakeup(&dev->dev) != !!adev->wakeup.prepare_count) + if (adev->wakeup.flags.valid && + device_may_wakeup(&dev->dev) != !!adev->wakeup.prepare_count) return true; if (acpi_target_system_state() == ACPI_STATE_S0) -- cgit From bb1869012d7b78d1474808cb4c8bd8b272645876 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Thu, 16 May 2019 12:43:19 +0200 Subject: ACPI: PM: Call pm_set_suspend_via_firmware() during hibernation On systems with ACPI platform firmware the last stage of hibernation is analogous to system suspend to S3 (suspend-to-RAM), so it should be handled analogously. In particular, pm_suspend_via_firmware() should return 'true' in that stage to let the callers of it know that control will be passed to the platform firmware going forward, so pm_set_suspend_via_firmware() needs to be called then in analogy with acpi_suspend_begin(). However, the platform hibernation ->begin() callback is invoked during the "freeze" transition (before creating a snapshot image of system memory) as well as during the "hibernate" transition which is the last stage of it and pm_set_suspend_via_firmware() should be invoked by that callback in the latter stage only. In order to implement that redefine the hibernation ->begin() callback to take a pm_message_t argument to indicate which stage of hibernation is taking place and rework acpi_hibernation_begin() and acpi_hibernation_begin_old() to take it into account as needed. Signed-off-by: Rafael J. Wysocki --- drivers/acpi/sleep.c | 39 ++++++++++++++++++++++++--------------- include/linux/suspend.h | 2 +- kernel/power/hibernate.c | 4 ++-- 3 files changed, 27 insertions(+), 18 deletions(-) diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c index e52f1238d2d6..a34deccd7317 100644 --- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -1132,15 +1132,19 @@ void __init acpi_no_s4_hw_signature(void) nosigcheck = true; } -static int acpi_hibernation_begin(void) +static int acpi_hibernation_begin(pm_message_t stage) { - int error; + if (!nvs_nosave) { + int error = suspend_nvs_alloc(); + if (error) + return error; + } - error = nvs_nosave ? 0 : suspend_nvs_alloc(); - if (!error) - acpi_pm_start(ACPI_STATE_S4); + if (stage.event == PM_EVENT_HIBERNATE) + pm_set_suspend_via_firmware(); - return error; + acpi_pm_start(ACPI_STATE_S4); + return 0; } static int acpi_hibernation_enter(void) @@ -1200,7 +1204,7 @@ static const struct platform_hibernation_ops acpi_hibernation_ops = { * function is used if the pre-ACPI 2.0 suspend ordering has been * requested. */ -static int acpi_hibernation_begin_old(void) +static int acpi_hibernation_begin_old(pm_message_t stage) { int error; /* @@ -1211,16 +1215,21 @@ static int acpi_hibernation_begin_old(void) acpi_sleep_tts_switch(ACPI_STATE_S4); error = acpi_sleep_prepare(ACPI_STATE_S4); + if (error) + return error; - if (!error) { - if (!nvs_nosave) - error = suspend_nvs_alloc(); - if (!error) { - acpi_target_sleep_state = ACPI_STATE_S4; - acpi_scan_lock_acquire(); - } + if (!nvs_nosave) { + error = suspend_nvs_alloc(); + if (error) + return error; } - return error; + + if (stage.event == PM_EVENT_HIBERNATE) + pm_set_suspend_via_firmware(); + + acpi_target_sleep_state = ACPI_STATE_S4; + acpi_scan_lock_acquire(); + return 0; } /* diff --git a/include/linux/suspend.h b/include/linux/suspend.h index 6b3ea9ea6a9e..4a2ffd678887 100644 --- a/include/linux/suspend.h +++ b/include/linux/suspend.h @@ -359,7 +359,7 @@ extern void mark_free_pages(struct zone *zone); * platforms which require special recovery actions in that situation. */ struct platform_hibernation_ops { - int (*begin)(void); + int (*begin)(pm_message_t stage); void (*end)(void); int (*pre_snapshot)(void); void (*finish)(void); diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c index c8c272df7154..97522630b1b6 100644 --- a/kernel/power/hibernate.c +++ b/kernel/power/hibernate.c @@ -129,7 +129,7 @@ static int hibernation_test(int level) { return 0; } static int platform_begin(int platform_mode) { return (platform_mode && hibernation_ops) ? - hibernation_ops->begin() : 0; + hibernation_ops->begin(PMSG_FREEZE) : 0; } /** @@ -542,7 +542,7 @@ int hibernation_platform_enter(void) * hibernation_ops->finish() before saving the image, so we should let * the firmware know that we're going to enter the sleep state after all */ - error = hibernation_ops->begin(); + error = hibernation_ops->begin(PMSG_HIBERNATE); if (error) goto Close; -- cgit From d491f2b75237ef37d8867830ab7fad8d9659e853 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 17 May 2019 11:08:50 +0200 Subject: PCI: PM: Avoid possible suspend-to-idle issue If a PCI driver leaves the device handled by it in D0 and calls pci_save_state() on the device in its ->suspend() or ->suspend_late() callback, it can expect the device to stay in D0 over the whole s2idle cycle. However, that may not be the case if there is a spurious wakeup while the system is suspended, because in that case pci_pm_suspend_noirq() will run again after pci_pm_resume_noirq() which calls pci_restore_state(), via pci_pm_default_resume_early(), so state_saved is cleared and the second iteration of pci_pm_suspend_noirq() will invoke pci_prepare_to_sleep() which may change the power state of the device. To avoid that, add a new internal flag, skip_bus_pm, that will be set by pci_pm_suspend_noirq() when it runs for the first time during the given system suspend-resume cycle if the state of the device has been saved already and the device is still in D0. Setting that flag will cause the next iterations of pci_pm_suspend_noirq() to set state_saved for pci_pm_resume_noirq(), so that it always restores the device state from the originally saved data, and avoid calling pci_prepare_to_sleep() for the device. Fixes: 33e4f80ee69b ("ACPI / PM: Ignore spurious SCI wakeups from suspend-to-idle") Signed-off-by: Rafael J. Wysocki Reviewed-by: Keith Busch Reviewed-by: Mika Westerberg --- drivers/pci/pci-driver.c | 17 ++++++++++++++++- include/linux/pci.h | 1 + 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index cae630fe6387..5eadbc3d0969 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -734,6 +734,8 @@ static int pci_pm_suspend(struct device *dev) struct pci_dev *pci_dev = to_pci_dev(dev); const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; + pci_dev->skip_bus_pm = false; + if (pci_has_legacy_pm_support(pci_dev)) return pci_legacy_suspend(dev, PMSG_SUSPEND); @@ -827,7 +829,20 @@ static int pci_pm_suspend_noirq(struct device *dev) } } - if (!pci_dev->state_saved) { + if (pci_dev->skip_bus_pm) { + /* + * The function is running for the second time in a row without + * going through full resume, which is possible only during + * suspend-to-idle in a spurious wakeup case. Moreover, the + * device was originally left in D0, so its power state should + * not be changed here and the device register values saved + * originally should be restored on resume again. + */ + pci_dev->state_saved = true; + } else if (pci_dev->state_saved) { + if (pci_dev->current_state == PCI_D0) + pci_dev->skip_bus_pm = true; + } else { pci_save_state(pci_dev); if (pci_power_manageable(pci_dev)) pci_prepare_to_sleep(pci_dev); diff --git a/include/linux/pci.h b/include/linux/pci.h index 4a5a84d7bdd4..dd436da7eccc 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -344,6 +344,7 @@ struct pci_dev { D3cold, not set for devices powered on/off by the corresponding bridge */ + unsigned int skip_bus_pm:1; /* Internal: Skip bus-level PM */ unsigned int ignore_hotplug:1; /* Ignore hotplug events */ unsigned int hotplug_user_indicators:1; /* SlotCtl indicators controlled exclusively by -- cgit