[PATCH v2 0/6] Update last busy timestamp in Runtime PM autosuspend callbacks

[PATCH v2 0/6] Update last busy timestamp in Runtime PM autosuspend callbacks

[Sakari Ailus]

Folks,

The original plan for adding pm_runtime_mark_last_busy() calls to
functions dealing with Runtime PM autosuspend originally included a few
intermediate steps of driver conversion, including the use of recently
added __pm_runtime_put_autosuspend(). The review of the set converting the
users first to __pm_runtime_put_autosuspend() concluded this wasn't
necessary. See
<URL:https://lore.kernel.org/all/20241004094101.113349-1-sakari.ailus@linux.intel.com/>.

This set extends the inclusion of the pm_runtime_mark_last_busy() call to
the _autosuspend() variants of the Runtime PM functions dealing with
suspending devices, i.e. pm_runtime_put_autosuspend(),
pm_runtime_put_sync_autosuspend(), pm_runtime_autosuspend() and
pm_request_autosuspend(). This will introduce, for a brief amount of time,
unnecessary calls to pm_runtime_mark_last_busy() but this wasn't seen as
an issue. Also, all users of these functions, including those that did not
call pm_runtime_mark_last_busy(), will now include that call. Presumably
in the vast majority of the cases a missing call would have been a bug.

Once this set is merged, I'll post further patches to remove the extra
pm_runtime_mark_last_busy() calls. The current set of these patches is
here
<URL:https://git.kernel.org/pub/scm/linux/kernel/git/sailus/linux-next.git/log/?h=pm-direct-on-next>.

It'd be best to have all merged within the same cycle.

Rafael: any thoughts on the merging? Would an immutable branch on top of
rc1 be an option? The patches apply cleanly on v6.16-rc1, v6.16-rc2 and
linux-next.

The changes in the patches to remove the pm_runtime_mark_last_busy() calls
have been generated using the following Coccinelle spatch:

@@
expression E;
identifier label, rval;
@@
- pm_runtime_mark_last_busy(E);
...
(
  label:
|
)
...
(
  pm_runtime_put_autosuspend(E);
|
  pm_runtime_put_sync_autosuspend(E);
|
  pm_runtime_autosuspend(E);
|
  pm_request_autosuspend(E);
|
  (void)pm_runtime_put_autosuspend(E);
|
  (void)pm_runtime_put_sync_autosuspend(E);
|
  (void)pm_runtime_autosuspend(E);
|
  (void)pm_request_autosuspend(E);
|
  return pm_runtime_put_autosuspend(E);
|
  return pm_runtime_put_sync_autosuspend(E);
|
  return pm_runtime_autosuspend(E);
|
  return pm_request_autosuspend(E);
|
  rval = pm_runtime_put_autosuspend(E);
|
  rval = pm_runtime_put_sync_autosuspend(E);
|
  rval = pm_runtime_autosuspend(E);
|
  rval = pm_request_autosuspend(E);
)

since v1:

- kernel-doc fix in patch "pm: runtime: Mark last busy stamp in
  pm_runtime_put_sync_autosuspend()".

- Dropped the first patch which has been merged already.

Sakari Ailus (6):
  pm: runtime: Document return values of suspend related API functions
  pm: runtime: Mark last busy stamp in pm_runtime_put_autosuspend()
  pm: runtime: Mark last busy stamp in pm_runtime_put_sync_autosuspend()
  pm: runtime: Mark last busy stamp in pm_runtime_autosuspend()
  pm: runtime: Mark last busy stamp in pm_request_autosuspend()
  Documentation: PM: *_autosuspend() functions update last busy time

 Documentation/power/runtime_pm.rst |  50 ++++----
 include/linux/pm_runtime.h         | 187 +++++++++++++++++++++++++----
 2 files changed, 186 insertions(+), 51 deletions(-)

-- 
2.39.5

[PATCH v2 1/6] pm: runtime: Document return values of suspend related API functions

[Sakari Ailus]

Document return values for device suspend and idle related API functions.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 include/linux/pm_runtime.h | 147 ++++++++++++++++++++++++++++++++++---
 1 file changed, 138 insertions(+), 9 deletions(-)

diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
index e7cb70fcc0af..9dd2e4031a27 100644
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -337,6 +337,20 @@ static inline void pm_runtime_release_supplier(struct device_link *link) {}
  * Invoke the "idle check" callback of @dev and, depending on its return value,
  * set up autosuspend of @dev or suspend it (depending on whether or not
  * autosuspend has been enabled for it).
