Re: [PATCH v3 07/24] dmaengine: dw-edma: Centralize LL doorbell decisions

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The DONE interrupt handler fails to clear EDMA_REQ_STOP and EDMA_REQ_PAUSE if the descriptor list is empty, contradicting the commit message's stated intent to prepare for LL progress reclamation.
--

--- Patch [7]: [PATCH v3 07/24] dmaengine: dw-edma: Centralize LL doorbell decisions ---
commit ee61354242533ea66a01a91592b3314e404a95eb
Author: Koichiro Den <[email protected]>

dmaengine: dw-edma: Centralize LL doorbell decisions
    
LL fill currently rings the doorbell every time. This patch moves that decision
to dw_edma_core_ch_maybe_doorbell(), which rings only for a BUSY LL channel with
pending entries. This centralization prevents EDMA_REQ_STOP completion from
restarting terminated work.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index b419ff2becf9e..c036510dabcb3 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c

[ ... ]

> @@ -733,6 +743,8 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
>  	default:
>  		break;
>  	}

[Severity: Medium]
Will this lead to a permanent channel hang when LL progress reclamation is
introduced?

Looking at dw_edma_done_interrupt() just above this change, the DONE interrupt
handler fails to clear EDMA_REQ_STOP and EDMA_REQ_PAUSE if the descriptor list
is empty:

drivers/dma/dw-edma/dw-edma-core.c:dw_edma_done_interrupt()
	case EDMA_REQ_STOP:
		vd = vchan_next_desc(&chan->vc);
		if (!vd)
			break;

		dw_edma_finish_termination(chan);
		break;

The commit "Dispatch DONE interrupts by channel request" explicitly claims to
restructure the code so that STOP and PAUSE still have state to update even
when no descriptor remains.

However, the implementation adds if (!vd) break; which completely skips the
state updates when no descriptor remains.

If a future patch asynchronously removes the last descriptor before the DONE
interrupt handles an EDMA_REQ_STOP, won't the channel remain permanently
stuck in EDMA_REQ_STOP without completing termination, leading to an infinite
loop in dw_edma_wait_termination?

> +	dw_edma_core_ch_maybe_doorbell(chan);
> +
>  	spin_unlock_irqrestore(&chan->vc.lock, flags);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7