Linux Power Management development
 help / color / mirror / Atom feed
* [PATCH v2 0/5] PM: runtime: Improve autosuspend child to parent propagation
@ 2026-11-03 22:14 Brian Norris
  2026-11-03 22:14 ` [PATCH v2 1/5] PM: runtime: Only queue an idle check for RPM-linked suppliers (part 2) Brian Norris
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Brian Norris @ 2026-11-03 22:14 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Fabrice Gasnier, linux-pm, linux-arm-kernel, Ulf Hansson,
	linux-kernel, linux-iio, Andy Shevchenko, Jonathan Cameron,
	David Lechner, Alexandre Torgue, Nuno Sá, Maxime Coquelin,
	Brian Norris

v1:
  https://lore.kernel.org/all/20261002230714.507921-1-briannorris@chromium.org/
  [PATCH 0/5] PM: runtime: Improve autosuspend child to parent propagation

See patch 3 for the primary change here.

In short: today, children can resume/suspend without extending the
autosuspend expiration of their parent. For parents where their activity
is largely driven by their children, this makes it easy to experience
"premature" suspend.

Many .runtime_idle() implementations seem to try to work around this
problem, and one in particular (stm32-adc) documented their problems
heavily in c537d3457542 ("iio: adc: stm32-adc: fix runtime autosuspend
delay when slow polling").

This is an attempt to improve the PM core, as suggested in review for that
change:
https://lore.kernel.org/all/CAPDyKFp=KTf8=zGBSzPYqhjnZpY8xwvjCeM1e-WTKT1QLSxaDA@mail.gmail.com/

This will make it easier to simplify many other drivers too, as a
significant use of .runtime_idle() is to simply extend a parent's
last_busy timestamp, as a proxy for "last activity performed by a
child".

The changes:

1) Preparatory fix, noticed while making the following changes

2) Because the last_busy mathematics get a little more complex, I have
   to change last_busy to use an atomic64_t type

3) Propagate last_busy from dependent devices (children, consumers) to
   their dependencies (parent, suppliers)

4) Add KUnit tests

5) Remove the pm_runtime_mark_last_busy() workaround from the stm32-adc
   driver

To follow on with #5: I believe almost every other .runtime_idle()
implementation in the tree can be removed after patch 3. (There are a
handful of drivers that should just use autosuspend. And perhaps 2 or 3
other drivers that do other non-trivial work with .runtime_idle(). But
most are redundant.) I may work on that next, if this works out.

- Brian

Changes in v2:
 * Update comments to explain when/why rpm_propagate_last_busy()
   performs the update
 * Minor kunit test improvements

Brian Norris (5):
  PM: runtime: Only queue an idle check for RPM-linked suppliers (part
    2)
  PM: runtime: Convert last_busy to atomic64_t
  PM: runtime: Propagate last_busy from dependent to dependency
  PM: runtime: Add tests for last_busy propagation
  iio: adc: stm32-adc: Drop runtime_idle()

 drivers/base/power/runtime-test.c | 99 +++++++++++++++++++++++++++++++
 drivers/base/power/runtime.c      | 30 +++++++++-
 drivers/iio/adc/stm32-adc-core.c  |  9 +--
 include/linux/pm.h                |  3 +-
 include/linux/pm_runtime.h        |  3 +-
 5 files changed, 132 insertions(+), 12 deletions(-)

-- 
2.56.0.398.g882cbad275-goog


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/5] PM: runtime: Only queue an idle check for RPM-linked suppliers (part 2)
  2026-11-03 22:14 [PATCH v2 0/5] PM: runtime: Improve autosuspend child to parent propagation Brian Norris
@ 2026-11-03 22:14 ` Brian Norris
  2026-11-04 14:44   ` Rafael J. Wysocki (Intel)
  2026-11-03 22:14 ` [PATCH v2 2/5] PM: runtime: Convert last_busy to atomic64_t Brian Norris
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Brian Norris @ 2026-11-03 22:14 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Fabrice Gasnier, linux-pm, linux-arm-kernel, Ulf Hansson,
	linux-kernel, linux-iio, Andy Shevchenko, Jonathan Cameron,
	David Lechner, Alexandre Torgue, Nuno Sá, Maxime Coquelin,
	Brian Norris

Commit be5755f023e7 ("PM: runtime: Only queue an idle check for
RPM-linked suppliers") was incomplete. There are two other cases where
we perform idle checks on non-DL_FLAG_PM_RUNTIME suppliers -- in
pm_runtime_set_suspended(), and in cases where we failed to resume
suppliers.

Fixes: 5244f5e2d801 ("PM: runtime: Defer suspending suppliers")
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
Noticed while touching this function in subsequent work.

(no changes since v1)

 drivers/base/power/runtime.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 304a2b123420..9d3508d9b4a2 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -343,6 +343,9 @@ static void __rpm_put_suppliers(struct device *dev, bool try_to_suspend)
 
 	list_for_each_entry_rcu(link, &dev->links.suppliers, c_node,
 				device_links_read_lock_held()) {
+		if (!device_link_test(link, DL_FLAG_PM_RUNTIME))
+			continue;
+
 		pm_runtime_release_supplier(link);
 		if (try_to_suspend)
 			pm_request_idle(link->supplier);
-- 
2.56.0.398.g882cbad275-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 2/5] PM: runtime: Convert last_busy to atomic64_t
  2026-11-03 22:14 [PATCH v2 0/5] PM: runtime: Improve autosuspend child to parent propagation Brian Norris
  2026-11-03 22:14 ` [PATCH v2 1/5] PM: runtime: Only queue an idle check for RPM-linked suppliers (part 2) Brian Norris
@ 2026-11-03 22:14 ` Brian Norris
  2026-11-04  8:21   ` Joshua Crofts
  2026-11-03 22:14 ` [PATCH v2 3/5] PM: runtime: Propagate last_busy from dependent to dependency Brian Norris
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Brian Norris @ 2026-11-03 22:14 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Fabrice Gasnier, linux-pm, linux-arm-kernel, Ulf Hansson,
	linux-kernel, linux-iio, Andy Shevchenko, Jonathan Cameron,
	David Lechner, Alexandre Torgue, Nuno Sá, Maxime Coquelin,
	Brian Norris

I want to use the atomic64 compare/exchange helpers. There are no
similar 32-bit-portable helpers for non-atomic64_t.

As a side effect, this also should avoid some minor load/store tearing
corner cases on 32-bit architectures.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---

Changes in v2:
 * Add <linux/atomic.h> to pm_runtime.h too

 drivers/base/power/runtime.c | 4 +++-
 include/linux/pm.h           | 3 ++-
 include/linux/pm_runtime.h   | 3 ++-
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 9d3508d9b4a2..2a117d6a776f 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -5,6 +5,8 @@
  * Copyright (c) 2009 Rafael J. Wysocki <rjw@sisk.pl>, Novell Inc.
  * Copyright (C) 2010 Alan Stern <stern@rowland.harvard.edu>
  */
+
+#include <linux/atomic.h>
 #include <linux/sched/mm.h>
 #include <linux/ktime.h>
 #include <linux/hrtimer.h>
@@ -186,7 +188,7 @@ u64 pm_runtime_autosuspend_expiration(struct device *dev)
 	if (autosuspend_delay < 0)
 		return 0;
 
-	expires  = READ_ONCE(dev->power.last_busy);
+	expires = atomic64_read(&dev->power.last_busy);
 	expires += (u64)autosuspend_delay * NSEC_PER_MSEC;
 	if (expires > ktime_get_mono_fast_ns())
 		return expires;	/* Expires in the future */
diff --git a/include/linux/pm.h b/include/linux/pm.h
index ef3f1310e749..531e46ff878e 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -8,6 +8,7 @@
 #ifndef _LINUX_PM_H
 #define _LINUX_PM_H
 
+#include <linux/atomic.h>
 #include <linux/completion.h>
 #include <linux/export.h>
 #include <linux/hrtimer_types.h>
@@ -812,7 +813,7 @@ struct dev_pm_info {
 	enum rpm_status		last_status;
 	int			runtime_error;
 	int			autosuspend_delay;
-	u64			last_busy;
+	atomic64_t		last_busy;
 	u64			active_time;
 	u64			suspended_time;
 	u64			accounting_timestamp;
diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
index ae68a9dc7fc5..da537be94732 100644
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -8,6 +8,7 @@
 #ifndef _LINUX_PM_RUNTIME_H
 #define _LINUX_PM_RUNTIME_H
 
+#include <linux/atomic.h>
 #include <linux/device.h>
 #include <linux/notifier.h>
 #include <linux/pm.h>
@@ -242,7 +243,7 @@ static inline bool pm_runtime_has_no_callbacks(struct device *dev)
  */
 static inline void pm_runtime_mark_last_busy(struct device *dev)
 {
-	WRITE_ONCE(dev->power.last_busy, ktime_get_mono_fast_ns());
+	atomic64_set(&dev->power.last_busy, ktime_get_mono_fast_ns());
 }
 
 /**
-- 
2.56.0.398.g882cbad275-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 3/5] PM: runtime: Propagate last_busy from dependent to dependency
  2026-11-03 22:14 [PATCH v2 0/5] PM: runtime: Improve autosuspend child to parent propagation Brian Norris
  2026-11-03 22:14 ` [PATCH v2 1/5] PM: runtime: Only queue an idle check for RPM-linked suppliers (part 2) Brian Norris
  2026-11-03 22:14 ` [PATCH v2 2/5] PM: runtime: Convert last_busy to atomic64_t Brian Norris
@ 2026-11-03 22:14 ` Brian Norris
  2026-11-03 23:17   ` Jonathan Cameron
  2026-11-04 13:34   ` Rafael J. Wysocki (Intel)
  2026-11-03 22:14 ` [PATCH v2 4/5] PM: runtime: Add tests for last_busy propagation Brian Norris
  2026-11-03 22:14 ` [PATCH v2 5/5] iio: adc: stm32-adc: Drop runtime_idle() Brian Norris
  4 siblings, 2 replies; 14+ messages in thread
From: Brian Norris @ 2026-11-03 22:14 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Fabrice Gasnier, linux-pm, linux-arm-kernel, Ulf Hansson,
	linux-kernel, linux-iio, Andy Shevchenko, Jonathan Cameron,
	David Lechner, Alexandre Torgue, Nuno Sá, Maxime Coquelin,
	Brian Norris

When a child device suspends, it does not update the last_busy timestamp
for its parent. If that parent configured autosuspend and didn't
otherwise maintain its last_busy timestamp, it may now be immediately
eligible to suspend. This is probably not expected -- the parent should
wait for its autosuspend delay before suspending.

The effect of this behavior is that a parent device may suspend sooner
than its autosuspend delay, simply because its usage was accounted by
its children, and not by direct references to the parent device.

This was noticed in several cases, and some have implemented
workarounds, such as in commit c537d3457542 ("iio: adc: stm32-adc: fix
runtime autosuspend delay when slow polling"). At the same time, Ulf
suggested these problems "should be solved in the runtime PM core" [1].

Instead of working around the problem in drivers, we propagate last_busy
timestamps from a dependent device to its dependencies any time it may
allow a dependency to suspend -- i.e., when releasing a refcount for its
parent or suppliers. We take care to only propagate the timestamp if it
is larger than the existing busy timestamp.

Note that this works best if the dependent device is using autosuspend
(and therefore updates its last_busy timestamps appropriately), but even
for a non-autosuspend child, this is still somewhat useful --
non-autosuspend devices still automatically update their last_busy every
time they resume.

Link: https://lore.kernel.org/all/CAPDyKFp=KTf8=zGBSzPYqhjnZpY8xwvjCeM1e-WTKT1QLSxaDA@mail.gmail.com/ [1]
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
Cc: Ulf Hansson <ulfh@kernel.org>

Changes in v2:
 * Update comments to explain when/why rpm_propagate_last_busy()
   performs the update

 drivers/base/power/runtime.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 2a117d6a776f..dd306cbabcd3 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -318,6 +318,22 @@ static int rpm_get_suppliers(struct device *dev)
 	return 0;
 }
 
+/*
+ * Propagate last_busy timestamp from one device to another, typically when a
+ * device (@dev) releases a reference count on a dependency (@target) and might
+ * allow it to (auto)suspend. This can, for example, prevent overactive suspend
+ * when a dependency's usage is primarily driven by one of its dependents.
+ */
+static void rpm_propagate_last_busy(struct device *dev, struct device *target)
+{
+	s64 busy = atomic64_read(&dev->power.last_busy);
+	s64 target_busy = atomic64_read(&target->power.last_busy);
+
+	while (target_busy < busy)
+		if (atomic64_try_cmpxchg(&target->power.last_busy, &target_busy, busy))
+			return;
+}
+
 /**
  * pm_runtime_release_supplier - Drop references to device link's supplier.
  * @link: Target device link.
@@ -348,6 +364,7 @@ static void __rpm_put_suppliers(struct device *dev, bool try_to_suspend)
 		if (!device_link_test(link, DL_FLAG_PM_RUNTIME))
 			continue;
 
+		rpm_propagate_last_busy(dev, link->supplier);
 		pm_runtime_release_supplier(link);
 		if (try_to_suspend)
 			pm_request_idle(link->supplier);
@@ -710,6 +727,8 @@ static int rpm_suspend(struct device *dev, int rpmflags)
 
 	if (dev->parent) {
 		parent = dev->parent;
+		if (!parent->power.ignore_children)
+			rpm_propagate_last_busy(dev, parent);
 		atomic_add_unless(&parent->power.child_count, -1, 0);
 	}
 	wake_up_all(&dev->power.wait_queue);
@@ -1368,8 +1387,10 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status)
 		goto out_set;
 
 	if (status == RPM_SUSPENDED) {
-		atomic_add_unless(&parent->power.child_count, -1, 0);
 		notify_parent = !parent->power.ignore_children;
+		if (notify_parent)
+			rpm_propagate_last_busy(dev, parent);
+		atomic_add_unless(&parent->power.child_count, -1, 0);
 	} else {
 		spin_lock_nested(&parent->power.lock, SINGLE_DEPTH_NESTING);
 
-- 
2.56.0.398.g882cbad275-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 4/5] PM: runtime: Add tests for last_busy propagation
  2026-11-03 22:14 [PATCH v2 0/5] PM: runtime: Improve autosuspend child to parent propagation Brian Norris
                   ` (2 preceding siblings ...)
  2026-11-03 22:14 ` [PATCH v2 3/5] PM: runtime: Propagate last_busy from dependent to dependency Brian Norris
@ 2026-11-03 22:14 ` Brian Norris
  2026-11-03 23:21   ` Jonathan Cameron
  2026-11-03 22:14 ` [PATCH v2 5/5] iio: adc: stm32-adc: Drop runtime_idle() Brian Norris
  4 siblings, 1 reply; 14+ messages in thread
From: Brian Norris @ 2026-11-03 22:14 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Fabrice Gasnier, linux-pm, linux-arm-kernel, Ulf Hansson,
	linux-kernel, linux-iio, Andy Shevchenko, Jonathan Cameron,
	David Lechner, Alexandre Torgue, Nuno Sá, Maxime Coquelin,
	Brian Norris

This is a regression test for the problem fixed in "PM: runtime:
Propagate last_busy from dependent to dependency".

Signed-off-by: Brian Norris <briannorris@chromium.org>
---

Changes in v2:
 * Use device_move() instead of manual re-parenting
 * fsleep() instead of msleep()

 drivers/base/power/runtime-test.c | 99 +++++++++++++++++++++++++++++++
 1 file changed, 99 insertions(+)

diff --git a/drivers/base/power/runtime-test.c b/drivers/base/power/runtime-test.c
index 24865ce844fc..00ca3b10543a 100644
--- a/drivers/base/power/runtime-test.c
+++ b/drivers/base/power/runtime-test.c
@@ -4,6 +4,7 @@
  */
 
 #include <linux/cleanup.h>
+#include <linux/delay.h>
 #include <linux/pm_runtime.h>
 #include <linux/workqueue.h>
 #include <kunit/device.h>
@@ -285,6 +286,102 @@ static void pm_runtime_supplier_suspend_test(struct kunit *test)
 	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(norpm_supplier));
 }
 
+static void pm_runtime_parent_last_used_test(struct kunit *test)
+{
+	struct device *parent = kunit_device_register(test, DEVICE_NAME "_parent");
+	struct device *dev = kunit_device_register(test, DEVICE_NAME);
+	u64 expiry1, expiry2;
+
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, parent);
+
+	KUNIT_ASSERT_EQ(test, 0, device_move(dev, parent, DPM_ORDER_NONE));
+
+	pm_runtime_set_active(parent);
+	pm_runtime_set_active(dev);
+	pm_runtime_use_autosuspend(parent);
+	pm_runtime_use_autosuspend(dev);
+
+	pm_runtime_set_autosuspend_delay(dev, 0);
+	/* Effectively infinite delay (prevent finishing autosuspend during this test). */
+	pm_runtime_set_autosuspend_delay(parent, 200000);
+
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_mark_last_busy(parent);
+
+	pm_runtime_get_noresume(dev);
+
+	pm_runtime_enable(parent);
+	pm_runtime_enable(dev);
+
+	expiry1 = pm_runtime_autosuspend_expiration(parent);
+	KUNIT_EXPECT_NE(test, 0, expiry1);
+
+	/* Make absolutely sure the clock advances. */
+	fsleep(1000);
+
+	KUNIT_EXPECT_EQ(test, 0, pm_runtime_put_sync_autosuspend(dev));
+
+	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
+
+	/* Flush, in case any request was pending. */
+	flush_workqueue(pm_wq);
+	KUNIT_EXPECT_FALSE(test, pm_runtime_suspended(parent));
+
+	expiry2 = pm_runtime_autosuspend_expiration(parent);
+	KUNIT_EXPECT_NE(test, 0, expiry2);
+	/* The child's last_busy time should have bumped up the parent's expiration. */
+	KUNIT_EXPECT_GT(test, expiry2, expiry1);
+}
+
+static void pm_runtime_supplier_last_used_test(struct kunit *test)
+{
+	struct device *supplier = kunit_device_register(test, DEVICE_NAME "_supplier");
+	struct device *dev = kunit_device_register(test, DEVICE_NAME);
+	u64 expiry1, expiry2;
+
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, supplier);
+
+	KUNIT_ASSERT_NOT_NULL(test, device_link_add(dev, supplier, DL_FLAG_PM_RUNTIME));
+
+	pm_runtime_set_active(supplier);
+	pm_runtime_set_active(dev);
+	pm_runtime_use_autosuspend(supplier);
+	pm_runtime_use_autosuspend(dev);
+
+	pm_runtime_set_autosuspend_delay(dev, 0);
+	/* Effectively infinite delay (prevent finishing autosuspend during this test). */
+	pm_runtime_set_autosuspend_delay(supplier, 200000);
+
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_mark_last_busy(supplier);
+
+	pm_runtime_get_noresume(dev);
+
+	pm_runtime_enable(supplier);
+	pm_runtime_enable(dev);
+
+	expiry1 = pm_runtime_autosuspend_expiration(supplier);
+	KUNIT_EXPECT_NE(test, 0, expiry1);
+
+	/* Make absolutely sure the clock advances. */
+	fsleep(1000);
+
+	KUNIT_EXPECT_EQ(test, 0, pm_runtime_put_sync_autosuspend(dev));
+
+	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
+
+	/* Flush, in case any request was pending. */
+	flush_workqueue(pm_wq);
+	KUNIT_EXPECT_FALSE(test, pm_runtime_suspended(supplier));
+
+	expiry2 = pm_runtime_autosuspend_expiration(supplier);
+	KUNIT_EXPECT_NE(test, 0, expiry2);
+	/* The consumer's last_busy time should have bumped up the supplier's expiration. */
+	KUNIT_EXPECT_GT(test, expiry2, expiry1);
+}
+
 static struct kunit_case pm_runtime_test_cases[] = {
 	KUNIT_CASE(pm_runtime_depth_test),
 	KUNIT_CASE(pm_runtime_already_suspended_test),
@@ -293,6 +390,8 @@ static struct kunit_case pm_runtime_test_cases[] = {
 	KUNIT_CASE(pm_runtime_error_test),
 	KUNIT_CASE(pm_runtime_probe_active_test),
 	KUNIT_CASE(pm_runtime_supplier_suspend_test),
+	KUNIT_CASE(pm_runtime_parent_last_used_test),
+	KUNIT_CASE(pm_runtime_supplier_last_used_test),
 	{}
 };
 
-- 
2.56.0.398.g882cbad275-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 5/5] iio: adc: stm32-adc: Drop runtime_idle()
  2026-11-03 22:14 [PATCH v2 0/5] PM: runtime: Improve autosuspend child to parent propagation Brian Norris
                   ` (3 preceding siblings ...)
  2026-11-03 22:14 ` [PATCH v2 4/5] PM: runtime: Add tests for last_busy propagation Brian Norris
