[PATCH v1 0/3] PM: runtime: Wrapper macros for usage counter guards

[PATCH v1 0/3] PM: runtime: Wrapper macros for usage counter guards

[Rafael J. Wysocki]

Hi All,

The runtime PM usage counter guards introduced recently:

https://lore.kernel.org/linux-pm/6196611.lOV4Wx5bFT@rafael.j.wysocki/

and then fixed:

https://lore.kernel.org/linux-pm/5943878.DvuYhMxLoT@rafael.j.wysocki/

should generally work, but using them feels sort of arcane and cryptic
even though the underlying concept is relatively straightforward.

For this reason, runtime PM wrapper macros around ACQUIRE() and
ACQUIRE_ERR() involving the new guards are introduced in this series
(patch [1/3]) and then used in the code already using the guards (patches
[2/3] and [3/3]) to make it look more straightforward.

Thanks!

[PATCH v1 1/3] PM: runtime: Wrapper macros for ACQUIRE()/ACQUIRE_ERR()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Add several wrapper macros for ACQUIRE()/ACQUIRE_ERR() and runtime PM
usage counter guards introduced recently: pm_runtime_active_try,
pm_runtime_active_auto_try, pm_runtime_active_try_enabled, and
pm_runtime_active_auto_try_enabled.

The new macros are simpler and should be more straightforward to use.
Moreover, they do not expose internal details that are not strictly
related to the code using the macros.

For example, they can be used for rewriting a piece of code like below:

        ACQUIRE(pm_runtime_active_try, pm)(dev);
        if ((ret = ACQUIRE_ERR(pm_runtime_active_try, &pm)))
                return ret;

in the following way:

        PM_RUNTIME_ACQUIRE(dev);
        if ((ret = PM_RUNTIME_ACQUIRE_ERR))
                return ret;

If the original code does not care about the specific error code
returned when attempting to resume the device:

        ACQUIRE(pm_runtime_active_try, pm)(dev);
        if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
                return -ENXIO;

