From 7faceeb158b074a653af317d0271ac93eca41818 Mon Sep 17 00:00:00 2001 From: Bart Kodden Date: Wed, 19 Aug 2026 10:52:35 +0200 Subject: [PATCH 1/3] st25r3916CheckForReceivedInterrupts: optional bounded IRQ register reads Upstream gates the IRQ register read on the IRQ pin level, which assumes the pin belongs to exactly one ST25R3916. When several ICs share one IRQ input -- or the line is otherwise not trustworthy as per-IC state (wired-AND, diode bus, a level shifter that holds it) -- the level describes the bus, not the selected IC, and the guard fails both ways: held de-asserted nothing is ever collected and every RFAL wait times out; held asserted by a neighbour the while() cannot terminate, since this IC has nothing left to clear. Define ST25R3916_SHARED_IRQ_LINE to read this IC's registers unconditionally once per call and bound the repeat count (ST25R3916_ISR_MAX_REG_READS, default 8). Undefined, the upstream loop is compiled verbatim -- it stays live code so it cannot bit-rot, and reverting is undefining one macro. Co-Authored-By: Claude Opus 5 --- src/st25r3916_interrupt.cpp | 38 +++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/st25r3916_interrupt.cpp b/src/st25r3916_interrupt.cpp index b0cf3a6..e685dc7 100644 --- a/src/st25r3916_interrupt.cpp +++ b/src/st25r3916_interrupt.cpp @@ -48,6 +48,15 @@ /*! Length of the interrupt registers */ #define ST25R3916_INT_REGS_LEN ( (ST25R3916_REG_IRQ_TARGET - ST25R3916_REG_IRQ_MAIN) + 1U ) +#ifdef ST25R3916_SHARED_IRQ_LINE + /*! Upper bound on IRQ-register reads per st25r3916Isr() call. Only reached + * when another device on the shared IRQ line is holding it asserted, which + * this IC cannot clear. Rationale in st25r3916CheckForReceivedInterrupts(). */ + #ifndef ST25R3916_ISR_MAX_REG_READS + #define ST25R3916_ISR_MAX_REG_READS 8U + #endif +#endif /* ST25R3916_SHARED_IRQ_LINE */ + /* ****************************************************************************** * LOCAL DATA TYPES @@ -99,6 +108,34 @@ void RfalRfST25R3916Class::st25r3916CheckForReceivedInterrupts(void) ST_MEMSET(iregs, (int32_t)(ST25R3916_IRQ_MASK_ALL & 0xFFU), ST25R3916_INT_REGS_LEN); +#ifdef ST25R3916_SHARED_IRQ_LINE + /* Read this IC's IRQ registers at least once, then repeat while the pin is + * still asserted, up to a bounded number of reads. + * + * The default path below uses the pin level as the sole entry condition, which + * assumes the pin belongs to exactly one IC. When the line is shared, its + * level describes the bus rather than the selected IC, and it fails in both + * directions: held de-asserted, nothing is ever collected and every wait times + * out; held asserted by another IC, the while() cannot terminate because this + * IC has nothing left to clear. Reading unconditionally makes collection + * depend on the IC rather than on the wire, and the bound makes a foreign + * assertion cost a fixed number of SPI transactions instead of hanging. + * + * Undefine ST25R3916_SHARED_IRQ_LINE to get the upstream behaviour back; that + * is the correct choice as soon as the pin can represent per-IC state, i.e. + * one IRQ line per IC, or a true wired-OR of active-high outputs. */ + uint8_t reads = 0U; + do { + st25r3916ReadMultipleRegisters(ST25R3916_REG_IRQ_MAIN, iregs, ST25R3916_INT_REGS_LEN); + + irqStatus |= (uint32_t)iregs[0]; + irqStatus |= (uint32_t)iregs[1] << 8; + irqStatus |= (uint32_t)iregs[2] << 16; + irqStatus |= (uint32_t)iregs[3] << 24; + + reads++; + } while ((digitalRead(int_pin) == HIGH) && (reads < ST25R3916_ISR_MAX_REG_READS)); +#else /* In case the IRQ is Edge (not Level) triggered read IRQs until done */ while (digitalRead(int_pin) == HIGH) { st25r3916ReadMultipleRegisters(ST25R3916_REG_IRQ_MAIN, iregs, ST25R3916_INT_REGS_LEN); @@ -108,6 +145,7 @@ void RfalRfST25R3916Class::st25r3916CheckForReceivedInterrupts(void) irqStatus |= (uint32_t)iregs[2] << 16; irqStatus |= (uint32_t)iregs[3] << 24; } +#endif /* ST25R3916_SHARED_IRQ_LINE */ /* Forward all interrupts, even masked ones to application */ st25r3916interrupt.status |= irqStatus; From b05c713657da46586651e27acb5e1b43b41a295a Mon Sep 17 00:00:00 2001 From: Bart Kodden Date: Wed, 19 Aug 2026 10:53:49 +0200 Subject: [PATCH 2/3] st25r3916WaitForInterruptsTimed: optional IRQ register polling The wait loop only reads st25r3916interrupt.status and relies on a pin-edge ISR to fill it in concurrently. On a shared or untrustworthy IRQ line that edge is not dependable, so .status stays empty and the loop runs to its timeout on every transceive. Define ST25R3916_SHARED_IRQ_LINE to call st25r3916Isr() every ST25R3916_IRQ_POLL_INTERVAL_US (default 200) while waiting, fetching this IC's own registers over SPI so progress does not depend on the pin. Undefined, the upstream loop is compiled verbatim. Timing uses micros() from Arduino.h -- already relied on by this file for digitalRead() -- so the change adds no platform-specific symbol. Co-Authored-By: Claude Opus 5 --- src/st25r3916_interrupt.cpp | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/st25r3916_interrupt.cpp b/src/st25r3916_interrupt.cpp index e685dc7..03261c3 100644 --- a/src/st25r3916_interrupt.cpp +++ b/src/st25r3916_interrupt.cpp @@ -55,6 +55,12 @@ #ifndef ST25R3916_ISR_MAX_REG_READS #define ST25R3916_ISR_MAX_REG_READS 8U #endif + + /*! Interval at which st25r3916WaitForInterruptsTimed() polls the IRQ registers + * instead of trusting a pin edge to have filled in the interrupt status. */ + #ifndef ST25R3916_IRQ_POLL_INTERVAL_US + #define ST25R3916_IRQ_POLL_INTERVAL_US 200U + #endif #endif /* ST25R3916_SHARED_IRQ_LINE */ /* @@ -184,10 +190,36 @@ uint32_t RfalRfST25R3916Class::st25r3916WaitForInterruptsTimed(uint32_t mask, ui tmrDelay = timerCalculateTimer(tmo); +#ifdef ST25R3916_SHARED_IRQ_LINE + /* Poll this IC's IRQ registers while waiting, rather than only spinning on + * st25r3916interrupt.status. + * + * The default path below assumes a pin-edge ISR is filling in .status + * concurrently. On a shared IRQ line that edge is not dependable (see + * st25r3916CheckForReceivedInterrupts()), so .status can stay empty and the + * loop always runs to its timeout -- which is every transceive. Calling + * st25r3916Isr() on an interval fetches this IC's own registers over SPI, so + * progress no longer depends on the pin. + * + * Timing uses micros() rather than the RFAL timer, since timerIsExpired() is + * the coarse ms timer being used for the outer bound. The unsigned delta is + * wraparound-safe at this interval. */ + uint32_t last_us = micros(); + + do { + uint32_t now_us = micros(); + if ((now_us - last_us) >= ST25R3916_IRQ_POLL_INTERVAL_US) { + st25r3916Isr(); + last_us = now_us; + } + status = (st25r3916interrupt.status & mask); + } while ((!timerIsExpired(tmrDelay) || (tmo == 0U)) && (status == 0U)); +#else /* Run until specific interrupt has happen or the timer has expired */ do { status = (st25r3916interrupt.status & mask); } while ((!timerIsExpired(tmrDelay) || (tmo == 0U)) && (status == 0U)); +#endif /* ST25R3916_SHARED_IRQ_LINE */ status = st25r3916interrupt.status & mask; From cb98f36f05dea329b8598609545a90e84667ed7e Mon Sep 17 00:00:00 2001 From: Bart Kodden Date: Wed, 19 Aug 2026 12:17:33 +0200 Subject: [PATCH 3/3] Use st25r3916CheckForReceivedInterrupts() rather than st25r3916Isr() when polling, since only the status collection is needed and this avoids invoking the IRQ callback from the timed wait loop. --- src/st25r3916_interrupt.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/st25r3916_interrupt.cpp b/src/st25r3916_interrupt.cpp index 03261c3..362a618 100644 --- a/src/st25r3916_interrupt.cpp +++ b/src/st25r3916_interrupt.cpp @@ -198,8 +198,8 @@ uint32_t RfalRfST25R3916Class::st25r3916WaitForInterruptsTimed(uint32_t mask, ui * concurrently. On a shared IRQ line that edge is not dependable (see * st25r3916CheckForReceivedInterrupts()), so .status can stay empty and the * loop always runs to its timeout -- which is every transceive. Calling - * st25r3916Isr() on an interval fetches this IC's own registers over SPI, so - * progress no longer depends on the pin. + * st25r3916CheckForReceivedInterrupts() on an interval fetches this IC's + * own registers over SPI, so progress no longer depends on the pin. * * Timing uses micros() rather than the RFAL timer, since timerIsExpired() is * the coarse ms timer being used for the outer bound. The unsigned delta is @@ -209,7 +209,7 @@ uint32_t RfalRfST25R3916Class::st25r3916WaitForInterruptsTimed(uint32_t mask, ui do { uint32_t now_us = micros(); if ((now_us - last_us) >= ST25R3916_IRQ_POLL_INTERVAL_US) { - st25r3916Isr(); + st25r3916CheckForReceivedInterrupts(); last_us = now_us; } status = (st25r3916interrupt.status & mask);