[PATCH 1/3] PM: runtime: Add basic kunit tests for API contracts

[PATCH 1/3] PM: runtime: Add basic kunit tests for API contracts

[Brian Norris]

In exploring the various return codes and failure modes of runtime PM
APIs, I found it helpful to verify and codify many of them in unit
tests, especially given that even the kerneldoc can be rather complex to
reason through, and it also has had subtle errors of its own.

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

 drivers/base/Kconfig              |   6 +
 drivers/base/power/Makefile       |   1 +
 drivers/base/power/runtime-test.c | 259 ++++++++++++++++++++++++++++++
 3 files changed, 266 insertions(+)
 create mode 100644 drivers/base/power/runtime-test.c

diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 064eb52ff7e2..1786d87b29e2 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -167,6 +167,12 @@ config PM_QOS_KUNIT_TEST
 	depends on KUNIT=y
 	default KUNIT_ALL_TESTS
 
+config PM_RUNTIME_KUNIT_TEST
+	tristate "KUnit Tests for runtime PM" if !KUNIT_ALL_TESTS
+	depends on KUNIT
+	depends on PM
+	default KUNIT_ALL_TESTS
+
 config HMEM_REPORTING
 	bool
 	default n
diff --git a/drivers/base/power/Makefile b/drivers/base/power/Makefile
index 01f11629d241..2989e42d0161 100644
--- a/drivers/base/power/Makefile
+++ b/drivers/base/power/Makefile
@@ -4,5 +4,6 @@ obj-$(CONFIG_PM_SLEEP)	+= main.o wakeup.o wakeup_stats.o
 obj-$(CONFIG_PM_TRACE_RTC)	+= trace.o
 obj-$(CONFIG_HAVE_CLK)	+= clock_ops.o
 obj-$(CONFIG_PM_QOS_KUNIT_TEST) += qos-test.o
+obj-$(CONFIG_PM_RUNTIME_KUNIT_TEST) += runtime-test.o
 
 ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
diff --git a/drivers/base/power/runtime-test.c b/drivers/base/power/runtime-test.c
new file mode 100644
index 000000000000..263c28d5fc50
--- /dev/null
+++ b/drivers/base/power/runtime-test.c
@@ -0,0 +1,259 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2025 Google, Inc.
+ */
+
+#include <linux/cleanup.h>
+#include <linux/pm_runtime.h>
+#include <kunit/device.h>
+#include <kunit/test.h>
+
+#define DEVICE_NAME "pm_runtime_test_device"
+
+static void pm_runtime_depth_test(struct kunit *test)
+{
+	struct device *dev = kunit_device_register(test, DEVICE_NAME);
+
+	KUNIT_ASSERT_PTR_NE(test, NULL, dev);
+
+	pm_runtime_enable(dev);
+
+	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
+	KUNIT_EXPECT_EQ(test, 0, pm_runtime_get_sync(dev));
+	KUNIT_EXPECT_TRUE(test, pm_runtime_active(dev));
+	KUNIT_EXPECT_EQ(test, 1, pm_runtime_get_sync(dev)); /* "already active" */
+	KUNIT_EXPECT_EQ(test, 0, pm_runtime_put_sync(dev));
+	KUNIT_EXPECT_EQ(test, 0, pm_runtime_put_sync(dev));
+	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
+}
+
+/* Test pm_runtime_put() and friends when already suspended. */
+static void pm_runtime_already_suspended_test(struct kunit *test)
+{
+	struct device *dev = kunit_device_register(test, DEVICE_NAME);
+
+	KUNIT_ASSERT_PTR_NE(test, NULL, dev);
+
+	pm_runtime_enable(dev);
+
+	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
+	pm_runtime_get_noresume(dev);
+
+	/* Flush, in case the above (non-sync) triggered any work. */
+	KUNIT_EXPECT_EQ(test, 0, pm_runtime_barrier(dev)); /* no wakeup needed */
+
+	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
+	/*
+	 * We never actually left RPM_SUSPENDED, but rpm_idle() still treats
+	 * this as -EAGAIN / "runtime PM status change ongoing".
+	 */
+	KUNIT_EXPECT_EQ(test, -EAGAIN, pm_runtime_put(dev));
+
+	pm_runtime_get_noresume(dev);
+	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
+	KUNIT_EXPECT_EQ(test, -EAGAIN, pm_runtime_put_sync(dev));
+
+	KUNIT_EXPECT_EQ(test, 1, pm_runtime_suspend(dev));
+	KUNIT_EXPECT_EQ(test, 1, pm_runtime_autosuspend(dev));
+	KUNIT_EXPECT_EQ(test, 1, pm_request_autosuspend(dev));
+
+	pm_runtime_get_noresume(dev);
+	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
+	KUNIT_EXPECT_EQ(test, 1, pm_runtime_put_sync_autosuspend(dev));
+
+	pm_runtime_get_noresume(dev);
+	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
+	KUNIT_EXPECT_EQ(test, 1, pm_runtime_put_autosuspend(dev));
+
+	/* Grab 2 refcounts */
+	pm_runtime_get_noresume(dev);
+	pm_runtime_get_noresume(dev);
+	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
+	/* The first put() sees usage_count 1 */
+	KUNIT_EXPECT_EQ(test, 0, pm_runtime_put_autosuspend(dev));
+	/* The second put() sees usage_count 0 but tells us "already suspended". */
+	KUNIT_EXPECT_EQ(test, 1, pm_runtime_put_autosuspend(dev));
+
+	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
+}
+
+static void pm_runtime_idle_test(struct kunit *test)
+{
+	struct device *dev = kunit_device_register(test, DEVICE_NAME);
+
+	KUNIT_ASSERT_PTR_NE(test, NULL, dev);
+
+	pm_runtime_enable(dev);
+
+	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
+	KUNIT_EXPECT_EQ(test, 0, pm_runtime_get_sync(dev));
+	KUNIT_EXPECT_TRUE(test, pm_runtime_active(dev));
+	KUNIT_EXPECT_EQ(test, -EAGAIN, pm_runtime_idle(dev));
+	KUNIT_EXPECT_TRUE(test, pm_runtime_active(dev));
+	pm_runtime_put_noidle(dev);
+	KUNIT_EXPECT_TRUE(test, pm_runtime_active(dev));
+	KUNIT_EXPECT_EQ(test, 0, pm_runtime_idle(dev));
+	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
+	KUNIT_EXPECT_EQ(test, -EAGAIN, pm_runtime_idle(dev));
+	KUNIT_EXPECT_EQ(test, -EAGAIN, pm_request_idle(dev));
+}
+
+static void pm_runtime_disabled_test(struct kunit *test)
+{
+	struct device *dev = kunit_device_register(test, DEVICE_NAME);
+
+	KUNIT_ASSERT_PTR_NE(test, NULL, dev);
+
+	/* Never called pm_runtime_enable() */
+	KUNIT_EXPECT_FALSE(test, pm_runtime_enabled(dev));
+
+	/* "disabled" is treated as "active" */
+	KUNIT_EXPECT_TRUE(test, pm_runtime_active(dev));
+	KUNIT_EXPECT_FALSE(test, pm_runtime_suspended(dev));
+
+	/*
+	 * Note: these "fail", but they still acquire/release refcounts, so
+	 * keep them balanced.
+	 */
+	KUNIT_EXPECT_EQ(test, -EACCES, pm_runtime_get(dev));
+	KUNIT_EXPECT_EQ(test, -EACCES, pm_runtime_put(dev));
+
+	KUNIT_EXPECT_EQ(test, -EACCES, pm_runtime_get_sync(dev));
+	KUNIT_EXPECT_EQ(test, -EACCES, pm_runtime_put_sync(dev));
+
+	KUNIT_EXPECT_EQ(test, -EACCES, pm_runtime_get(dev));
+	KUNIT_EXPECT_EQ(test, -EACCES, pm_runtime_put_autosuspend(dev));
+
+	KUNIT_EXPECT_EQ(test, -EACCES, pm_runtime_resume_and_get(dev));
+	KUNIT_EXPECT_EQ(test, -EACCES, pm_runtime_idle(dev));
+	KUNIT_EXPECT_EQ(test, -EACCES, pm_request_idle(dev));
+	KUNIT_EXPECT_EQ(test, -EACCES, pm_request_resume(dev));
+	KUNIT_EXPECT_EQ(test, -EACCES, pm_request_autosuspend(dev));
+	KUNIT_EXPECT_EQ(test, -EACCES, pm_runtime_suspend(dev));
+	KUNIT_EXPECT_EQ(test, -EACCES, pm_runtime_resume(dev));
+	KUNIT_EXPECT_EQ(test, -EACCES, pm_runtime_autosuspend(dev));
+
+	/* Still active */
+	KUNIT_EXPECT_TRUE(test, pm_runtime_active(dev));
+}
+
+static void pm_runtime_error_test(struct kunit *test)
+{
+	struct device *dev = kunit_device_register(test, DEVICE_NAME);
+
+	KUNIT_ASSERT_PTR_NE(test, NULL, dev);
+
+	pm_runtime_enable(dev);
+	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
+
+	/* Fake a .runtime_resume() error */
+	dev->power.runtime_error = -EIO;
+
+	/*
+	 * Note: these "fail", but they still acquire/release refcounts, so
+	 * keep them balanced.
+	 */
+	KUNIT_EXPECT_EQ(test, -EINVAL, pm_runtime_get(dev));
+	KUNIT_EXPECT_EQ(test, -EINVAL, pm_runtime_put(dev));
+
+	KUNIT_EXPECT_EQ(test, -EINVAL, pm_runtime_get_sync(dev));
+	KUNIT_EXPECT_EQ(test, -EINVAL, pm_runtime_put_sync(dev));
+
+	KUNIT_EXPECT_EQ(test, -EINVAL, pm_runtime_get(dev));
+	KUNIT_EXPECT_EQ(test, -EINVAL, pm_runtime_put_autosuspend(dev));
+
+	KUNIT_EXPECT_EQ(test, -EINVAL, pm_runtime_resume_and_get(dev));
+	KUNIT_EXPECT_EQ(test, -EINVAL, pm_runtime_idle(dev));
+	KUNIT_EXPECT_EQ(test, -EINVAL, pm_request_idle(dev));
+	KUNIT_EXPECT_EQ(test, -EINVAL, pm_request_resume(dev));
+	KUNIT_EXPECT_EQ(test, -EINVAL, pm_request_autosuspend(dev));
+	KUNIT_EXPECT_EQ(test, -EINVAL, pm_runtime_suspend(dev));
+	KUNIT_EXPECT_EQ(test, -EINVAL, pm_runtime_resume(dev));
+	KUNIT_EXPECT_EQ(test, -EINVAL, pm_runtime_autosuspend(dev));
+
+	/* Still suspended */
+	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
+	/* Clear error */
+	KUNIT_EXPECT_EQ(test, 0, pm_runtime_set_suspended(dev));
+	KUNIT_EXPECT_EQ(test, 0, dev->power.runtime_error);
+	/* Still suspended */
+	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
+
+	KUNIT_EXPECT_EQ(test, 0, pm_runtime_get(dev));
+	KUNIT_EXPECT_EQ(test, 1, pm_runtime_barrier(dev)); /* resume was pending */
+	KUNIT_EXPECT_EQ(test, 0, pm_runtime_put(dev));
+	pm_runtime_suspend(dev); /* flush the put(), to suspend */
+	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
+
+	KUNIT_EXPECT_EQ(test, 0, pm_runtime_get_sync(dev));
+	KUNIT_EXPECT_EQ(test, 0, pm_runtime_put_sync(dev));
+
+	KUNIT_EXPECT_EQ(test, 0, pm_runtime_get_sync(dev));
+	KUNIT_EXPECT_EQ(test, 0, pm_runtime_put_autosuspend(dev));
+
+	KUNIT_EXPECT_EQ(test, 0, pm_runtime_resume_and_get(dev));
+
+	/*
+	 * The following should all "fail" with -EAGAIN (usage is non-zero) or
+	 * 1 (already resumed).
+	 */
+	KUNIT_EXPECT_EQ(test, -EAGAIN, pm_runtime_idle(dev));
+	KUNIT_EXPECT_EQ(test, -EAGAIN, pm_request_idle(dev));
+	KUNIT_EXPECT_EQ(test, 1, pm_request_resume(dev));
+	KUNIT_EXPECT_EQ(test, -EAGAIN, pm_request_autosuspend(dev));
+	KUNIT_EXPECT_EQ(test, -EAGAIN, pm_runtime_suspend(dev));
+	KUNIT_EXPECT_EQ(test, 1, pm_runtime_resume(dev));
+	KUNIT_EXPECT_EQ(test, -EAGAIN, pm_runtime_autosuspend(dev));
+
+	KUNIT_EXPECT_EQ(test, 0, pm_runtime_put_sync(dev));
+
+	/* Suspended again */
+	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
+}
+
+/*
+ * Explore a typical probe() sequence in which a device marks itself powered,
+ * but doesn't hold any runtime PM reference, so it suspends as soon as it goes
+ * idle.
+ */
+static void pm_runtime_probe_active_test(struct kunit *test)
+{
+	struct device *dev = kunit_device_register(test, DEVICE_NAME);
+
+	KUNIT_ASSERT_PTR_NE(test, NULL, dev);
+
+	KUNIT_EXPECT_TRUE(test, pm_runtime_status_suspended(dev));
+
+	KUNIT_EXPECT_EQ(test, 0, pm_runtime_set_active(dev));
+	KUNIT_EXPECT_TRUE(test, pm_runtime_active(dev));
+
+	pm_runtime_enable(dev);
+	KUNIT_EXPECT_TRUE(test, pm_runtime_active(dev));
+
+	/* Flush, and ensure we stayed active. */
+	KUNIT_EXPECT_EQ(test, 0, pm_runtime_barrier(dev));
+	KUNIT_EXPECT_TRUE(test, pm_runtime_active(dev));
+
+	/* Ask for idle? Now we suspend. */
+	KUNIT_EXPECT_EQ(test, 0, pm_runtime_idle(dev));
+	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
+}
+
+static struct kunit_case pm_runtime_test_cases[] = {
+	KUNIT_CASE(pm_runtime_depth_test),
+	KUNIT_CASE(pm_runtime_already_suspended_test),
+	KUNIT_CASE(pm_runtime_idle_test),
+	KUNIT_CASE(pm_runtime_disabled_test),
+	KUNIT_CASE(pm_runtime_error_test),
+	KUNIT_CASE(pm_runtime_probe_active_test),
+	{}
+};
+
+static struct kunit_suite pm_runtime_test_suite = {
+	.name = "pm_runtime_test_cases",
+	.test_cases = pm_runtime_test_cases,
+};
+
+kunit_test_suite(pm_runtime_test_suite);
+MODULE_DESCRIPTION("Runtime power management unit test suite");
+MODULE_LICENSE("GPL");
-- 
2.51.0.318.gd7df087d1a-goog