+ *
+ * Return:
+ * * 0: Success.
+ * * -EINVAL: Runtime PM error.
+ * * -EACCES: Runtime PM disabled.
+ * * -EAGAIN: Runtime PM usage_count non-zero, Runtime PM status change ongoing
+ *            or device not in %RPM_ACTIVE state.
+ * * -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 idle and suspend callbacks.
  */
 static inline int pm_runtime_idle(struct device *dev)
 {
@@ -346,6 +360,18 @@ static inline int pm_runtime_idle(struct device *dev)
 /**
  * pm_runtime_suspend - Suspend a device synchronously.
  * @dev: Target device.
+ *
+ * Return:
+ * * 0: Success.
+ * * -EINVAL: Runtime PM error.
+ * * -EACCES: Runtime PM disabled.
+ * * -EAGAIN: Runtime PM usage_count 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.
  */
 static inline int pm_runtime_suspend(struct device *dev)
 {
@@ -358,6 +384,18 @@ static inline int pm_runtime_suspend(struct device *dev)
  *
  * Set up autosuspend of @dev or suspend it (depending on whether or not
  * autosuspend is enabled for it) without engaging its "idle check" callback.
+ *
+ * Return:
+ * * 0: Success.
+ * * -EINVAL: Runtime PM error.
+ * * -EACCES: Runtime PM disabled.
+ * * -EAGAIN: Runtime PM usage_count 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.
  */
 static inline int pm_runtime_autosuspend(struct device *dev)
 {
@@ -379,6 +417,18 @@ static inline int pm_runtime_resume(struct device *dev)
  *
  * Queue up a work item to run an equivalent of pm_runtime_idle() for @dev
  * asynchronously.
+ *
+ * Return:
+ * * 0: Success.
+ * * -EINVAL: Runtime PM error.
+ * * -EACCES: Runtime PM disabled.
+ * * -EAGAIN: Runtime PM usage_count non-zero, Runtime PM status change ongoing
+ *            or device not in %RPM_ACTIVE state.
+ * * -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_request_idle(struct device *dev)
 {
@@ -400,6 +450,17 @@ static inline int pm_request_resume(struct device *dev)
  *
  * Queue up a work item to run an equivalent pm_runtime_autosuspend() for @dev
  * asynchronously.
+ *
+ * Return:
+ * * 0: Success.
+ * * -EINVAL: Runtime PM error.
+ * * -EACCES: Runtime PM disabled.
+ * * -EAGAIN: Runtime PM usage_count 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_request_autosuspend(struct device *dev)
 {
@@ -464,6 +525,17 @@ static inline int pm_runtime_resume_and_get(struct device *dev)
  *
  * Decrement the runtime PM usage counter of @dev and if it turns out to be
  * equal to 0, queue up a work item for @dev like in pm_request_idle().
+ *
+ * Return:
+ * * 0: Success.
+ * * -EINVAL: Runtime PM error.
+ * * -EACCES: Runtime PM disabled.
+ * * -EAGAIN: Runtime PM usage_count 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)
 {
@@ -478,6 +550,17 @@ DEFINE_FREE(pm_runtime_put, struct device *, if (_T) pm_runtime_put(_T))
  *
  * Decrement the runtime PM usage counter of @dev and if it turns out to be
  * equal to 0, queue up a work item for @dev like in pm_request_autosuspend().
+ *
+ * Return:
+ * * 0: Success.
+ * * -EINVAL: Runtime PM error.
+ * * -EACCES: Runtime PM disabled.
+ * * -EAGAIN: Runtime PM usage_count 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)
 {
@@ -490,6 +573,17 @@ static inline int __pm_runtime_put_autosuspend(struct device *dev)
  *
  * Decrement the runtime PM usage counter of @dev and if it turns out to be
  * equal to 0, queue up a work item for @dev like in pm_request_autosuspend().
+ *
+ * Return:
+ * * 0: Success.
+ * * -EINVAL: Runtime PM error.
+ * * -EACCES: Runtime PM disabled.
+ * * -EAGAIN: Runtime PM usage_count 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)
 {
@@ -506,9 +600,20 @@ static inline int pm_runtime_put_autosuspend(struct device *dev)
  * return value, set up autosuspend of @dev or suspend it (depending on whether
  * or not autosuspend has been enabled for it).
  *
- * The possible return values of this function are the same as for
- * pm_runtime_idle() and the runtime PM usage counter of @dev remains
- * decremented in all cases, even if it returns an error code.
+ * The runtime PM usage counter of @dev remains decremented in all cases, even
+ * if it returns an error code.
+ *
+ * Return:
+ * * 0: Success.
+ * * -EINVAL: Runtime PM error.
+ * * -EACCES: Runtime PM disabled.
+ * * -EAGAIN: Runtime PM usage_count 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.
  */
 static inline int pm_runtime_put_sync(struct device *dev)
 {
@@ -522,9 +627,21 @@ static inline int pm_runtime_put_sync(struct device *dev)
  * Decrement the runtime PM usage counter of @dev and if it turns out to be
  * equal to 0, carry out runtime-suspend of @dev synchronously.
  *
- * The possible return values of this function are the same as for
- * pm_runtime_suspend() and the runtime PM usage counter of @dev remains
- * decremented in all cases, even if it returns an error code.
+ * The runtime PM usage counter of @dev remains decremented in all cases, even
+ * if it returns an error code.
+ *
+ * Return:
+ * * 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.
+ * * -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.
  */
 static inline int pm_runtime_put_sync_suspend(struct device *dev)
 {
@@ -539,9 +656,21 @@ static inline int pm_runtime_put_sync_suspend(struct device *dev)
  * equal to 0, set up autosuspend of @dev or suspend it synchronously (depending
  * on whether or not autosuspend has been enabled for it).
  *
- * The possible return values of this function are the same as for
- * pm_runtime_autosuspend() and the runtime PM usage counter of @dev remains
- * decremented in all cases, even if it returns an error code.
+ * The runtime PM usage counter of @dev remains decremented in all cases, even
+ * if it returns an error code.
+ *
+ * Return:
+ * * 0: Success.
+ * * -EINVAL: Runtime PM error.
+ * * -EACCES: Runtime PM disabled.
+ * * -EAGAIN: Runtime PM usage_count 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.
  */
 static inline int pm_runtime_put_sync_autosuspend(struct device *dev)
 {
-- 
2.39.5

[PATCH v2 2/6] pm: runtime: Mark last busy stamp in pm_runtime_put_autosuspend()

[Sakari Ailus]

Set device's last busy timestamp to current time in
pm_runtime_put_autosuspend(). Callers wishing not to do that will need to
use __pm_runtime_put_autosuspend().

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/power/runtime_pm.rst | 23 ++++++++++-------------
 include/linux/pm_runtime.h         | 12 +++++++-----
 2 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/Documentation/power/runtime_pm.rst b/Documentation/power/runtime_pm.rst
index 63344bea8393..e7bbdc66d64c 100644
--- a/Documentation/power/runtime_pm.rst
+++ b/Documentation/power/runtime_pm.rst
@@ -411,8 +411,9 @@ drivers/base/power/runtime.c and include/linux/pm_runtime.h:
       pm_request_idle(dev) and return its result
 
   `int pm_runtime_put_autosuspend(struct device *dev);`
-    - does the same as __pm_runtime_put_autosuspend() for now, but in the
-      future, will also call pm_runtime_mark_last_busy() as well, DO NOT USE!
+    - set the power.last_busy field to the current time and decrement the
+      device's usage counter; if the result is 0 then run
+      pm_request_autosuspend(dev) and return its result
 
   `int __pm_runtime_put_autosuspend(struct device *dev);`
     - decrement the device's usage counter; if the result is 0 then run
@@ -870,11 +871,9 @@ device is automatically suspended (the subsystem or driver still has to call
 the appropriate PM routines); rather it means that runtime suspends will
 automatically be delayed until the desired period of inactivity has elapsed.
 
-Inactivity is determined based on the power.last_busy field.  Drivers should
-call pm_runtime_mark_last_busy() to update this field after carrying out I/O,
-typically just before calling __pm_runtime_put_autosuspend().  The desired
-length of the inactivity period is a matter of policy.  Subsystems can set this
-length initially by calling pm_runtime_set_autosuspend_delay(), but after device
+Inactivity is determined based on the power.last_busy field. The desired length
+of the inactivity period is a matter of policy.  Subsystems can set this length
+initially by calling pm_runtime_set_autosuspend_delay(), but after device
 registration the length should be controlled by user space, using the
 /sys/devices/.../power/autosuspend_delay_ms attribute.
 
@@ -885,7 +884,7 @@ instead of the non-autosuspend counterparts::
 
 	Instead of: pm_runtime_suspend    use: pm_runtime_autosuspend;
 	Instead of: pm_schedule_suspend   use: pm_request_autosuspend;
-	Instead of: pm_runtime_put        use: __pm_runtime_put_autosuspend;
+	Instead of: pm_runtime_put        use: pm_runtime_put_autosuspend;
 	Instead of: pm_runtime_put_sync   use: pm_runtime_put_sync_autosuspend.
 
 Drivers may also continue to use the non-autosuspend helper functions; they
@@ -922,12 +921,10 @@ Here is a schematic pseudo-code example::
 	foo_io_completion(struct foo_priv *foo, void *req)
 	{
 		lock(&foo->private_lock);
-		if (--foo->num_pending_requests == 0) {
-			pm_runtime_mark_last_busy(&foo->dev);
-			__pm_runtime_put_autosuspend(&foo->dev);
-		} else {
+		if (--foo->num_pending_requests == 0)
+			pm_runtime_put_autosuspend(&foo->dev);
+		else
 			foo_process_next_request(foo);
-		}
 		unlock(&foo->private_lock);
 		/* Send req result back to the user ... */
 	}
diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
index 9dd2e4031a27..14ca7be96686 100644
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -568,11 +568,13 @@ static inline int __pm_runtime_put_autosuspend(struct device *dev)
 }
 
 /**
- * pm_runtime_put_autosuspend - Drop device usage counter and queue autosuspend if 0.
+ * pm_runtime_put_autosuspend - Update the last access time of a device, drop
+ * its usage counter and queue autosuspend if the usage counter becomes 0.
  * @dev: Target device.
  *
- * Decrement the runtime PM usage counter of @dev and if it turns out to be
- * equal to 0, queue up a work item for @dev like in pm_request_autosuspend().
+ * Update the last access time of @dev, decrement runtime PM usage counter of
+ * @dev and if it turns out to be equal to 0, queue up a work item for @dev like
+ * in pm_request_autosuspend().
  *
  * Return:
  * * 0: Success.
@@ -587,8 +589,8 @@ static inline int __pm_runtime_put_autosuspend(struct device *dev)
  */
 static inline int pm_runtime_put_autosuspend(struct device *dev)
 {
-	return __pm_runtime_suspend(dev,
-	    RPM_GET_PUT | RPM_ASYNC | RPM_AUTO);
+	pm_runtime_mark_last_busy(dev);
+	return __pm_runtime_put_autosuspend(dev);
 }
 
 /**
-- 
2.39.5

[PATCH v2 3/6] pm: runtime: Mark last busy stamp in pm_runtime_put_sync_autosuspend()

[Sakari Ailus]

Set device's last busy timestamp to current time in
pm_runtime_put_sync_autosuspend().

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/power/runtime_pm.rst |  3 ++-
 include/linux/pm_runtime.h         | 11 +++++++----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/Documentation/power/runtime_pm.rst b/Documentation/power/runtime_pm.rst
index e7bbdc66d64c..9c21c913f9cf 100644
--- a/Documentation/power/runtime_pm.rst
+++ b/Documentation/power/runtime_pm.rst
@@ -428,7 +428,8 @@ drivers/base/power/runtime.c and include/linux/pm_runtime.h:
       pm_runtime_suspend(dev) and return its result
 
   `int pm_runtime_put_sync_autosuspend(struct device *dev);`
-    - decrement the device's usage counter; if the result is 0 then run
+    - set the power.last_busy field to the current time and decrement the
+      device's usage counter; if the result is 0 then run
       pm_runtime_autosuspend(dev) and return its result
 
   `void pm_runtime_enable(struct device *dev);`
diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
index 14ca7be96686..3a0d5f0ea471 100644
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -651,12 +651,14 @@ static inline int pm_runtime_put_sync_suspend(struct device *dev)
 }
 
 /**
- * pm_runtime_put_sync_autosuspend - Drop device usage counter and autosuspend if 0.
+ * pm_runtime_put_sync_autosuspend - Update the last access time of a device,
+ * drop device usage counter and autosuspend if 0.
  * @dev: Target device.
  *
- * Decrement the runtime PM usage counter of @dev and if it turns out to be
- * equal to 0, set up autosuspend of @dev or suspend it synchronously (depending
- * on whether or not autosuspend has been enabled for it).
+ * Update the last access time of @dev, decrement the runtime PM usage counter
+ * of @dev and if it turns out to be equal to 0, set up autosuspend of @dev or
+ * suspend it synchronously (depending on whether or not autosuspend has been
+ * enabled for it).
  *
  * The runtime PM usage counter of @dev remains decremented in all cases, even
  * if it returns an error code.
@@ -676,6 +678,7 @@ static inline int pm_runtime_put_sync_suspend(struct device *dev)
  */
 static inline int pm_runtime_put_sync_autosuspend(struct device *dev)
 {
+	pm_runtime_mark_last_busy(dev);
 	return __pm_runtime_suspend(dev, RPM_GET_PUT | RPM_AUTO);
 }
 
-- 
2.39.5

[PATCH v2 4/6] pm: runtime: Mark last busy stamp in pm_runtime_autosuspend()

[Sakari Ailus]

Set device's last busy timestamp to current time in
pm_runtime_autosuspend().

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/power/runtime_pm.rst | 15 ++++++---------
 include/linux/pm_runtime.h         |  9 ++++++---
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/Documentation/power/runtime_pm.rst b/Documentation/power/runtime_pm.rst
index 9c21c913f9cf..39a0b62f6648 100644
--- a/Documentation/power/runtime_pm.rst
+++ b/Documentation/power/runtime_pm.rst
@@ -154,11 +154,9 @@ suspending the device are satisfied) and to queue up a suspend request for the
 device in that case.  If there is no idle callback, or if the callback returns
 0, then the PM core will attempt to carry out a runtime suspend of the device,
 also respecting devices configured for autosuspend.  In essence this means a
-call to pm_runtime_autosuspend() (do note that drivers needs to update the
-device last busy mark, pm_runtime_mark_last_busy(), to control the delay under
-this circumstance).  To prevent this (for example, if the callback routine has
-started a delayed suspend), the routine must return a non-zero value.  Negative
-error return codes are ignored by the PM core.
+call to pm_runtime_autosuspend(). To prevent this (for example, if the callback
+routine has started a delayed suspend), the routine must return a non-zero
+value.  Negative error return codes are ignored by the PM core.
 
 The helper functions provided by the PM core, described in Section 4, guarantee
 that the following constraints are met with respect to runtime PM callbacks for
@@ -330,10 +328,9 @@ drivers/base/power/runtime.c and include/linux/pm_runtime.h:
       'power.disable_depth' is different from 0
 
   `int pm_runtime_autosuspend(struct device *dev);`
-    - same as pm_runtime_suspend() except that the autosuspend delay is taken
-      `into account;` if pm_runtime_autosuspend_expiration() says the delay has
-      not yet expired then an autosuspend is scheduled for the appropriate time
-      and 0 is returned
+    - same as pm_runtime_suspend() except that a call to
+      pm_runtime_mark_last_busy() is made and an autosuspend is scheduled for
+      the appropriate time and 0 is returned
 
   `int pm_runtime_resume(struct device *dev);`
     - execute the subsystem-level resume callback for the device; returns 0 on
diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
index 3a0d5f0ea471..566a07b60f63 100644
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -379,11 +379,13 @@ static inline int pm_runtime_suspend(struct device *dev)
 }
 
 /**
- * pm_runtime_autosuspend - Set up autosuspend of a device or suspend it.
+ * pm_runtime_autosuspend - Update the last access time and set up autosuspend
+ * of a device.
  * @dev: Target device.
  *
- * Set up autosuspend of @dev or suspend it (depending on whether or not
- * autosuspend is enabled for it) without engaging its "idle check" callback.
+ * First update the last access time, then set up autosuspend of @dev or suspend
+ * it (depending on whether or not autosuspend is enabled for it) without
+ * engaging its "idle check" callback.
  *
  * Return:
  * * 0: Success.
@@ -399,6 +401,7 @@ static inline int pm_runtime_suspend(struct device *dev)
  */
 static inline int pm_runtime_autosuspend(struct device *dev)
 {
+	pm_runtime_mark_last_busy(dev);
 	return __pm_runtime_suspend(dev, RPM_AUTO);
 }
 
-- 
2.39.5

[PATCH v2 5/6] pm: runtime: Mark last busy stamp in pm_request_autosuspend()

[Sakari Ailus]

Set device's last busy timestamp to current time in
pm_request_autosuspend().

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/power/runtime_pm.rst | 6 +++---
 include/linux/pm_runtime.h         | 8 +++++---
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/Documentation/power/runtime_pm.rst b/Documentation/power/runtime_pm.rst
index 39a0b62f6648..91bc93422262 100644
--- a/Documentation/power/runtime_pm.rst
+++ b/Documentation/power/runtime_pm.rst
@@ -354,9 +354,9 @@ drivers/base/power/runtime.c and include/linux/pm_runtime.h:
       success or error code if the request has not been queued up
 
   `int pm_request_autosuspend(struct device *dev);`
-    - schedule the execution of the subsystem-level suspend callback for the
-      device when the autosuspend delay has expired; if the delay has already
-      expired then the work item is queued up immediately
+    - Call pm_runtime_mark_last_busy() and schedule the execution of the
+      subsystem-level suspend callback for the device when the autosuspend delay
+      expires
 
   `int pm_schedule_suspend(struct device *dev, unsigned int delay);`
     - schedule the execution of the subsystem-level suspend callback for the
diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
index 566a07b60f63..778d5988f35e 100644
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -448,11 +448,12 @@ static inline int pm_request_resume(struct device *dev)
 }
 
 /**
- * pm_request_autosuspend - Queue up autosuspend of a device.
+ * pm_request_autosuspend - Update the last access time and queue up autosuspend
+ * of a device.
  * @dev: Target device.
  *
- * Queue up a work item to run an equivalent pm_runtime_autosuspend() for @dev
- * asynchronously.
+ * Update the last access time of a device and queue up a work item to run an
+ * equivalent pm_runtime_autosuspend() for @dev asynchronously.
  *
  * Return:
  * * 0: Success.
@@ -467,6 +468,7 @@ static inline int pm_request_resume(struct device *dev)
  */
 static inline int pm_request_autosuspend(struct device *dev)
 {
+	pm_runtime_mark_last_busy(dev);
 	return __pm_runtime_suspend(dev, RPM_ASYNC | RPM_AUTO);
 }
 
-- 
2.39.5

[PATCH v2 6/6] Documentation: PM: *_autosuspend() functions update last busy time

[Sakari Ailus]

Document that the *_autosuspend() variants of the Runtime PM functions
update the last busy timestamp.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/power/runtime_pm.rst | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/power/runtime_pm.rst b/Documentation/power/runtime_pm.rst
index 91bc93422262..c8dbdb8595e5 100644
--- a/Documentation/power/runtime_pm.rst
+++ b/Documentation/power/runtime_pm.rst
@@ -887,7 +887,8 @@ instead of the non-autosuspend counterparts::
 
 Drivers may also continue to use the non-autosuspend helper functions; they
 will behave normally, which means sometimes taking the autosuspend delay into
-account (see pm_runtime_idle).
+account (see pm_runtime_idle). The autosuspend variants of the functions also
+call pm_runtime_mark_last_busy().
 
 Under some circumstances a driver or subsystem may want to prevent a device
 from autosuspending immediately, even though the usage counter is zero and the
-- 
2.39.5

Re: [PATCH v2 0/6] Update last busy timestamp in Runtime PM autosuspend callbacks

[Rafael J. Wysocki]

On Mon, Jun 16, 2025 at 8:12 AM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Folks,
>
> The original plan for adding pm_runtime_mark_last_busy() calls to
> functions dealing with Runtime PM autosuspend originally included a few
> intermediate steps of driver conversion, including the use of recently
> added __pm_runtime_put_autosuspend(). The review of the set converting the
> users first to __pm_runtime_put_autosuspend() concluded this wasn't
> necessary. See
> <URL:https://lore.kernel.org/all/20241004094101.113349-1-sakari.ailus@linux.intel.com/>.
>
> This set extends the inclusion of the pm_runtime_mark_last_busy() call to
> the _autosuspend() variants of the Runtime PM functions dealing with
> suspending devices, i.e. pm_runtime_put_autosuspend(),
> pm_runtime_put_sync_autosuspend(), pm_runtime_autosuspend() and
> pm_request_autosuspend(). This will introduce, for a brief amount of time,
> unnecessary calls to pm_runtime_mark_last_busy() but this wasn't seen as
> an issue. Also, all users of these functions, including those that did not
> call pm_runtime_mark_last_busy(), will now include that call. Presumably
> in the vast majority of the cases a missing call would have been a bug.
>
> Once this set is merged, I'll post further patches to remove the extra
> pm_runtime_mark_last_busy() calls. The current set of these patches is
> here
> <URL:https://git.kernel.org/pub/scm/linux/kernel/git/sailus/linux-next.git/log/?h=pm-direct-on-next>.
>
> It'd be best to have all merged within the same cycle.
>
> Rafael: any thoughts on the merging?

I'm going to queue this up for 6.17.

> Would an immutable branch on top of rc1 be an option?

I think so, but does anyone need it?

> The patches apply cleanly on v6.16-rc1, v6.16-rc2 and linux-next.

Awesome, thanks!

> The changes in the patches to remove the pm_runtime_mark_last_busy() calls
> have been generated using the following Coccinelle spatch:
>
> @@
> expression E;
> identifier label, rval;
> @@
> - pm_runtime_mark_last_busy(E);
> ...
> (
>   label:
> |
> )
> ...
> (
>   pm_runtime_put_autosuspend(E);
> |
>   pm_runtime_put_sync_autosuspend(E);
> |
>   pm_runtime_autosuspend(E);
> |
>   pm_request_autosuspend(E);
> |
>   (void)pm_runtime_put_autosuspend(E);
> |
>   (void)pm_runtime_put_sync_autosuspend(E);
> |
>   (void)pm_runtime_autosuspend(E);
> |
>   (void)pm_request_autosuspend(E);
> |
>   return pm_runtime_put_autosuspend(E);
> |
>   return pm_runtime_put_sync_autosuspend(E);
> |
>   return pm_runtime_autosuspend(E);
> |
>   return pm_request_autosuspend(E);
> |
>   rval = pm_runtime_put_autosuspend(E);
> |
>   rval = pm_runtime_put_sync_autosuspend(E);
> |
>   rval = pm_runtime_autosuspend(E);
> |
>   rval = pm_request_autosuspend(E);
> )
>
> since v1:
>
> - kernel-doc fix in patch "pm: runtime: Mark last busy stamp in
>   pm_runtime_put_sync_autosuspend()".
>
> - Dropped the first patch which has been merged already.
>
> Sakari Ailus (6):
>   pm: runtime: Document return values of suspend related API functions
>   pm: runtime: Mark last busy stamp in pm_runtime_put_autosuspend()
>   pm: runtime: Mark last busy stamp in pm_runtime_put_sync_autosuspend()
>   pm: runtime: Mark last busy stamp in pm_runtime_autosuspend()
>   pm: runtime: Mark last busy stamp in pm_request_autosuspend()
>   Documentation: PM: *_autosuspend() functions update last busy time
>
>  Documentation/power/runtime_pm.rst |  50 ++++----
>  include/linux/pm_runtime.h         | 187 +++++++++++++++++++++++++----
>  2 files changed, 186 insertions(+), 51 deletions(-)
>
> --

Re: [PATCH v2 0/6] Update last busy timestamp in Runtime PM autosuspend callbacks

[Sakari Ailus]

Hi Rafael,

On Mon, Jun 16, 2025 at 01:21:02PM +0200, Rafael J. Wysocki wrote:
> On Mon, Jun 16, 2025 at 8:12 AM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> >
> > Folks,
> >
> > The original plan for adding pm_runtime_mark_last_busy() calls to
> > functions dealing with Runtime PM autosuspend originally included a few
> > intermediate steps of driver conversion, including the use of recently
> > added __pm_runtime_put_autosuspend(). The review of the set converting the
> > users first to __pm_runtime_put_autosuspend() concluded this wasn't
> > necessary. See
> > <URL:https://lore.kernel.org/all/20241004094101.113349-1-sakari.ailus@linux.intel.com/>.
> >
> > This set extends the inclusion of the pm_runtime_mark_last_busy() call to
> > the _autosuspend() variants of the Runtime PM functions dealing with
> > suspending devices, i.e. pm_runtime_put_autosuspend(),
> > pm_runtime_put_sync_autosuspend(), pm_runtime_autosuspend() and
> > pm_request_autosuspend(). This will introduce, for a brief amount of time,
> > unnecessary calls to pm_runtime_mark_last_busy() but this wasn't seen as
> > an issue. Also, all users of these functions, including those that did not
> > call pm_runtime_mark_last_busy(), will now include that call. Presumably
> > in the vast majority of the cases a missing call would have been a bug.
> >
> > Once this set is merged, I'll post further patches to remove the extra
> > pm_runtime_mark_last_busy() calls. The current set of these patches is
> > here
> > <URL:https://git.kernel.org/pub/scm/linux/kernel/git/sailus/linux-next.git/log/?h=pm-direct-on-next>.
> >
> > It'd be best to have all merged within the same cycle.
> >
> > Rafael: any thoughts on the merging?
> 
> I'm going to queue this up for 6.17.

Thank you! :-)

> 
> > Would an immutable branch on top of rc1 be an option?
> 
> I think so, but does anyone need it?

I guess it's not mandatory but we'll have now a lot of redundant calls to
the pm_runtime_mark_last_busy(). It just doesn't look very elegant. In the
end it's all up to what the maintainers prefer.

-- 
Regards,

Sakari Ailus

Re: [PATCH v2 0/6] Update last busy timestamp in Runtime PM autosuspend callbacks

[Laurent Pinchart]

On Mon, Jun 16, 2025 at 07:20:21PM +0000, Sakari Ailus wrote:
> On Mon, Jun 16, 2025 at 01:21:02PM +0200, Rafael J. Wysocki wrote:
> > On Mon, Jun 16, 2025 at 8:12 AM Sakari Ailus wrote:
> > >
> > > Folks,
> > >
> > > The original plan for adding pm_runtime_mark_last_busy() calls to
> > > functions dealing with Runtime PM autosuspend originally included a few
> > > intermediate steps of driver conversion, including the use of recently
> > > added __pm_runtime_put_autosuspend(). The review of the set converting the
> > > users first to __pm_runtime_put_autosuspend() concluded this wasn't
> > > necessary. See
> > > <URL:https://lore.kernel.org/all/20241004094101.113349-1-sakari.ailus@linux.intel.com/>.
> > >
> > > This set extends the inclusion of the pm_runtime_mark_last_busy() call to
> > > the _autosuspend() variants of the Runtime PM functions dealing with
> > > suspending devices, i.e. pm_runtime_put_autosuspend(),
> > > pm_runtime_put_sync_autosuspend(), pm_runtime_autosuspend() and
> > > pm_request_autosuspend(). This will introduce, for a brief amount of time,
> > > unnecessary calls to pm_runtime_mark_last_busy() but this wasn't seen as
> > > an issue. Also, all users of these functions, including those that did not
> > > call pm_runtime_mark_last_busy(), will now include that call. Presumably
> > > in the vast majority of the cases a missing call would have been a bug.
> > >
> > > Once this set is merged, I'll post further patches to remove the extra
> > > pm_runtime_mark_last_busy() calls. The current set of these patches is
> > > here
> > > <URL:https://git.kernel.org/pub/scm/linux/kernel/git/sailus/linux-next.git/log/?h=pm-direct-on-next>.
> > >
> > > It'd be best to have all merged within the same cycle.
> > >
> > > Rafael: any thoughts on the merging?
> > 
> > I'm going to queue this up for 6.17.
> 
> Thank you! :-)
> 
> > > Would an immutable branch on top of rc1 be an option?
> > 
> > I think so, but does anyone need it?
> 
> I guess it's not mandatory but we'll have now a lot of redundant calls to
> the pm_runtime_mark_last_busy(). It just doesn't look very elegant. In the
> end it's all up to what the maintainers prefer.

Those calls should be removed, but for the drivers I maintain, I don't
mind much if the removal is delayed until v6.18. Still, an immutable
branch based on v6.16-rc1 would speed up this work, and I think it's
worth it.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 0/6] Update last busy timestamp in Runtime PM autosuspend callbacks

[Rafael J. Wysocki]

On Mon, Jun 16, 2025 at 9:20 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Rafael,
>
> On Mon, Jun 16, 2025 at 01:21:02PM +0200, Rafael J. Wysocki wrote:
> > On Mon, Jun 16, 2025 at 8:12 AM Sakari Ailus
> > <sakari.ailus@linux.intel.com> wrote:
> > >
> > > Folks,
> > >
> > > The original plan for adding pm_runtime_mark_last_busy() calls to
> > > functions dealing with Runtime PM autosuspend originally included a few
> > > intermediate steps of driver conversion, including the use of recently
> > > added __pm_runtime_put_autosuspend(). The review of the set converting the
> > > users first to __pm_runtime_put_autosuspend() concluded this wasn't
> > > necessary. See
> > > <URL:https://lore.kernel.org/all/20241004094101.113349-1-sakari.ailus@linux.intel.com/>.
> > >
> > > This set extends the inclusion of the pm_runtime_mark_last_busy() call to
> > > the _autosuspend() variants of the Runtime PM functions dealing with
> > > suspending devices, i.e. pm_runtime_put_autosuspend(),
> > > pm_runtime_put_sync_autosuspend(), pm_runtime_autosuspend() and
> > > pm_request_autosuspend(). This will introduce, for a brief amount of time,
> > > unnecessary calls to pm_runtime_mark_last_busy() but this wasn't seen as
> > > an issue. Also, all users of these functions, including those that did not
> > > call pm_runtime_mark_last_busy(), will now include that call. Presumably
> > > in the vast majority of the cases a missing call would have been a bug.
> > >
> > > Once this set is merged, I'll post further patches to remove the extra
> > > pm_runtime_mark_last_busy() calls. The current set of these patches is
> > > here
> > > <URL:https://git.kernel.org/pub/scm/linux/kernel/git/sailus/linux-next.git/log/?h=pm-direct-on-next>.
> > >
> > > It'd be best to have all merged within the same cycle.
> > >
> > > Rafael: any thoughts on the merging?
> >
> > I'm going to queue this up for 6.17.
>
> Thank you! :-)

