public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/3] PM: runtime: Auto-cleanup macros for runtime PM
@ 2025-09-26 15:40 Rafael J. Wysocki
  2025-09-26 15:47 ` [PATCH v4 1/3] PM: runtime: Add auto-cleanup macros for "resume and get" operations Rafael J. Wysocki
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Rafael J. Wysocki @ 2025-09-26 15:40 UTC (permalink / raw)
  To: Linux PM, Jonathan Cameron
  Cc: Takashi Iwai, LKML, Linux PCI, Alex Williamson, Bjorn Helgaas,
	Zhang Qilong, Ulf Hansson, Frank Li, Dhruva Gole

Hi All,

This supersedes

https://lore.kernel.org/linux-pm/12763087.O9o76ZdvQC@rafael.j.wysocki/

which was an update of

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

that superseded both

https://lore.kernel.org/linux-pm/5049058.31r3eYUQgx@rafael.j.wysocki/

and

https://lore.kernel.org/linux-pm/20250919163147.4743-1-tiwai@suse.de/

It follows the Jonathan's suggestion to use ACQUIRE()/ACQUIRE_ERR()
instead af raw CLASS() to make the code somewhat cleaner.

Thanks!




^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v4 1/3] PM: runtime: Add auto-cleanup macros for "resume and get" operations
  2025-09-26 15:40 [PATCH v4 0/3] PM: runtime: Auto-cleanup macros for runtime PM Rafael J. Wysocki
@ 2025-09-26 15:47 ` Rafael J. Wysocki
  2025-09-26 19:48   ` dan.j.williams
  2025-09-29 13:24   ` Dhruva Gole
  2025-09-26 16:24 ` [PATCH v4 2/3] PCI/sysfs: Use runtime PM guard macro for auto-cleanup Rafael J. Wysocki
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 13+ messages in thread
From: Rafael J. Wysocki @ 2025-09-26 15:47 UTC (permalink / raw)
  To: Linux PM, Jonathan Cameron
  Cc: Takashi Iwai, LKML, Linux PCI, Alex Williamson, Bjorn Helgaas,
	Zhang Qilong, Ulf Hansson, Frank Li, Dhruva Gole

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

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 guard definition 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 guard() like:

	guard(pm_runtime_active)(dev);
	.....
	return 0;

(see the 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:

	ACQUIRE(pm_runtime_active_try, pm)(dev);
	if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
		return -ENXIO;
	.....
	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 changed as follows:

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

(again, see the 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):

	ACQUIRE(pm_runtime_active_try_enabled, pm)(dev);
	ret = ACQUIRE_ERR(pm_runtime_active_try_enabled, &pm);
	if (ret < 0)
		return ret;
	.....
	return 0;

When the original code calls pm_runtime_put_autosuspend(), use one
of the "auto" guard variants, pm_runtime_active_auto/_try/_enabled,
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:

	ACQUIRE(pm_runtime_active_auto_try_enabled, pm)(dev);
	ret = ACQUIRE_ERR(pm_runtime_active_auto_try_enabled, &pm);
	if (ret < 0)
		return ret;
	.....
	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:

	ACQUIRE(pm_runtime_active_auto_try_enabled, pm)(dev);
	ret = ACQUIRE_ERR(pm_runtime_active_auto_try_enabled, &pm);
	if (ret < 0)
		return ret;
	.....
	return 0;

or pm_runtime_get_active_try can be used if transparent handling of
disabled runtime PM is desirable.

Link: https://lore.kernel.org/linux-pm/878qimv24u.wl-tiwai@suse.de/ [1]
Link: https://lore.kernel.org/linux-pm/20250926150613.000073a4@huawei.com/
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

v3 -> v4:
   * Use guard definition macros instead of raw DEFINE_CLASS() (Jonathan)
   * Change pm_runtime_get_active() helper definition to return an int instead
     of a pointer
   * Update changelog to match the new code

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   |   44 ++++++++++++++++++++++++++++++++++---------
 2 files changed, 37 insertions(+), 9 deletions(-)

--- 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;
 		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
@@ -511,6 +512,19 @@ static inline int pm_runtime_get_sync(st
 	return __pm_runtime_resume(dev, RPM_GET_PUT);
 }
 
+static inline int pm_runtime_get_active(struct device *dev, int rpmflags)
+{
+	int ret;
+
+	ret = __pm_runtime_resume(dev, RPM_GET_PUT | rpmflags);
+	if (ret < 0) {
+		pm_runtime_put_noidle(dev);
+		return ret;
+	}
+
+	return 0;
+}
+
 /**
  * pm_runtime_resume_and_get - Bump up usage counter of a device and resume it.
  * @dev: Target device.
@@ -521,15 +535,7 @@ static inline int pm_runtime_get_sync(st
  */
 static inline int pm_runtime_resume_and_get(struct device *dev)
 {
-	int ret;
-
-	ret = __pm_runtime_resume(dev, RPM_GET_PUT);
-	if (ret < 0) {
-		pm_runtime_put_noidle(dev);
-		return ret;
-	}
-
-	return 0;
+	return pm_runtime_get_active(dev, 0);
 }
 
 /**
@@ -606,6 +612,26 @@ static inline int pm_runtime_put_autosus
 	return __pm_runtime_put_autosuspend(dev);
 }
 
+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));
+/*
+ * Use the following guards with ACQUIRE()/ACQUIRE_ERR().
+ *
+ * The difference between the "_try" and "_try_enabled" variants is that the
+ * former do not produce an error when runtime PM is disabled for the given
+ * device.
+ */
+DEFINE_GUARD_COND(pm_runtime_active, _try,
+		  pm_runtime_get_active(_T, RPM_TRANSPARENT))
+DEFINE_GUARD_COND(pm_runtime_active, _try_enabled,
+		  pm_runtime_resume_and_get(_T))
+DEFINE_GUARD_COND(pm_runtime_active_auto, _try,
+		  pm_runtime_get_active(_T, RPM_TRANSPARENT))
+DEFINE_GUARD_COND(pm_runtime_active_auto, _try_enabled,
+		  pm_runtime_resume_and_get(_T))
+
 /**
  * pm_runtime_put_sync - Drop device usage counter and run "idle check" if 0.
  * @dev: Target device.




^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v4 2/3] PCI/sysfs: Use runtime PM guard macro for auto-cleanup
  2025-09-26 15:40 [PATCH v4 0/3] PM: runtime: Auto-cleanup macros for runtime PM Rafael J. Wysocki
  2025-09-26 15:47 ` [PATCH v4 1/3] PM: runtime: Add auto-cleanup macros for "resume and get" operations Rafael J. Wysocki
