On Thu, Dec 04, 2025 at 02:00:27PM -0600, Nishanth Menon wrote:
On 19:13-20251204, 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()
https://gist.github.com/nmenon/5ca89b617113e9dbb31d4630586af945#file-gistfil... next-20251204 + this patch -> The issue still exists at least on my platform.
Right, so from the log this is the one I think I mentioned earlier that the error path isn't being triggered by cqspi_setup_flash() but by something else which doesn't log anything:
[ 1.489445] 2840000.serial: ttyS4 at MMIO 0x2840000 (irq = 210, base_baud = 3000000) is a 8250 [ 1.498635] ------------[ cut here ]------------ [ 1.503239] clk:104:0 already disabled [ 1.507019] WARNING: drivers/clk/clk.c:1188 at clk_core_disable+0x80/0xa0, CPU#0: kworker/u8:2/55
so I'd not expect this to help in that case, it's specifically avoiding the issue Francesco reported where it's the DT parse. Can you put some debug statements in or something to confirm what triggers the error handling? Sorry, I'd not remembered that we'd got other triggers when I sent this.
Also, I wonder if the below completely untested change will help with that issue? I'm a bit nervous that this might mess up some power domain handling.
diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c index bdbeef05cd72..ff7beacbc085 100644 --- a/drivers/spi/spi-cadence-quadspi.c +++ b/drivers/spi/spi-cadence-quadspi.c @@ -1884,11 +1884,6 @@ static int cqspi_probe(struct platform_device *pdev) if (irq < 0) return -ENXIO;
- ret = pm_runtime_set_active(dev); - if (ret) - return ret; - - ret = clk_prepare_enable(cqspi->clk); if (ret) { dev_err(dev, "Cannot enable QSPI clock.\n"); @@ -1987,13 +1982,6 @@ static int cqspi_probe(struct platform_device *pdev) cqspi->current_cs = -1; cqspi->sclk = 0;
- if (!(ddata && (ddata->quirks & CQSPI_DISABLE_RUNTIME_PM))) { - pm_runtime_enable(dev); - pm_runtime_set_autosuspend_delay(dev, CQSPI_AUTOSUSPEND_TIMEOUT); - pm_runtime_use_autosuspend(dev); - pm_runtime_get_noresume(dev); - } - host->num_chipselect = cqspi->num_chipselect;
if (ddata && (ddata->quirks & CQSPI_SUPPORT_DEVICE_RESET)) @@ -2005,10 +1993,21 @@ static int cqspi_probe(struct platform_device *pdev) goto probe_setup_failed; }
+ if (!(ddata && (ddata->quirks & CQSPI_DISABLE_RUNTIME_PM))) { + ret = pm_runtime_set_active(dev); + if (ret) + goto err_probe_setup_failed; + + pm_runtime_enable(dev); + pm_runtime_set_autosuspend_delay(dev, CQSPI_AUTOSUSPEND_TIMEOUT); + pm_runtime_use_autosuspend(dev); + pm_runtime_get_noresume(dev); + } + ret = spi_register_controller(host); if (ret) { dev_err(&pdev->dev, "failed to register SPI ctlr %d\n", ret); - goto probe_setup_failed; + goto probe_pm_failed; }
if (!(ddata && (ddata->quirks & CQSPI_DISABLE_RUNTIME_PM))) { @@ -2017,9 +2016,10 @@ static int cqspi_probe(struct platform_device *pdev) }
return 0; -probe_setup_failed: +probe_pm_failed: if (!(ddata && (ddata->quirks & CQSPI_DISABLE_RUNTIME_PM))) pm_runtime_disable(dev); +probe_setup_failed: cqspi_controller_enable(cqspi, 0); probe_reset_failed: if (cqspi->is_jh7110)