Re: [PATCH v3 8/9] dmaengine: sun6i: synchronize callbacks before releasing channel resources

[Frank Li]

On Fri, Oct 23, 2026 at 05:38:27PM -0600, James Hilliard wrote:
> The driver has no device_synchronize callback. dmaengine_terminate_sync()
> therefore returns after stopping the channel without waiting for the
> virtual-channel tasklet. That tasklet may already have removed completed
> descriptors from the shared list and be executing their callbacks. For
> example, serial8250_release_dma() frees the RX buffer and unmaps the TX
> buffer immediately after terminate_sync(), while those callbacks can
> still access them.
>
> Use vchan_dma_synchronize() for device_synchronize. Channel termination
> excludes the interrupt handler and allocator before detaching the channel,
> so they cannot schedule another completion for that transfer. Also stop
> the channel and synchronize when freeing its resources.
>
> Keep terminated cyclic descriptors on desc_terminated until synchronization
> finishes. vchan_complete() may retain that descriptor outside vc.lock.
> Ordinary active descriptors have not published a callback and can still be
> freed by terminate_all(). This avoids accumulating descriptors on each
> 8250 partial-RX flush, which uses the atomic termination API.
>
> Fixes: 555859308723 ("dmaengine: sun6i: Add driver for the Allwinner A31 DMA controller")
> Signed-off-by: James Hilliard <james.hilliard1@xxxxxxxxx>
> ---

Reviewed-by: Frank Li <Frank.Li@xxxxxxx>

>  drivers/dma/sun6i-dma.c | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index 25987f550707..b83138745bd1 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -999,12 +999,20 @@ static int sun6i_dma_terminate_all(struct dma_chan *chan)
>  	if (pchan && pchan->desc) {
>  		struct virt_dma_desc *vd = &pchan->desc->vd;
>
> -		vchan_terminate_vdesc(vd);
> +		/* A cyclic callback may still hold this descriptor outside vc.lock. */
> +		if (vchan->cyclic)
> +			vchan_terminate_vdesc(vd);
> +		else
> +			list_add_tail(&vd->node, &head);
>  	}
>
>  	vchan->cyclic = false;
>  	vchan->paused = false;
> -	vchan_get_all_descriptors(&vchan->vc, &head);
> +	/* Leave terminated cyclic descriptors for the sleepable synchronize. */
> +	list_splice_tail_init(&vchan->vc.desc_allocated, &head);
> +	list_splice_tail_init(&vchan->vc.desc_submitted, &head);
> +	list_splice_tail_init(&vchan->vc.desc_issued, &head);
> +	list_splice_tail_init(&vchan->vc.desc_completed, &head);
>
>  	if (pchan) {
>  		writel(DMA_CHAN_ENABLE_STOP, pchan->base + DMA_CHAN_ENABLE);
> @@ -1103,14 +1111,10 @@ static void sun6i_dma_issue_pending(struct dma_chan *chan)
>
>  static void sun6i_dma_free_chan_resources(struct dma_chan *chan)
>  {
> -	struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(chan->device);
>  	struct sun6i_vchan *vchan = to_sun6i_vchan(chan);
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&sdev->lock, flags);
> -	list_del_init(&vchan->node);
> -	spin_unlock_irqrestore(&sdev->lock, flags);
>
> +	sun6i_dma_terminate_all(chan);
> +	vchan_dma_synchronize(chan);
>  	vchan_free_chan_resources(&vchan->vc);
>  }
>
> @@ -1483,6 +1487,7 @@ static int sun6i_dma_probe(struct platform_device *pdev)
>  	sdc->slave.device_pause			= sun6i_dma_pause;
>  	sdc->slave.device_resume		= sun6i_dma_resume;
>  	sdc->slave.device_terminate_all		= sun6i_dma_terminate_all;
> +	sdc->slave.device_synchronize		= vchan_dma_synchronize;
>  	sdc->slave.src_addr_widths		= sdc->cfg->src_addr_widths;
>  	sdc->slave.dst_addr_widths		= sdc->cfg->dst_addr_widths;
>  	sdc->slave.directions			= BIT(DMA_DEV_TO_MEM) |
>
> --
> 2.53.0
>