linux-kernel - Re: [PATCH] spi: cadence-quadspi: Fix clock enable underflows due to runtime PM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b2c0f5b38ed01836c025f2672a883484765d91f5.camel@ti.com>
Date: Wed, 3 Dec 2025 16:12:04 +0530
From: Siddharth Vadapalli <s-vadapalli@...com>
To: Mark Brown <broonie@...nel.org>, Francesco Dolcini <francesco@...cini.it>,
	Anurag Dutta <a-dutta@...com>
CC: <linux-spi@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<stable@...r.kernel.org>, <s-vadapalli@...com>
Subject: Re: [PATCH] spi: cadence-quadspi: Fix clock enable underflows due
 to runtime PM

On Tue, 2025-12-02 at 22:53 +0000, Mark Brown wrote:
> The recent refactoring of where runtime PM is enabled done in commit
> f1eb4e792bb1 ("spi: spi-cadence-quadspi: Enable pm runtime earlier to
> avoid imbalance") made the fact that when we do a pm_runtime_disable()
> in the error paths of probe() we can trigger a runtime disable which in
> turn results in duplicate clock disables. Early on in the probe function
> we do a pm_runtime_get_noresume() since the probe function leaves the
> device in a powered up state but in the error path we can't assume that PM
> is enabled so we also manually disable everything, including clocks. This
> means that when runtime PM is active both it and the probe function release
> the same reference to the main clock for the IP, triggering warnings from
> the clock subsystem:
> 
> [    8.693719] clk:75:7 already disabled
> [    8.693791] WARNING: CPU: 1 PID: 185 at /usr/src/kernel/drivers/clk/clk.c:1188 clk_core_disable+0xa0/0xb
> ...
> [    8.694261]  clk_core_disable+0xa0/0xb4 (P)
> [    8.694272]  clk_disable+0x38/0x60
> [    8.694283]  cqspi_probe+0x7c8/0xc5c [spi_cadence_quadspi]
> [    8.694309]  platform_probe+0x5c/0xa4
> 
> Avoid this confused ownership by moving the pm_runtime_get_noresume() to
> after the last point at which the probe() function can fail.
> 
> Reported-by: Francesco Dolcini <francesco@...cini.it>
> Closes: https://lore.kernel.org/r/20251201072844.GA6785@francesco-nb
> Signed-off-by: Mark Brown <broonie@...nel.org>
> Cc: stable@...r.kernel.org
> ---

Thank you for the patch. The 'clock already disabled' issue persists on
J721E SoC with the patch applied:
https://gist.github.com/Siddharth-Vadapalli-at-TI/a664f59366ad681856b862d621487b7f

>  drivers/spi/spi-cadence-quadspi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> index af6d050da1c8..0833b6f666d0 100644
> --- a/drivers/spi/spi-cadence-quadspi.c
> +++ b/drivers/spi/spi-cadence-quadspi.c
> @@ -1985,7 +1985,6 @@ static int cqspi_probe(struct platform_device *pdev)
>  		pm_runtime_enable(dev);
>  		pm_runtime_set_autosuspend_delay(dev, CQSPI_AUTOSUSPEND_TIMEOUT);
>  		pm_runtime_use_autosuspend(dev);
> -		pm_runtime_get_noresume(dev);
>  	}
>  
>  	ret = cqspi_setup_flash(cqspi);
> @@ -2012,6 +2011,7 @@ static int cqspi_probe(struct platform_device *pdev)
>  	}
>  
>  	if (!(ddata && (ddata->quirks & CQSPI_DISABLE_RUNTIME_PM))) {
> +		pm_runtime_get_noresume(dev);
>  		pm_runtime_mark_last_busy(dev);
>  		pm_runtime_put_autosuspend(dev);
>  	}

Regards,
Siddharth.