Commit 0c1e9c3e authored by Mike J. Chen's avatar Mike J. Chen Committed by Fabio Baltieri
Browse files

Revert "drivers: spi: mcux_flexcomm: use rxignore bit instead of dummy read." 



This reverts commit a3530d6a.

This change incorrect if chip-select is via a GPIO pin,
released by spi_context_cs_control(), and not the controller
default chip-select pin that can be released using a final
FIFOWR.

When GPIO is used for chip-select control, the chip-select could
be released too soon because the transfer callback is invoked
when the last byte is put into the TX FIFO but that byte might
not yet have clocked out yet on the SPI pins. The rx part of
the transfer is really what completes the transfer so ignoring
it is incorrect.

We could try to special case and use ignore only when
spi_context_cs_control() does nothing, but since it is a very
small optimization, it doesn't seem worth the extra
complexity.

Signed-off-by: default avatarMike J. Chen <mjchen@google.com>
parent 60e5a562

drivers/spi/spi_mcux_flexcomm.c

+19 −24
Original line number Diff line number Diff line
@@ -316,6 +316,9 @@ static int spi_mcux_configure(const struct device *dev, 
}



#ifdef CONFIG_SPI_MCUX_FLEXCOMM_DMA

/* Dummy buffer used as a sink when rc buf is null */

uint32_t dummy_rx_buffer;



/* This function is executed in the interrupt context */

static void spi_mcux_dma_callback(const struct device *dev, void *arg,

			 uint32_t channel, int status)
@@ -351,7 +354,7 @@ static void spi_mcux_dma_callback(const struct device *dev, void *arg, 


static void spi_mcux_prepare_txlastword(uint32_t *txLastWord,

				const uint8_t *buf, const struct spi_config *spi_cfg,

				size_t len, bool rx_ignore)

				size_t len)

