[v1] dma: fsl/mcf-edma: Bug fixes and enhancements for ColdFire support

[PATCH 4/7] dma: mcf-edma: Fix interrupt handler for 64 DMA channels

Posted by Jean-Michel Hautbois 1 week ago

Fix the DMA completion interrupt handler to properly handle all 64
channels on MCF54418 ColdFire processors.

The previous code used BIT(ch) to test interrupt status bits, which
causes undefined behavior on 32-bit architectures when ch >= 32 because
unsigned long is 32 bits and the shift would exceed the type width.

Replace with bitmap_from_u64() and for_each_set_bit() which correctly
handle 64-bit values on 32-bit systems by using a proper bitmap
representation.

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
---
 drivers/dma/mcf-edma-main.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/mcf-edma-main.c b/drivers/dma/mcf-edma-main.c
index 8a7c1787adb1f66f3b6729903635b072218afad1..dd64f50f2b0a70a4664b03c7d6a23e74c9bcd7ae 100644
--- a/drivers/dma/mcf-edma-main.c
+++ b/drivers/dma/mcf-edma-main.c
@@ -18,7 +18,8 @@ static irqreturn_t mcf_edma_tx_handler(int irq, void *dev_id)
 {
 	struct fsl_edma_engine *mcf_edma = dev_id;
 	struct edma_regs *regs = &mcf_edma->regs;
-	unsigned int ch;
+	unsigned long ch;
+	DECLARE_BITMAP(status_mask, 64);
 	u64 intmap;
 
 	intmap = ioread32(regs->inth);
@@ -27,11 +28,11 @@ static irqreturn_t mcf_edma_tx_handler(int irq, void *dev_id)
 	if (!intmap)
 		return IRQ_NONE;
 
-	for (ch = 0; ch < mcf_edma->n_chans; ch++) {
-		if (intmap & BIT(ch)) {
-			iowrite8(EDMA_MASK_CH(ch), regs->cint);
-			fsl_edma_tx_chan_handler(&mcf_edma->chans[ch]);
-		}
+	bitmap_from_u64(status_mask, intmap);
+
+	for_each_set_bit(ch, status_mask, mcf_edma->n_chans) {
+		iowrite8(EDMA_MASK_CH(ch), regs->cint);
+		fsl_edma_tx_chan_handler(&mcf_edma->chans[ch]);
 	}
 
 	return IRQ_HANDLED;

-- 
2.39.5

Re: [PATCH 4/7] dma: mcf-edma: Fix interrupt handler for 64 DMA channels

Posted by Frank Li 1 week ago

On Mon, Nov 24, 2025 at 01:50:25PM +0100, Jean-Michel Hautbois wrote:
> Fix the DMA completion interrupt handler to properly handle all 64
> channels on MCF54418 ColdFire processors.
>
> The previous code used BIT(ch) to test interrupt status bits, which
> causes undefined behavior on 32-bit architectures when ch >= 32 because
> unsigned long is 32 bits and the shift would exceed the type width.
>
> Replace with bitmap_from_u64() and for_each_set_bit() which correctly
> handle 64-bit values on 32-bit systems by using a proper bitmap
> representation.
>
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> ---
>  drivers/dma/mcf-edma-main.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/dma/mcf-edma-main.c b/drivers/dma/mcf-edma-main.c
> index 8a7c1787adb1f66f3b6729903635b072218afad1..dd64f50f2b0a70a4664b03c7d6a23e74c9bcd7ae 100644
> --- a/drivers/dma/mcf-edma-main.c
> +++ b/drivers/dma/mcf-edma-main.c
> @@ -18,7 +18,8 @@ static irqreturn_t mcf_edma_tx_handler(int irq, void *dev_id)
>  {
>  	struct fsl_edma_engine *mcf_edma = dev_id;
>  	struct edma_regs *regs = &mcf_edma->regs;
> -	unsigned int ch;
> +	unsigned long ch;

channel number max value is 63. unsigned int should be enough.

Frank
> +	DECLARE_BITMAP(status_mask, 64);
>  	u64 intmap;
>
>  	intmap = ioread32(regs->inth);
> @@ -27,11 +28,11 @@ static irqreturn_t mcf_edma_tx_handler(int irq, void *dev_id)
>  	if (!intmap)
>  		return IRQ_NONE;
>
> -	for (ch = 0; ch < mcf_edma->n_chans; ch++) {
> -		if (intmap & BIT(ch)) {
> -			iowrite8(EDMA_MASK_CH(ch), regs->cint);
> -			fsl_edma_tx_chan_handler(&mcf_edma->chans[ch]);
> -		}
> +	bitmap_from_u64(status_mask, intmap);
> +
> +	for_each_set_bit(ch, status_mask, mcf_edma->n_chans) {
> +		iowrite8(EDMA_MASK_CH(ch), regs->cint);
> +		fsl_edma_tx_chan_handler(&mcf_edma->chans[ch]);
>  	}
>
>  	return IRQ_HANDLED;
>
> --
> 2.39.5
>

Re: [PATCH 4/7] dma: mcf-edma: Fix interrupt handler for 64 DMA channels

Posted by Jean-Michel Hautbois 6 days, 16 hours ago

Hi Frank,

On Mon, Nov 24, 2025 at 11:09:06AM -0500, Frank Li wrote:
> On Mon, Nov 24, 2025 at 01:50:25PM +0100, Jean-Michel Hautbois wrote:
> > Fix the DMA completion interrupt handler to properly handle all 64
> > channels on MCF54418 ColdFire processors.
> >
> > The previous code used BIT(ch) to test interrupt status bits, which
> > causes undefined behavior on 32-bit architectures when ch >= 32 because
> > unsigned long is 32 bits and the shift would exceed the type width.
> >
> > Replace with bitmap_from_u64() and for_each_set_bit() which correctly
> > handle 64-bit values on 32-bit systems by using a proper bitmap
> > representation.
> >
> > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> > ---
> >  drivers/dma/mcf-edma-main.c | 13 +++++++------
> >  1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/dma/mcf-edma-main.c b/drivers/dma/mcf-edma-main.c
> > index 8a7c1787adb1f66f3b6729903635b072218afad1..dd64f50f2b0a70a4664b03c7d6a23e74c9bcd7ae 100644
> > --- a/drivers/dma/mcf-edma-main.c
> > +++ b/drivers/dma/mcf-edma-main.c
> > @@ -18,7 +18,8 @@ static irqreturn_t mcf_edma_tx_handler(int irq, void *dev_id)
> >  {
> >  	struct fsl_edma_engine *mcf_edma = dev_id;
> >  	struct edma_regs *regs = &mcf_edma->regs;
> > -	unsigned int ch;
> > +	unsigned long ch;
> 
> channel number max value is 63. unsigned int should be enough.

Yes, indeed, it is enough. I changed to unsigned long because
for_each_set_bit() calls find_next_bit which returns unsigned long. This
only avoiding an implicit conversion. But I can remove this change if it
does not make sense ?

Thanks,
JM

> 
> Frank
> > +	DECLARE_BITMAP(status_mask, 64);
> >  	u64 intmap;
> >
> >  	intmap = ioread32(regs->inth);
> > @@ -27,11 +28,11 @@ static irqreturn_t mcf_edma_tx_handler(int irq, void *dev_id)
> >  	if (!intmap)
> >  		return IRQ_NONE;
> >
> > -	for (ch = 0; ch < mcf_edma->n_chans; ch++) {
> > -		if (intmap & BIT(ch)) {
> > -			iowrite8(EDMA_MASK_CH(ch), regs->cint);
> > -			fsl_edma_tx_chan_handler(&mcf_edma->chans[ch]);
> > -		}
> > +	bitmap_from_u64(status_mask, intmap);
> > +
> > +	for_each_set_bit(ch, status_mask, mcf_edma->n_chans) {
> > +		iowrite8(EDMA_MASK_CH(ch), regs->cint);
> > +		fsl_edma_tx_chan_handler(&mcf_edma->chans[ch]);
> >  	}
> >
> >  	return IRQ_HANDLED;
> >
> > --
> > 2.39.5
> >

Re: [PATCH 4/7] dma: mcf-edma: Fix interrupt handler for 64 DMA channels

Posted by Frank Li 6 days, 8 hours ago

On Tue, Nov 25, 2025 at 09:02:36AM +0100, Jean-Michel Hautbois wrote:
> Hi Frank,
>
> On Mon, Nov 24, 2025 at 11:09:06AM -0500, Frank Li wrote:
> > On Mon, Nov 24, 2025 at 01:50:25PM +0100, Jean-Michel Hautbois wrote:
> > > Fix the DMA completion interrupt handler to properly handle all 64
> > > channels on MCF54418 ColdFire processors.
> > >
> > > The previous code used BIT(ch) to test interrupt status bits, which
> > > causes undefined behavior on 32-bit architectures when ch >= 32 because
> > > unsigned long is 32 bits and the shift would exceed the type width.
> > >
> > > Replace with bitmap_from_u64() and for_each_set_bit() which correctly
> > > handle 64-bit values on 32-bit systems by using a proper bitmap
> > > representation.
> > >
> > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@yoseli.org>
> > > ---
> > >  drivers/dma/mcf-edma-main.c | 13 +++++++------
> > >  1 file changed, 7 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/dma/mcf-edma-main.c b/drivers/dma/mcf-edma-main.c
> > > index 8a7c1787adb1f66f3b6729903635b072218afad1..dd64f50f2b0a70a4664b03c7d6a23e74c9bcd7ae 100644
> > > --- a/drivers/dma/mcf-edma-main.c
> > > +++ b/drivers/dma/mcf-edma-main.c
> > > @@ -18,7 +18,8 @@ static irqreturn_t mcf_edma_tx_handler(int irq, void *dev_id)
> > >  {
> > >  	struct fsl_edma_engine *mcf_edma = dev_id;
> > >  	struct edma_regs *regs = &mcf_edma->regs;
> > > -	unsigned int ch;
> > > +	unsigned long ch;
> >
> > channel number max value is 63. unsigned int should be enough.
>
> Yes, indeed, it is enough. I changed to unsigned long because
> for_each_set_bit() calls find_next_bit which returns unsigned long. This
> only avoiding an implicit conversion. But I can remove this change if it
> does not make sense ?

It is not big deal. That's fine.

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

>
> Thanks,
> JM
>
> >
> > Frank
> > > +	DECLARE_BITMAP(status_mask, 64);
> > >  	u64 intmap;
> > >
> > >  	intmap = ioread32(regs->inth);
> > > @@ -27,11 +28,11 @@ static irqreturn_t mcf_edma_tx_handler(int irq, void *dev_id)
> > >  	if (!intmap)
> > >  		return IRQ_NONE;
> > >
> > > -	for (ch = 0; ch < mcf_edma->n_chans; ch++) {
> > > -		if (intmap & BIT(ch)) {
> > > -			iowrite8(EDMA_MASK_CH(ch), regs->cint);
> > > -			fsl_edma_tx_chan_handler(&mcf_edma->chans[ch]);
> > > -		}
> > > +	bitmap_from_u64(status_mask, intmap);
> > > +
> > > +	for_each_set_bit(ch, status_mask, mcf_edma->n_chans) {
> > > +		iowrite8(EDMA_MASK_CH(ch), regs->cint);
> > > +		fsl_edma_tx_chan_handler(&mcf_edma->chans[ch]);
> > >  	}
> > >
> > >  	return IRQ_HANDLED;
> > >
> > > --
> > > 2.39.5
> > >

[PATCH 1/7] dma: fsl-edma: Add write barrier after TCD descriptor fill
[PATCH 2/7] dma: fsl-edma: Add FSL_EDMA_DRV_MCF flag for ColdFire eDMA
[PATCH 3/7] dma: mcf-edma: Add per-channel IRQ naming for debugging
[PATCH 4/7] dma: mcf-edma: Fix interrupt handler for 64 DMA channels
[PATCH 5/7] dma: fsl-edma: Move error handler out of header file
[PATCH 6/7] dma: mcf-edma: Fix error handler for all 64 DMA channels
[PATCH 7/7] dma: fsl-edma: Support source stride for interleaved DMA transfers