@ 2025-09-26 16:24 ` Rafael J. Wysocki
  2025-10-20 22:07   ` Farhan Ali
  2025-09-26 16:26 ` [PATCH v4 3/3] PM: runtime: Drop DEFINE_FREE() for pm_runtime_put() Rafael J. Wysocki
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Rafael J. Wysocki @ 2025-09-26 16:24 UTC (permalink / raw)
  To: Linux PM, Jonathan Cameron, Bjorn Helgaas
  Cc: Takashi Iwai, LKML, Linux PCI, Alex Williamson, Zhang Qilong,
	Ulf Hansson, Frank Li, Dhruva Gole

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

Use the newly introduced pm_runtime_active_try guard to simplify
the code and add the proper error handling for PM runtime resume
errors.

Based on an earlier patch from Takashi Iwai <tiwai@suse.de> [1].

Link: https://patch.msgid.link/20250919163147.4743-3-tiwai@suse.de [1]
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

v3 -> v4:
   * Use ACQUIRE()/ACQUIRE_ERR() (Jonathan)
   * Adjust subject and changelog
   * Take patch ownership (it's all different now)
   * Pick up Bjorn's ACK from v3 (Bjorn, please let me know if that's not OK)

v2 -> v3: No changes

v1 -> v2:
   * Adjust the name of the class to handle the disabled runtime PM case
     transparently (like the original code).

---
 drivers/pci/pci-sysfs.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -1475,8 +1475,9 @@ static ssize_t reset_method_store(struct
 		return count;
 	}
 
-	pm_runtime_get_sync(dev);
-	struct device *pmdev __free(pm_runtime_put) = dev;
+	ACQUIRE(pm_runtime_active_try, pm)(dev);
+	if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
+		return -ENXIO;
 
 	if (sysfs_streq(buf, "default")) {
 		pci_init_reset_methods(pdev);




^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v4 3/3] PM: runtime: Drop DEFINE_FREE() for pm_runtime_put()
  2025-09-26 15:40 [PATCH v4 0/3] PM: runtime: Auto-cleanup macros for runtime PM Rafael J. Wysocki
  2025-09-26 15:47 ` [PATCH v4 1/3] PM: runtime: Add auto-cleanup macros for "resume and get" operations Rafael J. Wysocki
  2025-09-26 16:24 ` [PATCH v4 2/3] PCI/sysfs: Use runtime PM guard macro for auto-cleanup Rafael J. Wysocki
@ 2025-09-26 16:26 ` Rafael J. Wysocki
  2025-09-26 16:48   ` Rafael J. Wysocki
  2025-09-27  7:55 ` [PATCH v4 0/3] PM: runtime: Auto-cleanup macros for runtime PM Takashi Iwai
  2025-09-29 11:15 ` Jonathan Cameron
  4 siblings, 1 reply; 13+ messages in thread
From: Rafael J. Wysocki @ 2025-09-26 16:26 UTC (permalink / raw)
  To: Linux PM, Jonathan Cameron
  Cc: Takashi Iwai, LKML, Linux PCI, Alex Williamson, Bjorn Helgaas,
	Zhang Qilong, Ulf Hansson, Frank Li, Dhruva Gole

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

The DEFINE_FREE() for pm_runtime_put has been superseded by recently
introduced runtime PM auto-cleanup macros and its only user has been
converted to using one of the new macros, so drop it.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Dhruva Gole <d-gole@ti.com>
---

v3 -> v4: Pick up R-by from Dhruva

v1 -> v3: No changes

---
 include/linux/pm_runtime.h |    2 --
 rust/kernel/cpufreq.rs     |    3 ++-
 2 files changed, 2 insertions(+), 3 deletions(-)

--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -561,8 +561,6 @@ static inline int pm_runtime_put(struct
 	return __pm_runtime_idle(dev, RPM_GET_PUT | RPM_ASYNC);
 }
 
-DEFINE_FREE(pm_runtime_put, struct device *, if (_T) pm_runtime_put(_T))
-
 /**
  * __pm_runtime_put_autosuspend - Drop device usage counter and queue autosuspend if 0.
  * @dev: Target device.
--- a/rust/kernel/cpufreq.rs
+++ b/rust/kernel/cpufreq.rs
@@ -39,7 +39,8 @@ use macros::vtable;
 const CPUFREQ_NAME_LEN: usize = bindings::CPUFREQ_NAME_LEN as usize;
 
 /// Default transition latency value in nanoseconds.
-pub const ETERNAL_LATENCY_NS: u32 = bindings::CPUFREQ_ETERNAL as u32;
+pub const DEFAULT_TRANSITION_LATENCY_NS: u32 =
+    bindings::CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS as u32;
 
 /// CPU frequency driver flags.
 pub mod flags {




^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 3/3] PM: runtime: Drop DEFINE_FREE() for pm_runtime_put()
  2025-09-26 16:26 ` [PATCH v4 3/3] PM: runtime: Drop DEFINE_FREE() for pm_runtime_put() Rafael J. Wysocki