@ 2026-11-03 22:14 ` Brian Norris
  4 siblings, 0 replies; 14+ messages in thread
From: Brian Norris @ 2026-11-03 22:14 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Fabrice Gasnier, linux-pm, linux-arm-kernel, Ulf Hansson,
	linux-kernel, linux-iio, Andy Shevchenko, Jonathan Cameron,
	David Lechner, Alexandre Torgue, Nuno Sá, Maxime Coquelin,
	Brian Norris, Fabrice Gasnier, Jonathan Cameron

This runtime_idle() implementation was added in commit c537d3457542 ("iio:
adc: stm32-adc: fix runtime autosuspend delay when slow polling"), to work
around a suboptimal behavior in the runtime PM core. It should no longer
be necessary as of "PM: runtime: Propagate last_busy from dependent to
dependency".

Signed-off-by: Brian Norris <briannorris@chromium.org>
Tested-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
Acked-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
---
I do not have these devices to test. I have tested other drivers, and I've
verified this behavior in unit tests as well.

(no changes since v1)

 drivers/iio/adc/stm32-adc-core.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-core.c
index c409edd3a827..0193db7a2a7f 100644
--- a/drivers/iio/adc/stm32-adc-core.c
+++ b/drivers/iio/adc/stm32-adc-core.c
@@ -873,17 +873,10 @@ static int stm32_adc_core_runtime_resume(struct device *dev)
 	return stm32_adc_core_hw_start(dev);
 }
 
-static int stm32_adc_core_runtime_idle(struct device *dev)
-{
-	pm_runtime_mark_last_busy(dev);
-
-	return 0;
-}
-
 static DEFINE_RUNTIME_DEV_PM_OPS(stm32_adc_core_pm_ops,
 				stm32_adc_core_runtime_suspend,
 				stm32_adc_core_runtime_resume,
-				stm32_adc_core_runtime_idle);
+				NULL);
 
 static const struct stm32_adc_priv_cfg stm32f4_adc_priv_cfg = {
 	.regs = &stm32f4_adc_common_regs,
-- 
2.56.0.398.g882cbad275-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/5] PM: runtime: Propagate last_busy from dependent to dependency
  2026-11-03 22:14 ` [PATCH v2 3/5] PM: runtime: Propagate last_busy from dependent to dependency Brian Norris
@ 2026-11-03 23:17   ` Jonathan Cameron
  2026-11-04  0:32     ` Brian Norris
  2026-11-04 13:34   ` Rafael J. Wysocki (Intel)
  1 sibling, 1 reply; 14+ messages in thread
From: Jonathan Cameron @ 2026-11-03 23:17 UTC (permalink / raw)
  To: Brian Norris
  Cc: Rafael J. Wysocki, Fabrice Gasnier, linux-pm, linux-arm-kernel,
	Ulf Hansson, linux-kernel, linux-iio, Andy Shevchenko,
	David Lechner, Alexandre Torgue, Nuno Sá, Maxime Coquelin

On Tue,  3 Nov 2026 14:14:41 -0800
Brian Norris <briannorris@chromium.org> wrote:

> When a child device suspends, it does not update the last_busy timestamp
> for its parent. If that parent configured autosuspend and didn't
> otherwise maintain its last_busy timestamp, it may now be immediately
> eligible to suspend. This is probably not expected -- the parent should
> wait for its autosuspend delay before suspending.

So reading this again - I wonder if this text is describing the
expected behaviour.

If the child is also using autosupend then, in extreme case, that time
might be longer than the autosuspend time of the parent.
If so it seems odd the parent still waits. So most expected to me
would be if the parent were to suspend it's autosuspend time after
the parent or child was last used for something that indicate a
desire not to suspend (that shouldn't include actions the child
did whilst suspending).

Maybe there isn't a sensible way to deal with this and what you have here
is the best we can do.  Note that this is just commenting on this
statement in the commit description.  From what follows I think
this is just a commit description thing rather than functional.


> 
> The effect of this behavior is that a parent device may suspend sooner
> than its autosuspend delay, simply because its usage was accounted by
> its children, and not by direct references to the parent device.

Sooner is indeed bad.

> 
> This was noticed in several cases, and some have implemented
> workarounds, such as in commit c537d3457542 ("iio: adc: stm32-adc: fix
> runtime autosuspend delay when slow polling"). At the same time, Ulf
> suggested these problems "should be solved in the runtime PM core" [1].
> 
> Instead of working around the problem in drivers, we propagate last_busy
> timestamps from a dependent device to its dependencies any time it may
> allow a dependency to suspend -- i.e., when releasing a refcount for its
> parent or suppliers. We take care to only propagate the timestamp if it
> is larger than the existing busy timestamp.

So this sounds fine but I don't think aligns with that first paragraph.

> 
> Note that this works best if the dependent device is using autosuspend
> (and therefore updates its last_busy timestamps appropriately), but even
> for a non-autosuspend child, this is still somewhat useful --
> non-autosuspend devices still automatically update their last_busy every
> time they resume.

Can we set the last busy on the suspend of child devices that aren't
suspending as a result of autosuspend timeout but for other reasons?
(may be some fiddly corners!)

> 
> Link: https://lore.kernel.org/all/CAPDyKFp=KTf8=zGBSzPYqhjnZpY8xwvjCeM1e-WTKT1QLSxaDA@mail.gmail.com/ [1]
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> Cc: Ulf Hansson <ulfh@kernel.org>
> 
> Changes in v2:
>  * Update comments to explain when/why rpm_propagate_last_busy()
>    performs the update
> 
>  drivers/base/power/runtime.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> index 2a117d6a776f..dd306cbabcd3 100644
> --- a/drivers/base/power/runtime.c
> +++ b/drivers/base/power/runtime.c
> @@ -318,6 +318,22 @@ static int rpm_get_suppliers(struct device *dev)
>  	return 0;
>  }
>  
> +/*
> + * Propagate last_busy timestamp from one device to another, typically when a
> + * device (@dev) releases a reference count on a dependency (@target) and might
> + * allow it to (auto)suspend. This can, for example, prevent overactive suspend
> + * when a dependency's usage is primarily driven by one of its dependents.
> + */
> +static void rpm_propagate_last_busy(struct device *dev, struct device *target)
> +{
> +	s64 busy = atomic64_read(&dev->power.last_busy);
> +	s64 target_busy = atomic64_read(&target->power.last_busy);
> +
> +	while (target_busy < busy)
> +		if (atomic64_try_cmpxchg(&target->power.last_busy, &target_busy, busy))
> +			return;
> +}
> +
>  /**
>   * pm_runtime_release_supplier - Drop references to device link's supplier.
>   * @link: Target device link.
> @@ -348,6 +364,7 @@ static void __rpm_put_suppliers(struct device *dev, bool try_to_suspend)
>  		if (!device_link_test(link, DL_FLAG_PM_RUNTIME))
>  			continue;
>  
> +		rpm_propagate_last_busy(dev, link->supplier);
>  		pm_runtime_release_supplier(link);
>  		if (try_to_suspend)
>  			pm_request_idle(link->supplier);
> @@ -710,6 +727,8 @@ static int rpm_suspend(struct device *dev, int rpmflags)
>  
>  	if (dev->parent) {
>  		parent = dev->parent;
> +		if (!parent->power.ignore_children)
> +			rpm_propagate_last_busy(dev, parent);
>  		atomic_add_unless(&parent->power.child_count, -1, 0);
>  	}
>  	wake_up_all(&dev->power.wait_queue);
> @@ -1368,8 +1387,10 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status)
>  		goto out_set;
>  
>  	if (status == RPM_SUSPENDED) {
> -		atomic_add_unless(&parent->power.child_count, -1, 0);
>  		notify_parent = !parent->power.ignore_children;
> +		if (notify_parent)
> +			rpm_propagate_last_busy(dev, parent);
> +		atomic_add_unless(&parent->power.child_count, -1, 0);
>  	} else {
>  		spin_lock_nested(&parent->power.lock, SINGLE_DEPTH_NESTING);
>  


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 4/5] PM: runtime: Add tests for last_busy propagation
  2026-11-03 22:14 ` [PATCH v2 4/5] PM: runtime: Add tests for last_busy propagation Brian Norris
