Re: [PATCH v4 4/7] spi: cadence-xspi: Clear "SDMA ready" IRQ flag after polling - 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>
Cc: "Kees Cook" <kees@kernel.org>, "Tony Luck" <tony.luck@intel.com>,
	"Guilherme G . Piccoli" <gpiccoli@igalia.com>,
	"Thomas Petazzoni" <thomas.petazzoni@bootlin.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 v4 4/7] spi: cadence-xspi: Clear "SDMA ready" IRQ flag after polling
Date: Fri, 16 Oct 2026 10:42:44 +0000	[thread overview]
Message-ID: <sashiko-outbox-174197@kernel.org> (raw)
In-Reply-To: <20261016102917.171095-5-paul.cercueil@bootlin.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] spi: shared IRQ handler steals W1C polling bits causing spurious timeouts
--

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

spi: cadence-xspi: Clear "SDMA ready" IRQ flag after polling

When polling for the "SDMA ready" event, once the corresponding bit is
set in the CDNS_XSPI_SDMA_TRIGGER we should reset it to zero. Otherwise,
any subsequent call to cdns_xspi_is_sdma_ready() will just return early.

> diff --git a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-xspi.c
> index 39c868a5b171c..095bf640b379a 100644
> --- a/drivers/spi/spi-cadence-xspi.c
> +++ b/drivers/spi/spi-cadence-xspi.c
> @@ -1068,12 +1068,20 @@ static bool cdns_xspi_is_sdma_ready(struct cdns_xspi_dev *cdns_xspi, bool sleep)
>  {
>  	u32 ctrl_stat;
>  
> -	return !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 (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)) {

[Severity: High]
Can the shared IRQ handler steal the W1C bits being polled here, causing a
spurious timeout?

Because cdns_xspi_probe() registers cdns_xspi_irq_handler() with the
IRQF_SHARED flag, an interrupt from another device on the same line could
trigger cdns_xspi_irq_handler() during this polling loop.

If that occurs, the handler executes the following:

drivers/spi/spi-cadence-xspi.c:cdns_xspi_irq_handler() {
    ...
    irq_status = readl(cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG);
    writel(irq_status, cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG);
    ...
}

This reads CDNS_XSPI_INTR_STATUS_REG and writes the exact value back, clearing
any active Write-1-to-Clear (W1C) bits. If CDNS_XSPI_SDMA_TRIGGER is set, the
IRQ handler would clear it before this polling loop observes it, leading to a
timeout and failing the SPI transfer.

> +		return false;
> +	}
> +
> +	/* Clear the flag */
> +	writel(CDNS_XSPI_SDMA_TRIGGER,
> +	       cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG);
> +
> +	return true;
>  }

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

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