drivers: spi: nxp_lpspi: Fix race condition in ISR

There was a race condition where `lpspi_end_xfer` can be called multiple
times per transfer. There was the case where a TX interrupt gets
triggered without the RX interrupt being set, and TX finishes writing
its last byte. Then, `spi_context_rx_len_left() == 0` is true and
`lpspi_end_xfer` happens, but the RX interrupt is still active. Then,
when the RX interrupt happens, `lpspi_end_xfer` will get called again.

To fix that, the architecture was adjusted to only call `lpspi_end_xfer`
once no interrupts are active any more, and the disabling of the
interrupts gets used to signal the end of the TX and RX part.

Minor adjustments were necessary to use the interrupt enable signals for
this purpose; the TX irq handler had its internal order reversed,
otherwise it wasn't guaranteed that the physical transfer is finished
when we disable the interrupt.

Also, the code where the RX interrupt gets disabled had to be moved out
of the RX irq handler, because the RX interrupt also needs to be
disabled if RX is finished but no RX interrupt is currently active.

Signed-off-by: Martin Stumpf <finomnis@gmail.com>
This commit is contained in:
Martin Stumpf 2025-05-06 19:35:37 +02:00 committed by Benjamin Cabé
commit 7dd6f94e57

View file

@ -89,11 +89,6 @@ static inline void lpspi_handle_rx_irq(const struct device *dev)
}
LOG_DBG("RX done %d words to spi buf", total_words_written);
if (spi_context_rx_len_left(ctx) == 0) {
base->IER &= ~LPSPI_IER_RDIE_MASK;
base->CR |= LPSPI_CR_RRF_MASK; /* flush rx fifo */
}
}
/* constructs the next word from the buffer */
@ -202,6 +197,17 @@ static inline void lpspi_handle_tx_irq(const struct device *dev)
struct lpspi_driver_data *lpspi_data = (struct lpspi_driver_data *)data->driver_data;
struct spi_context *ctx = &data->ctx;
base->SR = LPSPI_SR_TDF_MASK;
/* If we receive a TX interrupt but no more data is available,
* we can be sure that all data has been written to the bus.
* Disable the interrupt to signal that we are done.
*/
if (!spi_context_tx_on(ctx)) {
base->IER &= ~LPSPI_IER_TDIE_MASK;
return;
}
while (spi_context_tx_on(ctx) && lpspi_data->fill_len > 0) {
size_t this_buf_words_sent = MIN(lpspi_data->fill_len, ctx->tx_len);
@ -209,13 +215,6 @@ static inline void lpspi_handle_tx_irq(const struct device *dev)
lpspi_data->fill_len -= this_buf_words_sent;
}
base->SR = LPSPI_SR_TDF_MASK;
if (!spi_context_tx_on(ctx)) {
base->IER &= ~LPSPI_IER_TDIE_MASK;
return;
}
lpspi_next_tx_fill(data->dev);
}
@ -253,6 +252,11 @@ static void lpspi_isr(const struct device *dev)
lpspi_handle_tx_irq(dev);
}
if (spi_context_rx_len_left(ctx) == 0) {
base->IER &= ~LPSPI_IER_RDIE_MASK;
base->CR |= LPSPI_CR_RRF_MASK; /* flush rx fifo */
}
if (spi_context_tx_on(ctx)) {
return;
}
@ -275,7 +279,10 @@ static void lpspi_isr(const struct device *dev)
* need to end xfer in order to get last bit clocked out on bus.
*/
base->TCR |= LPSPI_TCR_CONT_MASK;
} else if (spi_context_rx_len_left(ctx) == 0) {
}
/* Both receive and transmit parts disable their interrupt once finished. */
if (base->IER == 0) {
lpspi_end_xfer(dev);
}
}