Re: [PATCH v3 09/24] dmaengine: dw-edma: Prepare deferred IRQ reporting for LL events

[Frank Li]

On Tue, Jul 28, 2026 at 02:03:08AM +0900, Koichiro Den wrote:
> A later change serializes status capture, interrupt clear, and event
> recording against channel restart. It must also publish all causes
> observed for a channel in that critical section through one callback.
> Otherwise, the deferred worker can consume one cause before another from
> the same status pass is recorded.
>
> Replace the DONE and ABORT callback pair with one callback carrying an
> event bitmap. For now, map the bitmap back to the existing deferred DONE
> and ABORT work, so behavior remains unchanged.
>
> Also split DONE handling into a lock-taking wrapper and a vc.lock-held
> body. The later LL event path needs to validate its recorded event under
> vc.lock before running the existing DONE state machine.
>
> No functional change.
>
> Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>
> ---

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

> Changes in v3:
>   - New prep patch split from the LL event/restart serialization change
>     to keep its functional diff focused. (Sashiko)
>
>  drivers/dma/dw-edma/dw-edma-core.c    | 44 ++++++++++++++++-----------
>  drivers/dma/dw-edma/dw-edma-core.h    | 16 +++++++---
>  drivers/dma/dw-edma/dw-edma-v0-core.c |  6 ++--
>  drivers/dma/dw-edma/dw-hdma-v0-core.c |  6 ++--
>  4 files changed, 44 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index b84aed59bf77..bb57f2d39d1c 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -702,17 +702,16 @@ dw_edma_device_prep_interleaved_dma(struct dma_chan *dchan,
>  	return dw_edma_device_transfer(&xfer, dw_edma_device_get_config(dchan, NULL));
>  }
>
> -static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> +/* Must be called with vc.lock held. */
> +static void dw_edma_done_interrupt_locked(struct dw_edma_chan *chan)
>  {
>  	struct dw_edma_desc *desc;
>  	struct virt_dma_desc *vd;
> -	unsigned long flags;
>
> -	spin_lock_irqsave(&chan->vc.lock, flags);
> -	if (chan->status == EDMA_ST_PAUSE) {
> -		spin_unlock_irqrestore(&chan->vc.lock, flags);
> +	lockdep_assert_held(&chan->vc.lock);
> +
> +	if (chan->status == EDMA_ST_PAUSE)
>  		return;
> -	}
>
>  	switch (chan->request) {
>  	case EDMA_REQ_NONE:
> @@ -751,7 +750,14 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
>  		break;
>  	}
>  	dw_edma_core_ch_maybe_doorbell(chan);
> +}
>
> +static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&chan->vc.lock, flags);
> +	dw_edma_done_interrupt_locked(chan);
>  	spin_unlock_irqrestore(&chan->vc.lock, flags);
>  }
>
> @@ -793,20 +799,24 @@ static void dw_edma_irq_work(struct work_struct *work)
>  }
>
>  static void dw_edma_queue_irq_work(struct dw_edma_chan *chan,
> -				   enum dw_edma_deferred_event event)
> +				   unsigned int events)
>  {
> -	atomic_or(event, &chan->irq_pending);
> +	atomic_or(events, &chan->irq_pending);
>  	queue_work(chan->dw->wq, &chan->irq_work);
>  }
>
> -static void dw_edma_done_interrupt_deferred(struct dw_edma_chan *chan)
> +static void dw_edma_record_irq(struct dw_edma_chan *chan, unsigned int events)
>  {
> -	dw_edma_queue_irq_work(chan, DW_EDMA_DEFERRED_DONE);
> -}
> +	unsigned int pending = 0;
>
> -static void dw_edma_abort_interrupt_deferred(struct dw_edma_chan *chan)
> -{
> -	dw_edma_queue_irq_work(chan, DW_EDMA_DEFERRED_ABORT);
> +	if (events & (DW_EDMA_IRQ_DONE | DW_EDMA_IRQ_PROGRESS |
> +		      DW_EDMA_IRQ_STOP))
> +		pending |= DW_EDMA_DEFERRED_DONE;
> +	if (events & DW_EDMA_IRQ_ABORT)
> +		pending |= DW_EDMA_DEFERRED_ABORT;
> +
> +	if (pending)
> +		dw_edma_queue_irq_work(chan, pending);
>  }
>
>  static void dw_edma_emul_irq_ack(struct irq_data *d)
> @@ -906,8 +916,7 @@ static inline irqreturn_t dw_edma_interrupt_write_inner(int irq, void *data)
>  	struct dw_edma_irq *dw_irq = data;
>
>  	return dw_edma_core_handle_int(dw_irq, EDMA_DIR_WRITE,
> -				       dw_edma_done_interrupt_deferred,
> -				       dw_edma_abort_interrupt_deferred);
> +				       dw_edma_record_irq);
>  }
>
>  static inline irqreturn_t dw_edma_interrupt_read_inner(int irq, void *data)
> @@ -915,8 +924,7 @@ static inline irqreturn_t dw_edma_interrupt_read_inner(int irq, void *data)
>  	struct dw_edma_irq *dw_irq = data;
>
>  	return dw_edma_core_handle_int(dw_irq, EDMA_DIR_READ,
> -				       dw_edma_done_interrupt_deferred,
> -				       dw_edma_abort_interrupt_deferred);
> +				       dw_edma_record_irq);
>  }
>
>  static inline irqreturn_t dw_edma_interrupt_write(int irq, void *data)
> diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
> index 95bd4bf47ea2..6b0d475bb8b5 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.h
> +++ b/drivers/dma/dw-edma/dw-edma-core.h
> @@ -41,6 +41,13 @@ enum dw_edma_xfer_type {
>  	EDMA_XFER_INTERLEAVED
>  };
>
> +enum dw_edma_irq_event {
> +	DW_EDMA_IRQ_DONE	= BIT(0),
> +	DW_EDMA_IRQ_PROGRESS	= BIT(1),
> +	DW_EDMA_IRQ_STOP	= BIT(2),
> +	DW_EDMA_IRQ_ABORT	= BIT(3),
> +};
> +
>  enum dw_edma_event_scope {
>  	DW_EDMA_EVENT_PER_CHAN,
>  	DW_EDMA_EVENT_PER_DIR,
> @@ -152,7 +159,8 @@ struct dw_edma {
>  	const struct dw_edma_core_ops	*core;
>  };
>
> -typedef void (*dw_edma_handler_t)(struct dw_edma_chan *);
> +typedef void (*dw_edma_handler_t)(struct dw_edma_chan *chan,
> +				  unsigned int events);
>
>  struct dw_edma_core_ops {
>  	void (*off)(struct dw_edma *dw);
> @@ -162,7 +170,7 @@ struct dw_edma_core_ops {
>  	enum dma_status (*ch_status)(struct dw_edma_chan *chan);
>  	enum dw_edma_event_scope event_scope;
>  	irqreturn_t (*handle_int)(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
> -				  dw_edma_handler_t done, dw_edma_handler_t abort);
> +				  dw_edma_handler_t handler);
>  	void (*non_ll_start)(struct dw_edma_chan *chan, struct dw_edma_burst *child);
>  	void (*ll_data)(struct dw_edma_chan *chan, struct dw_edma_burst *burst,
>  			u32 idx, bool cb, bool irq);
> @@ -262,9 +270,9 @@ enum dma_status dw_edma_core_ch_status(struct dw_edma_chan *chan)
>
>  static inline irqreturn_t
>  dw_edma_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
> -			dw_edma_handler_t done, dw_edma_handler_t abort)
> +			dw_edma_handler_t handler)
>  {
> -	return dw_irq->dw->core->handle_int(dw_irq, dir, done, abort);
> +	return dw_irq->dw->core->handle_int(dw_irq, dir, handler);
>  }
>
>  static inline
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> index d497d36f5b28..d883ca446f5a 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> @@ -336,7 +336,7 @@ static void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
>
>  static irqreturn_t
>  dw_edma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
> -			   dw_edma_handler_t done, dw_edma_handler_t abort)
> +			   dw_edma_handler_t handler)
>  {
>  	struct dw_edma *dw = dw_irq->dw;
>  	unsigned long total, pos, val;
> @@ -375,7 +375,7 @@ dw_edma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
>  			continue;
>
>  		dw_edma_v0_core_clear_done_int(chan);
> -		done(chan);
> +		handler(chan, DW_EDMA_IRQ_DONE);
>
>  		ret = IRQ_HANDLED;
>  	}
> @@ -389,7 +389,7 @@ dw_edma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
>  			continue;
>
>  		dw_edma_v0_core_clear_abort_int(chan);
> -		abort(chan);
> +		handler(chan, DW_EDMA_IRQ_ABORT);
>
>  		ret = IRQ_HANDLED;
>  	}
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> index b181603bf2c7..afc2f24fecd3 100644
> --- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> @@ -174,7 +174,7 @@ static u32 dw_hdma_v0_core_status_int(struct dw_edma_chan *chan)
>
>  static irqreturn_t
>  dw_hdma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
> -			   dw_edma_handler_t done, dw_edma_handler_t abort)
> +			   dw_edma_handler_t handler)
>  {
>  	struct dw_edma *dw = dw_irq->dw;
>  	unsigned long total, pos, val;
> @@ -200,14 +200,14 @@ dw_hdma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
>  		val = dw_hdma_v0_core_status_int(chan);
>  		if (FIELD_GET(HDMA_V0_STOP_INT_MASK, val)) {
>  			dw_hdma_v0_core_clear_done_int(chan);
> -			done(chan);
> +			handler(chan, DW_EDMA_IRQ_STOP);
>
>  			ret = IRQ_HANDLED;
>  		}
>
>  		if (FIELD_GET(HDMA_V0_ABORT_INT_MASK, val)) {
>  			dw_hdma_v0_core_clear_abort_int(chan);
> -			abort(chan);
> +			handler(chan, DW_EDMA_IRQ_ABORT);
>
>  			ret = IRQ_HANDLED;
>  		}
> --
> 2.51.0
>