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

[Koichiro Den]

On Wed, Jul 15, 2026 at 01:54:01PM -0500, Frank Li wrote:
> 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()

Thanks for the suggestion. Will update this in v4.

Best regards,
Koichiro

> 
> Frank