Re: [PATCH v3 5/9] dmaengine: dw-edma: Serialize channel state checks

[Frank Li]

On Thu, Jul 16, 2026 at 02:57:36AM +0900, Koichiro Den wrote:
> pause() and resume() read and update channel state without holding vc.lock,
> while the interrupt handlers update the same state under it. Take the same
> lock around those state checks so that request, status, and configured stay
> consistent.
>
> For example, pause() can observe EDMA_ST_BUSY right before the interrupt
> handler completes the final descriptor and moves the channel to
> EDMA_ST_IDLE, and then record EDMA_REQ_PAUSE on an already idle channel. No
> further interrupt will acknowledge the request, and since issue_pending()
> requires EDMA_REQ_NONE, the channel is wedged for good: terminate_all()
> leaves the stale request behind, so even reconfiguring the channel does not
> recover it.
>
> issue_pending() already runs under vc.lock, but it tests configured before
> taking it. Move that test under the lock as well, so that the decision to
> start work is made against the current value rather than one observed
> before a concurrent terminate_all() deconfigured the channel.
>
> Fixes: e63d79d1ffcd ("dmaengine: Add Synopsys eDMA IP core driver")
> Reviewed-by: Frank Li <Frank.Li@xxxxxxx>
> Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>
> ---
> Changes in v3:
>   - No changes.
>
>  drivers/dma/dw-edma/dw-edma-core.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index b06b299661c0..8b0e2af734da 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -300,8 +300,10 @@ static int dw_edma_device_config(struct dma_chan *dchan,
>  static int dw_edma_device_pause(struct dma_chan *dchan)
>  {
>  	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> +	unsigned long flags;
>  	int err = 0;
>
> +	spin_lock_irqsave(&chan->vc.lock, flags);

new code, can you use clean up guard()

Frank

>  	if (!chan->configured)
>  		err = -EPERM;
>  	else if (chan->status != EDMA_ST_BUSY)
> @@ -310,6 +312,7 @@ static int dw_edma_device_pause(struct dma_chan *dchan)
>  		err = -EPERM;
>  	else
>  		chan->request = EDMA_REQ_PAUSE;
> +	spin_unlock_irqrestore(&chan->vc.lock, flags);
>
>  	return err;
>  }
> @@ -317,8 +320,10 @@ static int dw_edma_device_pause(struct dma_chan *dchan)
>  static int dw_edma_device_resume(struct dma_chan *dchan)
>  {
>  	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
> +	unsigned long flags;
>  	int err = 0;
>
> +	spin_lock_irqsave(&chan->vc.lock, flags);
>  	if (!chan->configured) {
>  		err = -EPERM;
>  	} else if (chan->status != EDMA_ST_PAUSE) {
> @@ -330,6 +335,7 @@ static int dw_edma_device_resume(struct dma_chan *dchan)
>  		if (!dw_edma_start_transfer(chan))
>  			chan->status = EDMA_ST_IDLE;
>  	}
> +	spin_unlock_irqrestore(&chan->vc.lock, flags);
>
>  	return err;
>  }
> @@ -373,11 +379,9 @@ static void dw_edma_device_issue_pending(struct dma_chan *dchan)
>  	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
>  	unsigned long flags;
>
> -	if (!chan->configured)
> -		return;
> -
>  	spin_lock_irqsave(&chan->vc.lock, flags);
> -	if (vchan_issue_pending(&chan->vc) && chan->request == EDMA_REQ_NONE &&
> +	if (chan->configured && vchan_issue_pending(&chan->vc) &&
> +	    chan->request == EDMA_REQ_NONE &&
>  	    chan->status == EDMA_ST_IDLE) {
>  		chan->status = EDMA_ST_BUSY;
>  		dw_edma_start_transfer(chan);
> --
> 2.51.0
>