it may be changed like this:

        PM_RUNTIME_ACQUIRE(dev);
        if (PM_RUNTIME_ACQUIRE_ERR)
                return -ENXIO;

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 include/linux/pm_runtime.h |   55 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -637,6 +637,61 @@ DEFINE_GUARD_COND(pm_runtime_active_auto
 DEFINE_GUARD_COND(pm_runtime_active_auto, _try_enabled,
 		  pm_runtime_resume_and_get(_T), _RET == 0)
 
+/*
+ * ACQUIRE() wrapper macros for the guards defined above.
+ *
+ * The tagged __PM_RUNTIME_ACQUIRE*() variants are for the cases in which two or
+ * more of these macros are used in the same scope and the tags are necessary to
+ * distinguish the internal guard variables from each other. Don't do that
+ * unless you have to. No, really. If they are needed, using simple tags is
+ * recommended (for example, individual digits or letters).
+ *
+ * The simpler PM_RUNTIME_ACQUIRE*() variants are wrappers around the
+ * corresponding __PM_RUNTIME_ACQUIRE*() that use the underline character
+ * as a (special) tag.  They should be suitable for the vast majority of use
+ * cases.
+ *
+ * Don't mix up PM_RUNTIME_ACQUIRE*() with __PM_RUNTIME_ACQUIRE*() even though
+ * that may work.
+ */
+#define __PM_RUNTIME_ACQUIRE(dev, tag)	\
+	ACQUIRE(pm_runtime_active_try, _pm_runtime_guard_var_##tag)(dev)
+
+#define PM_RUNTIME_ACQUIRE(dev)	\
+	__PM_RUNTIME_ACQUIRE(dev, _)
+
+#define __PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, tag)	\
+	ACQUIRE(pm_runtime_active_auto_try, _pm_runtime_guard_var_##tag)(dev)
+
+#define PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev)	\
+	__PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, _)
+
+#define __PM_RUNTIME_ACQUIRE_ENABLED(dev, tag)	\
+	ACQUIRE(pm_runtime_active_try_enabled, _pm_runtime_guard_var_##tag)(dev)
+
+#define PM_RUNTIME_ACQUIRE_ENABLED(dev)	\
+	__PM_RUNTIME_ACQUIRE_ENABLED(dev, _)
+
+#define __PM_RUNTIME_ACQUIRE_ENABLED_AUTOSUSPEND(dev, tag)	\
+	ACQUIRE(pm_runtime_active_auto_try_enabled, _pm_runtime_guard_var_##tag)(dev)
+
+#define PM_RUNTIME_ACQUIRE_ENABLED_AUTOSUSPEND(dev)	\
+	__PM_RUNTIME_ACQUIRE_ENABLED_AUTOSUSPEND(dev, _)
+
+/*
+ * ACQUIRE_ERR() wrapper macros for guard pm_runtime_active.
+ *
+ * Always check __PM_RUNTIME_ACQUIRE_ERR() with a matching tag after using one
+ * of the tagged __PM_RUNTIME_ACQUIRE*() macros defined above (yes, it can be
+ * used with any of them) and avoid accessing the given device if it is nonzero.
+ * Analogously, always check PM_RUNTIME_ACQUIRE_ERR after using any of the
+ * simpler PM_RUNTIME_ACQUIRE*() macros.
+ */
+#define __PM_RUNTIME_ACQUIRE_ERR(tag)	\
+	ACQUIRE_ERR(pm_runtime_active, &_pm_runtime_guard_var_##tag)
+
+#define PM_RUNTIME_ACQUIRE_ERR	__PM_RUNTIME_ACQUIRE_ERR(_)
+
 /**
  * pm_runtime_put_sync - Drop device usage counter and run "idle check" if 0.
  * @dev: Target device.

[PATCH v1 2/3] PCI/sysfs: Use PM_RUNTIME_ACQUIRE()/PM_RUNTIME_ACQUIRE_ERR

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Use new PM_RUNTIME_ACQUIRE() and PM_RUNTIME_ACQUIRE_ERR wrapper macros
to make the code look more straightforward.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/pci/pci-sysfs.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -1517,8 +1517,8 @@ static ssize_t reset_method_store(struct
 		return count;
 	}
 
-	ACQUIRE(pm_runtime_active_try, pm)(dev);
-	if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
+	PM_RUNTIME_ACQUIRE(dev);
+	if (PM_RUNTIME_ACQUIRE_ERR)
 		return -ENXIO;
 
 	if (sysfs_streq(buf, "default")) {

[PATCH v1 3/3] ACPI: TAD: Use PM_RUNTIME_ACQUIRE()/PM_RUNTIME_ACQUIRE_ERR

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Use new PM_RUNTIME_ACQUIRE() and PM_RUNTIME_ACQUIRE_ERR wrapper macros
to make the code look more straightforward.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_tad.c |   24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

--- a/drivers/acpi/acpi_tad.c
+++ b/drivers/acpi/acpi_tad.c
@@ -90,8 +90,8 @@ static int acpi_tad_set_real_time(struct
 	args[0].buffer.pointer = (u8 *)rt;
 	args[0].buffer.length = sizeof(*rt);
 
-	ACQUIRE(pm_runtime_active_try, pm)(dev);
-	if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
+	PM_RUNTIME_ACQUIRE(dev);
+	if (PM_RUNTIME_ACQUIRE_ERR)
 		return -ENXIO;
 
 	status = acpi_evaluate_integer(handle, "_SRT", &arg_list, &retval);
@@ -137,8 +137,8 @@ static int acpi_tad_get_real_time(struct
 {
 	int ret;
 
-	ACQUIRE(pm_runtime_active_try, pm)(dev);
-	if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
+	PM_RUNTIME_ACQUIRE(dev);
+	if (PM_RUNTIME_ACQUIRE_ERR)
 		return -ENXIO;
 
 	ret = acpi_tad_evaluate_grt(dev, rt);
@@ -275,8 +275,8 @@ static int acpi_tad_wake_set(struct devi
 	args[0].integer.value = timer_id;
 	args[1].integer.value = value;
 
-	ACQUIRE(pm_runtime_active_try, pm)(dev);
-	if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
+	PM_RUNTIME_ACQUIRE(dev);
+	if (PM_RUNTIME_ACQUIRE_ERR)
 		return -ENXIO;
 
 	status = acpi_evaluate_integer(handle, method, &arg_list, &retval);
@@ -322,8 +322,8 @@ static ssize_t acpi_tad_wake_read(struct
 
 	args[0].integer.value = timer_id;
 
-	ACQUIRE(pm_runtime_active_try, pm)(dev);
-	if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
+	PM_RUNTIME_ACQUIRE(dev);
+	if (PM_RUNTIME_ACQUIRE_ERR)
 		return -ENXIO;
 
 	status = acpi_evaluate_integer(handle, method, &arg_list, &retval);
@@ -377,8 +377,8 @@ static int acpi_tad_clear_status(struct
 
 	args[0].integer.value = timer_id;
 
-	ACQUIRE(pm_runtime_active_try, pm)(dev);
-	if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
+	PM_RUNTIME_ACQUIRE(dev);
+	if (PM_RUNTIME_ACQUIRE_ERR)
 		return -ENXIO;
 
 	status = acpi_evaluate_integer(handle, "_CWS", &arg_list, &retval);
@@ -417,8 +417,8 @@ static ssize_t acpi_tad_status_read(stru
 
 	args[0].integer.value = timer_id;
 
-	ACQUIRE(pm_runtime_active_try, pm)(dev);
-	if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
+	PM_RUNTIME_ACQUIRE(dev);
+	if (PM_RUNTIME_ACQUIRE_ERR)
 		return -ENXIO;
 
 	status = acpi_evaluate_integer(handle, "_GWS", &arg_list, &retval);

Re: [PATCH v1 0/3] PM: runtime: Wrapper macros for usage counter guards

[Jonathan Cameron]

On Fri, 07 Nov 2025 19:35:09 +0100
"Rafael J. Wysocki" <rafael@kernel.org> wrote:

> Hi All,
> 
> The runtime PM usage counter guards introduced recently:
> 
> https://lore.kernel.org/linux-pm/6196611.lOV4Wx5bFT@rafael.j.wysocki/
> 
> and then fixed:
> 
> https://lore.kernel.org/linux-pm/5943878.DvuYhMxLoT@rafael.j.wysocki/
> 
> should generally work, but using them feels sort of arcane and cryptic
> even though the underlying concept is relatively straightforward.
> 
> For this reason, runtime PM wrapper macros around ACQUIRE() and
> ACQUIRE_ERR() involving the new guards are introduced in this series
> (patch [1/3]) and then used in the code already using the guards (patches
> [2/3] and [3/3]) to make it look more straightforward.
> 
> Thanks!

It's an interesting trade off between completely hiding the magic variables
and verbosity.  The PM_RUNTIME_ACQUIRE_ERR smells like something we'd
expect to be using global state (given no parameters), but it is of
course just local with a magic name.  Will take a little getting
used to but then so does all this cleanup.h magic.  So on balance
I think this is a good change.

Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

Re: [PATCH v1 1/3] PM: runtime: Wrapper macros for ACQUIRE()/ACQUIRE_ERR()

[Frank Li]

On Fri, Nov 07, 2025 at 07:39:55PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Add several wrapper macros for ACQUIRE()/ACQUIRE_ERR() and runtime PM
> usage counter guards introduced recently: pm_runtime_active_try,
> pm_runtime_active_auto_try, pm_runtime_active_try_enabled, and
> pm_runtime_active_auto_try_enabled.
>
> The new macros are simpler and should be more straightforward to use.
> Moreover, they do not expose internal details that are not strictly
> related to the code using the macros.
>
> For example, they can be used for rewriting a piece of code like below:
>
>         ACQUIRE(pm_runtime_active_try, pm)(dev);
>         if ((ret = ACQUIRE_ERR(pm_runtime_active_try, &pm)))
>                 return ret;
>
> in the following way:
>
>         PM_RUNTIME_ACQUIRE(dev);
>         if ((ret = PM_RUNTIME_ACQUIRE_ERR))

Personally, I feel like PM_RUNTIME_ACQUIRE_ERR hide too much informaiton.

There are not clear connection between PM_RUNTIME_ACQUIRE and
PM_RUNTIME_ACQUIRE_ERR. but previous code, the 'pm' is good connector.

Frank
>                 return ret;
>
> If the original code does not care about the specific error code
> returned when attempting to resume the device:
>
>         ACQUIRE(pm_runtime_active_try, pm)(dev);
>         if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
>                 return -ENXIO;
>
> it may be changed like this:
>
>         PM_RUNTIME_ACQUIRE(dev);
>         if (PM_RUNTIME_ACQUIRE_ERR)
>                 return -ENXIO;
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  include/linux/pm_runtime.h |   55 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
>
> --- a/include/linux/pm_runtime.h
> +++ b/include/linux/pm_runtime.h
> @@ -637,6 +637,61 @@ DEFINE_GUARD_COND(pm_runtime_active_auto
>  DEFINE_GUARD_COND(pm_runtime_active_auto, _try_enabled,
>  		  pm_runtime_resume_and_get(_T), _RET == 0)
>
> +/*
> + * ACQUIRE() wrapper macros for the guards defined above.
> + *
> + * The tagged __PM_RUNTIME_ACQUIRE*() variants are for the cases in which two or
> + * more of these macros are used in the same scope and the tags are necessary to
> + * distinguish the internal guard variables from each other. Don't do that
> + * unless you have to. No, really. If they are needed, using simple tags is
> + * recommended (for example, individual digits or letters).
> + *
> + * The simpler PM_RUNTIME_ACQUIRE*() variants are wrappers around the
> + * corresponding __PM_RUNTIME_ACQUIRE*() that use the underline character
> + * as a (special) tag.  They should be suitable for the vast majority of use
> + * cases.
> + *
> + * Don't mix up PM_RUNTIME_ACQUIRE*() with __PM_RUNTIME_ACQUIRE*() even though
> + * that may work.
> + */
> +#define __PM_RUNTIME_ACQUIRE(dev, tag)	\
> +	ACQUIRE(pm_runtime_active_try, _pm_runtime_guard_var_##tag)(dev)
> +
> +#define PM_RUNTIME_ACQUIRE(dev)	\
> +	__PM_RUNTIME_ACQUIRE(dev, _)
> +
> +#define __PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, tag)	\
> +	ACQUIRE(pm_runtime_active_auto_try, _pm_runtime_guard_var_##tag)(dev)
> +
> +#define PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev)	\
> +	__PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, _)
> +
> +#define __PM_RUNTIME_ACQUIRE_ENABLED(dev, tag)	\
> +	ACQUIRE(pm_runtime_active_try_enabled, _pm_runtime_guard_var_##tag)(dev)
> +
> +#define PM_RUNTIME_ACQUIRE_ENABLED(dev)	\
> +	__PM_RUNTIME_ACQUIRE_ENABLED(dev, _)
> +
> +#define __PM_RUNTIME_ACQUIRE_ENABLED_AUTOSUSPEND(dev, tag)	\
> +	ACQUIRE(pm_runtime_active_auto_try_enabled, _pm_runtime_guard_var_##tag)(dev)
> +
> +#define PM_RUNTIME_ACQUIRE_ENABLED_AUTOSUSPEND(dev)	\
> +	__PM_RUNTIME_ACQUIRE_ENABLED_AUTOSUSPEND(dev, _)
> +
> +/*
> + * ACQUIRE_ERR() wrapper macros for guard pm_runtime_active.
> + *
> + * Always check __PM_RUNTIME_ACQUIRE_ERR() with a matching tag after using one
> + * of the tagged __PM_RUNTIME_ACQUIRE*() macros defined above (yes, it can be
> + * used with any of them) and avoid accessing the given device if it is nonzero.
> + * Analogously, always check PM_RUNTIME_ACQUIRE_ERR after using any of the
> + * simpler PM_RUNTIME_ACQUIRE*() macros.
> + */
> +#define __PM_RUNTIME_ACQUIRE_ERR(tag)	\
> +	ACQUIRE_ERR(pm_runtime_active, &_pm_runtime_guard_var_##tag)
> +
> +#define PM_RUNTIME_ACQUIRE_ERR	__PM_RUNTIME_ACQUIRE_ERR(_)
> +
>  /**
>   * pm_runtime_put_sync - Drop device usage counter and run "idle check" if 0.
>   * @dev: Target device.
>
>
>

Re: [PATCH v1 0/3] PM: runtime: Wrapper macros for usage counter guards

[Dhruva Gole]

On Nov 07, 2025 at 19:35:09 +0100, Rafael J. Wysocki wrote:
> Hi All,
> 
> The runtime PM usage counter guards introduced recently:
> 
> https://lore.kernel.org/linux-pm/6196611.lOV4Wx5bFT@rafael.j.wysocki/
> 
> and then fixed:
> 
> https://lore.kernel.org/linux-pm/5943878.DvuYhMxLoT@rafael.j.wysocki/
> 
> should generally work, but using them feels sort of arcane and cryptic
> even though the underlying concept is relatively straightforward.
> 
> For this reason, runtime PM wrapper macros around ACQUIRE() and
> ACQUIRE_ERR() involving the new guards are introduced in this series
> (patch [1/3]) and then used in the code already using the guards (patches
> [2/3] and [3/3]) to make it look more straightforward.

The patches look okay to me,
Reviewed-by: Dhruva Gole <d-gole@ti.com>

-- 
Best regards,
Dhruva Gole
Texas Instruments Incorporated

Re: [PATCH v1 0/3] PM: runtime: Wrapper macros for usage counter guards

[Rafael J. Wysocki]

On Wednesday, November 12, 2025 7:39:41 AM CET Dhruva Gole wrote:
> On Nov 07, 2025 at 19:35:09 +0100, Rafael J. Wysocki wrote:
> > Hi All,
> > 
> > The runtime PM usage counter guards introduced recently:
> > 
> > https://lore.kernel.org/linux-pm/6196611.lOV4Wx5bFT@rafael.j.wysocki/
> > 
> > and then fixed:
> > 
> > https://lore.kernel.org/linux-pm/5943878.DvuYhMxLoT@rafael.j.wysocki/
> > 
> > should generally work, but using them feels sort of arcane and cryptic
> > even though the underlying concept is relatively straightforward.
> > 
> > For this reason, runtime PM wrapper macros around ACQUIRE() and
> > ACQUIRE_ERR() involving the new guards are introduced in this series
> > (patch [1/3]) and then used in the code already using the guards (patches
> > [2/3] and [3/3]) to make it look more straightforward.
> 
> The patches look okay to me,
> Reviewed-by: Dhruva Gole <d-gole@ti.com>

Thank you and Jonathan for the tags, but since Frank is not convinced, let me
bounce one more idea off all of you.

Namely, I think that Frank has a point when he wonders if PM_RUNTIME_ACQUIRE_ERR
hides too much information and I agree with Jonathan that may be misunderstood,
so what about defining the wrapper macros so they don't hide the guard variable
name, like in the patch below?

---
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Subject: [PATCH v2] PM: runtime: Wrapper macros for ACQUIRE()/ACQUIRE_ERR()

Add several wrapper macros for ACQUIRE()/ACQUIRE_ERR() and runtime PM
usage counter guards introduced recently: pm_runtime_active_try,
pm_runtime_active_auto_try, pm_runtime_active_try_enabled, and
pm_runtime_active_auto_try_enabled.

The new macros are simpler and should be more straightforward to use.

For example, they can be used for rewriting a piece of code like below:

        ACQUIRE(pm_runtime_active_try, pm)(dev);
        if ((ret = ACQUIRE_ERR(pm_runtime_active_try, &pm)))
                return ret;

in the following way:

        PM_RUNTIME_ACQUIRE(dev, pm);
        if ((ret = PM_RUNTIME_ACQUIRE_ERR(pm)))
                return ret;

If the original code does not care about the specific error code
returned when attempting to resume the device:

        ACQUIRE(pm_runtime_active_try, pm)(dev);
        if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
                return -ENXIO;

it may be changed like this:

        PM_RUNTIME_ACQUIRE(dev, pm);
        if (PM_RUNTIME_ACQUIRE_ERR(pm))
                return -ENXIO;

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 include/linux/pm_runtime.h |   25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -637,6 +637,31 @@ DEFINE_GUARD_COND(pm_runtime_active_auto
 DEFINE_GUARD_COND(pm_runtime_active_auto, _try_enabled,
 		  pm_runtime_resume_and_get(_T), _RET == 0)
 
+/* ACQUIRE() wrapper macros for the guards defined above. */
+
+#define PM_RUNTIME_ACQUIRE(dev, var_name)	\
+	ACQUIRE(pm_runtime_active_try, var_name)(dev)
+
+#define PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, var_name)	\
+	ACQUIRE(pm_runtime_active_auto_try, var_name)(dev)
+
+#define PM_RUNTIME_ACQUIRE_IF_ENABLED(dev, var_name)	\
+	ACQUIRE(pm_runtime_active_try_enabled, var_name)(dev)
+
+#define PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND(dev, var_name)	\
+	ACQUIRE(pm_runtime_active_auto_try_enabled, var_name)(dev)
+
+/*
+ * ACQUIRE_ERR() wrapper macro for guard pm_runtime_active.
+ *
+ * Always check PM_RUNTIME_ACQUIRE_ERR() after using one of the
+ * PM_RUNTIME_ACQUIRE*() macros defined above (yes, it can be used
+ * with any of them) and avoid accessing the given device if it is
+ * nonzero.
+ */
+#define PM_RUNTIME_ACQUIRE_ERR(var_name)	\
+	ACQUIRE_ERR(pm_runtime_active, &var_name)
+
 /**
  * pm_runtime_put_sync - Drop device usage counter and run "idle check" if 0.
  * @dev: Target device.

Re: [PATCH v1 0/3] PM: runtime: Wrapper macros for usage counter guards

[dan.j.williams]

Rafael J. Wysocki wrote:
> On Wednesday, November 12, 2025 7:39:41 AM CET Dhruva Gole wrote:
> > On Nov 07, 2025 at 19:35:09 +0100, Rafael J. Wysocki wrote:
> > > Hi All,
> > > 
> > > The runtime PM usage counter guards introduced recently:
> > > 
> > > https://lore.kernel.org/linux-pm/6196611.lOV4Wx5bFT@rafael.j.wysocki/
> > > 
> > > and then fixed:
> > > 
> > > https://lore.kernel.org/linux-pm/5943878.DvuYhMxLoT@rafael.j.wysocki/
> > > 
> > > should generally work, but using them feels sort of arcane and cryptic
> > > even though the underlying concept is relatively straightforward.
> > > 
> > > For this reason, runtime PM wrapper macros around ACQUIRE() and
> > > ACQUIRE_ERR() involving the new guards are introduced in this series
> > > (patch [1/3]) and then used in the code already using the guards (patches
> > > [2/3] and [3/3]) to make it look more straightforward.
> > 
> > The patches look okay to me,
> > Reviewed-by: Dhruva Gole <d-gole@ti.com>
> 
> Thank you and Jonathan for the tags, but since Frank is not convinced, let me
> bounce one more idea off all of you.
> 
> Namely, I think that Frank has a point when he wonders if PM_RUNTIME_ACQUIRE_ERR
> hides too much information and I agree with Jonathan that may be misunderstood,
> so what about defining the wrapper macros so they don't hide the guard variable
> name, like in the patch below?

I had been reluctant about offering an enthusiastic tag on this series
given that information hiding, but with this change:

Reviewed-by: Dan Williams <dan.j.williams@intel.com>

However, I prefer that the scope variable declaration vs usage
(reference) cases should maintain visual separation with an operator,
i.e.:

        PM_RUNTIME_ACQUIRE(dev, pm);
        if (PM_RUNTIME_ACQUIRE_ERR(&pm))
                return -ENXIO;

Otherwise we have a case of different flavors of *_ACQUIRE_ERR
implementing various styles. I initially looked at hiding the '&':

http://lore.kernel.org/681ea7d5ea04b_2a2bb100cf@dwillia2-mobl4.notmuch

...but it grew on me precisely because it provides a clue about how this
magic operates.

Re: [PATCH v1 0/3] PM: runtime: Wrapper macros for usage counter guards

[Rafael J. Wysocki]

On Wed, Nov 12, 2025 at 10:27 PM <dan.j.williams@intel.com> wrote:
>
> Rafael J. Wysocki wrote:
> > On Wednesday, November 12, 2025 7:39:41 AM CET Dhruva Gole wrote:
> > > On Nov 07, 2025 at 19:35:09 +0100, Rafael J. Wysocki wrote:
> > > > Hi All,
> > > >
> > > > The runtime PM usage counter guards introduced recently:
> > > >
> > > > https://lore.kernel.org/linux-pm/6196611.lOV4Wx5bFT@rafael.j.wysocki/
> > > >
> > > > and then fixed:
> > > >
> > > > https://lore.kernel.org/linux-pm/5943878.DvuYhMxLoT@rafael.j.wysocki/
> > > >
> > > > should generally work, but using them feels sort of arcane and cryptic
> > > > even though the underlying concept is relatively straightforward.
> > > >
> > > > For this reason, runtime PM wrapper macros around ACQUIRE() and
> > > > ACQUIRE_ERR() involving the new guards are introduced in this series
> > > > (patch [1/3]) and then used in the code already using the guards (patches
> > > > [2/3] and [3/3]) to make it look more straightforward.
> > >
> > > The patches look okay to me,
> > > Reviewed-by: Dhruva Gole <d-gole@ti.com>
> >
> > Thank you and Jonathan for the tags, but since Frank is not convinced, let me
> > bounce one more idea off all of you.
> >
> > Namely, I think that Frank has a point when he wonders if PM_RUNTIME_ACQUIRE_ERR
> > hides too much information and I agree with Jonathan that may be misunderstood,
> > so what about defining the wrapper macros so they don't hide the guard variable
> > name, like in the patch below?
>
> I had been reluctant about offering an enthusiastic tag on this series
> given that information hiding, but with this change:
>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>

Thanks!

> However, I prefer that the scope variable declaration vs usage
> (reference) cases should maintain visual separation with an operator,
> i.e.:
>
>         PM_RUNTIME_ACQUIRE(dev, pm);
>         if (PM_RUNTIME_ACQUIRE_ERR(&pm))
>                 return -ENXIO;
>
> Otherwise we have a case of different flavors of *_ACQUIRE_ERR
> implementing various styles. I initially looked at hiding the '&':
>
> http://lore.kernel.org/681ea7d5ea04b_2a2bb100cf@dwillia2-mobl4.notmuch
>
> ...but it grew on me precisely because it provides a clue about how this
> magic operates.

Fair enough.

I'll resend the series with this change then.

Thank you!

Re: [PATCH v1 0/3] PM: runtime: Wrapper macros for usage counter guards

[Jonathan Cameron]

On Wed, 12 Nov 2025 22:38:14 +0100
"Rafael J. Wysocki" <rafael@kernel.org> wrote:

> On Wed, Nov 12, 2025 at 10:27 PM <dan.j.williams@intel.com> wrote:
> >
> > Rafael J. Wysocki wrote:  
> > > On Wednesday, November 12, 2025 7:39:41 AM CET Dhruva Gole wrote:  
> > > > On Nov 07, 2025 at 19:35:09 +0100, Rafael J. Wysocki wrote:  
> > > > > Hi All,
> > > > >
> > > > > The runtime PM usage counter guards introduced recently:
> > > > >
> > > > > https://lore.kernel.org/linux-pm/6196611.lOV4Wx5bFT@rafael.j.wysocki/
> > > > >
> > > > > and then fixed:
> > > > >
> > > > > https://lore.kernel.org/linux-pm/5943878.DvuYhMxLoT@rafael.j.wysocki/
> > > > >
> > > > > should generally work, but using them feels sort of arcane and cryptic
> > > > > even though the underlying concept is relatively straightforward.
> > > > >
> > > > > For this reason, runtime PM wrapper macros around ACQUIRE() and
> > > > > ACQUIRE_ERR() involving the new guards are introduced in this series
> > > > > (patch [1/3]) and then used in the code already using the guards (patches
> > > > > [2/3] and [3/3]) to make it look more straightforward.  
> > > >
> > > > The patches look okay to me,
> > > > Reviewed-by: Dhruva Gole <d-gole@ti.com>  
> > >
> > > Thank you and Jonathan for the tags, but since Frank is not convinced, let me
> > > bounce one more idea off all of you.
> > >
> > > Namely, I think that Frank has a point when he wonders if PM_RUNTIME_ACQUIRE_ERR
> > > hides too much information and I agree with Jonathan that may be misunderstood,
> > > so what about defining the wrapper macros so they don't hide the guard variable
> > > name, like in the patch below?  
> >
> > I had been reluctant about offering an enthusiastic tag on this series
> > given that information hiding, but with this change:
> >
> > Reviewed-by: Dan Williams <dan.j.williams@intel.com>  
> 
> Thanks!
> 
> > However, I prefer that the scope variable declaration vs usage
> > (reference) cases should maintain visual separation with an operator,
> > i.e.:
> >
> >         PM_RUNTIME_ACQUIRE(dev, pm);
> >         if (PM_RUNTIME_ACQUIRE_ERR(&pm))
> >                 return -ENXIO;
> >
> > Otherwise we have a case of different flavors of *_ACQUIRE_ERR
> > implementing various styles. I initially looked at hiding the '&':
> >
> > http://lore.kernel.org/681ea7d5ea04b_2a2bb100cf@dwillia2-mobl4.notmuch
> >
> > ...but it grew on me precisely because it provides a clue about how this
> > magic operates.  
> 
> Fair enough.
> 
> I'll resend the series with this change then.
This new option is much nicer and not too verbose.

> 
> Thank you!
> 

Re: [PATCH v1 0/3] PM: runtime: Wrapper macros for usage counter guards

[Dhruva Gole]

On Nov 12, 2025 at 13:27:17 -0800, dan.j.williams@intel.com wrote:
> Rafael J. Wysocki wrote:
> > On Wednesday, November 12, 2025 7:39:41 AM CET Dhruva Gole wrote:
> > > On Nov 07, 2025 at 19:35:09 +0100, Rafael J. Wysocki wrote:
> > > > Hi All,
> > > > 
> > > > The runtime PM usage counter guards introduced recently:
> > > > 
> > > > https://lore.kernel.org/linux-pm/6196611.lOV4Wx5bFT@rafael.j.wysocki/
> > > > 
> > > > and then fixed:
> > > > 
> > > > https://lore.kernel.org/linux-pm/5943878.DvuYhMxLoT@rafael.j.wysocki/
> > > > 
> > > > should generally work, but using them feels sort of arcane and cryptic
> > > > even though the underlying concept is relatively straightforward.
> > > > 
> > > > For this reason, runtime PM wrapper macros around ACQUIRE() and
> > > > ACQUIRE_ERR() involving the new guards are introduced in this series
> > > > (patch [1/3]) and then used in the code already using the guards (patches
> > > > [2/3] and [3/3]) to make it look more straightforward.
> > > 
> > > The patches look okay to me,
> > > Reviewed-by: Dhruva Gole <d-gole@ti.com>
> > 
> > Thank you and Jonathan for the tags, but since Frank is not convinced, let me
> > bounce one more idea off all of you.
> > 
> > Namely, I think that Frank has a point when he wonders if PM_RUNTIME_ACQUIRE_ERR
> > hides too much information and I agree with Jonathan that may be misunderstood,
> > so what about defining the wrapper macros so they don't hide the guard variable
> > name, like in the patch below?
> 
> I had been reluctant about offering an enthusiastic tag on this series
> given that information hiding, but with this change:
> 
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> 
> However, I prefer that the scope variable declaration vs usage
> (reference) cases should maintain visual separation with an operator,
> i.e.:
> 
>         PM_RUNTIME_ACQUIRE(dev, pm);
>         if (PM_RUNTIME_ACQUIRE_ERR(&pm))
>                 return -ENXIO;
> 
> Otherwise we have a case of different flavors of *_ACQUIRE_ERR
> implementing various styles. I initially looked at hiding the '&':
> 
> http://lore.kernel.org/681ea7d5ea04b_2a2bb100cf@dwillia2-mobl4.notmuch
> 
> ...but it grew on me precisely because it provides a clue about how this
> magic operates.

Yeah you're right, I agree. Having users explicitly pass on the '&' provides much
more clarity on what's going on than hiding it internally.

-- 
Best regards,
Dhruva Gole
Texas Instruments Incorporated