Re: [PATCH 6/7] dma: mcf-edma: Fix error handler for all 64 DMA channels

Frank Li
2025-11-24T16:16:27Z

On Mon, Nov 24, 2025 at 01:50:27PM +0100, Jean-Michel Hautbois wrote:
> 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.
>
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>

It is bug fix for high 32 channel. Need fix tags.

> ---
> 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 dd64f50f2b0a70a4664b03c7d6a23e74c9bcd7ae..adae2914c23db3ce9244c0cb8d4208fd71874f76 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)
^ align to previous line

Frank

> #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
>