Re: [PATCH v5 08/10] dmaengine: dw-edma: Use common dw_edma_core_start() for both eDMA and HDMA

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Race condition and lockdep violation in dw_edma_device_resume via lockless virt-dma list access
--

--- Patch [8]: [PATCH v5 08/10] dmaengine: dw-edma: Use common dw_edma_core_start() for both eDMA and HDMA ---
commit bffbb7894cff6d55d1d7037fe698320bf5288b87
Author: Frank Li <Frank.Li@nxp.com>

dmaengine: dw-edma: Use common dw_edma_core_start() for both eDMA and HDMA

This patch consolidates the eDMA and HDMA start routines by introducing a
common dw_edma_core_start() function. It removes the now-redundant start
callbacks from the eDMA and HDMA core structures.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 2652ad8e7a8f6..f52d9fd18e573 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c

[ ... ]

> @@ -183,7 +211,7 @@ static int dw_edma_start_transfer(struct dw_edma_chan *chan)
>  	if (!child)
>  		return 0;
>  
> -	dw_edma_core_start(dw, child, !desc->xfer_sz);
> +	dw_edma_core_start(child, !desc->xfer_sz);

[Severity: High]
This is a pre-existing issue, but can this lead to a race condition and
lockdep splat when called from dw_edma_device_resume()?

When the client driver calls dmaengine_resume(), it reaches
dw_edma_device_resume(). This function evaluates if the channel is paused,
and calls dw_edma_start_transfer() without acquiring chan->vc.lock:

drivers/dma/dw-edma/dw-edma-core.c:dw_edma_device_resume() {
	...
	} else {
		chan->status = EDMA_ST_BUSY;
		dw_edma_start_transfer(chan);
	}
}

Inside dw_edma_start_transfer(), vchan_next_desc() is called, which
immediately trips lockdep_assert_held(&vc->lock). 

If an in-flight or delayed interrupt triggers dw_edma_done_interrupt()
concurrently, both paths may execute dw_edma_start_transfer() simultaneously
on the same descriptor, which could result in double list_del() and kfree()
on the chunk.

Should dw_edma_device_resume() take the chan->vc.lock before calling
dw_edma_start_transfer()?

>  	desc->xfer_sz += child->xfer_sz;
>  	dw_edma_free_burst(child);
>  	list_del(&child->list);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-edma_ll-v5-0-e199053d4300@nxp.com?part=8