Re: [PATCH 05/17] dmaengine: dw-edma: Serialize channel state checks

[Frank Li]

On Tue, Jun 16, 2026 at 12:40:59AM +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")
> Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>
> ---

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

>  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 2777dc0b2aed..489f7fe49840 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -177,8 +177,10 @@ dw_edma_device_get_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);
>  	if (!chan->configured)
>  		err = -EPERM;
>  	else if (chan->status != EDMA_ST_BUSY)
> @@ -187,6 +189,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;
>  }
> @@ -194,8 +197,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) {
> @@ -206,6 +211,7 @@ static int dw_edma_device_resume(struct dma_chan *dchan)
>  		chan->status = EDMA_ST_BUSY;
>  		dw_edma_start_transfer(chan);
>  	}
> +	spin_unlock_irqrestore(&chan->vc.lock, flags);
>
>  	return err;
>  }
> @@ -249,11 +255,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
>