The driver has no ->shutdown, and platform_drv_shutdown() has no
fallback of its own.  Unlike pci_device_shutdown(), which clears bus
mastering when kexec_in_progress, nothing on the platform bus disarms a
device that can still write to memory.  The normal kexec path never
calls ->suspend either, so the quiesce in zynqmp_qspi_suspend() is not
reached.

A controller that is still executing a DMA read may therefore keep
writing to memory across a kexec.  QSPIDMA_DST_ADDR still points at
memory owned by the kernel that called kexec, DST_SIZE is non-zero and
the flash is still clocked, so data can keep landing in RAM while the
new kernel is being relocated, and after it has started executing.

That destination is a physical address which means nothing to the new
kernel, so the writes can corrupt whatever now occupies it: kernel text
or data, page tables, or the initrd.  Nothing reports an error and the
resulting behaviour is undefined.

This can be observed by reading GQSPI_EN (offset 0x114) and
QSPIDMA_DST_ADDR/SIZE/STS/CTRL (offsets 0x800 to 0x80c) early in the new
kernel, before the driver probes: without this patch GQSPI_EN reads 1
and QSPIDMA_DST_ADDR still points into the previous kernel's memory.

Write 0 to GQSPI_EN_OFST, as zynqmp_qspi_remove() and
zynqmp_qspi_suspend() already do.  Skip it only when
pm_runtime_get_if_in_use() returns 0, i.e. runtime suspended: the clocks
are gated, so the registers are unreachable and the controller cannot be
mastering the bus.  A negative return is not the same thing - it is what
the CONFIG_PM=n stub always returns, and there probe() has enabled pclk
and refclk for good, so the controller is running and must be stopped.

Fixes: dfe11a11d523 ("spi: Add support for Zynq Ultrascale+ MPSoC GQSPI controller")
Cc: stable@vger.kernel.org
Signed-off-by: Itai Handler <itai.handler@gmail.com>
---
Note for stable: before commit 1c26372e5aa9 ("spi: spi-zynqmp-gqspi:
Update driver to use spi-mem framework") this driver stored the
spi_master in the platform drvdata rather than the zynqmp_qspi, so a
backport to those trees needs spi_master_get_devdata() in place of the
platform_get_drvdata() used here.

 drivers/spi/spi-zynqmp-gqspi.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/spi/spi-zynqmp-gqspi.c b/drivers/spi/spi-zynqmp-gqspi.c
index 4d55090..24f680b 100644
--- a/drivers/spi/spi-zynqmp-gqspi.c
+++ b/drivers/spi/spi-zynqmp-gqspi.c
@@ -1373,11 +1373,35 @@ static void zynqmp_qspi_remove(struct platform_device *pdev)
 	clk_disable_unprepare(xqspi->pclk);
 }
 
