[PATCH] dmaengine: sun4i: retain cyclic descriptors until callbacks drain

[PATCH] dmaengine: sun4i: retain cyclic descriptors until callbacks drain

[James Hilliard]

A cyclic IRQ queues a virt-dma tasklet with a pointer to the active
descriptor. sun4i_dma_terminate_all() frees it without clearing the queued
cyclic callback or waiting for one which has already started. The tasklet
can subsequently access the freed descriptor. There is no
device_synchronize callback to let clients drain these callbacks before
releasing their own resources either.

Serialize both half-transfer and end-transfer IRQ handling with termination
under the virtual-channel lock. Recheck the physical-channel association
after acquiring that lock so an IRQ which looked up the channel before
termination cannot use its detached transfer state.

Move terminated cyclic descriptors to the virt-dma terminated list. This
clears any pending cyclic callback. Retain them until a new synchronize
callback has waited for the IRQ handler and drained the tasklet. Drain
callbacks before freeing channel resources as well. Termination itself
remains usable from atomic context and from a completion callback.

Keep freeing other collected descriptors immediately: an in-flight
non-cyclic callback owns its descriptor on the tasklet's private list.

Fixes: b096c1377d1e ("dmaengine: sun4i: Add support for the DMA engine on sun[457]i SoCs")
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 drivers/dma/sun4i-dma.c | 46 +++++++++++++++++++++++++++++++++++-----------
 1 file changed, 35 insertions(+), 11 deletions(-)

diff --git a/drivers/dma/sun4i-dma.c b/drivers/dma/sun4i-dma.c
index 225e4a53502b..b92cd9c8676e 100644
--- a/drivers/dma/sun4i-dma.c
+++ b/drivers/dma/sun4i-dma.c
@@ -293,10 +293,20 @@ static int convert_buswidth(enum dma_slave_buswidth addr_width)
 	return (addr_width >> 1);
 }
 
+static void sun4i_dma_synchronize(struct dma_chan *chan)
+{
+	struct sun4i_dma_dev *priv = to_sun4i_dma_dev(chan->device);
+	struct sun4i_dma_vchan *vchan = to_sun4i_dma_vchan(chan);
+
+	synchronize_irq(priv->irq);
+	vchan_synchronize(&vchan->vc);
+}
+
 static void sun4i_dma_free_chan_resources(struct dma_chan *chan)
 {
 	struct sun4i_dma_vchan *vchan = to_sun4i_dma_vchan(chan);
 
+	sun4i_dma_synchronize(chan);
 	vchan_free_chan_resources(&vchan->vc);
 }
 
