Re: [PATCH 1/4] PM: runtime: Clear runtime_error on supplier after failed get_sync

[Rafael J. Wysocki (Intel)]

On Thu, Jul 2, 2026 at 1:53 PM Praveen Talari
<praveen.talari@xxxxxxxxxxxxxxxx> wrote:
>
> Hi Rafael,
>
> Thank you for review.
>
> On 02-07-2026 17:07, Rafael J. Wysocki (Intel) wrote:
> > On Thu, Jul 2, 2026 at 8:08 AM Praveen Talari
> > <praveen.talari@xxxxxxxxxxxxxxxx> wrote:
> >> When pm_runtime_get_sync() fails for a supplier device in
> >> rpm_get_suppliers(), the supplier's power.runtime_error field is left
> >> set. This causes any subsequent rpm_resume() call on that supplier to
> >> immediately return -EINVAL at the top of the function without
> >> attempting an actual resume, making the failure permanent until
> >> runtime PM is explicitly re-enabled.
> >>
> >> Fix this by calling pm_runtime_set_suspended() on the supplier after
> >> pm_runtime_put_noidle() in the error path. This clears runtime_error
> >> and resets the runtime PM status to RPM_SUSPENDED, allowing the next
> >> consumer resume attempt to retry the supplier resume normally.
> >>
> >> Change-Id: Id5067d09caca464f663fc95fe745d037e9c56664
> >> Signed-off-by: Praveen Talari <praveen.talari@xxxxxxxxxxxxxxxx>
> >> ---
> >>   drivers/base/power/runtime.c | 1 +
> >>   1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> >> index 335288e8b5b3..9811d024d140 100644
> >> --- a/drivers/base/power/runtime.c
> >> +++ b/drivers/base/power/runtime.c
> >> @@ -309,6 +309,7 @@ static int rpm_get_suppliers(struct device *dev)
> >>                  /* Ignore suppliers with disabled runtime PM. */
> >>                  if (retval < 0 && retval != -EACCES) {
> >>                          pm_runtime_put_noidle(link->supplier);
> >> +                       pm_runtime_set_suspended(link->supplier);
> >>                          return retval;
> >>                  }
> >>                  refcount_inc(&link->rpm_active);
> >>
> >> --
> > I don't think that this is the way to go here.
> >
> > Can you please say some more about the specific scenario in which this
> > happens and explain why it is OK to effectively discard runtime PM
> > errors occurring when suppliers are handled?
>
> This was observed with a consumer device whose supplier's
> ->runtime_resume callback returns an error (e.g., a transient failure
> such as a timeout or resource unavailability).

In runtime PM, errors returned by the suspend and resume callbacks,
except for -EAGAIN and -EBUSY are not regarded as transient, which is
why they stick.

If the driver or bus type wants to signal a transient error, it should
return one of the above error codes.

That said, I can see that this is problematic on the resume side where
it may be desirable to repeat resume attempts in the hope that one of
them will succeed.

I would then consider changing the runtime PM core code so that
power.runtime_error is only set on failing attempts to suspend, but it
will not be set on failing attempts to resume.

> When that happens,
> rpm_resume() sets power.runtime_error on the supplier and returns an
> error. pm_runtime_get_sync() propagates that error up to
> rpm_get_suppliers(), which then calls pm_runtime_put_noidle() and
> returns, aborting the consumer's resume.

Right.

> The problem is that power.runtime_error remains set on the supplier. On
> the next attempt to resume the consumer, rpm_get_suppliers() calls
> pm_runtime_get_sync() on the supplier again, but rpm_resume() now
> returns -EINVAL immediately at the top-of-function runtime_error check —
> without even attempting the resume callback. The supplier is stuck
> permanently in the error state until someone explicitly calls
> pm_runtime_reinit() or re-enables runtime PM on it.
>
> when rpm_get_suppliers() fails and we're already propagating the error
> to the consumer's resume path, we should not leave the supplier in a
> state that prevents all future retry attempts.
>
> Would you suggest a different approach here? I want to understand what
> the preferred recovery path should be when a supplier's resume fails
> inside rpm_get_suppliers().

Well, this isn't really different to any other cases when runtime
resume fails.  If it fails for a given device, it will also fail for
all devices depending on it.

But fair enough, please see above.