Re: [PATCH v2 08/19] dmaengine: dw-edma: Centralize LL doorbell decisions

On Thu, Jul 23, 2026 at 12:02:48PM -0500, Frank Li wrote:
> On Thu, Jul 23, 2026 at 05:41:39PM +0900, Koichiro Den wrote:
> > LL fill currently rings the doorbell every time. A running eDMA or HDMA
> > channel follows entries whose CB matches CCS without another kick. A
> > channel stopped at a CB mismatch needs one.
> >
> > Move that decision to dw_edma_core_ch_maybe_doorbell(). It rings only for
> > a BUSY LL channel with pending entries, no EDMA_REQ_STOP or
> > EDMA_REQ_PAUSE, and hardware that is not running. The BUSY check prevents
> > EDMA_REQ_STOP completion from restarting terminated work.
> >
> > issue_pending() still publishes LL entries only from IDLE. A later patch
> > allows live appends. Non-LL channels are unchanged.
> >
> > Signed-off-by: Koichiro Den <[email protected]>
> > ---
> > Changes in v2:
> >   - New patch.
> >
> >  drivers/dma/dw-edma/dw-edma-core.c | 28 ++++++++++++++++++++++++++--
> >  1 file changed, 26 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> > index fa104af4146e..e88bfb417ad4 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-core.c
> > @@ -129,6 +129,11 @@ static bool dw_edma_core_enable_ll_irq(struct dw_edma_desc *desc, u32 i,
> >  	return desc->chan->dw->core->ll_irq(desc, i, free);
> >  }
> >
> > +static bool dw_edma_ll_pending(struct dw_edma_chan *chan)
> > +{
> > +	return chan->ll_head != chan->ll_end;
> > +}
> > +
> >  static void dw_edma_core_ll_start(struct dw_edma_desc *desc)
> >  {
> >  	struct dw_edma_chan *chan = desc->chan;
> > @@ -163,8 +168,6 @@ static void dw_edma_core_ll_start(struct dw_edma_desc *desc)
> >
> >  	desc->done_burst = desc->start_burst;
> >  	desc->start_burst = i;
> > -
> > -	dw_edma_core_ch_doorbell(chan);
> >  }
> >
> >  static void dw_edma_core_start(struct dw_edma_desc *desc)
> > @@ -232,6 +235,23 @@ static void dw_edma_terminate_all_descs(struct dw_edma_chan *chan)
> >  	dw_edma_terminate_vdesc_list(&chan->vc.desc_submitted);
> >  }
> >
> > +/* Must be called with vc.lock held. */
> > +static void dw_edma_core_ch_maybe_doorbell(struct dw_edma_chan *chan)
> 
> suggest this function call dw_edma_core_ch_doorbell()
> 
> old dw_edma_core_ch_doorbell() rename to dw_edma_core_do_ch_doorbell()

TBH I'm not yet sure which is better, but I think either is fine.
Let me think about it again for v3.

Thanks for the review,
Koichiro

> 
> Frank
> 
> > +{
> > +	if (chan->non_ll || chan->request != EDMA_REQ_NONE ||
> > +	    chan->status != EDMA_ST_BUSY || !dw_edma_ll_pending(chan))
> > +		return;
> > +
> > +	/*
> > +	 * While running, both legacy eDMA and HDMA consume newly published
> > +	 * elements without another doorbell.
> > +	 */
> > +	if (dw_edma_core_ch_status(chan) == DMA_IN_PROGRESS)
> > +		return;
> > +
> > +	dw_edma_core_ch_doorbell(chan);
> > +}
> > +
> >  static void dw_edma_device_caps(struct dma_chan *dchan,
> >  				struct dma_slave_caps *caps)
> >  {
> > @@ -357,6 +377,7 @@ static int dw_edma_device_resume(struct dma_chan *dchan)
> >  		chan->status = EDMA_ST_BUSY;
> >  		if (!dw_edma_start_transfer(chan))
> >  			chan->status = EDMA_ST_IDLE;
> > +		dw_edma_core_ch_maybe_doorbell(chan);
> >  	}
> >
> >  	return err;
> > @@ -405,6 +426,7 @@ static void dw_edma_device_issue_pending(struct dma_chan *dchan)
> >  	    chan->status == EDMA_ST_IDLE) {
> >  		chan->status = EDMA_ST_BUSY;
> >  		dw_edma_start_transfer(chan);
> > +		dw_edma_core_ch_maybe_doorbell(chan);
> >  	}
> >  	spin_unlock_irqrestore(&chan->vc.lock, flags);
> >  }
> > @@ -720,6 +742,8 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> >  	default:
> >  		break;
> >  	}
> > +	dw_edma_core_ch_maybe_doorbell(chan);
> > +
> >  	spin_unlock_irqrestore(&chan->vc.lock, flags);
> >  }
> >
> > --
> > 2.51.0
> >