[PATCH] dmaengine: loongson: loongson2-apb: fix broken bus width validation in ls2x_dmac_detect

public inbox for dmaengine@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH] dmaengine: loongson: loongson2-apb: fix broken bus width validation in ls2x_dmac_detect_burst()
@ 2026-03-18 16:48 David Carlier
  2026-03-23  2:38 ` Binbin Zhou
  2026-03-30 16:28 ` Frank Li
  0 siblings, 2 replies; 6+ messages in thread
From: David Carlier @ 2026-03-18 16:48 UTC (permalink / raw)
  To: Binbin Zhou, Vinod Koul, Frank Li, Yingkun Meng; +Cc: dmaengine, David Carlier

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))
 		return 0;
 
 	if (lchan->sconfig.direction == DMA_MEM_TO_DEV) {
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] dmaengine: loongson: loongson2-apb: fix broken bus width validation in ls2x_dmac_detect_burst()
  2026-03-18 16:48 [PATCH] dmaengine: loongson: loongson2-apb: fix broken bus width validation in ls2x_dmac_detect_burst() David Carlier
@ 2026-03-23  2:38 ` Binbin Zhou
  2026-03-30 16:28 ` Frank Li
  1 sibling, 0 replies; 6+ messages in thread
From: Binbin Zhou @ 2026-03-23  2:38 UTC (permalink / raw)
  To: David Carlier, Vinod Koul, Frank Li; +Cc: dmaengine

Hi David:

On 2026/3/19 00:48, 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>

That was indeed my oversight. Thanks a lot!

Reviewed-by: Binbin Zhou <zhoubinbin@loongson.cn>

> ---
>   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))
>   		return 0;
>   
>   	if (lchan->sconfig.direction == DMA_MEM_TO_DEV) {
Thanks.
Binbin


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] dmaengine: loongson: loongson2-apb: fix broken bus width validation in ls2x_dmac_detect_burst()
  2026-03-18 16:48 [PATCH] dmaengine: loongson: loongson2-apb: fix broken bus width validation in ls2x_dmac_detect_burst() David Carlier
  2026-03-23  2:38 ` Binbin Zhou
@ 2026-03-30 16:28 ` Frank Li
  2026-03-30 16:57   ` David CARLIER
  2026-03-31 15:01   ` [PATCH v2] " David Carlier
  1 sibling, 2 replies; 6+ messages in thread
From: Frank Li @ 2026-03-30 16:28 UTC (permalink / raw)
  To: David Carlier; +Cc: Binbin Zhou, Vinod Koul, Frank Li, Yingkun Meng, dmaengine

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.

Frank

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] dmaengine: loongson: loongson2-apb: fix broken bus width validation in ls2x_dmac_detect_burst()
  2026-03-30 16:28 ` Frank Li
@ 2026-03-30 16:57   ` David CARLIER
  2026-03-31 14:23     ` Frank Li
  2026-03-31 15:01   ` [PATCH v2] " David Carlier
  1 sibling, 1 reply; 6+ messages in thread
From: David CARLIER @ 2026-03-30 16:57 UTC (permalink / raw)
  To: Frank Li; +Cc: Binbin Zhou, Vinod Koul, Frank Li, Yingkun Meng, dmaengine

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?

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] dmaengine: loongson: loongson2-apb: fix broken bus width validation in ls2x_dmac_detect_burst()
  2026-03-30 16:57   ` David CARLIER
@ 2026-03-31 14:23     ` Frank Li
  0 siblings, 0 replies; 6+ messages in thread
From: Frank Li @ 2026-03-31 14:23 UTC (permalink / raw)
  To: David CARLIER; +Cc: Binbin Zhou, Vinod Koul, Frank Li, Yingkun Meng, dmaengine

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] dmaengine: loongson: loongson2-apb: fix broken bus width validation in ls2x_dmac_detect_burst()
  2026-03-30 16:28 ` Frank Li
  2026-03-30 16:57   ` David CARLIER
@ 2026-03-31 15:01   ` David Carlier
  1 sibling, 0 replies; 6+ messages in thread
From: David Carlier @ 2026-03-31 15:01 UTC (permalink / raw)
  To: Frank Li, Binbin Zhou, Vinod Koul, Yingkun Meng; +Cc: dmaengine, David Carlier

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 comparing directly against the supported enum values, avoiding
BIT() which would overflow for large dma_slave_buswidth values like
DMA_SLAVE_BUSWIDTH_128_BYTES. Invert 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 | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/loongson/loongson2-apb-dma.c b/drivers/dma/loongson/loongson2-apb-dma.c
index aceb069e71fc..d219f40cb0ae 100644
--- a/drivers/dma/loongson/loongson2-apb-dma.c
+++ b/drivers/dma/loongson/loongson2-apb-dma.c
@@ -220,8 +220,10 @@ 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 (lchan->sconfig.src_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES &&
+	    lchan->sconfig.src_addr_width != DMA_SLAVE_BUSWIDTH_8_BYTES &&
+	    lchan->sconfig.dst_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES &&
+	    lchan->sconfig.dst_addr_width != DMA_SLAVE_BUSWIDTH_8_BYTES)
 		return 0;
 
 	if (lchan->sconfig.direction == DMA_MEM_TO_DEV) {
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-03-31 15:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-18 16:48 [PATCH] dmaengine: loongson: loongson2-apb: fix broken bus width validation in ls2x_dmac_detect_burst() David Carlier
2026-03-23  2:38 ` Binbin Zhou
2026-03-30 16:28 ` Frank Li
2026-03-30 16:57   ` David CARLIER
2026-03-31 14:23     ` Frank Li
2026-03-31 15:01   ` [PATCH v2] " David Carlier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox