From c111566bea7ccd8a05e2c56f1fb3cbb6f4b7b441 Mon Sep 17 00:00:00 2001 From: Sakari Ailus Date: Tue, 25 Feb 2020 11:31:02 +0200 Subject: PM: runtime: Add pm_runtime_get_if_active() pm_runtime_get_if_in_use() bumps up the PM-runtime usage count if it is not equal to zero and the device's PM-runtime status is 'active'. This works for drivers that do not use autoidle, but for those that do, the function returns zero even when the device is active. In order to maintain sane device state while the device is powered on in the hope that it'll be needed, pm_runtime_get_if_active(dev, true) returns a positive value if the device's PM-runtime status is 'active' when it is called, in which case it also increments the device's usage count. If the second argument of pm_runtime_get_if_active() is 'false', the function behaves just like pm_runtime_get_if_in_use(), so redefine the latter as a wrapper around the former. Signed-off-by: Sakari Ailus [ rjw: Changelog ] Signed-off-by: Rafael J. Wysocki --- Documentation/power/runtime_pm.rst | 6 ++++++ drivers/base/power/runtime.c | 36 +++++++++++++++++++++++++++--------- include/linux/pm_runtime.h | 12 +++++++++++- 3 files changed, 44 insertions(+), 10 deletions(-) diff --git a/Documentation/power/runtime_pm.rst b/Documentation/power/runtime_pm.rst index ab8406c84254..0553008b6279 100644 --- a/Documentation/power/runtime_pm.rst +++ b/Documentation/power/runtime_pm.rst @@ -382,6 +382,12 @@ drivers/base/power/runtime.c and include/linux/pm_runtime.h: nonzero, increment the counter and return 1; otherwise return 0 without changing the counter + `int pm_runtime_get_if_active(struct device *dev, bool ign_usage_count);` + - return -EINVAL if 'power.disable_depth' is nonzero; otherwise, if the + runtime PM status is RPM_ACTIVE, and either ign_usage_count is true + or the device's usage_count is non-zero, increment the counter and + return 1; otherwise return 0 without changing the counter + `void pm_runtime_put_noidle(struct device *dev);` - decrement the device's usage counter diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 16134a69bf6f..99c7da112c95 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1087,29 +1087,47 @@ int __pm_runtime_resume(struct device *dev, int rpmflags) EXPORT_SYMBOL_GPL(__pm_runtime_resume); /** - * pm_runtime_get_if_in_use - Conditionally bump up the device's usage counter. + * pm_runtime_get_if_active - Conditionally bump up the device's usage counter. * @dev: Device to handle. * * Return -EINVAL if runtime PM is disabled for the device. * - * If that's not the case and if the device's runtime PM status is RPM_ACTIVE - * and the runtime PM usage counter is nonzero, increment the counter and - * return 1. Otherwise return 0 without changing the counter. + * Otherwise, if the device's runtime PM status is RPM_ACTIVE and either + * ign_usage_count is true or the device's usage_count is non-zero, increment + * the counter and return 1. Otherwise return 0 without changing the counter. + * + * If ign_usage_count is true, the function can be used to prevent suspending + * the device when its runtime PM status is RPM_ACTIVE. + * + * If ign_usage_count is false, the function can be used to prevent suspending + * the device when both its runtime PM status is RPM_ACTIVE and its usage_count + * is non-zero. + * + * The caller is resposible for putting the device's usage count when ther + * return value is greater than zero. */ -int pm_runtime_get_if_in_use(struct device *dev) +int pm_runtime_get_if_active(struct device *dev, bool ign_usage_count) { unsigned long flags; int retval; spin_lock_irqsave(&dev->power.lock, flags); - retval = dev->power.disable_depth > 0 ? -EINVAL : - dev->power.runtime_status == RPM_ACTIVE - && atomic_inc_not_zero(&dev->power.usage_count); + if (dev->power.disable_depth > 0) { + retval = -EINVAL; + } else if (dev->power.runtime_status != RPM_ACTIVE) { + retval = 0; + } else if (ign_usage_count) { + retval = 1; + atomic_inc(&dev->power.usage_count); + } else { + retval = atomic_inc_not_zero(&dev->power.usage_count); + } trace_rpm_usage_rcuidle(dev, 0); spin_unlock_irqrestore(&dev->power.lock, flags); + return retval; } -EXPORT_SYMBOL_GPL(pm_runtime_get_if_in_use); +EXPORT_SYMBOL_GPL(pm_runtime_get_if_active); /** * __pm_runtime_set_status - Set runtime PM status of a device. diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h index 22af69d237a6..3bdcbce8141a 100644 --- a/include/linux/pm_runtime.h +++ b/include/linux/pm_runtime.h @@ -38,7 +38,7 @@ extern int pm_runtime_force_resume(struct device *dev); extern int __pm_runtime_idle(struct device *dev, int rpmflags); extern int __pm_runtime_suspend(struct device *dev, int rpmflags); extern int __pm_runtime_resume(struct device *dev, int rpmflags); -extern int pm_runtime_get_if_in_use(struct device *dev); +extern int pm_runtime_get_if_active(struct device *dev, bool ign_usage_count); extern int pm_schedule_suspend(struct device *dev, unsigned int delay); extern int __pm_runtime_set_status(struct device *dev, unsigned int status); extern int pm_runtime_barrier(struct device *dev); @@ -60,6 +60,11 @@ extern void pm_runtime_put_suppliers(struct device *dev); extern void pm_runtime_new_link(struct device *dev); extern void pm_runtime_drop_link(struct device *dev); +static inline int pm_runtime_get_if_in_use(struct device *dev) +{ + return pm_runtime_get_if_active(dev, false); +} + static inline void pm_suspend_ignore_children(struct device *dev, bool enable) { dev->power.ignore_children = enable; @@ -143,6 +148,11 @@ static inline int pm_runtime_get_if_in_use(struct device *dev) { return -EINVAL; } +static inline int pm_runtime_get_if_active(struct device *dev, + bool ign_usage_count) +{ + return -EINVAL; +} static inline int __pm_runtime_set_status(struct device *dev, unsigned int status) { return 0; } static inline int pm_runtime_barrier(struct device *dev) { return 0; } -- cgit From 42beb82ec4dc99a425e0fded34f7aa5276a709ab Mon Sep 17 00:00:00 2001 From: Madhuparna Bhowmik Date: Wed, 4 Mar 2020 01:11:30 +0530 Subject: PM: sleep: core: Use built-in RCU list checking This patch passes the cond argument to list_for_each_entry_rcu() to fix the following false-positive lockdep warnings: (with CONFIG_PROVE_RCU_LIST = y) [ 330.302784] ============================= [ 330.302789] WARNING: suspicious RCU usage [ 330.302796] 5.6.0-rc1+ #5 Not tainted [ 330.302801] ----------------------------- [ 330.302808] drivers/base/power/main.c:326 RCU-list traversed in non-reader section!! [ 330.303303] ============================= [ 330.303307] WARNING: suspicious RCU usage [ 330.303311] 5.6.0-rc1+ #5 Not tainted [ 330.303315] ----------------------------- [ 330.303319] drivers/base/power/main.c:1698 RCU-list traversed in non-reader section!! [ 331.934969] ============================= [ 331.934971] WARNING: suspicious RCU usage [ 331.934973] 5.6.0-rc1+ #5 Not tainted [ 331.934975] ----------------------------- [ 331.934977] drivers/base/power/main.c:1238 RCU-list traversed in non-reader section!! [ 332.467772] WARNING: suspicious RCU usage [ 332.467775] 5.6.0-rc1+ #5 Not tainted [ 332.467775] ----------------------------- [ 332.467778] drivers/base/power/main.c:269 RCU-list traversed in non-reader section!! Signed-off-by: Madhuparna Bhowmik Signed-off-by: Rafael J. Wysocki --- drivers/base/power/main.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c index 0e99a760aebd..6d1dee7051eb 100644 --- a/drivers/base/power/main.c +++ b/drivers/base/power/main.c @@ -40,6 +40,10 @@ typedef int (*pm_callback_t)(struct device *); +#define list_for_each_entry_rcu_locked(pos, head, member) \ + list_for_each_entry_rcu(pos, head, member, \ + device_links_read_lock_held()) + /* * The entries in the dpm_list list are in a depth first order, simply * because children are guaranteed to be discovered after parents, and @@ -266,7 +270,7 @@ static void dpm_wait_for_suppliers(struct device *dev, bool async) * callbacks freeing the link objects for the links in the list we're * walking. */ - list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) + list_for_each_entry_rcu_locked(link, &dev->links.suppliers, c_node) if (READ_ONCE(link->status) != DL_STATE_DORMANT) dpm_wait(link->supplier, async); @@ -323,7 +327,7 @@ static void dpm_wait_for_consumers(struct device *dev, bool async) * continue instead of trying to continue in parallel with its * unregistration). */ - list_for_each_entry_rcu(link, &dev->links.consumers, s_node) + list_for_each_entry_rcu_locked(link, &dev->links.consumers, s_node) if (READ_ONCE(link->status) != DL_STATE_DORMANT) dpm_wait(link->consumer, async); @@ -1235,7 +1239,7 @@ static void dpm_superior_set_must_resume(struct device *dev) idx = device_links_read_lock(); - list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) + list_for_each_entry_rcu_locked(link, &dev->links.suppliers, c_node) link->supplier->power.must_resume = true; device_links_read_unlock(idx); @@ -1695,7 +1699,7 @@ static void dpm_clear_superiors_direct_complete(struct device *dev) idx = device_links_read_lock(); - list_for_each_entry_rcu(link, &dev->links.suppliers, c_node) { + list_for_each_entry_rcu_locked(link, &dev->links.suppliers, c_node) { spin_lock_irq(&link->supplier->power.lock); link->supplier->power.direct_complete = false; spin_unlock_irq(&link->supplier->power.lock); -- cgit From 2591e7b17c0d3f1472f9ddb2dd5922f24fc5e4d9 Mon Sep 17 00:00:00 2001 From: Madhuparna Bhowmik Date: Wed, 4 Mar 2020 01:12:09 +0530 Subject: PM: sleep: wakeup: Use built-in RCU list checking Pass cond argument to list_for_each_entry_rcu() to fix the following false positive lockdep warning and other uses of list_for_each_entry_rcu() in wakeup.c. (CONFIG_PROVE_RCU_LIST = y) [ 331.934648] ============================= [ 331.934650] WARNING: suspicious RCU usage [ 331.934653] 5.6.0-rc1+ #5 Not tainted [ 331.934655] ----------------------------- [ 331.934657] drivers/base/power/wakeup.c:408 RCU-list traversed in non-reader section!! [ 333.025156] ============================= [ 333.025161] WARNING: suspicious RCU usage [ 333.025168] 5.6.0-rc1+ #5 Not tainted [ 333.025173] ----------------------------- [ 333.025180] drivers/base/power/wakeup.c:424 RCU-list traversed in non-reader section!! Signed-off-by: Madhuparna Bhowmik Signed-off-by: Rafael J. Wysocki --- drivers/base/power/wakeup.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c index 27f3e60608e5..7e33a8d829dc 100644 --- a/drivers/base/power/wakeup.c +++ b/drivers/base/power/wakeup.c @@ -24,6 +24,9 @@ suspend_state_t pm_suspend_target_state; #define pm_suspend_target_state (PM_SUSPEND_ON) #endif +#define list_for_each_entry_rcu_locked(pos, head, member) \ + list_for_each_entry_rcu(pos, head, member, \ + srcu_read_lock_held(&wakeup_srcu)) /* * If set, the suspend/hibernate code will abort transitions to a sleep state * if wakeup events are registered during or immediately before the transition. @@ -405,7 +408,7 @@ void device_wakeup_arm_wake_irqs(void) int srcuidx; srcuidx = srcu_read_lock(&wakeup_srcu); - list_for_each_entry_rcu(ws, &wakeup_sources, entry) + list_for_each_entry_rcu_locked(ws, &wakeup_sources, entry) dev_pm_arm_wake_irq(ws->wakeirq); srcu_read_unlock(&wakeup_srcu, srcuidx); } @@ -421,7 +424,7 @@ void device_wakeup_disarm_wake_irqs(void) int srcuidx; srcuidx = srcu_read_lock(&wakeup_srcu); - list_for_each_entry_rcu(ws, &wakeup_sources, entry) + list_for_each_entry_rcu_locked(ws, &wakeup_sources, entry) dev_pm_disarm_wake_irq(ws->wakeirq); srcu_read_unlock(&wakeup_srcu, srcuidx); } @@ -874,7 +877,7 @@ void pm_print_active_wakeup_sources(void) struct wakeup_source *last_activity_ws = NULL; srcuidx = srcu_read_lock(&wakeup_srcu); - list_for_each_entry_rcu(ws, &wakeup_sources, entry) { + list_for_each_entry_rcu_locked(ws, &wakeup_sources, entry) { if (ws->active) { pm_pr_dbg("active wakeup source: %s\n", ws->name); active = 1; @@ -1025,7 +1028,7 @@ void pm_wakep_autosleep_enabled(bool set) int srcuidx; srcuidx = srcu_read_lock(&wakeup_srcu); - list_for_each_entry_rcu(ws, &wakeup_sources, entry) { + list_for_each_entry_rcu_locked(ws, &wakeup_sources, entry) { spin_lock_irq(&ws->lock); if (ws->autosleep_enabled != set) { ws->autosleep_enabled = set; @@ -1104,7 +1107,7 @@ static void *wakeup_sources_stats_seq_start(struct seq_file *m, } *srcuidx = srcu_read_lock(&wakeup_srcu); - list_for_each_entry_rcu(ws, &wakeup_sources, entry) { + list_for_each_entry_rcu_locked(ws, &wakeup_sources, entry) { if (n-- <= 0) return ws; } -- cgit From 51995ff51231f1502987e18ba6820a55a422b684 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Sat, 7 Mar 2020 19:22:16 -0800 Subject: PM: hibernate: fix docs for ioctls that return loff_t via pointer Correctly document how the SNAPSHOT_GET_IMAGE_SIZE and SNAPSHOT_AVAIL_SWAP_SIZE ioctls return their result. Signed-off-by: Eric Biggers Signed-off-by: Rafael J. Wysocki --- Documentation/power/userland-swsusp.rst | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Documentation/power/userland-swsusp.rst b/Documentation/power/userland-swsusp.rst index a0fa51bb1a4d..1cf62d80a9ca 100644 --- a/Documentation/power/userland-swsusp.rst +++ b/Documentation/power/userland-swsusp.rst @@ -69,11 +69,13 @@ SNAPSHOT_PREF_IMAGE_SIZE SNAPSHOT_GET_IMAGE_SIZE return the actual size of the hibernation image + (the last argument should be a pointer to a loff_t variable that + will contain the result if the call is successful) SNAPSHOT_AVAIL_SWAP_SIZE - return the amount of available swap in bytes (the - last argument should be a pointer to an unsigned int variable that will - contain the result if the call is successful). + return the amount of available swap in bytes + (the last argument should be a pointer to a loff_t variable that + will contain the result if the call is successful) SNAPSHOT_ALLOC_SWAP_PAGE allocate a swap page from the resume partition -- cgit From fba616a49fe8929f4b4066f4384039db274cb73e Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Sat, 7 Mar 2020 19:27:01 -0800 Subject: PM / hibernate: Remove unnecessary compat ioctl overrides Since the SNAPSHOT_GET_IMAGE_SIZE, SNAPSHOT_AVAIL_SWAP_SIZE, and SNAPSHOT_ALLOC_SWAP_PAGE ioctls produce an loff_t result, and loff_t is always 64-bit even in the compat case, there's no reason to have the special compat handling for these ioctls. Just remove this unneeded code so that these ioctls call into snapshot_ioctl() directly, doing just the compat_ptr() conversion on the argument. (This unnecessary code was also causing a KMSAN false positive.) Signed-off-by: Eric Biggers Signed-off-by: Rafael J. Wysocki --- kernel/power/user.c | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/kernel/power/user.c b/kernel/power/user.c index 77438954cc2b..58ed9478787f 100644 --- a/kernel/power/user.c +++ b/kernel/power/user.c @@ -409,21 +409,7 @@ snapshot_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg) switch (cmd) { case SNAPSHOT_GET_IMAGE_SIZE: case SNAPSHOT_AVAIL_SWAP_SIZE: - case SNAPSHOT_ALLOC_SWAP_PAGE: { - compat_loff_t __user *uoffset = compat_ptr(arg); - loff_t offset; - mm_segment_t old_fs; - int err; - - old_fs = get_fs(); - set_fs(KERNEL_DS); - err = snapshot_ioctl(file, cmd, (unsigned long) &offset); - set_fs(old_fs); - if (!err && put_user(offset, uoffset)) - err = -EFAULT; - return err; - } - + case SNAPSHOT_ALLOC_SWAP_PAGE: case SNAPSHOT_CREATE_IMAGE: return snapshot_ioctl(file, cmd, (unsigned long) compat_ptr(arg)); -- cgit From 56cb26891ea4180121265dc6b596015772c4a4b8 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Tue, 10 Mar 2020 11:40:23 +0100 Subject: PM / Domains: Allow no domain-idle-states DT property in genpd when parsing Commit 2c361684803e ("PM / Domains: Don't treat zero found compatible idle states as an error"), moved of_genpd_parse_idle_states() towards allowing none compatible idle state to be found for the device node, rather than returning an error code. However, it didn't consider that the "domain-idle-states" DT property may be missing as it's optional, which makes of_count_phandle_with_args() to return -ENOENT. Let's fix this to make the behaviour consistent. Fixes: 2c361684803e ("PM / Domains: Don't treat zero found compatible idle states as an error") Reported-by: Benjamin Gaignard Cc: 4.20+ # 4.20+ Reviewed-by: Sudeep Holla Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 959d6d5eb000..0a01df608849 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -2653,7 +2653,7 @@ static int genpd_iterate_idle_states(struct device_node *dn, ret = of_count_phandle_with_args(dn, "domain-idle-states", NULL); if (ret <= 0) - return ret; + return ret == -ENOENT ? 0 : ret; /* Loop over the phandles until all the requested entry is found */ of_for_each_phandle(&it, ret, dn, "domain-idle-states", NULL, 0) { -- cgit From 7fbee48ea0fb00976ea15db3a2103ef20005a54a Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Tue, 10 Mar 2020 11:40:39 +0100 Subject: cpuidle: psci: Split psci_dt_cpu_init_idle() To make the code a bit more readable, let's move the OSI specific initialization out of the psci_dt_cpu_init_idle() and into a separate function. Reviewed-by: Sudeep Holla Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/cpuidle/cpuidle-psci.c | 46 +++++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/drivers/cpuidle/cpuidle-psci.c b/drivers/cpuidle/cpuidle-psci.c index edd7a54ef0d3..bae9140a65a5 100644 --- a/drivers/cpuidle/cpuidle-psci.c +++ b/drivers/cpuidle/cpuidle-psci.c @@ -160,6 +160,29 @@ int __init psci_dt_parse_state_node(struct device_node *np, u32 *state) return 0; } +static int __init psci_dt_cpu_init_topology(struct cpuidle_driver *drv, + struct psci_cpuidle_data *data, + unsigned int state_count, int cpu) +{ + /* Currently limit the hierarchical topology to be used in OSI mode. */ + if (!psci_has_osi_support()) + return 0; + + data->dev = psci_dt_attach_cpu(cpu); + if (IS_ERR_OR_NULL(data->dev)) + return PTR_ERR_OR_ZERO(data->dev); + + /* + * Using the deepest state for the CPU to trigger a potential selection + * of a shared state for the domain, assumes the domain states are all + * deeper states. + */ + drv->states[state_count - 1].enter = psci_enter_domain_idle_state; + psci_cpuidle_use_cpuhp = true; + + return 0; +} + static int __init psci_dt_cpu_init_idle(struct cpuidle_driver *drv, struct device_node *cpu_node, unsigned int state_count, int cpu) @@ -193,25 +216,10 @@ static int __init psci_dt_cpu_init_idle(struct cpuidle_driver *drv, goto free_mem; } - /* Currently limit the hierarchical topology to be used in OSI mode. */ - if (psci_has_osi_support()) { - data->dev = psci_dt_attach_cpu(cpu); - if (IS_ERR(data->dev)) { - ret = PTR_ERR(data->dev); - goto free_mem; - } - - /* - * Using the deepest state for the CPU to trigger a potential - * selection of a shared state for the domain, assumes the - * domain states are all deeper states. - */ - if (data->dev) { - drv->states[state_count - 1].enter = - psci_enter_domain_idle_state; - psci_cpuidle_use_cpuhp = true; - } - } + /* Initialize optional data, used for the hierarchical topology. */ + ret = psci_dt_cpu_init_topology(drv, data, state_count, cpu); + if (ret < 0) + goto free_mem; /* Idle states parsed correctly, store them in the per-cpu struct. */ data->psci_states = psci_states; -- cgit From 243a98894dc525ad2fbeb608722fcb682be3186d Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Fri, 20 Mar 2020 15:07:29 +0100 Subject: ACPI: PM: s2idle: Fix comment in acpi_s2idle_prepare_late() Fix a comment in acpi_s2idle_prepare_late() that has become outdated after commit f0ac20c3f613 ("ACPI: EC: Fix flushing of pending work"). Fixes: f0ac20c3f613 ("ACPI: EC: Fix flushing of pending work") Signed-off-by: Rafael J. Wysocki --- drivers/acpi/sleep.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c index e5f95922bc21..22b8af33e15f 100644 --- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -982,10 +982,7 @@ static int acpi_s2idle_prepare_late(void) static void acpi_s2idle_sync(void) { - /* - * The EC driver uses the system workqueue and an additional special - * one, so those need to be flushed too. - */ + /* The EC driver uses special workqueues that need to be flushed. */ acpi_ec_flush_work(); acpi_os_wait_events_complete(); /* synchronize Notify handling */ } -- cgit From 87de6594dc45dbf6819f3e0ef92f9331c5a9444c Mon Sep 17 00:00:00 2001 From: Neeraj Upadhyay Date: Mon, 23 Mar 2020 10:38:51 +0530 Subject: PM: sleep: wakeup: Skip wakeup_source_sysfs_remove() if device is not there Skip wakeup_source_sysfs_remove() to fix a NULL pinter dereference via ws->dev, if the wakeup source is unregistered before registering the wakeup class from device_add(). Fixes: 2ca3d1ecb8c4 ("PM / wakeup: Register wakeup class kobj after device is added") Signed-off-by: Neeraj Upadhyay Cc: 5.4+ # 5.4+ [ rjw: Subject & changelog, white space ] Signed-off-by: Rafael J. Wysocki --- drivers/base/power/wakeup.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c index 7e33a8d829dc..92073ac68473 100644 --- a/drivers/base/power/wakeup.c +++ b/drivers/base/power/wakeup.c @@ -244,7 +244,9 @@ void wakeup_source_unregister(struct wakeup_source *ws) { if (ws) { wakeup_source_remove(ws); - wakeup_source_sysfs_remove(ws); + if (ws->dev) + wakeup_source_sysfs_remove(ws); + wakeup_source_destroy(ws); } } -- cgit From 0ce792d660bda990c675eaf14ce09594a9b85cbf Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 25 Mar 2020 11:54:29 +0100 Subject: ACPICA: Allow acpi_any_gpe_status_set() to skip one GPE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The check carried out by acpi_any_gpe_status_set() is not precise enough for the suspend-to-idle implementation in Linux and in some cases it is necessary make it skip one GPE (specifically, the EC GPE) from the check to prevent a race condition leading to a premature system resume from occurring. For this reason, redefine acpi_any_gpe_status_set() to take the number of a GPE to skip as an argument. Link: https://bugzilla.kernel.org/show_bug.cgi?id=206629 Tested-by: Ondřej Caletka Cc: 5.4+ # 5.4+ Signed-off-by: Rafael J. Wysocki --- drivers/acpi/acpica/achware.h | 2 +- drivers/acpi/acpica/evxfgpe.c | 17 +++++++++++----- drivers/acpi/acpica/hwgpe.c | 47 ++++++++++++++++++++++++++++++++++--------- drivers/acpi/sleep.c | 2 +- include/acpi/acpixf.h | 2 +- 5 files changed, 53 insertions(+), 17 deletions(-) diff --git a/drivers/acpi/acpica/achware.h b/drivers/acpi/acpica/achware.h index 6ad0517553d5..ebf6453d0e21 100644 --- a/drivers/acpi/acpica/achware.h +++ b/drivers/acpi/acpica/achware.h @@ -101,7 +101,7 @@ acpi_status acpi_hw_enable_all_runtime_gpes(void); acpi_status acpi_hw_enable_all_wakeup_gpes(void); -u8 acpi_hw_check_all_gpes(void); +u8 acpi_hw_check_all_gpes(acpi_handle gpe_skip_device, u32 gpe_skip_number); acpi_status acpi_hw_enable_runtime_gpe_block(struct acpi_gpe_xrupt_info *gpe_xrupt_info, diff --git a/drivers/acpi/acpica/evxfgpe.c b/drivers/acpi/acpica/evxfgpe.c index f2de66bfd8a7..3be60673e461 100644 --- a/drivers/acpi/acpica/evxfgpe.c +++ b/drivers/acpi/acpica/evxfgpe.c @@ -799,17 +799,19 @@ ACPI_EXPORT_SYMBOL(acpi_enable_all_wakeup_gpes) * * FUNCTION: acpi_any_gpe_status_set * - * PARAMETERS: None + * PARAMETERS: gpe_skip_number - Number of the GPE to skip * * RETURN: Whether or not the status bit is set for any GPE * - * DESCRIPTION: Check the status bits of all enabled GPEs and return TRUE if any - * of them is set or FALSE otherwise. + * DESCRIPTION: Check the status bits of all enabled GPEs, except for the one + * represented by the "skip" argument, and return TRUE if any of + * them is set or FALSE otherwise. * ******************************************************************************/ -u32 acpi_any_gpe_status_set(void) +u32 acpi_any_gpe_status_set(u32 gpe_skip_number) { acpi_status status; + acpi_handle gpe_device; u8 ret; ACPI_FUNCTION_TRACE(acpi_any_gpe_status_set); @@ -819,7 +821,12 @@ u32 acpi_any_gpe_status_set(void) return (FALSE); } - ret = acpi_hw_check_all_gpes(); + status = acpi_get_gpe_device(gpe_skip_number, &gpe_device); + if (ACPI_FAILURE(status)) { + gpe_device = NULL; + } + + ret = acpi_hw_check_all_gpes(gpe_device, gpe_skip_number); (void)acpi_ut_release_mutex(ACPI_MTX_EVENTS); return (ret); diff --git a/drivers/acpi/acpica/hwgpe.c b/drivers/acpi/acpica/hwgpe.c index f4c285c2f595..49c46d4dd070 100644 --- a/drivers/acpi/acpica/hwgpe.c +++ b/drivers/acpi/acpica/hwgpe.c @@ -444,12 +444,19 @@ acpi_hw_enable_wakeup_gpe_block(struct acpi_gpe_xrupt_info *gpe_xrupt_info, return (AE_OK); } +struct acpi_gpe_block_status_context { + struct acpi_gpe_register_info *gpe_skip_register_info; + u8 gpe_skip_mask; + u8 retval; +}; + /****************************************************************************** * * FUNCTION: acpi_hw_get_gpe_block_status * * PARAMETERS: gpe_xrupt_info - GPE Interrupt info * gpe_block - Gpe Block info + * context - GPE list walk context data * * RETURN: Success * @@ -460,12 +467,13 @@ acpi_hw_enable_wakeup_gpe_block(struct acpi_gpe_xrupt_info *gpe_xrupt_info, static acpi_status acpi_hw_get_gpe_block_status(struct acpi_gpe_xrupt_info *gpe_xrupt_info, struct acpi_gpe_block_info *gpe_block, - void *ret_ptr) + void *context) { + struct acpi_gpe_block_status_context *c = context; struct acpi_gpe_register_info *gpe_register_info; u64 in_enable, in_status; acpi_status status; - u8 *ret = ret_ptr; + u8 ret_mask; u32 i; /* Examine each GPE Register within the block */ @@ -485,7 +493,11 @@ acpi_hw_get_gpe_block_status(struct acpi_gpe_xrupt_info *gpe_xrupt_info, continue; } - *ret |= in_enable & in_status; + ret_mask = in_enable & in_status; + if (ret_mask && c->gpe_skip_register_info == gpe_register_info) { + ret_mask &= ~c->gpe_skip_mask; + } + c->retval |= ret_mask; } return (AE_OK); @@ -561,24 +573,41 @@ acpi_status acpi_hw_enable_all_wakeup_gpes(void) * * FUNCTION: acpi_hw_check_all_gpes * - * PARAMETERS: None + * PARAMETERS: gpe_skip_device - GPE devoce of the GPE to skip + * gpe_skip_number - Number of the GPE to skip * * RETURN: Combined status of all GPEs * - * DESCRIPTION: Check all enabled GPEs in all GPE blocks and return TRUE if the + * DESCRIPTION: Check all enabled GPEs in all GPE blocks, except for the one + * represented by the "skip" arguments, and return TRUE if the * status bit is set for at least one of them of FALSE otherwise. * ******************************************************************************/ -u8 acpi_hw_check_all_gpes(void) +u8 acpi_hw_check_all_gpes(acpi_handle gpe_skip_device, u32 gpe_skip_number) { - u8 ret = 0; + struct acpi_gpe_block_status_context context = { + .gpe_skip_register_info = NULL, + .retval = 0, + }; + struct acpi_gpe_event_info *gpe_event_info; + acpi_cpu_flags flags; ACPI_FUNCTION_TRACE(acpi_hw_check_all_gpes); - (void)acpi_ev_walk_gpe_list(acpi_hw_get_gpe_block_status, &ret); + flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock); + + gpe_event_info = acpi_ev_get_gpe_event_info(gpe_skip_device, + gpe_skip_number); + if (gpe_event_info) { + context.gpe_skip_register_info = gpe_event_info->register_info; + context.gpe_skip_mask = acpi_hw_get_gpe_register_bit(gpe_event_info); + } + + acpi_os_release_lock(acpi_gbl_gpe_lock, flags); - return (ret != 0); + (void)acpi_ev_walk_gpe_list(acpi_hw_get_gpe_block_status, &context); + return (context.retval != 0); } #endif /* !ACPI_REDUCED_HARDWARE */ diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c index 22b8af33e15f..a66078644fc5 100644 --- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -1019,7 +1019,7 @@ static bool acpi_s2idle_wake(void) * status bit from unset to set between the checks with the * status bits of all the other GPEs unset. */ - if (acpi_any_gpe_status_set() && !acpi_ec_dispatch_gpe()) + if (acpi_any_gpe_status_set(U32_MAX) && !acpi_ec_dispatch_gpe()) return true; /* diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h index 8e8be989c2a6..a50d6dea44c3 100644 --- a/include/acpi/acpixf.h +++ b/include/acpi/acpixf.h @@ -752,7 +752,7 @@ ACPI_HW_DEPENDENT_RETURN_UINT32(u32 acpi_dispatch_gpe(acpi_handle gpe_device, u3 ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status acpi_disable_all_gpes(void)) ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status acpi_enable_all_runtime_gpes(void)) ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status acpi_enable_all_wakeup_gpes(void)) -ACPI_HW_DEPENDENT_RETURN_UINT32(u32 acpi_any_gpe_status_set(void)) +ACPI_HW_DEPENDENT_RETURN_UINT32(u32 acpi_any_gpe_status_set(u32 gpe_skip_number)) ACPI_HW_DEPENDENT_RETURN_UINT32(u32 acpi_any_fixed_event_status_set(void)) ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status -- cgit From d5406284ff803a578ca503373624312770319054 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 25 Mar 2020 11:55:48 +0100 Subject: ACPI: PM: s2idle: Refine active GPEs check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The check for any active GPEs added by commit fdde0ff8590b ("ACPI: PM: s2idle: Prevent spurious SCIs from waking up the system") turns out to be insufficiently precise to prevent some systems from resuming prematurely due to a spurious EC wakeup, so refine it by first checking if any GPEs other than the EC GPE are active and skipping all of the SCIs coming from the EC that do not produce any genuine wakeup events after processing. Link: https://bugzilla.kernel.org/show_bug.cgi?id=206629 Fixes: fdde0ff8590b ("ACPI: PM: s2idle: Prevent spurious SCIs from waking up the system") Reported-by: Ondřej Caletka Tested-by: Ondřej Caletka Cc: 5.4+ # 5.4+ Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 5 +++++ drivers/acpi/internal.h | 1 + drivers/acpi/sleep.c | 19 ++++++++++--------- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index d1f1cf5d4bf0..9b9094b0b73f 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -2037,6 +2037,11 @@ void acpi_ec_set_gpe_wake_mask(u8 action) acpi_set_gpe_wake_mask(NULL, first_ec->gpe, action); } +bool acpi_ec_other_gpes_active(void) +{ + return acpi_any_gpe_status_set(first_ec ? first_ec->gpe : U32_MAX); +} + bool acpi_ec_dispatch_gpe(void) { u32 ret; diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index 3616daec650b..d44c591c4ee4 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -202,6 +202,7 @@ void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit); #ifdef CONFIG_PM_SLEEP void acpi_ec_flush_work(void); +bool acpi_ec_other_gpes_active(void); bool acpi_ec_dispatch_gpe(void); #endif diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c index a66078644fc5..bb1ae400ec1f 100644 --- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -1010,18 +1010,19 @@ static bool acpi_s2idle_wake(void) return true; /* - * If there are no EC events to process and at least one of the - * other enabled GPEs is active, the wakeup is regarded as a - * genuine one. - * - * Note that the checks below must be carried out in this order - * to avoid returning prematurely due to a change of the EC GPE - * status bit from unset to set between the checks with the - * status bits of all the other GPEs unset. + * If the status bit is set for any enabled GPE other than the + * EC one, the wakeup is regarded as a genuine one. */ - if (acpi_any_gpe_status_set(U32_MAX) && !acpi_ec_dispatch_gpe()) + if (acpi_ec_other_gpes_active()) return true; + /* + * If the EC GPE status bit has not been set, the wakeup is + * regarded as a spurious one. + */ + if (!acpi_ec_dispatch_gpe()) + return false; + /* * Cancel the wakeup and process all pending events in case * there are any wakeup ones in there. -- cgit