[PATCH 2/3] PM: runtime: Make put{,_sync}() return 1 when already suspended

[Brian Norris]

The pm_runtime.h docs say pm_runtime_put() and pm_runtime_put_sync()
return 1 when already suspended, but this is not true -- they return
-EAGAIN. On the other hand, pm_runtime_put_sync_suspend() and
pm_runtime_put_sync_autosuspend() *do* return 1.

This is an artifact of the fact that the former are built on rpm_idle(),
whereas the latter are built on rpm_suspend().

There are precious few pm_runtime_put()/pm_runtime_put_sync() callers
that check the return code at all, but most of them only log errors, and
usually only for negative error codes. None of them should be treating
this as an error, so:

 * at best, this may fix some case where a driver treats this condition
   as an error, when it shouldn't;

 * at worst, this should make no effect; and

 * somewhere in between, we could potentially clear up non-fatal log
   messages.

Fix the pm_runtime_already_suspended_test() while tweaking the behavior.
The test makes a lot more sense when these all return 1 when the device
is already suspended:

    pm_runtime_put(dev);
    pm_runtime_put_sync(dev);
    pm_runtime_suspend(dev);
    pm_runtime_autosuspend(dev);
    pm_request_autosuspend(dev);
    pm_runtime_put_sync_autosuspend(dev);
    pm_runtime_put_autosuspend(dev);

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

 drivers/base/power/runtime-test.c | 8 ++------
 drivers/base/power/runtime.c      | 3 +++
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/base/power/runtime-test.c b/drivers/base/power/runtime-test.c
index 263c28d5fc50..1be18e871f1d 100644
--- a/drivers/base/power/runtime-test.c
+++ b/drivers/base/power/runtime-test.c
@@ -43,15 +43,11 @@ static void pm_runtime_already_suspended_test(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, 0, pm_runtime_barrier(dev)); /* no wakeup needed */
 
 	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
