Re: [PATCH v3 3/9] dmaengine: sun6i: take the device lock before the virtual channel lock

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

On Fri, Oct 23, 2026 at 05:38:22PM -0600, James Hilliard wrote:
> The pending queue is protected by sdev->lock, while virt-dma uses vc.lock
> for descriptor lists and cookies. issue_pending() and resume() currently
> take the device lock inside the virtual-channel lock when queueing work.
>
> Take sdev->lock first in both paths, and inspect the physical-channel
> assignment after acquiring the locks. This establishes the nesting order
> needed to keep allocation and descriptor startup in one critical section.
> virt-dma preparation, submission and callback processing continue to use
> vc.lock independently.
>
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---

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

>  drivers/dma/sun6i-dma.c | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index 3a6525dd3d32..435d7daa1c0f 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -952,23 +952,24 @@ static int sun6i_dma_resume(struct dma_chan *chan)
>  {
>  	struct sun6i_dma_dev *sdev = to_sun6i_dma_dev(chan->device);
>  	struct sun6i_vchan *vchan = to_sun6i_vchan(chan);
> -	struct sun6i_pchan *pchan = vchan->phy;
> +	struct sun6i_pchan *pchan;
>  	unsigned long flags;
>
>  	dev_dbg(dmaengine_chan_dev(chan), "vchan %p: resume\n", &vchan->vc);
>
> -	spin_lock_irqsave(&vchan->vc.lock, flags);
> +	spin_lock_irqsave(&sdev->lock, flags);
> +	spin_lock(&vchan->vc.lock);
> +	pchan = vchan->phy;
>
>  	if (pchan) {
>  		writel(DMA_CHAN_PAUSE_RESUME,
>  		       pchan->base + DMA_CHAN_PAUSE);
>  	} else if (!list_empty(&vchan->vc.desc_issued)) {
> -		spin_lock(&sdev->lock);
>  		list_add_tail(&vchan->node, &sdev->pending);
> -		spin_unlock(&sdev->lock);
>  	}
>
> -	spin_unlock_irqrestore(&vchan->vc.lock, flags);
> +	spin_unlock(&vchan->vc.lock);
> +	spin_unlock_irqrestore(&sdev->lock, flags);
>
>  	return 0;
>  }
> @@ -1057,25 +1058,23 @@ static void sun6i_dma_issue_pending(struct dma_chan *chan)
>  	struct sun6i_vchan *vchan = to_sun6i_vchan(chan);
>  	unsigned long flags;
>
> -	spin_lock_irqsave(&vchan->vc.lock, flags);
> +	spin_lock_irqsave(&sdev->lock, flags);
> +	spin_lock(&vchan->vc.lock);
>
>  	if (vchan_issue_pending(&vchan->vc)) {
> -		spin_lock(&sdev->lock);
> -
>  		if (!vchan->phy && list_empty(&vchan->node)) {
>  			list_add_tail(&vchan->node, &sdev->pending);
>  			tasklet_schedule(&sdev->task);
>  			dev_dbg(dmaengine_chan_dev(chan), "vchan %p: issued\n",
>  				&vchan->vc);
>  		}
> -
> -		spin_unlock(&sdev->lock);
>  	} else {
>  		dev_dbg(dmaengine_chan_dev(chan), "vchan %p: nothing to issue\n",
>  			&vchan->vc);
>  	}
>
> -	spin_unlock_irqrestore(&vchan->vc.lock, flags);
> +	spin_unlock(&vchan->vc.lock);
> +	spin_unlock_irqrestore(&sdev->lock, flags);
>  }
>
>  static void sun6i_dma_free_chan_resources(struct dma_chan *chan)
>
> --
> 2.53.0
>