Now applied.

> >
> > > Would an immutable branch on top of rc1 be an option?
> >
> > I think so, but does anyone need it?
>
> I guess it's not mandatory but we'll have now a lot of redundant calls to
> the pm_runtime_mark_last_busy(). It just doesn't look very elegant. In the
> end it's all up to what the maintainers prefer.

I'll let you know when the branch is ready to pull from.

Thanks!

Re: [PATCH v2 0/6] Update last busy timestamp in Runtime PM autosuspend callbacks

[Oliver Neukum]

On 16.06.25 08:12, Sakari Ailus wrote:
> Folks,

Hi,
  
> This set extends the inclusion of the pm_runtime_mark_last_busy() call to
> the _autosuspend() variants of the Runtime PM functions dealing with
> suspending devices, i.e. pm_runtime_put_autosuspend(),
> pm_runtime_put_sync_autosuspend(), pm_runtime_autosuspend() and
> pm_request_autosuspend(). This will introduce, for a brief amount of time,
> unnecessary calls to pm_runtime_mark_last_busy() but this wasn't seen as
> an issue. Also, all users of these functions, including those that did not
> call pm_runtime_mark_last_busy(), will now include that call. Presumably
> in the vast majority of the cases a missing call would have been a bug.

Now that I think about this, I am not sure of the logic behind this.
It seems to me that you are making marking a device busy and dropping
the reference synonymous.