@ 2025-09-26 16:48   ` Rafael J. Wysocki
  0 siblings, 0 replies; 13+ messages in thread
From: Rafael J. Wysocki @ 2025-09-26 16:48 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM, Jonathan Cameron, Takashi Iwai, LKML, Linux PCI,
	Alex Williamson, Bjorn Helgaas, Zhang Qilong, Ulf Hansson,
	Frank Li, Dhruva Gole

On Fri, Sep 26, 2025 at 6:27 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> The DEFINE_FREE() for pm_runtime_put has been superseded by recently
> introduced runtime PM auto-cleanup macros and its only user has been
> converted to using one of the new macros, so drop it.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Reviewed-by: Dhruva Gole <d-gole@ti.com>
> ---
>
> v3 -> v4: Pick up R-by from Dhruva
>
> v1 -> v3: No changes
>
> ---
>  include/linux/pm_runtime.h |    2 --
>  rust/kernel/cpufreq.rs     |    3 ++-
>  2 files changed, 2 insertions(+), 3 deletions(-)
>
> --- a/include/linux/pm_runtime.h
> +++ b/include/linux/pm_runtime.h
> @@ -561,8 +561,6 @@ static inline int pm_runtime_put(struct
>         return __pm_runtime_idle(dev, RPM_GET_PUT | RPM_ASYNC);
>  }
>
> -DEFINE_FREE(pm_runtime_put, struct device *, if (_T) pm_runtime_put(_T))
> -
>  /**
>   * __pm_runtime_put_autosuspend - Drop device usage counter and queue autosuspend if 0.
>   * @dev: Target device.

The hunk below does not belong to this patch.  It was added here by
mistake, sorry about that.

> --- a/rust/kernel/cpufreq.rs
> +++ b/rust/kernel/cpufreq.rs
> @@ -39,7 +39,8 @@ use macros::vtable;
>  const CPUFREQ_NAME_LEN: usize = bindings::CPUFREQ_NAME_LEN as usize;
>
>  /// Default transition latency value in nanoseconds.
> -pub const ETERNAL_LATENCY_NS: u32 = bindings::CPUFREQ_ETERNAL as u32;
> +pub const DEFAULT_TRANSITION_LATENCY_NS: u32 =
> +    bindings::CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS as u32;
>
>  /// CPU frequency driver flags.
>  pub mod flags {

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 1/3] PM: runtime: Add auto-cleanup macros for "resume and get" operations
  2025-09-26 15:47 ` [PATCH v4 1/3] PM: runtime: Add auto-cleanup macros for "resume and get" operations Rafael J. Wysocki
@ 2025-09-26 19:48   ` dan.j.williams
  2025-09-26 19:56     ` Rafael J. Wysocki
  2025-09-29 13:24   ` Dhruva Gole
  1 sibling, 1 reply; 13+ messages in thread
From: dan.j.williams @ 2025-09-26 19:48 UTC (permalink / raw)
  To: Rafael J. Wysocki, Linux PM, Jonathan Cameron
  Cc: Takashi Iwai, LKML, Linux PCI, Alex Williamson, Bjorn Helgaas,
	Zhang Qilong, Ulf Hansson, Frank Li, Dhruva Gole

Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> 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 guard definition 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 guard() like:
> 
> 	guard(pm_runtime_active)(dev);
> 	.....
> 	return 0;
> 
> (see the 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:
> 
> 	ACQUIRE(pm_runtime_active_try, pm)(dev);
> 	if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
> 		return -ENXIO;
> 	.....
> 	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 changed as follows:
> 
> 	ACQUIRE(pm_runtime_active_try, pm)(dev);
> 	ret = ACQUIRE_ERR(pm_runtime_active_try, &pm);
> 	if (ret < 0)
> 		return ret;
> 	.....
> 	return 0;
> 
> (again, see the 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

Leftover from CLASS() approach?

s/CLASS(pm_runtime_get_active_enabled)/ACQUIRE(pm_runtime_active_try_enabled)/

> needs to be used, that is (in the context of the example above):
> 
> 	ACQUIRE(pm_runtime_active_try_enabled, pm)(dev);
> 	ret = ACQUIRE_ERR(pm_runtime_active_try_enabled, &pm);
> 	if (ret < 0)
> 		return ret;
> 	.....
> 	return 0;
> 
> When the original code calls pm_runtime_put_autosuspend(), use one
> of the "auto" guard variants, pm_runtime_active_auto/_try/_enabled,
> 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:
> 
> 	ACQUIRE(pm_runtime_active_auto_try_enabled, pm)(dev);
> 	ret = ACQUIRE_ERR(pm_runtime_active_auto_try_enabled, &pm);
> 	if (ret < 0)
> 		return ret;
> 	.....
> 	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.

s/class/guard/

> 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:
> 
> 	ACQUIRE(pm_runtime_active_auto_try_enabled, pm)(dev);
> 	ret = ACQUIRE_ERR(pm_runtime_active_auto_try_enabled, &pm);
> 	if (ret < 0)
> 		return ret;
> 	.....
> 	return 0;

I like that this appears to unify the pm_runtime_resume_and_get() and
pm_runtime_get_sync() usages into common pattern.

> or pm_runtime_get_active_try can be used if transparent handling of
> disabled runtime PM is desirable.

Do you think the above should go in Documentation too?

Either way, for the usage of ACQUIRE(), looks good to me.

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 1/3] PM: runtime: Add auto-cleanup macros for "resume and get" operations
  2025-09-26 19:48   ` dan.j.williams
@ 2025-09-26 19:56     ` Rafael J. Wysocki
  0 siblings, 0 replies; 13+ messages in thread
From: Rafael J. Wysocki @ 2025-09-26 19:56 UTC (permalink / raw)
  To: dan.j.williams
  Cc: Rafael J. Wysocki, Linux PM, Jonathan Cameron, Takashi Iwai, LKML,
	Linux PCI, Alex Williamson, Bjorn Helgaas, Zhang Qilong,
	Ulf Hansson, Frank Li, Dhruva Gole

On Fri, Sep 26, 2025 at 9:48 PM <dan.j.williams@intel.com> wrote:
>
> Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > 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 guard definition 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 guard() like:
> >
> >       guard(pm_runtime_active)(dev);
> >       .....
> >       return 0;
> >
> > (see the 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:
> >
> >       ACQUIRE(pm_runtime_active_try, pm)(dev);
> >       if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
> >               return -ENXIO;
> >       .....
> >       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 changed as follows:
> >
> >       ACQUIRE(pm_runtime_active_try, pm)(dev);
> >       ret = ACQUIRE_ERR(pm_runtime_active_try, &pm);
> >       if (ret < 0)
> >               return ret;
> >       .....
> >       return 0;
> >
> > (again, see the 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
>
> Leftover from CLASS() approach?

Yup.

> s/CLASS(pm_runtime_get_active_enabled)/ACQUIRE(pm_runtime_active_try_enabled)/

I'll fix this when applying.

> > needs to be used, that is (in the context of the example above):
> >
> >       ACQUIRE(pm_runtime_active_try_enabled, pm)(dev);
> >       ret = ACQUIRE_ERR(pm_runtime_active_try_enabled, &pm);
> >       if (ret < 0)
> >               return ret;
> >       .....
> >       return 0;
> >
> > When the original code calls pm_runtime_put_autosuspend(), use one
> > of the "auto" guard variants, pm_runtime_active_auto/_try/_enabled,
> > 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:
> >
> >       ACQUIRE(pm_runtime_active_auto_try_enabled, pm)(dev);
> >       ret = ACQUIRE_ERR(pm_runtime_active_auto_try_enabled, &pm);
> >       if (ret < 0)
> >               return ret;
> >       .....
> >       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.
>
> s/class/guard/

Right, thanks!

> > 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:
> >
> >       ACQUIRE(pm_runtime_active_auto_try_enabled, pm)(dev);
> >       ret = ACQUIRE_ERR(pm_runtime_active_auto_try_enabled, &pm);
> >       if (ret < 0)
> >               return ret;
> >       .....
> >       return 0;
>
> I like that this appears to unify the pm_runtime_resume_and_get() and
> pm_runtime_get_sync() usages into common pattern.

That's intentional.

> > or pm_runtime_get_active_try can be used if transparent handling of
> > disabled runtime PM is desirable.
>
> Do you think the above should go in Documentation too?

It will, when early adopters tell me that they are happy with it.

> Either way, for the usage of ACQUIRE(), looks good to me.
>
> Acked-by: Dan Williams <dan.j.williams@intel.com>

Thank you!

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 0/3] PM: runtime: Auto-cleanup macros for runtime PM
  2025-09-26 15:40 [PATCH v4 0/3] PM: runtime: Auto-cleanup macros for runtime PM Rafael J. Wysocki
                   ` (2 preceding siblings ...)
  2025-09-26 16:26 ` [PATCH v4 3/3] PM: runtime: Drop DEFINE_FREE() for pm_runtime_put() Rafael J. Wysocki
@ 2025-09-27  7:55 ` Takashi Iwai
  2025-09-29 11:15 ` Jonathan Cameron
  4 siblings, 0 replies; 13+ messages in thread
From: Takashi Iwai @ 2025-09-27  7:55 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM, Jonathan Cameron, Takashi Iwai, LKML, Linux PCI,
	Alex Williamson, Bjorn Helgaas, Zhang Qilong, Ulf Hansson,
	Frank Li, Dhruva Gole

On Fri, 26 Sep 2025 17:40:29 +0200,
Rafael J. Wysocki wrote:
> 
> Hi All,
> 
> This supersedes
> 
> https://lore.kernel.org/linux-pm/12763087.O9o76ZdvQC@rafael.j.wysocki/
> 
> which was an update of
> 
> https://lore.kernel.org/linux-pm/6204724.lOV4Wx5bFT@rafael.j.wysocki/
> 
> that superseded both
> 
> https://lore.kernel.org/linux-pm/5049058.31r3eYUQgx@rafael.j.wysocki/
> 
> and
> 
> https://lore.kernel.org/linux-pm/20250919163147.4743-1-tiwai@suse.de/
> 
> It follows the Jonathan's suggestion to use ACQUIRE()/ACQUIRE_ERR()
> instead af raw CLASS() to make the code somewhat cleaner.

ACQUIRE() version looks simpler and more suitable, indeed.

Reviewed-by: Takashi Iwai <tiwai@suse.de>


Thanks!

Takashi

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 0/3] PM: runtime: Auto-cleanup macros for runtime PM
  2025-09-26 15:40 [PATCH v4 0/3] PM: runtime: Auto-cleanup macros for runtime PM Rafael J. Wysocki
                   ` (3 preceding siblings ...)
  2025-09-27  7:55 ` [PATCH v4 0/3] PM: runtime: Auto-cleanup macros for runtime PM Takashi Iwai
@ 2025-09-29 11:15 ` Jonathan Cameron
  2025-09-29 13:59   ` Rafael J. Wysocki
  4 siblings, 1 reply; 13+ messages in thread
From: Jonathan Cameron @ 2025-09-29 11:15 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM, Takashi Iwai, LKML, Linux PCI, Alex Williamson,
	Bjorn Helgaas, Zhang Qilong, Ulf Hansson, Frank Li, Dhruva Gole

On Fri, 26 Sep 2025 17:40:29 +0200
"Rafael J. Wysocki" <rafael@kernel.org> wrote:

> Hi All,
> 
> This supersedes
> 
> https://lore.kernel.org/linux-pm/12763087.O9o76ZdvQC@rafael.j.wysocki/
> 
> which was an update of
> 
> https://lore.kernel.org/linux-pm/6204724.lOV4Wx5bFT@rafael.j.wysocki/
> 
> that superseded both
> 
> https://lore.kernel.org/linux-pm/5049058.31r3eYUQgx@rafael.j.wysocki/
> 
> and
> 
> https://lore.kernel.org/linux-pm/20250919163147.4743-1-tiwai@suse.de/
> 
> It follows the Jonathan's suggestion to use ACQUIRE()/ACQUIRE_ERR()
> instead af raw CLASS() to make the code somewhat cleaner.
> 
> Thanks!

Looks excellent to me.  I've already been pointing a few people at this
in driver reviews, so I expect to see a lot of adoption in IIO (and elsewhere).
That RPM_TRANSPARENT handling is particularly nice.

With the tweaks you've already called out.

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

Given timing, if this ends up as a next cycle thing please could we have
an immutable branch?  If it is going to make the merge window then no need.

Thanks,

Jonathan

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 1/3] PM: runtime: Add auto-cleanup macros for "resume and get" operations
  2025-09-26 15:47 ` [PATCH v4 1/3] PM: runtime: Add auto-cleanup macros for "resume and get" operations Rafael J. Wysocki
  2025-09-26 19:48   ` dan.j.williams
@ 2025-09-29 13:24   ` Dhruva Gole
  1 sibling, 0 replies; 13+ messages in thread
From: Dhruva Gole @ 2025-09-29 13:24 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM, Jonathan Cameron, Takashi Iwai, LKML, Linux PCI,
	Alex Williamson, Bjorn Helgaas, Zhang Qilong, Ulf Hansson,
	Frank Li

On Sep 26, 2025 at 17:47:14 +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> 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 guard definition 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 guard() like:
> 
> 	guard(pm_runtime_active)(dev);
> 	.....
> 	return 0;
> 
> (see the 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:
> 
> 	ACQUIRE(pm_runtime_active_try, pm)(dev);
> 	if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
> 		return -ENXIO;
> 	.....
> 	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 changed as follows:
> 
> 	ACQUIRE(pm_runtime_active_try, pm)(dev);
> 	ret = ACQUIRE_ERR(pm_runtime_active_try, &pm);
> 	if (ret < 0)
> 		return ret;
> 	.....
> 	return 0;
> 
> (again, see the 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):
> 
> 	ACQUIRE(pm_runtime_active_try_enabled, pm)(dev);
> 	ret = ACQUIRE_ERR(pm_runtime_active_try_enabled, &pm);
> 	if (ret < 0)
> 		return ret;
> 	.....
> 	return 0;
> 
> When the original code calls pm_runtime_put_autosuspend(), use one
> of the "auto" guard variants, pm_runtime_active_auto/_try/_enabled,
> 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:
> 
> 	ACQUIRE(pm_runtime_active_auto_try_enabled, pm)(dev);
> 	ret = ACQUIRE_ERR(pm_runtime_active_auto_try_enabled, &pm);
> 	if (ret < 0)
> 		return ret;
> 	.....
> 	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:
> 
> 	ACQUIRE(pm_runtime_active_auto_try_enabled, pm)(dev);
> 	ret = ACQUIRE_ERR(pm_runtime_active_auto_try_enabled, &pm);
> 	if (ret < 0)
> 		return ret;
> 	.....
> 	return 0;
> 
> or pm_runtime_get_active_try can be used if transparent handling of
> disabled runtime PM is desirable.
> 
> Link: https://lore.kernel.org/linux-pm/878qimv24u.wl-tiwai@suse.de/ [1]
> Link: https://lore.kernel.org/linux-pm/20250926150613.000073a4@huawei.com/
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> v3 -> v4:
>    * Use guard definition macros instead of raw DEFINE_CLASS() (Jonathan)
>    * Change pm_runtime_get_active() helper definition to return an int instead
>      of a pointer
>    * Update changelog to match the new code

It does look like a lot has changed since I last gave my R-by so thanks
for not including it.

> 
> 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   |   44 ++++++++++++++++++++++++++++++++++---------
>  2 files changed, 37 insertions(+), 9 deletions(-)
> 
> --- 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;
>  		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
> @@ -511,6 +512,19 @@ static inline int pm_runtime_get_sync(st
>  	return __pm_runtime_resume(dev, RPM_GET_PUT);
>  }
>  
> +static inline int pm_runtime_get_active(struct device *dev, int rpmflags)
> +{
> +	int ret;
> +
> +	ret = __pm_runtime_resume(dev, RPM_GET_PUT | rpmflags);
> +	if (ret < 0) {
> +		pm_runtime_put_noidle(dev);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  /**
>   * pm_runtime_resume_and_get - Bump up usage counter of a device and resume it.
>   * @dev: Target device.
> @@ -521,15 +535,7 @@ static inline int pm_runtime_get_sync(st
>   */
>  static inline int pm_runtime_resume_and_get(struct device *dev)
>  {
> -	int ret;
> -
> -	ret = __pm_runtime_resume(dev, RPM_GET_PUT);
> -	if (ret < 0) {
> -		pm_runtime_put_noidle(dev);
> -		return ret;
> -	}
> -
> -	return 0;
> +	return pm_runtime_get_active(dev, 0);
>  }

I do like this reuse indeed.

>  
>  /**
> @@ -606,6 +612,26 @@ static inline int pm_runtime_put_autosus
>  	return __pm_runtime_put_autosuspend(dev);
>  }
>  
> +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));
> +/*
> + * Use the following guards with ACQUIRE()/ACQUIRE_ERR().
> + *
> + * The difference between the "_try" and "_try_enabled" variants is that the
> + * former do not produce an error when runtime PM is disabled for the given
> + * device.
> + */
> +DEFINE_GUARD_COND(pm_runtime_active, _try,
> +		  pm_runtime_get_active(_T, RPM_TRANSPARENT))
> +DEFINE_GUARD_COND(pm_runtime_active, _try_enabled,
> +		  pm_runtime_resume_and_get(_T))
> +DEFINE_GUARD_COND(pm_runtime_active_auto, _try,
> +		  pm_runtime_get_active(_T, RPM_TRANSPARENT))
> +DEFINE_GUARD_COND(pm_runtime_active_auto, _try_enabled,
> +		  pm_runtime_resume_and_get(_T))

Overall looks better to me than the earlier revisions, thanks.
Reviewed-by: Dhruva Gole <d-gole@ti.com>

-- 
Best regards,
Dhruva Gole
Texas Instruments Incorporated

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 0/3] PM: runtime: Auto-cleanup macros for runtime PM
  2025-09-29 11:15 ` Jonathan Cameron
@ 2025-09-29 13:59   ` Rafael J. Wysocki
  0 siblings, 0 replies; 13+ messages in thread
From: Rafael J. Wysocki @ 2025-09-29 13:59 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Rafael J. Wysocki, Linux PM, Takashi Iwai, LKML, Linux PCI,
	Alex Williamson, Bjorn Helgaas, Zhang Qilong, Ulf Hansson,
	Frank Li, Dhruva Gole

On Mon, Sep 29, 2025 at 1:15 PM Jonathan Cameron
<jonathan.cameron@huawei.com> wrote:
>
> On Fri, 26 Sep 2025 17:40:29 +0200
> "Rafael J. Wysocki" <rafael@kernel.org> wrote:
>
> > Hi All,
> >
> > This supersedes
> >
> > https://lore.kernel.org/linux-pm/12763087.O9o76ZdvQC@rafael.j.wysocki/
> >
> > which was an update of
> >
> > https://lore.kernel.org/linux-pm/6204724.lOV4Wx5bFT@rafael.j.wysocki/
> >
> > that superseded both
> >
> > https://lore.kernel.org/linux-pm/5049058.31r3eYUQgx@rafael.j.wysocki/
> >
> > and
> >
> > https://lore.kernel.org/linux-pm/20250919163147.4743-1-tiwai@suse.de/
> >
> > It follows the Jonathan's suggestion to use ACQUIRE()/ACQUIRE_ERR()
> > instead af raw CLASS() to make the code somewhat cleaner.
> >
> > Thanks!
>
> Looks excellent to me.  I've already been pointing a few people at this
> in driver reviews, so I expect to see a lot of adoption in IIO (and elsewhere).
> That RPM_TRANSPARENT handling is particularly nice.
>
> With the tweaks you've already called out.
>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

Thanks!

> Given timing, if this ends up as a next cycle thing please could we have
> an immutable branch?  If it is going to make the merge window then no need.

I'm actually going to push it for 6.18 during the second half of the
merge window.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 2/3] PCI/sysfs: Use runtime PM guard macro for auto-cleanup
  2025-09-26 16:24 ` [PATCH v4 2/3] PCI/sysfs: Use runtime PM guard macro for auto-cleanup Rafael J. Wysocki
@ 2025-10-20 22:07   ` Farhan Ali
  2025-10-21 12:44     ` Rafael J. Wysocki
  0 siblings, 1 reply; 13+ messages in thread
From: Farhan Ali @ 2025-10-20 22:07 UTC (permalink / raw)
  To: Rafael J. Wysocki, Linux PM, Jonathan Cameron, Bjorn Helgaas
  Cc: Takashi Iwai, LKML, Linux PCI, Alex Williamson, Zhang Qilong,
	Ulf Hansson, Frank Li, Dhruva Gole, Niklas Schnelle


On 9/26/2025 9:24 AM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Use the newly introduced pm_runtime_active_try guard to simplify
> the code and add the proper error handling for PM runtime resume
> errors.
>
> Based on an earlier patch from Takashi Iwai <tiwai@suse.de> [1].
>
> Link: https://patch.msgid.link/20250919163147.4743-3-tiwai@suse.de [1]
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>
> v3 -> v4:
>     * Use ACQUIRE()/ACQUIRE_ERR() (Jonathan)
>     * Adjust subject and changelog
>     * Take patch ownership (it's all different now)
>     * Pick up Bjorn's ACK from v3 (Bjorn, please let me know if that's not OK)
>
> v2 -> v3: No changes
>
> v1 -> v2:
>     * Adjust the name of the class to handle the disabled runtime PM case
>       transparently (like the original code).
>
> ---
>   drivers/pci/pci-sysfs.c |    5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -1475,8 +1475,9 @@ static ssize_t reset_method_store(struct
>   		return count;
>   	}
>   
> -	pm_runtime_get_sync(dev);
> -	struct device *pmdev __free(pm_runtime_put) = dev;
> +	ACQUIRE(pm_runtime_active_try, pm)(dev);
> +	if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
> +		return -ENXIO;
>   
>   	if (sysfs_streq(buf, "default")) {
>   		pci_init_reset_methods(pdev);
>
>
Hi Rafael,

This patch breaks updating the 'reset_method' sysfs file on s390. If we 
try to update the reset_method, we are hitting the ENXIO error. eg:

echo 'bus' > /sys/bus/pci/devices/0007\:00\:10.1/reset_method
-bash: echo: write error: No such device or address

I don't think s390 does anything different in this path, so this could 
also impact other platforms? Changing this to something like this fixes it


diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 9d6f74bd95f8..d7fc0dc81c30 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -1517,8 +1517,8 @@ static ssize_t reset_method_store(struct device *dev,
                 return count;
         }

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

This changes the logic to what it was previously which used 
pm_runtime_get_sync and pm_runtime_put. But I am not familiar with the 
PM runtime code, so not sure what would be the right fix here.

Thanks

Farhan



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 2/3] PCI/sysfs: Use runtime PM guard macro for auto-cleanup
  2025-10-20 22:07   ` Farhan Ali
@ 2025-10-21 12:44     ` Rafael J. Wysocki
  0 siblings, 0 replies; 13+ messages in thread
From: Rafael J. Wysocki @ 2025-10-21 12:44 UTC (permalink / raw)
  To: Farhan Ali
  Cc: Rafael J. Wysocki, Linux PM, Jonathan Cameron, Bjorn Helgaas,
	Takashi Iwai, LKML, Linux PCI, Alex Williamson, Zhang Qilong,
	Ulf Hansson, Frank Li, Dhruva Gole, Niklas Schnelle

On Tue, Oct 21, 2025 at 12:07 AM Farhan Ali <alifm@linux.ibm.com> wrote:
>
>
> On 9/26/2025 9:24 AM, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Use the newly introduced pm_runtime_active_try guard to simplify
> > the code and add the proper error handling for PM runtime resume
> > errors.
> >
> > Based on an earlier patch from Takashi Iwai <tiwai@suse.de> [1].
> >
> > Link: https://patch.msgid.link/20250919163147.4743-3-tiwai@suse.de [1]
> > Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >
> > v3 -> v4:
> >     * Use ACQUIRE()/ACQUIRE_ERR() (Jonathan)
> >     * Adjust subject and changelog
> >     * Take patch ownership (it's all different now)
> >     * Pick up Bjorn's ACK from v3 (Bjorn, please let me know if that's not OK)
> >
> > v2 -> v3: No changes
> >
> > v1 -> v2:
> >     * Adjust the name of the class to handle the disabled runtime PM case
> >       transparently (like the original code).
> >
> > ---
> >   drivers/pci/pci-sysfs.c |    5 +++--
> >   1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > --- a/drivers/pci/pci-sysfs.c
> > +++ b/drivers/pci/pci-sysfs.c
> > @@ -1475,8 +1475,9 @@ static ssize_t reset_method_store(struct
> >               return count;
> >       }
> >
> > -     pm_runtime_get_sync(dev);
> > -     struct device *pmdev __free(pm_runtime_put) = dev;
> > +     ACQUIRE(pm_runtime_active_try, pm)(dev);
> > +     if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
> > +             return -ENXIO;
> >
> >       if (sysfs_streq(buf, "default")) {
> >               pci_init_reset_methods(pdev);
> >
> >
> Hi Rafael,
>
> This patch breaks updating the 'reset_method' sysfs file on s390. If we
> try to update the reset_method, we are hitting the ENXIO error. eg:
>
> echo 'bus' > /sys/bus/pci/devices/0007\:00\:10.1/reset_method
> -bash: echo: write error: No such device or address
>
> I don't think s390 does anything different in this path, so this could
> also impact other platforms? Changing this to something like this fixes it
>
>
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 9d6f74bd95f8..d7fc0dc81c30 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -1517,8 +1517,8 @@ static ssize_t reset_method_store(struct device *dev,
>                  return count;
>          }
>
> -       ACQUIRE(pm_runtime_active_try, pm)(dev);
> -       if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
> +       ACQUIRE(pm_runtime_active, pm)(dev);
> +       if (ACQUIRE_ERR(pm_runtime_active, &pm))
>                  return -ENXIO;
>
> This changes the logic to what it was previously which used
> pm_runtime_get_sync and pm_runtime_put. But I am not familiar with the
> PM runtime code, so not sure what would be the right fix here.

Can you please check if this helps:

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2025-10-21 12:45 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-26 15:40 [PATCH v4 0/3] PM: runtime: Auto-cleanup macros for runtime PM Rafael J. Wysocki
2025-09-26 15:47 ` [PATCH v4 1/3] PM: runtime: Add auto-cleanup macros for "resume and get" operations Rafael J. Wysocki
2025-09-26 19:48   ` dan.j.williams
2025-09-26 19:56     ` Rafael J. Wysocki
2025-09-29 13:24   ` Dhruva Gole
2025-09-26 16:24 ` [PATCH v4 2/3] PCI/sysfs: Use runtime PM guard macro for auto-cleanup Rafael J. Wysocki
2025-10-20 22:07   ` Farhan Ali
2025-10-21 12:44     ` Rafael J. Wysocki
2025-09-26 16:26 ` [PATCH v4 3/3] PM: runtime: Drop DEFINE_FREE() for pm_runtime_put() Rafael J. Wysocki
2025-09-26 16:48   ` Rafael J. Wysocki
2025-09-27  7:55 ` [PATCH v4 0/3] PM: runtime: Auto-cleanup macros for runtime PM Takashi Iwai
2025-09-29 11:15 ` Jonathan Cameron
2025-09-29 13:59   ` Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox