[PATCH v2 0/2] spi: axi-spi-engine: fix stale SYNC IRQ pending

[PATCH v2 0/2] spi: axi-spi-engine: fix stale SYNC IRQ pending

[Jonathan Santos]

Both spi_engine_setup() and spi_engine_trigger_enable() send a SYNC(1)
command and poll SYNC_ID, but never clear INT_PENDING[SYNC] afterward.
This leaves a stale pending bit that fires spuriously when INT_SYNC is
enabled later by some driver, causing premature transfer completion at
low SPI clock frequencies (more likely). Fix both functions by clearing
the pending bit after polling.

v2 summary:
* Replaced "Link" tag for "Closes" tag.
* Included "Fixes" tag.
* Added new patch applying similar fix for the spi_engine_trigger_enable(),
  as pointed out by David.
* While at it, replaced plain "1000" by "1 * USEC_PER_MSEC" for better
  clarity.
* Link to v1: https://lore.kernel.org/linux-spi/04d51e99b8cddce51113db933d806e83848f04f5.1788313558.git.Jonathan.Santos@analog.com/T/#t

Jonathan Santos (2):
  spi: axi-spi-engine: fix stale SYNC IRQ pending
  spi: axi-spi-engine: fix stale SYNC IRQ pending in trigger_enable

 drivers/spi/spi-axi-spi-engine.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)


base-commit: a3b3580713f3ac5a32dc2874ee546828977a1d68
-- 
2.34.1

[PATCH v2 1/2] spi: axi-spi-engine: fix stale SYNC IRQ pending

[Jonathan Santos]

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>
Closes: 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
Fixes: 1d0ee0c9df31 ("spi: axi-spi-engine: wait for completion in setup")
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..0de3af5c429c 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, 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);
+
+	return ret;
 }
 
 static int spi_engine_transfer_one_message(struct spi_controller *host,
-- 
2.34.1

[PATCH v2 2/2] spi: axi-spi-engine: fix stale SYNC IRQ pending in trigger_enable

[Jonathan Santos]

spi_engine_trigger_enable() sends SYNC(1) and polls SYNC_ID, it never
clears the INT_PENDING[SYNC] bit afterward. This leaves a stale pending
interrupt that can fire spuriously when INT_SYNC is later enabled.

Simillarly to the fix on spi_engine_setup(), clear INT_PENDING[SYNC]
after polling the SYNC_ID.

Fixes: 8fc13b822c74 ("spi: axi-spi-engine: don't repeat mode config for offload")
Signed-off-by: Jonathan Santos <Jonathan.Santos@analog.com>
---
 drivers/spi/spi-axi-spi-engine.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
index 0de3af5c429c..d0e5b7bccf55 100644
--- a/drivers/spi/spi-axi-spi-engine.c
+++ b/drivers/spi/spi-axi-spi-engine.c
@@ -1034,7 +1034,11 @@ static int spi_engine_trigger_enable(struct spi_offload *offload)
 		spi_engine->base + SPI_ENGINE_REG_CMD_FIFO);
 
 	ret = readl_relaxed_poll_timeout(spi_engine->base + SPI_ENGINE_REG_SYNC_ID,
-					 reg, reg == 1, 1, 1000);
+					 reg, reg == 1, 1, 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);
+
 	if (ret)
 		return ret;
 
-- 
2.34.1

Re: [PATCH v2 0/2] spi: axi-spi-engine: fix stale SYNC IRQ pending

[Nuno Sá]

On Tue, Sep 29, 2026 at 11:17:06PM -0300, Jonathan Santos wrote:
> Both spi_engine_setup() and spi_engine_trigger_enable() send a SYNC(1)
> command and poll SYNC_ID, but never clear INT_PENDING[SYNC] afterward.
> This leaves a stale pending bit that fires spuriously when INT_SYNC is
> enabled later by some driver, causing premature transfer completion at
> low SPI clock frequencies (more likely). Fix both functions by clearing
> the pending bit after polling.
> 
> v2 summary:
> * Replaced "Link" tag for "Closes" tag.
> * Included "Fixes" tag.
> * Added new patch applying similar fix for the spi_engine_trigger_enable(),
>   as pointed out by David.
> * While at it, replaced plain "1000" by "1 * USEC_PER_MSEC" for better
>   clarity.
> * Link to v1: https://lore.kernel.org/linux-spi/04d51e99b8cddce51113db933d806e83848f04f5.1788313558.git.Jonathan.Santos@analog.com/T/#t
> 
> Jonathan Santos (2):
>   spi: axi-spi-engine: fix stale SYNC IRQ pending
>   spi: axi-spi-engine: fix stale SYNC IRQ pending in trigger_enable
> 
>  drivers/spi/spi-axi-spi-engine.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> 
> base-commit: a3b3580713f3ac5a32dc2874ee546828977a1d68
> -- 
> 2.34.1
> 

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

Re: [PATCH v2 2/2] spi: axi-spi-engine: fix stale SYNC IRQ pending in trigger_enable

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 705 bytes --]

On Tue, Sep 29, 2026 at 11:17:45PM -0300, Jonathan Santos wrote:
> spi_engine_trigger_enable() sends SYNC(1) and polls SYNC_ID, it never
> clears the INT_PENDING[SYNC] bit afterward. This leaves a stale pending
> interrupt that can fire spuriously when INT_SYNC is later enabled.

> +
> +	/* 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);
> +
>  	if (ret)
>  		return ret;

I'm not seeing anything here for the issue with ensuring posted writes
are completed before we drop the lock?  The write may not be visible on
enother CPU, we need a read from the controller or something.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 2/2] spi: axi-spi-engine: fix stale SYNC IRQ pending in trigger_enable

[Nuno Sá]

On Wed, 2026-09-30 at 14:09 +0100, Mark Brown wrote:
> On Tue, Sep 29, 2026 at 11:17:45PM -0300, Jonathan Santos wrote:
> > spi_engine_trigger_enable() sends SYNC(1) and polls SYNC_ID, it never
> > clears the INT_PENDING[SYNC] bit afterward. This leaves a stale pending
> > interrupt that can fire spuriously when INT_SYNC is later enabled.
> 
> > +
> > +	/* 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);
> > +
> >  	if (ret)
> >  		return ret;
> 
> I'm not seeing anything here for the issue with ensuring posted writes
> are completed before we drop the lock?  The write may not be visible on
> enother CPU, we need a read from the controller or something.

Just for my understanding, why would it be an issue? The trigger register seems to
only be accessible from the trigger callbacks which are protected by a lock where we
have implied barriers. Im sure I'm missing what can go wrong here :)

- Nuno Sá

Re: [PATCH v2 2/2] spi: axi-spi-engine: fix stale SYNC IRQ pending in trigger_enable

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 691 bytes --]

On Thu, Oct 01, 2026 at 07:55:45AM +0100, Nuno Sá wrote:
> On Wed, 2026-09-30 at 14:09 +0100, Mark Brown wrote:

> > I'm not seeing anything here for the issue with ensuring posted writes
> > are completed before we drop the lock?  The write may not be visible on
> > enother CPU, we need a read from the controller or something.

> Just for my understanding, why would it be an issue? The trigger register seems to
> only be accessible from the trigger callbacks which are protected by a lock where we
> have implied barriers. Im sure I'm missing what can go wrong here :)

Are you sure that's true for mutexes?  I thought they only covered
memory, not device stuff, but ICBW.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 2/2] spi: axi-spi-engine: fix stale SYNC IRQ pending in trigger_enable

[Nuno Sá]

On Thu, Oct 01, 2026 at 12:58:52PM +0100, Mark Brown wrote:
> On Thu, Oct 01, 2026 at 07:55:45AM +0100, Nuno Sá wrote:
> > On Wed, 2026-09-30 at 14:09 +0100, Mark Brown wrote:
> 
> > > I'm not seeing anything here for the issue with ensuring posted writes
> > > are completed before we drop the lock?  The write may not be visible on
> > > enother CPU, we need a read from the controller or something.
> 
> > Just for my understanding, why would it be an issue? The trigger register seems to
> > only be accessible from the trigger callbacks which are protected by a lock where we
> > have implied barriers. Im sure I'm missing what can go wrong here :)
> 
> Are you sure that's true for mutexes?  I thought they only covered
> memory, not device stuff, but ICBW.

Oh right, likely not a full barrier

- Nuno Sá