Is the time you use to derive the likelihood determined by the start
of IO or the end of IO?

	Regards
		Oliver

Re: [PATCH v2 0/6] Update last busy timestamp in Runtime PM autosuspend callbacks

[Sakari Ailus]

Hi Oliver,

On Mon, Jun 23, 2025 at 03:28:45PM +0200, Oliver Neukum wrote:
> On 16.06.25 08:12, Sakari Ailus wrote:
> > Folks,
> 
> Hi,
> > This set extends the inclusion of the pm_runtime_mark_last_busy() call to
> > the _autosuspend() variants of the Runtime PM functions dealing with
> > suspending devices, i.e. pm_runtime_put_autosuspend(),
> > pm_runtime_put_sync_autosuspend(), pm_runtime_autosuspend() and
> > pm_request_autosuspend(). This will introduce, for a brief amount of time,
> > unnecessary calls to pm_runtime_mark_last_busy() but this wasn't seen as
> > an issue. Also, all users of these functions, including those that did not
> > call pm_runtime_mark_last_busy(), will now include that call. Presumably
> > in the vast majority of the cases a missing call would have been a bug.
> 
> Now that I think about this, I am not sure of the logic behind this.
> It seems to me that you are making marking a device busy and dropping
> the reference synonymous.
> 
> Is the time you use to derive the likelihood determined by the start
> of IO or the end of IO?