{

	uint32_t word_size;
@@ -364,10 +367,6 @@ static void spi_mcux_prepare_txlastword(uint32_t *txLastWord, 
		*txLastWord = buf[len - 1U];

	}



	if (rx_ignore) {

		*txLastWord |= (uint32_t)SPI_FIFOWR_RXIGNORE_MASK;

	}



	*txLastWord |= (uint32_t)SPI_FIFOWR_EOT_MASK;



	*txLastWord |= ((uint32_t)SPI_DEASSERT_ALL &
@@ -378,8 +377,7 @@ static void spi_mcux_prepare_txlastword(uint32_t *txLastWord, 
}



static void spi_mcux_prepare_txdummy(uint32_t *dummy, bool last_packet,

				     const struct spi_config *spi_cfg,

				     bool rx_ignore)

				const struct spi_config *spi_cfg)

{

	uint32_t word_size;
@@ -388,9 +386,6 @@ static void spi_mcux_prepare_txdummy(uint32_t *dummy, bool last_packet, 
	if (last_packet) {

		*dummy |= (uint32_t)SPI_FIFOWR_EOT_MASK;

	}

	if (rx_ignore) {

		*dummy |= (uint32_t)SPI_FIFOWR_RXIGNORE_MASK;

	}



	*dummy |= ((uint32_t)SPI_DEASSERT_ALL &

				(~(uint32_t)SPI_DEASSERTNUM_SSEL((uint32_t)spi_cfg->slave)));
@@ -400,8 +395,7 @@ static void spi_mcux_prepare_txdummy(uint32_t *dummy, bool last_packet, 
}



static int spi_mcux_dma_tx_load(const struct device *dev, const uint8_t *buf,

				const struct spi_config *spi_cfg, size_t len,

				bool last_packet, bool rx_ignore)

				 const struct spi_config *spi_cfg, size_t len, bool last_packet)

{

	const struct spi_mcux_config *cfg = dev->config;

	struct spi_mcux_data *data = dev->data;
@@ -424,11 +418,11 @@ static int spi_mcux_dma_tx_load(const struct device *dev, const uint8_t *buf, 
	if (buf == NULL) {

		data->dummy_tx_buffer = 0;

		data->last_word = 0;

		spi_mcux_prepare_txdummy(&data->dummy_tx_buffer, last_packet, spi_cfg, rx_ignore);

		spi_mcux_prepare_txdummy(&data->dummy_tx_buffer, last_packet, spi_cfg);



		if (last_packet  &&

		    ((word_size > 8) ? (len > 2U) : (len > 1U))) {

			spi_mcux_prepare_txdummy(&data->last_word, last_packet, spi_cfg, rx_ignore);

			spi_mcux_prepare_txdummy(&data->last_word, last_packet, spi_cfg);

			blk_cfg->source_address = (uint32_t)&data->dummy_tx_buffer;

			blk_cfg->dest_address = (uint32_t)&base->FIFOWR;

			blk_cfg->block_size = (word_size > 8) ?
@@ -457,7 +451,7 @@ static int spi_mcux_dma_tx_load(const struct device *dev, const uint8_t *buf, 
		}

	} else {

		if (last_packet) {

			spi_mcux_prepare_txlastword(&data->last_word, buf, spi_cfg, len, rx_ignore);

			spi_mcux_prepare_txlastword(&data->last_word, buf, spi_cfg, len);

		}

		/* If last packet and data transfer frame is bigger then 1,

		 * use dma descriptor to send the last data.
@@ -505,7 +499,7 @@ static int spi_mcux_dma_tx_load(const struct device *dev, const uint8_t *buf, 


	uint32_t tmpData = 0U;



	spi_mcux_prepare_txdummy(&tmpData, last_packet, spi_cfg, rx_ignore);

	spi_mcux_prepare_txdummy(&tmpData, last_packet, spi_cfg);



	/* Setup the control info.

	 * Halfword writes to just the control bits (offset 0xE22) doesn't push
@@ -538,11 +532,6 @@ static int spi_mcux_dma_rx_load(const struct device *dev, uint8_t *buf, 
	/* retrieve active RX DMA channel (used in callback) */

	struct stream *stream = &data->dma_rx;



	if (buf == NULL) {

		data->status_flags |= SPI_MCUX_FLEXCOMM_DMA_RX_DONE_FLAG;

		return 0;

	}



	blk_cfg = &stream->dma_blk_cfg[0];



	/* prepare the block for this RX DMA channel */
@@ -550,7 +539,14 @@ static int spi_mcux_dma_rx_load(const struct device *dev, uint8_t *buf, 
	blk_cfg->block_size = len;



	/* rx direction has periph as source and mem as dest. */

	if (buf == NULL) {

		/* if rx buff is null, then write data to dummy address. */

		blk_cfg->dest_address = (uint32_t)&dummy_rx_buffer;

		blk_cfg->dest_addr_adj = DMA_ADDR_ADJ_NO_CHANGE;

	} else {

		blk_cfg->dest_address = (uint32_t)buf;

	}



	blk_cfg->source_address = (uint32_t)&base->FIFORD;



	/* direction is given by the DT */
@@ -576,7 +572,6 @@ static int spi_mcux_dma_move_buffers(const struct device *dev, size_t len, 
			const struct spi_config *spi_cfg, bool last_packet)

{

	struct spi_mcux_data *data = dev->data;

	bool rx_ignore = data->ctx.rx_buf ? false : true;

	int ret;



	ret = spi_mcux_dma_rx_load(dev, data->ctx.rx_buf, len);
@@ -586,7 +581,7 @@ static int spi_mcux_dma_move_buffers(const struct device *dev, size_t len, 
	}



	ret = spi_mcux_dma_tx_load(dev, data->ctx.tx_buf, spi_cfg,

				   len, last_packet, rx_ignore);

							len, last_packet);



	return ret;

}

CRA Git | Maintained and supported by SUSTech CRA and CCSE