[PATCH v2] spi: meson-spifc: use devm_pm_runtime_set_active_enabled

[PATCH v2] spi: meson-spifc: use devm_pm_runtime_set_active_enabled

[Felix Gu]

Use devm_pm_runtime_set_active_enabled to replace
pm_runtime_set_active() + pm_runtime_enable() and drop the out_pm
error label.

Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
Changes in v2:
- Fix Mark's comment.
- Link to v1: https://patch.msgid.link/20260722-spifc-v1-1-e4462a4c6a06@gmail.com

To: Mark Brown <broonie@kernel.org>
To: Neil Armstrong <neil.armstrong@linaro.org>
To: Kevin Hilman <khilman@baylibre.com>
To: Jerome Brunet <jbrunet@baylibre.com>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: linux-spi@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-amlogic@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/spi/spi-meson-spifc.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/spi/spi-meson-spifc.c b/drivers/spi/spi-meson-spifc.c
index e36aa94bbdae..82c1c3c0c84b 100644
--- a/drivers/spi/spi-meson-spifc.c
+++ b/drivers/spi/spi-meson-spifc.c
@@ -325,26 +325,20 @@ static int meson_spifc_probe(struct platform_device *pdev)
 
 	meson_spifc_hw_init(spifc);
 
-	pm_runtime_set_active(spifc->dev);
-	pm_runtime_enable(spifc->dev);
+	ret =  devm_pm_runtime_set_active_enabled(spifc->dev);
+	if (ret)
+		return dev_err_probe(spifc->dev, ret, "failed to set runtime PM\n");
 
 	ret = devm_spi_register_controller(spifc->dev, host);
-	if (ret) {
-		dev_err(spifc->dev, "failed to register spi host\n");
-		goto out_pm;
-	}
+	if (ret)
+		return dev_err_probe(spifc->dev, ret, "failed to register spi host\n");
 
 	return 0;
-out_pm:
-	pm_runtime_disable(spifc->dev);
-
-	return ret;
 }
 
 static void meson_spifc_remove(struct platform_device *pdev)
 {
 	pm_runtime_get_sync(&pdev->dev);
-	pm_runtime_disable(&pdev->dev);
 	pm_runtime_put_noidle(&pdev->dev);
 }
 

---
base-commit: 3fe08b9796f36ef437ab9328e7dd1e5ff2d66603
change-id: 20260722-spifc-336fbde9511b

Best regards,
--  
Felix Gu <ustc.gu@gmail.com>


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v2] spi: meson-spifc: use devm_pm_runtime_set_active_enabled

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing `pm_runtime_disable()` from `meson_spifc_remove()` introduces a race condition during device removal that can cause a clock usage underflow.
--

--- Patch [1]: [PATCH v2] spi: meson-spifc: use devm_pm_runtime_set_active_enabled ---
commit b45e95aa00b1b02a9f6c29275b73bb003b297870
Author: Felix Gu <ustc.gu@gmail.com>

spi: meson-spifc: use devm_pm_runtime_set_active_enabled

This commit converts the driver to use devm_pm_runtime_set_active_enabled(),
dropping explicit pm_runtime_enable and pm_runtime_disable calls along with
their associated error paths.

Link: https://patch.msgid.link/20260722-spifc-v1-1-e4462a4c6a06@gmail.com

> diff --git a/drivers/spi/spi-meson-spifc.c b/drivers/spi/spi-meson-spifc.c
> index e36aa94bbdae3..82c1c3c0c84b5 100644
> --- a/drivers/spi/spi-meson-spifc.c
> +++ b/drivers/spi/spi-meson-spifc.c
> @@ -325,26 +325,20 @@ static int meson_spifc_probe(struct platform_device *pdev)
>  
>  	meson_spifc_hw_init(spifc);
>  
> -	pm_runtime_set_active(spifc->dev);
> -	pm_runtime_enable(spifc->dev);
> +	ret =  devm_pm_runtime_set_active_enabled(spifc->dev);

[ ... ]

>  static void meson_spifc_remove(struct platform_device *pdev)
>  {
>  	pm_runtime_get_sync(&pdev->dev);
> -	pm_runtime_disable(&pdev->dev);
>  	pm_runtime_put_noidle(&pdev->dev);

[Severity: High]
Does removing pm_runtime_disable() here create a race condition that can
lead to a clock usage underflow?

When pm_runtime_put_noidle() drops the usage count to 0, PM runtime remains
enabled until the devres teardown phase.

Next, devres teardown runs devm_spi_unregister_controller(), which
unregisters child SPI devices.

If a child device (like an SPI flash) performs a final transfer during its
own removal, the SPI core will temporarily wake this parent controller and
then call pm_runtime_put_autosuspend() when the transfer completes.

Because the base usage count is now 0, this queues an asynchronous suspend.
When meson_spifc_runtime_suspend() runs, it calls clk_disable_unprepare(),
dropping the clock count to 0.

Later in the devres teardown sequence, the cleanup for devm_clk_get_enabled()
executes and unconditionally calls clk_disable_unprepare() again, causing
the clock count to underflow.

>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260802-spifc-v2-1-46e9d06a3217@gmail.com?part=1

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic