spi_nxp_lpspi: Fix extra byte sent issue

drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c

@@ -12,6 +12,8 @@ LOG_MODULE_DECLARE(spi_lpspi, CONFIG_SPI_LOG_LEVEL);
 #include "spi_nxp_lpspi_priv.h"
 
 struct lpspi_driver_data {
+	size_t total_words_to_clock;
+	size_t words_clocked;
 	uint8_t word_size_bytes;
 	uint8_t lpspi_op_mode;
 };
@@ -72,10 +74,10 @@ static inline void lpspi_handle_rx_irq(const struct device *dev)
 	struct lpspi_data *data = dev->data;
 	struct lpspi_driver_data *lpspi_data = (struct lpspi_driver_data *)data->driver_data;
 	struct spi_context *ctx = &data->ctx;
-	uint8_t rx_fsr = rx_fifo_cur_len(base);
 	uint8_t total_words_written = 0;
 	uint8_t total_words_read = 0;
 	uint8_t words_read;
+	uint8_t rx_fsr;
 
 	base->SR = LPSPI_SR_RDF_MASK;
 
@@ -125,18 +127,22 @@ static inline void lpspi_fill_tx_fifo(const struct device *dev, const uint8_t *b
 		buf_len -= word_size;
 	}
 
+	lpspi_data->words_clocked += fill_len;
 	LOG_DBG("Filled TX FIFO to %d words (%d bytes)", fill_len, offset);
 }
 
 /* just fills TX fifo with the specified amount of NOPS */
 static void lpspi_fill_tx_fifo_nop(const struct device *dev, size_t fill_len)
 {
 	LPSPI_Type *base = (LPSPI_Type *)DEVICE_MMIO_NAMED_GET(dev, reg_base);
+	struct lpspi_data *data = dev->data;
+	struct lpspi_driver_data *lpspi_data = (struct lpspi_driver_data *)data->driver_data;
 
 	for (int i = 0; i < fill_len; i++) {
 		base->TDR = 0;
 	}
 
+	lpspi_data->words_clocked += fill_len;
 	LOG_DBG("Filled TX fifo with %d NOPs", fill_len);
 }
 
@@ -243,15 +249,14 @@ static void lpspi_isr(const struct device *dev)
 	struct lpspi_data *data = dev->data;
 	struct lpspi_driver_data *lpspi_data = (struct lpspi_driver_data *)data->driver_data;
 	uint8_t word_size_bytes = lpspi_data->word_size_bytes;
-	uint8_t op_mode = lpspi_data->lpspi_op_mode;
 	struct spi_context *ctx = &data->ctx;
 	uint32_t status_flags = base->SR;
 
-	if (status_flags & LPSPI_SR_RDF_MASK) {
+	if (status_flags & LPSPI_SR_RDF_MASK && base->IER & LPSPI_IER_RDIE_MASK) {
 		lpspi_handle_rx_irq(dev);
 	}
 
-	if (status_flags & LPSPI_SR_TDF_MASK) {
+	if (status_flags & LPSPI_SR_TDF_MASK && base->IER & LPSPI_IER_TDIE_MASK) {
 		lpspi_handle_tx_irq(dev);
 	}
 
@@ -264,65 +269,59 @@ static void lpspi_isr(const struct device *dev)
 		return;
 	}
 
-	/* the lpspi v1 has an errata where it doesn't clock the last bit
-	 * in continuous master mode until you write the TCR
-	 */
-	bool likely_stalling_v1 = (data->major_version < 2) && (op_mode == SPI_OP_MODE_MASTER) &&
-		(DIV_ROUND_UP(spi_context_rx_len_left(ctx, word_size_bytes), word_size_bytes) == 1);
-
-	if (spi_context_rx_on(ctx)) {
-		/* capture these values because they could change during this code block */
-		size_t rx_fifo_len = rx_fifo_cur_len(base);
-		size_t tx_fifo_len = tx_fifo_cur_len(base);
-
-		/*
-		 * Goal here is to provide the number of TX NOPS to match the amount of RX
-		 * we have left to receive, so that we provide the correct number of
-		 * clocks to the bus, since the clocks only happen when TX data is being sent.
-		 *
-		 * The expected RX left is essentially what is left in the spi transfer
-		 * minus what we have just got in the fifo, but prevent underflow of this
-		 * subtraction since it is unsigned.
-		 */
-		size_t expected_rx_left = rx_fifo_len < ctx->rx_len ? ctx->rx_len - rx_fifo_len : 0;
+	/* Both receive and transmit parts disable their interrupt once finished. */
+	if (base->IER == 0) {
+		lpspi_end_xfer(dev);
+		return;
+	}
 
+	/* only explanation at this point is one of two things:
+	 * 1) that rx is bigger than tx and we need to clock nops
+	 * 2) this is a version of LPSPI which will not clock the last bit of the transfer
+	 *    in continuous mode until the TCR is written to end the XFER
+	 */
 
-		/*
-		 * We know the expected amount of RX we have left but only fill up to the
-		 * max of the RX fifo size so that we don't have some overflow of the FIFO later.
-		 * Similarly, we shouldn't overfill the TX fifo with too many NOPs.
+	if (lpspi_data->words_clocked >= lpspi_data->total_words_to_clock) {
+		/* Due to stalling behavior on older LPSPI, if we know we already wrote all the
+		 * words into the fifo, then we need to end xfer manually by writing TCR
+		 * in order to get last bit clocked out on bus. So all we need to do is touch the
+		 * TCR by writing to fifo through TCR register and wait for final RX interrupt.
 		 */
-		size_t tx_fifo_space_left = config->tx_fifo_size - tx_fifo_len;
-		size_t rx_fifo_space_left = config->rx_fifo_size - rx_fifo_len;
-		size_t max_fifo_fill = MIN(tx_fifo_space_left, rx_fifo_space_left);
-		size_t max_fill = MIN(max_fifo_fill, expected_rx_left);
-
-		if (likely_stalling_v1 && max_fill > 0) {
-			max_fill -= 1;
-		}
+		base->TCR = base->TCR;
+		return;
+	}
 
-		/* If we already have some words in the tx fifo, we should count those */
-		if (max_fill > tx_fifo_len) {
-			max_fill -= tx_fifo_len;
-		} else {
-			max_fill = 0;
-		}
+	/* At this point we know only case is that we need to put NOPs in the TX fifo
+	 * in order to get the rest of the RX
+	 */
 
-		/* So now we want to fill the fifo with the max amount of NOPs */
-		lpspi_fill_tx_fifo_nop(dev, max_fill);
-	}
+	size_t rx_fifo_len = rx_fifo_cur_len(base);
+	size_t tx_fifo_len = tx_fifo_cur_len(base);
+	size_t words_really_left = lpspi_data->total_words_to_clock - lpspi_data->words_clocked;
+
+	/*
+	 * Goal here is to provide the number of TX NOPS to match the amount of RX
+	 * we have left to receive, so that we provide the correct number of
+	 * clocks to the bus, since the clocks only happen when TX data is being sent.
+	 *
+	 * The expected RX left is essentially what is left in the spi transfer
+	 * minus what we have just got in the fifo, but prevent underflow of this
+	 * subtraction since it is unsigned.
+	 */
+	size_t expected_rx_left = rx_fifo_len < words_really_left ?
+					words_really_left - rx_fifo_len : 0;
 
