When the size of a transfer exceeds the size of the FIFO (32 bytes), RX overflows will be generated and receive data will be corrupted and warnings will be produced. For example, here's an error generated by a transfer of 36 bytes:
spi_master spi0: mchp_corespi_interrupt: RX OVERFLOW: rxlen: 4, txlen: 0
The driver is currently split between handling receiving in the interrupt handler, and sending outside of it. Move all handling out of the interrupt handling, and explicitly link the number of bytes read of of the RX FIFO to the number written into the TX one. This both resolves the overflow problems as well as simplifying the flow of the driver.
CC: stable@vger.kernel.org Fixes: 9ac8d17694b6 ("spi: add support for microchip fpga spi controllers") Signed-off-by: Conor Dooley conor.dooley@microchip.com ---
Contrary to last time, the reporter did actually manage to this this version, and it resolved their probably apparently.
Changes in v2: - Move all FIFO interactions out of the interrupt handler - Disable the non-error interrupts since they're dealt with out of the isr now
v1: https://lore.kernel.org/all/20250114-easiness-pregame-d1d2d4b57e7b@spud/
CC: Conor Dooley conor.dooley@microchip.com CC: Daire McNamara daire.mcnamara@microchip.com CC: Mark Brown broonie@kernel.org CC: linux-spi@vger.kernel.org CC: linux-kernel@vger.kernel.org --- drivers/spi/spi-microchip-core.c | 41 ++++++++++++++------------------ 1 file changed, 18 insertions(+), 23 deletions(-)
diff --git a/drivers/spi/spi-microchip-core.c b/drivers/spi/spi-microchip-core.c index 5b6af55855efc..62ba0bd9cbb7e 100644 --- a/drivers/spi/spi-microchip-core.c +++ b/drivers/spi/spi-microchip-core.c @@ -70,8 +70,7 @@ #define INT_RX_CHANNEL_OVERFLOW BIT(2) #define INT_TX_CHANNEL_UNDERRUN BIT(3)
-#define INT_ENABLE_MASK (CONTROL_RX_DATA_INT | CONTROL_TX_DATA_INT | \ - CONTROL_RX_OVER_INT | CONTROL_TX_UNDER_INT) +#define INT_ENABLE_MASK (CONTROL_RX_OVER_INT | CONTROL_TX_UNDER_INT)
#define REG_CONTROL (0x00) #define REG_FRAME_SIZE (0x04) @@ -133,10 +132,15 @@ static inline void mchp_corespi_disable(struct mchp_corespi *spi) mchp_corespi_write(spi, REG_CONTROL, control); }
-static inline void mchp_corespi_read_fifo(struct mchp_corespi *spi) +static inline void mchp_corespi_read_fifo(struct mchp_corespi *spi, int fifo_max) { - while (spi->rx_len >= spi->n_bytes && !(mchp_corespi_read(spi, REG_STATUS) & STATUS_RXFIFO_EMPTY)) { - u32 data = mchp_corespi_read(spi, REG_RX_DATA); + for (int i = 0; i < fifo_max; i++) { + u32 data; + + while (mchp_corespi_read(spi, REG_STATUS) & STATUS_RXFIFO_EMPTY) + ; + + data = mchp_corespi_read(spi, REG_RX_DATA);
spi->rx_len -= spi->n_bytes;
@@ -211,11 +215,10 @@ static inline void mchp_corespi_set_xfer_size(struct mchp_corespi *spi, int len) mchp_corespi_write(spi, REG_FRAMESUP, len); }
-static inline void mchp_corespi_write_fifo(struct mchp_corespi *spi) +static inline void mchp_corespi_write_fifo(struct mchp_corespi *spi, int fifo_max) { - int fifo_max, i = 0; + int i = 0;
- fifo_max = DIV_ROUND_UP(min(spi->tx_len, FIFO_DEPTH), spi->n_bytes); mchp_corespi_set_xfer_size(spi, fifo_max);
while ((i < fifo_max) && !(mchp_corespi_read(spi, REG_STATUS) & STATUS_TXFIFO_FULL)) { @@ -413,19 +416,6 @@ static irqreturn_t mchp_corespi_interrupt(int irq, void *dev_id) if (intfield == 0) return IRQ_NONE;
- if (intfield & INT_TXDONE) - mchp_corespi_write(spi, REG_INT_CLEAR, INT_TXDONE); - - if (intfield & INT_RXRDY) { - mchp_corespi_write(spi, REG_INT_CLEAR, INT_RXRDY); - - if (spi->rx_len) - mchp_corespi_read_fifo(spi); - } - - if (!spi->rx_len && !spi->tx_len) - finalise = true; - if (intfield & INT_RX_CHANNEL_OVERFLOW) { mchp_corespi_write(spi, REG_INT_CLEAR, INT_RX_CHANNEL_OVERFLOW); finalise = true; @@ -512,9 +502,14 @@ static int mchp_corespi_transfer_one(struct spi_controller *host,
mchp_corespi_write(spi, REG_SLAVE_SELECT, spi->pending_slave_select);
- while (spi->tx_len) - mchp_corespi_write_fifo(spi); + while (spi->tx_len) { + int fifo_max = DIV_ROUND_UP(min(spi->tx_len, FIFO_DEPTH), spi->n_bytes);
+ mchp_corespi_write_fifo(spi, fifo_max); + mchp_corespi_read_fifo(spi, fifo_max); + } + + spi_finalize_current_transfer(host); return 1; }
On Mon, 03 Mar 2025 10:47:40 +0000, Conor Dooley wrote:
When the size of a transfer exceeds the size of the FIFO (32 bytes), RX overflows will be generated and receive data will be corrupted and warnings will be produced. For example, here's an error generated by a transfer of 36 bytes:
spi_master spi0: mchp_corespi_interrupt: RX OVERFLOW: rxlen: 4, txlen: 0
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
Thanks!
[1/1] spi: microchip-core: prevent RX overflows when transmit size > FIFO size commit: 91cf42c63f2d8a9c1bcdfe923218e079b32e1a69
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
On Mon, Mar 03, 2025 at 10:47:40AM +0000, Conor Dooley wrote:
When the size of a transfer exceeds the size of the FIFO (32 bytes), RX overflows will be generated and receive data will be corrupted and warnings will be produced. For example, here's an error generated by a transfer of 36 bytes:
spi_master spi0: mchp_corespi_interrupt: RX OVERFLOW: rxlen: 4, txlen: 0
The driver is currently split between handling receiving in the interrupt handler, and sending outside of it. Move all handling out of the interrupt handling, and explicitly link the number of bytes read of of the RX FIFO to the number written into the TX one. This both resolves the overflow problems as well as simplifying the flow of the driver.
...
+static inline void mchp_corespi_read_fifo(struct mchp_corespi *spi, int fifo_max) {
- for (int i = 0; i < fifo_max; i++) {
u32 data;
while (mchp_corespi_read(spi, REG_STATUS) & STATUS_RXFIFO_EMPTY);
What may go wrong with this code? :-) I think limiting the infinite loop by a timeout is a good thing to have.
data = mchp_corespi_read(spi, REG_RX_DATA);spi->rx_len -= spi->n_bytes;
mchp_corespi_write(spi, REG_FRAMESUP, len); }
linux-stable-mirror@lists.linaro.org
-
Andy Shevchenko -
Conor Dooley -
Mark Brown