The purpose of autosuspend is to delay device suspend in case it might be
used again soon after the point of dropping Runtime PM usage_count. Are
there cases you're aware of where pm_runtime_mark_last_busy() isn't
directly followed pm_runtime_put_autosuspend() or one of its variants and
it is desirable? Just stamping the last busy timestamp is a trivial
operation so one or two extra in a few places isn't really a problem.

-- 
Kind regards,

Sakari Ailus

Re: [PATCH v2 0/6] Update last busy timestamp in Runtime PM autosuspend callbacks

[Sakari Ailus]

Hi Rafael,

On Wed, Jun 18, 2025 at 09:43:25PM +0200, Rafael J. Wysocki wrote:
> On Mon, Jun 16, 2025 at 9:20 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> >
> > Hi Rafael,
> >
> > On Mon, Jun 16, 2025 at 01:21:02PM +0200, Rafael J. Wysocki wrote:
> > > On Mon, Jun 16, 2025 at 8:12 AM Sakari Ailus
> > > <sakari.ailus@linux.intel.com> wrote:
> > > >
> > > > Folks,
> > > >
> > > > The original plan for adding pm_runtime_mark_last_busy() calls to
> > > > functions dealing with Runtime PM autosuspend originally included a few
> > > > intermediate steps of driver conversion, including the use of recently
> > > > added __pm_runtime_put_autosuspend(). The review of the set converting the
> > > > users first to __pm_runtime_put_autosuspend() concluded this wasn't
> > > > necessary. See
> > > > <URL:https://lore.kernel.org/all/20241004094101.113349-1-sakari.ailus@linux.intel.com/>.
> > > >
> > > > This set extends the inclusion of the pm_runtime_mark_last_busy() call to
> > > > the _autosuspend() variants of the Runtime PM functions dealing with
> > > > suspending devices, i.e. pm_runtime_put_autosuspend(),
> > > > pm_runtime_put_sync_autosuspend(), pm_runtime_autosuspend() and
> > > > pm_request_autosuspend(). This will introduce, for a brief amount of time,
> > > > unnecessary calls to pm_runtime_mark_last_busy() but this wasn't seen as
> > > > an issue. Also, all users of these functions, including those that did not
> > > > call pm_runtime_mark_last_busy(), will now include that call. Presumably
> > > > in the vast majority of the cases a missing call would have been a bug.
> > > >
> > > > Once this set is merged, I'll post further patches to remove the extra
> > > > pm_runtime_mark_last_busy() calls. The current set of these patches is
> > > > here
> > > > <URL:https://git.kernel.org/pub/scm/linux/kernel/git/sailus/linux-next.git/log/?h=pm-direct-on-next>.
> > > >
> > > > It'd be best to have all merged within the same cycle.
> > > >
> > > > Rafael: any thoughts on the merging?
> > >
> > > I'm going to queue this up for 6.17.
> >
> > Thank you! :-)
> 
> Now applied.
> 
> > >
> > > > Would an immutable branch on top of rc1 be an option?
> > >
> > > I think so, but does anyone need it?
> >
> > I guess it's not mandatory but we'll have now a lot of redundant calls to
> > the pm_runtime_mark_last_busy(). It just doesn't look very elegant. In the
> > end it's all up to what the maintainers prefer.
> 
> I'll let you know when the branch is ready to pull from.

