Re: [PATCH v2 3/5] PM: runtime: Propagate last_busy from dependent to dependency

[Jonathan Cameron]

On Tue,  3 Nov 2026 14:14:41 -0800
Brian Norris <briannorris@xxxxxxxxxxxx> 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@xxxxxxxxxxxxxx/ [1]
> Signed-off-by: Brian Norris <briannorris@xxxxxxxxxxxx>
> ---
> Cc: Ulf Hansson <ulfh@xxxxxxxxxx>
> 
> 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);
>