-	/*
-	 * We never actually left RPM_SUSPENDED, but rpm_idle() still treats
-	 * this as -EAGAIN / "runtime PM status change ongoing".
-	 */
-	KUNIT_EXPECT_EQ(test, -EAGAIN, pm_runtime_put(dev));
+	KUNIT_EXPECT_EQ(test, 1, pm_runtime_put(dev));
 
 	pm_runtime_get_noresume(dev);
 	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
-	KUNIT_EXPECT_EQ(test, -EAGAIN, pm_runtime_put_sync(dev));
+	KUNIT_EXPECT_EQ(test, 1, pm_runtime_put_sync(dev));
 
 	KUNIT_EXPECT_EQ(test, 1, pm_runtime_suspend(dev));
 	KUNIT_EXPECT_EQ(test, 1, pm_runtime_autosuspend(dev));
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 3e84dc4122de..17cf111d16aa 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -498,6 +498,9 @@ static int rpm_idle(struct device *dev, int rpmflags)
 	if (retval < 0)
 		;	/* Conditions are wrong. */
 
+	else if ((rpmflags & RPM_GET_PUT) && (retval == 1))
+		;	/* put() is allowed in RPM_SUSPENDED */
+
 	/* Idle notifications are allowed only in the RPM_ACTIVE state. */
 	else if (dev->power.runtime_status != RPM_ACTIVE)
 		retval = -EAGAIN;
-- 
2.51.0.318.gd7df087d1a-goog

[PATCH 3/3] PM: runtime: Update kerneldoc return codes

[Brian Norris]

APIs based on __pm_runtime_idle() (pm_runtime_idle(), pm_request_idle())
do not return 1 when already suspended. They return -EAGAIN. This is
already covered in the docs, so the entry for "1" is redundant and
conflicting.

(pm_runtime_put() and pm_runtime_put_sync() were previously incorrect,
but that's fixed in "PM: runtime: pm_runtime_put{,_sync}() returns 1
when already suspended", to ensure consistency with APIs like
pm_runtime_put_autosuspend().)

RPM_GET_PUT APIs based on __pm_runtime_suspend() do return 1 when
already suspended, but the language is a little unclear -- it's not
really an "error", so it seems better to list as a clarification before
the 0/success case. Additionally, they only actually return 1 when the
refcount makes it to 0; if the usage_count is still non-zero, we return
0.

pm_runtime_put(), etc., also don't appear at first like they can ever
see "-EAGAIN: Runtime PM usage_count non-zero", because in non-racy
conditions, pm_runtime_put() would drop its reference count, see it's
non-zero, and return early (in __pm_runtime_idle()). However, it's
possible to race with another actor that increments the usage_count
afterward, since rpm_idle() is protected by a separate lock; in such a
case, we may see -EAGAIN.

Because this case is only seen in the presence of concurrent actors, it
makes sense to clarify that this is when "usage_count **became**
non-zero", by way of some racing actor.

Lastly, pm_runtime_put_sync_suspend() duplicated some -EAGAIN language.
Fix that.

Fixes: 271ff96d6066 ("PM: runtime: Document return values of suspend-related API functions")
Link: https://lore.kernel.org/linux-pm/aJ5pkEJuixTaybV4@google.com/
Signed-off-by: Brian Norris <briannorris@chromium.org>
---

 include/linux/pm_runtime.h | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
