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

[Praveen Talari]

Hi Rafael,

Thank you for review.

On 02-07-2026 18:29, Rafael J. Wysocki (Intel) wrote:
> 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.
Yes, that's correct.
I hope the changes below address actual issue. Please let me know if you 
have any further comments or suggestions.


diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 9811d024d140..88450438dc88 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -470,7 +470,13 @@ static int rpm_callback(int (*cb)(struct device *), 
struct device *dev)
        if (retval == -EACCES)
                retval = -EAGAIN;

-       if (retval != -EAGAIN && retval != -EBUSY)
+       /*
+        * Only stick the error on suspend failures.  Resume failures 
are not
+        * treated as permanent so that the next resume attempt will run the
+        * callback again rather than short-circuiting on runtime_error.
+        */
+       if (retval != -EAGAIN && retval != -EBUSY &&
+           dev->power.runtime_status == RPM_SUSPENDING)
                dev->power.runtime_error = retval;

        return retval;


Thanks,

Praveen Talari


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