The branch is here
<URL:https://git.kernel.org/pub/scm/linux/kernel/git/sailus/linux.git/log/?h=pm-mark-last-busy-v6.16-rc1>,
I guess you can use:

	ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/sailus/linux.git	pm-mark-last-busy-v6.16-rc1

Let me know when you have pulled from there, I'll then post the updated set.

-- 
Kind regards,

Sakari Ailus

Re: [PATCH v2 1/6] pm: runtime: Document return values of suspend related API functions

[Brian Norris]

Hi,

On Mon, Jun 16, 2025 at 09:12:07AM +0300, Sakari Ailus wrote:
> Document return values for device suspend and idle related API functions.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

I appreciate the documentation attempt here. I've often found it a maze
trying to weave through the indirection and figure out the actual API
contract for some of these.

But I have a few questions below:

> ---
>  include/linux/pm_runtime.h | 147 ++++++++++++++++++++++++++++++++++---
>  1 file changed, 138 insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
> index e7cb70fcc0af..9dd2e4031a27 100644
> --- a/include/linux/pm_runtime.h
> +++ b/include/linux/pm_runtime.h


> @@ -464,6 +525,17 @@ static inline int pm_runtime_resume_and_get(struct device *dev)
>   *
>   * Decrement the runtime PM usage counter of @dev and if it turns out to be
>   * equal to 0, queue up a work item for @dev like in pm_request_idle().
> + *
> + * Return:
> + * * 0: Success.
> + * * -EINVAL: Runtime PM error.
> + * * -EACCES: Runtime PM disabled.
> + * * -EAGAIN: Runtime PM usage_count non-zero or Runtime PM status change ongoing.

