dma: Fix error_callback enable/disable confusion

teburd · nashif · commit 4180d70439f5 · 2024-04-11T17:08:10.000-04:00
Previously the logic was inverted for error_callback_en where 0 was
enablement and 1 was disable. This was likely done so that the default,
sensibly so, was to enable the error callback if possible. A variety of
in tree users had confused the enable/disable value.

Change the name of the flag to error_callback_dis where the default
remains 0 (do not disable the callback!) and correct in tree uses of the
flag where it seemed incorrect.

Signed-off-by: Tom Burdick &lt;thomas.burdick@intel.com&gt;
diff --git a/drivers/dma/dma_andes_atcdmac300.c b/drivers/dma/dma_andes_atcdmac300.c
@@ -273,7 +273,7 @@ static int dma_atcdmac300_config(const struct device *dev, uint32_t channel,
 	ch_ctrl |= DMA_CH_CTRL_INTABT;
 
 	/* Disable the error callback */
-	if (!cfg->error_callback_en) {
+	if (!cfg->error_callback_dis) {
 		ch_ctrl |= DMA_CH_CTRL_INTERR;
 	}
 
diff --git a/drivers/dma/dma_emul.c b/drivers/dma/dma_emul.c
@@ -129,7 +129,7 @@ static const char *dma_emul_xfer_config_to_string(const struct dma_config *cfg)
 		 "\n\tslot: %u"
 		 "\n\tchannel_direction: %u"
 		 "\n\tcomplete_callback_en: %u"
-		 "\n\terror_callback_en: %u"
+		 "\n\terror_callback_dis: %u"
 		 "\n\tsource_handshake: %u"
 		 "\n\tdest_handshake: %u"
 		 "\n\tchannel_priority: %u"
@@ -148,7 +148,7 @@ static const char *dma_emul_xfer_config_to_string(const struct dma_config *cfg)
 		 "\n\tdma_callback: %p"
 		 "\n}",
 		 cfg->dma_slot, cfg->channel_direction, cfg->complete_callback_en,
-		 cfg->error_callback_en, cfg->source_handshake, cfg->dest_handshake,
+		 cfg->error_callback_dis, cfg->source_handshake, cfg->dest_handshake,
 		 cfg->channel_priority, cfg->source_chaining_en, cfg->dest_chaining_en,
 		 cfg->linked_channel, cfg->cyclic, cfg->_reserved, cfg->source_data_size,
 		 cfg->dest_data_size, cfg->source_burst_length, cfg->dest_burst_length,
@@ -248,11 +248,11 @@ static void dma_emul_work_handler(struct k_work *work)
 
 				if (state == DMA_EMUL_CHANNEL_STOPPED) {
 					LOG_DBG("asynchronously canceled");
-					if (xfer_config.error_callback_en) {
+					if (!xfer_config.error_callback_dis) {
 						xfer_config.dma_callback(dev, xfer_config.user_data,
 									 channel, -ECANCELED);
 					} else {
-						LOG_DBG("error_callback_en is not set (async "
+						LOG_DBG("error_callback_dis is not set (async "
 							"cancel)");
 					}
 					goto out;
diff --git a/drivers/dma/dma_mchp_xec.c b/drivers/dma/dma_mchp_xec.c
@@ -272,7 +272,7 @@ static int check_blocks(struct dma_xec_channel *chdata, struct dma_block_config
  * dma_slot - peripheral source/target ID. Not used for Mem2Mem
  * channel_direction - HW supports Mem2Mem, Mem2Periph, and Periph2Mem
  * complete_callback_en - if true invoke callback on completion (no error)
- * error_callback_en - if true invoke callback on error
+ * error_callback_dis - if true disable callback on error
  * source_handshake - 0=HW, 1=SW
  * dest_handshake - 0=HW, 1=SW
  * channel_priority - 4-bit field. HW implements round-robin only.
@@ -384,7 +384,7 @@ static int dma_xec_configure(const struct device *dev, uint32_t channel,
 	if (config->complete_callback_en) {
 		chdata->flags |= BIT(DMA_XEC_CHAN_FLAGS_CB_EOB_POS);
 	}
-	if (config->error_callback_en) { /* disable callback on errors ? */
+	if (config->error_callback_dis) { /* disable callback on errors ? */
 		chdata->flags |= BIT(DMA_XEC_CHAN_FLAGS_CB_ERR_DIS_POS);
 	}
 
@@ -690,7 +690,7 @@ static int dmac_xec_pm_action(const struct device *dev,
  * completion_callback_en
  *	0 = invoke at completion of all blocks
  *	1 = invoke at completin of each block
- * error_callback_en
+ * error_callback_dis
  *	0 = invoke on all errors
  *	1 = disabled, do not invoke on errors
  */
diff --git a/drivers/dma/dma_sam0.c b/drivers/dma/dma_sam0.c
@@ -125,7 +125,7 @@ static int dma_sam0_config(const struct device *dev, uint32_t channel,
 
 	/* Enable the interrupts */
 	DMA_REGS->CHINTENSET.reg = DMAC_CHINTENSET_TCMPL;
-	if (!config->error_callback_en) {
+	if (!config->error_callback_dis) {
 		DMA_REGS->CHINTENSET.reg = DMAC_CHINTENSET_TERR;
 	} else {
 		DMA_REGS->CHINTENCLR.reg = DMAC_CHINTENSET_TERR;
@@ -179,7 +179,7 @@ static int dma_sam0_config(const struct device *dev, uint32_t channel,
 
 	/* Enable the interrupts */
 	chcfg->CHINTENSET.reg = DMAC_CHINTENSET_TCMPL;
-	if (!config->error_callback_en) {
+	if (!config->error_callback_dis) {
 		chcfg->CHINTENSET.reg = DMAC_CHINTENSET_TERR;
 	} else {
 		chcfg->CHINTENCLR.reg = DMAC_CHINTENSET_TERR;
diff --git a/drivers/dma/dma_sam_xdmac.c b/drivers/dma/dma_sam_xdmac.c
@@ -264,7 +264,7 @@ static int sam_xdmac_config(const struct device *dev, uint32_t channel,
 	channel_cfg.dus = 0U;
 	channel_cfg.cie =
 		  (cfg->complete_callback_en ? XDMAC_CIE_BIE : XDMAC_CIE_LIE)
-		| (cfg->error_callback_en ? XDMAC_INT_ERR : 0);
+		| (cfg->error_callback_dis ? 0 : XDMAC_INT_ERR);
 
 	ret = sam_xdmac_channel_configure(dev, channel, &channel_cfg);
 	if (ret < 0) {
diff --git a/drivers/dma/dma_sedi.c b/drivers/dma/dma_sedi.c
@@ -57,7 +57,7 @@ static void dma_handler(sedi_dma_t dma_device, int channel, int event_id,
 		    (config->complete_callback_en)) {
 			config->dma_callback(dev, config->user_data,
 					channel, 0);
-		} else if (config->error_callback_en) {
+		} else if (!config->error_callback_dis) {
 			config->dma_callback(dev, config->user_data,
 					channel, event_id);
 		}
diff --git a/drivers/dma/dma_smartbond.c b/drivers/dma/dma_smartbond.c
@@ -483,7 +483,7 @@ static int dma_smartbond_config(const struct device *dev, uint32_t channel, stru
 	}
 
 	/* Error handling is not supported; just warn user. */
-	if (cfg->error_callback_en) {
+	if (!cfg->error_callback_dis) {
 		LOG_WRN("Error handling is not supported");
 	}
 
diff --git a/drivers/dma/dma_xmc4xxx.c b/drivers/dma/dma_xmc4xxx.c
@@ -294,7 +294,7 @@ static int dma_xmc4xxx_config(const struct device *dev, uint32_t channel, struct
 		XMC_DMA_CH_EnableEvent(dma, channel, XMC_DMA_CH_EVENT_BLOCK_TRANSFER_COMPLETE);
 	}
 
-	if (config->error_callback_en) {
+	if (!config->error_callback_dis) {
 		XMC_DMA_CH_EnableEvent(dma, channel, XMC_DMA_CH_EVENT_ERROR);
 	}
 
diff --git a/drivers/i2c/i2c_dw.c b/drivers/i2c/i2c_dw.c
@@ -124,7 +124,7 @@ int32_t i2c_dw_idma_rx_transfer(const struct device *dev)
 	dma_cfg.dma_callback = cb_i2c_idma_transfer;
 	dma_cfg.user_data = (void *)dev;
 	dma_cfg.complete_callback_en = 0U;
-	dma_cfg.error_callback_en = 1U;
+	dma_cfg.error_callback_dis = 0U;
 	dma_cfg.block_count = 1U;
 	dma_cfg.head_block = &dma_block_cfg;
 
@@ -172,7 +172,7 @@ int32_t i2c_dw_idma_tx_transfer(const struct device *dev,
 	dma_cfg.dma_callback = cb_i2c_idma_transfer;
 	dma_cfg.user_data = (void *)dev;
 	dma_cfg.complete_callback_en = 0U;
-	dma_cfg.error_callback_en = 1U;
+	dma_cfg.error_callback_dis = 0U;
 	dma_cfg.block_count = 1U;
 	dma_cfg.head_block = &dma_block_cfg;
 
diff --git a/drivers/i2s/i2s_mcux_flexcomm.c b/drivers/i2s/i2s_mcux_flexcomm.c
@@ -449,7 +449,7 @@ static void i2s_mcux_config_dma_blocks(const struct device *dev,
 	LOG_DBG("channel_direction is %d", stream->dma_cfg.channel_direction);
 	LOG_DBG("complete_callback_en is %d",
 		stream->dma_cfg.complete_callback_en);
-	LOG_DBG("error_callback_en is %d", stream->dma_cfg.error_callback_en);
+	LOG_DBG("error_callback_dis is %d", stream->dma_cfg.error_callback_dis);
 	LOG_DBG("source_handshake is %d", stream->dma_cfg.source_handshake);
 	LOG_DBG("dest_handshake is %d", stream->dma_cfg.dest_handshake);
 	LOG_DBG("channel_priority is %d", stream->dma_cfg.channel_priority);
diff --git a/drivers/i2s/i2s_mcux_sai.c b/drivers/i2s/i2s_mcux_sai.c
@@ -1303,7 +1303,7 @@ static const struct i2s_driver_api i2s_mcux_driver_api = {
 				CONFIG_I2S_EDMA_BURST_SIZE,		\
 			  .dma_callback = i2s_dma_tx_callback,		\
 			  .complete_callback_en = 1,			\
-			  .error_callback_en = 1,			\
+			  .error_callback_dis = 1,			\
 			  .block_count = 1,				\
 			  .head_block =					\
 				&i2s_##i2s_id##_data.tx.dma_block,	\
@@ -1323,7 +1323,7 @@ static const struct i2s_driver_api i2s_mcux_driver_api = {
 				CONFIG_I2S_EDMA_BURST_SIZE,		\
 			  .dma_callback = i2s_dma_rx_callback,		\
 			  .complete_callback_en = 1,			\
-			  .error_callback_en = 1,			\
+			  .error_callback_dis = 1,			\
 			  .block_count = 1,				\
 			  .head_block =					\
 				&i2s_##i2s_id##_data.rx.dma_block,	\
diff --git a/drivers/serial/uart_mcux_flexcomm.c b/drivers/serial/uart_mcux_flexcomm.c
@@ -1144,7 +1144,7 @@ DT_INST_FOREACH_STATUS_OKAY(UART_MCUX_FLEXCOMM_RX_TIMEOUT_FUNC);
 			.source_data_size = 1,					\
 			.dest_data_size = 1,					\
 			.complete_callback_en = 1,				\
-			.error_callback_en = 1,					\
+			.error_callback_dis = 1,				\
 			.block_count = 1,					\
 			.head_block =						\
 				&mcux_flexcomm_##n##_data.tx_data.active_block,	\
@@ -1165,7 +1165,7 @@ DT_INST_FOREACH_STATUS_OKAY(UART_MCUX_FLEXCOMM_RX_TIMEOUT_FUNC);
 			.source_data_size = 1,					\
 			.dest_data_size = 1,					\
 			.complete_callback_en = 1,				\
-			.error_callback_en = 1,					\
+			.error_callback_dis = 1,				\
 			.block_count = 1,					\
 			.head_block =						\
 				&mcux_flexcomm_##n##_data.rx_data.active_block,	\
diff --git a/drivers/serial/uart_mcux_lpuart.c b/drivers/serial/uart_mcux_lpuart.c
@@ -1176,7 +1176,7 @@ static const struct uart_driver_api mcux_lpuart_driver_api = {
 			.source_data_size = 1,						       \
 			.dest_data_size = 1,						       \
 			.complete_callback_en = 1,					       \
-			.error_callback_en = 1,						       \
+			.error_callback_dis = 0,					       \
 			.block_count = 1,						       \
 			.head_block =							       \
 				&mcux_lpuart_##id##_data.async.tx_dma_params.active_dma_block, \
@@ -1199,7 +1199,7 @@ static const struct uart_driver_api mcux_lpuart_driver_api = {
 			.source_data_size = 1,						       \
 			.dest_data_size = 1,						       \
 			.complete_callback_en = 1,					       \
-			.error_callback_en = 1,						       \
+			.error_callback_dis = 0,					       \
 			.block_count = 1,						       \
 			.head_block =							       \
 				&mcux_lpuart_##id##_data.async.rx_dma_params.active_dma_block, \
diff --git a/drivers/serial/uart_ns16550.c b/drivers/serial/uart_ns16550.c
@@ -1862,7 +1862,7 @@ static const struct uart_driver_api uart_ns16550_driver_api = {
 			.source_data_size = 1,					\
 			.dest_data_size = 1,					\
 			.complete_callback_en = 0,				\
-			.error_callback_en = 1,					\
+			.error_callback_dis = 1,				\
 			.block_count = 1,					\
 			.channel_direction = MEMORY_TO_PERIPHERAL,		\
 			.dma_slot = DT_INST_DMAS_CELL_BY_NAME(n, tx, channel),	\
@@ -1881,7 +1881,7 @@ static const struct uart_driver_api uart_ns16550_driver_api = {
 			.source_data_size = 1,					\
 			.dest_data_size = 1,					\
 			.complete_callback_en = 0,				\
-			.error_callback_en = 1,					\
+			.error_callback_dis = 1,				\
 			.block_count = 1,					\
 			.channel_direction = PERIPHERAL_TO_MEMORY,		\
 			.dma_slot = DT_INST_DMAS_CELL_BY_NAME(n, rx, channel),	\
diff --git a/drivers/spi/spi_andes_atcspi200.c b/drivers/spi/spi_andes_atcspi200.c
@@ -887,7 +887,7 @@ static void spi_atcspi200_irq_handler(void *arg)
 		.channel_direction = ANDES_DMA_CONFIG_DIRECTION(		\
 				     DMA_CHANNEL_CONFIG(index, dir)),		\
 		.complete_callback_en = 0,					\
-		.error_callback_en = 0,						\
+		.error_callback_dis = 0,					\
 		.source_data_size =						\
 			ANDES_DMA_CONFIG_##src_dev##_DATA_SIZE(			\
 					DMA_CHANNEL_CONFIG(index, dir)		\
diff --git a/drivers/spi/spi_mcux_dspi.c b/drivers/spi/spi_mcux_dspi.c
@@ -836,7 +836,7 @@ static const struct spi_driver_api spi_mcux_driver_api = {
 			.dest_data_size = 4,				\
 			.dma_callback = dma_callback,			\
 			.complete_callback_en = 1,			\
-			.error_callback_en = 1,				\
+			.error_callback_dis = 0,			\
 			.block_count = 1,				\
 			.head_block = &spi_mcux_data_##id.tx_dma_block,	\
 			.channel_direction = MEMORY_TO_PERIPHERAL,	\
@@ -857,7 +857,7 @@ static const struct spi_driver_api spi_mcux_driver_api = {
 			.dest_data_size = 2,				\
 			.dma_callback = dma_callback,			\
 			.complete_callback_en = 1,			\
-			.error_callback_en = 1,				\
+			.error_callback_dis = 0,			\
 			.block_count =					\
 			_UTIL_AND2(DT_INST_NODE_HAS_PROP(		\
 				id, nxp_rx_tx_chn_share), 2),		\
diff --git a/drivers/spi/spi_sam.c b/drivers/spi/spi_sam.c
@@ -340,7 +340,6 @@ static int spi_sam_dma_txrx(const struct device *dev,
 		.source_burst_length = 1,
 		.dest_burst_length = 1,
 		.complete_callback_en = true,
-		.error_callback_en = true,
 		.dma_callback = NULL,
 		.user_data = (void *)dev,
 	};
@@ -373,7 +372,6 @@ static int spi_sam_dma_txrx(const struct device *dev,
 		.source_burst_length = 1,
 		.dest_burst_length = 1,
 		.complete_callback_en = true,
-		.error_callback_en = true,
 		.dma_callback = dma_callback,
 		.user_data = (void *)dev,
 	};
diff --git a/include/zephyr/drivers/dma.h b/include/zephyr/drivers/dma.h
@@ -217,12 +217,12 @@ struct dma_config {
 	 */
 	uint32_t  complete_callback_en : 1;
 	/**
-	 * Error callback enable
+	 * Error callback disable
 	 *
 	 * - 0b0 error callback enabled
 	 * - 0b1 error callback disabled
 	 */
-	uint32_t  error_callback_en :    1;
+	uint32_t  error_callback_dis :    1;
 	/**
 	 * Source handshake, HW specific
 	 *
diff --git a/tests/drivers/dma/chan_blen_transfer/src/test_dma.c b/tests/drivers/dma/chan_blen_transfer/src/test_dma.c
@@ -52,7 +52,7 @@ static int test_task(const struct device *dma, uint32_t chan_id, uint32_t blen)
 	dma_cfg.dest_burst_length = blen;
 	dma_cfg.dma_callback = test_done;
 	dma_cfg.complete_callback_en = 0U;
-	dma_cfg.error_callback_en = 1U;
+	dma_cfg.error_callback_dis = 0U;
 	dma_cfg.block_count = 1U;
 	dma_cfg.head_block = &dma_block_cfg;
 #ifdef CONFIG_DMA_MCUX_TEST_SLOT_START
diff --git a/tests/drivers/dma/chan_link_transfer/src/test_dma.c b/tests/drivers/dma/chan_link_transfer/src/test_dma.c
@@ -71,7 +71,7 @@ static int test_task(int minor, int major)
 	dma_cfg.dest_burst_length = 16;
 	dma_cfg.dma_callback = test_done;
 	dma_cfg.complete_callback_en = 0U;
-	dma_cfg.error_callback_en = 1U;
+	dma_cfg.error_callback_dis = 0U;
 	dma_cfg.block_count = 1U;
 	dma_cfg.head_block = &dma_block_cfg;
 #ifdef CONFIG_DMA_MCUX_TEST_SLOT_START

Original file line number	Diff line number	Diff line change
`@@ -273,7 +273,7 @@ static int dma_atcdmac300_config(const struct device *dev, uint32_t channel,`
`273`	`273`	`ch_ctrl \|= DMA_CH_CTRL_INTABT;`
`274`	`274`
`275`	`275`	`/* Disable the error callback */`
`276`		`- if (!cfg->error_callback_en) {`
	`276`	`+ if (!cfg->error_callback_dis) {`
`277`	`277`	`ch_ctrl \|= DMA_CH_CTRL_INTERR;`
`278`	`278`	`}`
`279`	`279`
Original file line number	Diff line number	Diff line change
`@@ -57,7 +57,7 @@ static void dma_handler(sedi_dma_t dma_device, int channel, int event_id,`
`57`	`57`	`(config->complete_callback_en)) {`
`58`	`58`	`config->dma_callback(dev, config->user_data,`
`59`	`59`	`channel, 0);`
`60`		`- } else if (config->error_callback_en) {`
	`60`	`+ } else if (!config->error_callback_dis) {`
`61`	`61`	`config->dma_callback(dev, config->user_data,`
`62`	`62`	`channel, event_id);`
`63`	`63`	`}`
Original file line number	Diff line number	Diff line change
`@@ -483,7 +483,7 @@ static int dma_smartbond_config(const struct device *dev, uint32_t channel, stru`
`483`	`483`	`}`
`484`	`484`
`485`	`485`	`/* Error handling is not supported; just warn user. */`
`486`		`- if (cfg->error_callback_en) {`
	`486`	`+ if (!cfg->error_callback_dis) {`
`487`	`487`	`LOG_WRN("Error handling is not supported");`
`488`	`488`	`}`
`489`	`489`
Original file line number	Diff line number	Diff line change
`@@ -294,7 +294,7 @@ static int dma_xmc4xxx_config(const struct device *dev, uint32_t channel, struct`
`294`	`294`	`XMC_DMA_CH_EnableEvent(dma, channel, XMC_DMA_CH_EVENT_BLOCK_TRANSFER_COMPLETE);`
`295`	`295`	`}`
`296`	`296`
`297`		`- if (config->error_callback_en) {`
	`297`	`+ if (!config->error_callback_dis) {`
`298`	`298`	`XMC_DMA_CH_EnableEvent(dma, channel, XMC_DMA_CH_EVENT_ERROR);`
`299`	`299`	`}`
`300`	`300`