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

[sashiko-bot@kernel.org]

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