Re: [PATCH v2 1/2] PM: runtime: Only set runtime_error on suspend callback failures

[Praveen Talari]

Hi Rafael,

On 04-07-2026 01:53, Rafael J. Wysocki (Intel) wrote:
> On Fri, Jul 3, 2026 at 5:05 PM Praveen Talari
> <praveen.talari@xxxxxxxxxxxxxxxx> wrote:
> > When a runtime resume callback returns an error, rpm_callback() sets
> > power.runtime_error on the device.  This causes all subsequent calls to
> > rpm_resume() to return -EINVAL immediately at the top of the function
> > without invoking the callback again, making the failure permanent until
> > runtime PM is explicitly re-initialized.
> >
> > Unlike suspend failures, resume failures should be retryable.  If a
> > device's resume callback fails transiently, there is no reason to
> > permanently block future resume attempts on that device and all of its
> > consumers.
> >
> > Fix this by conditioning the power.runtime_error assignment in
> > rpm_callback() on the device being in the RPM_SUSPENDING state.  At the
> > point rpm_callback() runs, __update_runtime_status() has already set the
> > device status to either RPM_SUSPENDING or RPM_RESUMING, so the two paths
> > are reliably distinguishable.  Suspend callback failures continue to set
> > power.runtime_error as before.  Resume callback failures return the error
> > to the caller but leave power.runtime_error clear, allowing the next
> > resume attempt to invoke the callback normally.
> >
> > Signed-off-by: Praveen Talari <praveen.talari@xxxxxxxxxxxxxxxx>
> > ---
> >   drivers/base/power/runtime.c | 8 +++++++-
> >   1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> > index 335288e8b5b3..70d933bcd295 100644
> > --- a/drivers/base/power/runtime.c
> > +++ b/drivers/base/power/runtime.c
> > @@ -469,7 +469,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;
> Why don't you move this check to rpm_suspend()?
Yes, we can do that.
I hope the changes below address your concerns. Please let me know if 
there is anything else that needs to be adjusted.

diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 335288e8b5b3..fab38bc98113 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -469,9 +469,6 @@ static int rpm_callback(int (*cb)(struct device *), 
struct device *dev)
        if (retval == -EACCES)
                retval = -EAGAIN;

-       if (retval != -EAGAIN && retval != -EBUSY)
-               dev->power.runtime_error = retval;
-
        return retval;
 }

@@ -751,6 +748,9 @@ static int rpm_suspend(struct device *dev, int rpmflags)
        dev->power.deferred_resume = false;
        wake_up_all(&dev->power.wait_queue);

+       if (retval != -EAGAIN && retval != -EBUSY)
+               dev->power.runtime_error = retval;

+


Thanks,

Praveen Talari