1. Patchew
  2. linux
  3. View series

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

Jonathan Santos posted 1 patch 3 weeks, 1 day ago 
drivers/spi/spi-axi-spi-engine.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
[PATCH] spi: axi-spi-engine: fix stale SYNC IRQ pending
Posted by Jonathan Santos 3 weeks, 1 day ago 
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
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
-- 
2.34.1
Re: [PATCH] spi: axi-spi-engine: fix stale SYNC IRQ pending
Posted by Mark Brown 3 weeks, 1 day ago 
On Thu, Sep 03, 2026 at 10:49:22AM -0300, 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

> +	/* 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);

Is there something later which posts the write?
Re: [PATCH] spi: axi-spi-engine: fix stale SYNC IRQ pending
Posted by Andy Shevchenko 3 weeks, 1 day ago 
On Thu, Sep 03, 2026 at 10:49:22AM -0300, 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

We have a Closes tag.

> Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>

...

> +	ret = readl_relaxed_poll_timeout(spi_engine->base + SPI_ENGINE_REG_SYNC_ID,
> +					 reg, reg == 1, 1, 1000);

While at it I would replace 1000 with 1 * USEC_PER_MSEC

> +	/* 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);

In both cases? Error (timeout) and not?

> +	return ret;

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH] spi: axi-spi-engine: fix stale SYNC IRQ pending
Posted by Jonathan Santos 3 weeks ago 
On 09/03, Andy Shevchenko wrote:
> On Thu, Sep 03, 2026 at 10:49:22AM -0300, 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
> 
> We have a Closes tag.
> 
> > Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
> 
> ...
> 
> > +	ret = readl_relaxed_poll_timeout(spi_engine->base + SPI_ENGINE_REG_SYNC_ID,
> > +					 reg, reg == 1, 1, 1000);
> 
> While at it I would replace 1000 with 1 * USEC_PER_MSEC
> 
> > +	/* 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);
> 
> In both cases? Error (timeout) and not?
> 

The error (timeout) indicates the SYNC command was not parsed within the
deadline, but it can be executed at any time. We consider the timeout big
enough, so this is unlikely to happen. But in any case, the cpu command to
clear INT_PENDING is harmeless and can still clear the interrupt if the 
SYNC is done parsing until right before this command is executed.

> > +	return ret;
> 
> -- 
> With Best Regards,
> Andy Shevchenko
>
Re: [PATCH] spi: axi-spi-engine: fix stale SYNC IRQ pending
Posted by David Lechner 3 weeks, 1 day ago 
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?
Re: [PATCH] spi: axi-spi-engine: fix stale SYNC IRQ pending
Posted by Jonathan Santos 3 weeks ago 
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.

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.