-	if (likely_stalling_v1) {
-		/* Due to stalling behavior on older LPSPI,
-		 * need to end xfer in order to get last bit clocked out on bus.
-		 */
-		base->TCR |= LPSPI_TCR_CONT_MASK;
-	}
+	/*
+	 * We know the expected amount of RX we have left but only fill up to the
+	 * max of the RX fifo size so that we don't have some overflow of the FIFO later.
+	 * Similarly, we shouldn't overfill the TX fifo with too many NOPs.
+	 */
+	size_t tx_fifo_space_left = config->tx_fifo_size - tx_fifo_len;
+	size_t rx_fifo_space_left = config->rx_fifo_size - rx_fifo_len;
+	size_t max_fifo_fill = MIN(tx_fifo_space_left, rx_fifo_space_left);
+	size_t max_fill = MIN(max_fifo_fill, expected_rx_left);
 
-	/* Both receive and transmit parts disable their interrupt once finished. */
-	if (base->IER == 0) {
-		lpspi_end_xfer(dev);
-	}
+	lpspi_fill_tx_fifo_nop(dev, max_fill);
 }
 
 static void lpspi_master_setup_native_cs(const struct device *dev, const struct spi_config *spi_cfg)
@@ -372,6 +371,14 @@ static int transceive(const struct device *dev, const struct spi_config *spi_cfg
 		goto error;
 	}
 
+	if (data->major_version < 2 && spi_cfg->operation & SPI_HOLD_ON_CS) {
+		/* on this version of LPSPI, due to errata in design
+		 * CS must be deasserted in order to clock all words,
+		 * so HOLD_ON_CS flag cannot be supported.
+		 */
+		return -EINVAL;
+	}
+
 	spi_context_buffers_setup(ctx, tx_bufs, rx_bufs, lpspi_data->word_size_bytes);
 	lpspi_data->lpspi_op_mode = op_mode;
 
@@ -384,6 +391,12 @@ static int transceive(const struct device *dev, const struct spi_config *spi_cfg
 	base->IER = 0;
 	base->SR |= LPSPI_INTERRUPT_BITS;
 
+	size_t max_side_clocks = MAX(spi_context_total_tx_len(ctx), spi_context_total_rx_len(ctx));
+
+	lpspi_data->total_words_to_clock =
+				DIV_ROUND_UP(max_side_clocks, lpspi_data->word_size_bytes);
+	lpspi_data->words_clocked = 0;
+
 	LOG_DBG("Starting LPSPI transfer");
 	spi_context_cs_control(ctx, true);
 

tests/drivers/spi/spi_loopback/src/spi.c

@@ -1017,6 +1017,8 @@ ZTEST(spi_extra_api_features, test_spi_hold_on_cs)
 early_exit:
 	hold_spec->config.operation &= ~SPI_HOLD_ON_CS;
 	zassert_false(ret, "SPI transceive failed, code %d", ret);
+	/* if there was no error then it was meant to be a skip at this point */
+	ztest_test_skip();
 }
 
 /*