Re: [PATCH 03/17] dmaengine: dw-edma: Terminate STOP requests without callbacks

[Koichiro Den]

On Mon, Jun 15, 2026 at 01:37:06PM -0500, Frank Li wrote:
> On Tue, Jun 16, 2026 at 12:40:57AM +0900, Koichiro Den wrote:
> > The STOP request path handles device_terminate_all(). 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 used vchan_cookie_complete() for a stopped descriptor. This
> > queues the descriptor on the completed list and schedules its callback.
> > A late callback after dmaengine_terminate_sync() can dereference
> > callback state, such as a request object, that the client has already
> > freed.
> >
> > Move stopped descriptors to the terminated list. Complete the cookie
> > before doing so, so cookie polling observes that the transfer is no
> > longer in flight, but do not schedule the completion callback. Add a
> > synchronize callback so virt-dma can release terminated descriptors.
> >
> > Fixes: e63d79d1ffcd ("dmaengine: Add Synopsys eDMA IP core driver")
> > Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>
> > ---
> >  drivers/dma/dw-edma/dw-edma-core.c | 18 ++++++++++++++++--
> >  1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> > index d99b6256660a..bedaee6d30ab 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > @@ -106,6 +106,13 @@ static int dw_edma_start_transfer(struct dw_edma_chan *chan)
> >  	return 1;
> >  }
> >
> > +static void dw_edma_terminate_vdesc(struct virt_dma_desc *vd)
> > +{
> > +	list_del(&vd->node);
> > +	dma_cookie_complete(&vd->tx);
> > +	vchan_terminate_vdesc(vd);
> 
> Is it vchan_terminate_vdesc() missing call dma_cookie_complete()?

I think it is technically possible with some adjustments here and there, but I
am not sure it would be worth it.

- If vchan_terminate_vdesc() starts calling dma_cookie_complete(), all callers
  would have to pass descriptors to it in cookie order. That is not part of the
  helper's contract today.
- There would also be some fallout in existing users. A few drivers already call
  dma_cookie_complete() before vchan_terminate_vdesc(), so they would need to be
  changed, or dma_cookie_complete() would have to become idempotent instead of
  BUG_ON()-ing on an already completed cookie.
- Some users would need ordering changes as well. For example,
  xilinx_dpdma_synchronize() currently handles pending before active.

> 
> > +}
> > +
> >  static void dw_edma_device_caps(struct dma_chan *dchan,
> >  				struct dma_slave_caps *caps)
> >  {
> > @@ -537,8 +544,7 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> >  			break;
> >
> >  		case EDMA_REQ_STOP:
> > -			list_del(&vd->node);
> > -			vchan_cookie_complete(vd);
> > +			dw_edma_terminate_vdesc(vd);
> >  			chan->request = EDMA_REQ_NONE;
> >  			chan->status = EDMA_ST_IDLE;
> >  			break;
> > @@ -610,6 +616,13 @@ static int dw_edma_alloc_chan_resources(struct dma_chan *dchan)
> >  	return 0;
> >  }
> >
> > +static void dw_edma_device_synchronize(struct dma_chan *dchan)
> > +{
> > +	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> > +
> > +	vchan_synchronize(&chan->vc);
> > +}
> > +
> >  static void dw_edma_free_chan_resources(struct dma_chan *dchan)
> >  {
> >  	unsigned long timeout = jiffies + msecs_to_jiffies(5000);
> > @@ -723,6 +736,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;
> 
> Can we provide generally call back like, vchan_synchroniz_dmachan(), or
> change existing vchan_synchronize() to by using struct dma_chan *dchan.

I think the former is possible. We could add a vchan_synchronize_dmachan()
helper that calls vchan_synchronize(to_virt_chan(dchan)) internally. That would
avoid bouncing back through the driver-specific channel representation just to
get back to the virt-dma channel. I can send a small cleanup series for that.

Best regards,
Koichiro

> 
> avoid duplicate it every dmaegine drivers.
> 
> Frank
> 
> >  	dma->device_issue_pending = dw_edma_device_issue_pending;
> >  	dma->device_tx_status = dw_edma_device_tx_status;
> >  	dma->device_prep_slave_sg_config = dw_edma_device_prep_slave_sg_config;
> > --
> > 2.51.0
> >