@ 2026-11-03 23:21   ` Jonathan Cameron
  0 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2026-11-03 23:21 UTC (permalink / raw)
  To: Brian Norris
  Cc: Rafael J. Wysocki, Fabrice Gasnier, linux-pm, linux-arm-kernel,
	Ulf Hansson, linux-kernel, linux-iio, Andy Shevchenko,
	David Lechner, Alexandre Torgue, Nuno Sá, Maxime Coquelin

On Tue,  3 Nov 2026 14:14:42 -0800
Brian Norris <briannorris@chromium.org> wrote:

> This is a regression test for the problem fixed in "PM: runtime:
> Propagate last_busy from dependent to dependency".
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>

Trivial style stuff.

> ---
> 
> Changes in v2:
>  * Use device_move() instead of manual re-parenting
>  * fsleep() instead of msleep()
> 
>  drivers/base/power/runtime-test.c | 99 +++++++++++++++++++++++++++++++
>  1 file changed, 99 insertions(+)
> 
> diff --git a/drivers/base/power/runtime-test.c b/drivers/base/power/runtime-test.c
> index 24865ce844fc..00ca3b10543a 100644
> --- a/drivers/base/power/runtime-test.c
> +++ b/drivers/base/power/runtime-test.c
> @@ -4,6 +4,7 @@
>   */
>  
>  #include <linux/cleanup.h>
> +#include <linux/delay.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/workqueue.h>
>  #include <kunit/device.h>
> @@ -285,6 +286,102 @@ static void pm_runtime_supplier_suspend_test(struct kunit *test)
>  	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(norpm_supplier));
>  }
>  
> +static void pm_runtime_parent_last_used_test(struct kunit *test)
> +{
> +	struct device *parent = kunit_device_register(test, DEVICE_NAME "_parent");
> +	struct device *dev = kunit_device_register(test, DEVICE_NAME);

Similar style comment to below.

> +	u64 expiry1, expiry2;
> +
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, parent);
> +
> +	KUNIT_ASSERT_EQ(test, 0, device_move(dev, parent, DPM_ORDER_NONE));
> +
> +	pm_runtime_set_active(parent);
> +	pm_runtime_set_active(dev);
> +	pm_runtime_use_autosuspend(parent);
> +	pm_runtime_use_autosuspend(dev);
> +
> +	pm_runtime_set_autosuspend_delay(dev, 0);
> +	/* Effectively infinite delay (prevent finishing autosuspend during this test). */
> +	pm_runtime_set_autosuspend_delay(parent, 200000);
> +
> +	pm_runtime_mark_last_busy(dev);
> +	pm_runtime_mark_last_busy(parent);
> +
> +	pm_runtime_get_noresume(dev);
> +
> +	pm_runtime_enable(parent);
> +	pm_runtime_enable(dev);
> +
> +	expiry1 = pm_runtime_autosuspend_expiration(parent);
> +	KUNIT_EXPECT_NE(test, 0, expiry1);
> +
> +	/* Make absolutely sure the clock advances. */
> +	fsleep(1000);
> +
> +	KUNIT_EXPECT_EQ(test, 0, pm_runtime_put_sync_autosuspend(dev));
> +
> +	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
> +
> +	/* Flush, in case any request was pending. */
> +	flush_workqueue(pm_wq);
> +	KUNIT_EXPECT_FALSE(test, pm_runtime_suspended(parent));
> +
> +	expiry2 = pm_runtime_autosuspend_expiration(parent);
> +	KUNIT_EXPECT_NE(test, 0, expiry2);
> +	/* The child's last_busy time should have bumped up the parent's expiration. */
> +	KUNIT_EXPECT_GT(test, expiry2, expiry1);
> +}
> +
> +static void pm_runtime_supplier_last_used_test(struct kunit *test)
> +{
> +	struct device *supplier = kunit_device_register(test, DEVICE_NAME "_supplier");
> +	struct device *dev = kunit_device_register(test, DEVICE_NAME);
Having the assignment clear of the error check is never great, but now there are multiple
devs, I'd definitely move them.

	struct device *supplier, *dev;

> +	u64 expiry1, expiry2;
> +
	supplier = kunit_device_register(test, DEVICE_NAME "_supplier");
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev);

	dev = kunit_device_register(test, DEVICE_NAME);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, supplier);

> +
> +	KUNIT_ASSERT_NOT_NULL(test, device_link_add(dev, supplier, DL_FLAG_PM_RUNTIME));
> +
> +	pm_runtime_set_active(supplier);
> +	pm_runtime_set_active(dev);
> +	pm_runtime_use_autosuspend(supplier);
> +	pm_runtime_use_autosuspend(dev);
> +
> +	pm_runtime_set_autosuspend_delay(dev, 0);
> +	/* Effectively infinite delay (prevent finishing autosuspend during this test). */
> +	pm_runtime_set_autosuspend_delay(supplier, 200000);
> +
> +	pm_runtime_mark_last_busy(dev);
> +	pm_runtime_mark_last_busy(supplier);
> +
> +	pm_runtime_get_noresume(dev);
> +
> +	pm_runtime_enable(supplier);
> +	pm_runtime_enable(dev);
> +
> +	expiry1 = pm_runtime_autosuspend_expiration(supplier);
> +	KUNIT_EXPECT_NE(test, 0, expiry1);
> +
> +	/* Make absolutely sure the clock advances. */
> +	fsleep(1000);
> +
> +	KUNIT_EXPECT_EQ(test, 0, pm_runtime_put_sync_autosuspend(dev));
> +
> +	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
> +
> +	/* Flush, in case any request was pending. */
> +	flush_workqueue(pm_wq);
> +	KUNIT_EXPECT_FALSE(test, pm_runtime_suspended(supplier));
> +
> +	expiry2 = pm_runtime_autosuspend_expiration(supplier);
> +	KUNIT_EXPECT_NE(test, 0, expiry2);
> +	/* The consumer's last_busy time should have bumped up the supplier's expiration. */
> +	KUNIT_EXPECT_GT(test, expiry2, expiry1);
> +}
> +
>  static struct kunit_case pm_runtime_test_cases[] = {
>  	KUNIT_CASE(pm_runtime_depth_test),
>  	KUNIT_CASE(pm_runtime_already_suspended_test),
> @@ -293,6 +390,8 @@ static struct kunit_case pm_runtime_test_cases[] = {
>  	KUNIT_CASE(pm_runtime_error_test),
>  	KUNIT_CASE(pm_runtime_probe_active_test),
>  	KUNIT_CASE(pm_runtime_supplier_suspend_test),
> +	KUNIT_CASE(pm_runtime_parent_last_used_test),
> +	KUNIT_CASE(pm_runtime_supplier_last_used_test),
>  	{}
>  };
>  


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/5] PM: runtime: Propagate last_busy from dependent to dependency
  2026-11-03 23:17   ` Jonathan Cameron
@ 2026-11-04  0:32     ` Brian Norris
  2026-11-04  1:40       ` Jonathan Cameron
  0 siblings, 1 reply; 14+ messages in thread
From: Brian Norris @ 2026-11-04  0:32 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Rafael J. Wysocki, Fabrice Gasnier, linux-pm, linux-arm-kernel,
	Ulf Hansson, linux-kernel, linux-iio, Andy Shevchenko,
	David Lechner, Alexandre Torgue, Nuno Sá, Maxime Coquelin

Hi Jonathan,

On Tue, Nov 03, 2026 at 11:17:19PM +0000, Jonathan Cameron wrote:
> On Tue,  3 Nov 2026 14:14:41 -0800
> Brian Norris <briannorris@chromium.org> wrote:
> 
> > When a child device suspends, it does not update the last_busy timestamp
> > for its parent. If that parent configured autosuspend and didn't
> > otherwise maintain its last_busy timestamp, it may now be immediately
> > eligible to suspend. This is probably not expected -- the parent should
> > wait for its autosuspend delay before suspending.
> 
> So reading this again - I wonder if this text is describing the
> expected behaviour.

Agreed, this might give the wrong impression, depending on the
assumptions the reader makes about autosuspend delay. (I think I had a
particular problem scenario in mind, but didn't translate that properly
here.)

> If the child is also using autosupend then, in extreme case, that time
> might be longer than the autosuspend time of the parent.
> If so it seems odd the parent still waits.

Right, that seems wrong, and indeed that's not what this patch does.

> So most expected to me
> would be if the parent were to suspend it's autosuspend time after
> the parent or child was last used for something that indicate a
> desire not to suspend (that shouldn't include actions the child
> did whilst suspending).

