Re: [PATCH v2 6/7] spi: cadence-xspi: Add support for panic writes - sashiko-bot

From: sashiko-bot@kernel.org
To: "Paul Cercueil" <paul.cercueil@bootlin.com>,
	"Pratyush Yadav" <pratyush@kernel.org>,
	"Michael Walle" <mwalle@kernel.org>,
	"Takahiro Kuwano" <takahiro.kuwano@infineon.com>,
	"Miquel Raynal" <miquel.raynal@bootlin.com>,
	"Richard Weinberger" <richard@nod.at>,
	"Vignesh Raghavendra" <vigneshr@ti.com>,
	"Mark Brown" <broonie@kernel.org>,
	"Thomas Petazzoni" <thomas.petazzoni@bootlin.com>
Cc: "Kees Cook" <kees@kernel.org>, "Tony Luck" <tony.luck@intel.com>,
	"Guilherme G . Piccoli" <gpiccoli@igalia.com>,
	linux-mtd@lists.infradead.org,
	"Bharat Bhushan" <bbhushan2@marvell.com>,
	"Sunil Kovvuri Goutham" <sgoutham@marvell.com>,
	"Tanmay Jagdale" <tanmay@marvell.com>
Subject: Re: [PATCH v2 6/7] spi: cadence-xspi: Add support for panic writes
Date: Tue, 13 Oct 2026 14:29:23 +0000	[thread overview]
Message-ID: <sashiko-outbox-169909@kernel.org> (raw)
In-Reply-To: <20261013141350.189996-7-paul.cercueil@bootlin.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] spi: cadence-xspi: Data corruption in panic writes due to stale interrupt status
--

commit f18a015e23b878b52027e1103a55a1cf4327cf51
Author: Tanmay Jagdale <tanmay@marvell.com>

spi: cadence-xspi: Add support for panic writes

Advertise support for panic writes. When a memory op is received with
the 'panic' flag set, the driver will then work without relying on
interrupts and without sleeping.

> diff --git a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-xspi.c
> index 185dffda54bbd..6310d0aa2d211 100644
> --- a/drivers/spi/spi-cadence-xspi.c
> +++ b/drivers/spi/spi-cadence-xspi.c
[ ... ]
> @@ -353,40 +358,33 @@ struct cdns_xspi_dev {
>  	int current_xfer_qword;
>  };
[ ... ]
>  static bool cdns_xspi_is_sdma_ready(struct cdns_xspi_dev *cdns_xspi, bool sleep)
>  {
>  	u32 ctrl_stat;
>  
> -	if (readl_relaxed_poll_timeout
> -	    (cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG,
> -	     ctrl_stat,
> -	     (ctrl_stat & CDNS_XSPI_SDMA_TRIGGER),
> -	     sleep ? MRVL_XSPI_POLL_DELAY_US : 0,
> -	     sleep ? MRVL_XSPI_POLL_TIMEOUT_US : 0)) {
> +	if (cdns_readl_poll(cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG,
> +			    ctrl_stat, (ctrl_stat & CDNS_XSPI_SDMA_TRIGGER),
> +			    MRVL_XSPI_POLL_DELAY_US, MRVL_XSPI_POLL_TIMEOUT_US,
> +			    sleep)) {
>  		return false;
>  	}
[ ... ]
> @@ -558,14 +556,14 @@ static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi,
>  	int ret;
>  	int dummybytes = op->dummy.nbytes;
>  
> -	ret = cdns_xspi_wait_for_controller_idle(cdns_xspi);
> +	ret = cdns_xspi_wait_for_controller_idle(cdns_xspi, !op->panic_write);
>  	if (ret < 0)
>  		return -EIO;
>  
>  	writel(FIELD_PREP(CDNS_XSPI_CTRL_WORK_MODE, CDNS_XSPI_WORK_MODE_STIG),
>  	       cdns_xspi->iobase + CDNS_XSPI_CTRL_CONFIG_REG);
>  
> -	if (cdns_xspi->irq >= 0)
> +	if (cdns_xspi->irq >= 0 && !op->panic_write)
>  		cdns_xspi->set_interrupts_handler(cdns_xspi, true);

[Severity: High]
Does the driver need to clear the sticky status bits in
CDNS_XSPI_INTR_STATUS_REG when bypassing the interrupt handler for panic
writes?

If a kernel panic occurs immediately after a previous hardware operation
finishes but before the interrupt handler can execute, bits like
CDNS_XSPI_SDMA_TRIGGER could remain set in the hardware status register.

Because the interrupt handler is bypassed here, would the polling loop in
cdns_xspi_is_sdma_ready() immediately read the stale CDNS_XSPI_SDMA_TRIGGER
bit as true and proceed prematurely before the hardware has prepared the new
transfer?

>  	cdns_xspi->sdma_error = false;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013141350.189996-1-paul.cercueil@bootlin.com?part=6

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/