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

[Brian Norris <briannorris@chromium.org>]

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