index d88d6b6ccf5b..fd17ffe1bc79 100644
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -356,7 +356,6 @@ static inline int pm_runtime_force_resume(struct device *dev) { return -ENXIO; }
  * * -EPERM: Device PM QoS resume latency 0.
  * * -EINPROGRESS: Suspend already in progress.
  * * -ENOSYS: CONFIG_PM not enabled.
- * * 1: Device already suspended.
  * Other values and conditions for the above values are possible as returned by
  * Runtime PM idle and suspend callbacks.
  */
@@ -439,7 +438,6 @@ static inline int pm_runtime_resume(struct device *dev)
  * * -EPERM: Device PM QoS resume latency 0.
  * * -EINPROGRESS: Suspend already in progress.
  * * -ENOSYS: CONFIG_PM not enabled.
- * * 1: Device already suspended.
  */
 static inline int pm_request_idle(struct device *dev)
 {
@@ -540,15 +538,16 @@ static inline int pm_runtime_resume_and_get(struct device *dev)
  * equal to 0, queue up a work item for @dev like in pm_request_idle().
  *
  * Return:
+ * * 1: Usage counts dropped to zero, but device was already suspended.
  * * 0: Success.
  * * -EINVAL: Runtime PM error.
  * * -EACCES: Runtime PM disabled.
- * * -EAGAIN: Runtime PM usage_count non-zero or Runtime PM status change ongoing.
+ * * -EAGAIN: Runtime PM usage_count became non-zero or Runtime PM status
+ *            change ongoing.
  * * -EBUSY: Runtime PM child_count non-zero.
  * * -EPERM: Device PM QoS resume latency 0.
  * * -EINPROGRESS: Suspend already in progress.
  * * -ENOSYS: CONFIG_PM not enabled.
- * * 1: Device already suspended.
  */
 static inline int pm_runtime_put(struct device *dev)
 {
@@ -565,15 +564,16 @@ DEFINE_FREE(pm_runtime_put, struct device *, if (_T) pm_runtime_put(_T))
  * equal to 0, queue up a work item for @dev like in pm_request_autosuspend().
  *
  * Return:
+ * * 1: Usage counts dropped to zero, but device was already suspended.
  * * 0: Success.
  * * -EINVAL: Runtime PM error.
  * * -EACCES: Runtime PM disabled.
- * * -EAGAIN: Runtime PM usage_count non-zero or Runtime PM status change ongoing.
+ * * -EAGAIN: Runtime PM usage_count became non-zero or Runtime PM status
+ *            change ongoing.
  * * -EBUSY: Runtime PM child_count non-zero.
  * * -EPERM: Device PM QoS resume latency 0.
  * * -EINPROGRESS: Suspend already in progress.
  * * -ENOSYS: CONFIG_PM not enabled.
- * * 1: Device already suspended.
  */
 static inline int __pm_runtime_put_autosuspend(struct device *dev)
 {
@@ -590,15 +590,16 @@ static inline int __pm_runtime_put_autosuspend(struct device *dev)
  * in pm_request_autosuspend().
  *
  * Return:
+ * * 1: Usage counts dropped to zero, but device was already suspended.
  * * 0: Success.
  * * -EINVAL: Runtime PM error.
  * * -EACCES: Runtime PM disabled.
- * * -EAGAIN: Runtime PM usage_count non-zero or Runtime PM status change ongoing.
+ * * -EAGAIN: Runtime PM usage_count became non-zero or Runtime PM status
+ *            change ongoing.
  * * -EBUSY: Runtime PM child_count non-zero.
  * * -EPERM: Device PM QoS resume latency 0.
  * * -EINPROGRESS: Suspend already in progress.
  * * -ENOSYS: CONFIG_PM not enabled.
- * * 1: Device already suspended.
  */
 static inline int pm_runtime_put_autosuspend(struct device *dev)
 {
@@ -619,14 +620,15 @@ static inline int pm_runtime_put_autosuspend(struct device *dev)
  * if it returns an error code.
  *
  * Return:
+ * * 1: Usage counts dropped to zero, but device was already suspended.
  * * 0: Success.
  * * -EINVAL: Runtime PM error.
  * * -EACCES: Runtime PM disabled.
- * * -EAGAIN: Runtime PM usage_count non-zero or Runtime PM status change ongoing.
+ * * -EAGAIN: Runtime PM usage_count became non-zero or Runtime PM status
+ *            change ongoing.
  * * -EBUSY: Runtime PM child_count non-zero.
  * * -EPERM: Device PM QoS resume latency 0.
  * * -ENOSYS: CONFIG_PM not enabled.
- * * 1: Device already suspended.
  * Other values and conditions for the above values are possible as returned by
  * Runtime PM suspend callbacks.
  */
@@ -646,15 +648,15 @@ static inline int pm_runtime_put_sync(struct device *dev)
  * if it returns an error code.
  *
  * Return:
+ * * 1: Usage counts dropped to zero, but device was already suspended.
  * * 0: Success.
  * * -EINVAL: Runtime PM error.
  * * -EACCES: Runtime PM disabled.
- * * -EAGAIN: Runtime PM usage_count non-zero or Runtime PM status change ongoing.
- * * -EAGAIN: usage_count non-zero or Runtime PM status change ongoing.
+ * * -EAGAIN: Runtime PM usage_count became non-zero or Runtime PM status
+ *            change ongoing.
  * * -EBUSY: Runtime PM child_count non-zero.
  * * -EPERM: Device PM QoS resume latency 0.
  * * -ENOSYS: CONFIG_PM not enabled.
- * * 1: Device already suspended.
  * Other values and conditions for the above values are possible as returned by
  * Runtime PM suspend callbacks.
  */
@@ -677,15 +679,16 @@ static inline int pm_runtime_put_sync_suspend(struct device *dev)
  * if it returns an error code.
  *
  * Return:
+ * * 1: Usage counts dropped to zero, but device was already suspended.
  * * 0: Success.
  * * -EINVAL: Runtime PM error.
  * * -EACCES: Runtime PM disabled.
- * * -EAGAIN: Runtime PM usage_count non-zero or Runtime PM status change ongoing.
+ * * -EAGAIN: Runtime PM usage_count became non-zero or Runtime PM status
+ *            change ongoing.
  * * -EBUSY: Runtime PM child_count non-zero.
  * * -EPERM: Device PM QoS resume latency 0.
  * * -EINPROGRESS: Suspend already in progress.
  * * -ENOSYS: CONFIG_PM not enabled.
- * * 1: Device already suspended.
  * Other values and conditions for the above values are possible as returned by
  * Runtime PM suspend callbacks.
  */
-- 
2.51.0.318.gd7df087d1a-goog

Re: [PATCH 3/3] PM: runtime: Update kerneldoc return codes

[Sakari Ailus]

Hi Brian,

Thanks for posting this. A few comments below.

On Thu, Aug 28, 2025 at 05:28:28PM -0700, Brian Norris wrote:
> APIs based on __pm_runtime_idle() (pm_runtime_idle(), pm_request_idle())
> do not return 1 when already suspended. They return -EAGAIN. This is
> already covered in the docs, so the entry for "1" is redundant and
> conflicting.
> 
> (pm_runtime_put() and pm_runtime_put_sync() were previously incorrect,
> but that's fixed in "PM: runtime: pm_runtime_put{,_sync}() returns 1
> when already suspended", to ensure consistency with APIs like
> pm_runtime_put_autosuspend().)
> 
> RPM_GET_PUT APIs based on __pm_runtime_suspend() do return 1 when
> already suspended, but the language is a little unclear -- it's not
> really an "error", so it seems better to list as a clarification before
> the 0/success case. Additionally, they only actually return 1 when the
> refcount makes it to 0; if the usage_count is still non-zero, we return
> 0.
> 
> pm_runtime_put(), etc., also don't appear at first like they can ever
> see "-EAGAIN: Runtime PM usage_count non-zero", because in non-racy
> conditions, pm_runtime_put() would drop its reference count, see it's
> non-zero, and return early (in __pm_runtime_idle()). However, it's
> possible to race with another actor that increments the usage_count
> afterward, since rpm_idle() is protected by a separate lock; in such a
> case, we may see -EAGAIN.
> 
> Because this case is only seen in the presence of concurrent actors, it
> makes sense to clarify that this is when "usage_count **became**
> non-zero", by way of some racing actor.
> 
> Lastly, pm_runtime_put_sync_suspend() duplicated some -EAGAIN language.
> Fix that.
> 
> Fixes: 271ff96d6066 ("PM: runtime: Document return values of suspend-related API functions")
> Link: https://lore.kernel.org/linux-pm/aJ5pkEJuixTaybV4@google.com/
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> 
>  include/linux/pm_runtime.h | 33 ++++++++++++++++++---------------
>  1 file changed, 18 insertions(+), 15 deletions(-)
> 
> diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
> index d88d6b6ccf5b..fd17ffe1bc79 100644
> --- a/include/linux/pm_runtime.h
> +++ b/include/linux/pm_runtime.h
> @@ -356,7 +356,6 @@ static inline int pm_runtime_force_resume(struct device *dev) { return -ENXIO; }
>   * * -EPERM: Device PM QoS resume latency 0.
>   * * -EINPROGRESS: Suspend already in progress.
>   * * -ENOSYS: CONFIG_PM not enabled.
> - * * 1: Device already suspended.
>   * Other values and conditions for the above values are possible as returned by
>   * Runtime PM idle and suspend callbacks.
>   */
> @@ -439,7 +438,6 @@ static inline int pm_runtime_resume(struct device *dev)
>   * * -EPERM: Device PM QoS resume latency 0.
>   * * -EINPROGRESS: Suspend already in progress.
>   * * -ENOSYS: CONFIG_PM not enabled.
> - * * 1: Device already suspended.
>   */
>  static inline int pm_request_idle(struct device *dev)
>  {
> @@ -540,15 +538,16 @@ static inline int pm_runtime_resume_and_get(struct device *dev)
>   * equal to 0, queue up a work item for @dev like in pm_request_idle().
>   *
>   * Return:
> + * * 1: Usage counts dropped to zero, but device was already suspended.

Does this actually happen? pm_runtime_put() calls __pm_runtime_idle() that
doesn't appear to return 1 in any case.

>   * * 0: Success.
>   * * -EINVAL: Runtime PM error.
>   * * -EACCES: Runtime PM disabled.
> - * * -EAGAIN: Runtime PM usage_count non-zero or Runtime PM status change ongoing.
> + * * -EAGAIN: Runtime PM usage_count became non-zero or Runtime PM status
> + *            change ongoing.
>   * * -EBUSY: Runtime PM child_count non-zero.
>   * * -EPERM: Device PM QoS resume latency 0.
>   * * -EINPROGRESS: Suspend already in progress.
>   * * -ENOSYS: CONFIG_PM not enabled.
> - * * 1: Device already suspended.
>   */
>  static inline int pm_runtime_put(struct device *dev)
>  {
> @@ -565,15 +564,16 @@ DEFINE_FREE(pm_runtime_put, struct device *, if (_T) pm_runtime_put(_T))
>   * equal to 0, queue up a work item for @dev like in pm_request_autosuspend().
>   *
>   * Return:
> + * * 1: Usage counts dropped to zero, but device was already suspended.
>   * * 0: Success.

"usage_count" and "usage counter" is being used in kernel-doc already, I'd
use either of the two. "usage_count" refers directly to the field in struct
dev_pm_info (and is used a few lines below). Same elsewhere.

>   * * -EINVAL: Runtime PM error.
>   * * -EACCES: Runtime PM disabled.
> - * * -EAGAIN: Runtime PM usage_count non-zero or Runtime PM status change ongoing.
> + * * -EAGAIN: Runtime PM usage_count became non-zero or Runtime PM status
> + *            change ongoing.
>   * * -EBUSY: Runtime PM child_count non-zero.
>   * * -EPERM: Device PM QoS resume latency 0.
>   * * -EINPROGRESS: Suspend already in progress.
>   * * -ENOSYS: CONFIG_PM not enabled.
> - * * 1: Device already suspended.
>   */
>  static inline int __pm_runtime_put_autosuspend(struct device *dev)
>  {
> @@ -590,15 +590,16 @@ static inline int __pm_runtime_put_autosuspend(struct device *dev)
>   * in pm_request_autosuspend().
>   *
>   * Return:
> + * * 1: Usage counts dropped to zero, but device was already suspended.
>   * * 0: Success.
>   * * -EINVAL: Runtime PM error.
>   * * -EACCES: Runtime PM disabled.
> - * * -EAGAIN: Runtime PM usage_count non-zero or Runtime PM status change ongoing.
> + * * -EAGAIN: Runtime PM usage_count became non-zero or Runtime PM status
> + *            change ongoing.
>   * * -EBUSY: Runtime PM child_count non-zero.
>   * * -EPERM: Device PM QoS resume latency 0.
>   * * -EINPROGRESS: Suspend already in progress.
>   * * -ENOSYS: CONFIG_PM not enabled.
> - * * 1: Device already suspended.
>   */
>  static inline int pm_runtime_put_autosuspend(struct device *dev)
>  {
> @@ -619,14 +620,15 @@ static inline int pm_runtime_put_autosuspend(struct device *dev)
>   * if it returns an error code.
>   *
>   * Return:
> + * * 1: Usage counts dropped to zero, but device was already suspended.
>   * * 0: Success.

Does this happen (pm_runtime_put_sync() calls __pm_runtime_idle())?

>   * * -EINVAL: Runtime PM error.
>   * * -EACCES: Runtime PM disabled.
> - * * -EAGAIN: Runtime PM usage_count non-zero or Runtime PM status change ongoing.
> + * * -EAGAIN: Runtime PM usage_count became non-zero or Runtime PM status
> + *            change ongoing.
>   * * -EBUSY: Runtime PM child_count non-zero.
>   * * -EPERM: Device PM QoS resume latency 0.
>   * * -ENOSYS: CONFIG_PM not enabled.
> - * * 1: Device already suspended.
>   * Other values and conditions for the above values are possible as returned by
>   * Runtime PM suspend callbacks.
>   */
> @@ -646,15 +648,15 @@ static inline int pm_runtime_put_sync(struct device *dev)
>   * if it returns an error code.
>   *
>   * Return:
> + * * 1: Usage counts dropped to zero, but device was already suspended.
>   * * 0: Success.
>   * * -EINVAL: Runtime PM error.
>   * * -EACCES: Runtime PM disabled.
> - * * -EAGAIN: Runtime PM usage_count non-zero or Runtime PM status change ongoing.
> - * * -EAGAIN: usage_count non-zero or Runtime PM status change ongoing.
> + * * -EAGAIN: Runtime PM usage_count became non-zero or Runtime PM status
> + *            change ongoing.
>   * * -EBUSY: Runtime PM child_count non-zero.
>   * * -EPERM: Device PM QoS resume latency 0.
>   * * -ENOSYS: CONFIG_PM not enabled.
> - * * 1: Device already suspended.
>   * Other values and conditions for the above values are possible as returned by
>   * Runtime PM suspend callbacks.
>   */
> @@ -677,15 +679,16 @@ static inline int pm_runtime_put_sync_suspend(struct device *dev)
>   * if it returns an error code.
>   *
>   * Return:
> + * * 1: Usage counts dropped to zero, but device was already suspended.
>   * * 0: Success.
>   * * -EINVAL: Runtime PM error.
>   * * -EACCES: Runtime PM disabled.
> - * * -EAGAIN: Runtime PM usage_count non-zero or Runtime PM status change ongoing.
> + * * -EAGAIN: Runtime PM usage_count became non-zero or Runtime PM status
> + *            change ongoing.
>   * * -EBUSY: Runtime PM child_count non-zero.
>   * * -EPERM: Device PM QoS resume latency 0.
>   * * -EINPROGRESS: Suspend already in progress.
>   * * -ENOSYS: CONFIG_PM not enabled.
> - * * 1: Device already suspended.
>   * Other values and conditions for the above values are possible as returned by
>   * Runtime PM suspend callbacks.
>   */

-- 
Kind regards,

Sakari Ailus

Re: [PATCH 2/3] PM: runtime: Make put{,_sync}() return 1 when already suspended

[Sakari Ailus]

Hi Brian,

On Thu, Aug 28, 2025 at 05:28:27PM -0700, Brian Norris wrote:
> The pm_runtime.h docs say pm_runtime_put() and pm_runtime_put_sync()
> return 1 when already suspended, but this is not true -- they return
> -EAGAIN. On the other hand, pm_runtime_put_sync_suspend() and
> pm_runtime_put_sync_autosuspend() *do* return 1.
> 
> This is an artifact of the fact that the former are built on rpm_idle(),
> whereas the latter are built on rpm_suspend().
> 
> There are precious few pm_runtime_put()/pm_runtime_put_sync() callers
> that check the return code at all, but most of them only log errors, and
> usually only for negative error codes. None of them should be treating
> this as an error, so:
> 
>  * at best, this may fix some case where a driver treats this condition
>    as an error, when it shouldn't;
> 
>  * at worst, this should make no effect; and
> 
>  * somewhere in between, we could potentially clear up non-fatal log
>    messages.
> 
> Fix the pm_runtime_already_suspended_test() while tweaking the behavior.
> The test makes a lot more sense when these all return 1 when the device
> is already suspended:
> 
>     pm_runtime_put(dev);
>     pm_runtime_put_sync(dev);
>     pm_runtime_suspend(dev);
>     pm_runtime_autosuspend(dev);
>     pm_request_autosuspend(dev);
>     pm_runtime_put_sync_autosuspend(dev);
>     pm_runtime_put_autosuspend(dev);
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> 
>  drivers/base/power/runtime-test.c | 8 ++------
>  drivers/base/power/runtime.c      | 3 +++
>  2 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/base/power/runtime-test.c b/drivers/base/power/runtime-test.c
> index 263c28d5fc50..1be18e871f1d 100644
> --- a/drivers/base/power/runtime-test.c
> +++ b/drivers/base/power/runtime-test.c
> @@ -43,15 +43,11 @@ static void pm_runtime_already_suspended_test(struct kunit *test)
>  	KUNIT_EXPECT_EQ(test, 0, pm_runtime_barrier(dev)); /* no wakeup needed */
>  
>  	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
> -	/*
> -	 * We never actually left RPM_SUSPENDED, but rpm_idle() still treats
> -	 * this as -EAGAIN / "runtime PM status change ongoing".
> -	 */
> -	KUNIT_EXPECT_EQ(test, -EAGAIN, pm_runtime_put(dev));
> +	KUNIT_EXPECT_EQ(test, 1, pm_runtime_put(dev));
>  
>  	pm_runtime_get_noresume(dev);
>  	KUNIT_EXPECT_TRUE(test, pm_runtime_suspended(dev));
> -	KUNIT_EXPECT_EQ(test, -EAGAIN, pm_runtime_put_sync(dev));
> +	KUNIT_EXPECT_EQ(test, 1, pm_runtime_put_sync(dev));
>  
>  	KUNIT_EXPECT_EQ(test, 1, pm_runtime_suspend(dev));
>  	KUNIT_EXPECT_EQ(test, 1, pm_runtime_autosuspend(dev));
> diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> index 3e84dc4122de..17cf111d16aa 100644
> --- a/drivers/base/power/runtime.c
> +++ b/drivers/base/power/runtime.c
> @@ -498,6 +498,9 @@ static int rpm_idle(struct device *dev, int rpmflags)
>  	if (retval < 0)
>  		;	/* Conditions are wrong. */
>  
> +	else if ((rpmflags & RPM_GET_PUT) && (retval == 1))
> +		;	/* put() is allowed in RPM_SUSPENDED */

Ah, I missed this while reviewing the 3rd patch. Makes sense. Please ignore
my comments regarding the 3rd patch on whether the return value 1 is
applicable.

The latter parentheses are redundant (the former, too, actually, but the
compiler warns so let them be).

> +
>  	/* Idle notifications are allowed only in the RPM_ACTIVE state. */
>  	else if (dev->power.runtime_status != RPM_ACTIVE)
>  		retval = -EAGAIN;

-- 
Kind regards,

Sakari Ailus