Re: [PATCH v4 2/6] PM: runtime: Add pm_runtime_if_active guard and conditional variant

["Golla, Nagendra" <Nagendra.Golla@amd.com>]

Hi Rafael,

On 10/13/2026 12:14 AM, Rafael J. Wysocki (Intel) wrote:
> On Fri, Oct 9, 2026 at 6:00 AM Golla Nagendra <nagendra.golla@amd.com> wrote:
>>
>> Add a pm_runtime_if_active base guard and its _try conditional variant
>> to pm_runtime.h for drivers that need to conditionally acquire a runtime
>> PM reference only when the device is already active.
>>
>> The base guard must not be used directly via guard()/scoped_guard()
>> because pm_runtime_get_if_active() only acquires a reference when it
>> returns 1; the destructor unconditionally calls pm_runtime_put(), which
>> would underflow usage_count on a suspended device. The base guard
>> exists solely to back the DEFINE_GUARD_COND _try variant.
>>
>> The _try variant (used via PM_RUNTIME_ACQUIRE_IF_ACTIVE) satisfies the
>> _try API contract that guards must not produce an error when runtime PM
>> is disabled for the device. When pm_runtime_get_if_active() returns
>> -EINVAL (runtime PM disabled), the expression calls
>> pm_runtime_get_noresume() to pre-balance the destructor's unconditional
>> pm_runtime_put(), then returns 1 so the guard condition succeeds
>> (bypass). This mirrors the behaviour of pm_runtime_active_try which
>> uses RPM_TRANSPARENT for the same purpose. When the device is genuinely
>> runtime-suspended (return value 0), the guard condition fails and
>> ACQUIRE_ERR returns true, allowing callers such as interrupt handlers to
>> skip MMIO accesses.
>>
>> Signed-off-by: Golla Nagendra <nagendra.golla@amd.com>
>> ---
>>   include/linux/pm_runtime.h | 27 +++++++++++++++++++++++++++
>>   1 file changed, 27 insertions(+)
>>
>> diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
>> index 64921b10ac74..ae2a8740ffb3 100644
>> --- a/include/linux/pm_runtime.h
>> +++ b/include/linux/pm_runtime.h
>> @@ -609,6 +609,13 @@ DEFINE_GUARD(pm_runtime_active, struct device *,
>>               pm_runtime_get_sync(_T), pm_runtime_put(_T));
>>   DEFINE_GUARD(pm_runtime_active_auto, struct device *,
>>               pm_runtime_get_sync(_T), pm_runtime_put_autosuspend(_T));
>> +/*
>> + * Do not use directly -- the destructor calls pm_runtime_put()
>> + * unconditionally, which underflows if no reference was acquired.
>> + * Use only via the _try variant below.
>> + */
>> +DEFINE_GUARD(pm_runtime_if_active, struct device *,
>> +            pm_runtime_get_if_active(_T), pm_runtime_put(_T));
>>   /*
>>    * Use the following guards with ACQUIRE()/ACQUIRE_ERR().
>>    *
>> @@ -624,6 +631,23 @@ DEFINE_GUARD_COND(pm_runtime_active_auto, _try,
>>                    pm_runtime_get_active(_T, RPM_TRANSPARENT), _RET == 0)
>>   DEFINE_GUARD_COND(pm_runtime_active_auto, _try_enabled,
>>                    pm_runtime_resume_and_get(_T), _RET == 0)
>> +/*
>> + * The _try contract is that no error is produced when runtime PM is disabled,
>> + * so bypass the guard when pm_runtime_get_if_active() returns -EINVAL: take a
>> + * reference with pm_runtime_get_noresume() to pre-balance the destructor's
>> + * pm_runtime_put() and return 1 so the guard condition succeeds. Interrupt
>> + * handlers then keep servicing interrupts when runtime PM is disabled for the
>> + * device or CONFIG_PM is not enabled.
>> + */
>> +DEFINE_GUARD_COND(pm_runtime_if_active, _try,
>> +                 ({
>> +                       int _r = pm_runtime_get_if_active(_T);
>> +
>> +                       if (_r == -EINVAL)
>> +                               pm_runtime_get_noresume(_T);
>> +                       _r == -EINVAL ? 1 : (_r ?: -EAGAIN);
>> +                 }),
>> +                 _RET == 1)
>>
>>   /* ACQUIRE() wrapper macros for the guards defined above. */
>>
>> @@ -639,6 +663,9 @@ DEFINE_GUARD_COND(pm_runtime_active_auto, _try_enabled,
>>   #define PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND(_dev, _var)  \
>>          ACQUIRE(pm_runtime_active_auto_try_enabled, _var)(_dev)
>>
>> +#define PM_RUNTIME_ACQUIRE_IF_ACTIVE(_dev, _var)       \
>> +       ACQUIRE(pm_runtime_if_active_try, _var)(_dev)
>> +
>>   /*
>>    * ACQUIRE_ERR() wrapper macro for guard pm_runtime_active.
>>    *
>>
>> --
> 
> For the record, I'm still unconvinced about the guard definitions above.
> 
> Please try to reimplement the series without them for the time being.
> 
> Thanks!

Thanks, for the review

Sure — I’ll drop these two patches from the next version (v5) and
re-implement the series without the PM guard helpers for now. I’ll keep
the reset-related patches in v5,and discuss the PM guard work in a 
separate series later.

Thanks,
Nagendra