Re: [PATCH] spi: axi-spi-engine: fix stale SYNC IRQ pending

[Jonathan Santos <jonath4nns@gmail.com>]

On 09/03, David Lechner wrote:
> On 9/3/26 8:49 AM, Jonathan Santos wrote:
> > spi_engine_setup() sends a SYNC(1) command and polls SYNC_ID to confirm
> > it was parsed by the FPGA, but never clears the corresponding interrupt
> > pending bit (INT_PENDING[SYNC]). When the first real SPI transfer starts
> > and INT_SYNC is enabled, that stale pending bit fires immediately, causing
> > the IRQ handler to see the leftover SYNC_ID from setup, match it against
> > the current transfer's ID, and prematurely signal completion before the
> > hardware finishes.
> > 
> > This race manifests at low SPI clock frequencies (~2-3 MHz), where the
> > FPGA takes long enough to execute the transfer that handler is parsed
> > before it finishes. At higher SCLK rates the transfer completes fast
> > enough that the issue is masked.
> > 
> > Fix this by clearing INT_PENDING[SYNC] after the polled SYNC, ensuring no
> > stale interrupt is left pending.
> > 
> > Reported-by: Dennis Heinzel <dennis.heinzel@irs.systems>
> > Link: https://ez.analog.com/linux-software-drivers/f/q-a/604145/axi-spi-engine-stale-sync-pending-can-complete-first-transfer-early-at-low-spi-clock-2-3-mhz
> 
> Should be Closes rather than Link in this case.
> 
> And needs a Fixes tag.
> 
> > Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> > ---
> >  drivers/spi/spi-axi-spi-engine.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
> > index 02bbc5d0cfc5..9e9bbe109ce5 100644
> > --- a/drivers/spi/spi-axi-spi-engine.c
> > +++ b/drivers/spi/spi-axi-spi-engine.c
> > @@ -887,6 +887,7 @@ static int spi_engine_setup(struct spi_device *device)
> >  	struct spi_controller *host = device->controller;
> >  	struct spi_engine *spi_engine = spi_controller_get_devdata(host);
> >  	unsigned int reg;
> > +	int ret;
> >  
> >  	if (device->mode & SPI_CS_HIGH)
> >  		spi_engine->cs_inv |= BIT(spi_get_chipselect(device, 0));
> > @@ -922,8 +923,13 @@ static int spi_engine_setup(struct spi_device *device)
> >  	writel_relaxed(SPI_ENGINE_CMD_SYNC(1),
> >  		       spi_engine->base + SPI_ENGINE_REG_CMD_FIFO);
> >  
> > -	return readl_relaxed_poll_timeout(spi_engine->base + SPI_ENGINE_REG_SYNC_ID,
> > -					  reg, reg == 1, 1, 1000);
> > +	ret = readl_relaxed_poll_timeout(spi_engine->base + SPI_ENGINE_REG_SYNC_ID,
> > +					 reg, reg == 1, 1, 1000);
> > +
> > +	/* Clear the stale SYNC pending bit so it doesn't fire when the IRQ is later enabled */
> > +	writel_relaxed(SPI_ENGINE_INT_SYNC, spi_engine->base + SPI_ENGINE_REG_INT_PENDING);
> > +
> > +	return ret;
> >  }
> >  
> >  static int spi_engine_transfer_one_message(struct spi_controller *host,
> > 
> > base-commit: 183f05a300eab41e4578337eac59335730dfebf9
> 
> We have the same poll timeout in spi_engine_trigger_enable(). Do we need
> a similar fix there too?
>

Yes, Thanks for pointing that out! We have the same issue there:
The _trigger_enable() generates the SYNC interrupt, which is not cleared
while in offload mode and it is fired in the next FIFO mode transfer
when the SYNC interrupt is enabled.

I will include this in the next version.