| Message ID | 20250410153106.4146265-5-sakari.ailus@linux.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Update last busy timestamp in Runtime PM autosuspend callbacks | expand |
Hi Laurent, On Thu, Apr 10, 2025 at 11:23:18PM +0300, Laurent Pinchart wrote: > 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 ? The documentation (Documentation/power/runtime_pm.rst, section 9) appears to say that functions should be used instead of pm_runtime_put_sync() if the driver has enabled autosuspend. I wonder if the documentation should be changed, to tell to use pm_runtime_put_sync() instead, for pm_runtime_put_sync_autosuspend() indeed is effectively pm_runtime_put_autosuspend() if autosuspend has been enabled. I wonder what Rafael thinks.
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(-)