Re: [PATCH v2 1/3] dmaengine: dw-edma: Implement device_synchronize() callback

[Manivannan Sadhasivam]

On Tue, Jul 21, 2026 at 09:05:59PM -0400, Frank Li wrote:
> On Tue, Jul 14, 2026 at 02:08:32PM +0200, Manivannan Sadhasivam wrote:
> > device_synchronize() callback is required by the client drivers to ensure
> > all the DMA operations are completed so that they can free the memory
> > associated with the complete callbacks.
> >
> > So implement this callback by first making sure that all the in-flight DMA
> > operations are completed and then call vchan_synchronize() to drain the
> > DMA tasklet.
> >
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxxxxxxxx>
> > ---
> 
> Similar implement already in linux-next, recent many fixes and update about
> dw edma already merged in dma tree.
> 

Ah, I didn't notice this patch. I've dropped this patch from my series.

This patch also suffers from the same RMW issue that Sashiko pointed out. So
I'll send a separate fix for that.

- Mani

> commit 99109a51efd28c9a661fbfb9469b023c517b31d1
> Author: Koichiro Den <den@xxxxxxxxxxxxx>
> Date:   Sat Jul 18 03:06:32 2026 +0900
> 
>     dmaengine: dw-edma: Terminate all descriptors without callbacks
> 
>     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 for such a pending STOP to
>     complete 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")
>     Reviewed-by: Frank Li <Frank.Li@xxxxxxx>
>     Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>
>     Link: https://patch.msgid.link/20260717180639.2643243-3-den@xxxxxxxxxxxxx
>     Signed-off-by: Vinod Koul <vkoul@xxxxxxxxxx>
> 
> Frank
> 
> >  drivers/dma/dw-edma/dw-edma-core.c | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> > index c2feb3adc79f..df0d1a946ed0 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > @@ -12,6 +12,7 @@
> >  #include <linux/dmaengine.h>
> >  #include <linux/err.h>
> >  #include <linux/interrupt.h>
> > +#include <linux/iopoll.h>
> >  #include <linux/irq.h>
> >  #include <linux/dma/edma.h>
> >  #include <linux/dma-mapping.h>
> > @@ -331,6 +332,20 @@ static int dw_edma_device_terminate_all(struct dma_chan *dchan)
> >  	return err;
> >  }
> >
> > +static void dw_edma_device_synchronize(struct dma_chan *dchan)
> > +{
> > +	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> > +
> > +	/*
> > +	 * Make sure all the in-flight DMA operations are completed before
> > +	 * draining the tasklet using vchan_synchronize().
> > +	 */
> > +	read_poll_timeout(READ_ONCE, chan->status, chan->status != EDMA_ST_BUSY,
> > +			  10, 0, false, chan->status);
> > +
> > +	vchan_synchronize(&chan->vc);
> > +}
> > +
> >  static void dw_edma_device_issue_pending(struct dma_chan *dchan)
> >  {
> >  	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> > @@ -968,6 +983,7 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
> >  	dma->device_pause = dw_edma_device_pause;
> >  	dma->device_resume = dw_edma_device_resume;
> >  	dma->device_terminate_all = dw_edma_device_terminate_all;
> > +	dma->device_synchronize = dw_edma_device_synchronize;
> >  	dma->device_issue_pending = dw_edma_device_issue_pending;
> >  	dma->device_tx_status = dw_edma_device_tx_status;
> >  	dma->device_prep_slave_sg = dw_edma_device_prep_slave_sg;
> >
> > --
> > 2.43.0
> >

-- 
மணிவண்ணன் சதாசிவம்