Re: [PATCH v3 4/5] dmaengine: mcf-edma: Fix error handler for all 64 DMA channels

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete Fix / Hardware State Corruption: fsl_edma_disable_request() truncates channel IDs >= 32 when disabling error interrupts.
--

commit 95f12f5cb94b6214f42fed40a0a6ec8cb8002d51
Author: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>

dmaengine: mcf-edma: Fix error handler for all 64 DMA channels

This commit updates the DMA error interrupt handler to properly process errors
on all 64 channels by iterating over a 64-bit mask with for_each_set_bit(). It
also ensures both low and high channel groups consistently use
fsl_edma_err_chan_handler() for error status reporting.

> diff --git a/drivers/dma/mcf-edma-main.c b/drivers/dma/mcf-edma-main.c
> index 953b20f99f251..3dab5d475d1b4 100644
> --- a/drivers/dma/mcf-edma-main.c
> +++ b/drivers/dma/mcf-edma-main.c
[ ... ]
> @@ -42,30 +42,22 @@ static irqreturn_t mcf_edma_err_handler(int irq, void *dev_id)
[ ... ]
> -	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;
> -		}
> +	for_each_set_bit(ch, err_mask, mcf_edma->n_chans) {
> +		fsl_edma_disable_request(&mcf_edma->chans[ch]);

[Severity: High]
This is a pre-existing issue, but does fsl_edma_disable_request() properly
handle channel IDs >= 32?

Looking at fsl_edma_disable_request() in drivers/dma/fsl-edma-common.c, it
uses the EDMA_CEEI_CEEI() macro to write to the ceei register:

    iowrite8(ch, regs->cerq);
    iowrite8(EDMA_CEEI_CEEI(ch), regs->ceei);

However, EDMA_CEEI_CEEI(x) masks the channel ID to 5 bits using GENMASK(4, 0).

When fsl_edma_disable_request() is called for high channels (32-63), the
channel ID will be truncated. For instance, channel 33 would be truncated to 1.

Could this truncation fail to disable the error interrupt for the intended
high channel, leading to a continuous interrupt loop, while erroneously
disabling the interrupt for an unrelated active low channel?

> +		iowrite8(EDMA_MASK_CH(ch), regs->cerr);
> +		fsl_edma_err_chan_handler(&mcf_edma->chans[ch]);
>  	}
>  
>  	return IRQ_HANDLED;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625-b4-edma-dmaengine-v3-0-44be00ace37d@yoseli.org?part=4