On Tue, Sep 23, 2025 at 10:53 AM Dhruva Gole <d-gole@xxxxxx> wrote: > > On Sep 22, 2025 at 17:30:43 +0200, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> > > > > It is generally useful to be able to automatically drop a device's > > runtime PM usage counter incremented by runtime PM operations that > > resume a device and bump up its usage counter [1]. > > > > To that end, add DEFINE_CLASS() macros allowing pm_runtime_put() > > and pm_runtime_put_autosuspend() to be used for the auto-cleanup in > > those cases. > > > > Simply put, a piece of code like below: > > > > pm_runtime_get_sync(dev); > > ..... > > pm_runtime_put(dev); > > return 0; > > > > can be transformed with CLASS(pm_runtime_get_sync) like: > > > > guard(pm_runtime_get_sync)(dev); > > ..... > > return 0; > > > > (see pm_runtime_put() call is gone). > > > > However, it is better to do proper error handling in the majority of > > cases, so doing something like this instead of the above is recommended: > > > > CLASS(pm_runtime_get_active, pm)(dev); > > if (IS_ERR(pm)) > > return PTR_ERR(pm); > > ..... > > return 0; > > > > In all of the cases in which runtime PM is known to be enabled for the > > given device or the device can be regarded as operational (and so it can > > be accessed) with runtime PM disabled, a piece of code like: > > > > ret = pm_runtime_resume_and_get(dev); > > if (ret < 0) > > return ret; > > ..... > > pm_runtime_put(dev); > > return 0; > > > > can be simplified with CLASS() like: > > > > CLASS(pm_runtime_get_active, pm)(dev); > > if (IS_ERR(pm)) > > return PTR_ERR(pm); > > ..... > > return 0; > > > > (again, see pm_runtime_put() call is gone). > > > > Still, if the device cannot be accessed unless runtime PM has been > > enabled for it, the CLASS(pm_runtime_get_active_enabled) variant > > needs to be used, that is (in the context of the example above): > > > > CLASS(pm_runtime_get_active_enabled, pm)(dev); > > if (IS_ERR(pm)) > > return PTR_ERR(pm); > > ..... > > return 0; > > > > When the original code calls pm_runtime_put_autosuspend(), use one > > of the "auto" class variants, CLASS(pm_runtime_get_active_auto) or > > CLASS(pm_runtime_get_active_enabled_auto), so for example, a piece > > of code like: > > > > ret = pm_runtime_resume_and_get(dev); > > if (ret < 0) > > return ret; > > ..... > > pm_runtime_put_autosuspend(dev); > > return 0; > > > > will become: > > > > CLASS(pm_runtime_get_active_enabled_auto, pm)(dev); > > if (IS_ERR(pm)) > > return PTR_ERR(pm); > > ..... > > return 0; > > > > Note that the cases in which the return value of pm_runtime_get_sync() > > is checked can also be handled with the help of the new class macros. > > For example, a piece of code like: > > > > ret = pm_runtime_get_sync(dev); > > if (ret < 0) { > > pm_runtime_put(dev); > > return ret; > > } > > ..... > > pm_runtime_put(dev); > > return 0; > > > > can be rewritten as: > > > > CLASS(pm_runtime_get_active_enabled, pm)(dev); > > if (IS_ERR(pm)) > > return PTR_ERR(pm); > > ..... > > return 0; > > > > or CLASS(pm_runtime_get_active) can be used if transparent handling of > > disabled runtime PM is desirable. > > > > Firstly, please can we add all this documentation in runtime_pm [1] > Otherwise there's just far less developers aware of the new APIs getting > introduced other than people directly involved. Not everyone is going to > come down here to look at git log for API docs (even though we proud > ourselves in having git log as our main source of Documentation in > kernel ;) ) > > [1] https://docs.kernel.org/power/runtime_pm.html That will happen when the early adopters tell me that it works for them. > > Link: https://lore.kernel.org/linux-pm/878qimv24u.wl-tiwai@xxxxxxx/ [1] > > Co-developed-by: Takashi Iwai <tiwai@xxxxxxx> > > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx> > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> > > --- > > > > v2 -> v3: > > * Two more class definitions for the case in which resume errors can be > > neglected. > > * Update of new code comments (for more clarity). > > * Changelog update. > > > > v1 -> v2: > > * Rename the new classes and the new static inline helper. > > * Add two classes for handling disabled runtime PM. > > * Expand the changelog. > > * Adjust the subject. > > > > --- > > drivers/base/power/runtime.c | 2 + > > include/linux/pm_runtime.h | 82 +++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 84 insertions(+) > > > > > --- a/drivers/base/power/runtime.c > > +++ b/drivers/base/power/runtime.c > > @@ -796,6 +796,8 @@ static int rpm_resume(struct device *dev > > if (dev->power.runtime_status == RPM_ACTIVE && > > dev->power.last_status == RPM_ACTIVE) > > retval = 1; > > + else if (rpmflags & RPM_TRANSPARENT) > > + goto out; > > "TRANSPARENT" doesn't tell you exactly what happens. It should be something like > RPM_IGNORE_DISABLED or RPM_ALLOW_DISABLED IMO. There is a description in the header file and TRANSPARENT is shorter than the alternatives. Besides, this is mostly for internal use. > > else > > retval = -EACCES; > > } > > --- a/include/linux/pm_runtime.h > > +++ b/include/linux/pm_runtime.h > > @@ -21,6 +21,7 @@ > > #define RPM_GET_PUT 0x04 /* Increment/decrement the > > usage_count */ > > #define RPM_AUTO 0x08 /* Use autosuspend_delay */ > > +#define RPM_TRANSPARENT 0x10 /* Succeed if runtime PM is disabled */ > > > > /* > > * Use this for defining a set of PM operations to be used in all situations > > @@ -533,6 +534,32 @@ static inline int pm_runtime_resume_and_ > > } > > > > /** > > + * pm_runtime_get_active_dev - Resume a device and bump up its usage counter. > > I am getting no clue as to why this is different than regular > pm_runtime_get_sync then? Can we describe this API better? Again, this is for internal use. I may as well drop the kerneldoc for this function altogether, but I'm not going to send a v4 just for this purpose.