linux-kernel - Re: [PATCH] spi: cadence-quadspi: Fix cqspi

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] [day] [month] [year] [list]

Message-ID: <5f288d65-4209-41cd-8f1a-3690f38da7ef@sirena.org.uk>
Date: Tue, 2 Dec 2025 16:28:25 +0000
From: Mark Brown <broonie@...nel.org>
To: Siddharth Vadapalli <s-vadapalli@...com>
Cc: Francesco Dolcini <francesco@...cini.it>, a-dutta@...com,
	linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, srk@...com
Subject: Re: [PATCH] spi: cadence-quadspi: Fix cqspi_probe() error handling
 for runtime pm

On Mon, Dec 01, 2025 at 06:08:07PM +0530, Siddharth Vadapalli wrote:
> On Mon, 2025-12-01 at 08:43 +0100, Francesco Dolcini wrote:
> > On Mon, Dec 01, 2025 at 08:28:44AM +0100, Francesco Dolcini wrote:

> > > > The error is due to the second invocation of 'clk_disable_unprepare()' on
> > > > 'cqspi->clk' in the error handling within 'cqspi_probe()', with the first
> > > > invocation being within 'cqspi_runtime_suspend()'.

> > > [    8.648915] cadence-qspi fc40000.spi: No flash device declared
> > > [    8.675671] cadence-qspi fc40000.spi: failed to setup flash parameters -19

> Could you please check if the driver's suspend callback has executed before
> the above? The patch intended to fix the very issue being reported.
> Therefore, the commit corresponding to the current patch might still
> trigger the issue. Ideally, reverting the commit for the current patch and
> the commit under Fixes (f1eb4e792bb1 ("spi: spi-cadence-quadspi: Enable pm
> runtime earlier to avoid imbalance"))
> should prevent the issue.

Given the above log we've failed at cqspi_setup_flash() so we shouldn't
have got as far as the code you changed for handling direct mode - I
think the actual issue is likely Anurag's earlier change f1eb4e792bb1
(spi: spi-cadence-quadspi: Enable pm runtime earlier to avoid imbalance)
that you mention, it means we've done a pm_runtime_get_noresume()
previously.  That shouldn't have done a clock enable due to the
noresume, but the get() being there will confuse pm_runtime_disable()
and cause it to invoke the suspend callback as Francesco identified.
This will have been failing previously if controller registration
failed, but it's now happening in more situations.

This is all very annoying since runtime PM can be configured off so we
can't rely on it's callbacks actually happening.  I *think* something
like the below ought to DTRT (I'd expect it should avoid the reported
error at least):

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);
 	}

Unfortunately none of my testing appears to show the issue.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)