From e7d90cfac5510f8c94baa18f9f3f7808859c8332 Mon Sep 17 00:00:00 2001
From: Ulf Hansson <ulf.hansson@linaro.org>
Date: Thu, 17 Feb 2022 13:49:50 +0100
Subject: PM: domains: Prevent power off for parent unless child is in deepest
state
A PM domain managed by genpd may support multiple idlestates (power-off
states). During genpd_power_off() a genpd governor may be asked to select
one of the idlestates based upon the dev PM QoS constraints, for example.
However, there is a problem with the behaviour around this in genpd. More
precisely, a parent-domain is allowed to be powered off, no matter of what
idlestate that has been selected for the child-domain.
For the stm32mp1 platform from STMicro, this behaviour doesn't play well.
Instead, the parent-domain must not be powered off, unless the deepest
idlestate has been selected for the child-domain. As the current behaviour
in genpd is quite questionable anyway, let's simply change it into what is
needed by the stm32mp1 platform.
If it surprisingly turns out that other platforms may need a different
behaviour from genpd, then we will have to revisit this to find a way to
make it configurable.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/base/power/domain.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
(limited to 'drivers/base')
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 5db704f02e71..c87588c21700 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -636,6 +636,18 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
atomic_read(&genpd->sd_count) > 0)
return -EBUSY;
+ /*
+ * The children must be in their deepest (powered-off) states to allow
+ * the parent to be powered off. Note that, there's no need for
+ * additional locking, as powering on a child, requires the parent's
+ * lock to be acquired first.
+ */
+ list_for_each_entry(link, &genpd->parent_links, parent_node) {
+ struct generic_pm_domain *child = link->child;
+ if (child->state_idx < child->state_count - 1)
+ return -EBUSY;
+ }
+
list_for_each_entry(pdd, &genpd->dev_list, list_node) {
enum pm_qos_flags_status stat;
@@ -1073,6 +1085,13 @@ static void genpd_sync_power_off(struct generic_pm_domain *genpd, bool use_lock,
|| atomic_read(&genpd->sd_count) > 0)
return;
+ /* Check that the children are in their deepest (powered-off) state. */
+ list_for_each_entry(link, &genpd->parent_links, parent_node) {
+ struct generic_pm_domain *child = link->child;
+ if (child->state_idx < child->state_count - 1)
+ return;
+ }
+
/* Choose the deepest state when suspending */
genpd->state_idx = genpd->state_count - 1;
if (_genpd_power_off(genpd, false))
--
cgit
From 9a6582b839281ee0e874621f1a2139d2aeb9489e Mon Sep 17 00:00:00 2001
From: Ahmad Fatoum <a.fatoum@pengutronix.de>
Date: Wed, 23 Feb 2022 09:03:23 +0100
Subject: PM: domains: use dev_err_probe() to simplify error handling
dev_err_probe() can reduce code size, makes the code easier to read
and has the added benefit of recording the defer reason for later
read out. Use it where appropriate.
This also fixes an issue, where an error message in __genpd_dev_pm_attach
was not terminated by a line break.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/base/power/domain.c | 21 ++++++---------------
1 file changed, 6 insertions(+), 15 deletions(-)
(limited to 'drivers/base')
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index c87588c21700..c0d9ad01b32c 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2267,12 +2267,8 @@ int of_genpd_add_provider_simple(struct device_node *np,
/* Parse genpd OPP table */
if (genpd->set_performance_state) {
ret = dev_pm_opp_of_add_table(&genpd->dev);
- if (ret) {
- if (ret != -EPROBE_DEFER)
- dev_err(&genpd->dev, "Failed to add OPP table: %d\n",
- ret);
- return ret;
- }
+ if (ret)
+ return dev_err_probe(&genpd->dev, ret, "Failed to add OPP table\n");
/*
* Save table for faster processing while setting performance
@@ -2331,9 +2327,8 @@ int of_genpd_add_provider_onecell(struct device_node *np,
if (genpd->set_performance_state) {
ret = dev_pm_opp_of_add_table_indexed(&genpd->dev, i);
if (ret) {
- if (ret != -EPROBE_DEFER)
- dev_err(&genpd->dev, "Failed to add OPP table for index %d: %d\n",
- i, ret);
+ dev_err_probe(&genpd->dev, ret,
+ "Failed to add OPP table for index %d\n", i);
goto error;
}
@@ -2691,12 +2686,8 @@ static int __genpd_dev_pm_attach(struct device *dev, struct device *base_dev,
ret = genpd_add_device(pd, dev, base_dev);
mutex_unlock(&gpd_list_lock);
- if (ret < 0) {
- if (ret != -EPROBE_DEFER)
- dev_err(dev, "failed to add to PM domain %s: %d",
- pd->name, ret);
- return ret;
- }
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "failed to add to PM domain %s\n", pd->name);
dev->pm_domain->detach = genpd_dev_pm_detach;
dev->pm_domain->sync = genpd_dev_pm_sync;
--
cgit
From f6bfe8b5b2c2a5ac8bd2fc7bca3706e6c3fc26d8 Mon Sep 17 00:00:00 2001
From: Shawn Guo <shawn.guo@linaro.org>
Date: Fri, 25 Feb 2022 14:48:15 +0800
Subject: PM: domains: Fix sleep-in-atomic bug caused by genpd_debug_remove()
When a genpd with GENPD_FLAG_IRQ_SAFE gets removed, the following
sleep-in-atomic bug will be seen, as genpd_debug_remove() will be called
with a spinlock being held.
[ 0.029183] BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:1460
[ 0.029204] in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 1, name: swapper/0
[ 0.029219] preempt_count: 1, expected: 0
[ 0.029230] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.17.0-rc4+ #489
[ 0.029245] Hardware name: Thundercomm TurboX CM2290 (DT)
[ 0.029256] Call trace:
[ 0.029265] dump_backtrace.part.0+0xbc/0xd0
[ 0.029285] show_stack+0x3c/0xa0
[ 0.029298] dump_stack_lvl+0x7c/0xa0
[ 0.029311] dump_stack+0x18/0x34
[ 0.029323] __might_resched+0x10c/0x13c
[ 0.029338] __might_sleep+0x4c/0x80
[ 0.029351] down_read+0x24/0xd0
[ 0.029363] lookup_one_len_unlocked+0x9c/0xcc
[ 0.029379] lookup_positive_unlocked+0x10/0x50
[ 0.029392] debugfs_lookup+0x68/0xac
[ 0.029406] genpd_remove.part.0+0x12c/0x1b4
[ 0.029419] of_genpd_remove_last+0xa8/0xd4
[ 0.029434] psci_cpuidle_domain_probe+0x174/0x53c
[ 0.029449] platform_probe+0x68/0xe0
[ 0.029462] really_probe+0x190/0x430
[ 0.029473] __driver_probe_device+0x90/0x18c
[ 0.029485] driver_probe_device+0x40/0xe0
[ 0.029497] __driver_attach+0xf4/0x1d0
[ 0.029508] bus_for_each_dev+0x70/0xd0
[ 0.029523] driver_attach+0x24/0x30
[ 0.029534] bus_add_driver+0x164/0x22c
[ 0.029545] driver_register+0x78/0x130
[ 0.029556] __platform_driver_register+0x28/0x34
[ 0.029569] psci_idle_init_domains+0x1c/0x28
[ 0.029583] do_one_initcall+0x50/0x1b0
[ 0.029595] kernel_init_freeable+0x214/0x280
[ 0.029609] kernel_init+0x2c/0x13c
[ 0.029622] ret_from_fork+0x10/0x20
It doesn't seem necessary to call genpd_debug_remove() with the lock, so
move it out from locking to fix the problem.
Fixes: 718072ceb211 ("PM: domains: create debugfs nodes when adding power domains")
Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Cc: 5.11+ <stable@vger.kernel.org> # 5.11+
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/base/power/domain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
(limited to 'drivers/base')
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index c0d9ad01b32c..1ee878d126fd 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2077,9 +2077,9 @@ static int genpd_remove(struct generic_pm_domain *genpd)
kfree(link);
}
- genpd_debug_remove(genpd);
list_del(&genpd->gpd_list_node);
genpd_unlock(genpd);
+ genpd_debug_remove(genpd);
cancel_work_sync(&genpd->power_off_work);
if (genpd_is_cpu_domain(genpd))
free_cpumask_var(genpd->cpus);
--
cgit
From 7dfe105dfc724c82ed3d79a4c47439c516a2410b Mon Sep 17 00:00:00 2001
From: Tom Rix <trix@redhat.com>
Date: Fri, 11 Feb 2022 08:10:27 -0800
Subject: PM: sleep: wakeup: Fix typos in comments
Remove the second 'the'.
Replace the second 'of' with 'the'.
Replace 'couter' with 'counter'.
Signed-off-by: Tom Rix <trix@redhat.com>
Acked-by: Randy Dunlap <rdunlap@infradead.org>
Acked-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/base/power/wakeirq.c | 2 +-
drivers/base/power/wakeup.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
(limited to 'drivers/base')
diff --git a/drivers/base/power/wakeirq.c b/drivers/base/power/wakeirq.c
index 0004db4a9d3b..d487a6bac630 100644
--- a/drivers/base/power/wakeirq.c
+++ b/drivers/base/power/wakeirq.c
@@ -289,7 +289,7 @@ EXPORT_SYMBOL_GPL(dev_pm_disable_wake_irq);
*
* Enables wakeirq conditionally. We need to enable wake-up interrupt
* lazily on the first rpm_suspend(). This is needed as the consumer device
- * starts in RPM_SUSPENDED state, and the the first pm_runtime_get() would
+ * starts in RPM_SUSPENDED state, and the first pm_runtime_get() would
* otherwise try to disable already disabled wakeirq. The wake-up interrupt
* starts disabled with IRQ_NOAUTOEN set.
*
diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index 8666590201c9..a57d469676ca 100644
--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -587,7 +587,7 @@ static bool wakeup_source_not_registered(struct wakeup_source *ws)
* @ws: Wakeup source to handle.
*
* Update the @ws' statistics and, if @ws has just been activated, notify the PM
- * core of the event by incrementing the counter of of wakeup events being
+ * core of the event by incrementing the counter of the wakeup events being
* processed.
*/
static void wakeup_source_activate(struct wakeup_source *ws)
@@ -733,7 +733,7 @@ static void wakeup_source_deactivate(struct wakeup_source *ws)
/*
* Increment the counter of registered wakeup events and decrement the
- * couter of wakeup events in progress simultaneously.
+ * counter of wakeup events in progress simultaneously.
*/
cec = atomic_add_return(MAX_IN_PROGRESS, &combined_event_count);
trace_wakeup_source_deactivate(ws->name, cec);
--
cgit
From b4060db9251f919506e4d672737c6b8ab9a84701 Mon Sep 17 00:00:00 2001
From: Douglas Anderson <dianders@chromium.org>
Date: Wed, 23 Feb 2022 08:34:48 -0800
Subject: PM: runtime: Have devm_pm_runtime_enable() handle
pm_runtime_dont_use_autosuspend()
The PM Runtime docs say:
Drivers in ->remove() callback should undo the runtime PM changes done
in ->probe(). Usually this means calling pm_runtime_disable(),
pm_runtime_dont_use_autosuspend() etc.
From grepping code, it's clear that many people aren't aware of the
need to call pm_runtime_dont_use_autosuspend().
When brainstorming solutions, one idea that came up was to leverage
the new-ish devm_pm_runtime_enable() function. The idea here is that:
* When the devm action is called we know that the driver is being
removed. It's the perfect time to undo the use_autosuspend.
* The code of pm_runtime_dont_use_autosuspend() already handles the
case of being called when autosuspend wasn't enabled.
Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/base/power/runtime.c | 5 +++++
1 file changed, 5 insertions(+)
(limited to 'drivers/base')
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 2f3cce17219b..d4059e6ffeae 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -1476,11 +1476,16 @@ EXPORT_SYMBOL_GPL(pm_runtime_enable);
static void pm_runtime_disable_action(void *data)
{
+ pm_runtime_dont_use_autosuspend(data);
pm_runtime_disable(data);
}
/**
* devm_pm_runtime_enable - devres-enabled version of pm_runtime_enable.
+ *
+ * NOTE: this will also handle calling pm_runtime_dont_use_autosuspend() for
+ * you at driver exit time if needed.
+ *
* @dev: Device to handle.
*/
int devm_pm_runtime_enable(struct device *dev)
--
cgit
From a759de6991b35ad437adba32b5f0cb2fd9e75929 Mon Sep 17 00:00:00 2001
From: Youngjin Jang <yj84.jang@samsung.com>
Date: Tue, 8 Mar 2022 04:07:39 +0900
Subject: PM: sleep: Add device name to suspend_report_result()
Currently, suspend_report_result() prints only function information.
If any driver uses a common PM function, nobody knows who exactly
called the failing function.
A device pinter is needed to recognize the failing device.
For example:
PM: dpm_run_callback(): pnp_bus_suspend+0x0/0x10 returns 0
PM: dpm_run_callback(): pci_pm_suspend+0x0/0x150 returns 0
become after the change:
serial 00:05: PM: dpm_run_callback(): pnp_bus_suspend+0x0/0x10 returns 0
pci 0000:00:01.3: PM: dpm_run_callback(): pci_pm_suspend+0x0/0x150 returns 0
Signed-off-by: Youngjin Jang <yj84.jang@samsung.com>
[ rjw: Changelog edits ]
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/base/power/main.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
(limited to 'drivers/base')
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 04ea92cbd9cf..41e17b8c2c20 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -485,7 +485,7 @@ static int dpm_run_callback(pm_callback_t cb, struct device *dev,
trace_device_pm_callback_start(dev, info, state.event);
error = cb(dev);
trace_device_pm_callback_end(dev, error);
- suspend_report_result(cb, error);
+ suspend_report_result(dev, cb, error);
initcall_debug_report(dev, calltime, cb, error);
@@ -1568,7 +1568,7 @@ static int legacy_suspend(struct device *dev, pm_message_t state,
trace_device_pm_callback_start(dev, info, state.event);
error = cb(dev, state);
trace_device_pm_callback_end(dev, error);
- suspend_report_result(cb, error);
+ suspend_report_result(dev, cb, error);
initcall_debug_report(dev, calltime, cb, error);
@@ -1855,7 +1855,7 @@ unlock:
device_unlock(dev);
if (ret < 0) {
- suspend_report_result(callback, ret);
+ suspend_report_result(dev, callback, ret);
pm_runtime_put(dev);
return ret;
}
@@ -1960,10 +1960,10 @@ int dpm_suspend_start(pm_message_t state)
}
EXPORT_SYMBOL_GPL(dpm_suspend_start);
-void __suspend_report_result(const char *function, void *fn, int ret)
+void __suspend_report_result(const char *function, struct device *dev, void *fn, int ret)
{
if (ret)
- pr_err("%s(): %pS returns %d\n", function, fn, ret);
+ dev_err(dev, "%s(): %pS returns %d\n", function, fn, ret);
}
EXPORT_SYMBOL_GPL(__suspend_report_result);
--
cgit
From 524bb1da785a7ae43dd413cd392b5071c6c367f8 Mon Sep 17 00:00:00 2001
From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Date: Sat, 5 Mar 2022 14:02:14 +0300
Subject: PM: core: keep irq flags in device_pm_check_callbacks()
The function device_pm_check_callbacks() can be called under the spin
lock (in the reported case it happens from genpd_add_device() ->
dev_pm_domain_set(), when the genpd uses spinlocks rather than mutexes.
However this function uncoditionally uses spin_lock_irq() /
spin_unlock_irq(), thus not preserving the CPU flags. Use the
irqsave/irqrestore instead.
The backtrace for the reference:
[ 2.752010] ------------[ cut here ]------------
[ 2.756769] raw_local_irq_restore() called with IRQs enabled
[ 2.762596] WARNING: CPU: 4 PID: 1 at kernel/locking/irqflag-debug.c:10 warn_bogus_irq_restore+0x34/0x50
[ 2.772338] Modules linked in:
[ 2.775487] CPU: 4 PID: 1 Comm: swapper/0 Tainted: G S 5.17.0-rc6-00384-ge330d0d82eff-dirty #684
[ 2.781384] Freeing initrd memory: 46024K
[ 2.785839] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 2.785841] pc : warn_bogus_irq_restore+0x34/0x50
[ 2.785844] lr : warn_bogus_irq_restore+0x34/0x50
[ 2.785846] sp : ffff80000805b7d0
[ 2.785847] x29: ffff80000805b7d0 x28: 0000000000000000 x27: 0000000000000002
[ 2.785850] x26: ffffd40e80930b18 x25: ffff7ee2329192b8 x24: ffff7edfc9f60800
[ 2.785853] x23: ffffd40e80930b18 x22: ffffd40e80930d30 x21: ffff7edfc0dffa00
[ 2.785856] x20: ffff7edfc09e3768 x19: 0000000000000000 x18: ffffffffffffffff
[ 2.845775] x17: 6572206f74206465 x16: 6c696166203a3030 x15: ffff80008805b4f7
[ 2.853108] x14: 0000000000000000 x13: ffffd40e809550b0 x12: 00000000000003d8
[ 2.860441] x11: 0000000000000148 x10: ffffd40e809550b0 x9 : ffffd40e809550b0
[ 2.867774] x8 : 00000000ffffefff x7 : ffffd40e809ad0b0 x6 : ffffd40e809ad0b0
[ 2.875107] x5 : 000000000000bff4 x4 : 0000000000000000 x3 : 0000000000000000
[ 2.882440] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff7edfc03a8000
[ 2.889774] Call trace:
[ 2.892290] warn_bogus_irq_restore+0x34/0x50
[ 2.896770] _raw_spin_unlock_irqrestore+0x94/0xa0
[ 2.901690] genpd_unlock_spin+0x20/0x30
[ 2.905724] genpd_add_device+0x100/0x2d0
[ 2.909850] __genpd_dev_pm_attach+0xa8/0x23c
[ 2.914329] genpd_dev_pm_attach_by_id+0xc4/0x190
[ 2.919167] genpd_dev_pm_attach_by_name+0x3c/0xd0
[ 2.924086] dev_pm_domain_attach_by_name+0x24/0x30
[ 2.929102] psci_dt_attach_cpu+0x24/0x90
[ 2.933230] psci_cpuidle_probe+0x2d4/0x46c
[ 2.937534] platform_probe+0x68/0xe0
[ 2.941304] really_probe.part.0+0x9c/0x2fc
[ 2.945605] __driver_probe_device+0x98/0x144
[ 2.950085] driver_probe_device+0x44/0x15c
[ 2.954385] __device_attach_driver+0xb8/0x120
[ 2.958950] bus_for_each_drv+0x78/0xd0
[ 2.962896] __device_attach+0xd8/0x180
[ 2.966843] device_initial_probe+0x14/0x20
[ 2.971144] bus_probe_device+0x9c/0xa4
[ 2.975092] device_add+0x380/0x88c
[ 2.978679] platform_device_add+0x114/0x234
[ 2.983067] platform_device_register_full+0x100/0x190
[ 2.988344] psci_idle_init+0x6c/0xb0
[ 2.992113] do_one_initcall+0x74/0x3a0
[ 2.996060] kernel_init_freeable+0x2fc/0x384
[ 3.000543] kernel_init+0x28/0x130
[ 3.004132] ret_from_fork+0x10/0x20
[ 3.007817] irq event stamp: 319826
[ 3.011404] hardirqs last enabled at (319825): [<ffffd40e7eda0268>] __up_console_sem+0x78/0x84
[ 3.020332] hardirqs last disabled at (319826): [<ffffd40e7fd6d9d8>] el1_dbg+0x24/0x8c
[ 3.028458] softirqs last enabled at (318312): [<ffffd40e7ec90410>] _stext+0x410/0x588
[ 3.036678] softirqs last disabled at (318299): [<ffffd40e7ed1bf68>] __irq_exit_rcu+0x158/0x174
[ 3.045607] ---[ end trace 0000000000000000 ]---
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/base/power/main.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
(limited to 'drivers/base')
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 04ea92cbd9cf..08c8a69d7b81 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -2018,7 +2018,9 @@ static bool pm_ops_is_empty(const struct dev_pm_ops *ops)
void device_pm_check_callbacks(struct device *dev)
{
- spin_lock_irq(&dev->power.lock);
+ unsigned long flags;
+
+ spin_lock_irqsave(&dev->power.lock, flags);
dev->power.no_pm_callbacks =
(!dev->bus || (pm_ops_is_empty(dev->bus->pm) &&
!dev->bus->suspend && !dev->bus->resume)) &&
@@ -2027,7 +2029,7 @@ void device_pm_check_callbacks(struct device *dev)
(!dev->pm_domain || pm_ops_is_empty(&dev->pm_domain->ops)) &&
(!dev->driver || (pm_ops_is_empty(dev->driver->pm) &&
!dev->driver->suspend && !dev->driver->resume));
- spin_unlock_irq(&dev->power.lock);
+ spin_unlock_irqrestore(&dev->power.lock, flags);
}
bool dev_pm_skip_suspend(struct device *dev)
--
cgit