Re: [PATCH] dmaengine: loongson: loongson2-apb: fix broken bus width validation in ls2x_dmac_detect_burst()

[Frank Li <Frank.li@nxp.com>]

On Mon, Mar 30, 2026 at 05:57:44PM +0100, David CARLIER wrote:
> Hi,
>
> On Mon, 30 Mar 2026 at 17:28, Frank Li <Frank.li@nxp.com> wrote:
> >
> > On Wed, Mar 18, 2026 at 04:48:03PM +0000, David Carlier wrote:
> > > The bus width validation check in ls2x_dmac_detect_burst() compares raw
> > > enum dma_slave_buswidth values (e.g. 4, 8) directly against
> > > LDMA_SLAVE_BUSWIDTHS, which is a BIT()-encoded bitmask
> > > (BIT(4) | BIT(8) = 0x110). Since 4 & 0x110 == 0 and 8 & 0x110 == 0,
> > > the condition is always false for valid bus widths, making the
> > > validation dead code.
> > >
> > > Additionally, the logic was inverted: it rejected configurations where
> > > both widths matched valid values, rather than rejecting when neither
> > > width is supported.
> > >
> > > Fix by wrapping the enum values with BIT() before masking (matching the
> > > pattern used in sun6i-dma.c) and inverting the logic to reject when
> > > neither width is supported by the hardware.
> > >
> > > Fixes: 71e7d3cb6e55 ("dmaengine: ls2x-apb: New driver for the Loongson LS2X APB DMA controller")
> > > Signed-off-by: David Carlier <devnexen@gmail.com>
> > > ---
> > >  drivers/dma/loongson/loongson2-apb-dma.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/dma/loongson/loongson2-apb-dma.c b/drivers/dma/loongson/loongson2-apb-dma.c
> > > index aceb069e71fc..102c01f993ef 100644
> > > --- a/drivers/dma/loongson/loongson2-apb-dma.c
> > > +++ b/drivers/dma/loongson/loongson2-apb-dma.c
> > > @@ -220,8 +220,8 @@ static size_t ls2x_dmac_detect_burst(struct ls2x_dma_chan *lchan)
> > >       u32 maxburst, buswidth;
> > >
> > >       /* Reject definitely invalid configurations */
> > > -     if ((lchan->sconfig.src_addr_width & LDMA_SLAVE_BUSWIDTHS) &&
> > > -         (lchan->sconfig.dst_addr_width & LDMA_SLAVE_BUSWIDTHS))
> > > +     if (!(BIT(lchan->sconfig.src_addr_width) & LDMA_SLAVE_BUSWIDTHS) &&
> > > +         !(BIT(lchan->sconfig.dst_addr_width) & LDMA_SLAVE_BUSWIDTHS))
> >
> > src_addr_width is enum dma_slave_buswidth, which allow
> > DMA_SLAVE_BUSWIDTH_128_BYTES = 128,
> >
> > BIT(128) will overflow.
>
> Thanks for the review Frank. You're right that BIT() would overflow
> for DMA_SLAVE_BUSWIDTH_128_BYTES. While this driver only supports
> 4-byte and 8-byte widths
>   today, relying on BIT() for buswidth validation is fragile. I'll
> send a v2 that avoids BIT() altogether — would a direct comparison
> against the supported widths work
>   for you, or do you have a preferred pattern in mind?

I think LDMA_SLAVE_BUSWIDTHS use enum dma_slave_buswidth OR together.

Frank

>
> >
> > Frank
> >
> > >               return 0;
> > >
> > >       if (lchan->sconfig.direction == DMA_MEM_TO_DEV) {
> > > --
> > > 2.53.0
> > >