Commit 4e0e3c99 authored by Mike J. Chen's avatar Mike J. Chen Committed by Fabio Baltieri
Browse files

drivers: dma: dma_mcux_lpc: fix src_inc/dst_inc for block chain



The dma driver was determining src_inc and dst_inc from the
config of the first block buffer and ignoring the config
flags for any additional buffers in the chain, which could
lead to incorrect transfers (e.g. in a multiple rx buffer
case, if the first buffer was to receive to NULL,
but the subsequent buffers were not NULL, the bug
would manifest as all transfers being made with
dst_inc of 0). Change the driver to setup
each dma descriptor according to the addr_adj flag
of each block_buffer.

Add check that peripheral transfers have addr_adj set to
NO_CHANGE instead of assuming it, to help catch errors.

Also now check for invalid addr_adj request of
decrement, which this controller doesn't support.

Signed-off-by: default avatarMike J. Chen <mjchen@google.com>
parent 1c4351a2
Loading
Loading
Loading
Loading
+109 −65
Original line number Diff line number Diff line
@@ -336,6 +336,101 @@ static void dma_mcux_lpc_clear_channel_data(struct channel_data *data)
	data->width = 0;
}

static int get_block_increments(const struct dma_config *config,
				struct dma_block_config *block_config, uint8_t *src_inc_p,
				uint8_t *dst_inc_p)
{
	uint8_t src_inc = 1;
	uint8_t dst_inc = 1;

	uint8_t width = config->dest_data_size;

	if ((block_config->source_addr_adj == DMA_ADDR_ADJ_DECREMENT) ||
	    (block_config->dest_addr_adj == DMA_ADDR_ADJ_DECREMENT)) {
		LOG_ERR("DMA_ADDR_ADJ_DECREMENT not supported");
		return -EINVAL;
	}

	switch (config->channel_direction) {
	case MEMORY_TO_MEMORY:
	case HOST_TO_MEMORY:
	case MEMORY_TO_HOST:
		if (block_config->source_gather_en && (block_config->source_gather_interval != 0)) {
			src_inc = block_config->source_gather_interval / width;
			/* The current controller only supports incrementing the
			 * source and destination up to 4 time transfer width
			 */
			if ((src_inc > 4) || (src_inc == 3)) {
				return -EINVAL;
			}
		}

		if (block_config->dest_scatter_en && (block_config->dest_scatter_interval != 0)) {
			dst_inc = block_config->dest_scatter_interval / width;
			/* The current controller only supports incrementing the
			 * source and destination up to 4 time transfer width
			 */
			if ((dst_inc > 4) || (dst_inc == 3)) {
				return -EINVAL;
			}
		}
		break;
	case LPC_DMA_SPI_MCUX_FLEXCOMM_TX:
	case MEMORY_TO_PERIPHERAL:
		/* Set the source increment value */
		if (block_config->source_gather_en) {
			src_inc = block_config->source_gather_interval / width;
			/* The current controller only supports incrementing the
			 * source and destination up to 4 time transfer width
			 */
			if ((src_inc > 4) || (src_inc == 3)) {
				return -EINVAL;
			}
		}

		dst_inc = 0;
		if (block_config->dest_addr_adj != DMA_ADDR_ADJ_NO_CHANGE) {
			LOG_ERR("DMA to peripheral must set DMA_ADDR_ADJ_NO_CHANGE");
			return -EINVAL;
		}
		break;
	case PERIPHERAL_TO_MEMORY:
		src_inc = 0;
		if (block_config->source_addr_adj != DMA_ADDR_ADJ_NO_CHANGE) {
			LOG_ERR("DMA from peripheral must set DMA_ADDR_ADJ_NO_CHANGE");
			return -EINVAL;
		}

		/* Set the destination increment value */
		if (block_config->dest_scatter_en) {
			dst_inc = block_config->dest_scatter_interval / width;
			/* The current controller only supports incrementing the
			 * source and destination up to 4 time transfer width
			 */
			if ((dst_inc > 4) || (dst_inc == 3)) {
				return -EINVAL;
			}
		}
		break;
	default:
		LOG_ERR("not support transfer direction");
		return -EINVAL;
	}

	/* Check if user does not want to increment address */
	if (block_config->source_addr_adj == DMA_ADDR_ADJ_NO_CHANGE) {
		src_inc = 0;
	}
	if (block_config->dest_addr_adj == DMA_ADDR_ADJ_NO_CHANGE) {
		dst_inc = 0;
	}

	*src_inc_p = src_inc;
	*dst_inc_p = dst_inc;

	return 0;
}