(Not sure I'm parsing this sentence quite right, but...)

The point is that last_busy should propagate (rather than, say, the
expiration time). So each device still gets its own-delay-based
autosuspend timer expiration. That may mean the parent waits longer than
the child, but it may not, if the child's delay was longer than the
parent's.

> Maybe there isn't a sensible way to deal with this and what you have here
> is the best we can do.  Note that this is just commenting on this
> statement in the commit description.  From what follows I think
> this is just a commit description thing rather than functional.

If I'm understanding what you're saying correctly: maybe it would be an
improvement here to say:

  If that parent configured an autosuspend delay longer than its child's
  and didn't otherwise maintain its last_busy timestamp, it may now be
  immediately eligible to suspend.

I think that would more accurately describe the target problem, even if
it doesn't cover all cases (such as when the parent delay is shorter).

> > The effect of this behavior is that a parent device may suspend sooner
> > than its autosuspend delay, simply because its usage was accounted by
> > its children, and not by direct references to the parent device.
> 
> Sooner is indeed bad.
> 
> > 
> > This was noticed in several cases, and some have implemented
> > workarounds, such as in commit c537d3457542 ("iio: adc: stm32-adc: fix
> > runtime autosuspend delay when slow polling"). At the same time, Ulf
> > suggested these problems "should be solved in the runtime PM core" [1].
> > 
> > Instead of working around the problem in drivers, we propagate last_busy
> > timestamps from a dependent device to its dependencies any time it may
> > allow a dependency to suspend -- i.e., when releasing a refcount for its
> > parent or suppliers. We take care to only propagate the timestamp if it
> > is larger than the existing busy timestamp.
> 
> So this sounds fine but I don't think aligns with that first paragraph.
> 
> > 
> > Note that this works best if the dependent device is using autosuspend
> > (and therefore updates its last_busy timestamps appropriately), but even
> > for a non-autosuspend child, this is still somewhat useful --
> > non-autosuspend devices still automatically update their last_busy every
> > time they resume.
> 
> Can we set the last busy on the suspend of child devices that aren't
> suspending as a result of autosuspend timeout but for other reasons?
> (may be some fiddly corners!)

Do you have concrete use cases in mind, or do you bring this up simply
because of my note?

For one, I feel like for devices with power.use_autosuspend==true, it's
reasonable to expect drivers to use the appropriate helpers to update
last_busy at the appropriate times, and so any "corner cases" are from
misuse.

But if you mean non-use_autosuspend devices: I suppose it'd be possible
but I haven't reasoned through the implementation very far yet. It
almost seems like we'd need to plumb pm_runtime_mark_last_busy() into
*all* the put() helpers, including the non-autosuspend ones
(pm_runtime_put(), pm_runtime_put_sync()). I'm not quite sure if that's
desirable. But I could try to look a bit more, if you think there are
important use cases that would need this.

Brian

> > Link: https://lore.kernel.org/all/CAPDyKFp=KTf8=zGBSzPYqhjnZpY8xwvjCeM1e-WTKT1QLSxaDA@mail.gmail.com/ [1]
> > Signed-off-by: Brian Norris <briannorris@chromium.org>
> > ---
> > Cc: Ulf Hansson <ulfh@kernel.org>
> > 
> > Changes in v2:
> >  * Update comments to explain when/why rpm_propagate_last_busy()
> >    performs the update
> > 
> >  drivers/base/power/runtime.c | 23 ++++++++++++++++++++++-
> >  1 file changed, 22 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> > index 2a117d6a776f..dd306cbabcd3 100644
> > --- a/drivers/base/power/runtime.c
> > +++ b/drivers/base/power/runtime.c
> > @@ -318,6 +318,22 @@ static int rpm_get_suppliers(struct device *dev)
> >  	return 0;
> >  }
> >  
> > +/*
> > + * Propagate last_busy timestamp from one device to another, typically when a
> > + * device (@dev) releases a reference count on a dependency (@target) and might
> > + * allow it to (auto)suspend. This can, for example, prevent overactive suspend
> > + * when a dependency's usage is primarily driven by one of its dependents.
> > + */
> > +static void rpm_propagate_last_busy(struct device *dev, struct device *target)
> > +{
> > +	s64 busy = atomic64_read(&dev->power.last_busy);
> > +	s64 target_busy = atomic64_read(&target->power.last_busy);
> > +
> > +	while (target_busy < busy)
> > +		if (atomic64_try_cmpxchg(&target->power.last_busy, &target_busy, busy))
> > +			return;
> > +}
> > +
> >  /**
> >   * pm_runtime_release_supplier - Drop references to device link's supplier.
> >   * @link: Target device link.
> > @@ -348,6 +364,7 @@ static void __rpm_put_suppliers(struct device *dev, bool try_to_suspend)
> >  		if (!device_link_test(link, DL_FLAG_PM_RUNTIME))
> >  			continue;
> >  
> > +		rpm_propagate_last_busy(dev, link->supplier);
> >  		pm_runtime_release_supplier(link);
> >  		if (try_to_suspend)
> >  			pm_request_idle(link->supplier);
> > @@ -710,6 +727,8 @@ static int rpm_suspend(struct device *dev, int rpmflags)
> >  
> >  	if (dev->parent) {
> >  		parent = dev->parent;
> > +		if (!parent->power.ignore_children)
> > +			rpm_propagate_last_busy(dev, parent);
> >  		atomic_add_unless(&parent->power.child_count, -1, 0);
> >  	}
> >  	wake_up_all(&dev->power.wait_queue);
> > @@ -1368,8 +1387,10 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status)
> >  		goto out_set;
> >  
> >  	if (status == RPM_SUSPENDED) {
> > -		atomic_add_unless(&parent->power.child_count, -1, 0);
> >  		notify_parent = !parent->power.ignore_children;
> > +		if (notify_parent)
> > +			rpm_propagate_last_busy(dev, parent);
> > +		atomic_add_unless(&parent->power.child_count, -1, 0);
> >  	} else {
> >  		spin_lock_nested(&parent->power.lock, SINGLE_DEPTH_NESTING);
> >  
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/5] PM: runtime: Propagate last_busy from dependent to dependency
  2026-11-04  0:32     ` Brian Norris
@ 2026-11-04  1:40       ` Jonathan Cameron
  0 siblings, 0 replies; 14+ messages in thread
From: Jonathan Cameron @ 2026-11-04  1:40 UTC (permalink / raw)
  To: Brian Norris
  Cc: Rafael J. Wysocki, Fabrice Gasnier, linux-pm, linux-arm-kernel,
	Ulf Hansson, linux-kernel, linux-iio, Andy Shevchenko,
	David Lechner, Alexandre Torgue, Nuno Sá, Maxime Coquelin

On Tue, 3 Nov 2026 16:32:44 -0800
Brian Norris <briannorris@chromium.org> wrote:

> Hi Jonathan,
> 
> On Tue, Nov 03, 2026 at 11:17:19PM +0000, Jonathan Cameron wrote:
> > On Tue,  3 Nov 2026 14:14:41 -0800
> > Brian Norris <briannorris@chromium.org> wrote:
> >   
> > > When a child device suspends, it does not update the last_busy timestamp
> > > for its parent. If that parent configured autosuspend and didn't
> > > otherwise maintain its last_busy timestamp, it may now be immediately
> > > eligible to suspend. This is probably not expected -- the parent should
> > > wait for its autosuspend delay before suspending.  
> > 
> > So reading this again - I wonder if this text is describing the
> > expected behaviour.  
> 
> Agreed, this might give the wrong impression, depending on the
> assumptions the reader makes about autosuspend delay. (I think I had a
> particular problem scenario in mind, but didn't translate that properly
> here.)
> 
> > If the child is also using autosupend then, in extreme case, that time
> > might be longer than the autosuspend time of the parent.
> > If so it seems odd the parent still waits.  
> 
> Right, that seems wrong, and indeed that's not what this patch does.
> 
> > So most expected to me
> > would be if the parent were to suspend it's autosuspend time after
> > the parent or child was last used for something that indicate a
> > desire not to suspend (that shouldn't include actions the child
> > did whilst suspending).  
> 
> (Not sure I'm parsing this sentence quite right, but...)
> 
> The point is that last_busy should propagate (rather than, say, the
> expiration time). So each device still gets its own-delay-based
> autosuspend timer expiration. That may mean the parent waits longer than
> the child, but it may not, if the child's delay was longer than the
> parent's.
> 
> > Maybe there isn't a sensible way to deal with this and what you have here
> > is the best we can do.  Note that this is just commenting on this
> > statement in the commit description.  From what follows I think
> > this is just a commit description thing rather than functional.  
> 
> If I'm understanding what you're saying correctly: maybe it would be an
> improvement here to say:
> 
>   If that parent configured an autosuspend delay longer than its child's
>   and didn't otherwise maintain its last_busy timestamp, it may now be
>   immediately eligible to suspend.
> 
> I think that would more accurately describe the target problem, even if
> it doesn't cover all cases (such as when the parent delay is shorter).

Agreed. That covers the main case nicely.

> 
> > > The effect of this behavior is that a parent device may suspend sooner
> > > than its autosuspend delay, simply because its usage was accounted by
> > > its children, and not by direct references to the parent device.  
> > 
> > Sooner is indeed bad.
> >   
> > > 
> > > This was noticed in several cases, and some have implemented
> > > workarounds, such as in commit c537d3457542 ("iio: adc: stm32-adc: fix
> > > runtime autosuspend delay when slow polling"). At the same time, Ulf
> > > suggested these problems "should be solved in the runtime PM core" [1].
> > > 
> > > Instead of working around the problem in drivers, we propagate last_busy
> > > timestamps from a dependent device to its dependencies any time it may
> > > allow a dependency to suspend -- i.e., when releasing a refcount for its
> > > parent or suppliers. We take care to only propagate the timestamp if it
> > > is larger than the existing busy timestamp.  
> > 
> > So this sounds fine but I don't think aligns with that first paragraph.
> >   
> > > 
> > > Note that this works best if the dependent device is using autosuspend
> > > (and therefore updates its last_busy timestamps appropriately), but even
> > > for a non-autosuspend child, this is still somewhat useful --
> > > non-autosuspend devices still automatically update their last_busy every
> > > time they resume.  
> > 
> > Can we set the last busy on the suspend of child devices that aren't
> > suspending as a result of autosuspend timeout but for other reasons?
> > (may be some fiddly corners!)  
> 
> Do you have concrete use cases in mind, or do you bring this up simply
> because of my note?

Just your note ;)
> 
> For one, I feel like for devices with power.use_autosuspend==true, it's
> reasonable to expect drivers to use the appropriate helpers to update
> last_busy at the appropriate times, and so any "corner cases" are from
> misuse.
> 
> But if you mean non-use_autosuspend devices: I suppose it'd be possible
> but I haven't reasoned through the implementation very far yet. It
> almost seems like we'd need to plumb pm_runtime_mark_last_busy() into
> *all* the put() helpers, including the non-autosuspend ones
> (pm_runtime_put(), pm_runtime_put_sync()). I'm not quite sure if that's
> desirable. But I could try to look a bit more, if you think there are
> important use cases that would need this.
If it is hard not a problem to leave it as is and call out the limitation
as you have done.  Someone who needs it can figure it out!


Jonathan

> 
> Brian
> 
> > > Link: https://lore.kernel.org/all/CAPDyKFp=KTf8=zGBSzPYqhjnZpY8xwvjCeM1e-WTKT1QLSxaDA@mail.gmail.com/ [1]
> > > Signed-off-by: Brian Norris <briannorris@chromium.org>
> > > ---
> > > Cc: Ulf Hansson <ulfh@kernel.org>
> > > 
> > > Changes in v2:
> > >  * Update comments to explain when/why rpm_propagate_last_busy()
> > >    performs the update
> > > 
> > >  drivers/base/power/runtime.c | 23 ++++++++++++++++++++++-
> > >  1 file changed, 22 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> > > index 2a117d6a776f..dd306cbabcd3 100644
> > > --- a/drivers/base/power/runtime.c
> > > +++ b/drivers/base/power/runtime.c
> > > @@ -318,6 +318,22 @@ static int rpm_get_suppliers(struct device *dev)
> > >  	return 0;
> > >  }
> > >  
> > > +/*
> > > + * Propagate last_busy timestamp from one device to another, typically when a
> > > + * device (@dev) releases a reference count on a dependency (@target) and might
> > > + * allow it to (auto)suspend. This can, for example, prevent overactive suspend
> > > + * when a dependency's usage is primarily driven by one of its dependents.
> > > + */
> > > +static void rpm_propagate_last_busy(struct device *dev, struct device *target)
> > > +{
> > > +	s64 busy = atomic64_read(&dev->power.last_busy);
> > > +	s64 target_busy = atomic64_read(&target->power.last_busy);
> > > +
> > > +	while (target_busy < busy)
> > > +		if (atomic64_try_cmpxchg(&target->power.last_busy, &target_busy, busy))
> > > +			return;
> > > +}
> > > +
> > >  /**
> > >   * pm_runtime_release_supplier - Drop references to device link's supplier.
> > >   * @link: Target device link.
> > > @@ -348,6 +364,7 @@ static void __rpm_put_suppliers(struct device *dev, bool try_to_suspend)
> > >  		if (!device_link_test(link, DL_FLAG_PM_RUNTIME))
> > >  			continue;
> > >  
> > > +		rpm_propagate_last_busy(dev, link->supplier);
> > >  		pm_runtime_release_supplier(link);
> > >  		if (try_to_suspend)
> > >  			pm_request_idle(link->supplier);
> > > @@ -710,6 +727,8 @@ static int rpm_suspend(struct device *dev, int rpmflags)
> > >  
> > >  	if (dev->parent) {
> > >  		parent = dev->parent;
> > > +		if (!parent->power.ignore_children)
> > > +			rpm_propagate_last_busy(dev, parent);
> > >  		atomic_add_unless(&parent->power.child_count, -1, 0);
> > >  	}
> > >  	wake_up_all(&dev->power.wait_queue);
> > > @@ -1368,8 +1387,10 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status)
> > >  		goto out_set;
> > >  
> > >  	if (status == RPM_SUSPENDED) {
> > > -		atomic_add_unless(&parent->power.child_count, -1, 0);
> > >  		notify_parent = !parent->power.ignore_children;
> > > +		if (notify_parent)
> > > +			rpm_propagate_last_busy(dev, parent);
> > > +		atomic_add_unless(&parent->power.child_count, -1, 0);
> > >  	} else {
> > >  		spin_lock_nested(&parent->power.lock, SINGLE_DEPTH_NESTING);
> > >    
> >   
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/5] PM: runtime: Convert last_busy to atomic64_t
  2026-11-03 22:14 ` [PATCH v2 2/5] PM: runtime: Convert last_busy to atomic64_t Brian Norris
@ 2026-11-04  8:21   ` Joshua Crofts
  0 siblings, 0 replies; 14+ messages in thread
From: Joshua Crofts @ 2026-11-04  8:21 UTC (permalink / raw)
  To: Brian Norris
  Cc: Rafael J. Wysocki, Fabrice Gasnier, linux-pm, linux-arm-kernel,
	Ulf Hansson, linux-kernel, linux-iio, Andy Shevchenko,
	Jonathan Cameron, David Lechner, Alexandre Torgue, Nuno Sá,
	Maxime Coquelin

On Tue,  3 Nov 2026 14:14:40 -0800
Brian Norris <briannorris@chromium.org> wrote:

> I want to use the atomic64 compare/exchange helpers. There are no
> similar 32-bit-portable helpers for non-atomic64_t.

IMO starting a commit message with "I want to use" isn't
exactly great...

-- 
Kind regards,
Joshua Crofts

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/5] PM: runtime: Propagate last_busy from dependent to dependency
  2026-11-03 22:14 ` [PATCH v2 3/5] PM: runtime: Propagate last_busy from dependent to dependency Brian Norris
  2026-11-03 23:17   ` Jonathan Cameron
@ 2026-11-04 13:34   ` Rafael J. Wysocki (Intel)
  2026-11-04 19:31     ` Brian Norris
  1 sibling, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki (Intel) @ 2026-11-04 13:34 UTC (permalink / raw)
  To: Brian Norris
  Cc: Rafael J. Wysocki, Fabrice Gasnier, linux-pm, linux-arm-kernel,
	Ulf Hansson, linux-kernel, linux-iio, Andy Shevchenko,
	Jonathan Cameron, David Lechner, Alexandre Torgue, Nuno Sá,
	Maxime Coquelin

I should have responded to the v1 of this, but didn't manage to get to
it.  Sorry about that.

On Tue, Nov 3, 2026 at 11:23 PM Brian Norris <briannorris@chromium.org> wrote:
>
> When a child device suspends, it does not update the last_busy timestamp
> for its parent. If that parent configured autosuspend and didn't
> otherwise maintain its last_busy timestamp, it may now be immediately
> eligible to suspend. This is probably not expected -- the parent should
> wait for its autosuspend delay before suspending.

Let's first talk about what autosuspend is for: My understanding is
that it is there to avoid incurring the runtime PM suspend and resume
latency (and cost in terms of energy) in the cases when the device is
known or at least is expected to be needed in the near future.  IOW,
we want to defer the runtime suspend of the device because if it
suspended, it would be resumed shortly thereafter.

I'm not sure if that is always the case when a dependent device
suspends and its reference to the parent/supplier is dropped.

The core calls pm_runtime_mark_last_busy() for resuming devices under
the assumption that they are resumed for a reason and so it is likely
that they will be needed in near future, but I'm not sure if that
assumption can be extended to a child suspend case.  One might argue
though that extending it to the child/consumer resume case would make
sense.

> The effect of this behavior is that a parent device may suspend sooner
> than its autosuspend delay, simply because its usage was accounted by
> its children, and not by direct references to the parent device.

The distinction between "direct references to the parent device" and
"usage accounted for by its children" is not clear to me TBH.

There are references to the parent device in both cases and it is not
like drivers can always expect all runtime PM references to come from
specific code paths updating last_busy.  There is at least one
reference controlled by user space (the "control" attribute in sysfs)
and dropping it may also cause the device to suspend immediately if
last_busy has not been updated recently.  How is this situation
different from the one related to child/consumer suspend?

> This was noticed in several cases, and some have implemented
> workarounds, such as in commit c537d3457542 ("iio: adc: stm32-adc: fix
> runtime autosuspend delay when slow polling"). At the same time, Ulf
> suggested these problems "should be solved in the runtime PM core" [1].

So I need to be explained how commit 9bdbb1139ca1 ("iio: adc:
stm32-adc: add power management support") works because ATM I don't
seem to understand it.  It looks to me like the .runtime_idle()
callback updates last_busy for the device for which it is called, not
for its parent.

> Instead of working around the problem in drivers, we propagate last_busy
> timestamps from a dependent device to its dependencies any time it may
> allow a dependency to suspend -- i.e., when releasing a refcount for its
> parent or suppliers. We take care to only propagate the timestamp if it
> is larger than the existing busy timestamp.

I'm not sure why propagating last_busy timestamps when dropping the
reference to the parent/supplier is a good idea.

Why is the child's last_busy timestamp suitable for the parent in the
first place?

Why isn't done when dropping the reference?

> Note that this works best if the dependent device is using autosuspend
> (and therefore updates its last_busy timestamps appropriately), but even
> for a non-autosuspend child, this is still somewhat useful --
> non-autosuspend devices still automatically update their last_busy every
> time they resume.
>
> Link: https://lore.kernel.org/all/CAPDyKFp=KTf8=zGBSzPYqhjnZpY8xwvjCeM1e-WTKT1QLSxaDA@mail.gmail.com/ [1]
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> Cc: Ulf Hansson <ulfh@kernel.org>
>
> Changes in v2:
>  * Update comments to explain when/why rpm_propagate_last_busy()
>    performs the update
>
>  drivers/base/power/runtime.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> index 2a117d6a776f..dd306cbabcd3 100644
> --- a/drivers/base/power/runtime.c
> +++ b/drivers/base/power/runtime.c
> @@ -318,6 +318,22 @@ static int rpm_get_suppliers(struct device *dev)
>         return 0;
>  }
>
> +/*
> + * Propagate last_busy timestamp from one device to another, typically when a
> + * device (@dev) releases a reference count on a dependency (@target) and might
> + * allow it to (auto)suspend. This can, for example, prevent overactive suspend
> + * when a dependency's usage is primarily driven by one of its dependents.

AFAIAC, you need to say much more than this to explain all of it.

> + */
> +static void rpm_propagate_last_busy(struct device *dev, struct device *target)
> +{
> +       s64 busy = atomic64_read(&dev->power.last_busy);
> +       s64 target_busy = atomic64_read(&target->power.last_busy);
> +
> +       while (target_busy < busy)
> +               if (atomic64_try_cmpxchg(&target->power.last_busy, &target_busy, busy))
> +                       return;
> +}

And I'd rather not do anything like the above unless absolutely necessary.

> +
>  /**
>   * pm_runtime_release_supplier - Drop references to device link's supplier.
>   * @link: Target device link.
> @@ -348,6 +364,7 @@ static void __rpm_put_suppliers(struct device *dev, bool try_to_suspend)
>                 if (!device_link_test(link, DL_FLAG_PM_RUNTIME))
>                         continue;
>
> +               rpm_propagate_last_busy(dev, link->supplier);
>                 pm_runtime_release_supplier(link);
>                 if (try_to_suspend)
>                         pm_request_idle(link->supplier);
> @@ -710,6 +727,8 @@ static int rpm_suspend(struct device *dev, int rpmflags)
>
>         if (dev->parent) {
>                 parent = dev->parent;
> +               if (!parent->power.ignore_children)
> +                       rpm_propagate_last_busy(dev, parent);
>                 atomic_add_unless(&parent->power.child_count, -1, 0);
>         }
>         wake_up_all(&dev->power.wait_queue);
> @@ -1368,8 +1387,10 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status)
>                 goto out_set;
>
>         if (status == RPM_SUSPENDED) {
> -               atomic_add_unless(&parent->power.child_count, -1, 0);
>                 notify_parent = !parent->power.ignore_children;
> +               if (notify_parent)
> +                       rpm_propagate_last_busy(dev, parent);
> +               atomic_add_unless(&parent->power.child_count, -1, 0);
>         } else {
>                 spin_lock_nested(&parent->power.lock, SINGLE_DEPTH_NESTING);
>
> --

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/5] PM: runtime: Only queue an idle check for RPM-linked suppliers (part 2)
  2026-11-03 22:14 ` [PATCH v2 1/5] PM: runtime: Only queue an idle check for RPM-linked suppliers (part 2) Brian Norris
@ 2026-11-04 14:44   ` Rafael J. Wysocki (Intel)
  0 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki (Intel) @ 2026-11-04 14:44 UTC (permalink / raw)
  To: Brian Norris
  Cc: Rafael J. Wysocki, Fabrice Gasnier, linux-pm, linux-arm-kernel,
	Ulf Hansson, linux-kernel, linux-iio, Andy Shevchenko,
	Jonathan Cameron, David Lechner, Alexandre Torgue, Nuno Sá,
	Maxime Coquelin

On Tue, Nov 3, 2026 at 11:23 PM Brian Norris <briannorris@chromium.org> wrote:
>
> Commit be5755f023e7 ("PM: runtime: Only queue an idle check for
> RPM-linked suppliers") was incomplete. There are two other cases where
> we perform idle checks on non-DL_FLAG_PM_RUNTIME suppliers -- in
> pm_runtime_set_suspended(), and in cases where we failed to resume
> suppliers.
>
> Fixes: 5244f5e2d801 ("PM: runtime: Defer suspending suppliers")
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> Noticed while touching this function in subsequent work.
>
> (no changes since v1)
>
>  drivers/base/power/runtime.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> index 304a2b123420..9d3508d9b4a2 100644
> --- a/drivers/base/power/runtime.c
> +++ b/drivers/base/power/runtime.c
> @@ -343,6 +343,9 @@ static void __rpm_put_suppliers(struct device *dev, bool try_to_suspend)
>
>         list_for_each_entry_rcu(link, &dev->links.suppliers, c_node,
>                                 device_links_read_lock_held()) {
> +               if (!device_link_test(link, DL_FLAG_PM_RUNTIME))
> +                       continue;
> +
>                 pm_runtime_release_supplier(link);
>                 if (try_to_suspend)
>                         pm_request_idle(link->supplier);
> --

Applied as 7.4-rc material, thanks!

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/5] PM: runtime: Propagate last_busy from dependent to dependency
  2026-11-04 13:34   ` Rafael J. Wysocki (Intel)
@ 2026-11-04 19:31     ` Brian Norris
  0 siblings, 0 replies; 14+ messages in thread
From: Brian Norris @ 2026-11-04 19:31 UTC (permalink / raw)
  To: Rafael J. Wysocki (Intel)
  Cc: Fabrice Gasnier, linux-pm, linux-arm-kernel, Ulf Hansson,
	linux-kernel, linux-iio, Andy Shevchenko, Jonathan Cameron,
	David Lechner, Alexandre Torgue, Nuno Sá, Maxime Coquelin

On Wed, Nov 04, 2026 at 02:34:22PM +0100, Rafael J. Wysocki (Intel) wrote:
> I should have responded to the v1 of this, but didn't manage to get to
> it.  Sorry about that.

Hehe, well that's what v2 is for :)

Thanks for looking.

> On Tue, Nov 3, 2026 at 11:23 PM Brian Norris <briannorris@chromium.org> wrote:
> >
> > When a child device suspends, it does not update the last_busy timestamp
> > for its parent. If that parent configured autosuspend and didn't
> > otherwise maintain its last_busy timestamp, it may now be immediately
> > eligible to suspend. This is probably not expected -- the parent should
> > wait for its autosuspend delay before suspending.
> 
> Let's first talk about what autosuspend is for: My understanding is
> that it is there to avoid incurring the runtime PM suspend and resume
> latency (and cost in terms of energy) in the cases when the device is
> known or at least is expected to be needed in the near future.  IOW,
> we want to defer the runtime suspend of the device because if it
> suspended, it would be resumed shortly thereafter.
> 
> I'm not sure if that is always the case when a dependent device
> suspends and its reference to the parent/supplier is dropped.

To be clear: dropping a reference is not the point at which we extend
the timer -- it's simply the point at which we propagate the last_busy
timestamp.

The naive algorithm is:
Every time a device updates its last_busy, sync that timestamp to all
its dependencies.

My proposal modifies that naive algorithm so that it avoids additional
graph walks -- it defers that "sync" step until later. (Now that I'm
spelling this out, I feel like I should have included this in the commit
message...)

With that said:

> The core calls pm_runtime_mark_last_busy() for resuming devices under
> the assumption that they are resumed for a reason and so it is likely
> that they will be needed in near future, but I'm not sure if that
> assumption can be extended to a child suspend case.

But that's not what I'm doing: I'm not calling mark_last_busy() -- I'm
just making mark_last_busy() transitive to all dependencies. So any time
we *already* say the child is busy, I also extend that to the parent --
I'm not generating any new timestamps; I only reuse the timestamps we
already selected.

My view on the logic (using your terms above): when a child calls
mark_last_busy(), it suggests that child might be needed in the near
future. Whenever a child device is needed, so is its parent (barring
ignore_children). Thus, mark_last_busy() should also refresh the
parent's busy timestamp.

Importantly (in the same topic Jonathan was already asking about): this
does *not* mean the parent gets a whole new autosuspend delay period --
but it *does* mean that if the parent has a longer delay period
(presumably because it has a more expensive/slow suspend/resume latency)
it waits a bit longer (in case the child wakes back up, for one) to
autosuspend.

In concrete terms of commit c537d3457542 ("iio: adc: stm32-adc: fix
runtime autosuspend delay when slow polling"):

 * child device has 100ms delay
 * parent devices has 2s delay

SEQUENCE:
 1) child is active for longer than 2 seconds
 2) [timestamp N] child finishes I/O, drops reference with put_autosuspend()
 3) [N + 100ms] child suspends

CURRENT BEHVIOR:
 4) [N + 100ms] parent suspends immediately after #3

PROPOSAL:
 4) [N + 2s] parent suspends (if there was no other last_busy update)

The only last_busy timestamp is N; we just use it in more places,
because the parent was, in fact, busy at timestamp N.

> One might argue
> though that extending it to the child/consumer resume case would make
> sense.
> 
> > The effect of this behavior is that a parent device may suspend sooner
> > than its autosuspend delay, simply because its usage was accounted by
> > its children, and not by direct references to the parent device.
> 
> The distinction between "direct references to the parent device" and
> "usage accounted for by its children" is not clear to me TBH.

"Direct reference" is to mean "pm_runtime_put_autosuspend()" or anything
similar that updates last_busy. We expect that when the device in
question is the target object for I/O or whatever is keeping the device
busy.

But if the I/O is performed by a child, then it's no longer "direct
reference" -- it's "usage accounted by its children".

> There are references to the parent device in both cases and it is not
> like drivers can always expect all runtime PM references to come from
> specific code paths updating last_busy.  There is at least one
> reference controlled by user space (the "control" attribute in sysfs)
> and dropping it may also cause the device to suspend immediately if
> last_busy has not been updated recently.  How is this situation
> different from the one related to child/consumer suspend?

I think I've improperly directed you to focus on the "reference" part,
which IMO is a bit of a distraction. The point is: when was the device
"busy" (or "last used")? The 'control' attribute does not represent
"last used"; it just represents some unrelated policy to prevent
suspension.

> > This was noticed in several cases, and some have implemented
> > workarounds, such as in commit c537d3457542 ("iio: adc: stm32-adc: fix
> > runtime autosuspend delay when slow polling"). At the same time, Ulf
> > suggested these problems "should be solved in the runtime PM core" [1].
> 
> So I need to be explained how commit 9bdbb1139ca1 ("iio: adc:
> stm32-adc: add power management support") works because ATM I don't
> seem to understand it.  It looks to me like the .runtime_idle()
> callback updates last_busy for the device for which it is called, not
> for its parent.

Note: stm32-adc-core (drivers/iio/adc/stm32-adc-core.c) is the parent.
stm32-adc (drivers/iio/adc/stm32-adc.c) is the child.

When the child suspends, the parent checks .runtime_idle(). This updates
last_busy but still falls through to autosuspend -- that means we get a
slightly suboptimal sequence like the following:

 1) child is active for longer than 2 seconds
 2) [timestamp N] child finishes I/O, drops reference with put_autosuspend()
 3) [N + 100ms] child suspends
 4) parent runtime_idle() updates last_busy to [P = approx N + 100ms]
 5) [P + 2s] parent suspends

Step 5 happens approximately 100ms later than it really should, but it's
good enough.

NB: I think this is also why greybus (gb_bundle_idle(),
gb_interface_runtime_idle(), etc.) and USB (usb_runtime_idle(),
dwc3_runtime_idle()) look the way they do, with a built-in
mark_last_busy() in runtime_idle().

I also think that some other areas could use some logic like this. For
example, drivers/pci/pcie/portdrv.c has a 100ms autosuspend delay so
that lspci doesn't cause repeated suspend/resume. But suppose config
access for a child of a port are slow ... then the port still might
suspend immediately, between config accesses. (Never mind that
pci_config_pm_runtime_put() doesn't update last_busy...)

> > Instead of working around the problem in drivers, we propagate last_busy
> > timestamps from a dependent device to its dependencies any time it may
> > allow a dependency to suspend -- i.e., when releasing a refcount for its
> > parent or suppliers. We take care to only propagate the timestamp if it
> > is larger than the existing busy timestamp.
> 
> I'm not sure why propagating last_busy timestamps when dropping the
> reference to the parent/supplier is a good idea.
> 
> Why is the child's last_busy timestamp suitable for the parent in the
> first place?

I think that's an area I could improve in the commit message. But see my
comments above ("My view on the logic") which I hope answers your
question.

> Why isn't done when dropping the reference?
> 
> > Note that this works best if the dependent device is using autosuspend
> > (and therefore updates its last_busy timestamps appropriately), but even
> > for a non-autosuspend child, this is still somewhat useful --
> > non-autosuspend devices still automatically update their last_busy every
> > time they resume.
> >
> > Link: https://lore.kernel.org/all/CAPDyKFp=KTf8=zGBSzPYqhjnZpY8xwvjCeM1e-WTKT1QLSxaDA@mail.gmail.com/ [1]
> > Signed-off-by: Brian Norris <briannorris@chromium.org>
> > ---
> > Cc: Ulf Hansson <ulfh@kernel.org>
> >
> > Changes in v2:
> >  * Update comments to explain when/why rpm_propagate_last_busy()
> >    performs the update
> >
> >  drivers/base/power/runtime.c | 23 ++++++++++++++++++++++-
> >  1 file changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> > index 2a117d6a776f..dd306cbabcd3 100644
> > --- a/drivers/base/power/runtime.c
> > +++ b/drivers/base/power/runtime.c
> > @@ -318,6 +318,22 @@ static int rpm_get_suppliers(struct device *dev)
> >         return 0;
> >  }
> >
> > +/*
> > + * Propagate last_busy timestamp from one device to another, typically when a
> > + * device (@dev) releases a reference count on a dependency (@target) and might
> > + * allow it to (auto)suspend. This can, for example, prevent overactive suspend
> > + * when a dependency's usage is primarily driven by one of its dependents.
> 
> AFAIAC, you need to say much more than this to explain all of it.

OK. If we agree on the what and why, I'll take another pass at this. For
one, I imagine describing the "naive algorithm" I mention above would be
a good start. And maybe the principles below.

> > + */
> > +static void rpm_propagate_last_busy(struct device *dev, struct device *target)
> > +{
> > +       s64 busy = atomic64_read(&dev->power.last_busy);
> > +       s64 target_busy = atomic64_read(&target->power.last_busy);
> > +
> > +       while (target_busy < busy)
> > +               if (atomic64_try_cmpxchg(&target->power.last_busy, &target_busy, busy))
> > +                       return;
> > +}
> 
> And I'd rather not do anything like the above unless absolutely necessary.

Which part do you not like? The loop? The use of compare/exchange?

The principles involved are:

1) copy the timestamp
2) avoid going backward in time (e.g., if target already updated to a
   newer stamp)
