| Message ID | 12620037.O9o76ZdvQC@rjwysocki.net (mailing list archive) |
|---|---|
| State | Queued |
| Delegated to: | Rafael Wysocki |
| Headers | show |
| Series | [v2] PM: runtime: Unify error handling during suspend and resume | expand |
On Tue, 25 Feb 2025 at 18:06, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > There is a confusing difference in error handling between rpm_suspend() > and rpm_resume() related to the special way in which -EAGAIN and -EBUSY > error values are treated by the former. Also, converting -EACCES coming > from the callback to I/O error, which it quite likely is not, may > confuse runtime PM users. > > To address the above, modify rpm_callback() to convert -EACCES coming > from the driver to -EAGAIN and to set power.runtime_error only if the > return value is not -EAGAIN or -EBUSY. > > This will cause the error handling in rpm_resume() and rpm_suspend() to > work consistently, so drop the no longer needed -EAGAIN or -EBUSY > special case from the latter and make it retry autosuspend if > power.runtime_error is unset. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Seems reasonable to me! Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> Kind regards Uffe > --- > > v1 -> v2: Add comment explaining the -EACCES error code conversion (Raag) > > --- > drivers/base/power/runtime.c | 40 ++++++++++++++++++++++++---------------- > 1 file changed, 24 insertions(+), 16 deletions(-) > > --- a/drivers/base/power/runtime.c > +++ b/drivers/base/power/runtime.c > @@ -448,8 +448,19 @@ > retval = __rpm_callback(cb, dev); > } > > - dev->power.runtime_error = retval; > - return retval != -EACCES ? retval : -EIO; > + /* > + * Since -EACCES means that runtime PM is disabled for the given device, > + * it should not be returned by runtime PM callbacks. If it is returned > + * nevertheless, assume it to be a transient error and convert it to > + * -EAGAIN. > + */ > + if (retval == -EACCES) > + retval = -EAGAIN; > + > + if (retval != -EAGAIN && retval != -EBUSY) > + dev->power.runtime_error = retval; > + > + return retval; > } > > /** > @@ -725,21 +736,18 @@ > dev->power.deferred_resume = false; > wake_up_all(&dev->power.wait_queue); > > - if (retval == -EAGAIN || retval == -EBUSY) { > - dev->power.runtime_error = 0; > + /* > + * On transient errors, if the callback routine failed an autosuspend, > + * and if the last_busy time has been updated so that there is a new > + * autosuspend expiration time, automatically reschedule another > + * autosuspend. > + */ > + if (!dev->power.runtime_error && (rpmflags & RPM_AUTO) && > + pm_runtime_autosuspend_expiration(dev) != 0) > + goto repeat; > + > + pm_runtime_cancel_pending(dev); > > - /* > - * If the callback routine failed an autosuspend, and > - * if the last_busy time has been updated so that there > - * is a new autosuspend expiration time, automatically > - * reschedule another autosuspend. > - */ > - if ((rpmflags & RPM_AUTO) && > - pm_runtime_autosuspend_expiration(dev) != 0) > - goto repeat; > - } else { > - pm_runtime_cancel_pending(dev); > - } > goto out; > } > > > >
--- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -448,8 +448,19 @@ retval = __rpm_callback(cb, dev); } - dev->power.runtime_error = retval; - return retval != -EACCES ? retval : -EIO; + /* + * Since -EACCES means that runtime PM is disabled for the given device, + * it should not be returned by runtime PM callbacks. If it is returned + * nevertheless, assume it to be a transient error and convert it to + * -EAGAIN. + */ + if (retval == -EACCES) + retval = -EAGAIN; + + if (retval != -EAGAIN && retval != -EBUSY) + dev->power.runtime_error = retval; + + return retval; } /** @@ -725,21 +736,18 @@ dev->power.deferred_resume = false; wake_up_all(&dev->power.wait_queue); - if (retval == -EAGAIN || retval == -EBUSY) { - dev->power.runtime_error = 0; + /* + * On transient errors, if the callback routine failed an autosuspend, + * and if the last_busy time has been updated so that there is a new + * autosuspend expiration time, automatically reschedule another + * autosuspend. + */ + if (!dev->power.runtime_error && (rpmflags & RPM_AUTO) && + pm_runtime_autosuspend_expiration(dev) != 0) + goto repeat; + + pm_runtime_cancel_pending(dev); - /* - * If the callback routine failed an autosuspend, and - * if the last_busy time has been updated so that there - * is a new autosuspend expiration time, automatically - * reschedule another autosuspend. - */ - if ((rpmflags & RPM_AUTO) && - pm_runtime_autosuspend_expiration(dev) != 0) - goto repeat; - } else { - pm_runtime_cancel_pending(dev); - } goto out; }