/* Configure a channel */
static int dma_mcux_lpc_configure(const struct device *dev, uint32_t channel,
				  struct dma_config *config)
@@ -348,8 +443,8 @@ static int dma_mcux_lpc_configure(const struct device *dev, uint32_t channel,
	struct dma_block_config *block_config;
	uint32_t virtual_channel;
	uint8_t otrig_index;
	uint8_t src_inc = 1, dst_inc = 1;
	bool is_periph = true;
	uint8_t src_inc, dst_inc;
	bool is_periph;
	uint8_t width;
	uint32_t max_xfer_bytes;
	uint8_t reload = 0;
@@ -414,73 +509,16 @@ static int dma_mcux_lpc_configure(const struct device *dev, uint32_t channel,
		return -EINVAL;
	}

	switch (config->channel_direction) {
	case MEMORY_TO_MEMORY:
	case HOST_TO_MEMORY:
	case MEMORY_TO_HOST:
	if ((config->channel_direction == MEMORY_TO_PERIPHERAL) ||
	    (config->channel_direction == PERIPHERAL_TO_MEMORY)) {
		is_periph = true;
	} else {
		is_periph = false;
		if (block_config->source_gather_en && (block_config->source_gather_interval != 0)) {
			src_inc = block_config->source_gather_interval / width;
			/* The current controller only supports incrementing the
			 * source and destination up to 4 time transfer width
			 */
			if ((src_inc > 4) || (src_inc == 3)) {
				return -EINVAL;
			}
	}

		if (block_config->dest_scatter_en && (block_config->dest_scatter_interval != 0)) {
			dst_inc = block_config->dest_scatter_interval / width;
			/* The current controller only supports incrementing the
			 * source and destination up to 4 time transfer width
			 */
			if ((dst_inc > 4) || (dst_inc == 3)) {
	if (get_block_increments(config, block_config, &src_inc, &dst_inc) != 0) {
		return -EINVAL;
	}
		}
		break;
	case LPC_DMA_SPI_MCUX_FLEXCOMM_TX:
	case MEMORY_TO_PERIPHERAL:
		/* Set the source increment value */
		if (block_config->source_gather_en) {
			src_inc = block_config->source_gather_interval / width;
			/* The current controller only supports incrementing the
			 * source and destination up to 4 time transfer width
			 */
			if ((src_inc > 4) || (src_inc == 3)) {
				return -EINVAL;
			}
		}

		dst_inc = 0;
		break;
	case PERIPHERAL_TO_MEMORY:
		src_inc = 0;

		/* Set the destination increment value */
		if (block_config->dest_scatter_en) {
			dst_inc = block_config->dest_scatter_interval / width;
			/* The current controller only supports incrementing the
			 * source and destination up to 4 time transfer width
			 */
			if ((dst_inc > 4) || (dst_inc == 3)) {
				return -EINVAL;
			}
		}
		break;
	default:
		LOG_ERR("not support transfer direction");
		return -EINVAL;
	}

	/* Check if user does not want to increment address */
	if (block_config->source_addr_adj == DMA_ADDR_ADJ_NO_CHANGE) {
		src_inc = 0;
	}

	if (block_config->dest_addr_adj == DMA_ADDR_ADJ_NO_CHANGE) {
		dst_inc = 0;
	}

	/* If needed, allocate a slot to store dma channel data */
	if (dma_data->channel_index[channel] == -1) {
@@ -682,6 +720,12 @@ static int dma_mcux_lpc_configure(const struct device *dev, uint32_t channel,
		block_config = block_config->next_block;

		while (block_config != NULL) {
			/* Each block can have a different configuration so update
			 * src_inc/dst_inc based on the block_config.
			 */
			if (get_block_increments(config, block_config, &src_inc, &dst_inc) != 0) {
				return -EINVAL;
			}
			block_config->source_reload_en = reload;

			/* DMA controller requires that the address be aligned to transfer size */
+3 −0
Original line number Diff line number Diff line
@@ -426,6 +426,7 @@ static void i2s_mcux_config_dma_blocks(const struct device *dev,
	if (dir == I2S_DIR_RX) {

		blk_cfg->source_address = (uint32_t)&base->FIFORD;
		blk_cfg->source_addr_adj = DMA_ADDR_ADJ_NO_CHANGE;
		blk_cfg->dest_address = (uint32_t)buffer[0];
		blk_cfg->block_size = block_size;
		blk_cfg->next_block = &dev_data->rx_dma_blocks[1];
@@ -433,10 +434,12 @@ static void i2s_mcux_config_dma_blocks(const struct device *dev,

		blk_cfg = &dev_data->rx_dma_blocks[1];
		blk_cfg->source_address = (uint32_t)&base->FIFORD;
		blk_cfg->source_addr_adj = DMA_ADDR_ADJ_NO_CHANGE;
		blk_cfg->dest_address = (uint32_t)buffer[1];
		blk_cfg->block_size = block_size;
	} else {
		blk_cfg->dest_address = (uint32_t)&base->FIFOWR;
		blk_cfg->dest_addr_adj = DMA_ADDR_ADJ_NO_CHANGE;
		blk_cfg->source_address = (uint32_t)buffer;
		blk_cfg->block_size = block_size;
	}
+2 −0
Original line number Diff line number Diff line
@@ -531,6 +531,7 @@ static int mcux_flexcomm_uart_tx(const struct device *dev, const uint8_t *buf,
	data->tx_data.xfer_len = len;
	data->tx_data.active_block.source_address = (uint32_t)buf;
	data->tx_data.active_block.dest_address = (uint32_t) &config->base->FIFOWR;
	data->tx_data.active_block.dest_addr_adj = DMA_ADDR_ADJ_NO_CHANGE;
	data->tx_data.active_block.block_size = len;
	data->tx_data.active_block.next_block = NULL;

@@ -659,6 +660,7 @@ static int mcux_flexcomm_uart_rx_enable(const struct device *dev, uint8_t *buf,
	data->rx_data.xfer_len = len;
	data->rx_data.active_block.dest_address = (uint32_t)data->rx_data.xfer_buf;
	data->rx_data.active_block.source_address = (uint32_t) &config->base->FIFORD;
	data->tx_data.active_block.source_addr_adj = DMA_ADDR_ADJ_NO_CHANGE;
	data->rx_data.active_block.block_size = data->rx_data.xfer_len;

	ret = dma_config(config->rx_dma.dev, config->rx_dma.channel,
+3 −0
Original line number Diff line number Diff line
@@ -400,6 +400,7 @@ static int spi_mcux_dma_tx_load(const struct device *dev, const struct spi_confi
	/* tx direction has memory as source and periph as dest. */
	blk_cfg->dest_address = (uint32_t)&base->FIFOWR;
	blk_cfg->dest_addr_adj = DMA_ADDR_ADJ_NO_CHANGE;
	blk_cfg->dest_scatter_en = 0;
	blk_cfg->source_gather_en = 0;
	if (last_packet) {
		data->last_word = spi_mcux_get_last_tx_word(spi_cfg, buf, len, config->def_char,
@@ -480,6 +481,8 @@ static int spi_mcux_dma_rx_load(const struct device *dev, uint8_t *buf, size_t l
	}
	/* rx direction has periph as source and mem as dest. */
	blk_cfg->source_address = (uint32_t)&base->FIFORD;
	blk_cfg->source_addr_adj = DMA_ADDR_ADJ_NO_CHANGE;
	blk_cfg->source_gather_en = 0;
	if (buf == NULL) {
		/* if rx buff is null, then write data to dummy address. */
		blk_cfg->dest_address = (uint32_t)&dummy_rx_buffer;