[PATCH v2 5/5] dma: mcf-edma: Fix error handler for all 64 DMA channels

Jean-Michel Hautbois via B4 Relay posted 5 patches 5 days, 15 hours ago
[PATCH v2 5/5] dma: mcf-edma: Fix error handler for all 64 DMA channels
Posted by Jean-Michel Hautbois via B4 Relay 5 days, 15 hours ago
From: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>

Fix the DMA error interrupt handler to properly handle errors on all
64 channels. The previous implementation had several issues:

1. Returned IRQ_NONE if low channels had no errors, even if high
   channels did
2. Used direct status assignment instead of fsl_edma_err_chan_handler()
   for high channels

Split the error handling into two separate loops for the low (0-31)
and high (32-63) channel groups, using for_each_set_bit() for cleaner
iteration. Both groups now consistently use fsl_edma_err_chan_handler()
for proper error status reporting.

Fixes: e7a3ff92eaf1 ("dmaengine: fsl-edma: add ColdFire mcf5441x edma support")
Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
---
 drivers/dma/mcf-edma-main.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/dma/mcf-edma-main.c b/drivers/dma/mcf-edma-main.c
index 6353303df957..5bd04faa639c 100644
--- a/drivers/dma/mcf-edma-main.c
+++ b/drivers/dma/mcf-edma-main.c
@@ -12,6 +12,7 @@
 #include "fsl-edma-common.h"
 
 #define EDMA_CHANNELS		64
+#define EDMA_CHANS_PER_REG	(EDMA_CHANNELS / 2)
 #define EDMA_MASK_CH(x)		((x) & GENMASK(5, 0))
 
 static irqreturn_t mcf_edma_tx_handler(int irq, void *dev_id)