3) avoid TOCTOU races between reading the target stamp and replacing it
   (e.g., if multiple children are doing this at the same time)

I suppose I could compromise on #3 by only doing a single
compare/exchange. This would remove the looping (which is admittedly a
little hard to reason about), at the expense of getting a suboptimal
update in the (likely rare) case of concurrent updates.

Brian

> > +
> >  /**
> >   * pm_runtime_release_supplier - Drop references to device link's supplier.
> >   * @link: Target device link.
> > @@ -348,6 +364,7 @@ static void __rpm_put_suppliers(struct device *dev, bool try_to_suspend)
> >                 if (!device_link_test(link, DL_FLAG_PM_RUNTIME))
> >                         continue;
> >
> > +               rpm_propagate_last_busy(dev, link->supplier);
> >                 pm_runtime_release_supplier(link);
> >                 if (try_to_suspend)
> >                         pm_request_idle(link->supplier);
> > @@ -710,6 +727,8 @@ static int rpm_suspend(struct device *dev, int rpmflags)
> >
> >         if (dev->parent) {
> >                 parent = dev->parent;
> > +               if (!parent->power.ignore_children)
> > +                       rpm_propagate_last_busy(dev, parent);
> >                 atomic_add_unless(&parent->power.child_count, -1, 0);
> >         }
> >         wake_up_all(&dev->power.wait_queue);
> > @@ -1368,8 +1387,10 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status)
> >                 goto out_set;
> >
> >         if (status == RPM_SUSPENDED) {
> > -               atomic_add_unless(&parent->power.child_count, -1, 0);
> >                 notify_parent = !parent->power.ignore_children;
> > +               if (notify_parent)
> > +                       rpm_propagate_last_busy(dev, parent);
> > +               atomic_add_unless(&parent->power.child_count, -1, 0);
> >         } else {
> >                 spin_lock_nested(&parent->power.lock, SINGLE_DEPTH_NESTING);
> >
> > --

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2026-11-04 19:31 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-03 22:14 [PATCH v2 0/5] PM: runtime: Improve autosuspend child to parent propagation Brian Norris
2026-11-03 22:14 ` [PATCH v2 1/5] PM: runtime: Only queue an idle check for RPM-linked suppliers (part 2) Brian Norris
2026-11-04 14:44   ` Rafael J. Wysocki (Intel)
2026-11-03 22:14 ` [PATCH v2 2/5] PM: runtime: Convert last_busy to atomic64_t Brian Norris
2026-11-04  8:21   ` Joshua Crofts
2026-11-03 22:14 ` [PATCH v2 3/5] PM: runtime: Propagate last_busy from dependent to dependency Brian Norris
2026-11-03 23:17   ` Jonathan Cameron
2026-11-04  0:32     ` Brian Norris
2026-11-04  1:40       ` Jonathan Cameron
2026-11-04 13:34   ` Rafael J. Wysocki (Intel)
2026-11-04 19:31     ` Brian Norris
2026-11-03 22:14 ` [PATCH v2 4/5] PM: runtime: Add tests for last_busy propagation Brian Norris
2026-11-03 23:21   ` Jonathan Cameron
2026-11-03 22:14 ` [PATCH v2 5/5] iio: adc: stm32-adc: Drop runtime_idle() Brian Norris

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox