[PATCH v6 0/2] Add more devm_ functions to fix PM imbalance in spi/atmel-quadspi.c

[PATCH v6 0/2] Add more devm_ functions to fix PM imbalance in spi/atmel-quadspi.c

[Bence Csókás]

The probe() function of the atmel-quadspi driver got quite convoluted,
especially since the addition of SAMA7G5 support, that was forward-ported
from an older vendor kernel. During the port, a bug was introduced, where
the PM get() and put() calls were imbalanced. To alleivate this - and
similar problems in the future - an effort was made to migrate as many
functions as possible, to their devm_ managed counterparts. The few
functions, which did not yet have a devm_ variant, are added in patch 1 of
this series. Patch 2 then uses these APIs to fix the probe() function.

Change in v4:
* the DMA cleanup was split out and will be submitted separately for 6.15

Change in v5:
* rebased to linux-pm/linux-next, will now target 6.15

Change in v6:
* add `devm_pm_runtime_set_active_enabled()`, which is safer

Links to previous versions:
pre-series:
https://lore.kernel.org/linux-kernel/20250114222851.1023194-1-csokas.bence@prolan.hu/
v1:
https://lore.kernel.org/linux-kernel/20250115160244.1102881-1-csokas.bence@prolan.hu/
v2:
https://lore.kernel.org/linux-kernel/20250124085221.766303-8-csokas.bence@prolan.hu/
v3:
https://lore.kernel.org/linux-kernel/20250207124802.165408-1-csokas.bence@prolan.hu/
v4:
https://lore.kernel.org/linux-kernel/20250210111008.248929-1-csokas.bence@prolan.hu/
v5:
https://lore.kernel.org/linux-kernel/20250317093445.361821-1-csokas.bence@prolan.hu

Bence Csókás (2):
  pm: runtime: Add new devm functions
  spi: atmel-quadspi: Fix unbalanced pm_runtime by using devm_ API

 drivers/base/power/runtime.c | 44 ++++++++++++++++++++++++++++++++++++
 drivers/spi/atmel-quadspi.c  | 17 ++++----------
 include/linux/pm_runtime.h   |  4 ++++
 3 files changed, 52 insertions(+), 13 deletions(-)


base-commit: 8f61ea3ebb4de9a4cacff995b6885e91450cb094
-- 
2.49.0

[PATCH v6 1/2] pm: runtime: Add new devm functions

[Bence Csókás]

Add `devm_pm_runtime_set_active_enabled()` and
`devm_pm_runtime_get_noresume()` for simplifying common cases in drivers.

Signed-off-by: Bence Csókás <csokas.bence@prolan.hu>
---
 drivers/base/power/runtime.c | 44 ++++++++++++++++++++++++++++++++++++
 include/linux/pm_runtime.h   |  4 ++++
 2 files changed, 48 insertions(+)

diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 0e127b0329c0..205a4f8828b0 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -1568,6 +1568,32 @@ void pm_runtime_enable(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(pm_runtime_enable);
 
+static void pm_runtime_set_suspended_action(void *data)
+{
+	pm_runtime_set_suspended(data);
+}
+
+/**
+ * devm_pm_runtime_set_active_enabled - set_active version of devm_pm_runtime_enable.
+ *
+ * @dev: Device to handle.
+ */
+int devm_pm_runtime_set_active_enabled(struct device *dev)
+{
+	int err;
+
+	err = pm_runtime_set_active(dev);
+	if (err)
+		return err;
+
+	err = devm_add_action_or_reset(dev, pm_runtime_set_suspended_action, dev);
+	if (err)
+		return err;
+
+	return devm_pm_runtime_enable(dev);
+}
+EXPORT_SYMBOL_GPL(devm_pm_runtime_set_active_enabled);
+
 static void pm_runtime_disable_action(void *data)
 {
 	pm_runtime_dont_use_autosuspend(data);
@@ -1590,6 +1616,24 @@ int devm_pm_runtime_enable(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(devm_pm_runtime_enable);
 
+static void pm_runtime_put_noidle_action(void *data)
+{
+	pm_runtime_put_noidle(data);
+}
+
+/**
+ * devm_pm_runtime_get_noresume - devres-enabled version of pm_runtime_get_noresume.
+ *
+ * @dev: Device to handle.
+ */
+int devm_pm_runtime_get_noresume(struct device *dev)
+{
+	pm_runtime_get_noresume(dev);
+
+	return devm_add_action_or_reset(dev, pm_runtime_put_noidle_action, dev);
+}
+EXPORT_SYMBOL_GPL(devm_pm_runtime_get_noresume);
+
 /**
  * pm_runtime_forbid - Block runtime PM of a device.
  * @dev: Device to handle.
diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
index 7fb5a459847e..756b842dcd30 100644
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -96,7 +96,9 @@ extern void pm_runtime_new_link(struct device *dev);
 extern void pm_runtime_drop_link(struct device_link *link);
 extern void pm_runtime_release_supplier(struct device_link *link);
 
+int devm_pm_runtime_set_active_enabled(struct device *dev);
 extern int devm_pm_runtime_enable(struct device *dev);
+int devm_pm_runtime_get_noresume(struct device *dev);
 
 /**
  * pm_suspend_ignore_children - Set runtime PM behavior regarding children.
@@ -294,7 +296,9 @@ static inline bool pm_runtime_blocked(struct device *dev) { return true; }
 static inline void pm_runtime_allow(struct device *dev) {}
 static inline void pm_runtime_forbid(struct device *dev) {}
 
+static inline int devm_pm_runtime_set_active_enabled(struct device *dev) { return 0; }
 static inline int devm_pm_runtime_enable(struct device *dev) { return 0; }
+static inline int devm_pm_runtime_get_noresume(struct device *dev) { return 0; }
 
 static inline void pm_suspend_ignore_children(struct device *dev, bool enable) {}
 static inline void pm_runtime_get_noresume(struct device *dev) {}
-- 
2.49.0

Re: [PATCH v6 1/2] pm: runtime: Add new devm functions

[Rafael J. Wysocki]

On Thu, Mar 27, 2025 at 8:59 PM Bence Csókás <csokas.bence@prolan.hu> wrote:
>
> Add `devm_pm_runtime_set_active_enabled()` and
> `devm_pm_runtime_get_noresume()` for simplifying common cases in drivers.
>
> Signed-off-by: Bence Csókás <csokas.bence@prolan.hu>

I can apply this one alone if you want me to do that, but I could also
apply the other patch in the series if it got an ACK from the driver
maintainer.

> ---
>  drivers/base/power/runtime.c | 44 ++++++++++++++++++++++++++++++++++++
>  include/linux/pm_runtime.h   |  4 ++++
>  2 files changed, 48 insertions(+)
>
> diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> index 0e127b0329c0..205a4f8828b0 100644
> --- a/drivers/base/power/runtime.c
> +++ b/drivers/base/power/runtime.c
> @@ -1568,6 +1568,32 @@ void pm_runtime_enable(struct device *dev)
>  }
>  EXPORT_SYMBOL_GPL(pm_runtime_enable);
>
> +static void pm_runtime_set_suspended_action(void *data)
> +{
> +       pm_runtime_set_suspended(data);
> +}
> +
> +/**
> + * devm_pm_runtime_set_active_enabled - set_active version of devm_pm_runtime_enable.
> + *
> + * @dev: Device to handle.
> + */
> +int devm_pm_runtime_set_active_enabled(struct device *dev)
> +{
> +       int err;
> +
> +       err = pm_runtime_set_active(dev);
> +       if (err)
> +               return err;
> +
> +       err = devm_add_action_or_reset(dev, pm_runtime_set_suspended_action, dev);
> +       if (err)
> +               return err;
> +
> +       return devm_pm_runtime_enable(dev);
> +}
> +EXPORT_SYMBOL_GPL(devm_pm_runtime_set_active_enabled);
> +
>  static void pm_runtime_disable_action(void *data)
>  {
>         pm_runtime_dont_use_autosuspend(data);
> @@ -1590,6 +1616,24 @@ int devm_pm_runtime_enable(struct device *dev)
>  }
>  EXPORT_SYMBOL_GPL(devm_pm_runtime_enable);
>
> +static void pm_runtime_put_noidle_action(void *data)
> +{
> +       pm_runtime_put_noidle(data);
> +}
> +
> +/**
> + * devm_pm_runtime_get_noresume - devres-enabled version of pm_runtime_get_noresume.
> + *
> + * @dev: Device to handle.
> + */
> +int devm_pm_runtime_get_noresume(struct device *dev)
> +{
> +       pm_runtime_get_noresume(dev);
> +
> +       return devm_add_action_or_reset(dev, pm_runtime_put_noidle_action, dev);
> +}
> +EXPORT_SYMBOL_GPL(devm_pm_runtime_get_noresume);
> +
>  /**
>   * pm_runtime_forbid - Block runtime PM of a device.
>   * @dev: Device to handle.
> diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
> index 7fb5a459847e..756b842dcd30 100644
> --- a/include/linux/pm_runtime.h
> +++ b/include/linux/pm_runtime.h
> @@ -96,7 +96,9 @@ extern void pm_runtime_new_link(struct device *dev);
>  extern void pm_runtime_drop_link(struct device_link *link);
>  extern void pm_runtime_release_supplier(struct device_link *link);
>
> +int devm_pm_runtime_set_active_enabled(struct device *dev);
>  extern int devm_pm_runtime_enable(struct device *dev);
> +int devm_pm_runtime_get_noresume(struct device *dev);
>
>  /**
>   * pm_suspend_ignore_children - Set runtime PM behavior regarding children.
> @@ -294,7 +296,9 @@ static inline bool pm_runtime_blocked(struct device *dev) { return true; }
>  static inline void pm_runtime_allow(struct device *dev) {}
>  static inline void pm_runtime_forbid(struct device *dev) {}
>
> +static inline int devm_pm_runtime_set_active_enabled(struct device *dev) { return 0; }
>  static inline int devm_pm_runtime_enable(struct device *dev) { return 0; }
> +static inline int devm_pm_runtime_get_noresume(struct device *dev) { return 0; }
>
>  static inline void pm_suspend_ignore_children(struct device *dev, bool enable) {}
>  static inline void pm_runtime_get_noresume(struct device *dev) {}
> --
> 2.49.0
>
>

Re: [PATCH v6 1/2] pm: runtime: Add new devm functions

[Csókás Bence]

Hi,

On 2025. 04. 09. 19:43, Rafael J. Wysocki wrote:
> On Thu, Mar 27, 2025 at 8:59 PM Bence Csókás <csokas.bence@prolan.hu> wrote:
>>
>> Add `devm_pm_runtime_set_active_enabled()` and
>> `devm_pm_runtime_get_noresume()` for simplifying common cases in drivers.
>>
>> Signed-off-by: Bence Csókás <csokas.bence@prolan.hu>
> 
> I can apply this one alone if you want me to do that, but I could also
> apply the other patch in the series if it got an ACK from the driver
> maintainer.

I think you can apply it and then Mark can apply the SPI part to his 
tree. @broonie what do you think?

Bence

Re: [PATCH v6 1/2] pm: runtime: Add new devm functions

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 530 bytes --]

On Mon, Apr 14, 2025 at 03:56:47PM +0200, Csókás Bence wrote:
> On 2025. 04. 09. 19:43, Rafael J. Wysocki wrote:

> > I can apply this one alone if you want me to do that, but I could also
> > apply the other patch in the series if it got an ACK from the driver
> > maintainer.

> I think you can apply it and then Mark can apply the SPI part to his tree.
> @broonie what do you think?

Please don't add random characters to my name.

I'd need a tag since the driver change isn't going to build without the
new API.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v6 1/2] pm: runtime: Add new devm functions

[Csókás Bence]

Hi,

On 2025. 04. 09. 19:43, Rafael J. Wysocki wrote:
> On Thu, Mar 27, 2025 at 8:59 PM Bence Csókás <csokas.bence@prolan.hu> wrote:
>>
>> Add `devm_pm_runtime_set_active_enabled()` and
>> `devm_pm_runtime_get_noresume()` for simplifying common cases in drivers.
>>
>> Signed-off-by: Bence Csókás <csokas.bence@prolan.hu>
> 
> I can apply this one alone if you want me to do that, but I could also
> apply the other patch in the series if it got an ACK from the driver
> maintainer.

Did this end up being applied?

Bence

Re: [PATCH v6 1/2] pm: runtime: Add new devm functions

[Rafael J. Wysocki]

On Mon, Apr 28, 2025 at 10:44 AM Csókás Bence <csokas.bence@prolan.hu> wrote:
>
> Hi,
>
> On 2025. 04. 09. 19:43, Rafael J. Wysocki wrote:
> > On Thu, Mar 27, 2025 at 8:59 PM Bence Csókás <csokas.bence@prolan.hu> wrote:
> >>
> >> Add `devm_pm_runtime_set_active_enabled()` and
> >> `devm_pm_runtime_get_noresume()` for simplifying common cases in drivers.
> >>
> >> Signed-off-by: Bence Csókás <csokas.bence@prolan.hu>
> >
> > I can apply this one alone if you want me to do that, but I could also
> > apply the other patch in the series if it got an ACK from the driver
> > maintainer.
>
> Did this end up being applied?

Just applied as 6.16 material, I'll let you know when the tag to pull
from is ready.

Thanks!

Re: [PATCH v6 1/2] pm: runtime: Add new devm functions

[Csókás Bence]

Hi Rafael,

On 2025. 04. 29. 13:08, Rafael J. Wysocki wrote:
>> Did this end up being applied?
> 
> Just applied as 6.16 material, I'll let you know when the tag to pull
> from is ready.
> 
> Thanks!
> 

Thank you! In the meantime, I submitted the DMA cleanup part as well, 
for now rebased onto pm-next, but there seems to be no diff between it 
and dma-next in the relevant files (drivers/dma/dmaengine.c and 
drivers/spi/atmel-quadspi.c).

Bence

[GIT TAG] Power management (runtime PM) change for 6.16

[Rafael J. Wysocki]

The tag

git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git \
pm-runtime-devm-6.16

with top-most commit 73db799bf5efc5a04654bb3ff6c9bf63a0dfa473

 PM: runtime: Add new devm functions

on top of commit b4432656b36e5cc1d50a1f2dc15357543add530e

 Linux 6.15-rc4

is available to pull from to receive a power management (runtime PM)
change for 6.16.

It adds new devm functions for setting runtime PM status to "active"
and incrementing runtime PM usage counter (Bence Csókás).

Thanks!

Re: [PATCH v6 1/2] pm: runtime: Add new devm functions

[Rafael J. Wysocki]

Hi,

On Mon, May 5, 2025 at 9:02 PM Csókás Bence <csokas.bence@prolan.hu> wrote:
>
> Hi Rafael,
>
> On 2025. 04. 29. 13:08, Rafael J. Wysocki wrote:
> >> Did this end up being applied?
> >
> > Just applied as 6.16 material, I'll let you know when the tag to pull
> > from is ready.
> >
> > Thanks!
> >
>
> Thank you! In the meantime, I submitted the DMA cleanup part as well,
> for now rebased onto pm-next, but there seems to be no diff between it
> and dma-next in the relevant files (drivers/dma/dmaengine.c and
> drivers/spi/atmel-quadspi.c).

I've just added a tag from which this change can be pulled:

https://lore.kernel.org/linux-pm/CAJZ5v0hNb8eB2tAig1BiUMHLQ3JRak3EXFid9qmHz6iWvh5q6g@mail.gmail.com/

[PATCH v6 2/2] spi: atmel-quadspi: Fix unbalanced pm_runtime by using devm_ API

[Bence Csókás]

Fix unbalanced PM in error path of `atmel_qspi_probe()`
by using `devm_pm_runtime_*()` functions.

Reported-by: Alexander Dahl <ada@thorsis.com>
Closes: https://lore.kernel.org/linux-spi/20250110-paycheck-irregular-bcddab1276c7@thorsis.com/
Fixes: 5af42209a4d2 ("spi: atmel-quadspi: Add support for sama7g5 QSPI")
Signed-off-by: Bence Csókás <csokas.bence@prolan.hu>
---
 drivers/spi/atmel-quadspi.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
index 244ac0106862..e7b61dc4ce67 100644
--- a/drivers/spi/atmel-quadspi.c
+++ b/drivers/spi/atmel-quadspi.c
@@ -1436,22 +1436,17 @@ static int atmel_qspi_probe(struct platform_device *pdev)
 
 	pm_runtime_set_autosuspend_delay(&pdev->dev, 500);
 	pm_runtime_use_autosuspend(&pdev->dev);
-	pm_runtime_set_active(&pdev->dev);
-	pm_runtime_enable(&pdev->dev);
-	pm_runtime_get_noresume(&pdev->dev);
+	devm_pm_runtime_set_active_enabled(&pdev->dev);
+	devm_pm_runtime_get_noresume(&pdev->dev);
 
 	err = atmel_qspi_init(aq);
 	if (err)
 		goto dma_release;
 
 	err = spi_register_controller(ctrl);
-	if (err) {
-		pm_runtime_put_noidle(&pdev->dev);
-		pm_runtime_disable(&pdev->dev);
-		pm_runtime_set_suspended(&pdev->dev);
-		pm_runtime_dont_use_autosuspend(&pdev->dev);
+	if (err)
 		goto dma_release;
-	}
+
 	pm_runtime_mark_last_busy(&pdev->dev);
 	pm_runtime_put_autosuspend(&pdev->dev);
 
@@ -1530,10 +1525,6 @@ static void atmel_qspi_remove(struct platform_device *pdev)
 		 */
 		dev_warn(&pdev->dev, "Failed to resume device on remove\n");
 	}
-
-	pm_runtime_disable(&pdev->dev);
-	pm_runtime_dont_use_autosuspend(&pdev->dev);
-	pm_runtime_put_noidle(&pdev->dev);
 }
 
 static int __maybe_unused atmel_qspi_suspend(struct device *dev)
-- 
2.49.0

Re: [PATCH v6 0/2] Add more devm_ functions to fix PM imbalance in spi/atmel-quadspi.c

[Mark Brown]

On Thu, 27 Mar 2025 20:59:25 +0100, Bence Csókás wrote:
> The probe() function of the atmel-quadspi driver got quite convoluted,
> especially since the addition of SAMA7G5 support, that was forward-ported
> from an older vendor kernel. During the port, a bug was introduced, where
> the PM get() and put() calls were imbalanced. To alleivate this - and
> similar problems in the future - an effort was made to migrate as many
> functions as possible, to their devm_ managed counterparts. The few
> functions, which did not yet have a devm_ variant, are added in patch 1 of
> this series. Patch 2 then uses these APIs to fix the probe() function.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/2] pm: runtime: Add new devm functions
      (no commit info)
[2/2] spi: atmel-quadspi: Fix unbalanced pm_runtime by using devm_ API
      commit: 8856eafcc05ecf54d6dd2b6c67804fefd276472c

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark