Re: [PATCH v3 2/9] dmaengine: dw-edma: Terminate all descriptors without callbacks

[Frank Li]

On Thu, Jul 16, 2026 at 02:57:33AM +0900, Koichiro Den wrote:
> The DMA Engine client documentation says in the "Terminate APIs" section
> of Documentation/driver-api/dmaengine/client.rst:
>
> "No callback functions will be called for any incomplete transfers."
>
> dw-edma instead calls vchan_cookie_complete() when a deferred STOP reaches
> the interrupt handler. This schedules a callback for the active descriptor
> and leaves other issued or submitted descriptors queued. A late callback
> after dmaengine_terminate_sync() can dereference client state that has
> already been freed, while leftover descriptors may later restart into
> reused buffers or leak.
>
> Move all issued and submitted descriptors to the terminated list whenever
> termination completes. For a pending STOP, do this from both the DONE and
> ABORT paths. Complete their cookies in order without scheduling callbacks.
>
> A STOP can remain pending until the running transfer raises an interrupt.
> Make device_synchronize() wait until the channel has stopped and
> terminate_all() deconfigures it before releasing terminated descriptors.
> Reuse it from free_chan_resources(), then release the remaining virt-dma
> resources. Sleep instead of busy-polling while waiting, and warn if the
> existing timeout expires.
>
> Fixes: e63d79d1ffcd ("dmaengine: Add Synopsys eDMA IP core driver")
> Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>
> ---
> Changes in v3:
>   - Merge the descriptor cleanup patch so device_synchronize() is safe
>     when introduced. (Sashiko)
>   - Handle pending STOP requests from the ABORT path as well.
>   - Drop a redundant cond_resched() after usleep_range().
>   - Drop Frank's Reviewed-by tag due to these changes.
>   - Polish a source comment.
>
>  drivers/dma/dw-edma/dw-edma-core.c | 87 ++++++++++++++++++++++++++----
>  1 file changed, 76 insertions(+), 11 deletions(-)
>
...
> +static void dw_edma_wait_termination(struct dma_chan *dchan)
>  {
> +	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
>  	unsigned long timeout = jiffies + msecs_to_jiffies(5000);
> -	int ret;
> +	unsigned long flags;
> +	bool configured = true;
>
> +	/*
> +	 * dw_edma_device_terminate_all() may defer cleanup to a later interrupt
> +	 * while the channel is still running. Retry until the channel is
> +	 * deconfigured, which means termination is complete.
> +	 */
>  	while (time_before(jiffies, timeout)) {
> -		ret = dw_edma_device_terminate_all(dchan);
> -		if (!ret)
> -			break;
> +		dw_edma_device_terminate_all(dchan);
>
> -		if (time_after_eq(jiffies, timeout))
> +		spin_lock_irqsave(&chan->vc.lock, flags);
> +		configured = chan->configured;
> +		spin_unlock_irqrestore(&chan->vc.lock, flags);
> +		if (!configured)
>  			return;
>
> -		cpu_relax();
> +		usleep_range(1000, 2000);

now use fsleep()

Frank