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

On Mon, Jul 27, 2026 at 05:23:25PM +0000, [email protected] wrote:
> 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?

Is it true?

Frank
>
> > +	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