| Message ID | 20250410153106.4146265-5-sakari.ailus@linux.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Update last busy timestamp in Runtime PM autosuspend callbacks | expand |
Hi Sakari, Thank you for the patch. On Thu, Apr 10, 2025 at 06:31:03PM +0300, Sakari Ailus wrote: > Set device's last busy timestamp to current time in > pm_runtime_put_sync_autosuspend(). > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> I was a bit puzzled by why this function exists. Reading Documentation/power/runtime_pm.rst answered that question: if I understand it correctly, the function is meant to be used by code that doesn't know whether or not autosuspend has been enabled for a device, such as core code in subsystems. I looked at usage patterns, and found the function being used in drivers as well, for instance in drivers/media/i2c/tc358746.c. Given that the driver unconditionally enabled autosuspend, is this incorrect usage of the API ? > --- > Documentation/power/runtime_pm.rst | 3 ++- > include/linux/pm_runtime.h | 11 +++++++---- > 2 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/Documentation/power/runtime_pm.rst b/Documentation/power/runtime_pm.rst > index e7bbdc66d64c..9c21c913f9cf 100644 > --- a/Documentation/power/runtime_pm.rst > +++ b/Documentation/power/runtime_pm.rst > @@ -428,7 +428,8 @@ drivers/base/power/runtime.c and include/linux/pm_runtime.h: > pm_runtime_suspend(dev) and return its result > > `int pm_runtime_put_sync_autosuspend(struct device *dev);` > - - decrement the device's usage counter; if the result is 0 then run > + - set the power.last_busy field to the current time and decrement the > + device's usage counter; if the result is 0 then run > pm_runtime_autosuspend(dev) and return its result > > `void pm_runtime_enable(struct device *dev);` > diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h > index 0ade3f75d903..e26caf2c0552 100644 > --- a/include/linux/pm_runtime.h > +++ b/include/linux/pm_runtime.h > @@ -645,12 +645,14 @@ static inline int pm_runtime_put_sync_suspend(struct device *dev) > } > > /** > - * pm_runtime_put_sync_autosuspend - Drop device usage counter and autosuspend if 0. > + * pm_runtime_put_sync_autosuspend - Update the last access time of a device, > + * drop device usage counter and autosuspend if 0. > * @dev: Target device. > * > - * Decrement the runtime PM usage counter of @dev and if it turns out to be > - * equal to 0, set up autosuspend of @dev or suspend it synchronously (depending > - * on whether or not autosuspend has been enabled for it). > + * Update the last access time of @dev, decrement the runtime PM usage counter > + * of @dev and if it turns out to be equal to 0, set up autosuspend of @dev or > + * suspend it synchronously (depending on whether or not autosuspend has been > + * enabled for it). > * > * The runtime PM usage counter of @dev remains decremented in all cases, even > * if it returns an error code. > @@ -670,6 +672,7 @@ static inline int pm_runtime_put_sync_suspend(struct device *dev) > */ > static inline int pm_runtime_put_sync_autosuspend(struct device *dev) > { > + pm_runtime_mark_last_busy(dev); > return __pm_runtime_suspend(dev, RPM_GET_PUT | RPM_AUTO); > } >
diff --git a/Documentation/power/runtime_pm.rst b/Documentation/power/runtime_pm.rst index e7bbdc66d64c..9c21c913f9cf 100644 --- a/Documentation/power/runtime_pm.rst +++ b/Documentation/power/runtime_pm.rst @@ -428,7 +428,8 @@ drivers/base/power/runtime.c and include/linux/pm_runtime.h: pm_runtime_suspend(dev) and return its result `int pm_runtime_put_sync_autosuspend(struct device *dev);` - - decrement the device's usage counter; if the result is 0 then run + - set the power.last_busy field to the current time and decrement the + device's usage counter; if the result is 0 then run pm_runtime_autosuspend(dev) and return its result `void pm_runtime_enable(struct device *dev);` diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h index 0ade3f75d903..e26caf2c0552 100644 --- a/include/linux/pm_runtime.h +++ b/include/linux/pm_runtime.h @@ -645,12 +645,14 @@ static inline int pm_runtime_put_sync_suspend(struct device *dev) } /** - * pm_runtime_put_sync_autosuspend - Drop device usage counter and autosuspend if 0. + * pm_runtime_put_sync_autosuspend - Update the last access time of a device, + * drop device usage counter and autosuspend if 0. * @dev: Target device. * - * Decrement the runtime PM usage counter of @dev and if it turns out to be - * equal to 0, set up autosuspend of @dev or suspend it synchronously (depending - * on whether or not autosuspend has been enabled for it). + * Update the last access time of @dev, decrement the runtime PM usage counter + * of @dev and if it turns out to be equal to 0, set up autosuspend of @dev or + * suspend it synchronously (depending on whether or not autosuspend has been + * enabled for it). * * The runtime PM usage counter of @dev remains decremented in all cases, even * if it returns an error code. @@ -670,6 +672,7 @@ static inline int pm_runtime_put_sync_suspend(struct device *dev) */ static inline int pm_runtime_put_sync_autosuspend(struct device *dev) { + pm_runtime_mark_last_busy(dev); return __pm_runtime_suspend(dev, RPM_GET_PUT | RPM_AUTO); }
Set device's last busy timestamp to current time in pm_runtime_put_sync_autosuspend(). Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> --- Documentation/power/runtime_pm.rst | 3 ++- include/linux/pm_runtime.h | 11 +++++++---- 2 files changed, 9 insertions(+), 5 deletions(-)