| Message ID | 94fe944c-c528-9459-fc75-7c94273dd2b1@gmail.com (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
| Series | PM: runtime: add RPM_IDLE_SUSPEND / RPM_IDLE_NO_SUSPEND constants | expand |
On Sat, May 30, 2020 at 6:33 PM Heiner Kallweit <hkallweit1@gmail.com> wrote: > > runtime_idle() callback implementations have to return a non-zero value > if they don't intend to suspend now. Several drivers use an errno like > -EBUSY for this purpose. This can be problematic because the return > value is propagated up the call chain, from rpm_idle() to > __pm_runtime_idle(), and from there to callers like > pm_runtime_put_sync(). A driver author checking the return value of > e.g. pm_runtime_put_sync() may as usual check for retval < 0 and > bail out. Which would be a bug anyway, because rpm_idle() may return -EAGAIN or -EINPROGRESS due to concurrency and -EBUSY should be treated similarly. > Therefore a positive value should be returned. While it is recommended to return a positive value then, returning -EBUSY or -EAGAIN should still work if the callers are careful enough (and they should be). Thanks!
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index 9f62790f6..4f529075e 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -453,7 +453,11 @@ static int rpm_idle(struct device *dev, int rpmflags) out: trace_rpm_return_int_rcuidle(dev, _THIS_IP_, retval); - return retval ? retval : rpm_suspend(dev, rpmflags | RPM_AUTO); + + if (retval == RPM_IDLE_SUSPEND) + return rpm_suspend(dev, rpmflags | RPM_AUTO); + + return retval; } /** diff --git a/include/linux/pm.h b/include/linux/pm.h index 121c104a4..971ed3d77 100644 --- a/include/linux/pm.h +++ b/include/linux/pm.h @@ -227,8 +227,9 @@ typedef struct pm_message { * * @runtime_idle: Device appears to be inactive and it might be put into a * low-power state if all of the necessary conditions are satisfied. - * Check these conditions, and return 0 if it's appropriate to let the PM - * core queue a suspend request for the device. + * Check these conditions, and return RPM_IDLE_SUSPEND if it's + * appropriate to let the PM core queue a suspend request for the device. + * Return RPM_IDLE_NO_SUSPEND if you don't want to suspend now. * * Several device power state transitions are externally visible, affecting * the state of pending I/O queues and (for drivers that touch hardware) @@ -523,6 +524,11 @@ enum rpm_request { RPM_REQ_RESUME, }; +enum rpm_idle { + RPM_IDLE_SUSPEND = 0, + RPM_IDLE_NO_SUSPEND, +}; + struct wakeup_source; struct wake_irq; struct pm_domain_data;
runtime_idle() callback implementations have to return a non-zero value if they don't intend to suspend now. Several drivers use an errno like -EBUSY for this purpose. This can be problematic because the return value is propagated up the call chain, from rpm_idle() to __pm_runtime_idle(), and from there to callers like pm_runtime_put_sync(). A driver author checking the return value of e.g. pm_runtime_put_sync() may as usual check for retval < 0 and bail out. Therefore a positive value should be returned. To facilitate this add constants RPM_IDLE_SUSPEND and RPM_IDLE_NO_SUSPEND. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/base/power/runtime.c | 6 +++++- include/linux/pm.h | 10 ++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-)