+static void zynqmp_qspi_shutdown(struct platform_device *pdev)
+{
+	struct zynqmp_qspi *xqspi = platform_get_drvdata(pdev);
+	int ret;
+
+	/*
+	 * Only a runtime suspended controller can be left alone: its clocks
+	 * are gated, so it cannot be mastering the bus, and its registers
+	 * must not be accessed either.  Any other answer means it may be
+	 * running and has to be stopped.  In particular, on a kernel built
+	 * without runtime PM this returns -EINVAL, and there the clocks
+	 * enabled in probe() are never gated at all.
+	 */
+	ret = pm_runtime_get_if_in_use(&pdev->dev);
+	if (!ret)
+		return;
+
+	zynqmp_gqspi_write(xqspi, GQSPI_EN_OFST, 0x0);
+
+	if (ret > 0)
+		pm_runtime_put_noidle(&pdev->dev);
+}
+
 MODULE_DEVICE_TABLE(of, zynqmp_qspi_of_match);
 
 static struct platform_driver zynqmp_qspi_driver = {
 	.probe = zynqmp_qspi_probe,
 	.remove = zynqmp_qspi_remove,
+	.shutdown = zynqmp_qspi_shutdown,
 	.driver = {
 		.name = "zynqmp-qspi",
 		.of_match_table = zynqmp_qspi_of_match,
-- 
2.34.1

On Thu, Sep 10, 2026 at 02:33:25PM +0300, Itai Handler wrote:

> +static void zynqmp_qspi_shutdown(struct platform_device *pdev)
> +{
> +	struct zynqmp_qspi *xqspi = platform_get_drvdata(pdev);
> +	int ret;
> +
> +	/*
> +	 * Only a runtime suspended controller can be left alone: its clocks
> +	 * are gated, so it cannot be mastering the bus, and its registers
> +	 * must not be accessed either.  Any other answer means it may be
> +	 * running and has to be stopped.  In particular, on a kernel built
> +	 * without runtime PM this returns -EINVAL, and there the clocks
> +	 * enabled in probe() are never gated at all.
> +	 */
> +	ret = pm_runtime_get_if_in_use(&pdev->dev);
> +	if (!ret)
> +		return;
> +
> +	zynqmp_gqspi_write(xqspi, GQSPI_EN_OFST, 0x0);

What ensures that nothing can start new transactions after this has run,
and if there's any operations in flight will the controller be OK with
just being stopped like this?

On Thu, Sep 10, 2026 at 7:56 PM Mark Brown <broonie@kernel.org> wrote:
>
> > +static void zynqmp_qspi_shutdown(struct platform_device *pdev)
> > +{
> > +     struct zynqmp_qspi *xqspi = platform_get_drvdata(pdev);
> > +     int ret;
> > +
> > +     /*
> > +      * Only a runtime suspended controller can be left alone: its clocks
> > +      * are gated, so it cannot be mastering the bus, and its registers
> > +      * must not be accessed either.  Any other answer means it may be
> > +      * running and has to be stopped.  In particular, on a kernel built
> > +      * without runtime PM this returns -EINVAL, and there the clocks
> > +      * enabled in probe() are never gated at all.
> > +      */
> > +     ret = pm_runtime_get_if_in_use(&pdev->dev);
> > +     if (!ret)
> > +             return;
> > +
> > +     zynqmp_gqspi_write(xqspi, GQSPI_EN_OFST, 0x0);
>
> What ensures that nothing can start new transactions after this has
> run,

Nothing does, and that is a real hole in v1 - thanks.

Two things narrow it without closing it: device_shutdown() has already
called device_block_probing(), and it walks devices_kset backwards, so
the SPI slaves and the MTD stack above this controller are shut down
before the controller itself. Neither of those stops a late spi_sync()
from anywhere else.

I will fix it in v2 by quiescing through the core first, which is what
this driver's own ->suspend already does:

        ret = spi_controller_suspend(xqspi->ctlr);
        if (ret)
                dev_warn(&pdev->dev, "could not stop the queue: %d\n", ret);

        ...
        zynqmp_gqspi_write(xqspi, GQSPI_EN_OFST, 0x0);

spi_controller_suspend() answers both halves: spi_stop_queue() waits for
ctlr->queue to empty and ctlr->busy to clear, and __spi_mark_suspended()
makes every subsequent __spi_sync() return -ESHUTDOWN. It can sleep,
which is fine here - device_shutdown() runs in process context and
already holds device_lock() across the callback.

Unlike ->suspend, shutdown cannot abort on error. If the queue has not
drained after spi_stop_queue()'s ~5 s cap I still want to write
GQSPI_EN, because a controller left mastering the bus into memory the
next kernel is about to reuse is worse than a truncated transfer. So the
return value gets logged rather than propagated.

I did consider making ->shutdown just call the remove path, as
spi-fsl-dspi and spi-bcm2835 do, and it would quiesce correctly here
too since zynqmp_qspi_remove() calls spi_unregister_controller() first.
But it then disables runtime PM, drops both clocks and tears down the
controller and its children, which is a lot more teardown than the
shutdown path needs and none of it stops the hardware any harder.
spi_controller_suspend() is the part that actually answers your
question, so I would rather call just that.

> and if there's any operations in flight will the controller be OK with
> just being stopped like this?

With the quiesce above there should be no message in flight by the time
of the write. For the case where one still is (the -EBUSY path):

- Writing GQSPI_EN_OFST is the driver's existing stop primitive.
  zynqmp_qspi_remove() and zynqmp_qspi_suspend() both do exactly this
  write, so this is not a new way of stopping the hardware, just a new
  caller.

- No controller state has to survive. Whatever runs next reinitialises
  it from scratch: zynqmp_qspi_init_hw() clears the ISR and the DMA
  status, writes GQSPI_EN 0, resets the TX, RX and generic FIFOs, and
  writes GQSPI_QSPIDMA_DST_CTRL_RESET_VAL. A power cycle obviously does
  the same. So a controller stopped mid-operation is recovered by the
  next probe.

- The flash is the part that outlives the write, and stopping the
  controller does not make its state worse. A read is aborted when CS is
  dropped and the chip goes back to idle. A program or erase carries on
  inside the chip whatever the controller does, and the next kernel sees
  WIP and waits for it. Waiting for those belongs above this driver - I
  have a separate patch making spi_nor_shutdown() take the flash lock so
  the upper layer does not walk away from its own operation - and it is
  not something a controller ->shutdown can fix.

The failure this is aimed at is the opposite case, where the controller
is left running with nobody driving it: after a kexec without this patch
GQSPI_EN still reads 1 and QSPIDMA_DST_ADDR still points into the
previous kernel's memory, so the engine keeps writing there while the
new kernel is relocated and started.

I will send v2 with the spi_controller_suspend() call shortly.