@@ -42,33 +43,33 @@ static irqreturn_t mcf_edma_err_handler(int irq, void *dev_id)
 {
 	struct fsl_edma_engine *mcf_edma = dev_id;
 	struct edma_regs *regs = &mcf_edma->regs;
-	unsigned int err, ch;
+	unsigned int ch;
+	unsigned long err;
+	bool handled = false;
 
+	/* Check low 32 channels (0-31) */
 	err = ioread32(regs->errl);
-	if (!err)
-		return IRQ_NONE;
-
-	for (ch = 0; ch < (EDMA_CHANNELS / 2); ch++) {
-		if (err & BIT(ch)) {
+	if (err) {
+		handled = true;
+		for_each_set_bit(ch, &err, EDMA_CHANS_PER_REG) {
 			fsl_edma_disable_request(&mcf_edma->chans[ch]);
 			iowrite8(EDMA_CERR_CERR(ch), regs->cerr);
 			fsl_edma_err_chan_handler(&mcf_edma->chans[ch]);
 		}
 	}
 
+	/* Check high 32 channels (32-63) */
 	err = ioread32(regs->errh);
-	if (!err)
-		return IRQ_NONE;
-
-	for (ch = (EDMA_CHANNELS / 2); ch < EDMA_CHANNELS; ch++) {
-		if (err & (BIT(ch - (EDMA_CHANNELS / 2)))) {
-			fsl_edma_disable_request(&mcf_edma->chans[ch]);
-			iowrite8(EDMA_CERR_CERR(ch), regs->cerr);
-			mcf_edma->chans[ch].status = DMA_ERROR;
+	if (err) {
+		handled = true;
+		for_each_set_bit(ch, &err, EDMA_CHANS_PER_REG) {
+			fsl_edma_disable_request(&mcf_edma->chans[ch + EDMA_CHANS_PER_REG]);
+			iowrite8(EDMA_CERR_CERR(ch + EDMA_CHANS_PER_REG), regs->cerr);
+			fsl_edma_err_chan_handler(&mcf_edma->chans[ch + EDMA_CHANS_PER_REG]);
 		}
 	}
 
-	return IRQ_HANDLED;
+	return handled ? IRQ_HANDLED : IRQ_NONE;
 }
 
 static int mcf_edma_irq_init(struct platform_device *pdev,

-- 
2.39.5
Re: [PATCH v2 5/5] dma: mcf-edma: Fix error handler for all 64 DMA channels
Posted by Frank Li 5 days, 7 hours ago
On Wed, Nov 26, 2025 at 09:36:06AM +0100, Jean-Michel Hautbois via B4 Relay wrote:
> From: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
>
> Fix the DMA error interrupt handler to properly handle errors on all
> 64 channels. The previous implementation had several issues:
>
> 1. Returned IRQ_NONE if low channels had no errors, even if high
>    channels did
> 2. Used direct status assignment instead of fsl_edma_err_chan_handler()
>    for high channels
>
> Split the error handling into two separate loops for the low (0-31)
> and high (32-63) channel groups, using for_each_set_bit() for cleaner
> iteration. Both groups now consistently use fsl_edma_err_chan_handler()
> for proper error status reporting.
>
> Fixes: e7a3ff92eaf1 ("dmaengine: fsl-edma: add ColdFire mcf5441x edma support")
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/dma/mcf-edma-main.c | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/dma/mcf-edma-main.c b/drivers/dma/mcf-edma-main.c
> index 6353303df957..5bd04faa639c 100644
> --- a/drivers/dma/mcf-edma-main.c
> +++ b/drivers/dma/mcf-edma-main.c
> @@ -12,6 +12,7 @@
>  #include "fsl-edma-common.h"
>
>  #define EDMA_CHANNELS		64
> +#define EDMA_CHANS_PER_REG	(EDMA_CHANNELS / 2)
>  #define EDMA_MASK_CH(x)		((x) & GENMASK(5, 0))
>
>  static irqreturn_t mcf_edma_tx_handler(int irq, void *dev_id)
> @@ -42,33 +43,33 @@ static irqreturn_t mcf_edma_err_handler(int irq, void *dev_id)
>  {
>  	struct fsl_edma_engine *mcf_edma = dev_id;
>  	struct edma_regs *regs = &mcf_edma->regs;
> -	unsigned int err, ch;
> +	unsigned int ch;
> +	unsigned long err;
> +	bool handled = false;
>
> +	/* Check low 32 channels (0-31) */
>  	err = ioread32(regs->errl);
> -	if (!err)
> -		return IRQ_NONE;
> -
> -	for (ch = 0; ch < (EDMA_CHANNELS / 2); ch++) {
> -		if (err & BIT(ch)) {
> +	if (err) {
> +		handled = true;
> +		for_each_set_bit(ch, &err, EDMA_CHANS_PER_REG) {
>  			fsl_edma_disable_request(&mcf_edma->chans[ch]);
>  			iowrite8(EDMA_CERR_CERR(ch), regs->cerr);
>  			fsl_edma_err_chan_handler(&mcf_edma->chans[ch]);
>  		}
>  	}
>
> +	/* Check high 32 channels (32-63) */
>  	err = ioread32(regs->errh);
> -	if (!err)
> -		return IRQ_NONE;
> -
> -	for (ch = (EDMA_CHANNELS / 2); ch < EDMA_CHANNELS; ch++) {
> -		if (err & (BIT(ch - (EDMA_CHANNELS / 2)))) {
> -			fsl_edma_disable_request(&mcf_edma->chans[ch]);
> -			iowrite8(EDMA_CERR_CERR(ch), regs->cerr);
> -			mcf_edma->chans[ch].status = DMA_ERROR;
> +	if (err) {
> +		handled = true;
> +		for_each_set_bit(ch, &err, EDMA_CHANS_PER_REG) {
> +			fsl_edma_disable_request(&mcf_edma->chans[ch + EDMA_CHANS_PER_REG]);
> +			iowrite8(EDMA_CERR_CERR(ch + EDMA_CHANS_PER_REG), regs->cerr);
> +			fsl_edma_err_chan_handler(&mcf_edma->chans[ch + EDMA_CHANS_PER_REG]);
>  		}
>  	}
>
> -	return IRQ_HANDLED;
> +	return handled ? IRQ_HANDLED : IRQ_NONE;
>  }
>
>  static int mcf_edma_irq_init(struct platform_device *pdev,
>
> --
> 2.39.5
>
>