DMA Engine development
 help / color / mirror / Atom feed
* [PATCH] dmaengine: dw-axi-dmac: demote apb_regs warning to debug
@ 2026-09-10  8:09 Baineng Shou
  2026-09-10  8:22 ` sashiko-bot
  0 siblings, 1 reply; 4+ messages in thread
From: Baineng Shou @ 2026-09-10  8:09 UTC (permalink / raw)
  To: Eugeniy.Paltsev, vkoul; +Cc: Frank.Li, dmaengine, linux-kernel, Baineng Shou

dw_axi_dma_set_hw_channel() logs "apb_regs not initialized" at err
level when apb_regs is absent. However, only Intel KeemBay
("intel,kmb-axi-dma") carries apb_regs; all other supported SoCs
(snps,axi-dma-1.01a, sophgo,cv1800b, starfive,jh7110/jh8100) operate
without it. For those, reaching this path during slave/cyclic prep and
terminate is the normal no-op case, not an error, and the err log spams
dmesg on every transfer.

Demote it to dev_dbg to match the identical check in
dw_axi_dma_set_byte_halfword().

Signed-off-by: Baineng Shou <shoubaineng@gmail.com>
---
 drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
index bcefaff03b5c..d829b9af5491 100644
--- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
+++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
@@ -574,7 +574,7 @@ static void dw_axi_dma_set_hw_channel(struct axi_dma_chan *chan, bool set)
 	unsigned long reg_value, val;
 
 	if (!chip->apb_regs) {
-		dev_err(chip->dev, "apb_regs not initialized\n");
+		dev_dbg(chip->dev, "apb_regs not initialized\n");
 		return;
 	}
 
-- 
2.34.1


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

* Re: [PATCH] dmaengine: dw-axi-dmac: demote apb_regs warning to debug
  2026-09-10  8:09 Baineng Shou
@ 2026-09-10  8:22 ` sashiko-bot
  0 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-09-10  8:22 UTC (permalink / raw)
  To: Baineng Shou; +Cc: vkoul, Frank.Li, dmaengine

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

Pre-existing issues:
- [High] In `dw_axi_dma_set_hw_channel()`, bitwise operations on a 32-bit signed integer and improper use of `unsigned long` cause severe data loss and Undefined Behavior, permanently corrupting the configuration for channels 4-7 in the 64-bit hardware register.
- [High] In several descriptor preparation functions, the error cleanup path loops over all hardware descriptors to free them using dma_pool_free(), regardless of whether they were successfully allocated. This causes a NULL pointer dereference crash and corrupts atomic tracking counters.
--

commit f6abedf368282d2b40d4a72081898ecaa3428d91
Author: Baineng Shou <shoubaineng@gmail.com>

dmaengine: dw-axi-dmac: demote apb_regs warning to debug

This commit changes the log level of the "apb_regs not initialized" message
from error to debug in dw_axi_dma_set_hw_channel(). This prevents log spam
on supported SoCs that do not carry apb_regs, where reaching this path is
a normal no-op during slave or cyclic transfers.

> diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> index eebed2474210d..dc04b73c5987b 100644
> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> @@ -574,7 +574,7 @@ static void dw_axi_dma_set_hw_channel(struct axi_dma_chan *chan, bool set)
>  	unsigned long reg_value, val;

[Severity: High]
This is a pre-existing issue, but does using unsigned long here silently
truncate the 64-bit value returned by lo_hi_readq() on 32-bit platforms?
Assigning the 64-bit result to a 32-bit unsigned long would cause immediate
data loss for the upper 32 bits.

Also, further down in dw_axi_dma_set_hw_channel(), is there a risk of
undefined behavior and unintended zero-extension?

	reg_value &= ~(DMA_APB_HS_SEL_MASK <<
			(chan->id * DMA_APB_HS_SEL_BIT_SIZE));

If DMA_APB_HS_SEL_MASK is a 32-bit signed integer, shifting it by 24 (when
chan->id >= 3) produces a sign-extended 0x00FFFFFF. When promoted to 64-bit,
this zero-extends to 0x0000000000FFFFFFUL, which unintentionally wipes out
the configuration for channels 4-7. For chan->id >= 4, it shifts by 32 or
more, invoking undefined behavior.

>  
>  	if (!chip->apb_regs) {
> -		dev_err(chip->dev, "apb_regs not initialized\n");
> +		dev_dbg(chip->dev, "apb_regs not initialized\n");
>  		return;
>  	}

[Severity: High]
This is another pre-existing issue, but while reviewing the error handling
paths for descriptor allocation, I noticed a potential null pointer
dereference in axi_desc_put().

