Re: spi: Regression with v7.0-rc1 on VisionFive 2

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 3462 bytes --]

On Wed, Mar 04, 2026 at 01:52:08PM -0800, Ron Economos wrote:
> On 3/4/26 09:02, Miquel Raynal wrote:

> > The other change that could be "it" is the change of order between reset
> > handling and clock. That would be quite messy if that was the error but
> > I cannot find another explanation. Ron, can you please try to revert
> > this patch locally and then move the clk_prepare_enable() of the APB and
> > AHB clocks earlier, right after the ref clock is also enabled? If the
> > platform fails to boot, there is maybe a weird internal relationship
> > with the resets.

> > Otherwise can you compare the clk_get_rate() on all three clocks in both
> > cases?

> I'm happy to help you debug this, but you have to send me a patch to
> the reverted file. I have no idea where the ref clock is enabled.

I think Miquel means

commit c9117602a87c441c4f05a2cc4d9be45c96140146
Author: Mark Brown <broonie@kernel.org>
Date:   Fri Mar 6 20:16:33 2026 +0000

    test

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 2d287950d44c..d74446f6db5a 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -1932,6 +1932,12 @@ static int cqspi_probe(struct platform_device *pdev)
 	reset_control_assert(rstc_ocp);
 	reset_control_deassert(rstc_ocp);
 
+	if (ddata->jh7110_clk_init) {
+		ret = cqspi_jh7110_clk_init(pdev, cqspi);
+		if (ret)
+			goto disable_clk;
+	}
+
 	cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
 	host->max_speed_hz = cqspi->master_ref_clk_hz;
 
@@ -1959,11 +1965,6 @@ static int cqspi_probe(struct platform_device *pdev)
 		if (ddata->quirks & CQSPI_NEEDS_APB_AHB_HAZARD_WAR)
 			cqspi->apb_ahb_hazard = true;
 
-		if (ddata->jh7110_clk_init) {
-			ret = cqspi_jh7110_clk_init(pdev, cqspi);
-			if (ret)
-				goto disable_clk;
-		}
 		if (ddata->quirks & CQSPI_DISABLE_STIG_MODE)
 			cqspi->disable_stig_mode = true;
 
which does the right thing:

   https://lava.sirena.org.uk/scheduler/job/2535610

but I'm fairly sure it's not an ordering thing.  The patch includes:

-       static struct clk_bulk_data qspiclk[] = {
-               { .id = "apb" },
-               { .id = "ahb" },
-       };

but never adds those names back, this means that while we do ask for
three clocks none of them have IDs specified so we just end up
requesting the same clock three times which might be what's needed on
most integrations but not here.  The below seems to do the trick for me,
I'll write a commit log and post - testing appreciated:

  https://lava.sirena.org.uk/scheduler/job/2535662

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 649ff55333f0..7a7f92e9c7a3 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -76,6 +76,11 @@ struct cqspi_flash_pdata {
 	u8		cs;
 };
 
+static struct clk_bulk_data cqspi_clks[CLK_QSPI_NUM] = {
+	[CLK_QSPI_APB] = { .id = "apb" },
+	[CLK_QSPI_AHB] = { .id = "ahb" },
+};
+
 struct cqspi_st {
 	struct platform_device	*pdev;
 	struct spi_controller	*host;
@@ -1823,6 +1828,7 @@ static int cqspi_probe(struct platform_device *pdev)
 	}
 
 	/* Obtain QSPI clocks. */
+	memcpy(&cqspi->clks, &cqspi_clks, sizeof(cqspi->clks));
 	ret = devm_clk_bulk_get_optional(dev, CLK_QSPI_NUM, cqspi->clks);
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to get clocks\n");

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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