Re: [PATCH v4 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:

Pre-existing issues:
- [High] The first burst of the initial chunk is populated but skipped during hardware descriptor programming, leading to silent data loss.
--

--- Patch [6]: [PATCH v4 06/10] dmaengine: dw-edma: Add callbacks to fill link list entries ---
commit 8217e6927a330e1d9e36c8d3201ab9c4807b55cf
Author: Frank Li <Frank.Li@nxp.com>
Subject: dmaengine: dw-edma: Add callbacks to fill link list entries

This commit introduces four new callbacks to fill link list entries in
preparation for replacing the existing core start routines. The abstraction
allows the code to be cleanly shared between eDMA and HDMA implementations.

Tested-by: Koichiro Den <den@valinux.co.jp>
Signed-off-by: Frank Li <Frank.Li@nxp.com>

> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> index 8d38867cd9983..c0746e5351410 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_data(struct dw_edma_chan *chan, struct dw_edma_burst *burst,
> +			u32 idx, bool cb, bool irq)
> +{

[Severity: High]
This isn't a bug introduced by this patch, but since this new callback is
being introduced to handle filling link list data entries, does the existing
driver logic silently drop the first data segment?

Looking at dw_edma_device_transfer() in dw-edma-core.c, it allocates and
populates the first burst directly into chunk->burst:

dw_edma_device_transfer()
    ...
    burst = dw_edma_alloc_burst(chunk);
    if (unlikely(!burst))
        goto err_alloc;

    if (xfer->type == EDMA_XFER_CYCLIC)
        burst->sz = xfer->xfer.cyclic.len;

However, the hardware descriptor programming loop in dw-edma-v0-core.c treats
chunk->burst->list as a dummy head rather than a data element:

dw_edma_v0_core_write_chunk()
    ...
    list_for_each_entry(child, &chunk->burst->list, list) {
        ...
        dw_edma_v0_write_ll_data(chan, i++, control, child->sz,
                                 child->sar, child->dar);
    }

Since list_for_each_entry() starts from head->next and skips the head itself,
will this skip programming the first burst entirely and cause silent data loss
for scatter-gather, cyclic, or single block transfers?

> +	u32 control = 0;
> +
> +	if (cb)
> +		control |= DW_EDMA_V0_CB;
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-edma_ll-v4-0-cc128f0afb61@nxp.com?part=6