When functions like dw_axi_dma_chan_prep_cyclic() fail to allocate or set up
hardware descriptors, they jump to their error paths and call axi_desc_put().
The array of hardware descriptors is zero-initialized, and axi_desc_put()
unconditionally loops over all elements to free them:

	dma_pool_free(chan->desc_pool, hw_desc->lli, hw_desc->llp);

If unallocated descriptors are passed in, could hw_desc->lli be null?
Passing a null pointer to dma_pool_free() dereferences it immediately,
crashing the kernel.

Additionally, in the same function:

	atomic_sub(descs_put, &chan->descs_allocated);

Could this corrupt the atomic tracking counter? It appears to subtract the
full array length (descs_put) regardless of how many descriptors were
actually allocated, which would underflow the counter.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910080956.2342267-1-shoubaineng@gmail.com?part=1

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

* [PATCH] dmaengine: dw-axi-dmac: demote apb_regs warning to debug
@ 2026-09-18 10:14 Baineng Shou
  2026-09-18 14:04 ` Frank Li
  0 siblings, 1 reply; 4+ messages in thread
From: Baineng Shou @ 2026-09-18 10:14 UTC (permalink / raw)
  To: Eugeniy.Paltsev, vkoul; +Cc: Frank.Li, dmaengine, linux-kernel, Baineng Shou

dw_axi_dma_set_hw_channel() logs "apb_regs not initialized" at err
level when apb_regs is absent. However, only Intel KeemBay
("intel,kmb-axi-dma") carries apb_regs; all other supported SoCs
(snps,axi-dma-1.01a, sophgo,cv1800b, starfive,jh7110/jh8100) operate
without it. For those, reaching this path during slave/cyclic prep and
terminate is the normal no-op case, not an error, and the err log spams
dmesg on every transfer.

Demote it to dev_dbg to match the identical check in
dw_axi_dma_set_byte_halfword().

Signed-off-by: Baineng Shou <shoubaineng@gmail.com>
---
 drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
index bcefaff03b5c..d829b9af5491 100644
--- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
+++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
@@ -574,7 +574,7 @@ static void dw_axi_dma_set_hw_channel(struct axi_dma_chan *chan, bool set)
 	unsigned long reg_value, val;
 
 	if (!chip->apb_regs) {
-		dev_err(chip->dev, "apb_regs not initialized\n");
+		dev_dbg(chip->dev, "apb_regs not initialized\n");
 		return;
 	}
 
-- 
2.34.1


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

* Re: [PATCH] dmaengine: dw-axi-dmac: demote apb_regs warning to debug
  2026-09-18 10:14 [PATCH] dmaengine: dw-axi-dmac: demote apb_regs warning to debug Baineng Shou
@ 2026-09-18 14:04 ` Frank Li
  0 siblings, 0 replies; 4+ messages in thread
From: Frank Li @ 2026-09-18 14:04 UTC (permalink / raw)
  To: Baineng Shou; +Cc: Eugeniy.Paltsev, vkoul, Frank.Li, dmaengine, linux-kernel

On Fri, Sep 18, 2026 at 06:14:14PM +0800, Baineng Shou wrote:
> dw_axi_dma_set_hw_channel() logs "apb_regs not initialized" at err
> level when apb_regs is absent. However, only Intel KeemBay
> ("intel,kmb-axi-dma") carries apb_regs; all other supported SoCs
> (snps,axi-dma-1.01a, sophgo,cv1800b, starfive,jh7110/jh8100) operate
> without it. For those, reaching this path during slave/cyclic prep and
> terminate is the normal no-op case, not an error, and the err log spams
> dmesg on every transfer.
>
> Demote it to dev_dbg to match the identical check in
> dw_axi_dma_set_byte_halfword().
>
> Signed-off-by: Baineng Shou <shoubaineng@gmail.com>
> ---

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

>  drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> index bcefaff03b5c..d829b9af5491 100644
> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
> @@ -574,7 +574,7 @@ static void dw_axi_dma_set_hw_channel(struct axi_dma_chan *chan, bool set)
>  	unsigned long reg_value, val;
>
>  	if (!chip->apb_regs) {
> -		dev_err(chip->dev, "apb_regs not initialized\n");
> +		dev_dbg(chip->dev, "apb_regs not initialized\n");
>  		return;
>  	}
>
> --
> 2.34.1
>

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

end of thread, other threads:[~2026-09-18 14:04 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-18 10:14 [PATCH] dmaengine: dw-axi-dmac: demote apb_regs warning to debug Baineng Shou
2026-09-18 14:04 ` Frank Li
  -- strict thread matches above, loose matches on Subject: below --
2026-09-10  8:09 Baineng Shou
2026-09-10  8:22 ` sashiko-bot

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