[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aSXV4A60wx4M7ymj@lizhi-Precision-Tower-5810>
Date: Tue, 25 Nov 2025 11:14:24 -0500
From: Frank Li <Frank.li@....com>
To: Jean-Michel Hautbois <jeanmichel.hautbois@...eli.org>
Cc: Vinod Koul <vkoul@...nel.org>, Greg Ungerer <gerg@...ux-m68k.org>,
imx@...ts.linux.dev, dmaengine@...r.kernel.org,
linux-m68k@...ts.linux-m68k.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/7] dma: mcf-edma: Fix interrupt handler for 64 DMA
channels
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@...eli.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@....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
> > >
Powered by blists - more mailing lists