@@ -322,7 +332,7 @@ static struct sun4i_dma_pchan *find_and_use_pchan(struct sun4i_dma_dev *priv,
 	spin_lock_irqsave(&priv->lock, flags);
 	for_each_clear_bit_from(i, priv->pchans_used, max) {
 		pchan = &pchans[i];
-		pchan->vchan = vchan;
+		WRITE_ONCE(pchan->vchan, vchan);
 		set_bit(i, priv->pchans_used);
 		break;
 	}
@@ -339,7 +349,7 @@ static void release_pchan(struct sun4i_dma_dev *priv,
 
 	spin_lock_irqsave(&priv->lock, flags);
 
-	pchan->vchan = NULL;
+	WRITE_ONCE(pchan->vchan, NULL);
 	clear_bit(nr, priv->pchans_used);
 
 	spin_unlock_irqrestore(&priv->lock, flags);
@@ -970,13 +980,13 @@ static int sun4i_dma_terminate_all(struct dma_chan *chan)
 {
 	struct sun4i_dma_dev *priv = to_sun4i_dma_dev(chan->device);
 	struct sun4i_dma_vchan *vchan = to_sun4i_dma_vchan(chan);
-	struct sun4i_dma_pchan *pchan = vchan->pchan;
+	struct sun4i_dma_pchan *pchan;
+	struct virt_dma_desc *vd, *tmp;
 	LIST_HEAD(head);
 	unsigned long flags;
 
 	spin_lock_irqsave(&vchan->vc.lock, flags);
-	vchan_get_all_descriptors(&vchan->vc, &head);
-	spin_unlock_irqrestore(&vchan->vc.lock, flags);
+	pchan = vchan->pchan;
 
 	/*
 	 * Clearing the configuration register will halt the pchan. Interrupts
@@ -991,10 +1001,20 @@ static int sun4i_dma_terminate_all(struct dma_chan *chan)
 		release_pchan(priv, pchan);
 	}
 
-	spin_lock_irqsave(&vchan->vc.lock, flags);
 	/* Clear these so the vchan is usable again */
 	vchan->processing = NULL;
 	vchan->pchan = NULL;
+	vchan->contract = NULL;
+
+	/* A tasklet may already be using the active cyclic descriptor. */
+	vchan_get_all_descriptors(&vchan->vc, &head);
+	list_for_each_entry_safe(vd, tmp, &head, node) {
+		if (!to_sun4i_dma_contract(vd)->is_cyclic)
+			continue;
+
+		list_del(&vd->node);
+		vchan_terminate_vdesc(vd);
+	}
 	spin_unlock_irqrestore(&vchan->vc.lock, flags);
 
 	vchan_dma_desc_free_list(&vchan->vc, &head);
@@ -1128,9 +1148,15 @@ static irqreturn_t sun4i_dma_interrupt(int irq, void *dev_id)
 
 	for_each_set_bit(bit, &pendirq, 32) {
 		pchan = &pchans[bit >> 1];
-		vchan = pchan->vchan;
+		vchan = READ_ONCE(pchan->vchan);
 		if (!vchan) /* a terminated channel may still interrupt */
 			continue;
+
+		spin_lock(&vchan->vc.lock);
+		if (vchan->pchan != pchan || !vchan->processing) {
+			spin_unlock(&vchan->vc.lock);
+			continue;
+		}
 		contract = vchan->contract;
 
 		/*
@@ -1138,8 +1164,6 @@ static irqreturn_t sun4i_dma_interrupt(int irq, void *dev_id)
 		 * interrupt (odd bit)
 		 */
 		if (bit & 1) {
-			spin_lock(&vchan->vc.lock);
-
 			/*
 			 * Move the promise into the completed list now that
 			 * we're done with it
@@ -1173,8 +1197,6 @@ static irqreturn_t sun4i_dma_interrupt(int irq, void *dev_id)
 				disableirqs |= BIT(bit);
 				release_pchan(priv, pchan);
 			}
-
-			spin_unlock(&vchan->vc.lock);
 		} else {
 			/* Half done interrupt */
 			if (contract->is_cyclic)
@@ -1182,6 +1204,7 @@ static irqreturn_t sun4i_dma_interrupt(int irq, void *dev_id)
 			else
 				disableirqs |= BIT(bit);
 		}
+		spin_unlock(&vchan->vc.lock);
 	}
 
 	/* Disable the IRQs for events we handled */
@@ -1276,6 +1299,7 @@ static int sun4i_dma_probe(struct platform_device *pdev)
 	priv->slave.device_prep_dma_cyclic	= sun4i_dma_prep_dma_cyclic;
 	priv->slave.device_config		= sun4i_dma_config;
 	priv->slave.device_terminate_all	= sun4i_dma_terminate_all;
+	priv->slave.device_synchronize	= sun4i_dma_synchronize;
 	priv->slave.copy_align			= 2;
 	priv->slave.src_addr_widths		= BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
 						  BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |

---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
change-id: 20261103-submit-sun4i-dma-callback-lifetime-v1-b6192060b76f

Best regards,
--  
James Hilliard <james.hilliard1@gmail.com>

Re: [PATCH] dmaengine: sun4i: retain cyclic descriptors until callbacks drain

[Frank Li]

On Tue, Nov 03, 2026 at 11:25:58PM -0700, James Hilliard wrote:
> A cyclic IRQ queues a virt-dma tasklet with a pointer to the active
> descriptor. sun4i_dma_terminate_all() frees it without clearing the queued
> cyclic callback or waiting for one which has already started. The tasklet
> can subsequently access the freed descriptor. There is no
> device_synchronize callback to let clients drain these callbacks before
> releasing their own resources either.
>
> Serialize both half-transfer and end-transfer IRQ handling with termination
> under the virtual-channel lock. Recheck the physical-channel association
> after acquiring that lock so an IRQ which looked up the channel before
> termination cannot use its detached transfer state.
>
> Move terminated cyclic descriptors to the virt-dma terminated list. This
> clears any pending cyclic callback. Retain them until a new synchronize
> callback has waited for the IRQ handler and drained the tasklet. Drain
> callbacks before freeing channel resources as well. Termination itself
> remains usable from atomic context and from a completion callback.
>
> Keep freeing other collected descriptors immediately: an in-flight
> non-cyclic callback owns its descriptor on the tasklet's private list.
>
> Fixes: b096c1377d1e ("dmaengine: sun4i: Add support for the DMA engine on sun[457]i SoCs")
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---
>  drivers/dma/sun4i-dma.c | 46 +++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 35 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/dma/sun4i-dma.c b/drivers/dma/sun4i-dma.c
> index 225e4a53502b..b92cd9c8676e 100644
> --- a/drivers/dma/sun4i-dma.c
> +++ b/drivers/dma/sun4i-dma.c
> @@ -293,10 +293,20 @@ static int convert_buswidth(enum dma_slave_buswidth addr_width)
>  	return (addr_width >> 1);
>  }
>
> +static void sun4i_dma_synchronize(struct dma_chan *chan)
> +{
> +	struct sun4i_dma_dev *priv = to_sun4i_dma_dev(chan->device);
> +	struct sun4i_dma_vchan *vchan = to_sun4i_dma_vchan(chan);
> +
> +	synchronize_irq(priv->irq);
> +	vchan_synchronize(&vchan->vc);
> +}
> +
>  static void sun4i_dma_free_chan_resources(struct dma_chan *chan)
>  {
>  	struct sun4i_dma_vchan *vchan = to_sun4i_dma_vchan(chan);
>
> +	sun4i_dma_synchronize(chan);
>  	vchan_free_chan_resources(&vchan->vc);
>  }
>
> @@ -322,7 +332,7 @@ static struct sun4i_dma_pchan *find_and_use_pchan(struct sun4i_dma_dev *priv,
>  	spin_lock_irqsave(&priv->lock, flags);
>  	for_each_clear_bit_from(i, priv->pchans_used, max) {
>  		pchan = &pchans[i];
> -		pchan->vchan = vchan;
> +		WRITE_ONCE(pchan->vchan, vchan);
>  		set_bit(i, priv->pchans_used);
>  		break;
>  	}
> @@ -339,7 +349,7 @@ static void release_pchan(struct sun4i_dma_dev *priv,
>
>  	spin_lock_irqsave(&priv->lock, flags);
>
> -	pchan->vchan = NULL;
> +	WRITE_ONCE(pchan->vchan, NULL);
>  	clear_bit(nr, priv->pchans_used);
>
>  	spin_unlock_irqrestore(&priv->lock, flags);
> @@ -970,13 +980,13 @@ static int sun4i_dma_terminate_all(struct dma_chan *chan)
>  {
>  	struct sun4i_dma_dev *priv = to_sun4i_dma_dev(chan->device);
>  	struct sun4i_dma_vchan *vchan = to_sun4i_dma_vchan(chan);
> -	struct sun4i_dma_pchan *pchan = vchan->pchan;
> +	struct sun4i_dma_pchan *pchan;
> +	struct virt_dma_desc *vd, *tmp;
>  	LIST_HEAD(head);
>  	unsigned long flags;
>
>  	spin_lock_irqsave(&vchan->vc.lock, flags);
> -	vchan_get_all_descriptors(&vchan->vc, &head);
> -	spin_unlock_irqrestore(&vchan->vc.lock, flags);
> +	pchan = vchan->pchan;
>
>  	/*
>  	 * Clearing the configuration register will halt the pchan. Interrupts
> @@ -991,10 +1001,20 @@ static int sun4i_dma_terminate_all(struct dma_chan *chan)
>  		release_pchan(priv, pchan);
>  	}
>
> -	spin_lock_irqsave(&vchan->vc.lock, flags);
>  	/* Clear these so the vchan is usable again */
>  	vchan->processing = NULL;
>  	vchan->pchan = NULL;
> +	vchan->contract = NULL;
> +
> +	/* A tasklet may already be using the active cyclic descriptor. */
> +	vchan_get_all_descriptors(&vchan->vc, &head);
> +	list_for_each_entry_safe(vd, tmp, &head, node) {
> +		if (!to_sun4i_dma_contract(vd)->is_cyclic)
> +			continue;
> +
> +		list_del(&vd->node);
> +		vchan_terminate_vdesc(vd);
> +	}

Can you move this logic to vchan-dma.c/h ? I supose other vchan should use
this similar logic also.

Frank

>  	spin_unlock_irqrestore(&vchan->vc.lock, flags);
>
>  	vchan_dma_desc_free_list(&vchan->vc, &head);
> @@ -1128,9 +1148,15 @@ static irqreturn_t sun4i_dma_interrupt(int irq, void *dev_id)
>
>  	for_each_set_bit(bit, &pendirq, 32) {
>  		pchan = &pchans[bit >> 1];
> -		vchan = pchan->vchan;
> +		vchan = READ_ONCE(pchan->vchan);
>  		if (!vchan) /* a terminated channel may still interrupt */
>  			continue;
> +
> +		spin_lock(&vchan->vc.lock);
> +		if (vchan->pchan != pchan || !vchan->processing) {
> +			spin_unlock(&vchan->vc.lock);
> +			continue;
> +		}
>  		contract = vchan->contract;
>
>  		/*
> @@ -1138,8 +1164,6 @@ static irqreturn_t sun4i_dma_interrupt(int irq, void *dev_id)
>  		 * interrupt (odd bit)
>  		 */
>  		if (bit & 1) {
> -			spin_lock(&vchan->vc.lock);
> -
>  			/*
>  			 * Move the promise into the completed list now that
>  			 * we're done with it
> @@ -1173,8 +1197,6 @@ static irqreturn_t sun4i_dma_interrupt(int irq, void *dev_id)
>  				disableirqs |= BIT(bit);
>  				release_pchan(priv, pchan);
>  			}
> -
> -			spin_unlock(&vchan->vc.lock);
>  		} else {
>  			/* Half done interrupt */
>  			if (contract->is_cyclic)
> @@ -1182,6 +1204,7 @@ static irqreturn_t sun4i_dma_interrupt(int irq, void *dev_id)
>  			else
>  				disableirqs |= BIT(bit);
>  		}
> +		spin_unlock(&vchan->vc.lock);
>  	}
>
>  	/* Disable the IRQs for events we handled */
> @@ -1276,6 +1299,7 @@ static int sun4i_dma_probe(struct platform_device *pdev)
>  	priv->slave.device_prep_dma_cyclic	= sun4i_dma_prep_dma_cyclic;
>  	priv->slave.device_config		= sun4i_dma_config;
>  	priv->slave.device_terminate_all	= sun4i_dma_terminate_all;
> +	priv->slave.device_synchronize	= sun4i_dma_synchronize;
>  	priv->slave.copy_align			= 2;
>  	priv->slave.src_addr_widths		= BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
>  						  BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
>
> ---
> base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
> change-id: 20261103-submit-sun4i-dma-callback-lifetime-v1-b6192060b76f
>
> Best regards,
> --
> James Hilliard <james.hilliard1@gmail.com>
>