^^ Is the "usage_count non-zero" true? For RPM_GET_PUT, we drop the
refcount, and if it's still nonzero, we simply return 0.

> + * * -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.

This part isn't very clear to me: can we even hit this case? If the
usage count was already 0, we'd hit EINVAL, since this is a PUT
operation. If the usage count was non-zero, we can't already be
suspended. At a minimum, we'd be RESUMING (e.g., an async resume), no?

>   */
>  static inline int pm_runtime_put(struct device *dev)
>  {

If these are indeed errors, I expect they're repeated on some of the
other related APIs too (like pm_runtime_put_sync(), pm_runtime_idle(),
and probably more).

I ask mostly for my own understanding, but I might consider patching the
docs if I'm not hallucinating these errors.

Brian

Re: [PATCH v2 1/6] pm: runtime: Document return values of suspend related API functions

[Brian Norris]

After reading and rereading ... and then writing unit tests, because I
can't trust my reading ... I think I can answer my own questions:

On Thu, Aug 14, 2025 at 03:56:24PM -0700, Brian Norris wrote:
> Hi,
> 
> On Mon, Jun 16, 2025 at 09:12:07AM +0300, Sakari Ailus wrote:
> > Document return values for device suspend and idle related API functions.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> 
> I appreciate the documentation attempt here. I've often found it a maze
> trying to weave through the indirection and figure out the actual API
> contract for some of these.
> 
> But I have a few questions below:
> 
> > ---
> >  include/linux/pm_runtime.h | 147 ++++++++++++++++++++++++++++++++++---
> >  1 file changed, 138 insertions(+), 9 deletions(-)
> > 
> > diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
> > index e7cb70fcc0af..9dd2e4031a27 100644
> > --- a/include/linux/pm_runtime.h
> > +++ b/include/linux/pm_runtime.h
> 
> 
> > @@ -464,6 +525,17 @@ static inline int pm_runtime_resume_and_get(struct device *dev)
> >   *
> >   * Decrement the runtime PM usage counter of @dev and if it turns out to be
> >   * equal to 0, queue up a work item for @dev like in pm_request_idle().
> > + *
> > + * Return:
> > + * * 0: Success.
> > + * * -EINVAL: Runtime PM error.
> > + * * -EACCES: Runtime PM disabled.
> > + * * -EAGAIN: Runtime PM usage_count non-zero or Runtime PM status change ongoing.
> 
> ^^ Is the "usage_count non-zero" true? For RPM_GET_PUT, we drop the
> refcount, and if it's still nonzero, we simply return 0.

It *is* possible, but it would only occur with the following:

(1) we've acquired our usage_count by way of pm_runtime_get_noresume();
    and
(2) some other actor is racing with us, acquiring a usage_count between
    when we decremented usage_count to 0, and when we check again in
    rpm_idle()

IMO, as-is, the language is a bit misleading though. But then, the
behavior is pretty subtle and hard to describe succinctly...

> > + * * -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.
> 
> This part isn't very clear to me: can we even hit this case? If the
> usage count was already 0, we'd hit EINVAL, since this is a PUT
> operation. If the usage count was non-zero, we can't already be
> suspended. At a minimum, we'd be RESUMING (e.g., an async resume), no?

This is sort of possible in the same scenario as above. But it doesn't
actually return 1; it still returns -EAGAIN.

Confusingly, pm_runtime_put_autosuspend() behaves differently, because
it's based on rpm_suspend() instead of rpm_idle().

I plan to fix this, because I don't see why pm_runtime_put() and
pm_runtime_put_autosuspend() should differ here.

> >   */
> >  static inline int pm_runtime_put(struct device *dev)
> >  {
> 
> If these are indeed errors, I expect they're repeated on some of the
> other related APIs too (like pm_runtime_put_sync(), pm_runtime_idle(),
> and probably more).
> 
> I ask mostly for my own understanding, but I might consider patching the
> docs if I'm not hallucinating these errors.

I've sent a patch series to fix some of the inconsistencies in the API
and to fix the API docs:

Subject: [PATCH 3/3] PM: runtime: Update kerneldoc return codes
https://lore.kernel.org/all/20250829003319.2785282-3-briannorris@chromium.org/

Brian

Re: [PATCH v2 1/6] pm: runtime: Document return values of suspend related API functions

[Sakari Ailus]

Hi Brian,

Thank you for the review.

On Thu, Aug 28, 2025 at 05:46:07PM -0700, Brian Norris wrote:
> After reading and rereading ... and then writing unit tests, because I
> can't trust my reading ... I think I can answer my own questions:
> 
> On Thu, Aug 14, 2025 at 03:56:24PM -0700, Brian Norris wrote:
> > Hi,
> > 
> > On Mon, Jun 16, 2025 at 09:12:07AM +0300, Sakari Ailus wrote:
> > > Document return values for device suspend and idle related API functions.
> > > 
> > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > 
> > I appreciate the documentation attempt here. I've often found it a maze
> > trying to weave through the indirection and figure out the actual API
> > contract for some of these.
> > 
> > But I have a few questions below:
> > 
> > > ---
> > >  include/linux/pm_runtime.h | 147 ++++++++++++++++++++++++++++++++++---
> > >  1 file changed, 138 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
> > > index e7cb70fcc0af..9dd2e4031a27 100644
> > > --- a/include/linux/pm_runtime.h
> > > +++ b/include/linux/pm_runtime.h
> > 
> > 
> > > @@ -464,6 +525,17 @@ static inline int pm_runtime_resume_and_get(struct device *dev)
> > >   *
> > >   * Decrement the runtime PM usage counter of @dev and if it turns out to be
> > >   * equal to 0, queue up a work item for @dev like in pm_request_idle().
> > > + *
> > > + * Return:
> > > + * * 0: Success.
> > > + * * -EINVAL: Runtime PM error.
> > > + * * -EACCES: Runtime PM disabled.
> > > + * * -EAGAIN: Runtime PM usage_count non-zero or Runtime PM status change ongoing.
> > 
> > ^^ Is the "usage_count non-zero" true? For RPM_GET_PUT, we drop the
> > refcount, and if it's still nonzero, we simply return 0.
> 
> It *is* possible, but it would only occur with the following:
> 
> (1) we've acquired our usage_count by way of pm_runtime_get_noresume();
>     and
> (2) some other actor is racing with us, acquiring a usage_count between
>     when we decremented usage_count to 0, and when we check again in
>     rpm_idle()
> 
> IMO, as-is, the language is a bit misleading though. But then, the
> behavior is pretty subtle and hard to describe succinctly...

I think it'd be indeed best to improve this. The current description is
indeed at bit misleading: usage_count should be non-zero when this function
is called.

> 
> > > + * * -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.
> > 
> > This part isn't very clear to me: can we even hit this case? If the
> > usage count was already 0, we'd hit EINVAL, since this is a PUT
> > operation. If the usage count was non-zero, we can't already be
> > suspended. At a minimum, we'd be RESUMING (e.g., an async resume), no?
> 
> This is sort of possible in the same scenario as above. But it doesn't
> actually return 1; it still returns -EAGAIN.

Yes; rpm_idle() does set retval to -EAGAIN, after it was assigned to 1 (via
rpm_check_suspend_allowed()). I missed this while writing the
documentation.

> 
> Confusingly, pm_runtime_put_autosuspend() behaves differently, because
> it's based on rpm_suspend() instead of rpm_idle().
> 
> I plan to fix this, because I don't see why pm_runtime_put() and
> pm_runtime_put_autosuspend() should differ here.

I agree.

> 
> > >   */
> > >  static inline int pm_runtime_put(struct device *dev)
> > >  {
> > 
> > If these are indeed errors, I expect they're repeated on some of the
> > other related APIs too (like pm_runtime_put_sync(), pm_runtime_idle(),
> > and probably more).
> > 
> > I ask mostly for my own understanding, but I might consider patching the
> > docs if I'm not hallucinating these errors.
> 
> I've sent a patch series to fix some of the inconsistencies in the API
> and to fix the API docs:
> 
> Subject: [PATCH 3/3] PM: runtime: Update kerneldoc return codes
> https://lore.kernel.org/all/20250829003319.2785282-3-briannorris@chromium.org/

Thanks, I'll take a look.

-- 
Kind regards,

Sakari Ailus