Re: [PATCH v3 5/9] dmaengine: sun6i: detach completed descriptors before invoking callbacks

[Frank Li <Frank.li@oss.nxp.com>]

On Fri, Oct 23, 2026 at 05:38:24PM -0600, James Hilliard wrote:
> Finite completion publishes a descriptor to virt-dma but leaves pchan->desc
> pointing to it until the allocator runs. The virtual-channel tasklet may
> free that descriptor first. A residue query for another incomplete cookie
> can still pass the retired descriptor to sun6i_get_chan_size().
>
> Clear the active descriptor pointer before publishing completion. An
> assigned physical channel without a descriptor is ready for the allocator
> to start its next transfer or release it. Use that state in place of the
> completed-descriptor pointer, so residue queries and termination only
> consult descriptors still owned by the physical channel.
>
> Fixes: 555859308723 ("dmaengine: sun6i: Add driver for the Allwinner A31 DMA controller")
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/dma/sun6i-dma.c | 23 ++++++++++-------------
>  1 file changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index d3a63c866645..900bb629ba3b 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -198,7 +198,6 @@ struct sun6i_pchan {
>  	void __iomem		*base;
>  	struct sun6i_vchan	*vchan;
>  	struct sun6i_desc	*desc;
> -	struct sun6i_desc	*done;
>  };
>
>  struct sun6i_vchan {
> @@ -490,16 +489,12 @@ static int sun6i_dma_start_desc(struct sun6i_vchan *vchan)
>  	if (!pchan)
>  		return -EAGAIN;
>
> -	if (!desc) {
> -		pchan->desc = NULL;
> -		pchan->done = NULL;
> +	if (!desc)
>  		return -EAGAIN;
> -	}
>
>  	list_del(&desc->node);
>
>  	pchan->desc = to_sun6i_desc(&desc->tx);
> -	pchan->done = NULL;
>
>  	sun6i_dma_dump_lli(vchan, pchan->desc->v_lli, pchan->desc->p_lli);
>
> @@ -536,7 +531,7 @@ static void sun6i_dma_tasklet(struct tasklet_struct *t)
>
>  		pchan = vchan->phy;
>
> -		if (pchan && pchan->done) {
> +		if (pchan && !pchan->desc) {
>  			if (sun6i_dma_start_desc(vchan)) {
>  				/*
>  				 * No current txd associated with this channel
> @@ -582,6 +577,7 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
>  	u32 channels_per_reg = sdev->cfg->num_channels_per_reg;
>  	struct sun6i_vchan *vchan;
>  	struct sun6i_pchan *pchan;
> +	struct sun6i_desc *desc;
>  	unsigned long flags;
>  	int i, j, ret = IRQ_NONE;
>  	u32 status;
> @@ -601,13 +597,15 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
>  		     i * channels_per_reg + j < sdev->num_pchans; j++) {
>  			pchan = sdev->pchans + i * channels_per_reg + j;
>  			vchan = pchan->vchan;
> -			if (vchan && (status & vchan->irq_type)) {
> +			desc = pchan->desc;
> +			if (vchan && desc && (status & vchan->irq_type)) {
>  				spin_lock(&vchan->vc.lock);
>  				if (vchan->cyclic) {
> -					vchan_cyclic_callback(&pchan->desc->vd);
> +					vchan_cyclic_callback(&desc->vd);
>  				} else {
> -					vchan_cookie_complete(&pchan->desc->vd);
> -					pchan->done = pchan->desc;
> +					/* Completion transfers ownership to virt-dma. */
> +					pchan->desc = NULL;
> +					vchan_cookie_complete(&desc->vd);
>  				}
>  				spin_unlock(&vchan->vc.lock);
>  			}
> @@ -983,7 +981,7 @@ static int sun6i_dma_terminate_all(struct dma_chan *chan)
>  	pchan = vchan->phy;
>  	list_del_init(&vchan->node);
>
> -	if (pchan && pchan->desc && pchan->desc != pchan->done) {
> +	if (pchan && pchan->desc) {
>  		struct virt_dma_desc *vd = &pchan->desc->vd;
>
>  		vchan_terminate_vdesc(vd);
> @@ -999,7 +997,6 @@ static int sun6i_dma_terminate_all(struct dma_chan *chan)
>  		vchan->phy = NULL;
>  		pchan->vchan = NULL;
>  		pchan->desc = NULL;
> -		pchan->done = NULL;
>  	}
>
>  	spin_unlock(&vchan->vc.lock);
>
> --
> 2.53.0
>