* [PATCH v6 1/5] dmaengine: dw-edma: Add a channel transfer size operation
2026-10-30 15:07 [PATCH v6 0/5] dmaengine: dw-edma: Support dynamic LL appends Koichiro Den
@ 2026-10-30 15:07 ` Koichiro Den
2026-10-30 15:11 ` sashiko-bot
2026-10-30 16:18 ` Frank Li
2026-10-30 15:07 ` [PATCH v6 2/5] dmaengine: dw-edma: Reclaim LL entries from hardware progress Koichiro Den
` (3 subsequent siblings)
4 siblings, 2 replies; 14+ messages in thread
From: Koichiro Den @ 2026-10-30 15:07 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Manivannan Sadhasivam
Cc: Gustavo Pimentel, Serge Semin, Cai Huoqing, Niklas Cassel,
Devendra K Verma, dmaengine, linux-kernel, linux-pci
Expose the channel transfer size through a core operation for eDMA and
HDMA. LL progress handling will use it to check for a stopped channel
with no payload left to transfer.
PCI Express DM Controller Databook 6.10a-lca06, section 7.3.4,
"Detecting the End of the HDMA Transfer", documents this polling method:
STATUS field of the HDMA Write/Read Channel Status register
(HDMA_STATUS_OFF_[WR|RD]CH_i) is Stopped, and the HDMA Transfer Size
register is 0x0.
The same condition applies to eDMA as well.
No functional change on its own.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Split from v5 patch 13:
https://lore.kernel.org/r/20260812155721.2807506-14-den@valinux.co.jp/
v5 used native HDMA's STOP event directly, so it did not need an HDMA
transfer-size callback. v6 uses channel status and transfer size to
identify stopped channels on both backends, so add the HDMA callback too.
drivers/dma/dw-edma/dw-edma-core.h | 6 ++++++
drivers/dma/dw-edma/dw-edma-v0-core.c | 6 ++++++
drivers/dma/dw-edma/dw-hdma-v0-core.c | 6 ++++++
3 files changed, 18 insertions(+)
diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index 76fd5a41a707..84a811bdcb6d 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -163,6 +163,7 @@ struct dw_edma_core_ops {
int (*ch_quiesce)(struct dw_edma_chan *chan);
u16 (*ch_count)(struct dw_edma *dw, enum dw_edma_dir dir);
enum dma_status (*ch_status)(struct dw_edma_chan *chan);
+ u32 (*ch_transfer_size)(struct dw_edma_chan *chan);
irqreturn_t (*handle_int)(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
dw_edma_handler_t handler);
void (*non_ll_start)(struct dw_edma_chan *chan, struct dw_edma_burst *child);
@@ -262,6 +263,11 @@ enum dma_status dw_edma_core_ch_status(struct dw_edma_chan *chan)
return chan->dw->core->ch_status(chan);
}
+static inline u32 dw_edma_core_ch_transfer_size(struct dw_edma_chan *chan)
+{
+ return chan->dw->core->ch_transfer_size(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 handler)
diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
index c053cb4c2844..fa8be85c7ac1 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
@@ -318,6 +318,11 @@ static enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
return DMA_ERROR;
}
+static u32 dw_edma_v0_core_ch_transfer_size(struct dw_edma_chan *chan)
+{
+ return GET_CH_32(chan->dw, chan->dir, chan->id, transfer_size);
+}
+
static void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
{
struct dw_edma *dw = chan->dw;
@@ -665,6 +670,7 @@ static const struct dw_edma_core_ops dw_edma_v0_core = {
.ch_quiesce = dw_edma_v0_core_ch_quiesce,
.ch_count = dw_edma_v0_core_ch_count,
.ch_status = dw_edma_v0_core_ch_status,
+ .ch_transfer_size = dw_edma_v0_core_ch_transfer_size,
.handle_int = dw_edma_v0_core_handle_int,
.ll_data = dw_edma_v0_core_ll_data,
.ll_link = dw_edma_v0_core_ll_link,
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
index d42cb4ad49a2..aa430219d0af 100644
--- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
@@ -156,6 +156,11 @@ static enum dma_status dw_hdma_v0_core_ch_status(struct dw_edma_chan *chan)
return DMA_ERROR;
}
+static u32 dw_hdma_v0_core_ch_transfer_size(struct dw_edma_chan *chan)
+{
+ return GET_CH_32(chan->dw, chan->dir, chan->id, transfer_size);
+}
+
static void dw_hdma_v0_core_clear_done_int(struct dw_edma_chan *chan)
{
struct dw_edma *dw = chan->dw;
@@ -425,6 +430,7 @@ static const struct dw_edma_core_ops dw_hdma_v0_core = {
.ch_quiesce = dw_hdma_v0_core_ch_quiesce,
.ch_count = dw_hdma_v0_core_ch_count,
.ch_status = dw_hdma_v0_core_ch_status,
+ .ch_transfer_size = dw_hdma_v0_core_ch_transfer_size,
.handle_int = dw_hdma_v0_core_handle_int,
.non_ll_start = dw_hdma_v0_core_non_ll_start,
.ll_data = dw_hdma_v0_core_ll_data,
--
2.54.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v6 1/5] dmaengine: dw-edma: Add a channel transfer size operation
2026-10-30 15:07 ` [PATCH v6 1/5] dmaengine: dw-edma: Add a channel transfer size operation Koichiro Den
@ 2026-10-30 15:11 ` sashiko-bot
2026-10-30 16:18 ` Frank Li
1 sibling, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-10-30 15:11 UTC (permalink / raw)
To: Koichiro Den; +Cc: linux-pci
> Expose the channel transfer size through a core operation for eDMA and
> HDMA. LL progress handling will use it to check for a stopped channel
> with no payload left to transfer.
>
> PCI Express DM Controller Databook 6.10a-lca06, section 7.3.4,
> "Detecting the End of the HDMA Transfer", documents this polling method:
>
> STATUS field of the HDMA Write/Read Channel Status register
> (HDMA_STATUS_OFF_[WR|RD]CH_i) is Stopped, and the HDMA Transfer Size
> register is 0x0.
>
> The same condition applies to eDMA as well.
>
> No functional change on its own.
>
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261030150742.2586776-1-den@valinux.co.jp?part=1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6 1/5] dmaengine: dw-edma: Add a channel transfer size operation
2026-10-30 15:07 ` [PATCH v6 1/5] dmaengine: dw-edma: Add a channel transfer size operation Koichiro Den
2026-10-30 15:11 ` sashiko-bot
@ 2026-10-30 16:18 ` Frank Li
1 sibling, 0 replies; 14+ messages in thread
From: Frank Li @ 2026-10-30 16:18 UTC (permalink / raw)
To: Koichiro Den
Cc: Vinod Koul, Frank Li, Manivannan Sadhasivam, Gustavo Pimentel,
Serge Semin, Cai Huoqing, Niklas Cassel, Devendra K Verma,
dmaengine, linux-kernel, linux-pci
On Sat, Oct 31, 2026 at 12:07:38AM +0900, Koichiro Den wrote:
> Expose the channel transfer size through a core operation for eDMA and
> HDMA. LL progress handling will use it to check for a stopped channel
> with no payload left to transfer.
>
> PCI Express DM Controller Databook 6.10a-lca06, section 7.3.4,
> "Detecting the End of the HDMA Transfer", documents this polling method:
>
> STATUS field of the HDMA Write/Read Channel Status register
> (HDMA_STATUS_OFF_[WR|RD]CH_i) is Stopped, and the HDMA Transfer Size
> register is 0x0.
>
> The same condition applies to eDMA as well.
>
> No functional change on its own.
>
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Split from v5 patch 13:
> https://lore.kernel.org/r/20260812155721.2807506-14-den@valinux.co.jp/
>
> v5 used native HDMA's STOP event directly, so it did not need an HDMA
> transfer-size callback. v6 uses channel status and transfer size to
> identify stopped channels on both backends, so add the HDMA callback too.
>
> drivers/dma/dw-edma/dw-edma-core.h | 6 ++++++
> drivers/dma/dw-edma/dw-edma-v0-core.c | 6 ++++++
> drivers/dma/dw-edma/dw-hdma-v0-core.c | 6 ++++++
> 3 files changed, 18 insertions(+)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
> index 76fd5a41a707..84a811bdcb6d 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.h
> +++ b/drivers/dma/dw-edma/dw-edma-core.h
> @@ -163,6 +163,7 @@ struct dw_edma_core_ops {
> int (*ch_quiesce)(struct dw_edma_chan *chan);
> u16 (*ch_count)(struct dw_edma *dw, enum dw_edma_dir dir);
> enum dma_status (*ch_status)(struct dw_edma_chan *chan);
> + u32 (*ch_transfer_size)(struct dw_edma_chan *chan);
> irqreturn_t (*handle_int)(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
> dw_edma_handler_t handler);
> void (*non_ll_start)(struct dw_edma_chan *chan, struct dw_edma_burst *child);
> @@ -262,6 +263,11 @@ enum dma_status dw_edma_core_ch_status(struct dw_edma_chan *chan)
> return chan->dw->core->ch_status(chan);
> }
>
> +static inline u32 dw_edma_core_ch_transfer_size(struct dw_edma_chan *chan)
> +{
> + return chan->dw->core->ch_transfer_size(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 handler)
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> index c053cb4c2844..fa8be85c7ac1 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> @@ -318,6 +318,11 @@ static enum dma_status dw_edma_v0_core_ch_status(struct dw_edma_chan *chan)
> return DMA_ERROR;
> }
>
> +static u32 dw_edma_v0_core_ch_transfer_size(struct dw_edma_chan *chan)
> +{
> + return GET_CH_32(chan->dw, chan->dir, chan->id, transfer_size);
> +}
> +
> static void dw_edma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> {
> struct dw_edma *dw = chan->dw;
> @@ -665,6 +670,7 @@ static const struct dw_edma_core_ops dw_edma_v0_core = {
> .ch_quiesce = dw_edma_v0_core_ch_quiesce,
> .ch_count = dw_edma_v0_core_ch_count,
> .ch_status = dw_edma_v0_core_ch_status,
> + .ch_transfer_size = dw_edma_v0_core_ch_transfer_size,
> .handle_int = dw_edma_v0_core_handle_int,
> .ll_data = dw_edma_v0_core_ll_data,
> .ll_link = dw_edma_v0_core_ll_link,
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> index d42cb4ad49a2..aa430219d0af 100644
> --- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> @@ -156,6 +156,11 @@ static enum dma_status dw_hdma_v0_core_ch_status(struct dw_edma_chan *chan)
> return DMA_ERROR;
> }
>
> +static u32 dw_hdma_v0_core_ch_transfer_size(struct dw_edma_chan *chan)
> +{
> + return GET_CH_32(chan->dw, chan->dir, chan->id, transfer_size);
> +}
> +
> static void dw_hdma_v0_core_clear_done_int(struct dw_edma_chan *chan)
> {
> struct dw_edma *dw = chan->dw;
> @@ -425,6 +430,7 @@ static const struct dw_edma_core_ops dw_hdma_v0_core = {
> .ch_quiesce = dw_hdma_v0_core_ch_quiesce,
> .ch_count = dw_hdma_v0_core_ch_count,
> .ch_status = dw_hdma_v0_core_ch_status,
> + .ch_transfer_size = dw_hdma_v0_core_ch_transfer_size,
> .handle_int = dw_hdma_v0_core_handle_int,
> .non_ll_start = dw_hdma_v0_core_non_ll_start,
> .ll_data = dw_hdma_v0_core_ll_data,
> --
> 2.54.0
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v6 2/5] dmaengine: dw-edma: Reclaim LL entries from hardware progress
2026-10-30 15:07 [PATCH v6 0/5] dmaengine: dw-edma: Support dynamic LL appends Koichiro Den
2026-10-30 15:07 ` [PATCH v6 1/5] dmaengine: dw-edma: Add a channel transfer size operation Koichiro Den
@ 2026-10-30 15:07 ` Koichiro Den
2026-10-30 15:39 ` sashiko-bot
2026-10-30 16:54 ` Frank Li
2026-10-30 15:07 ` [PATCH v6 3/5] dmaengine: dw-edma: Use HDMA watermark interrupts as LL progress Koichiro Den
` (2 subsequent siblings)
4 siblings, 2 replies; 14+ messages in thread
From: Koichiro Den @ 2026-10-30 15:07 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Manivannan Sadhasivam
Cc: Gustavo Pimentel, Serge Semin, Cai Huoqing, Niklas Cassel,
Devendra K Verma, dmaengine, linux-kernel, linux-pci
Once requests are appended while the channel runs, a DONE interrupt no
longer means that all published entries have completed.
Use DMA_LLP to advance ll_done and complete descriptors in issue order.
Reclaim entries as they complete so descriptors larger than the ring
can be submitted in batches.
DMA_LLP may advance before payload completion, so keep the running
completion boundary one entry behind it. Use DMA_LLP itself once the
channel is stopped with a zero transfer size. Hold vc.lock to serialize
progress with ring resets and doorbells.
Refill reclaimed entries and restart a stopped channel if work remains.
Defer STOP and PAUSE completion until the channel stops.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Reworked v5 patch 13:
https://lore.kernel.org/r/20260812155721.2807506-14-den@valinux.co.jp/
Changes since v5:
- Read the registers in the progress handler instead of retaining an
IRQ-paired LLP snapshot.
- Allow index 0 to wrap. Reject running samples that would reclaim all
pending entries, so an unchanged start pointer cannot look like a
full lap.
drivers/dma/dw-edma/dw-edma-core.c | 140 ++++++++++++++++++++++++++---
1 file changed, 126 insertions(+), 14 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index b08bb31657c9..74613c95ae11 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -53,13 +53,6 @@ dw_edma_alloc_desc(struct dw_edma_chan *chan, size_t nburst)
{
struct dw_edma_desc *desc;
- /*
- * For now, a descriptor that does not fit would stall the channel
- * forever: reject it up front.
- */
- if (!chan->non_ll && nburst > chan->ll_max - 1)
- return NULL;
-
desc = kzalloc_flex(*desc, burst, nburst, GFP_NOWAIT);
if (unlikely(!desc))
return NULL;
@@ -146,6 +139,76 @@ static bool dw_edma_ll_pending(struct dw_edma_chan *chan)
return chan->ll_head != chan->ll_done;
}
+static bool dw_edma_ll_stopped(struct dw_edma_chan *chan)
+{
+ return dw_edma_core_ch_status(chan) == DMA_COMPLETE &&
+ !dw_edma_core_ch_transfer_size(chan);
+}
+
+/* Return the first entry we cannot reclaim yet. */
+static int dw_edma_ll_boundary(struct dw_edma_chan *chan, int idx,
+ bool stopped)
+{
+ if (idx < 0 || (u32)idx > chan->ll_max)
+ return -EINVAL;
+
+ /* After a stop, LLP points past the completed entries. */
+ if (stopped)
+ return idx == chan->ll_max ? 0 : idx;
+
+ /*
+ * DMA_LLP can advance before payload completion. Keep the boundary
+ * one entry behind it, wrapping at the start of the ring.
+ */
+ return idx ? idx - 1 : chan->ll_max - 1;
+}
+
+/* Must be called with vc.lock held. */
+static void dw_edma_ll_consume(struct dw_edma_chan *chan, int idx,
+ bool stopped)
+{
+ struct virt_dma_desc *vd, *_vd;
+ u32 done, used;
+
+ idx = dw_edma_ll_boundary(chan, idx, stopped);
+ if (idx < 0)
+ return;
+
+ done = dw_edma_core_get_ll_dist(chan, chan->ll_done, idx);
+ used = dw_edma_core_get_used_num(chan);
+
+ /*
+ * LLP may still point to the first pending entry. Subtracting one
+ * can wrap the index and falsely mark all pending entries complete.
+ * Reclaim them all only after the channel has stopped.
+ */
+ if (!done || done > used || (!stopped && done == used))
+ return;
+
+ chan->ll_done = idx;
+
+ /* Match reclaimed entries to descriptors in issue order. */
+ list_for_each_entry_safe(vd, _vd, &chan->vc.desc_issued, node) {
+ struct dw_edma_desc *desc = vd2dw_edma_desc(vd);
+ u32 consumed = min_t(u32, done,
+ desc->start_burst - desc->done_burst);
+
+ if (!consumed)
+ break;
+
+ desc->done_burst += consumed;
+ done -= consumed;
+ if (desc->done_burst != desc->nburst)
+ break;
+
+ dw_hdma_set_callback_result(vd, DMA_TRANS_NOERROR);
+ list_del(&vd->node);
+ vchan_cookie_complete(vd);
+ }
+
+ WARN_ON_ONCE(done);
+}
+
static void dw_edma_core_ll_start(struct dw_edma_desc *desc)
{
struct dw_edma_chan *chan = desc->chan;
@@ -174,7 +237,6 @@ static void dw_edma_core_ll_start(struct dw_edma_desc *desc)
}
}
- desc->done_burst = desc->start_burst;
desc->start_burst = i;
}
@@ -283,6 +345,45 @@ static void dw_edma_core_ch_maybe_doorbell(struct dw_edma_chan *chan)
dw_edma_core_ch_doorbell(chan);
}
+/*
+ * Must be called with vc.lock held.
+ *
+ * HDMA does not update LLP continuously. Only use it to track running
+ * transfers when handling an interrupt.
+ */
+static void dw_edma_ll_advance(struct dw_edma_chan *chan, bool event)
+{
+ bool stopped = dw_edma_ll_stopped(chan);
+
+ if (stopped || event)
+ dw_edma_ll_consume(chan, dw_edma_core_ll_cur_idx(chan),
+ stopped);
+
+ if (chan->request != EDMA_REQ_NONE) {
+ /* Wait for the channel to stop before completing STOP or PAUSE. */
+ if (!stopped)
+ return;
+
+ if (chan->request == EDMA_REQ_STOP) {
+ dw_edma_finish_termination(chan);
+ } else if (chan->request == EDMA_REQ_PAUSE) {
+ dw_edma_set_request(chan, EDMA_REQ_NONE);
+ chan->status = EDMA_ST_PAUSE;
+ }
+ return;
+ }
+
+ dw_edma_start_transfer(chan);
+ if (!dw_edma_ll_pending(chan)) {
+ chan->status = EDMA_ST_IDLE;
+ return;
+ }
+
+ chan->status = EDMA_ST_BUSY;
+ if (stopped)
+ dw_edma_core_ch_doorbell(chan);
+}
+
static void dw_edma_device_caps(struct dma_chan *dchan,
struct dma_slave_caps *caps)
{
@@ -749,7 +850,7 @@ dw_edma_device_prep_interleaved_dma(struct dma_chan *dchan,
return dw_edma_device_transfer(&xfer, dw_edma_device_get_config(dchan, NULL));
}
-/* Must be called with vc.lock held. */
+/* Non-LL completion; vc.lock must be held. */
static void dw_edma_done_interrupt_locked(struct dw_edma_chan *chan)
{
struct dw_edma_desc *desc;
@@ -772,8 +873,6 @@ static void dw_edma_done_interrupt_locked(struct dw_edma_chan *chan)
dw_hdma_set_callback_result(vd, DMA_TRANS_NOERROR);
list_del(&vd->node);
vchan_cookie_complete(vd);
- if (!chan->non_ll)
- chan->ll_done = chan->ll_head;
}
if (chan->request == EDMA_REQ_PAUSE) {
@@ -796,7 +895,6 @@ static void dw_edma_done_interrupt_locked(struct dw_edma_chan *chan)
default:
break;
}
- dw_edma_core_ch_maybe_doorbell(chan);
}
static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
@@ -808,6 +906,16 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
spin_unlock_irqrestore(&chan->vc.lock, flags);
}
+static void dw_edma_ll_interrupt(struct dw_edma_chan *chan)
+{
+ guard(spinlock_irqsave)(&chan->vc.lock);
+
+ if (chan->status == EDMA_ST_PAUSE)
+ return;
+
+ dw_edma_ll_advance(chan, true);
+}
+
static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
{
struct virt_dma_desc *vd;
@@ -838,8 +946,12 @@ static void dw_edma_irq_work(struct work_struct *work)
do {
events = atomic_xchg(&chan->irq_pending, 0);
- if (events & DW_EDMA_DEFERRED_DONE)
- dw_edma_done_interrupt(chan);
+ if (events & DW_EDMA_DEFERRED_DONE) {
+ if (chan->non_ll)
+ dw_edma_done_interrupt(chan);
+ else
+ dw_edma_ll_interrupt(chan);
+ }
if (events & DW_EDMA_DEFERRED_ABORT)
dw_edma_abort_interrupt(chan);
} while (atomic_read(&chan->irq_pending));
--
2.54.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v6 2/5] dmaengine: dw-edma: Reclaim LL entries from hardware progress
2026-10-30 15:07 ` [PATCH v6 2/5] dmaengine: dw-edma: Reclaim LL entries from hardware progress Koichiro Den
@ 2026-10-30 15:39 ` sashiko-bot
2026-10-30 16:21 ` Frank Li
2026-10-30 16:54 ` Frank Li
1 sibling, 1 reply; 14+ messages in thread
From: sashiko-bot @ 2026-10-30 15:39 UTC (permalink / raw)
To: Koichiro Den; +Cc: dmaengine, linux-pci, vkoul, Frank.Li
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] dmaengine: dw-edma: callbacks scheduled on STOP in dw_edma_ll_advance()
--
commit cb11666c731e633531785427e3b24bbfbd73eb2b
Author: Koichiro Den <den@valinux.co.jp>
dmaengine: dw-edma: Reclaim LL entries from hardware progress
This commit updates the linked-list completion handling to use DMA_LLP to
advance ll_done and reclaim completed ring entries in issue order. It also
defers STOP and PAUSE completion until the channel stops.
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index b08bb31657c9e..74613c95ae112 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
> @@ -283,6 +345,45 @@ static void dw_edma_core_ch_maybe_doorbell(struct dw_edma_chan *chan)
> dw_edma_core_ch_doorbell(chan);
> }
>
> +/*
> + * Must be called with vc.lock held.
> + *
> + * HDMA does not update LLP continuously. Only use it to track running
> + * transfers when handling an interrupt.
> + */
> +static void dw_edma_ll_advance(struct dw_edma_chan *chan, bool event)
> +{
> + bool stopped = dw_edma_ll_stopped(chan);
> +
> + if (stopped || event)
> + dw_edma_ll_consume(chan, dw_edma_core_ll_cur_idx(chan),
> + stopped);
> +
> + if (chan->request != EDMA_REQ_NONE) {
> + /* Wait for the channel to stop before completing STOP or PAUSE. */
> + if (!stopped)
> + return;
> +
> + if (chan->request == EDMA_REQ_STOP) {
> + dw_edma_finish_termination(chan);
[Severity: High]
Does calling dw_edma_ll_consume() in drivers/dma/dw-edma/dw-edma-core.c
before checking chan->request == EDMA_REQ_STOP cause completion callbacks to
be scheduled on a channel that is being terminated?
When dw_edma_device_terminate_all() is called while an LL channel is actively
transferring, it defers termination by setting chan->request = EDMA_REQ_STOP.
When the interrupt fires:
dw_edma_irq_work()
dw_edma_ll_interrupt()
dw_edma_ll_advance(chan, true)
dw_edma_ll_consume()
list_del(&vd->node);
vchan_cookie_complete(vd);
In dw_edma_ll_consume(), any descriptor in chan->vc.desc_issued whose bursts
finished before the channel stopped is removed from chan->vc.desc_issued,
placed on chan->vc.desc_completed, and has chan->vc.task scheduled via
vchan_cookie_complete() to run its completion callback.
When dw_edma_ll_advance() then checks chan->request == EDMA_REQ_STOP and
calls dw_edma_finish_termination(), dw_edma_terminate_all_descs() only cleans
up descriptors still on chan->vc.desc_issued and chan->vc.desc_submitted,
leaving the descriptors already moved to chan->vc.desc_completed to invoke
their callbacks.
By comparison, the non-LL handler dw_edma_done_interrupt_locked() checks
EDMA_REQ_STOP and calls dw_edma_finish_termination() without calling
vchan_cookie_complete(), matching commit 99109a51efd2 ("dmaengine: dw-edma:
Terminate all descriptors without callbacks").
Should dw_edma_ll_advance() check for EDMA_REQ_STOP before calling
dw_edma_ll_consume() so that terminated descriptors do not schedule
completion callbacks?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261030150742.2586776-1-den@valinux.co.jp?part=2
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v6 2/5] dmaengine: dw-edma: Reclaim LL entries from hardware progress
2026-10-30 15:39 ` sashiko-bot
@ 2026-10-30 16:21 ` Frank Li
0 siblings, 0 replies; 14+ messages in thread
From: Frank Li @ 2026-10-30 16:21 UTC (permalink / raw)
To: sashiko-reviews; +Cc: Koichiro Den, dmaengine, linux-pci, vkoul, Frank.Li
On Fri, Oct 30, 2026 at 03:39:11PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] dmaengine: dw-edma: callbacks scheduled on STOP in dw_edma_ll_advance()
> --
Need fix all sashiko report problem or provide explain.
Frank
>
> commit cb11666c731e633531785427e3b24bbfbd73eb2b
> Author: Koichiro Den <den@valinux.co.jp>
>
> dmaengine: dw-edma: Reclaim LL entries from hardware progress
>
> This commit updates the linked-list completion handling to use DMA_LLP to
> advance ll_done and reclaim completed ring entries in issue order. It also
> defers STOP and PAUSE completion until the channel stops.
>
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> > index b08bb31657c9e..74613c95ae112 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-core.c
>
> [ ... ]
>
> > @@ -283,6 +345,45 @@ static void dw_edma_core_ch_maybe_doorbell(struct dw_edma_chan *chan)
> > dw_edma_core_ch_doorbell(chan);
> > }
> >
> > +/*
> > + * Must be called with vc.lock held.
> > + *
> > + * HDMA does not update LLP continuously. Only use it to track running
> > + * transfers when handling an interrupt.
> > + */
> > +static void dw_edma_ll_advance(struct dw_edma_chan *chan, bool event)
> > +{
> > + bool stopped = dw_edma_ll_stopped(chan);
> > +
> > + if (stopped || event)
> > + dw_edma_ll_consume(chan, dw_edma_core_ll_cur_idx(chan),
> > + stopped);
> > +
> > + if (chan->request != EDMA_REQ_NONE) {
> > + /* Wait for the channel to stop before completing STOP or PAUSE. */
> > + if (!stopped)
> > + return;
> > +
> > + if (chan->request == EDMA_REQ_STOP) {
> > + dw_edma_finish_termination(chan);
>
> [Severity: High]
> Does calling dw_edma_ll_consume() in drivers/dma/dw-edma/dw-edma-core.c
> before checking chan->request == EDMA_REQ_STOP cause completion callbacks to
> be scheduled on a channel that is being terminated?
>
> When dw_edma_device_terminate_all() is called while an LL channel is actively
> transferring, it defers termination by setting chan->request = EDMA_REQ_STOP.
> When the interrupt fires:
>
> dw_edma_irq_work()
> dw_edma_ll_interrupt()
> dw_edma_ll_advance(chan, true)
> dw_edma_ll_consume()
> list_del(&vd->node);
> vchan_cookie_complete(vd);
>
> In dw_edma_ll_consume(), any descriptor in chan->vc.desc_issued whose bursts
> finished before the channel stopped is removed from chan->vc.desc_issued,
> placed on chan->vc.desc_completed, and has chan->vc.task scheduled via
> vchan_cookie_complete() to run its completion callback.
>
> When dw_edma_ll_advance() then checks chan->request == EDMA_REQ_STOP and
> calls dw_edma_finish_termination(), dw_edma_terminate_all_descs() only cleans
> up descriptors still on chan->vc.desc_issued and chan->vc.desc_submitted,
> leaving the descriptors already moved to chan->vc.desc_completed to invoke
> their callbacks.
>
> By comparison, the non-LL handler dw_edma_done_interrupt_locked() checks
> EDMA_REQ_STOP and calls dw_edma_finish_termination() without calling
> vchan_cookie_complete(), matching commit 99109a51efd2 ("dmaengine: dw-edma:
> Terminate all descriptors without callbacks").
>
> Should dw_edma_ll_advance() check for EDMA_REQ_STOP before calling
> dw_edma_ll_consume() so that terminated descriptors do not schedule
> completion callbacks?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261030150742.2586776-1-den@valinux.co.jp?part=2
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6 2/5] dmaengine: dw-edma: Reclaim LL entries from hardware progress
2026-10-30 15:07 ` [PATCH v6 2/5] dmaengine: dw-edma: Reclaim LL entries from hardware progress Koichiro Den
2026-10-30 15:39 ` sashiko-bot
@ 2026-10-30 16:54 ` Frank Li
1 sibling, 0 replies; 14+ messages in thread
From: Frank Li @ 2026-10-30 16:54 UTC (permalink / raw)
To: Koichiro Den
Cc: Vinod Koul, Frank Li, Manivannan Sadhasivam, Gustavo Pimentel,
Serge Semin, Cai Huoqing, Niklas Cassel, Devendra K Verma,
dmaengine, linux-kernel, linux-pci
On Sat, Oct 31, 2026 at 12:07:39AM +0900, Koichiro Den wrote:
> Once requests are appended while the channel runs, a DONE interrupt no
> longer means that all published entries have completed.
>
> Use DMA_LLP to advance ll_done and complete descriptors in issue order.
> Reclaim entries as they complete so descriptors larger than the ring
> can be submitted in batches.
>
> DMA_LLP may advance before payload completion, so keep the running
> completion boundary one entry behind it. Use DMA_LLP itself once the
> channel is stopped with a zero transfer size. Hold vc.lock to serialize
> progress with ring resets and doorbells.
>
> Refill reclaimed entries and restart a stopped channel if work remains.
> Defer STOP and PAUSE completion until the channel stops.
>
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---
> Reworked v5 patch 13:
> https://lore.kernel.org/r/20260812155721.2807506-14-den@valinux.co.jp/
>
> Changes since v5:
> - Read the registers in the progress handler instead of retaining an
> IRQ-paired LLP snapshot.
> - Allow index 0 to wrap. Reject running samples that would reclaim all
> pending entries, so an unchanged start pointer cannot look like a
> full lap.
>
> drivers/dma/dw-edma/dw-edma-core.c | 140 ++++++++++++++++++++++++++---
> 1 file changed, 126 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index b08bb31657c9..74613c95ae11 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -53,13 +53,6 @@ dw_edma_alloc_desc(struct dw_edma_chan *chan, size_t nburst)
> {
> struct dw_edma_desc *desc;
>
> - /*
> - * For now, a descriptor that does not fit would stall the channel
> - * forever: reject it up front.
> - */
> - if (!chan->non_ll && nburst > chan->ll_max - 1)
> - return NULL;
> -
> desc = kzalloc_flex(*desc, burst, nburst, GFP_NOWAIT);
> if (unlikely(!desc))
> return NULL;
> @@ -146,6 +139,76 @@ static bool dw_edma_ll_pending(struct dw_edma_chan *chan)
> return chan->ll_head != chan->ll_done;
> }
>
> +static bool dw_edma_ll_stopped(struct dw_edma_chan *chan)
> +{
> + return dw_edma_core_ch_status(chan) == DMA_COMPLETE &&
> + !dw_edma_core_ch_transfer_size(chan);
> +}
> +
> +/* Return the first entry we cannot reclaim yet. */
> +static int dw_edma_ll_boundary(struct dw_edma_chan *chan, int idx,
> + bool stopped)
> +{
> + if (idx < 0 || (u32)idx > chan->ll_max)
> + return -EINVAL;
> +
> + /* After a stop, LLP points past the completed entries. */
> + if (stopped)
> + return idx == chan->ll_max ? 0 : idx;
> +
> + /*
> + * DMA_LLP can advance before payload completion. Keep the boundary
> + * one entry behind it, wrapping at the start of the ring.
> + */
> + return idx ? idx - 1 : chan->ll_max - 1;
use mod
(idx + (chan->ll_max - 1) - 1) % chan->ll_max - 1;
> +}
> +
> +/* Must be called with vc.lock held. */
> +static void dw_edma_ll_consume(struct dw_edma_chan *chan, int idx,
> + bool stopped)
> +{
> + struct virt_dma_desc *vd, *_vd;
> + u32 done, used;
lockdep_assert_held()
> +
> + idx = dw_edma_ll_boundary(chan, idx, stopped);
> + if (idx < 0)
> + return;
> +
> + done = dw_edma_core_get_ll_dist(chan, chan->ll_done, idx);
> + used = dw_edma_core_get_used_num(chan);
> +
> + /*
> + * LLP may still point to the first pending entry. Subtracting one
> + * can wrap the index and falsely mark all pending entries complete.
> + * Reclaim them all only after the channel has stopped.
> + */
Do you means !done for this comments?
> + if (!done || done > used || (!stopped && done == used))
> + return;
> +
> + chan->ll_done = idx;
> +
> + /* Match reclaimed entries to descriptors in issue order. */
> + list_for_each_entry_safe(vd, _vd, &chan->vc.desc_issued, node) {
> + struct dw_edma_desc *desc = vd2dw_edma_desc(vd);
> + u32 consumed = min_t(u32, done,
> + desc->start_burst - desc->done_burst);
> +
> + if (!consumed)
> + break;
> +
> + desc->done_burst += consumed;
> + done -= consumed;
> + if (desc->done_burst != desc->nburst)
> + break;
> +
> + dw_hdma_set_callback_result(vd, DMA_TRANS_NOERROR);
> + list_del(&vd->node);
> + vchan_cookie_complete(vd);
> + }
> +
> + WARN_ON_ONCE(done);
> +}
> +
> static void dw_edma_core_ll_start(struct dw_edma_desc *desc)
> {
> struct dw_edma_chan *chan = desc->chan;
> @@ -174,7 +237,6 @@ static void dw_edma_core_ll_start(struct dw_edma_desc *desc)
> }
> }
>
> - desc->done_burst = desc->start_burst;
> desc->start_burst = i;
> }
>
> @@ -283,6 +345,45 @@ static void dw_edma_core_ch_maybe_doorbell(struct dw_edma_chan *chan)
> dw_edma_core_ch_doorbell(chan);
> }
>
> +/*
> + * Must be called with vc.lock held.
> + *
> + * HDMA does not update LLP continuously. Only use it to track running
> + * transfers when handling an interrupt.
> + */
> +static void dw_edma_ll_advance(struct dw_edma_chan *chan, bool event)
> +{
> + bool stopped = dw_edma_ll_stopped(chan);
> +
> + if (stopped || event)
> + dw_edma_ll_consume(chan, dw_edma_core_ll_cur_idx(chan),
> + stopped);
> +
> + if (chan->request != EDMA_REQ_NONE) {
> + /* Wait for the channel to stop before completing STOP or PAUSE. */
> + if (!stopped)
> + return;
> +
> + if (chan->request == EDMA_REQ_STOP) {
> + dw_edma_finish_termination(chan);
> + } else if (chan->request == EDMA_REQ_PAUSE) {
> + dw_edma_set_request(chan, EDMA_REQ_NONE);
> + chan->status = EDMA_ST_PAUSE;
> + }
> + return;
> + }
> +
> + dw_edma_start_transfer(chan);
> + if (!dw_edma_ll_pending(chan)) {
> + chan->status = EDMA_ST_IDLE;
> + return;
> + }
> +
> + chan->status = EDMA_ST_BUSY;
> + if (stopped)
> + dw_edma_core_ch_doorbell(chan);
> +}
> +
> static void dw_edma_device_caps(struct dma_chan *dchan,
> struct dma_slave_caps *caps)
> {
> @@ -749,7 +850,7 @@ dw_edma_device_prep_interleaved_dma(struct dma_chan *dchan,
> return dw_edma_device_transfer(&xfer, dw_edma_device_get_config(dchan, NULL));
> }
>
> -/* Must be called with vc.lock held. */
> +/* Non-LL completion; vc.lock must be held. */
> static void dw_edma_done_interrupt_locked(struct dw_edma_chan *chan)
> {
> struct dw_edma_desc *desc;
> @@ -772,8 +873,6 @@ static void dw_edma_done_interrupt_locked(struct dw_edma_chan *chan)
> dw_hdma_set_callback_result(vd, DMA_TRANS_NOERROR);
> list_del(&vd->node);
> vchan_cookie_complete(vd);
why not use dw_edma_ll_consume(), let vchan_cookie_complete() at one
place only
Frank
> - if (!chan->non_ll)
> - chan->ll_done = chan->ll_head;
> }
>
> if (chan->request == EDMA_REQ_PAUSE) {
> @@ -796,7 +895,6 @@ static void dw_edma_done_interrupt_locked(struct dw_edma_chan *chan)
> default:
> break;
> }
> - dw_edma_core_ch_maybe_doorbell(chan);
> }
>
> static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> @@ -808,6 +906,16 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> spin_unlock_irqrestore(&chan->vc.lock, flags);
> }
>
> +static void dw_edma_ll_interrupt(struct dw_edma_chan *chan)
> +{
> + guard(spinlock_irqsave)(&chan->vc.lock);
> +
> + if (chan->status == EDMA_ST_PAUSE)
> + return;
> +
> + dw_edma_ll_advance(chan, true);
> +}
> +
> static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
> {
> struct virt_dma_desc *vd;
> @@ -838,8 +946,12 @@ static void dw_edma_irq_work(struct work_struct *work)
> do {
> events = atomic_xchg(&chan->irq_pending, 0);
>
> - if (events & DW_EDMA_DEFERRED_DONE)
> - dw_edma_done_interrupt(chan);
> + if (events & DW_EDMA_DEFERRED_DONE) {
> + if (chan->non_ll)
> + dw_edma_done_interrupt(chan);
> + else
> + dw_edma_ll_interrupt(chan);
> + }
> if (events & DW_EDMA_DEFERRED_ABORT)
> dw_edma_abort_interrupt(chan);
> } while (atomic_read(&chan->irq_pending));
> --
> 2.54.0
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v6 3/5] dmaengine: dw-edma: Use HDMA watermark interrupts as LL progress
2026-10-30 15:07 [PATCH v6 0/5] dmaengine: dw-edma: Support dynamic LL appends Koichiro Den
2026-10-30 15:07 ` [PATCH v6 1/5] dmaengine: dw-edma: Add a channel transfer size operation Koichiro Den
2026-10-30 15:07 ` [PATCH v6 2/5] dmaengine: dw-edma: Reclaim LL entries from hardware progress Koichiro Den
@ 2026-10-30 15:07 ` Koichiro Den
2026-10-30 15:21 ` sashiko-bot
2026-10-30 15:07 ` [PATCH v6 4/5] dmaengine: dw-edma: Dynamically append requests while running Koichiro Den
2026-10-30 15:07 ` [PATCH v6 5/5] dmaengine: dw-edma: Add trace support Koichiro Den
4 siblings, 1 reply; 14+ messages in thread
From: Koichiro Den @ 2026-10-30 15:07 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Manivannan Sadhasivam
Cc: Gustavo Pimentel, Serge Semin, Cai Huoqing, Niklas Cassel,
Devendra K Verma, dmaengine, linux-kernel, linux-pci
While HDMA is running, DMA_LLP is updated only at watermark events.
Enable watermark interrupts to track progress without waiting for the
channel to stop.
Set the watermark bits on entries that request an interrupt, configure
local or remote delivery, and pass events to the LL progress handler.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Reworked v5 patch 16:
https://lore.kernel.org/r/20260812155721.2807506-17-den@valinux.co.jp/
Use the common progress handler and omit the unused ll_irq_clear
callback change from v5.
drivers/dma/dw-edma/dw-hdma-v0-core.c | 65 ++++++++++++++++++++++-----
drivers/dma/dw-edma/dw-hdma-v0-regs.h | 1 +
2 files changed, 54 insertions(+), 12 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
index aa430219d0af..7ca5afa181fe 100644
--- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
@@ -58,7 +58,8 @@ static u32 dw_hdma_v0_core_int_setup(struct dw_edma_chan *chan, u32 val)
{
val &= ~(HDMA_V0_LOCAL_ABORT_INT_EN | HDMA_V0_REMOTE_ABORT_INT_EN |
HDMA_V0_LOCAL_STOP_INT_EN | HDMA_V0_REMOTE_STOP_INT_EN |
- HDMA_V0_ABORT_INT_MASK | HDMA_V0_STOP_INT_MASK);
+ HDMA_V0_ABORT_INT_MASK | HDMA_V0_WATERMARK_INT_MASK |
+ HDMA_V0_STOP_INT_MASK);
/*
* HDMA_INT_STATUS.STOP and .ABORT are latched only when LSIE and
@@ -68,10 +69,14 @@ static u32 dw_hdma_v0_core_int_setup(struct dw_edma_chan *chan, u32 val)
*/
val |= HDMA_V0_LOCAL_ABORT_INT_EN | HDMA_V0_LOCAL_STOP_INT_EN;
- if (chan->irq_mode == DW_EDMA_CH_IRQ_REMOTE)
+ if (chan->irq_mode == DW_EDMA_CH_IRQ_REMOTE) {
+ if (!chan->non_ll)
+ val |= HDMA_V0_WATERMARK_INT_MASK;
+
val |= HDMA_V0_REMOTE_ABORT_INT_EN |
HDMA_V0_REMOTE_STOP_INT_EN |
HDMA_V0_ABORT_INT_MASK | HDMA_V0_STOP_INT_MASK;
+ }
return val;
}
@@ -81,10 +86,12 @@ static void dw_hdma_v0_core_ch_off(struct dw_edma *dw, enum dw_edma_dir dir,
u16 id)
{
SET_CH_32(dw, dir, id, int_setup,
- HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
+ HDMA_V0_STOP_INT_MASK | HDMA_V0_WATERMARK_INT_MASK |
+ HDMA_V0_ABORT_INT_MASK);
SET_CH_32(dw, dir, id, ch_en, 0);
SET_CH_32(dw, dir, id, int_clear,
- HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
+ HDMA_V0_STOP_INT_MASK | HDMA_V0_WATERMARK_INT_MASK |
+ HDMA_V0_ABORT_INT_MASK);
}
static void dw_hdma_v0_core_off(struct dw_edma *dw)
@@ -95,18 +102,26 @@ static void dw_hdma_v0_core_off(struct dw_edma *dw)
dir = EDMA_DIR_WRITE;
for (id = 0; id < dw->wr_ch_cnt; id++) {
SET_CH_32(dw, dir, id, int_setup,
- HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
+ HDMA_V0_STOP_INT_MASK |
+ HDMA_V0_WATERMARK_INT_MASK |
+ HDMA_V0_ABORT_INT_MASK);
SET_CH_32(dw, dir, id, int_clear,
- HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
+ HDMA_V0_STOP_INT_MASK |
+ HDMA_V0_WATERMARK_INT_MASK |
+ HDMA_V0_ABORT_INT_MASK);
SET_CH_32(dw, dir, id, ch_en, 0);
}
dir = EDMA_DIR_READ;
for (id = 0; id < dw->rd_ch_cnt; id++) {
SET_CH_32(dw, dir, id, int_setup,
- HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
+ HDMA_V0_STOP_INT_MASK |
+ HDMA_V0_WATERMARK_INT_MASK |
+ HDMA_V0_ABORT_INT_MASK);
SET_CH_32(dw, dir, id, int_clear,
- HDMA_V0_STOP_INT_MASK | HDMA_V0_ABORT_INT_MASK);
+ HDMA_V0_STOP_INT_MASK |
+ HDMA_V0_WATERMARK_INT_MASK |
+ HDMA_V0_ABORT_INT_MASK);
SET_CH_32(dw, dir, id, ch_en, 0);
}
}
@@ -168,6 +183,14 @@ static void dw_hdma_v0_core_clear_done_int(struct dw_edma_chan *chan)
SET_CH_32(dw, chan->dir, chan->id, int_clear, HDMA_V0_STOP_INT_MASK);
}
+static void dw_hdma_v0_core_clear_watermark_int(struct dw_edma_chan *chan)
+{
+ struct dw_edma *dw = chan->dw;
+
+ SET_CH_32(dw, chan->dir, chan->id, int_clear,
+ HDMA_V0_WATERMARK_INT_MASK);
+}
+
static void dw_hdma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
{
struct dw_edma *dw = chan->dw;
@@ -208,17 +231,23 @@ dw_hdma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
continue;
val = dw_hdma_v0_core_status_int(chan);
- if (FIELD_GET(HDMA_V0_STOP_INT_MASK, val)) {
+
+ if (FIELD_GET(HDMA_V0_STOP_INT_MASK, val))
dw_hdma_v0_core_clear_done_int(chan);
- handler(chan, DW_EDMA_IRQ_STOP);
+ if (FIELD_GET(HDMA_V0_WATERMARK_INT_MASK, val))
+ dw_hdma_v0_core_clear_watermark_int(chan);
+ if (FIELD_GET(HDMA_V0_STOP_INT_MASK, val)) {
+ handler(chan, DW_EDMA_IRQ_STOP);
+ ret = IRQ_HANDLED;
+ } else if (FIELD_GET(HDMA_V0_WATERMARK_INT_MASK, val)) {
+ handler(chan, DW_EDMA_IRQ_PROGRESS);
ret = IRQ_HANDLED;
}
if (FIELD_GET(HDMA_V0_ABORT_INT_MASK, val)) {
dw_hdma_v0_core_clear_abort_int(chan);
handler(chan, DW_EDMA_IRQ_ABORT);
-
ret = IRQ_HANDLED;
}
}
@@ -275,7 +304,7 @@ static void dw_hdma_v0_core_ch_enable(struct dw_edma_chan *chan)
/* Enable engine */
SET_CH_32(dw, chan->dir, chan->id, ch_en, BIT(0));
- /* Interrupt unmask - stop, abort */
+ /* Interrupt unmask - stop, watermark, abort */
tmp = GET_CH_32(dw, chan->dir, chan->id, int_setup);
tmp = dw_hdma_v0_core_int_setup(chan, tmp);
SET_CH_32(dw, chan->dir, chan->id, int_setup, tmp);
@@ -336,6 +365,11 @@ static void dw_hdma_v0_core_ch_config(struct dw_edma_chan *chan)
/* MSI done addr - low, high */
SET_CH_32(dw, chan->dir, chan->id, msi_stop.lsb, chan->msi.address_lo);
SET_CH_32(dw, chan->dir, chan->id, msi_stop.msb, chan->msi.address_hi);
+ /* MSI watermark addr - low, high */
+ SET_CH_32(dw, chan->dir, chan->id, msi_watermark.lsb,
+ chan->msi.address_lo);
+ SET_CH_32(dw, chan->dir, chan->id, msi_watermark.msb,
+ chan->msi.address_hi);
/* MSI abort addr - low, high */
SET_CH_32(dw, chan->dir, chan->id, msi_abort.lsb, chan->msi.address_lo);
SET_CH_32(dw, chan->dir, chan->id, msi_abort.msb, chan->msi.address_hi);
@@ -355,6 +389,13 @@ dw_hdma_v0_core_ll_data(struct dw_edma_chan *chan, struct dw_edma_burst *burst,
if (cb)
control |= DW_HDMA_V0_CB;
+ if (irq) {
+ control |= DW_HDMA_V0_LWIE;
+
+ if (chan->irq_mode == DW_EDMA_CH_IRQ_REMOTE)
+ control |= DW_HDMA_V0_RWIE;
+ }
+
dw_hdma_v0_write_ll_data(chan, idx, control, burst->sz, burst->sar,
burst->dar);
}
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-regs.h b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
index b88226cdd791..e240ffb3dea7 100644
--- a/drivers/dma/dw-edma/dw-hdma-v0-regs.h
+++ b/drivers/dma/dw-edma/dw-hdma-v0-regs.h
@@ -18,6 +18,7 @@
#define HDMA_V0_LOCAL_STOP_INT_EN BIT(4)
#define HDMA_V0_REMOTE_STOP_INT_EN BIT(3)
#define HDMA_V0_ABORT_INT_MASK BIT(2)
+#define HDMA_V0_WATERMARK_INT_MASK BIT(1)
#define HDMA_V0_STOP_INT_MASK BIT(0)
#define HDMA_V0_LINKLIST_EN BIT(0)
#define HDMA_V0_CONSUMER_CYCLE_STAT BIT(1)
--
2.54.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v6 3/5] dmaengine: dw-edma: Use HDMA watermark interrupts as LL progress
2026-10-30 15:07 ` [PATCH v6 3/5] dmaengine: dw-edma: Use HDMA watermark interrupts as LL progress Koichiro Den
@ 2026-10-30 15:21 ` sashiko-bot
0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-10-30 15:21 UTC (permalink / raw)
To: Koichiro Den; +Cc: dmaengine, linux-pci, vkoul, Frank.Li
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dmaengine: dw-edma: non-LL watermark unmasked in dw_hdma_v0_core_int_setup()
--
commit 5e1e3ed55b7d4e9a83bc0d3c9ec3bbccd4a0395c
Author: Koichiro Den <den@valinux.co.jp>
dmaengine: dw-edma: Use HDMA watermark interrupts as LL progress
This commit enables HDMA watermark interrupts on linked-list entries that
request an interrupt, configures local or remote interrupt delivery, and
routes watermark events to the linked-list progress handler.
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> index aa430219d0af2..7ca5afa181fe5 100644
> --- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> @@ -58,7 +58,8 @@ static u32 dw_hdma_v0_core_int_setup(struct dw_edma_chan *chan, u32 val)
> {
> val &= ~(HDMA_V0_LOCAL_ABORT_INT_EN | HDMA_V0_REMOTE_ABORT_INT_EN |
> HDMA_V0_LOCAL_STOP_INT_EN | HDMA_V0_REMOTE_STOP_INT_EN |
> - HDMA_V0_ABORT_INT_MASK | HDMA_V0_STOP_INT_MASK);
> + HDMA_V0_ABORT_INT_MASK | HDMA_V0_WATERMARK_INT_MASK |
> + HDMA_V0_STOP_INT_MASK);
>
> /*
> * HDMA_INT_STATUS.STOP and .ABORT are latched only when LSIE and
> @@ -68,10 +69,14 @@ static u32 dw_hdma_v0_core_int_setup(struct dw_edma_chan *chan, u32 val)
> */
> val |= HDMA_V0_LOCAL_ABORT_INT_EN | HDMA_V0_LOCAL_STOP_INT_EN;
>
> - if (chan->irq_mode == DW_EDMA_CH_IRQ_REMOTE)
> + if (chan->irq_mode == DW_EDMA_CH_IRQ_REMOTE) {
> + if (!chan->non_ll)
> + val |= HDMA_V0_WATERMARK_INT_MASK;
> +
> val |= HDMA_V0_REMOTE_ABORT_INT_EN |
> HDMA_V0_REMOTE_STOP_INT_EN |
> HDMA_V0_ABORT_INT_MASK | HDMA_V0_STOP_INT_MASK;
> + }
>
> return val;
> }
[Severity: Medium]
In dw_hdma_v0_core_int_setup(), HDMA_V0_WATERMARK_INT_MASK is unconditionally
cleared at the start of the function, unmasking the local watermark interrupt
pin. Later, it is only set back to 1 (masked) when chan->irq_mode is
DW_EDMA_CH_IRQ_REMOTE and !chan->non_ll.
When a channel is started in non-LL mode via:
dw_edma_device_issue_pending()
-> dw_edma_start_transfer()
-> dw_edma_core_start()
-> dw_hdma_v0_core_non_ll_start()
-> dw_hdma_v0_core_int_setup()
chan->non_ll is true, which leaves HDMA_V0_WATERMARK_INT_MASK cleared (0,
unmasked) in both DW_EDMA_CH_IRQ_LOCAL and DW_EDMA_CH_IRQ_REMOTE modes.
In remote IRQ mode, all local interrupt pins are expected to be masked. In
addition, if a watermark interrupt fires on a non-LL channel (for instance, if
the hardware watermark_en register is non-zero), dw_hdma_v0_core_handle_int()
will report DW_EDMA_IRQ_PROGRESS, causing dw_edma_done_interrupt() to
prematurely complete the active non-LL burst while the channel is still
running.
Should HDMA_V0_WATERMARK_INT_MASK be set whenever chan->non_ll is true or
chan->irq_mode == DW_EDMA_CH_IRQ_REMOTE, so that local watermark interrupts
are only unmasked for LL channels in local IRQ mode?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261030150742.2586776-1-den@valinux.co.jp?part=3
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v6 4/5] dmaengine: dw-edma: Dynamically append requests while running
2026-10-30 15:07 [PATCH v6 0/5] dmaengine: dw-edma: Support dynamic LL appends Koichiro Den
` (2 preceding siblings ...)
2026-10-30 15:07 ` [PATCH v6 3/5] dmaengine: dw-edma: Use HDMA watermark interrupts as LL progress Koichiro Den
@ 2026-10-30 15:07 ` Koichiro Den
2026-10-30 15:45 ` sashiko-bot
2026-10-30 15:07 ` [PATCH v6 5/5] dmaengine: dw-edma: Add trace support Koichiro Den
4 siblings, 1 reply; 14+ messages in thread
From: Koichiro Den @ 2026-10-30 15:07 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Manivannan Sadhasivam
Cc: Gustavo Pimentel, Serge Semin, Cai Huoqing, Niklas Cassel,
Devendra K Verma, dmaengine, linux-kernel, linux-pci
From: Frank Li <Frank.Li@nxp.com>
Allow issue_pending() to fill free LL entries while the channel is busy,
then check progress and restart it if it has stopped. Publish requests
in descriptor order.
Multiple descriptors can now have entries in the LL ring. When resetting
the ring after an abort, mark the other descriptors with pending entries
as aborted too, so they cannot be replayed.
On resume, keep an LL channel BUSY while entries remain published,
even if dw_edma_start_transfer() publishes nothing new.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Co-developed-by: Koichiro Den <den@valinux.co.jp>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Reworked v5 patches 18 and 20:
https://lore.kernel.org/r/20260812155721.2807506-19-den@valinux.co.jp/
https://lore.kernel.org/r/20260812155721.2807506-21-den@valinux.co.jp/
drivers/dma/dw-edma/dw-edma-core.c | 67 +++++++++++++++++++++++-------
1 file changed, 51 insertions(+), 16 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 74613c95ae11..3e23d177a579 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -102,8 +102,24 @@ static void dw_hdma_set_callback_result(struct virt_dma_desc *vd,
static void dw_edma_core_reset_ll(struct dw_edma_chan *chan)
{
+ struct virt_dma_desc *vd, *tmp;
u32 i;
+ /*
+ * We cannot tell which pending entries completed before the reset.
+ * Abort their descriptors to avoid repeating a transfer.
+ */
+ list_for_each_entry_safe(vd, tmp, &chan->vc.desc_issued, node) {
+ struct dw_edma_desc *desc = vd2dw_edma_desc(vd);
+
+ if (desc->start_burst == desc->done_burst)
+ continue;
+
+ dw_hdma_set_callback_result(vd, DMA_TRANS_ABORTED);
+ list_del(&vd->node);
+ vchan_cookie_complete(vd);
+ }
+
chan->ll_head = 0;
chan->ll_done = 0;
/* Drop stale CB bits before reusing the circular LL ring. */
@@ -259,21 +275,34 @@ static int dw_edma_start_transfer(struct dw_edma_chan *chan)
{
struct dw_edma_desc *desc;
struct virt_dma_desc *vd;
+ int ret = 0;
- vd = vchan_next_desc(&chan->vc);
- if (!vd)
- return 0;
+ if (chan->non_ll) {
+ vd = vchan_next_desc(&chan->vc);
+ if (!vd)
+ return 0;
- desc = vd2dw_edma_desc(vd);
- if (!desc)
- return 0;
+ dw_edma_core_start(vd2dw_edma_desc(vd));
+ return 1;
+ }
- if (!chan->non_ll && !chan->ll_valid)
- dw_edma_core_reset_ll(chan);
+ list_for_each_entry(vd, &chan->vc.desc_issued, node) {
+ if (!dw_edma_core_get_free_num(chan))
+ break;
- dw_edma_core_start(desc);
+ desc = vd2dw_edma_desc(vd);
+ /* Fully published descriptors may still be in flight. */
+ if (desc->start_burst == desc->nburst)
+ continue;
- return 1;
+ if (!chan->ll_valid)
+ dw_edma_core_reset_ll(chan);
+
+ dw_edma_core_start(desc);
+ ret = 1;
+ }
+
+ return ret;
}
static void dw_edma_terminate_vdesc(struct virt_dma_desc *vd)
@@ -535,7 +564,8 @@ static int dw_edma_device_resume(struct dma_chan *dchan)
err = -EPERM;
} else {
chan->status = EDMA_ST_BUSY;
- if (!dw_edma_start_transfer(chan))
+ if (!dw_edma_start_transfer(chan) &&
+ (chan->non_ll || !dw_edma_ll_pending(chan)))
chan->status = EDMA_ST_IDLE;
dw_edma_core_ch_maybe_doorbell(chan);
}
@@ -580,11 +610,16 @@ static void dw_edma_device_issue_pending(struct dma_chan *dchan)
spin_lock_irqsave(&chan->vc.lock, flags);
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);
- dw_edma_core_ch_maybe_doorbell(chan);
+ chan->request == EDMA_REQ_NONE) {
+ if (chan->status == EDMA_ST_IDLE) {
+ chan->status = EDMA_ST_BUSY;
+ dw_edma_start_transfer(chan);
+ dw_edma_core_ch_maybe_doorbell(chan);
+ } else if (!chan->non_ll &&
+ chan->status == EDMA_ST_BUSY) {
+ dw_edma_start_transfer(chan);
+ dw_edma_ll_advance(chan, false);
+ }
}
spin_unlock_irqrestore(&chan->vc.lock, flags);
}
--
2.54.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v6 5/5] dmaengine: dw-edma: Add trace support
2026-10-30 15:07 [PATCH v6 0/5] dmaengine: dw-edma: Support dynamic LL appends Koichiro Den
` (3 preceding siblings ...)
2026-10-30 15:07 ` [PATCH v6 4/5] dmaengine: dw-edma: Dynamically append requests while running Koichiro Den
@ 2026-10-30 15:07 ` Koichiro Den
2026-10-30 15:17 ` sashiko-bot
4 siblings, 1 reply; 14+ messages in thread
From: Koichiro Den @ 2026-10-30 15:07 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Manivannan Sadhasivam
Cc: Gustavo Pimentel, Serge Semin, Cai Huoqing, Niklas Cassel,
Devendra K Verma, dmaengine, linux-kernel, linux-pci
From: Frank Li <Frank.Li@nxp.com>
Add tracepoints for LL publication, descriptor append and completion,
and doorbells. Record channel and ring indices to help diagnose stalls.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Co-developed-by: Koichiro Den <den@valinux.co.jp>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Reworked v5 patch 24:
https://lore.kernel.org/r/20260812155721.2807506-25-den@valinux.co.jp/
drivers/dma/dw-edma/Makefile | 2 +
drivers/dma/dw-edma/dw-edma-core.c | 9 +++
drivers/dma/dw-edma/dw-edma-core.h | 2 +
drivers/dma/dw-edma/dw-edma-trace.c | 4 +
drivers/dma/dw-edma/dw-edma-trace.h | 117 ++++++++++++++++++++++++++++
5 files changed, 134 insertions(+)
create mode 100644 drivers/dma/dw-edma/dw-edma-trace.c
create mode 100644 drivers/dma/dw-edma/dw-edma-trace.h
diff --git a/drivers/dma/dw-edma/Makefile b/drivers/dma/dw-edma/Makefile
index 83ab58f87760..5f395fa1ede4 100644
--- a/drivers/dma/dw-edma/Makefile
+++ b/drivers/dma/dw-edma/Makefile
@@ -1,8 +1,10 @@
# SPDX-License-Identifier: GPL-2.0
+CFLAGS_dw-edma-trace.o := -I$(src)
obj-$(CONFIG_DW_EDMA) += dw-edma.o
dw-edma-$(CONFIG_DEBUG_FS) := dw-edma-v0-debugfs.o \
dw-hdma-v0-debugfs.o
+dw-edma-$(CONFIG_TRACING) += dw-edma-trace.o
dw-edma-objs := dw-edma-core.o \
dw-edma-v0-core.o \
dw-hdma-v0-core.o $(dw-edma-y)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 3e23d177a579..be8816528b2e 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -219,6 +219,7 @@ static void dw_edma_ll_consume(struct dw_edma_chan *chan, int idx,
dw_hdma_set_callback_result(vd, DMA_TRANS_NOERROR);
list_del(&vd->node);
+ trace_edma_complete_desc(desc);
vchan_cookie_complete(vd);
}
@@ -245,6 +246,12 @@ static void dw_edma_core_ll_start(struct dw_edma_desc *desc)
chan->ll_head, chan->cb,
i == desc->nburst - 1 || free == 1);
+ trace_edma_fill_ll(chan, chan->ll_head,
+ desc->vd.tx.cookie,
+ desc->burst[i].sar,
+ desc->burst[i].dar, desc->burst[i].sz,
+ chan->cb);
+
chan->ll_head++;
if (chan->ll_head == chan->ll_max) {
@@ -298,6 +305,7 @@ static int dw_edma_start_transfer(struct dw_edma_chan *chan)
if (!chan->ll_valid)
dw_edma_core_reset_ll(chan);
+ trace_edma_append_desc(desc);
dw_edma_core_start(desc);
ret = 1;
}
@@ -361,6 +369,7 @@ static void dw_edma_core_ll_sync(struct dw_edma_chan *chan)
static void dw_edma_core_ch_doorbell(struct dw_edma_chan *chan)
{
dw_edma_core_ll_sync(chan);
+ trace_edma_doorbell(chan);
dw_edma_core_do_ch_doorbell(chan);
}
diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index 84a811bdcb6d..769c0c08a711 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -154,6 +154,8 @@ struct dw_edma {
const struct dw_edma_core_ops *core;
};
+#include "dw-edma-trace.h"
+
typedef void (*dw_edma_handler_t)(struct dw_edma_chan *chan,
unsigned int events);
diff --git a/drivers/dma/dw-edma/dw-edma-trace.c b/drivers/dma/dw-edma/dw-edma-trace.c
new file mode 100644
index 000000000000..2620ad61a943
--- /dev/null
+++ b/drivers/dma/dw-edma/dw-edma-trace.c
@@ -0,0 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#define CREATE_TRACE_POINTS
+#include "dw-edma-core.h"
diff --git a/drivers/dma/dw-edma/dw-edma-trace.h b/drivers/dma/dw-edma/dw-edma-trace.h
new file mode 100644
index 000000000000..653b36acff15
--- /dev/null
+++ b/drivers/dma/dw-edma/dw-edma-trace.h
@@ -0,0 +1,117 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright 2023 NXP.
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM dw_edma
+
+#if !defined(__LINUX_DW_EDMA_TRACE) || defined(TRACE_HEADER_MULTI_READ)
+#define __LINUX_DW_EDMA_TRACE
+
+#include <linux/types.h>
+#include <linux/tracepoint.h>
+
+DECLARE_EVENT_CLASS(edma_desc_info,
+ TP_PROTO(struct dw_edma_desc *desc),
+ TP_ARGS(desc),
+ TP_STRUCT__entry(
+ __field(size_t, nburst)
+ __field(size_t, start_burst)
+ __field(dma_cookie_t, cookie)
+ __field(u32, id)
+ __field(u8, dir)
+ ),
+ TP_fast_assign(
+ __entry->nburst = desc->nburst;
+ __entry->start_burst = desc->start_burst;
+ __entry->id = desc->chan->id;
+ __entry->dir = desc->chan->dir;
+ __entry->cookie = desc->vd.tx.cookie;
+ ),
+ TP_printk("chan %u%c desc %d nburst %zu start_burst %zu",
+ __entry->id,
+ __entry->dir ? 'R' : 'W',
+ __entry->cookie,
+ __entry->nburst,
+ __entry->start_burst)
+);
+
+DEFINE_EVENT(edma_desc_info, edma_append_desc,
+ TP_PROTO(struct dw_edma_desc *desc),
+ TP_ARGS(desc)
+);
+
+DEFINE_EVENT(edma_desc_info, edma_complete_desc,
+ TP_PROTO(struct dw_edma_desc *desc),
+ TP_ARGS(desc)
+);
+
+TRACE_EVENT(edma_doorbell,
+ TP_PROTO(struct dw_edma_chan *chan),
+ TP_ARGS(chan),
+ TP_STRUCT__entry(
+ __field(u32, id)
+ __field(u8, dir)
+ __field(u32, done)
+ __field(u32, head)
+ ),
+ TP_fast_assign(
+ __entry->id = chan->id;
+ __entry->dir = chan->dir;
+ __entry->done = chan->ll_done;
+ __entry->head = chan->ll_head;
+ ),
+ TP_printk("chan %u%c done %u head %u",
+ __entry->id,
+ __entry->dir ? 'R' : 'W',
+ __entry->done,
+ __entry->head)
+);
+
+TRACE_EVENT(edma_fill_ll,
+ TP_PROTO(struct dw_edma_chan *chan, u32 idx, dma_cookie_t cookie, u64 src,
+ u64 dest, u32 sz, bool flag),
+ TP_ARGS(chan, idx, cookie, src, dest, sz, flag),
+ TP_STRUCT__entry(
+ __field(u32, idx)
+ __field(u64, src)
+ __field(u64, dest)
+ __field(u32, sz)
+ __field(u32, id)
+ __field(dma_cookie_t, cookie)
+ __field(bool, flag)
+ __field(u8, dir)
+ ),
+ TP_fast_assign(
+ __entry->idx = idx;
+ __entry->src = src;
+ __entry->dest = dest;
+ __entry->sz = sz;
+ __entry->id = chan->id;
+ __entry->dir = chan->dir;
+ __entry->cookie = cookie;
+ __entry->flag = flag;
+ ),
+ TP_printk("chan %u%c %d [%u] %c src: %08llx dest: %08llx sz: %04x",
+ __entry->id,
+ __entry->dir ? 'R' : 'W',
+ __entry->cookie,
+ __entry->idx,
+ __entry->flag ? 'C' : 'c',
+ __entry->src,
+ __entry->dest,
+ __entry->sz)
+);
+
+#endif
+
+/* This part must be outside the header guard. */
+
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH .
+
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_FILE dw-edma-trace
+
+#include <trace/define_trace.h>
--
2.54.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v6 5/5] dmaengine: dw-edma: Add trace support
2026-10-30 15:07 ` [PATCH v6 5/5] dmaengine: dw-edma: Add trace support Koichiro Den
@ 2026-10-30 15:17 ` sashiko-bot
0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-10-30 15:17 UTC (permalink / raw)
To: Koichiro Den; +Cc: dmaengine, linux-pci, vkoul, Frank.Li
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] dmaengine: dw-edma: tracepoints use edma_ prefix instead of dw_edma_
--
commit 8d4bbe2d0d3de12559142ae2213cc91782526216
Author: Frank Li <Frank.Li@nxp.com>
dmaengine: dw-edma: Add trace support
This commit adds tracepoints for linked-list publication, descriptor append
and completion, and doorbells in the dw-edma driver to help diagnose stalls.
> diff --git a/drivers/dma/dw-edma/dw-edma-trace.h b/drivers/dma/dw-edma/dw-edma-trace.h
> new file mode 100644
> index 0000000000000..653b36acff158
> --- /dev/null
> +++ b/drivers/dma/dw-edma/dw-edma-trace.h
> @@ -0,0 +1,117 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright 2023 NXP.
> + */
> +
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM dw_edma
> +
> +#if !defined(__LINUX_DW_EDMA_TRACE) || defined(TRACE_HEADER_MULTI_READ)
> +#define __LINUX_DW_EDMA_TRACE
> +
> +#include <linux/types.h>
> +#include <linux/tracepoint.h>
> +
> +DECLARE_EVENT_CLASS(edma_desc_info,
> + TP_PROTO(struct dw_edma_desc *desc),
> + TP_ARGS(desc),
> + TP_STRUCT__entry(
> + __field(size_t, nburst)
> + __field(size_t, start_burst)
> + __field(dma_cookie_t, cookie)
> + __field(u32, id)
> + __field(u8, dir)
> + ),
> + TP_fast_assign(
> + __entry->nburst = desc->nburst;
> + __entry->start_burst = desc->start_burst;
> + __entry->id = desc->chan->id;
> + __entry->dir = desc->chan->dir;
> + __entry->cookie = desc->vd.tx.cookie;
> + ),
> + TP_printk("chan %u%c desc %d nburst %zu start_burst %zu",
> + __entry->id,
> + __entry->dir ? 'R' : 'W',
> + __entry->cookie,
> + __entry->nburst,
> + __entry->start_burst)
> +);
> +
> +DEFINE_EVENT(edma_desc_info, edma_append_desc,
> + TP_PROTO(struct dw_edma_desc *desc),
> + TP_ARGS(desc)
> +);
> +
> +DEFINE_EVENT(edma_desc_info, edma_complete_desc,
> + TP_PROTO(struct dw_edma_desc *desc),
> + TP_ARGS(desc)
> +);
> +
> +TRACE_EVENT(edma_doorbell,
> + TP_PROTO(struct dw_edma_chan *chan),
> + TP_ARGS(chan),
> + TP_STRUCT__entry(
> + __field(u32, id)
> + __field(u8, dir)
> + __field(u32, done)
> + __field(u32, head)
> + ),
> + TP_fast_assign(
> + __entry->id = chan->id;
> + __entry->dir = chan->dir;
> + __entry->done = chan->ll_done;
> + __entry->head = chan->ll_head;
> + ),
> + TP_printk("chan %u%c done %u head %u",
> + __entry->id,
> + __entry->dir ? 'R' : 'W',
> + __entry->done,
> + __entry->head)
> +);
> +
> +TRACE_EVENT(edma_fill_ll,
> + TP_PROTO(struct dw_edma_chan *chan, u32 idx, dma_cookie_t cookie, u64 src,
> + u64 dest, u32 sz, bool flag),
> + TP_ARGS(chan, idx, cookie, src, dest, sz, flag),
[Severity: Low]
This isn't a bug, but since TRACE_SYSTEM in drivers/dma/dw-edma/dw-edma-trace.h
is defined as dw_edma, should the event class and tracepoints follow the
subsystem_event naming convention and use the dw_edma_ prefix instead of
edma_?
Specifically, edma_desc_info, edma_append_desc, edma_complete_desc,
edma_doorbell, and edma_fill_ll export global tracepoint symbols under the
generic edma_ prefix (which is also used by drivers/dma/fsl-edma-trace.h)
and are invoked in drivers/dma/dw-edma/dw-edma-core.c from
dw_edma_ll_consume(), dw_edma_core_ll_start(), dw_edma_start_transfer(),
and dw_edma_core_ch_doorbell().
Renaming them to dw_edma_desc_info, dw_edma_append_desc,
dw_edma_complete_desc, dw_edma_doorbell, and dw_edma_fill_ll (along with
their callers in dw-edma-core.c) would match the TRACE_SYSTEM name and avoid
generic edma_ symbol names.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261030150742.2586776-1-den@valinux.co.jp?part=5
^ permalink raw reply [flat|nested] 14+ messages in thread