Re: [PATCH v3 06/10] dmaengine: dw-edma: Add callbacks to fill link list entries

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The `addr` parameter is completely ignored in both the `dw_edma_v0_core_ll_link` and `dw_hdma_v0_core_ll_link` callbacks, with the code incorrectly hardcoding `chan->ll_region.paddr` instead.
--

commit b095560e132d78a5d96555e8baeab1d49da1b0c1
Author: Frank Li <Frank.Li@nxp.com>

dmaengine: dw-edma: Add callbacks to fill link list entries

This commit introduces four new callbacks to fill link list entries and handle
channel operations. This abstraction allows sharing logic between the eDMA and
HDMA paths to avoid duplicating code.

> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> index 8d38867cd9983..10ad63d7e6016 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c

[ ... ]

> @@ -509,6 +509,48 @@ static void dw_edma_v0_core_ch_config(struct dw_edma_chan *chan)
>  	}
>  }
>  
[ ... ]
> +static void
> +dw_edma_v0_core_ll_link(struct dw_edma_chan *chan, u32 idx, bool cb, u64 addr)
> +{
> +	u32 control = DW_EDMA_V0_LLP | DW_EDMA_V0_TCB;
> +
> +	if (!cb)
> +		control |= DW_EDMA_V0_CB;
> +
> +	dw_edma_v0_write_ll_link(chan, idx, control, chan->ll_region.paddr);

[Severity: Low]
Could this omission cause a regression if the caller expects the provided addr
to be used? The callback signature explicitly takes a u64 addr argument, but
the implementation ignores it and hardcodes chan->ll_region.paddr instead.

> +}
> +

[ ... ]

> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> index 31bbdc6a40642..52c6ea09fcab5 100644
> --- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c

[ ... ]

> @@ -348,6 +348,40 @@ static void dw_hdma_v0_core_ch_config(struct dw_edma_chan *chan)
>  	SET_CH_32(dw, chan->dir, chan->id, msi_msgdata, chan->msi.data);
>  }
>  
[ ... ]
> +static void
> +dw_hdma_v0_core_ll_link(struct dw_edma_chan *chan, u32 idx, bool cb, u64 addr)
> +{
> +	u32 control = DW_HDMA_V0_LLP | DW_HDMA_V0_TCB;
> +
> +	if (!cb)
> +		control |= DW_HDMA_V0_CB;
> +
> +	dw_hdma_v0_write_ll_link(chan, idx, control, chan->ll_region.paddr);

[Severity: Low]
What about the HDMA implementation? The addr argument passed to
dw_hdma_v0_core_ll_link() is also completely ignored in favor of hardcoding
chan->ll_region.paddr.

> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-edma_ll-v3-0-877aa463740c@nxp.com?part=6