* [PATCH v2 01/19] dmaengine: dw-edma: Add dw_edma_core_ll_cur_idx() to get current LL entry index
2026-07-23 8:41 [PATCH v2 00/19] dmaengine: dw-edma: Support dynamic LL appends Koichiro Den
@ 2026-07-23 8:41 ` Koichiro Den
2026-07-23 16:25 ` Frank Li
2026-07-23 8:41 ` [PATCH v2 02/19] dmaengine: dw-edma: Add dw_edma_core_ll_clear() to clear LL control-word Koichiro Den
` (17 subsequent siblings)
18 siblings, 1 reply; 37+ messages in thread
From: Koichiro Den @ 2026-07-23 8:41 UTC (permalink / raw)
To: Manivannan Sadhasivam, Vinod Koul, Frank Li, Gustavo Pimentel,
Kees Cook, Krzysztof Wilczyński, Kishon Vijay Abraham I,
Bjorn Helgaas, Christoph Hellwig, Serge Semin, Cai Huoqing,
Niklas Cassel
Cc: Devendra K Verma, dmaengine, linux-kernel
From: Frank Li <Frank.Li@nxp.com>
Add dw_edma_core_ll_cur_idx() to get the current LL entry index and
prepare for dynamic addition of DMA requests while the DMA engine is
running.
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>
---
Based on Frank's original submission:
https://lore.kernel.org/dmaengine/20260109-edma_dymatic-v1-1-9a98c9c98536@nxp.com/
Changes in v2:
- Handle zero low words and low-word wrap without reading DMA_LLP.MSB
on the normal path, and clarify the lost-context comments.
drivers/dma/dw-edma/dw-edma-core.h | 10 ++++++++++
drivers/dma/dw-edma/dw-edma-v0-core.c | 19 +++++++++++++++++++
drivers/dma/dw-edma/dw-hdma-v0-core.c | 19 +++++++++++++++++++
3 files changed, 48 insertions(+)
diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index f6a5ad317567..e50fa7965778 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -138,6 +138,7 @@ struct dw_edma_core_ops {
void (*ll_data)(struct dw_edma_chan *chan, struct dw_edma_burst *burst,
u32 idx, bool cb, bool irq);
void (*ll_link)(struct dw_edma_chan *chan, u32 idx, bool cb, u64 addr);
+ int (*ll_cur_idx)(struct dw_edma_chan *chan);
void (*ch_doorbell)(struct dw_edma_chan *chan);
void (*ch_enable)(struct dw_edma_chan *chan);
void (*ch_config)(struct dw_edma_chan *chan);
@@ -181,6 +182,15 @@ struct dw_edma_chan *dchan2dw_edma_chan(struct dma_chan *dchan)
return vc2dw_edma_chan(to_virt_chan(dchan));
}
+/*
+ * Return the current LL entry index. A negative value means that the channel
+ * context is not initialized or was lost after a link reset.
+ */
+static inline int dw_edma_core_ll_cur_idx(struct dw_edma_chan *chan)
+{
+ return chan->dw->core->ll_cur_idx(chan);
+}
+
static inline u64 dw_edma_core_get_ll_paddr(struct dw_edma_chan *chan)
{
if (chan->dir == EDMA_DIR_WRITE)
diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
index abc1bd4c0be2..ba84811234e1 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
@@ -616,6 +616,24 @@ static void dw_edma_v0_core_ch_doorbell(struct dw_edma_chan *chan)
FIELD_PREP(EDMA_V0_DOORBELL_CH_MASK, chan->id));
}
+static int dw_edma_v0_core_ll_cur_idx(struct dw_edma_chan *chan)
+{
+ u32 base, val;
+
+ val = GET_CH_32(chan->dw, chan->dir, chan->id, llp.lsb);
+ base = lower_32_bits(dw_edma_core_get_ll_paddr(chan));
+
+ /*
+ * An all-zero pointer means that the channel context is unavailable,
+ * so an LL region starting at DMA address zero is not supported.
+ */
+ if (!val && !GET_CH_32(chan->dw, chan->dir, chan->id, llp.msb))
+ return -EINVAL;
+
+ /* u32 subtraction handles a low-word wrap at a 4 GiB boundary. */
+ return (val - base) / EDMA_LL_SZ;
+}
+
/* eDMA debugfs callbacks */
static void dw_edma_v0_core_debugfs_on(struct dw_edma *dw)
{
@@ -650,6 +668,7 @@ static const struct dw_edma_core_ops dw_edma_v0_core = {
.handle_int = dw_edma_v0_core_handle_int,
.ll_data = dw_edma_v0_core_ll_data,
.ll_link = dw_edma_v0_core_ll_link,
+ .ll_cur_idx = dw_edma_v0_core_ll_cur_idx,
.ch_doorbell = dw_edma_v0_core_ch_doorbell,
.ch_enable = dw_edma_v0_core_ch_enable,
.ch_config = dw_edma_v0_core_ch_config,
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
index 36ee72efcd31..2fc7f8e85b43 100644
--- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
@@ -384,6 +384,24 @@ static void dw_hdma_v0_core_ch_doorbell(struct dw_edma_chan *chan)
SET_CH_32(dw, chan->dir, chan->id, doorbell, HDMA_V0_DOORBELL_START);
}
+static int dw_hdma_v0_core_ll_cur_idx(struct dw_edma_chan *chan)
+{
+ u32 base, val;
+
+ val = GET_CH_32(chan->dw, chan->dir, chan->id, llp.lsb);
+ base = lower_32_bits(dw_edma_core_get_ll_paddr(chan));
+
+ /*
+ * An all-zero pointer means that the channel context is unavailable,
+ * so an LL region starting at DMA address zero is not supported.
+ */
+ if (!val && !GET_CH_32(chan->dw, chan->dir, chan->id, llp.msb))
+ return -EINVAL;
+
+ /* u32 subtraction handles a low-word wrap at a 4 GiB boundary. */
+ return (val - base) / EDMA_LL_SZ;
+}
+
/* HDMA debugfs callbacks */
static void dw_hdma_v0_core_debugfs_on(struct dw_edma *dw)
{
@@ -406,6 +424,7 @@ static const struct dw_edma_core_ops dw_hdma_v0_core = {
.non_ll_start = dw_hdma_v0_core_non_ll_start,
.ll_data = dw_hdma_v0_core_ll_data,
.ll_link = dw_hdma_v0_core_ll_link,
+ .ll_cur_idx = dw_hdma_v0_core_ll_cur_idx,
.ch_doorbell = dw_hdma_v0_core_ch_doorbell,
.ch_enable = dw_hdma_v0_core_ch_enable,
.ch_config = dw_hdma_v0_core_ch_config,
--
2.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v2 01/19] dmaengine: dw-edma: Add dw_edma_core_ll_cur_idx() to get current LL entry index
2026-07-23 8:41 ` [PATCH v2 01/19] dmaengine: dw-edma: Add dw_edma_core_ll_cur_idx() to get current LL entry index Koichiro Den
@ 2026-07-23 16:25 ` Frank Li
0 siblings, 0 replies; 37+ messages in thread
From: Frank Li @ 2026-07-23 16:25 UTC (permalink / raw)
To: Koichiro Den
Cc: Manivannan Sadhasivam, Vinod Koul, Frank Li, Gustavo Pimentel,
Kees Cook, Krzysztof Wilczyński, Kishon Vijay Abraham I,
Bjorn Helgaas, Christoph Hellwig, Serge Semin, Cai Huoqing,
Niklas Cassel, Devendra K Verma, dmaengine, linux-kernel
On Thu, Jul 23, 2026 at 05:41:32PM +0900, Koichiro Den wrote:
> From: Frank Li <Frank.Li@nxp.com>
>
> Add dw_edma_core_ll_cur_idx() to get the current LL entry index and
> prepare for dynamic addition of DMA requests while the DMA engine is
> running.
>
> 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>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Based on Frank's original submission:
> https://lore.kernel.org/dmaengine/20260109-edma_dymatic-v1-1-9a98c9c98536@nxp.com/
>
> Changes in v2:
> - Handle zero low words and low-word wrap without reading DMA_LLP.MSB
> on the normal path, and clarify the lost-context comments.
>
> drivers/dma/dw-edma/dw-edma-core.h | 10 ++++++++++
> drivers/dma/dw-edma/dw-edma-v0-core.c | 19 +++++++++++++++++++
> drivers/dma/dw-edma/dw-hdma-v0-core.c | 19 +++++++++++++++++++
> 3 files changed, 48 insertions(+)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
> index f6a5ad317567..e50fa7965778 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.h
> +++ b/drivers/dma/dw-edma/dw-edma-core.h
> @@ -138,6 +138,7 @@ struct dw_edma_core_ops {
> void (*ll_data)(struct dw_edma_chan *chan, struct dw_edma_burst *burst,
> u32 idx, bool cb, bool irq);
> void (*ll_link)(struct dw_edma_chan *chan, u32 idx, bool cb, u64 addr);
> + int (*ll_cur_idx)(struct dw_edma_chan *chan);
> void (*ch_doorbell)(struct dw_edma_chan *chan);
> void (*ch_enable)(struct dw_edma_chan *chan);
> void (*ch_config)(struct dw_edma_chan *chan);
> @@ -181,6 +182,15 @@ struct dw_edma_chan *dchan2dw_edma_chan(struct dma_chan *dchan)
> return vc2dw_edma_chan(to_virt_chan(dchan));
> }
>
> +/*
> + * Return the current LL entry index. A negative value means that the channel
> + * context is not initialized or was lost after a link reset.
> + */
> +static inline int dw_edma_core_ll_cur_idx(struct dw_edma_chan *chan)
> +{
> + return chan->dw->core->ll_cur_idx(chan);
> +}
> +
> static inline u64 dw_edma_core_get_ll_paddr(struct dw_edma_chan *chan)
> {
> if (chan->dir == EDMA_DIR_WRITE)
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> index abc1bd4c0be2..ba84811234e1 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> @@ -616,6 +616,24 @@ static void dw_edma_v0_core_ch_doorbell(struct dw_edma_chan *chan)
> FIELD_PREP(EDMA_V0_DOORBELL_CH_MASK, chan->id));
> }
>
> +static int dw_edma_v0_core_ll_cur_idx(struct dw_edma_chan *chan)
> +{
> + u32 base, val;
> +
> + val = GET_CH_32(chan->dw, chan->dir, chan->id, llp.lsb);
> + base = lower_32_bits(dw_edma_core_get_ll_paddr(chan));
> +
> + /*
> + * An all-zero pointer means that the channel context is unavailable,
> + * so an LL region starting at DMA address zero is not supported.
> + */
> + if (!val && !GET_CH_32(chan->dw, chan->dir, chan->id, llp.msb))
> + return -EINVAL;
> +
> + /* u32 subtraction handles a low-word wrap at a 4 GiB boundary. */
> + return (val - base) / EDMA_LL_SZ;
> +}
> +
> /* eDMA debugfs callbacks */
> static void dw_edma_v0_core_debugfs_on(struct dw_edma *dw)
> {
> @@ -650,6 +668,7 @@ static const struct dw_edma_core_ops dw_edma_v0_core = {
> .handle_int = dw_edma_v0_core_handle_int,
> .ll_data = dw_edma_v0_core_ll_data,
> .ll_link = dw_edma_v0_core_ll_link,
> + .ll_cur_idx = dw_edma_v0_core_ll_cur_idx,
> .ch_doorbell = dw_edma_v0_core_ch_doorbell,
> .ch_enable = dw_edma_v0_core_ch_enable,
> .ch_config = dw_edma_v0_core_ch_config,
> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> index 36ee72efcd31..2fc7f8e85b43 100644
> --- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> @@ -384,6 +384,24 @@ static void dw_hdma_v0_core_ch_doorbell(struct dw_edma_chan *chan)
> SET_CH_32(dw, chan->dir, chan->id, doorbell, HDMA_V0_DOORBELL_START);
> }
>
> +static int dw_hdma_v0_core_ll_cur_idx(struct dw_edma_chan *chan)
> +{
> + u32 base, val;
> +
> + val = GET_CH_32(chan->dw, chan->dir, chan->id, llp.lsb);
> + base = lower_32_bits(dw_edma_core_get_ll_paddr(chan));
> +
> + /*
> + * An all-zero pointer means that the channel context is unavailable,
> + * so an LL region starting at DMA address zero is not supported.
> + */
> + if (!val && !GET_CH_32(chan->dw, chan->dir, chan->id, llp.msb))
> + return -EINVAL;
> +
> + /* u32 subtraction handles a low-word wrap at a 4 GiB boundary. */
> + return (val - base) / EDMA_LL_SZ;
> +}
> +
> /* HDMA debugfs callbacks */
> static void dw_hdma_v0_core_debugfs_on(struct dw_edma *dw)
> {
> @@ -406,6 +424,7 @@ static const struct dw_edma_core_ops dw_hdma_v0_core = {
> .non_ll_start = dw_hdma_v0_core_non_ll_start,
> .ll_data = dw_hdma_v0_core_ll_data,
> .ll_link = dw_hdma_v0_core_ll_link,
> + .ll_cur_idx = dw_hdma_v0_core_ll_cur_idx,
> .ch_doorbell = dw_hdma_v0_core_ch_doorbell,
> .ch_enable = dw_hdma_v0_core_ch_enable,
> .ch_config = dw_hdma_v0_core_ch_config,
> --
> 2.51.0
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v2 02/19] dmaengine: dw-edma: Add dw_edma_core_ll_clear() to clear LL control-word
2026-07-23 8:41 [PATCH v2 00/19] dmaengine: dw-edma: Support dynamic LL appends Koichiro Den
2026-07-23 8:41 ` [PATCH v2 01/19] dmaengine: dw-edma: Add dw_edma_core_ll_cur_idx() to get current LL entry index Koichiro Den
@ 2026-07-23 8:41 ` Koichiro Den
2026-07-23 8:41 ` [PATCH v2 03/19] dmaengine: dw-edma: Factor out linked-list transfer start Koichiro Den
` (16 subsequent siblings)
18 siblings, 0 replies; 37+ messages in thread
From: Koichiro Den @ 2026-07-23 8:41 UTC (permalink / raw)
To: Manivannan Sadhasivam, Vinod Koul, Frank Li, Gustavo Pimentel,
Kees Cook, Krzysztof Wilczyński, Kishon Vijay Abraham I,
Bjorn Helgaas, Christoph Hellwig, Serge Semin, Cai Huoqing,
Niklas Cassel
Cc: Devendra K Verma, dmaengine, linux-kernel
Add a core operation for clearing the control word of one linked-list
entry. A later patch uses the operation to invalidate stale CB when
resetting a circular LL ring.
No functional change.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
- Split the provider operation from "Clear LL data entries on reset".
drivers/dma/dw-edma/dw-edma-core.h | 6 ++++++
drivers/dma/dw-edma/dw-edma-v0-core.c | 16 ++++++++++++++++
drivers/dma/dw-edma/dw-hdma-v0-core.c | 16 ++++++++++++++++
3 files changed, 38 insertions(+)
diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index e50fa7965778..089f913fd247 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -138,6 +138,7 @@ struct dw_edma_core_ops {
void (*ll_data)(struct dw_edma_chan *chan, struct dw_edma_burst *burst,
u32 idx, bool cb, bool irq);
void (*ll_link)(struct dw_edma_chan *chan, u32 idx, bool cb, u64 addr);
+ void (*ll_clear)(struct dw_edma_chan *chan, u32 idx);
int (*ll_cur_idx)(struct dw_edma_chan *chan);
void (*ch_doorbell)(struct dw_edma_chan *chan);
void (*ch_enable)(struct dw_edma_chan *chan);
@@ -255,6 +256,11 @@ dw_edma_core_ll_link(struct dw_edma_chan *chan, u32 idx, bool cb, u64 addr)
chan->dw->core->ll_link(chan, idx, cb, addr);
}
+static inline void dw_edma_core_ll_clear(struct dw_edma_chan *chan, u32 idx)
+{
+ chan->dw->core->ll_clear(chan, idx);
+}
+
static inline void dw_edma_core_ch_doorbell(struct dw_edma_chan *chan)
{
chan->dw->core->ch_doorbell(chan);
diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
index ba84811234e1..c31fff095b4f 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
@@ -605,6 +605,21 @@ dw_edma_v0_core_ll_link(struct dw_edma_chan *chan, u32 idx, bool cb, u64 addr)
dw_edma_v0_write_ll_link(chan, idx, control, addr);
}
+static void dw_edma_v0_core_ll_clear(struct dw_edma_chan *chan, u32 idx)
+{
+ ptrdiff_t ofs = idx * sizeof(struct dw_edma_v0_lli);
+
+ if (chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL) {
+ struct dw_edma_v0_lli *lli = chan->ll_region.vaddr.mem + ofs;
+
+ lli->control = 0;
+ } else {
+ struct dw_edma_v0_lli __iomem *lli = chan->ll_region.vaddr.io + ofs;
+
+ writel(0, &lli->control);
+ }
+}
+
static void dw_edma_v0_core_ch_doorbell(struct dw_edma_chan *chan)
{
struct dw_edma *dw = chan->dw;
@@ -668,6 +683,7 @@ static const struct dw_edma_core_ops dw_edma_v0_core = {
.handle_int = dw_edma_v0_core_handle_int,
.ll_data = dw_edma_v0_core_ll_data,
.ll_link = dw_edma_v0_core_ll_link,
+ .ll_clear = dw_edma_v0_core_ll_clear,
.ll_cur_idx = dw_edma_v0_core_ll_cur_idx,
.ch_doorbell = dw_edma_v0_core_ch_doorbell,
.ch_enable = dw_edma_v0_core_ch_enable,
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
index 2fc7f8e85b43..b2d35f0b7b6d 100644
--- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
@@ -374,6 +374,21 @@ dw_hdma_v0_core_ll_link(struct dw_edma_chan *chan, u32 idx, bool cb, u64 addr)
dw_hdma_v0_write_ll_link(chan, idx, control, addr);
}
+static void dw_hdma_v0_core_ll_clear(struct dw_edma_chan *chan, u32 idx)
+{
+ ptrdiff_t ofs = idx * sizeof(struct dw_hdma_v0_lli);
+
+ if (chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL) {
+ struct dw_hdma_v0_lli *lli = chan->ll_region.vaddr.mem + ofs;
+
+ lli->control = 0;
+ } else {
+ struct dw_hdma_v0_lli __iomem *lli = chan->ll_region.vaddr.io + ofs;
+
+ writel(0, &lli->control);
+ }
+}
+
static void dw_hdma_v0_core_ch_doorbell(struct dw_edma_chan *chan)
{
struct dw_edma *dw = chan->dw;
@@ -424,6 +439,7 @@ static const struct dw_edma_core_ops dw_hdma_v0_core = {
.non_ll_start = dw_hdma_v0_core_non_ll_start,
.ll_data = dw_hdma_v0_core_ll_data,
.ll_link = dw_hdma_v0_core_ll_link,
+ .ll_clear = dw_hdma_v0_core_ll_clear,
.ll_cur_idx = dw_hdma_v0_core_ll_cur_idx,
.ch_doorbell = dw_hdma_v0_core_ch_doorbell,
.ch_enable = dw_hdma_v0_core_ch_enable,
--
2.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v2 03/19] dmaengine: dw-edma: Factor out linked-list transfer start
2026-07-23 8:41 [PATCH v2 00/19] dmaengine: dw-edma: Support dynamic LL appends Koichiro Den
2026-07-23 8:41 ` [PATCH v2 01/19] dmaengine: dw-edma: Add dw_edma_core_ll_cur_idx() to get current LL entry index Koichiro Den
2026-07-23 8:41 ` [PATCH v2 02/19] dmaengine: dw-edma: Add dw_edma_core_ll_clear() to clear LL control-word Koichiro Den
@ 2026-07-23 8:41 ` Koichiro Den
2026-07-23 16:31 ` Frank Li
2026-07-23 8:41 ` [PATCH v2 04/19] dmaengine: dw-edma: Make DMA link list work as a circular buffer Koichiro Den
` (15 subsequent siblings)
18 siblings, 1 reply; 37+ messages in thread
From: Koichiro Den @ 2026-07-23 8:41 UTC (permalink / raw)
To: Manivannan Sadhasivam, Vinod Koul, Frank Li, Gustavo Pimentel,
Kees Cook, Krzysztof Wilczyński, Kishon Vijay Abraham I,
Bjorn Helgaas, Christoph Hellwig, Serge Semin, Cai Huoqing,
Niklas Cassel
Cc: Devendra K Verma, dmaengine, linux-kernel
Move linked-list programming into dw_edma_core_ll_start(). It can derive
the first-batch state from desc->start_burst, so dw_edma_core_start() no
longer needs a pass-through argument.
No functional change intended.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
- New patch.
drivers/dma/dw-edma/dw-edma-core.c | 29 +++++++++++++++++++----------
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 1f893dc54c79..6a25a050b89c 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -67,17 +67,11 @@ static void vchan_free_desc(struct virt_dma_desc *vdesc)
kfree(vd2dw_edma_desc(vdesc));
}
-static void dw_edma_core_start(struct dw_edma_desc *desc, bool first)
+static void dw_edma_core_ll_start(struct dw_edma_desc *desc)
{
struct dw_edma_chan *chan = desc->chan;
- size_t i = 0;
-
- if (chan->non_ll) {
- chan->dw->core->non_ll_start(chan, &desc->burst[desc->start_burst]);
- desc->done_burst = desc->start_burst;
- desc->start_burst += 1;
- return;
- }
+ size_t i;
+ bool first = !desc->start_burst;
for (i = 0; i + desc->start_burst < desc->nburst; i++) {
u32 idx = i + desc->start_burst;
@@ -101,6 +95,21 @@ static void dw_edma_core_start(struct dw_edma_desc *desc, bool first)
dw_edma_core_ch_doorbell(chan);
}
+static void dw_edma_core_start(struct dw_edma_desc *desc)
+{
+ struct dw_edma_chan *chan = desc->chan;
+
+ if (chan->non_ll) {
+ chan->dw->core->non_ll_start(chan,
+ &desc->burst[desc->start_burst]);
+ desc->done_burst = desc->start_burst;
+ desc->start_burst += 1;
+ return;
+ }
+
+ dw_edma_core_ll_start(desc);
+}
+
static int dw_edma_start_transfer(struct dw_edma_chan *chan)
{
struct dw_edma_desc *desc;
@@ -114,7 +123,7 @@ static int dw_edma_start_transfer(struct dw_edma_chan *chan)
if (!desc)
return 0;
- dw_edma_core_start(desc, !desc->start_burst);
+ dw_edma_core_start(desc);
desc->cb = !desc->cb;
--
2.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v2 03/19] dmaengine: dw-edma: Factor out linked-list transfer start
2026-07-23 8:41 ` [PATCH v2 03/19] dmaengine: dw-edma: Factor out linked-list transfer start Koichiro Den
@ 2026-07-23 16:31 ` Frank Li
0 siblings, 0 replies; 37+ messages in thread
From: Frank Li @ 2026-07-23 16:31 UTC (permalink / raw)
To: Koichiro Den
Cc: Manivannan Sadhasivam, Vinod Koul, Frank Li, Gustavo Pimentel,
Kees Cook, Krzysztof Wilczyński, Kishon Vijay Abraham I,
Bjorn Helgaas, Christoph Hellwig, Serge Semin, Cai Huoqing,
Niklas Cassel, Devendra K Verma, dmaengine, linux-kernel
On Thu, Jul 23, 2026 at 05:41:34PM +0900, Koichiro Den wrote:
> Move linked-list programming into dw_edma_core_ll_start(). It can derive
> the first-batch state from desc->start_burst, so dw_edma_core_start() no
> longer needs a pass-through argument.
>
> No functional change intended.
>
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Changes in v2:
> - New patch.
>
> drivers/dma/dw-edma/dw-edma-core.c | 29 +++++++++++++++++++----------
> 1 file changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 1f893dc54c79..6a25a050b89c 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -67,17 +67,11 @@ static void vchan_free_desc(struct virt_dma_desc *vdesc)
> kfree(vd2dw_edma_desc(vdesc));
> }
>
> -static void dw_edma_core_start(struct dw_edma_desc *desc, bool first)
> +static void dw_edma_core_ll_start(struct dw_edma_desc *desc)
> {
> struct dw_edma_chan *chan = desc->chan;
> - size_t i = 0;
> -
> - if (chan->non_ll) {
> - chan->dw->core->non_ll_start(chan, &desc->burst[desc->start_burst]);
> - desc->done_burst = desc->start_burst;
> - desc->start_burst += 1;
> - return;
> - }
> + size_t i;
> + bool first = !desc->start_burst;
>
> for (i = 0; i + desc->start_burst < desc->nburst; i++) {
> u32 idx = i + desc->start_burst;
> @@ -101,6 +95,21 @@ static void dw_edma_core_start(struct dw_edma_desc *desc, bool first)
> dw_edma_core_ch_doorbell(chan);
> }
>
> +static void dw_edma_core_start(struct dw_edma_desc *desc)
> +{
> + struct dw_edma_chan *chan = desc->chan;
> +
> + if (chan->non_ll) {
> + chan->dw->core->non_ll_start(chan,
> + &desc->burst[desc->start_burst]);
> + desc->done_burst = desc->start_burst;
> + desc->start_burst += 1;
> + return;
> + }
> +
> + dw_edma_core_ll_start(desc);
> +}
> +
> static int dw_edma_start_transfer(struct dw_edma_chan *chan)
> {
> struct dw_edma_desc *desc;
> @@ -114,7 +123,7 @@ static int dw_edma_start_transfer(struct dw_edma_chan *chan)
> if (!desc)
> return 0;
>
> - dw_edma_core_start(desc, !desc->start_burst);
> + dw_edma_core_start(desc);
>
> desc->cb = !desc->cb;
>
> --
> 2.51.0
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v2 04/19] dmaengine: dw-edma: Make DMA link list work as a circular buffer
2026-07-23 8:41 [PATCH v2 00/19] dmaengine: dw-edma: Support dynamic LL appends Koichiro Den
` (2 preceding siblings ...)
2026-07-23 8:41 ` [PATCH v2 03/19] dmaengine: dw-edma: Factor out linked-list transfer start Koichiro Den
@ 2026-07-23 8:41 ` Koichiro Den
2026-07-23 9:07 ` sashiko-bot
2026-07-23 8:41 ` [PATCH v2 05/19] dmaengine: dw-edma: Add LL interrupt placement policy Koichiro Den
` (14 subsequent siblings)
18 siblings, 1 reply; 37+ messages in thread
From: Koichiro Den @ 2026-07-23 8:41 UTC (permalink / raw)
To: Manivannan Sadhasivam, Vinod Koul, Frank Li, Gustavo Pimentel,
Kees Cook, Krzysztof Wilczyński, Kishon Vijay Abraham I,
Bjorn Helgaas, Christoph Hellwig, Serge Semin, Cai Huoqing,
Niklas Cassel
Cc: Devendra K Verma, dmaengine, linux-kernel
From: Frank Li <Frank.Li@nxp.com>
The driver currently rebuilds the whole linked list for every transfer.
Use it as a circular ring instead. Append entries at ll_head with the
current cycle bit, and reserve the final entry for the link back to the
start.
Clear control words before first use so stale cycle bits cannot become
valid entries. Reject rings without usable data slots and, until reclaim
support lands, descriptors that exceed the usable ring capacity.
This prepares the driver for appending requests while the engine runs.
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>
---
Changes in v2:
- Track first use in software instead of inferring it from DMA_LLP.
- Clear stale entries and validate the LL layout; leave non-LL
configurations unchanged.
- Reject oversized descriptors until reclaim support lands.
drivers/dma/dw-edma/dw-edma-core.c | 86 ++++++++++++++++++++++++------
drivers/dma/dw-edma/dw-edma-core.h | 26 ++++++++-
2 files changed, 94 insertions(+), 18 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 6a25a050b89c..a9f25a65294e 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -51,13 +51,19 @@ 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;
desc->chan = chan;
desc->nburst = nburst;
- desc->cb = true;
return desc;
}
@@ -67,30 +73,65 @@ static void vchan_free_desc(struct virt_dma_desc *vdesc)
kfree(vd2dw_edma_desc(vdesc));
}
+static void dw_edma_core_reset_ll(struct dw_edma_chan *chan)
+{
+ u32 i;
+
+ chan->ll_head = 0;
+ chan->ll_end = 0;
+ /* Drop stale CB bits before reusing the circular LL ring. */
+ for (i = 0; i < chan->ll_max; i++)
+ dw_edma_core_ll_clear(chan, i);
+ chan->cb = true;
+
+ dw_edma_core_ll_link(chan, chan->ll_max, chan->cb,
+ chan->ll_region.paddr);
+
+ dw_edma_core_ch_enable(chan);
+ chan->ll_valid = true;
+}
+
+static u32 dw_edma_core_get_free_num(struct dw_edma_chan *chan)
+{
+ /* Keep one data entry free so equal indices mean an empty ring. */
+ return (chan->ll_end + chan->ll_max - 1 - chan->ll_head) %
+ chan->ll_max;
+}
+
static void dw_edma_core_ll_start(struct dw_edma_desc *desc)
{
struct dw_edma_chan *chan = desc->chan;
size_t i;
- bool first = !desc->start_burst;
+ u32 free;
- for (i = 0; i + desc->start_burst < desc->nburst; i++) {
- u32 idx = i + desc->start_burst;
+ for (i = desc->start_burst; i < desc->nburst; i++) {
+ free = dw_edma_core_get_free_num(chan);
- if (i == chan->ll_max)
+ if (!free)
break;
- dw_edma_core_ll_data(chan, &desc->burst[idx],
- i, desc->cb,
- idx == desc->nburst - 1 || i == chan->ll_max - 1);
- }
+ /*
+ * Refresh the link element before filling the last data slot so
+ * the next lap has the updated CB value.
+ */
+ if (chan->ll_head == chan->ll_max - 1)
+ dw_edma_core_ll_link(chan, chan->ll_max, chan->cb,
+ chan->ll_region.paddr);
- desc->done_burst = desc->start_burst;
- desc->start_burst += i;
+ dw_edma_core_ll_data(chan, &desc->burst[i],
+ chan->ll_head, chan->cb,
+ i == desc->nburst - 1 || free == 1);
- dw_edma_core_ll_link(chan, i, desc->cb, chan->ll_region.paddr);
+ chan->ll_head++;
- if (first)
- dw_edma_core_ch_enable(chan);
+ if (chan->ll_head == chan->ll_max) {
+ chan->cb = !chan->cb;
+ chan->ll_head = 0;
+ }
+ }
+
+ desc->done_burst = desc->start_burst;
+ desc->start_burst = i;
dw_edma_core_ch_doorbell(chan);
}
@@ -123,9 +164,10 @@ static int dw_edma_start_transfer(struct dw_edma_chan *chan)
if (!desc)
return 0;
- dw_edma_core_start(desc);
+ if (!chan->non_ll && !chan->ll_valid)
+ dw_edma_core_reset_ll(chan);
- desc->cb = !desc->cb;
+ dw_edma_core_start(desc);
return 1;
}
@@ -645,6 +687,8 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
DMA_TRANS_NOERROR);
list_del(&vd->node);
vchan_cookie_complete(vd);
+ if (!chan->non_ll)
+ chan->ll_end = chan->ll_head;
}
if (chan->request == EDMA_REQ_PAUSE) {
@@ -871,6 +915,9 @@ static int dw_edma_alloc_chan_resources(struct dma_chan *dchan)
if (chan->status != EDMA_ST_IDLE)
return -EBUSY;
+ /* The hardware context may have been invalidated while unowned. */
+ chan->ll_valid = false;
+
return 0;
}
@@ -962,6 +1009,13 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
else
chan->ll_region = chip->ll_region_rd[chan->id];
+ if (!chip->cfg_non_ll && chan->ll_region.sz < 3 * EDMA_LL_SZ) {
+ dev_err(dev,
+ "channel %s[%u]: LL region has fewer than 2 data entries\n",
+ str_write_read(chan->dir == EDMA_DIR_WRITE),
+ chan->id);
+ return -EINVAL;
+ }
chan->ll_max = chan->ll_region.sz / EDMA_LL_SZ - 1;
dev_vdbg(dev, "L. List:\tChannel %s[%u] max_cnt=%u\n",
diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index 089f913fd247..f2c2d5af1fff 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -60,7 +60,6 @@ struct dw_edma_desc {
size_t done_burst;
size_t start_burst;
- u8 cb;
size_t nburst;
struct dw_edma_burst burst[] __counted_by(nburst);
};
@@ -72,8 +71,31 @@ struct dw_edma_chan {
enum dw_edma_dir dir;
u8 func_no;
- u32 ll_max;
+ /*
+ * New LL entries are appended at ll_head. Entries between ll_end
+ * and ll_head, modulo the LL ring, are owned by DMA; the rest are
+ * owned by software.
+ *
+ * software-owned DMA-owned software-owned
+ * +---------------+-------------------+---------------+
+ * ^ ^ ^
+ * 0 ll_end ll_head
+ *
+ * The link entry points back to the region start. ll_head == ll_end
+ * means all entries are software-owned and previous DMA work is
+ * done.
+ *
+ * Software always keeps at least one free entry, so the ring is
+ * never completely DMA-owned.
+ */
+ u32 ll_head;
+ u32 ll_end;
+
+ u32 ll_max; /* Data entries */
struct dw_edma_region ll_region; /* Linked list */
+ bool ll_valid; /* LL context programmed */
+
+ bool cb;
struct msi_msg msi;
--
2.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v2 04/19] dmaengine: dw-edma: Make DMA link list work as a circular buffer
2026-07-23 8:41 ` [PATCH v2 04/19] dmaengine: dw-edma: Make DMA link list work as a circular buffer Koichiro Den
@ 2026-07-23 9:07 ` sashiko-bot
2026-07-23 16:43 ` Frank Li
0 siblings, 1 reply; 37+ messages in thread
From: sashiko-bot @ 2026-07-23 9:07 UTC (permalink / raw)
To: Koichiro Den; +Cc: vkoul, Frank.Li, dmaengine
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Ring buffer capacity is permanently leaked on error and termination paths because `ll_end` is not advanced.
--
--- Patch [4]: [PATCH v2 04/19] dmaengine: dw-edma: Make DMA link list work as a circular buffer ---
commit 2d5b6554b58830487f70a13eab5e1ac3de304658
Author: Frank Li <Frank.Li@nxp.com>
dmaengine: dw-edma: Make DMA link list work as a circular buffer
This commit updates the driver to use the DMA linked list as a circular
ring buffer. It appends entries at ll_head with the current cycle bit and
reserves the final entry to link back to the start.
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
> @@ -645,6 +687,8 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> DMA_TRANS_NOERROR);
> list_del(&vd->node);
> vchan_cookie_complete(vd);
> + if (!chan->non_ll)
> + chan->ll_end = chan->ll_head;
> }
>
> if (chan->request == EDMA_REQ_PAUSE) {
[Severity: High]
Does this leak ring capacity when descriptors are aborted or terminated?
This block successfully reclaims ring capacity on successful completion by
advancing chan->ll_end. However, if a client driver queues transfers and
then aborts them (for example, via dmaengine_terminate_all() or during an
abort interrupt), the descriptors are removed from the list but chan->ll_end
does not appear to be updated to reflect the aborted descriptors.
Since chan->ll_valid remains true after a termination, subsequent transfers
will bypass the dw_edma_core_reset_ll() ring reset logic and continue
appending to the un-reclaimed ring.
Will this eventually cause dw_edma_core_get_free_num() to permanently return
0 and deadlock the channel once the un-reclaimed slots exhaust the ring?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260723084150.521366-1-den@valinux.co.jp?part=4
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v2 04/19] dmaengine: dw-edma: Make DMA link list work as a circular buffer
2026-07-23 9:07 ` sashiko-bot
@ 2026-07-23 16:43 ` Frank Li
0 siblings, 0 replies; 37+ messages in thread
From: Frank Li @ 2026-07-23 16:43 UTC (permalink / raw)
To: sashiko-reviews; +Cc: Koichiro Den, vkoul, Frank.Li, dmaengine
On Thu, Jul 23, 2026 at 09:07:05AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Ring buffer capacity is permanently leaked on error and termination paths because `ll_end` is not advanced.
> --
>
> --- Patch [4]: [PATCH v2 04/19] dmaengine: dw-edma: Make DMA link list work as a circular buffer ---
> commit 2d5b6554b58830487f70a13eab5e1ac3de304658
> Author: Frank Li <Frank.Li@nxp.com>
>
> dmaengine: dw-edma: Make DMA link list work as a circular buffer
>
> This commit updates the driver to use the DMA linked list as a circular
> ring buffer. It appends entries at ll_head with the current cycle bit and
> reserves the final entry to link back to the start.
>
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-core.c
>
> [ ... ]
>
> > @@ -645,6 +687,8 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> > DMA_TRANS_NOERROR);
> > list_del(&vd->node);
> > vchan_cookie_complete(vd);
> > + if (!chan->non_ll)
> > + chan->ll_end = chan->ll_head;
> > }
> >
> > if (chan->request == EDMA_REQ_PAUSE) {
>
> [Severity: High]
> Does this leak ring capacity when descriptors are aborted or terminated?
>
> This block successfully reclaims ring capacity on successful completion by
> advancing chan->ll_end. However, if a client driver queues transfers and
> then aborts them (for example, via dmaengine_terminate_all() or during an
> abort interrupt), the descriptors are removed from the list but chan->ll_end
> does not appear to be updated to reflect the aborted descriptors.
>
> Since chan->ll_valid remains true after a termination, subsequent transfers
> will bypass the dw_edma_core_reset_ll() ring reset logic and continue
> appending to the un-reclaimed ring.
Make sense. Need change at these abnormal case.
Frank
>
> Will this eventually cause dw_edma_core_get_free_num() to permanently return
> 0 and deadlock the channel once the un-reclaimed slots exhaust the ring?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260723084150.521366-1-den@valinux.co.jp?part=4
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v2 05/19] dmaengine: dw-edma: Add LL interrupt placement policy
2026-07-23 8:41 [PATCH v2 00/19] dmaengine: dw-edma: Support dynamic LL appends Koichiro Den
` (3 preceding siblings ...)
2026-07-23 8:41 ` [PATCH v2 04/19] dmaengine: dw-edma: Make DMA link list work as a circular buffer Koichiro Den
@ 2026-07-23 8:41 ` Koichiro Den
2026-07-23 8:41 ` [PATCH v2 06/19] dmaengine: dw-edma: Move callback result helper before LL helpers Koichiro Den
` (13 subsequent siblings)
18 siblings, 0 replies; 37+ messages in thread
From: Koichiro Den @ 2026-07-23 8:41 UTC (permalink / raw)
To: Manivannan Sadhasivam, Vinod Koul, Frank Li, Gustavo Pimentel,
Kees Cook, Krzysztof Wilczyński, Kishon Vijay Abraham I,
Bjorn Helgaas, Christoph Hellwig, Serge Semin, Cai Huoqing,
Niklas Cassel
Cc: Devendra K Verma, dmaengine, linux-kernel
Move linked-list interrupt placement behind a core callback so eDMA and
HDMA can use different policies.
Keep the eDMA behavior. For HDMA, place watermarks at descriptor ends,
before link entries, and every four entries when a descriptor exceeds the
usable ring capacity or another issued descriptor follows. A later patch
handles these watermarks as progress events.
Four entries is an empirically chosen coalescing interval.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
- Fix the zero-based interval so the first watermark is on the fourth
entry, not the fifth.
- Revise the commit message and source comments.
drivers/dma/dw-edma/dw-edma-core.c | 8 +++++++-
drivers/dma/dw-edma/dw-edma-core.h | 2 ++
drivers/dma/dw-edma/dw-edma-v0-core.c | 10 ++++++++++
drivers/dma/dw-edma/dw-hdma-v0-core.c | 28 +++++++++++++++++++++++++++
4 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index a9f25a65294e..78d1bb6302fb 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -98,6 +98,12 @@ static u32 dw_edma_core_get_free_num(struct dw_edma_chan *chan)
chan->ll_max;
}
+static bool dw_edma_core_enable_ll_irq(struct dw_edma_desc *desc, u32 i,
+ u32 free)
+{
+ return desc->chan->dw->core->ll_irq(desc, i, free);
+}
+
static void dw_edma_core_ll_start(struct dw_edma_desc *desc)
{
struct dw_edma_chan *chan = desc->chan;
@@ -120,7 +126,7 @@ static void dw_edma_core_ll_start(struct dw_edma_desc *desc)
dw_edma_core_ll_data(chan, &desc->burst[i],
chan->ll_head, chan->cb,
- i == desc->nburst - 1 || free == 1);
+ dw_edma_core_enable_ll_irq(desc, i, free));
chan->ll_head++;
diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index f2c2d5af1fff..8ed37e8a12cb 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -162,6 +162,8 @@ struct dw_edma_core_ops {
void (*ll_link)(struct dw_edma_chan *chan, u32 idx, bool cb, u64 addr);
void (*ll_clear)(struct dw_edma_chan *chan, u32 idx);
int (*ll_cur_idx)(struct dw_edma_chan *chan);
+ /* Called with chan->vc.lock held. */
+ bool (*ll_irq)(struct dw_edma_desc *desc, u32 i, u32 free);
void (*ch_doorbell)(struct dw_edma_chan *chan);
void (*ch_enable)(struct dw_edma_chan *chan);
void (*ch_config)(struct dw_edma_chan *chan);
diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
index c31fff095b4f..62141aa32e50 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
@@ -649,6 +649,15 @@ static int dw_edma_v0_core_ll_cur_idx(struct dw_edma_chan *chan)
return (val - base) / EDMA_LL_SZ;
}
+static bool dw_edma_v0_core_ll_irq(struct dw_edma_desc *desc, u32 i, u32 free)
+{
+ /*
+ * eDMA reports LL interrupts through DONE. Keep them at
+ * descriptor ends, plus the last free slot to refill the ring.
+ */
+ return i == desc->nburst - 1 || free == 1;
+}
+
/* eDMA debugfs callbacks */
static void dw_edma_v0_core_debugfs_on(struct dw_edma *dw)
{
@@ -685,6 +694,7 @@ static const struct dw_edma_core_ops dw_edma_v0_core = {
.ll_link = dw_edma_v0_core_ll_link,
.ll_clear = dw_edma_v0_core_ll_clear,
.ll_cur_idx = dw_edma_v0_core_ll_cur_idx,
+ .ll_irq = dw_edma_v0_core_ll_irq,
.ch_doorbell = dw_edma_v0_core_ch_doorbell,
.ch_enable = dw_edma_v0_core_ch_enable,
.ch_config = dw_edma_v0_core_ch_config,
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
index b2d35f0b7b6d..fb651a83f0f3 100644
--- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
@@ -13,6 +13,9 @@
#include "dw-hdma-v0-regs.h"
#include "dw-hdma-v0-debugfs.h"
+/* Empirically chosen watermark interval. */
+#define HDMA_V0_WATERMARK_INTERVAL 4
+
enum dw_hdma_control {
DW_HDMA_V0_CB = BIT(0),
DW_HDMA_V0_TCB = BIT(1),
@@ -417,6 +420,30 @@ static int dw_hdma_v0_core_ll_cur_idx(struct dw_edma_chan *chan)
return (val - base) / EDMA_LL_SZ;
}
+static bool dw_hdma_v0_core_ll_irq(struct dw_edma_desc *desc, u32 i, u32 free)
+{
+ struct dw_edma_chan *chan = desc->chan;
+ bool needs_progress;
+
+ /*
+ * Always place a watermark at a descriptor end and before the link
+ * element. The first reports descriptor progress; the second provides
+ * one progress point per ring lap.
+ */
+ if (i == desc->nburst - 1 || chan->ll_head == chan->ll_max - 1)
+ return true;
+
+ /*
+ * Use periodic watermarks when a descriptor exceeds the usable ring or
+ * when more issued work follows it.
+ */
+ needs_progress = desc->nburst > chan->ll_max - 1 ||
+ !list_is_last(&desc->vd.node, &chan->vc.desc_issued);
+
+ return needs_progress &&
+ (chan->ll_head + 1) % HDMA_V0_WATERMARK_INTERVAL == 0;
+}
+
/* HDMA debugfs callbacks */
static void dw_hdma_v0_core_debugfs_on(struct dw_edma *dw)
{
@@ -441,6 +468,7 @@ static const struct dw_edma_core_ops dw_hdma_v0_core = {
.ll_link = dw_hdma_v0_core_ll_link,
.ll_clear = dw_hdma_v0_core_ll_clear,
.ll_cur_idx = dw_hdma_v0_core_ll_cur_idx,
+ .ll_irq = dw_hdma_v0_core_ll_irq,
.ch_doorbell = dw_hdma_v0_core_ch_doorbell,
.ch_enable = dw_hdma_v0_core_ch_enable,
.ch_config = dw_hdma_v0_core_ch_config,
--
2.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v2 06/19] dmaengine: dw-edma: Move callback result helper before LL helpers
2026-07-23 8:41 [PATCH v2 00/19] dmaengine: dw-edma: Support dynamic LL appends Koichiro Den
` (4 preceding siblings ...)
2026-07-23 8:41 ` [PATCH v2 05/19] dmaengine: dw-edma: Add LL interrupt placement policy Koichiro Den
@ 2026-07-23 8:41 ` Koichiro Den
2026-07-23 16:51 ` Frank Li
2026-07-23 8:41 ` [PATCH v2 07/19] dmaengine: dw-edma: Dispatch DONE interrupts by channel request Koichiro Den
` (12 subsequent siblings)
18 siblings, 1 reply; 37+ messages in thread
From: Koichiro Den @ 2026-07-23 8:41 UTC (permalink / raw)
To: Manivannan Sadhasivam, Vinod Koul, Frank Li, Gustavo Pimentel,
Kees Cook, Krzysztof Wilczyński, Kishon Vijay Abraham I,
Bjorn Helgaas, Christoph Hellwig, Serge Semin, Cai Huoqing,
Niklas Cassel
Cc: Devendra K Verma, dmaengine, linux-kernel
From: Frank Li <Frank.Li@nxp.com>
Move dw_hdma_set_callback_result() next to the descriptor helpers so
linked-list completion and reset code can use it without a forward
declaration.
No functional change.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Based on Frank's original submission:
https://lore.kernel.org/dmaengine/20260109-edma_dymatic-v1-2-9a98c9c98536@nxp.com/
Changes in v2:
- Move the helper before the LL helpers so later patches do not need
another relocation.
drivers/dma/dw-edma/dw-edma-core.c | 50 +++++++++++++++---------------
1 file changed, 25 insertions(+), 25 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 78d1bb6302fb..29f78405e9fb 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -73,6 +73,31 @@ static void vchan_free_desc(struct virt_dma_desc *vdesc)
kfree(vd2dw_edma_desc(vdesc));
}
+static void dw_hdma_set_callback_result(struct virt_dma_desc *vd,
+ enum dmaengine_tx_result result)
+{
+ u32 residue = 0;
+ struct dw_edma_desc *desc;
+ struct dmaengine_result *res;
+
+ if (!vd->tx.callback_result)
+ return;
+
+ desc = vd2dw_edma_desc(vd);
+ if (desc) {
+ residue = desc->alloc_sz;
+
+ if (result == DMA_TRANS_NOERROR)
+ residue -= desc->burst[desc->start_burst - 1].xfer_sz;
+ else if (desc->done_burst)
+ residue -= desc->burst[desc->done_burst - 1].xfer_sz;
+ }
+
+ res = &vd->tx_result;
+ res->result = result;
+ res->residue = residue;
+}
+
static void dw_edma_core_reset_ll(struct dw_edma_chan *chan)
{
u32 i;
@@ -645,31 +670,6 @@ 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_hdma_set_callback_result(struct virt_dma_desc *vd,
- enum dmaengine_tx_result result)
-{
- u32 residue = 0;
- struct dw_edma_desc *desc;
- struct dmaengine_result *res;
-
- if (!vd->tx.callback_result)
- return;
-
- desc = vd2dw_edma_desc(vd);
- if (desc) {
- residue = desc->alloc_sz;
-
- if (result == DMA_TRANS_NOERROR)
- residue -= desc->burst[desc->start_burst - 1].xfer_sz;
- else if (desc->done_burst)
- residue -= desc->burst[desc->done_burst - 1].xfer_sz;
- }
-
- res = &vd->tx_result;
- res->result = result;
- res->residue = residue;
-}
-
static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
{
struct dw_edma_desc *desc;
--
2.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v2 06/19] dmaengine: dw-edma: Move callback result helper before LL helpers
2026-07-23 8:41 ` [PATCH v2 06/19] dmaengine: dw-edma: Move callback result helper before LL helpers Koichiro Den
@ 2026-07-23 16:51 ` Frank Li
0 siblings, 0 replies; 37+ messages in thread
From: Frank Li @ 2026-07-23 16:51 UTC (permalink / raw)
To: Koichiro Den
Cc: Manivannan Sadhasivam, Vinod Koul, Frank Li, Gustavo Pimentel,
Kees Cook, Krzysztof Wilczyński, Kishon Vijay Abraham I,
Bjorn Helgaas, Christoph Hellwig, Serge Semin, Cai Huoqing,
Niklas Cassel, Devendra K Verma, dmaengine, linux-kernel
On Thu, Jul 23, 2026 at 05:41:37PM +0900, Koichiro Den wrote:
> From: Frank Li <Frank.Li@nxp.com>
>
> Move dw_hdma_set_callback_result() next to the descriptor helpers so
> linked-list completion and reset code can use it without a forward
> declaration.
>
> No functional change.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Based on Frank's original submission:
> https://lore.kernel.org/dmaengine/20260109-edma_dymatic-v1-2-9a98c9c98536@nxp.com/
>
> Changes in v2:
> - Move the helper before the LL helpers so later patches do not need
> another relocation.
>
> drivers/dma/dw-edma/dw-edma-core.c | 50 +++++++++++++++---------------
> 1 file changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 78d1bb6302fb..29f78405e9fb 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -73,6 +73,31 @@ static void vchan_free_desc(struct virt_dma_desc *vdesc)
> kfree(vd2dw_edma_desc(vdesc));
> }
>
> +static void dw_hdma_set_callback_result(struct virt_dma_desc *vd,
> + enum dmaengine_tx_result result)
> +{
> + u32 residue = 0;
> + struct dw_edma_desc *desc;
> + struct dmaengine_result *res;
> +
> + if (!vd->tx.callback_result)
> + return;
> +
> + desc = vd2dw_edma_desc(vd);
> + if (desc) {
> + residue = desc->alloc_sz;
> +
> + if (result == DMA_TRANS_NOERROR)
> + residue -= desc->burst[desc->start_burst - 1].xfer_sz;
> + else if (desc->done_burst)
> + residue -= desc->burst[desc->done_burst - 1].xfer_sz;
> + }
> +
> + res = &vd->tx_result;
> + res->result = result;
> + res->residue = residue;
> +}
> +
> static void dw_edma_core_reset_ll(struct dw_edma_chan *chan)
> {
> u32 i;
> @@ -645,31 +670,6 @@ 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_hdma_set_callback_result(struct virt_dma_desc *vd,
> - enum dmaengine_tx_result result)
> -{
> - u32 residue = 0;
> - struct dw_edma_desc *desc;
> - struct dmaengine_result *res;
> -
> - if (!vd->tx.callback_result)
> - return;
> -
> - desc = vd2dw_edma_desc(vd);
> - if (desc) {
> - residue = desc->alloc_sz;
> -
> - if (result == DMA_TRANS_NOERROR)
> - residue -= desc->burst[desc->start_burst - 1].xfer_sz;
> - else if (desc->done_burst)
> - residue -= desc->burst[desc->done_burst - 1].xfer_sz;
> - }
> -
> - res = &vd->tx_result;
> - res->result = result;
> - res->residue = residue;
> -}
> -
> static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> {
> struct dw_edma_desc *desc;
> --
> 2.51.0
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v2 07/19] dmaengine: dw-edma: Dispatch DONE interrupts by channel request
2026-07-23 8:41 [PATCH v2 00/19] dmaengine: dw-edma: Support dynamic LL appends Koichiro Den
` (5 preceding siblings ...)
2026-07-23 8:41 ` [PATCH v2 06/19] dmaengine: dw-edma: Move callback result helper before LL helpers Koichiro Den
@ 2026-07-23 8:41 ` Koichiro Den
2026-07-23 8:55 ` sashiko-bot
2026-07-23 16:57 ` Frank Li
2026-07-23 8:41 ` [PATCH v2 08/19] dmaengine: dw-edma: Centralize LL doorbell decisions Koichiro Den
` (11 subsequent siblings)
18 siblings, 2 replies; 37+ messages in thread
From: Koichiro Den @ 2026-07-23 8:41 UTC (permalink / raw)
To: Manivannan Sadhasivam, Vinod Koul, Frank Li, Gustavo Pimentel,
Kees Cook, Krzysztof Wilczyński, Kishon Vijay Abraham I,
Bjorn Helgaas, Christoph Hellwig, Serge Semin, Cai Huoqing,
Niklas Cassel
Cc: Devendra K Verma, dmaengine, linux-kernel
Handle the channel request first in the DONE interrupt path, then look
at the issued descriptor list only in the cases that need it.
This keeps the existing behavior, including the current STOP and PAUSE
handling when no issued descriptor is present.
No functional change intended.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
- Limit this patch to request dispatch with no functional change;
move LL progress and empty-issued-list handling to later patches.
drivers/dma/dw-edma/dw-edma-core.c | 60 +++++++++++++++---------------
1 file changed, 31 insertions(+), 29 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 29f78405e9fb..fa104af4146e 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -682,41 +682,43 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
return;
}
- vd = vchan_next_desc(&chan->vc);
- if (vd) {
- switch (chan->request) {
- case EDMA_REQ_NONE:
- case EDMA_REQ_PAUSE:
- desc = vd2dw_edma_desc(vd);
- if (desc->start_burst >= desc->nburst) {
- dw_hdma_set_callback_result(vd,
- DMA_TRANS_NOERROR);
- list_del(&vd->node);
- vchan_cookie_complete(vd);
- if (!chan->non_ll)
- chan->ll_end = chan->ll_head;
- }
-
- if (chan->request == EDMA_REQ_PAUSE) {
- chan->request = EDMA_REQ_NONE;
- chan->status = EDMA_ST_PAUSE;
- break;
- }
-
- /* Continue transferring if there are remaining chunks or issued requests.
- */
- chan->status = dw_edma_start_transfer(chan) ? EDMA_ST_BUSY : EDMA_ST_IDLE;
+ switch (chan->request) {
+ case EDMA_REQ_NONE:
+ case EDMA_REQ_PAUSE:
+ vd = vchan_next_desc(&chan->vc);
+ if (!vd)
break;
- case EDMA_REQ_STOP:
- dw_edma_terminate_all_descs(chan);
+ desc = vd2dw_edma_desc(vd);
+ if (desc->start_burst >= desc->nburst) {
+ dw_hdma_set_callback_result(vd, DMA_TRANS_NOERROR);
+ list_del(&vd->node);
+ vchan_cookie_complete(vd);
+ if (!chan->non_ll)
+ chan->ll_end = chan->ll_head;
+ }
+
+ if (chan->request == EDMA_REQ_PAUSE) {
chan->request = EDMA_REQ_NONE;
- chan->status = EDMA_ST_IDLE;
+ chan->status = EDMA_ST_PAUSE;
break;
+ }
- default:
+ chan->status = dw_edma_start_transfer(chan) ? EDMA_ST_BUSY : EDMA_ST_IDLE;
+ break;
+
+ case EDMA_REQ_STOP:
+ vd = vchan_next_desc(&chan->vc);
+ if (!vd)
break;
- }
+
+ dw_edma_terminate_all_descs(chan);
+ chan->request = EDMA_REQ_NONE;
+ chan->status = EDMA_ST_IDLE;
+ break;
+
+ default:
+ break;
}
spin_unlock_irqrestore(&chan->vc.lock, flags);
}
--
2.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v2 07/19] dmaengine: dw-edma: Dispatch DONE interrupts by channel request
2026-07-23 8:41 ` [PATCH v2 07/19] dmaengine: dw-edma: Dispatch DONE interrupts by channel request Koichiro Den
@ 2026-07-23 8:55 ` sashiko-bot
2026-07-23 16:57 ` Frank Li
1 sibling, 0 replies; 37+ messages in thread
From: sashiko-bot @ 2026-07-23 8:55 UTC (permalink / raw)
To: Koichiro Den; +Cc: vkoul, Frank.Li, dmaengine
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Channel state machine can get permanently stuck if a delayed interrupt arrives after a termination timeout (or if the interrupt never arrives).
--
--- Patch [7]: [PATCH v2 07/19] dmaengine: dw-edma: Dispatch DONE interrupts by channel request ---
commit 820c52e0edf662b1111ec33a334d76d30c13a546
Author: Koichiro Den <den@valinux.co.jp>
dmaengine: dw-edma: Dispatch DONE interrupts by channel request
This patch modifies the DONE interrupt path to handle the channel request
first before looking at the issued descriptor list. The change is intended
to keep the existing behavior without introducing functional changes.
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
> @@ -682,41 +682,43 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
[ ... ]
> + case EDMA_REQ_STOP:
> + vd = vchan_next_desc(&chan->vc);
> + if (!vd)
> break;
[Severity: High]
This is a pre-existing issue, but does this early break leave the channel
permanently stuck in EDMA_ST_BUSY if a delayed interrupt arrives after a
termination timeout?
If a driver calls dmaengine_terminate_sync() and the hardware hangs,
dw_edma_wait_termination() will time out. Then
dw_edma_free_chan_resources() calls vchan_free_chan_resources(),
clearing the descriptor list.
If a delayed interrupt arrives afterward, dw_edma_done_interrupt() handles
EDMA_REQ_STOP but hits this if (!vd) check because the list is now empty.
By breaking early, the code skips resetting chan->request = EDMA_REQ_NONE
and chan->status = EDMA_ST_IDLE. Will this cause subsequent calls to
dw_edma_alloc_chan_resources() to fail with -EBUSY and permanently leak
the DMA channel resource?
> +
> + dw_edma_terminate_all_descs(chan);
> + chan->request = EDMA_REQ_NONE;
> + chan->status = EDMA_ST_IDLE;
> + break;
> +
> + default:
> + break;
> }
> spin_unlock_irqrestore(&chan->vc.lock, flags);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260723084150.521366-1-den@valinux.co.jp?part=7
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v2 07/19] dmaengine: dw-edma: Dispatch DONE interrupts by channel request
2026-07-23 8:41 ` [PATCH v2 07/19] dmaengine: dw-edma: Dispatch DONE interrupts by channel request Koichiro Den
2026-07-23 8:55 ` sashiko-bot
@ 2026-07-23 16:57 ` Frank Li
1 sibling, 0 replies; 37+ messages in thread
From: Frank Li @ 2026-07-23 16:57 UTC (permalink / raw)
To: Koichiro Den
Cc: Manivannan Sadhasivam, Vinod Koul, Frank Li, Gustavo Pimentel,
Kees Cook, Krzysztof Wilczyński, Kishon Vijay Abraham I,
Bjorn Helgaas, Christoph Hellwig, Serge Semin, Cai Huoqing,
Niklas Cassel, Devendra K Verma, dmaengine, linux-kernel
On Thu, Jul 23, 2026 at 05:41:38PM +0900, Koichiro Den wrote:
Subject:
Check channel request firstly at DONE channel interrupts ?
> Handle the channel request first in the DONE interrupt path, then look
> at the issued descriptor list only in the cases that need it.
>
> This keeps the existing behavior, including the current STOP and PAUSE
> handling when no issued descriptor is present.
Need descript why need this change?
Frank
>
> No functional change intended.
>
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---
> Changes in v2:
> - Limit this patch to request dispatch with no functional change;
> move LL progress and empty-issued-list handling to later patches.
>
> drivers/dma/dw-edma/dw-edma-core.c | 60 +++++++++++++++---------------
> 1 file changed, 31 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 29f78405e9fb..fa104af4146e 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -682,41 +682,43 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> return;
> }
>
> - vd = vchan_next_desc(&chan->vc);
> - if (vd) {
> - switch (chan->request) {
> - case EDMA_REQ_NONE:
> - case EDMA_REQ_PAUSE:
> - desc = vd2dw_edma_desc(vd);
> - if (desc->start_burst >= desc->nburst) {
> - dw_hdma_set_callback_result(vd,
> - DMA_TRANS_NOERROR);
> - list_del(&vd->node);
> - vchan_cookie_complete(vd);
> - if (!chan->non_ll)
> - chan->ll_end = chan->ll_head;
> - }
> -
> - if (chan->request == EDMA_REQ_PAUSE) {
> - chan->request = EDMA_REQ_NONE;
> - chan->status = EDMA_ST_PAUSE;
> - break;
> - }
> -
> - /* Continue transferring if there are remaining chunks or issued requests.
> - */
> - chan->status = dw_edma_start_transfer(chan) ? EDMA_ST_BUSY : EDMA_ST_IDLE;
> + switch (chan->request) {
> + case EDMA_REQ_NONE:
> + case EDMA_REQ_PAUSE:
> + vd = vchan_next_desc(&chan->vc);
> + if (!vd)
> break;
>
> - case EDMA_REQ_STOP:
> - dw_edma_terminate_all_descs(chan);
> + desc = vd2dw_edma_desc(vd);
> + if (desc->start_burst >= desc->nburst) {
> + dw_hdma_set_callback_result(vd, DMA_TRANS_NOERROR);
> + list_del(&vd->node);
> + vchan_cookie_complete(vd);
> + if (!chan->non_ll)
> + chan->ll_end = chan->ll_head;
> + }
> +
> + if (chan->request == EDMA_REQ_PAUSE) {
> chan->request = EDMA_REQ_NONE;
> - chan->status = EDMA_ST_IDLE;
> + chan->status = EDMA_ST_PAUSE;
> break;
> + }
>
> - default:
> + chan->status = dw_edma_start_transfer(chan) ? EDMA_ST_BUSY : EDMA_ST_IDLE;
> + break;
> +
> + case EDMA_REQ_STOP:
> + vd = vchan_next_desc(&chan->vc);
> + if (!vd)
> break;
> - }
> +
> + dw_edma_terminate_all_descs(chan);
> + chan->request = EDMA_REQ_NONE;
> + chan->status = EDMA_ST_IDLE;
> + break;
> +
> + default:
> + break;
> }
> spin_unlock_irqrestore(&chan->vc.lock, flags);
> }
> --
> 2.51.0
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v2 08/19] dmaengine: dw-edma: Centralize LL doorbell decisions
2026-07-23 8:41 [PATCH v2 00/19] dmaengine: dw-edma: Support dynamic LL appends Koichiro Den
` (6 preceding siblings ...)
2026-07-23 8:41 ` [PATCH v2 07/19] dmaengine: dw-edma: Dispatch DONE interrupts by channel request Koichiro Den
@ 2026-07-23 8:41 ` Koichiro Den
2026-07-23 9:09 ` sashiko-bot
2026-07-23 17:02 ` Frank Li
2026-07-23 8:41 ` [PATCH v2 09/19] dmaengine: dw-edma: Reclaim issued descriptors from IRQ-paired LL progress Koichiro Den
` (10 subsequent siblings)
18 siblings, 2 replies; 37+ messages in thread
From: Koichiro Den @ 2026-07-23 8:41 UTC (permalink / raw)
To: Manivannan Sadhasivam, Vinod Koul, Frank Li, Gustavo Pimentel,
Kees Cook, Krzysztof Wilczyński, Kishon Vijay Abraham I,
Bjorn Helgaas, Christoph Hellwig, Serge Semin, Cai Huoqing,
Niklas Cassel
Cc: Devendra K Verma, dmaengine, linux-kernel
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 <den@valinux.co.jp>
---
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)
+{
+ 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
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v2 08/19] dmaengine: dw-edma: Centralize LL doorbell decisions
2026-07-23 8:41 ` [PATCH v2 08/19] dmaengine: dw-edma: Centralize LL doorbell decisions Koichiro Den
@ 2026-07-23 9:09 ` sashiko-bot
2026-07-23 17:02 ` Frank Li
1 sibling, 0 replies; 37+ messages in thread
From: sashiko-bot @ 2026-07-23 9:09 UTC (permalink / raw)
To: Koichiro Den; +Cc: Frank.Li, dmaengine, vkoul
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] A Time-of-Check to Time-of-Use (TOCTOU) race when reading the hardware status can permanently stall the DMA channel.
Pre-existing issues:
- [High] DMA structures allocated with devm_ are freed before clients release their references, causing a Use-After-Free (UAF).
--
--- Patch [8]: [PATCH v2 08/19] dmaengine: dw-edma: Centralize LL doorbell decisions ---
commit d0e41965abfce0d50ea05e24f3f479e6cabd8ab0
Author: Koichiro Den <den@valinux.co.jp>
dmaengine: dw-edma: Centralize LL doorbell decisions
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.
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 1f893dc54c793..e88bfb417ad4a 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
> @@ -150,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)
> +{
> + 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;
[Severity: High]
Is there a time-of-check to time-of-use regression here that can permanently
stall the channel?
If the hardware fetches a descriptor just before the software updates it,
it sees a cycle bit mismatch and begins transitioning to DMA_COMPLETE.
If dw_edma_core_ch_status(chan) reads the status during this transition,
it sees DMA_IN_PROGRESS and incorrectly skips ringing the doorbell.
If the hardware stops on an element without the local interrupt enable
bit, no interrupt is raised. Because the doorbell was skipped, the channel
never resumes.
> +
> + dw_edma_core_ch_doorbell(chan);
> +}
> +
[ ... ]
[Severity: High]
This isn't a bug introduced by this patch, but are the dma structures
vulnerable to a use-after-free when a client holds a reference during
device removal?
The structures are allocated using devm_kzalloc() in dw_edma_probe():
drivers/dma/dw-edma/dw-edma-core.c:dw_edma_probe() {
...
dw = devm_kzalloc(dev, sizeof(*dw), GFP_KERNEL);
if (!dw)
return -ENOMEM;
...
}
During removal, dma_async_device_unregister() is called but does not wait
for clients to drop references before devres frees the memory:
drivers/dma/dw-edma/dw-edma-core.c:dw_edma_remove() {
...
/* Deregister eDMA device */
dma_async_device_unregister(&dw->dma);
...
}
If a client later drops its reference, dw_edma_free_chan_resources() is
called on the already freed memory:
drivers/dma/dw-edma/dw-edma-core.c:dw_edma_free_chan_resources() {
struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
...
}
Could the memory be improperly tied to the device's devres lifecycle
instead of using explicit lifecycle management (like a kref) or the dma
engine's device_release callback?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260723084150.521366-1-den@valinux.co.jp?part=8
^ permalink raw reply [flat|nested] 37+ messages in thread* Re: [PATCH v2 08/19] dmaengine: dw-edma: Centralize LL doorbell decisions
2026-07-23 8:41 ` [PATCH v2 08/19] dmaengine: dw-edma: Centralize LL doorbell decisions Koichiro Den
2026-07-23 9:09 ` sashiko-bot
@ 2026-07-23 17:02 ` Frank Li
1 sibling, 0 replies; 37+ messages in thread
From: Frank Li @ 2026-07-23 17:02 UTC (permalink / raw)
To: Koichiro Den
Cc: Manivannan Sadhasivam, Vinod Koul, Frank Li, Gustavo Pimentel,
Kees Cook, Krzysztof Wilczyński, Kishon Vijay Abraham I,
Bjorn Helgaas, Christoph Hellwig, Serge Semin, Cai Huoqing,
Niklas Cassel, Devendra K Verma, dmaengine, linux-kernel
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 <den@valinux.co.jp>
> ---
> 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()
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
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v2 09/19] dmaengine: dw-edma: Reclaim issued descriptors from IRQ-paired LL progress
2026-07-23 8:41 [PATCH v2 00/19] dmaengine: dw-edma: Support dynamic LL appends Koichiro Den
` (7 preceding siblings ...)
2026-07-23 8:41 ` [PATCH v2 08/19] dmaengine: dw-edma: Centralize LL doorbell decisions Koichiro Den
@ 2026-07-23 8:41 ` Koichiro Den
2026-07-23 9:01 ` sashiko-bot
2026-07-23 8:41 ` [PATCH v2 10/19] dmaengine: dw-edma: Use HDMA watermarks as progress events Koichiro Den
` (9 subsequent siblings)
18 siblings, 1 reply; 37+ messages in thread
From: Koichiro Den @ 2026-07-23 8:41 UTC (permalink / raw)
To: Manivannan Sadhasivam, Vinod Koul, Frank Li, Gustavo Pimentel,
Kees Cook, Krzysztof Wilczyński, Kishon Vijay Abraham I,
Bjorn Helgaas, Christoph Hellwig, Serge Semin, Cai Huoqing,
Niklas Cassel
Cc: Devendra K Verma, dmaengine, linux-kernel
Dynamic append can put entries from several descriptors in the ring.
Track the consumed boundary in ll_done, reuse consumed entries, and
complete descriptors in issued order. Keep one data entry free so a
physical index is unambiguous within the active producer window.
Sample DMA_LLP in the hard IRQ under vc.lock after clearing status, then
normalize it to the exclusive boundary used by ll_done. A deferred read
could attach progress to the wrong interrupt.
A stopped sample points to the next entry. Keep a running sample one
entry behind because legacy eDMA fetch can advance DMA_LLP before payload
completion. The following HDMA watermark patch uses the same conservative
boundary. Handle EDMA_REQ_STOP and EDMA_REQ_PAUSE even if progress empties
the issued list.
Suggested-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
- Replace ll_end completion accounting with ll_done progress tracking.
- Sample and normalize DMA_LLP in the hard IRQ; v1 read it from deferred
consumers.
- Use the raw next-entry boundary for stopped samples and keep running
samples one entry behind it.
- Handle STOP and PAUSE after progress empties the issued list.
Frank, dw_edma_ll_clean_pending() is based on the idea in your RFT,
although the implementation has changed substantially. Please let me
know if you would prefer different attribution other than Suggested-by.
drivers/dma/dw-edma/dw-edma-core.c | 248 ++++++++++++++++++++++----
drivers/dma/dw-edma/dw-edma-core.h | 32 ++--
drivers/dma/dw-edma/dw-edma-v0-core.c | 10 +-
drivers/dma/dw-edma/dw-hdma-v0-core.c | 4 +-
4 files changed, 250 insertions(+), 44 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index e88bfb417ad4..44a0ff906f2f 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -51,13 +51,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;
@@ -73,6 +66,12 @@ static void vchan_free_desc(struct virt_dma_desc *vdesc)
kfree(vd2dw_edma_desc(vdesc));
}
+static void dw_edma_ll_irq_idx_discard(struct dw_edma_chan *chan)
+{
+ chan->ll_irq_idx = -1;
+ chan->ll_irq_stopped = false;
+}
+
static void dw_hdma_set_callback_result(struct virt_dma_desc *vd,
enum dmaengine_tx_result result)
{
@@ -103,7 +102,8 @@ static void dw_edma_core_reset_ll(struct dw_edma_chan *chan)
u32 i;
chan->ll_head = 0;
- chan->ll_end = 0;
+ chan->ll_done = 0;
+ dw_edma_ll_irq_idx_discard(chan);
/* Drop stale CB bits before reusing the circular LL ring. */
for (i = 0; i < chan->ll_max; i++)
dw_edma_core_ll_clear(chan, i);
@@ -116,11 +116,26 @@ static void dw_edma_core_reset_ll(struct dw_edma_chan *chan)
chan->ll_valid = true;
}
+static u32 dw_edma_core_get_ll_dist(struct dw_edma_chan *chan, u32 from, u32 to)
+{
+ return (to + chan->ll_max - from) % chan->ll_max;
+}
+
+static u32 dw_edma_core_get_used_num(struct dw_edma_chan *chan)
+{
+ return dw_edma_core_get_ll_dist(chan, chan->ll_done, chan->ll_head);
+}
+
static u32 dw_edma_core_get_free_num(struct dw_edma_chan *chan)
{
- /* Keep one data entry free so equal indices mean an empty ring. */
- return (chan->ll_end + chan->ll_max - 1 - chan->ll_head) %
- chan->ll_max;
+ /*
+ * Measure occupancy from ll_done: entries consumed by the
+ * hardware can be reused even if the descriptor that owns them has not
+ * completed yet; this lets descriptors larger than the ring move forward.
+ *
+ * Keep one data entry free so equal indices mean an empty ring.
+ */
+ return chan->ll_max - 1 - dw_edma_core_get_used_num(chan);
}
static bool dw_edma_core_enable_ll_irq(struct dw_edma_desc *desc, u32 i,
@@ -129,9 +144,51 @@ 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_advance(struct dw_edma_chan *chan, int idx, u32 *old_done)
+{
+ u32 done;
+
+ if (idx < 0 || (u32)idx >= chan->ll_max)
+ return false;
+
+ done = dw_edma_core_get_ll_dist(chan, chan->ll_done, idx);
+ if (!done || done > dw_edma_core_get_used_num(chan))
+ return false;
+
+ *old_done = chan->ll_done;
+ chan->ll_done = idx;
+
+ return true;
+}
+
static bool dw_edma_ll_pending(struct dw_edma_chan *chan)
{
- return chan->ll_head != chan->ll_end;
+ return dw_edma_core_get_used_num(chan);
+}
+
+static u32 dw_edma_core_ch_transfer_size(struct dw_edma_chan *chan)
+{
+ if (!chan->dw->core->ch_transfer_size)
+ return U32_MAX;
+
+ return chan->dw->core->ch_transfer_size(chan);
+}
+
+static bool dw_edma_ll_irq_is_stopped(struct dw_edma_chan *chan, bool stopped)
+{
+ if (stopped)
+ return true;
+
+ /*
+ * Native HDMA reports STOP separately. The legacy interrupt interface
+ * uses DONE for both progress and stop, so confirm a stopped boundary
+ * with channel status and transfer size.
+ */
+ if (!chan->dw->core->ch_transfer_size)
+ return false;
+
+ return dw_edma_core_ch_status(chan) == DMA_COMPLETE &&
+ dw_edma_core_ch_transfer_size(chan) == 0;
}
static void dw_edma_core_ll_start(struct dw_edma_desc *desc)
@@ -166,7 +223,6 @@ static void dw_edma_core_ll_start(struct dw_edma_desc *desc)
}
}
- desc->done_burst = desc->start_burst;
desc->start_burst = i;
}
@@ -235,6 +291,110 @@ 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_ll_clean_pending(struct dw_edma_chan *chan, u32 old_done)
+{
+ struct virt_dma_desc *vd, *_vd;
+ u32 done = dw_edma_core_get_ll_dist(chan, old_done, chan->ll_done);
+
+ list_for_each_entry_safe(vd, _vd, &chan->vc.desc_issued, node) {
+ struct dw_edma_desc *desc = vd2dw_edma_desc(vd);
+ u32 consumed, started;
+
+ if (!done)
+ break;
+
+ /*
+ * start_burst is the append boundary. done_burst is the
+ * hardware-consumed boundary reported through LL progress.
+ */
+ started = desc->start_burst - desc->done_burst;
+ if (!started)
+ break;
+
+ consumed = min(done, started);
+ desc->done_burst += consumed;
+ done -= consumed;
+
+ /*
+ * Descriptors are appended in list order, so later descriptors
+ * cannot be complete if this one has not been fully consumed.
+ */
+ if (desc->done_burst != desc->nburst)
+ break;
+
+ /* Hardware has consumed this descriptor's LL entries. */
+ dw_hdma_set_callback_result(vd, DMA_TRANS_NOERROR);
+ list_del(&vd->node);
+ vchan_cookie_complete(vd);
+ }
+}
+
+/* Must be called with vc.lock held. */
+static int dw_edma_ll_recycle_idx(struct dw_edma_chan *chan, int idx,
+ bool stopped)
+{
+ if (idx < 0 || (u32)idx > chan->ll_max)
+ return -EINVAL;
+
+ /*
+ * Convert the raw LLP index to the exclusive boundary used by ll_done.
+ * For both eDMA and HDMA, once the engine has stopped, LLP points to
+ * the next element. ll_max is the link element, hence the following
+ * data boundary is 0.
+ */
+ if (stopped)
+ return idx == chan->ll_max ? 0 : idx;
+
+ /*
+ * Moving a running index one entry back cannot represent index 0
+ * without wrapping it to ll_max - 1. That could falsely consume a full
+ * producer window, so wait for another sample or STOP.
+ */
+ if (!idx)
+ return -EINVAL;
+
+ /*
+ * A running eDMA LLP can move ahead of payload completion, so keep the
+ * boundary one entry behind it.
+ *
+ * DWC PCIe Controller Databook 6.10a-lca06, Section 7.2.1, Table 7-3
+ * describes an HDMA watermark LLP as an inclusive LLE recycling
+ * boundary, which would normally translate to idx + 1. During testing
+ * on a DWC HDMA 6.30a integration, using that boundary for DMAengine
+ * completion let clients release DMA mappings while hardware still
+ * accessed them, causing IOMMU faults. Keep the boundary one entry
+ * behind the raw LLP for HDMA as well. Stopped samples continue to use
+ * the next-entry boundary above.
+ */
+ return idx == chan->ll_max ? chan->ll_max - 1 : idx - 1;
+}
+
+/*
+ * Must be called with vc.lock held. Consume a recorded LL progress
+ * boundary, if any. Advance ll_done and complete descriptors covered by
+ * the newly consumed range. Return true if ll_done advanced.
+ */
+static bool dw_edma_ll_consume_progress(struct dw_edma_chan *chan)
+{
+ int idx = chan->ll_irq_idx;
+ u32 old_done;
+
+ /* No progress sample is pending. */
+ if (idx < 0)
+ return false;
+
+ dw_edma_ll_irq_idx_discard(chan);
+
+ /* Ignore duplicate or stale progress. */
+ if (!dw_edma_ll_advance(chan, idx, &old_done))
+ return false;
+
+ dw_edma_ll_clean_pending(chan, old_done);
+
+ return true;
+}
+
/* Must be called with vc.lock held. */
static void dw_edma_core_ch_maybe_doorbell(struct dw_edma_chan *chan)
{
@@ -424,6 +584,8 @@ static void dw_edma_device_issue_pending(struct dma_chan *dchan)
if (chan->configured && vchan_issue_pending(&chan->vc) &&
chan->request == EDMA_REQ_NONE &&
chan->status == EDMA_ST_IDLE) {
+ if (!chan->non_ll && !dw_edma_ll_pending(chan))
+ dw_edma_ll_irq_idx_discard(chan);
chan->status = EDMA_ST_BUSY;
dw_edma_start_transfer(chan);
dw_edma_core_ch_maybe_doorbell(chan);
@@ -697,6 +859,7 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
struct dw_edma_desc *desc;
struct virt_dma_desc *vd;
unsigned long flags;
+ bool active;
spin_lock_irqsave(&chan->vc.lock, flags);
if (chan->status == EDMA_ST_PAUSE) {
@@ -704,20 +867,21 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
return;
}
+ if (!chan->non_ll)
+ dw_edma_ll_consume_progress(chan);
+
switch (chan->request) {
case EDMA_REQ_NONE:
case EDMA_REQ_PAUSE:
vd = vchan_next_desc(&chan->vc);
- if (!vd)
- break;
-
- desc = vd2dw_edma_desc(vd);
- if (desc->start_burst >= desc->nburst) {
- dw_hdma_set_callback_result(vd, DMA_TRANS_NOERROR);
- list_del(&vd->node);
- vchan_cookie_complete(vd);
- if (!chan->non_ll)
- chan->ll_end = chan->ll_head;
+ if (vd && chan->non_ll) {
+ desc = vd2dw_edma_desc(vd);
+ if (desc->start_burst >= desc->nburst) {
+ dw_hdma_set_callback_result(vd,
+ DMA_TRANS_NOERROR);
+ list_del(&vd->node);
+ vchan_cookie_complete(vd);
+ }
}
if (chan->request == EDMA_REQ_PAUSE) {
@@ -726,14 +890,15 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
break;
}
- chan->status = dw_edma_start_transfer(chan) ? EDMA_ST_BUSY : EDMA_ST_IDLE;
+ active = false;
+ if (vd)
+ active = dw_edma_start_transfer(chan);
+ if (!chan->non_ll)
+ active = dw_edma_ll_pending(chan);
+ chan->status = active ? EDMA_ST_BUSY : EDMA_ST_IDLE;
break;
case EDMA_REQ_STOP:
- vd = vchan_next_desc(&chan->vc);
- if (!vd)
- break;
-
dw_edma_terminate_all_descs(chan);
chan->request = EDMA_REQ_NONE;
chan->status = EDMA_ST_IDLE;
@@ -789,12 +954,34 @@ static void dw_edma_queue_irq_work(struct dw_edma_chan *chan,
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_idx(struct dw_edma_chan *chan, bool stopped)
+{
+ int idx;
+
+ guard(spinlock_irqsave)(&chan->vc.lock);
+
+ if (!chan->non_ll) {
+ idx = dw_edma_core_ll_cur_idx(chan);
+ if (idx >= 0) {
+ stopped = dw_edma_ll_irq_is_stopped(chan, stopped);
+ idx = dw_edma_ll_recycle_idx(chan, idx, stopped);
+ if (idx >= 0) {
+ chan->ll_irq_idx = idx;
+ chan->ll_irq_stopped = stopped;
+ }
+ }
+ }
+}
+
+static void dw_edma_done_interrupt_deferred(struct dw_edma_chan *chan,
+ bool stopped)
{
+ dw_edma_record_irq_idx(chan, stopped);
dw_edma_queue_irq_work(chan, DW_EDMA_IRQ_DONE);
}
-static void dw_edma_abort_interrupt_deferred(struct dw_edma_chan *chan)
+static void dw_edma_abort_interrupt_deferred(struct dw_edma_chan *chan,
+ bool stopped)
{
dw_edma_queue_irq_work(chan, DW_EDMA_IRQ_ABORT);
}
@@ -1033,6 +1220,7 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
chan->request = EDMA_REQ_NONE;
chan->status = EDMA_ST_IDLE;
chan->irq_mode = dw_edma_get_default_irq_mode(chan);
+ dw_edma_ll_irq_idx_discard(chan);
INIT_WORK(&chan->irq_work, dw_edma_irq_work);
atomic_set(&chan->irq_pending, 0);
diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index 8ed37e8a12cb..90dcbe415f57 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -72,24 +72,35 @@ struct dw_edma_chan {
u8 func_no;
/*
- * New LL entries are appended at ll_head. Entries between ll_end
- * and ll_head, modulo the LL ring, are owned by DMA; the rest are
- * owned by software.
+ * New LL entries are appended at ll_head. Entries between ll_done and
+ * ll_head, modulo the LL ring, are owned by DMA; the rest are available
+ * to software.
*
* software-owned DMA-owned software-owned
* +---------------+-------------------+---------------+
* ^ ^ ^
- * 0 ll_end ll_head
+ * 0 ll_done ll_head
*
- * The link entry points back to the region start. ll_head == ll_end
- * means all entries are software-owned and previous DMA work is
- * done.
+ * The link entry points back to the region start. No DMA-owned entries
+ * remain once ll_done catches up with ll_head.
*
* Software always keeps at least one free entry, so the ring is
- * never completely DMA-owned.
+ * never completely DMA-owned. That keeps a hardware-reported physical
+ * LL index unique within the current ll_done..ll_head producer window.
*/
u32 ll_head;
- u32 ll_end;
+ u32 ll_done;
+
+ /*
+ * LL progress boundary sampled by the hard IRQ handler after clearing
+ * the DONE/watermark status it accompanies. It is normalized to the
+ * exclusive convention used by ll_done. The sample and all later
+ * accesses are serialized by vc.lock; -1 means consumed or invalid.
+ * ll_irq_stopped records whether the same event found the channel
+ * stopped.
+ */
+ int ll_irq_idx;
+ bool ll_irq_stopped;
u32 ll_max; /* Data entries */
struct dw_edma_region ll_region; /* Linked list */
@@ -146,7 +157,7 @@ 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 *, bool);
struct dw_edma_core_ops {
void (*off)(struct dw_edma *dw);
@@ -154,6 +165,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 done, dw_edma_handler_t abort);
void (*non_ll_start)(struct dw_edma_chan *chan, struct dw_edma_burst *child);
diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
index 62141aa32e50..3b69fa26bf1c 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;
@@ -375,7 +380,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);
+ done(chan, false);
ret = IRQ_HANDLED;
}
@@ -389,7 +394,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);
+ abort(chan, false);
ret = IRQ_HANDLED;
}
@@ -689,6 +694,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 fb651a83f0f3..4440e3ebaeac 100644
--- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
@@ -203,14 +203,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);
+ done(chan, true);
ret = IRQ_HANDLED;
}
if (FIELD_GET(HDMA_V0_ABORT_INT_MASK, val)) {
dw_hdma_v0_core_clear_abort_int(chan);
- abort(chan);
+ abort(chan, false);
ret = IRQ_HANDLED;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v2 09/19] dmaengine: dw-edma: Reclaim issued descriptors from IRQ-paired LL progress
2026-07-23 8:41 ` [PATCH v2 09/19] dmaengine: dw-edma: Reclaim issued descriptors from IRQ-paired LL progress Koichiro Den
@ 2026-07-23 9:01 ` sashiko-bot
0 siblings, 0 replies; 37+ messages in thread
From: sashiko-bot @ 2026-07-23 9:01 UTC (permalink / raw)
To: Koichiro Den; +Cc: vkoul, Frank.Li, dmaengine
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Processing EDMA_REQ_STOP unconditionally terminates descriptors and marks the channel IDLE on any interrupt, leaving the hardware running against freed memory.
- [High] Hardware abort interrupts fail to reset the continuous ring buffer state, permanently deadlocking the DMA channel for subsequent descriptors.
--
--- Patch [9]: [PATCH v2 09/19] dmaengine: dw-edma: Reclaim issued descriptors from IRQ-paired LL progress ---
commit b319de2e00cdfc3228e59ce503f4609c77c06d4b
Author: Koichiro Den <den@valinux.co.jp>
Date: Thu Jul 23 08:41:40 2026 +0000
dmaengine: dw-edma: Reclaim issued descriptors from IRQ-paired LL progress
Dynamic append can put entries from several descriptors in the ring.
Track the consumed boundary in ll_done, reuse consumed entries, and
complete descriptors in issued order. Keep one data entry free so a
physical index is unambiguous within the active producer window.
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index e88bfb417ad4a..44a0ff906f2f7 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
> @@ -726,14 +890,15 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> break;
> }
>
> - chan->status = dw_edma_start_transfer(chan) ? EDMA_ST_BUSY : EDMA_ST_IDLE;
> + active = false;
> + if (vd)
> + active = dw_edma_start_transfer(chan);
> + if (!chan->non_ll)
> + active = dw_edma_ll_pending(chan);
> + chan->status = active ? EDMA_ST_BUSY : EDMA_ST_IDLE;
> break;
>
> case EDMA_REQ_STOP:
> - vd = vchan_next_desc(&chan->vc);
> - if (!vd)
> - break;
> -
> dw_edma_terminate_all_descs(chan);
> chan->request = EDMA_REQ_NONE;
> chan->status = EDMA_ST_IDLE;
> break;
[Severity: Critical]
If a client calls dmaengine_terminate_all(), it sets chan->request to
EDMA_REQ_STOP without immediately writing a hardware stop command.
If the DMA engine then fires a progress interrupt, such as a periodic
watermark or eDMA block completion, before it has actually stopped, does
this unconditionally terminate all descriptors and mark the channel IDLE?
If the client sees the termination complete and frees the DMA buffers, but
the hardware is still executing the remainder of the continuous circular
ring buffer against the freed memory, could this lead to a use-after-free
and IOMMU faults?
[ ... ]
> @@ -789,12 +954,34 @@ static void dw_edma_queue_irq_work(struct dw_edma_chan *chan,
> queue_work(chan->dw->wq, &chan->irq_work);
> }
>
[ ... ]
> -static void dw_edma_abort_interrupt_deferred(struct dw_edma_chan *chan)
> +static void dw_edma_abort_interrupt_deferred(struct dw_edma_chan *chan,
> + bool stopped)
> {
> dw_edma_queue_irq_work(chan, DW_EDMA_IRQ_ABORT);
> }
[Severity: High]
Does dw_edma_abort_interrupt() completely reset the continuous ring buffer
state when handling a hardware abort?
If the hardware encounters a bus error and fires an ABORT interrupt, it
appears dw_edma_abort_interrupt() pops a single descriptor and sets the
channel to IDLE, but fails to clear chan->ll_valid or flush the remaining
published descriptors.
When the next transfer is submitted, would dw_edma_start_transfer() skip
resetting and re-enabling the channel hardware (via
dw_edma_core_ch_enable) because ll_valid is still true?
Could this leave the hardware permanently halted while software blindly
appends to the stale ring?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260723084150.521366-1-den@valinux.co.jp?part=9
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v2 10/19] dmaengine: dw-edma: Use HDMA watermarks as progress events
2026-07-23 8:41 [PATCH v2 00/19] dmaengine: dw-edma: Support dynamic LL appends Koichiro Den
` (8 preceding siblings ...)
2026-07-23 8:41 ` [PATCH v2 09/19] dmaengine: dw-edma: Reclaim issued descriptors from IRQ-paired LL progress Koichiro Den
@ 2026-07-23 8:41 ` Koichiro Den
2026-07-23 8:41 ` [PATCH v2 11/19] dmaengine: dw-edma: Reconcile lost completions from a stopped LLP re-sample Koichiro Den
` (8 subsequent siblings)
18 siblings, 0 replies; 37+ messages in thread
From: Koichiro Den @ 2026-07-23 8:41 UTC (permalink / raw)
To: Manivannan Sadhasivam, Vinod Koul, Frank Li, Gustavo Pimentel,
Kees Cook, Krzysztof Wilczyński, Kishon Vijay Abraham I,
Bjorn Helgaas, Christoph Hellwig, Serge Semin, Cai Huoqing,
Niklas Cassel
Cc: Devendra K Verma, dmaengine, linux-kernel
HDMA reports running LL progress through WATERMARK and channel stop
through STOP. Wire watermark setup, clearing, MSI programming, and
LWIE/RWIE into the core interrupt path. Route WATERMARK to the common
progress handler; STOP remains the done event.
DWC PCIe Controller Databook 6.10a-lca06, Section 7.2.1, permits recycling
LLEs through the watermark LLP. Testing on a 6.30a integration found that
boundary unsafe for DMAengine completion: clients released mappings while
hardware still accessed them. Keep completion one entry behind the raw
LLP. Once the channel has stopped, LLP points to the next entry and needs
no such margin.
Leave EDMA_REQ_STOP and EDMA_REQ_PAUSE to the STOP path instead of
handling their watermark as normal running progress. The eDMA path is
unchanged.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
- Keep completion one entry behind the raw LLP; v1 used the reported
boundary.
- Fold watermark masks into dw_hdma_v0_core_int_setup() while keeping
v1's local and remote routing.
- Use the deferred per-channel IRQ work from the groundwork series.
- Mask local watermark interrupts only for remote LL channels, and
document the STOP/ABORT and WATERMARK distinction.
drivers/dma/dw-edma/dw-edma-core.c | 32 ++++++++++-
drivers/dma/dw-edma/dw-edma-core.h | 9 ++--
drivers/dma/dw-edma/dw-edma-v0-core.c | 4 +-
drivers/dma/dw-edma/dw-hdma-v0-core.c | 76 ++++++++++++++++++++++-----
drivers/dma/dw-edma/dw-hdma-v0-regs.h | 1 +
5 files changed, 104 insertions(+), 18 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 44a0ff906f2f..c6c2655c6c43 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -32,7 +32,8 @@ struct dw_edma_desc *vd2dw_edma_desc(struct virt_dma_desc *vd)
enum dw_edma_irq_event {
DW_EDMA_IRQ_DONE = BIT(0),
- DW_EDMA_IRQ_ABORT = BIT(1),
+ DW_EDMA_IRQ_PROGRESS = BIT(1),
+ DW_EDMA_IRQ_ABORT = BIT(2),
};
static inline
@@ -912,6 +913,24 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
spin_unlock_irqrestore(&chan->vc.lock, flags);
}
+static void dw_edma_progress_interrupt(struct dw_edma_chan *chan)
+{
+ if (chan->non_ll)
+ return;
+
+ guard(spinlock_irqsave)(&chan->vc.lock);
+
+ if (chan->request == EDMA_REQ_NONE && chan->status != EDMA_ST_PAUSE) {
+ if (dw_edma_ll_consume_progress(chan))
+ dw_edma_start_transfer(chan);
+ chan->status = dw_edma_ll_pending(chan) ?
+ EDMA_ST_BUSY : EDMA_ST_IDLE;
+ }
+
+ /* The channel may have stopped after the progress point was sampled. */
+ dw_edma_core_ch_maybe_doorbell(chan);
+}
+
static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
{
struct virt_dma_desc *vd;
@@ -942,6 +961,8 @@ static void dw_edma_irq_work(struct work_struct *work)
if (events & DW_EDMA_IRQ_DONE)
dw_edma_done_interrupt(chan);
+ if (events & DW_EDMA_IRQ_PROGRESS)
+ dw_edma_progress_interrupt(chan);
if (events & DW_EDMA_IRQ_ABORT)
dw_edma_abort_interrupt(chan);
} while (atomic_read(&chan->irq_pending));
@@ -980,6 +1001,13 @@ static void dw_edma_done_interrupt_deferred(struct dw_edma_chan *chan,
dw_edma_queue_irq_work(chan, DW_EDMA_IRQ_DONE);
}
+static void dw_edma_progress_interrupt_deferred(struct dw_edma_chan *chan,
+ bool stopped)
+{
+ dw_edma_record_irq_idx(chan, stopped);
+ dw_edma_queue_irq_work(chan, DW_EDMA_IRQ_PROGRESS);
+}
+
static void dw_edma_abort_interrupt_deferred(struct dw_edma_chan *chan,
bool stopped)
{
@@ -1084,6 +1112,7 @@ static inline irqreturn_t dw_edma_interrupt_write_inner(int irq, void *data)
return dw_edma_core_handle_int(dw_irq, EDMA_DIR_WRITE,
dw_edma_done_interrupt_deferred,
+ dw_edma_progress_interrupt_deferred,
dw_edma_abort_interrupt_deferred);
}
@@ -1093,6 +1122,7 @@ static inline irqreturn_t dw_edma_interrupt_read_inner(int irq, void *data)
return dw_edma_core_handle_int(dw_irq, EDMA_DIR_READ,
dw_edma_done_interrupt_deferred,
+ dw_edma_progress_interrupt_deferred,
dw_edma_abort_interrupt_deferred);
}
diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index 90dcbe415f57..d074506f63ed 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -167,7 +167,9 @@ struct dw_edma_core_ops {
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 done, dw_edma_handler_t abort);
+ dw_edma_handler_t done,
+ dw_edma_handler_t progress,
+ dw_edma_handler_t abort);
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);
@@ -268,9 +270,10 @@ 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 done, dw_edma_handler_t progress,
+ dw_edma_handler_t abort)
{
- return dw_irq->dw->core->handle_int(dw_irq, dir, done, abort);
+ return dw_irq->dw->core->handle_int(dw_irq, dir, done, progress, abort);
}
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 3b69fa26bf1c..42036239ba79 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
@@ -341,7 +341,9 @@ 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 done,
+ dw_edma_handler_t progress,
+ dw_edma_handler_t abort)
{
struct dw_edma *dw = dw_irq->dw;
unsigned long total, pos, val;
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
index 4440e3ebaeac..535832e1ed0d 100644
--- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
@@ -56,7 +56,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
@@ -66,10 +67,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;
}
@@ -79,10 +84,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)
@@ -93,18 +100,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);
}
}
@@ -161,6 +176,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;
@@ -177,7 +200,9 @@ 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 done,
+ dw_edma_handler_t progress,
+ dw_edma_handler_t abort)
{
struct dw_edma *dw = dw_irq->dw;
unsigned long total, pos, val;
@@ -196,18 +221,31 @@ dw_hdma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
}
for_each_set_bit(pos, mask, total) {
+ bool has_stop, has_watermark;
+
chan = &dw->chan[pos + off];
if (unlikely(dw_edma_core_ch_ignore_irq(chan)))
continue;
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, true);
+ has_stop = FIELD_GET(HDMA_V0_STOP_INT_MASK, val);
+ has_watermark = FIELD_GET(HDMA_V0_WATERMARK_INT_MASK, val);
+ if (has_watermark) {
+ dw_hdma_v0_core_clear_watermark_int(chan);
ret = IRQ_HANDLED;
}
+ if (has_stop) {
+ dw_hdma_v0_core_clear_done_int(chan);
+ ret = IRQ_HANDLED;
+ }
+
+ if (has_stop)
+ done(chan, true);
+ else if (has_watermark)
+ progress(chan, false);
+
if (FIELD_GET(HDMA_V0_ABORT_INT_MASK, val)) {
dw_hdma_v0_core_clear_abort_int(chan);
abort(chan, false);
@@ -268,7 +306,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);
@@ -343,6 +381,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);
@@ -362,6 +405,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 2124c162a62f..60dc054adafa 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.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v2 11/19] dmaengine: dw-edma: Reconcile lost completions from a stopped LLP re-sample
2026-07-23 8:41 [PATCH v2 00/19] dmaengine: dw-edma: Support dynamic LL appends Koichiro Den
` (9 preceding siblings ...)
2026-07-23 8:41 ` [PATCH v2 10/19] dmaengine: dw-edma: Use HDMA watermarks as progress events Koichiro Den
@ 2026-07-23 8:41 ` Koichiro Den
2026-07-23 8:41 ` [PATCH v2 12/19] dmaengine: dw-edma: Recover stopped channels from tx_status() Koichiro Den
` (7 subsequent siblings)
18 siblings, 0 replies; 37+ messages in thread
From: Koichiro Den @ 2026-07-23 8:41 UTC (permalink / raw)
To: Manivannan Sadhasivam, Vinod Koul, Frank Li, Gustavo Pimentel,
Kees Cook, Krzysztof Wilczyński, Kishon Vijay Abraham I,
Bjorn Helgaas, Christoph Hellwig, Serge Semin, Cai Huoqing,
Niklas Cassel
Cc: Devendra K Verma, dmaengine, linux-kernel
The IRQ-time DMA_LLP read can race the final internal LLP update and leave
the recorded boundary one entry short. No later DONE may arrive to report
that progress.
Before re-kicking a stopped channel, read LLP again. Legacy eDMA requires
STOPPED status and zero transfer size; native HDMA has a dedicated STOP
event. If the boundary advanced, consume the progress and refill the ring
before deciding whether another kick is needed.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
- New patch.
drivers/dma/dw-edma/dw-edma-core.c | 45 ++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index c6c2655c6c43..9489ca7e70fd 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -396,6 +396,47 @@ static bool dw_edma_ll_consume_progress(struct dw_edma_chan *chan)
return true;
}
+static bool dw_edma_ll_has_hdma_stop_event(struct dw_edma_chan *chan)
+{
+ return chan->dw->chip->mf == EDMA_MF_HDMA_NATIVE;
+}
+
+/*
+ * Must be called with vc.lock held. A DONE-time DMA_LLP sample may miss
+ * the final burst element. For the legacy interrupt interface, accept a
+ * fresh LLP sample only when status is STOPPED and transfer size is zero.
+ * Native HDMA reports STOP directly.
+ */
+static bool dw_edma_ll_reconcile_and_refill(struct dw_edma_chan *chan)
+{
+ u32 old_done;
+ int idx;
+
+ if (dw_edma_core_ch_status(chan) != DMA_COMPLETE)
+ return false;
+
+ /* Native HDMA reports STOP directly, without a transfer-size check. */
+ if (!dw_edma_ll_has_hdma_stop_event(chan) &&
+ dw_edma_core_ch_transfer_size(chan) != 0)
+ return false;
+
+ idx = dw_edma_ll_recycle_idx(chan, dw_edma_core_ll_cur_idx(chan),
+ true);
+ if (idx < 0)
+ return false;
+
+ if (!dw_edma_ll_advance(chan, idx, &old_done))
+ return false;
+
+ dw_edma_ll_irq_idx_discard(chan);
+ dw_edma_ll_clean_pending(chan, old_done);
+ dw_edma_start_transfer(chan);
+ chan->status = dw_edma_ll_pending(chan) ?
+ EDMA_ST_BUSY : EDMA_ST_IDLE;
+
+ return true;
+}
+
/* Must be called with vc.lock held. */
static void dw_edma_core_ch_maybe_doorbell(struct dw_edma_chan *chan)
{
@@ -410,6 +451,10 @@ static void dw_edma_core_ch_maybe_doorbell(struct dw_edma_chan *chan)
if (dw_edma_core_ch_status(chan) == DMA_IN_PROGRESS)
return;
+ dw_edma_ll_reconcile_and_refill(chan);
+ if (!dw_edma_ll_pending(chan))
+ return;
+
dw_edma_core_ch_doorbell(chan);
}
--
2.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v2 12/19] dmaengine: dw-edma: Recover stopped channels from tx_status()
2026-07-23 8:41 [PATCH v2 00/19] dmaengine: dw-edma: Support dynamic LL appends Koichiro Den
` (10 preceding siblings ...)
2026-07-23 8:41 ` [PATCH v2 11/19] dmaengine: dw-edma: Reconcile lost completions from a stopped LLP re-sample Koichiro Den
@ 2026-07-23 8:41 ` Koichiro Den
2026-07-23 8:59 ` sashiko-bot
2026-07-23 8:41 ` [PATCH v2 13/19] dmaengine: dw-edma: Make the LL ring reset a full channel resync Koichiro Den
` (6 subsequent siblings)
18 siblings, 1 reply; 37+ messages in thread
From: Koichiro Den @ 2026-07-23 8:41 UTC (permalink / raw)
To: Manivannan Sadhasivam, Vinod Koul, Frank Li, Gustavo Pimentel,
Kees Cook, Krzysztof Wilczyński, Kishon Vijay Abraham I,
Bjorn Helgaas, Christoph Hellwig, Serge Semin, Cai Huoqing,
Niklas Cassel
Cc: Devendra K Verma, dmaengine, linux-kernel
An LL channel can stop after an append without another interrupt. A
synchronous user may then time out with work still pending.
Let tx_status() consume IRQ-recorded progress under vc.lock. If the
channel is stopped with pending entries, reconcile a fresh LLP boundary
and re-kick it. Do not use a running LLP read as progress because it can
move ahead of payload completion.
Refill the ring after consuming progress and update BUSY or IDLE from the
remaining entries.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
- Replace v1's raw running LLP poll with IRQ-recorded progress and the
common stopped-channel reconciliation.
- Reuse one channel-status sample for the stopped-pending checks.
- Refresh BUSY or IDLE after consuming progress.
drivers/dma/dw-edma/dw-edma-core.c | 38 ++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 9489ca7e70fd..3fc929a6bcf5 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -192,6 +192,14 @@ static bool dw_edma_ll_irq_is_stopped(struct dw_edma_chan *chan, bool stopped)
dw_edma_core_ch_transfer_size(chan) == 0;
}
+static bool dw_edma_ll_recoverable_pending(struct dw_edma_chan *chan,
+ enum dma_status ch_status)
+{
+ return chan->request == EDMA_REQ_NONE &&
+ chan->status != EDMA_ST_PAUSE &&
+ dw_edma_ll_pending(chan) && ch_status == DMA_COMPLETE;
+}
+
static void dw_edma_core_ll_start(struct dw_edma_desc *desc)
{
struct dw_edma_chan *chan = desc->chan;
@@ -654,6 +662,36 @@ dw_edma_device_tx_status(struct dma_chan *dchan, dma_cookie_t cookie,
if (ret == DMA_COMPLETE)
return ret;
+ if (!chan->non_ll) {
+ scoped_guard(spinlock_irqsave, &chan->vc.lock) {
+ enum dma_status ch_status;
+
+ ch_status = dw_edma_core_ch_status(chan);
+
+ /*
+ * Do not treat raw DMA_LLP polling as normal progress here. Only
+ * recycle progress recorded by the IRQ pass that consumed the
+ * status event.
+ */
+ if (chan->request == EDMA_REQ_NONE &&
+ chan->status != EDMA_ST_PAUSE &&
+ ch_status == DMA_COMPLETE &&
+ dw_edma_ll_consume_progress(chan)) {
+ dw_edma_start_transfer(chan);
+ chan->status = dw_edma_ll_pending(chan) ?
+ EDMA_ST_BUSY : EDMA_ST_IDLE;
+ }
+
+ if (dw_edma_ll_recoverable_pending(chan, ch_status))
+ chan->status = EDMA_ST_BUSY;
+ dw_edma_core_ch_maybe_doorbell(chan);
+ }
+
+ /* dw_edma_ll_clean_pending() may have completed this cookie. */
+ ret = dma_cookie_status(dchan, cookie, txstate);
+ if (ret == DMA_COMPLETE)
+ return ret;
+ }
if (ret == DMA_IN_PROGRESS && chan->status == EDMA_ST_PAUSE)
ret = DMA_PAUSED;
--
2.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v2 12/19] dmaengine: dw-edma: Recover stopped channels from tx_status()
2026-07-23 8:41 ` [PATCH v2 12/19] dmaengine: dw-edma: Recover stopped channels from tx_status() Koichiro Den
@ 2026-07-23 8:59 ` sashiko-bot
0 siblings, 0 replies; 37+ messages in thread
From: sashiko-bot @ 2026-07-23 8:59 UTC (permalink / raw)
To: Koichiro Den; +Cc: Frank.Li, vkoul, dmaengine
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Polling and restarting a stopped channel in tx_status races with the hardware STOP interrupt handler, causing the newly running transfer to be misinterpreted as stopped, leading to premature completion and data corruption.
--
--- Patch [12]: [PATCH v2 12/19] dmaengine: dw-edma: Recover stopped channels from tx_status() ---
commit b34ccd5c57949fabf1e4daf7d4e7cfdaa0576cc4
Author: Koichiro Den <den@valinux.co.jp>
dmaengine: dw-edma: Recover stopped channels from tx_status()
This commit allows tx_status() to consume IRQ-recorded progress under
vc.lock. If a channel is stopped with pending entries, it reconciles a
fresh LLP boundary and restarts the channel.
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 9489ca7e70fda..3fc929a6bcf52 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
> @@ -654,6 +662,36 @@ dw_edma_device_tx_status(struct dma_chan *dchan, dma_cookie_t cookie,
> if (ret == DMA_COMPLETE)
> return ret;
>
> + if (!chan->non_ll) {
> + scoped_guard(spinlock_irqsave, &chan->vc.lock) {
> + enum dma_status ch_status;
> +
> + ch_status = dw_edma_core_ch_status(chan);
> +
> + /*
> + * Do not treat raw DMA_LLP polling as normal progress here. Only
> + * recycle progress recorded by the IRQ pass that consumed the
> + * status event.
> + */
> + if (chan->request == EDMA_REQ_NONE &&
> + chan->status != EDMA_ST_PAUSE &&
> + ch_status == DMA_COMPLETE &&
> + dw_edma_ll_consume_progress(chan)) {
> + dw_edma_start_transfer(chan);
> + chan->status = dw_edma_ll_pending(chan) ?
> + EDMA_ST_BUSY : EDMA_ST_IDLE;
> + }
> +
> + if (dw_edma_ll_recoverable_pending(chan, ch_status))
> + chan->status = EDMA_ST_BUSY;
> + dw_edma_core_ch_maybe_doorbell(chan);
[Severity: Critical]
Can this race with the hard IRQ handler and cause data corruption?
If a transfer completes and the hardware stops, the hard IRQ handler
(e.g., dw_hdma_v0_core_handle_int()) will read and clear the STOP status,
then call done(chan, true) which queues the deferred handler with stopped=true.
Before the deferred handler acquires chan->vc.lock, could a client polling
dw_edma_device_tx_status() acquire the lock, observe DMA_COMPLETE, and restart
the channel by hitting the doorbell here?
If so, when the deferred handler finally acquires the lock in
dw_edma_record_irq_idx(), it samples the LLP which is now moving since the
channel has been restarted. Because it uses the stale stopped=true flag,
dw_edma_ll_recycle_idx() returns the exact moving LLP index without applying
the idx - 1 safety margin.
Could this cause descriptors to be completed prematurely while the hardware is
still actively transferring data?
> + }
> +
> + /* dw_edma_ll_clean_pending() may have completed this cookie. */
> + ret = dma_cookie_status(dchan, cookie, txstate);
> + if (ret == DMA_COMPLETE)
> + return ret;
> + }
> if (ret == DMA_IN_PROGRESS && chan->status == EDMA_ST_PAUSE)
> ret = DMA_PAUSED;
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260723084150.521366-1-den@valinux.co.jp?part=12
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v2 13/19] dmaengine: dw-edma: Make the LL ring reset a full channel resync
2026-07-23 8:41 [PATCH v2 00/19] dmaengine: dw-edma: Support dynamic LL appends Koichiro Den
` (11 preceding siblings ...)
2026-07-23 8:41 ` [PATCH v2 12/19] dmaengine: dw-edma: Recover stopped channels from tx_status() Koichiro Den
@ 2026-07-23 8:41 ` Koichiro Den
2026-07-23 9:10 ` sashiko-bot
2026-07-23 8:41 ` [PATCH v2 14/19] dmaengine: dw-edma: Reset LL state after terminate and abort Koichiro Den
` (5 subsequent siblings)
18 siblings, 1 reply; 37+ messages in thread
From: Koichiro Den @ 2026-07-23 8:41 UTC (permalink / raw)
To: Manivannan Sadhasivam, Vinod Koul, Frank Li, Gustavo Pimentel,
Kees Cook, Krzysztof Wilczyński, Kishon Vijay Abraham I,
Bjorn Helgaas, Christoph Hellwig, Serge Semin, Cai Huoqing,
Niklas Cassel
Cc: Devendra K Verma, dmaengine, linux-kernel
Reset currently clears the ring but leaves issued descriptors unchanged.
A descriptor with published entries is then stranded because LL progress
can no longer account for those entries.
Keep descriptors with no outstanding published entries so their
unpublished work can use the fresh ring. Abort descriptors with
outstanding entries because their consumption is unknown and replay is
unsafe.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
- New patch.
drivers/dma/dw-edma/dw-edma-core.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 3fc929a6bcf5..963abecdc59e 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -100,8 +100,27 @@ 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;
+ /*
+ * Software cannot tell which published entries completed before the
+ * reset. Replaying one could duplicate a transfer after its target
+ * changed ownership, while keeping descriptors with uncertain entries
+ * would block later in-order completions. Abort descriptors with published
+ * entries; untouched descriptors remain available for republishing.
+ */
+ 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;
dw_edma_ll_irq_idx_discard(chan);
--
2.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v2 13/19] dmaengine: dw-edma: Make the LL ring reset a full channel resync
2026-07-23 8:41 ` [PATCH v2 13/19] dmaengine: dw-edma: Make the LL ring reset a full channel resync Koichiro Den
@ 2026-07-23 9:10 ` sashiko-bot
0 siblings, 0 replies; 37+ messages in thread
From: sashiko-bot @ 2026-07-23 9:10 UTC (permalink / raw)
To: Koichiro Den; +Cc: Frank.Li, vkoul, dmaengine
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Caching `desc` prior to channel reset in `dw_edma_start_transfer()` leads to starting hardware DMA on an aborted and completed descriptor, causing Use-After-Free and Rogue DMA.
--
--- Patch [13]: [PATCH v2 13/19] dmaengine: dw-edma: Make the LL ring reset a full channel resync ---
commit 4087cc9178be36e7f92be792abfbec15027f1b49
Author: Koichiro Den <den@valinux.co.jp>
dmaengine: dw-edma: Make the LL ring reset a full channel resync
Reset currently clears the ring but leaves issued descriptors unchanged.
This patch updates the LL ring reset to abort descriptors with published
entries to prevent unsafe replay and to allow unpublished descriptors to use
the fresh ring.
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 3fc929a6bcf52..963abecdc59ed 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -100,8 +100,27 @@ 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;
>
> + /*
> + * Software cannot tell which published entries completed before the
> + * reset. Replaying one could duplicate a transfer after its target
> + * changed ownership, while keeping descriptors with uncertain entries
> + * would block later in-order completions. Abort descriptors with published
> + * entries; untouched descriptors remain available for republishing.
> + */
> + 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);
[Severity: High]
If this aborts partially issued descriptors by unlinking and completing them,
doesn't it cause a use-after-free when called from dw_edma_start_transfer()?
In dw_edma_start_transfer(), the first descriptor is cached before calling the
reset function:
drivers/dma/dw-edma/dw-edma-core.c:dw_edma_start_transfer() {
...
vd = vchan_next_desc(&chan->vc);
if (!vd)
return 0;
desc = vd2dw_edma_desc(vd);
if (!desc)
return 0;
if (!chan->non_ll && !chan->ll_valid)
dw_edma_core_reset_ll(chan);
dw_edma_core_start(desc);
...
}
Because dw_edma_core_reset_ll() may complete the cached descriptor, the client
could free the DMA buffer and the descriptor while dw_edma_core_start() is
subsequently passing it to the hardware. Can this result in rogue hardware DMA
and memory corruption?
> + }
> +
> chan->ll_head = 0;
> chan->ll_done = 0;
> dw_edma_ll_irq_idx_discard(chan);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260723084150.521366-1-den@valinux.co.jp?part=13
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v2 14/19] dmaengine: dw-edma: Reset LL state after terminate and abort
2026-07-23 8:41 [PATCH v2 00/19] dmaengine: dw-edma: Support dynamic LL appends Koichiro Den
` (12 preceding siblings ...)
2026-07-23 8:41 ` [PATCH v2 13/19] dmaengine: dw-edma: Make the LL ring reset a full channel resync Koichiro Den
@ 2026-07-23 8:41 ` Koichiro Den
2026-07-23 9:14 ` sashiko-bot
2026-07-23 8:41 ` [PATCH v2 15/19] dmaengine: dw-edma: Add engine reset and enable operations Koichiro Den
` (4 subsequent siblings)
18 siblings, 1 reply; 37+ messages in thread
From: Koichiro Den @ 2026-07-23 8:41 UTC (permalink / raw)
To: Manivannan Sadhasivam, Vinod Koul, Frank Li, Gustavo Pimentel,
Kees Cook, Krzysztof Wilczyński, Kishon Vijay Abraham I,
Bjorn Helgaas, Christoph Hellwig, Serge Semin, Cai Huoqing,
Niklas Cassel
Cc: Devendra K Verma, dmaengine, linux-kernel
Termination and abort can leave published entries in the circular ring.
Reset such a ring before reuse, but leave an already clean ring alone.
This avoids clearing the whole ring after every synchronous transfer.
For eDMA LL, neither channel status nor an arbitrary DONE interrupt proves
that hardware stopped. Keep EDMA_REQ_STOP and EDMA_REQ_PAUSE pending while
entries remain. Accept a stopped IRQ sample that observed the request, or
one that immediately preceded it only if the sample drains the ring.
Native HDMA STOP and non-LL completion are unambiguous. An acknowledged
EDMA_ST_PAUSE is also safe to terminate.
When EDMA_REQ_STOP is acknowledged, terminate submitted and issued
descriptors without callbacks and reset the ring. Abort resets it as
well. Both paths leave the channel configured for reuse.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
- Record whether a request was pending with each IRQ progress sample.
- Gate eDMA LL cleanup on a stopped sample; accept an older sample only
if it drains the ring.
- Use immediate teardown for native HDMA's dedicated STOP event.
- Reset only LL rings that still have published entries.
- Preserve the channel configuration after termination, following the
groundwork series' termination contract.
drivers/dma/dw-edma/dw-edma-core.c | 96 ++++++++++++++++++++++--------
drivers/dma/dw-edma/dw-edma-core.h | 4 +-
2 files changed, 75 insertions(+), 25 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 963abecdc59e..9305137f85f6 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -71,6 +71,7 @@ static void dw_edma_ll_irq_idx_discard(struct dw_edma_chan *chan)
{
chan->ll_irq_idx = -1;
chan->ll_irq_stopped = false;
+ chan->ll_irq_requested = false;
}
static void dw_hdma_set_callback_result(struct virt_dma_desc *vd,
@@ -616,36 +617,64 @@ static int dw_edma_device_resume(struct dma_chan *dchan)
return err;
}
+/* Must be called with vc.lock held. */
+static bool dw_edma_may_be_active(struct dw_edma_chan *chan)
+{
+ if (chan->non_ll)
+ return dw_edma_core_ch_status(chan) == DMA_IN_PROGRESS;
+
+ if (!dw_edma_ll_pending(chan))
+ return false;
+
+ /*
+ * Legacy eDMA can report STOPPED while a doorbell-triggered start is
+ * still pending. Native HDMA has a dedicated STOP event.
+ */
+ return !dw_edma_ll_has_hdma_stop_event(chan) ||
+ dw_edma_core_ch_status(chan) == DMA_IN_PROGRESS;
+}
+
+/* Must be called with vc.lock held after the channel has stopped. */
+static void dw_edma_finish_termination(struct dw_edma_chan *chan)
+{
+ dw_edma_terminate_all_descs(chan);
+
+ /* Preserve a clean ring; resync only if entries remain published. */
+ if (!chan->non_ll && dw_edma_ll_pending(chan))
+ dw_edma_core_reset_ll(chan);
+
+ chan->request = EDMA_REQ_NONE;
+ chan->status = EDMA_ST_IDLE;
+}
+
static int dw_edma_device_terminate_all(struct dma_chan *dchan)
{
struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
- int err = 0;
guard(spinlock_irqsave)(&chan->vc.lock);
if (!chan->configured) {
dw_edma_terminate_all_descs(chan);
+ chan->request = EDMA_REQ_NONE;
} else if (chan->status == EDMA_ST_PAUSE) {
- dw_edma_terminate_all_descs(chan);
- chan->status = EDMA_ST_IDLE;
- } else if (chan->status == EDMA_ST_IDLE) {
- dw_edma_terminate_all_descs(chan);
- } else if (dw_edma_core_ch_status(chan) == DMA_COMPLETE) {
+ /* A paused channel has already stopped. */
+ dw_edma_finish_termination(chan);
+ } else if (dw_edma_may_be_active(chan)) {
/*
- * The channel is in a false BUSY state, probably didn't
- * receive or lost an interrupt
+ * Keep published entries intact until an IRQ sample confirms
+ * that the channel stopped. An outstanding EDMA_REQ_PAUSE is
+ * replaced by EDMA_REQ_STOP.
*/
- dw_edma_terminate_all_descs(chan);
- chan->status = EDMA_ST_IDLE;
- } else if (chan->request > EDMA_REQ_PAUSE) {
- err = -EPERM;
- } else {
chan->request = EDMA_REQ_STOP;
+ } else {
+ /*
+ * No LL entry is hardware-owned, or non-LL status confirms that
+ * the one programmed burst is not running.
+ */
+ dw_edma_finish_termination(chan);
}
- if (chan->status == EDMA_ST_IDLE)
- chan->request = EDMA_REQ_NONE;
- return err;
+ return 0;
}
static void dw_edma_device_issue_pending(struct dma_chan *dchan)
@@ -965,17 +994,30 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
bool active;
spin_lock_irqsave(&chan->vc.lock, flags);
- if (chan->status == EDMA_ST_PAUSE) {
- spin_unlock_irqrestore(&chan->vc.lock, flags);
- return;
- }
+ if (chan->status == EDMA_ST_PAUSE)
+ goto out;
+
+ /*
+ * A stopped event sampled after the request can complete it directly.
+ * If it was sampled just before the request, complete the request only
+ * when its boundary drains all published entries. A running event only
+ * advances LL progress.
+ */
+ if (chan->request != EDMA_REQ_NONE && !chan->non_ll &&
+ (!chan->ll_irq_stopped || !chan->ll_irq_requested)) {
+ bool stopped = chan->ll_irq_stopped;
- if (!chan->non_ll)
dw_edma_ll_consume_progress(chan);
+ if (!stopped || dw_edma_ll_pending(chan))
+ goto out;
+ }
switch (chan->request) {
case EDMA_REQ_NONE:
case EDMA_REQ_PAUSE:
+ if (!chan->non_ll)
+ dw_edma_ll_consume_progress(chan);
+
vd = vchan_next_desc(&chan->vc);
if (vd && chan->non_ll) {
desc = vd2dw_edma_desc(vd);
@@ -988,6 +1030,8 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
}
if (chan->request == EDMA_REQ_PAUSE) {
+ if (!chan->non_ll)
+ dw_edma_ll_irq_idx_discard(chan);
chan->request = EDMA_REQ_NONE;
chan->status = EDMA_ST_PAUSE;
break;
@@ -1002,14 +1046,14 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
break;
case EDMA_REQ_STOP:
- dw_edma_terminate_all_descs(chan);
- chan->request = EDMA_REQ_NONE;
- chan->status = EDMA_ST_IDLE;
+ dw_edma_finish_termination(chan);
break;
default:
break;
}
+
+out:
dw_edma_core_ch_maybe_doorbell(chan);
spin_unlock_irqrestore(&chan->vc.lock, flags);
@@ -1047,6 +1091,8 @@ static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
list_del(&vd->node);
vchan_cookie_complete(vd);
}
+ if (!chan->non_ll)
+ dw_edma_core_reset_ll(chan);
chan->request = EDMA_REQ_NONE;
chan->status = EDMA_ST_IDLE;
spin_unlock_irqrestore(&chan->vc.lock, flags);
@@ -1091,6 +1137,8 @@ static void dw_edma_record_irq_idx(struct dw_edma_chan *chan, bool stopped)
if (idx >= 0) {
chan->ll_irq_idx = idx;
chan->ll_irq_stopped = stopped;
+ chan->ll_irq_requested =
+ chan->request != EDMA_REQ_NONE;
}
}
}
diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index d074506f63ed..d235046ccb90 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -97,10 +97,12 @@ struct dw_edma_chan {
* exclusive convention used by ll_done. The sample and all later
* accesses are serialized by vc.lock; -1 means consumed or invalid.
* ll_irq_stopped records whether the same event found the channel
- * stopped.
+ * stopped. ll_irq_requested records whether EDMA_REQ_STOP or
+ * EDMA_REQ_PAUSE was already pending at sample time.
*/
int ll_irq_idx;
bool ll_irq_stopped;
+ bool ll_irq_requested;
u32 ll_max; /* Data entries */
struct dw_edma_region ll_region; /* Linked list */
--
2.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v2 14/19] dmaengine: dw-edma: Reset LL state after terminate and abort
2026-07-23 8:41 ` [PATCH v2 14/19] dmaengine: dw-edma: Reset LL state after terminate and abort Koichiro Den
@ 2026-07-23 9:14 ` sashiko-bot
0 siblings, 0 replies; 37+ messages in thread
From: sashiko-bot @ 2026-07-23 9:14 UTC (permalink / raw)
To: Koichiro Den; +Cc: Frank.Li, vkoul, dmaengine
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] Deferring `EDMA_REQ_STOP` for Legacy eDMA when `ll_pending > 0` causes a permanent channel stall if the hardware has already stopped without a pending doorbell.
Pre-existing issues:
- [High] Virtual descriptors are permanently leaked during channel termination because `dw_edma_terminate_vdesc` removes them from the tracking lists without ever freeing their backing memory.
--
--- Patch [14]: [PATCH v2 14/19] dmaengine: dw-edma: Reset LL state after terminate and abort ---
commit 88e57ef5a9d3ec5120e32fad81665662de2e275a
Author: Koichiro Den <den@valinux.co.jp>
dmaengine: dw-edma: Reset LL state after terminate and abort
Termination and abort can leave published entries in the circular ring.
Reset such a ring before reuse, but leave an already clean ring alone.
This avoids clearing the whole ring after every synchronous transfer.
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 963abecdc59ed..9305137f85f67 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
> @@ -616,36 +617,64 @@ static int dw_edma_device_resume(struct dma_chan *dchan)
> return err;
> }
>
> +/* Must be called with vc.lock held. */
> +static bool dw_edma_may_be_active(struct dw_edma_chan *chan)
> +{
> + if (chan->non_ll)
> + return dw_edma_core_ch_status(chan) == DMA_IN_PROGRESS;
> +
> + if (!dw_edma_ll_pending(chan))
> + return false;
> +
> + /*
> + * Legacy eDMA can report STOPPED while a doorbell-triggered start is
> + * still pending. Native HDMA has a dedicated STOP event.
> + */
> + return !dw_edma_ll_has_hdma_stop_event(chan) ||
> + dw_edma_core_ch_status(chan) == DMA_IN_PROGRESS;
> +}
> +
> +/* Must be called with vc.lock held after the channel has stopped. */
> +static void dw_edma_finish_termination(struct dw_edma_chan *chan)
> +{
> + dw_edma_terminate_all_descs(chan);
[Severity: High]
This is a pre-existing issue, but does dw_edma_terminate_all_descs() leak
virtual descriptors during channel termination?
Looking at the termination path, dw_edma_terminate_all_descs() uses
dw_edma_terminate_vdesc_list() to move descriptors to the desc_terminated list:
drivers/dma/dw-edma/dw-edma-core.c:dw_edma_terminate_all_descs() {
dw_edma_terminate_vdesc_list(&chan->vc.desc_issued);
dw_edma_terminate_vdesc_list(&chan->vc.desc_submitted);
}
However, there appears to be no call to vchan_dma_desc_free_list() within the
termination path.
Are these descriptors permanently leaked until the entire channel is released
via free_chan_resources(), potentially exhausting system memory if triggered
repeatedly by userspace?
> +
> + /* Preserve a clean ring; resync only if entries remain published. */
> + if (!chan->non_ll && dw_edma_ll_pending(chan))
> + dw_edma_core_reset_ll(chan);
> +
> + chan->request = EDMA_REQ_NONE;
> + chan->status = EDMA_ST_IDLE;
> +}
[ ... ]
> @@ -965,17 +994,30 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> bool active;
>
> spin_lock_irqsave(&chan->vc.lock, flags);
> - if (chan->status == EDMA_ST_PAUSE) {
> - spin_unlock_irqrestore(&chan->vc.lock, flags);
> - return;
> - }
> + if (chan->status == EDMA_ST_PAUSE)
> + goto out;
> +
> + /*
> + * A stopped event sampled after the request can complete it directly.
> + * If it was sampled just before the request, complete the request only
> + * when its boundary drains all published entries. A running event only
> + * advances LL progress.
> + */
> + if (chan->request != EDMA_REQ_NONE && !chan->non_ll &&
> + (!chan->ll_irq_stopped || !chan->ll_irq_requested)) {
> + bool stopped = chan->ll_irq_stopped;
>
> - if (!chan->non_ll)
> dw_edma_ll_consume_progress(chan);
> + if (!stopped || dw_edma_ll_pending(chan))
> + goto out;
[Severity: High]
Can this cause a permanent channel stall for Legacy eDMA when descriptors
are pending?
If dw_edma_device_terminate_all() is called while ll_pending > 0, it defers
termination by setting EDMA_REQ_STOP and waits for a DONE IRQ.
If the hardware has already stopped (e.g., reaching the end of the previous
chain) and the doorbell was bypassed, the DONE IRQ handler processes completed
elements but sees ll_pending > 0. It takes the goto out path:
drivers/dma/dw-edma/dw-edma-core.c:dw_edma_done_interrupt() {
if (!stopped || dw_edma_ll_pending(chan))
goto out;
...
out:
dw_edma_core_ch_maybe_doorbell(chan);
}
However, dw_edma_core_ch_maybe_doorbell() will refuse to ring the doorbell
because EDMA_REQ_STOP is set:
drivers/dma/dw-edma/dw-edma-core.c:dw_edma_core_ch_maybe_doorbell() {
if (... || chan->request != EDMA_REQ_NONE || ...)
return;
...
}
With the hardware stopped and no doorbell rung, will any further IRQs arrive,
or does this permanently stall the channel?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260723084150.521366-1-den@valinux.co.jp?part=14
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v2 15/19] dmaengine: dw-edma: Add engine reset and enable operations
2026-07-23 8:41 [PATCH v2 00/19] dmaengine: dw-edma: Support dynamic LL appends Koichiro Den
` (13 preceding siblings ...)
2026-07-23 8:41 ` [PATCH v2 14/19] dmaengine: dw-edma: Reset LL state after terminate and abort Koichiro Den
@ 2026-07-23 8:41 ` Koichiro Den
2026-07-23 9:11 ` sashiko-bot
2026-07-23 8:41 ` [PATCH v2 16/19] dmaengine: dw-edma: Add engine recovery infrastructure Koichiro Den
` (3 subsequent siblings)
18 siblings, 1 reply; 37+ messages in thread
From: Koichiro Den @ 2026-07-23 8:41 UTC (permalink / raw)
To: Manivannan Sadhasivam, Vinod Koul, Frank Li, Gustavo Pimentel,
Kees Cook, Krzysztof Wilczyński, Kishon Vijay Abraham I,
Bjorn Helgaas, Christoph Hellwig, Serge Semin, Cai Huoqing,
Niklas Cassel
Cc: Devendra K Verma, dmaengine, linux-kernel
Direction recovery must keep the engine disabled while its configured LL
channel contexts are rebuilt.
Add separate engine_reset and engine_enable operations. For eDMA, reset
clears ENGINE_EN, waits for it to read back zero, then clears latched DONE
and ABORT status. If ENGINE_EN does not clear, dump the state and do not
try to enable it again. A separate enable operation lets the caller
rebuild channel contexts first.
Enable the direction after channel context programming. Keep enable a
no-op in HDMA compatibility mode, which does not expose ENGINE_EN.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
- New patch.
drivers/dma/dw-edma/dw-edma-core.h | 5 +++
drivers/dma/dw-edma/dw-edma-v0-core.c | 63 ++++++++++++++++++++++++++-
2 files changed, 66 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index d235046ccb90..e3b42ad14f78 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -180,6 +180,9 @@ struct dw_edma_core_ops {
int (*ll_cur_idx)(struct dw_edma_chan *chan);
/* Called with chan->vc.lock held. */
bool (*ll_irq)(struct dw_edma_desc *desc, u32 i, u32 free);
+ /* Reset one direction, discard its IRQ status, and leave it disabled. */
+ bool (*engine_reset)(struct dw_edma *dw, enum dw_edma_dir dir);
+ void (*engine_enable)(struct dw_edma *dw, enum dw_edma_dir dir);
void (*ch_doorbell)(struct dw_edma_chan *chan);
void (*ch_enable)(struct dw_edma_chan *chan);
void (*ch_config)(struct dw_edma_chan *chan);
@@ -310,6 +313,8 @@ static inline void dw_edma_core_ch_doorbell(struct dw_edma_chan *chan)
static inline void dw_edma_core_ch_enable(struct dw_edma_chan *chan)
{
chan->dw->core->ch_enable(chan);
+ if (chan->dw->core->engine_enable)
+ chan->dw->core->engine_enable(chan->dw, chan->dir);
}
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 42036239ba79..d834ae1633ef 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
@@ -220,6 +220,65 @@ static int dw_edma_v0_core_engine_disable(struct dw_edma *dw,
return ret;
}
+static void dw_edma_v0_core_engine_dump(struct dw_edma *dw,
+ enum dw_edma_dir dir)
+{
+ u16 cnt = dir == EDMA_DIR_WRITE ? dw->wr_ch_cnt : dw->rd_ch_cnt;
+ u16 i;
+
+ dev_warn(dw->chip->dev, "%s engine: int_status 0x%08x err_status 0x%08x\n",
+ dir == EDMA_DIR_WRITE ? "wr" : "rd",
+ GET_RW_32(dw, dir, int_status),
+ dir == EDMA_DIR_WRITE ? GET_32(dw, wr_err_status) :
+ GET_32(dw, rd_err_status.lsb));
+ for (i = 0; i < cnt; i++)
+ dev_warn(dw->chip->dev,
+ " ch%u: ch_control1 0x%08x transfer_size 0x%08x llp.lsb 0x%08x\n",
+ i, GET_CH_32(dw, dir, i, ch_control1),
+ GET_CH_32(dw, dir, i, transfer_size),
+ GET_CH_32(dw, dir, i, llp.lsb));
+}
+
+static bool dw_edma_v0_core_engine_reset(struct dw_edma *dw,
+ enum dw_edma_dir dir)
+{
+ /*
+ * The databook says clearing ENGINE_EN resets the DMA logic while
+ * preserving configuration registers, but does not document drain
+ * semantics. On the tested integration, ENGINE_EN read back as zero
+ * only after outstanding transactions appeared to drain; the stalled
+ * channel then resumed after reset.
+ *
+ * If ENGINE_EN does not clear, do not re-enable the engine. On the
+ * tested integration, re-enabling after such a timeout wedged the
+ * controller and also blocked inbound BAR accesses. Return failure so
+ * the caller can keep channels gated and retry.
+ */
+ if (dw_edma_v0_core_engine_disable(dw, dir)) {
+ dw_edma_v0_core_engine_dump(dw, dir);
+ return false;
+ }
+
+ /* Discard interrupt status belonging to the contexts being reset. */
+ SET_RW_32(dw, dir, int_clear,
+ EDMA_V0_DONE_INT_MASK | EDMA_V0_ABORT_INT_MASK);
+ GET_RW_32(dw, dir, int_status);
+
+ return true;
+}
+
+/*
+ * Keep enable separate from reset. Re-enabling can resume a previously
+ * running channel from preserved internal state without a doorbell, so the
+ * caller must rebuild every channel context first.
+ */
+static void dw_edma_v0_core_engine_enable(struct dw_edma *dw,
+ enum dw_edma_dir dir)
+{
+ if (dw->chip->mf != EDMA_MF_HDMA_COMPAT)
+ SET_RW_32(dw, dir, engine_en, BIT(0));
+}
+
static int dw_edma_v0_core_dir_off(struct dw_edma *dw, enum dw_edma_dir dir)
{
u16 count, id;
@@ -452,8 +511,6 @@ static void dw_edma_v0_core_ch_enable(struct dw_edma_chan *chan)
unsigned long flags;
u32 tmp;
- /* Enable engine */
- SET_RW_32(dw, chan->dir, engine_en, BIT(0));
if (dw->chip->mf == EDMA_MF_HDMA_COMPAT)
dw_edma_v0_core_ch_power(dw, chan->dir, chan->id, true);
/* Interrupt mask/unmask - done, abort */
@@ -703,6 +760,8 @@ static const struct dw_edma_core_ops dw_edma_v0_core = {
.ll_clear = dw_edma_v0_core_ll_clear,
.ll_cur_idx = dw_edma_v0_core_ll_cur_idx,
.ll_irq = dw_edma_v0_core_ll_irq,
+ .engine_reset = dw_edma_v0_core_engine_reset,
+ .engine_enable = dw_edma_v0_core_engine_enable,
.ch_doorbell = dw_edma_v0_core_ch_doorbell,
.ch_enable = dw_edma_v0_core_ch_enable,
.ch_config = dw_edma_v0_core_ch_config,
--
2.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v2 15/19] dmaengine: dw-edma: Add engine reset and enable operations
2026-07-23 8:41 ` [PATCH v2 15/19] dmaengine: dw-edma: Add engine reset and enable operations Koichiro Den
@ 2026-07-23 9:11 ` sashiko-bot
0 siblings, 0 replies; 37+ messages in thread
From: sashiko-bot @ 2026-07-23 9:11 UTC (permalink / raw)
To: Koichiro Den; +Cc: dmaengine, vkoul, Frank.Li
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] dw_edma_v0_core_engine_reset() fails to handle HDMA compatibility mode, unconditionally accessing the unimplemented ENGINE_EN register and failing to properly stop channels.
--
--- Patch [15]: [PATCH v2 15/19] dmaengine: dw-edma: Add engine reset and enable operations ---
commit 480649ecac905454f1c85b3804e44856e4e73d24
Author: Koichiro Den <den@valinux.co.jp>
dmaengine: dw-edma: Add engine reset and enable operations
This commit adds separate engine_reset and engine_enable operations to allow
rebuilding LL channel contexts while the engine is disabled. Reset clears
ENGINE_EN and waits for it to read back zero, while enable restores the
direction and handles HDMA compatibility mode.
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
[ ... ]
> @@ -220,6 +220,65 @@ static int dw_edma_v0_core_engine_disable(struct dw_edma *dw,
[ ... ]
> +static bool dw_edma_v0_core_engine_reset(struct dw_edma *dw,
> + enum dw_edma_dir dir)
> +{
> + /*
> + * The databook says clearing ENGINE_EN resets the DMA logic while
> + * preserving configuration registers, but does not document drain
> + * semantics. On the tested integration, ENGINE_EN read back as zero
> + * only after outstanding transactions appeared to drain; the stalled
> + * channel then resumed after reset.
> + *
> + * If ENGINE_EN does not clear, do not re-enable the engine. On the
> + * tested integration, re-enabling after such a timeout wedged the
> + * controller and also blocked inbound BAR accesses. Return failure so
> + * the caller can keep channels gated and retry.
> + */
> + if (dw_edma_v0_core_engine_disable(dw, dir)) {
[Severity: High]
Does this function need to handle HDMA compatibility mode similarly to how
it is handled in dw_edma_v0_core_engine_enable() and
dw_edma_v0_core_dir_off()?
Since dw_edma_v0_core_engine_disable() unconditionally accesses the
ENGINE_EN register, wouldn't calling this in EDMA_MF_HDMA_COMPAT mode cause
a 200ms timeout in read_poll_timeout() since that register isn't
implemented?
As noted in the driver comments, failing to clear this register might wedge
the controller and block inbound PCIe BAR accesses. Could this regression be
avoided by bypassing or handling the disable step correctly when in HDMA
mode?
> + dw_edma_v0_core_engine_dump(dw, dir);
> + return false;
> + }
> +
> + /* Discard interrupt status belonging to the contexts being reset. */
> + SET_RW_32(dw, dir, int_clear,
> + EDMA_V0_DONE_INT_MASK | EDMA_V0_ABORT_INT_MASK);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260723084150.521366-1-den@valinux.co.jp?part=15
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v2 16/19] dmaengine: dw-edma: Add engine recovery infrastructure
2026-07-23 8:41 [PATCH v2 00/19] dmaengine: dw-edma: Support dynamic LL appends Koichiro Den
` (14 preceding siblings ...)
2026-07-23 8:41 ` [PATCH v2 15/19] dmaengine: dw-edma: Add engine reset and enable operations Koichiro Den
@ 2026-07-23 8:41 ` Koichiro Den
2026-07-23 9:14 ` sashiko-bot
2026-07-23 8:41 ` [PATCH v2 17/19] dmaengine: dw-edma: Detect and recover a stalled eDMA engine Koichiro Den
` (2 subsequent siblings)
18 siblings, 1 reply; 37+ messages in thread
From: Koichiro Den @ 2026-07-23 8:41 UTC (permalink / raw)
To: Manivannan Sadhasivam, Vinod Koul, Frank Li, Gustavo Pimentel,
Kees Cook, Krzysztof Wilczyński, Kishon Vijay Abraham I,
Bjorn Helgaas, Christoph Hellwig, Serge Semin, Cai Huoqing,
Niklas Cassel
Cc: Devendra K Verma, dmaengine, linux-kernel
Resetting one direction affects every channel in it. Add a worker to
coordinate that recovery.
Gate publication and doorbells, then wait for active LL channels to stop.
If a transfer is still active, compare two transfer-size samples. Retry
when it is moving; treat an unchanged value as frozen.
For a frozen direction, reset the engine, synchronize hard and deferred
IRQ handling, rebuild configured LL contexts while the engine is disabled,
and then enable it. Honor EDMA_REQ_STOP and EDMA_REQ_PAUSE during the
rebuild. After five reset failures, leave the channels gated.
Providers opt in with engine_reset(), engine_enable(), and
ch_transfer_size(). Disable recovery before shutdown. No caller requests
recovery until the next patch.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
- New patch.
drivers/dma/dw-edma/dw-edma-core.c | 291 ++++++++++++++++++++++++++++-
drivers/dma/dw-edma/dw-edma-core.h | 30 ++-
2 files changed, 310 insertions(+), 11 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 9305137f85f6..14cd0038dbd2 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -24,6 +24,10 @@
#include "../dmaengine.h"
#include "../virt-dma.h"
+#define DW_EDMA_ENGINE_QUIESCE_TIMEOUT_MS 250
+#define DW_EDMA_ENGINE_RESET_MAX_FAILS 5
+#define DW_EDMA_MAX_DIR_CH MAX(HDMA_MAX_WR_CH, HDMA_MAX_RD_CH)
+
static inline
struct dw_edma_desc *vd2dw_edma_desc(struct virt_dma_desc *vd)
{
@@ -125,6 +129,7 @@ static void dw_edma_core_reset_ll(struct dw_edma_chan *chan)
chan->ll_head = 0;
chan->ll_done = 0;
dw_edma_ll_irq_idx_discard(chan);
+ chan->ll_recovery_pending = false;
/* Drop stale CB bits before reusing the circular LL ring. */
for (i = 0; i < chan->ll_max; i++)
dw_edma_core_ll_clear(chan, i);
@@ -178,6 +183,7 @@ static bool dw_edma_ll_advance(struct dw_edma_chan *chan, int idx, u32 *old_done
*old_done = chan->ll_done;
chan->ll_done = idx;
+ chan->ll_recovery_pending = false;
return true;
}
@@ -275,6 +281,10 @@ static int dw_edma_start_transfer(struct dw_edma_chan *chan)
struct dw_edma_desc *desc;
struct virt_dma_desc *vd;
+ /* Stop publishing until recovery can rebuild and republish the ring. */
+ if (!chan->non_ll && chan->ll_recovering)
+ return 0;
+
vd = vchan_next_desc(&chan->vc);
if (!vd)
return 0;
@@ -468,8 +478,11 @@ static bool dw_edma_ll_reconcile_and_refill(struct dw_edma_chan *chan)
/* Must be called with vc.lock held. */
static void dw_edma_core_ch_maybe_doorbell(struct dw_edma_chan *chan)
{
- if (chan->non_ll || chan->request != EDMA_REQ_NONE ||
- chan->status != EDMA_ST_BUSY || !dw_edma_ll_pending(chan))
+ if (chan->non_ll ||
+ chan->ll_recovering ||
+ chan->request != EDMA_REQ_NONE ||
+ chan->status != EDMA_ST_BUSY ||
+ !dw_edma_ll_pending(chan))
return;
/*
@@ -486,6 +499,258 @@ static void dw_edma_core_ch_maybe_doorbell(struct dw_edma_chan *chan)
dw_edma_core_ch_doorbell(chan);
}
+static bool
+dw_edma_engine_recovery_needed(struct dw_edma_engine_recovery *rec)
+{
+ struct dw_edma *dw = rec->dw;
+ u16 off = rec->dir == EDMA_DIR_WRITE ? 0 : dw->wr_ch_cnt;
+ u16 cnt = rec->dir == EDMA_DIR_WRITE ? dw->wr_ch_cnt : dw->rd_ch_cnt;
+ struct dw_edma_chan *chan;
+ bool found = false;
+ bool needed;
+ u16 i;
+
+ for (i = 0; i < cnt; i++) {
+ chan = &dw->chan[off + i];
+ scoped_guard(spinlock_irqsave, &chan->vc.lock) {
+ needed = chan->ll_recovery_pending && chan->configured &&
+ !chan->non_ll &&
+ chan->status != EDMA_ST_PAUSE;
+ if (needed && chan->request == EDMA_REQ_NONE &&
+ dw_edma_ll_reconcile_and_refill(chan))
+ dw_edma_core_ch_maybe_doorbell(chan);
+ needed = needed && chan->ll_recovery_pending &&
+ dw_edma_ll_pending(chan);
+ if (!needed)
+ chan->ll_recovery_pending = false;
+ }
+ found |= needed;
+ }
+
+ return found;
+}
+
+static void
+dw_edma_engine_recovery_release(struct dw_edma_engine_recovery *rec)
+{
+ struct dw_edma *dw = rec->dw;
+ u16 off = rec->dir == EDMA_DIR_WRITE ? 0 : dw->wr_ch_cnt;
+ u16 cnt = rec->dir == EDMA_DIR_WRITE ? dw->wr_ch_cnt : dw->rd_ch_cnt;
+ struct dw_edma_chan *chan;
+ u16 i;
+
+ rec->fails = 0;
+
+ for (i = 0; i < cnt; i++) {
+ chan = &dw->chan[off + i];
+ guard(spinlock_irqsave)(&chan->vc.lock);
+ chan->ll_recovering = false;
+
+ if (chan->configured && !chan->non_ll &&
+ chan->request == EDMA_REQ_NONE &&
+ chan->status != EDMA_ST_PAUSE) {
+ dw_edma_start_transfer(chan);
+ chan->status = dw_edma_ll_pending(chan) ?
+ EDMA_ST_BUSY : EDMA_ST_IDLE;
+ dw_edma_core_ch_maybe_doorbell(chan);
+ }
+ }
+
+ WRITE_ONCE(rec->active, false);
+}
+
+static void
+dw_edma_engine_recovery_sync_irqs(struct dw_edma_engine_recovery *rec)
+{
+ struct dw_edma *dw = rec->dw;
+ struct device *dev = dw->chip->dev;
+ u16 off = rec->dir == EDMA_DIR_WRITE ? 0 : dw->wr_ch_cnt;
+ u16 cnt = rec->dir == EDMA_DIR_WRITE ? dw->wr_ch_cnt : dw->rd_ch_cnt;
+ struct dw_edma_chan *chan;
+ u16 i;
+
+ /* Wait until no hard IRQ can queue an event from the old contexts. */
+ for (i = 0; i < dw->nr_irqs; i++) {
+ if (bitmap_empty(rec->dir == EDMA_DIR_WRITE ?
+ dw->irq[i].wr_mask : dw->irq[i].rd_mask, cnt))
+ continue;
+
+ synchronize_irq(dw->chip->ops->irq_vector(dev, i));
+ }
+
+ for (i = 0; i < cnt; i++) {
+ chan = &dw->chan[off + i];
+ cancel_work_sync(&chan->irq_work);
+
+ guard(spinlock_irqsave)(&chan->vc.lock);
+ atomic_set(&chan->irq_pending, 0);
+ dw_edma_ll_irq_idx_discard(chan);
+ }
+}
+
+static void dw_edma_engine_recovery_work(struct work_struct *work)
+{
+ struct dw_edma_engine_recovery *rec =
+ container_of(work, struct dw_edma_engine_recovery, work);
+ struct dw_edma *dw = rec->dw;
+ const char *dir_name = str_write_read(rec->dir == EDMA_DIR_WRITE);
+ u16 off = rec->dir == EDMA_DIR_WRITE ? 0 : dw->wr_ch_cnt;
+ u16 cnt = rec->dir == EDMA_DIR_WRITE ? dw->wr_ch_cnt : dw->rd_ch_cnt;
+ unsigned long timeout =
+ jiffies + msecs_to_jiffies(DW_EDMA_ENGINE_QUIESCE_TIMEOUT_MS);
+ struct dw_edma_chan *chan;
+ bool configured_ll;
+ bool busy;
+ u16 i;
+
+ if (!dw_edma_engine_recovery_needed(rec)) {
+ if (READ_ONCE(rec->active))
+ dw_edma_engine_recovery_release(rec);
+ return;
+ }
+
+ WRITE_ONCE(rec->active, true);
+
+ /* Gate each channel before inspecting or resetting the direction. */
+ for (i = 0; i < cnt; i++) {
+ chan = &dw->chan[off + i];
+ guard(spinlock_irqsave)(&chan->vc.lock);
+ chan->ll_recovering = true;
+ }
+
+ /*
+ * Gated channels stop at their first unpublished element. Wait for that
+ * point before resetting; otherwise a restart could skip the remainder
+ * of an in-flight element.
+ */
+ do {
+ busy = false;
+ for (i = 0; i < cnt; i++) {
+ chan = &dw->chan[off + i];
+ scoped_guard(spinlock_irqsave, &chan->vc.lock)
+ configured_ll = chan->configured && !chan->non_ll;
+ if (configured_ll &&
+ dw_edma_core_ch_status(chan) == DMA_IN_PROGRESS)
+ busy = true;
+ }
+ if (!busy)
+ break;
+ fsleep(1000);
+ } while (time_before(jiffies, timeout));
+
+ /* Progress after queueing makes this recovery request obsolete. */
+ if (!dw_edma_engine_recovery_needed(rec))
+ goto out_release;
+
+ if (busy) {
+ DECLARE_BITMAP(busy_mask, DW_EDMA_MAX_DIR_CH) = { 0 };
+ u32 tsz[DW_EDMA_MAX_DIR_CH];
+ bool moving = false;
+
+ /*
+ * A transfer_size change or a transition out of DMA_IN_PROGRESS
+ * shows movement after the first sample. Retry while producers
+ * remain gated. If neither occurs, treat the channel as frozen
+ * within one element.
+ */
+ for (i = 0; i < cnt; i++) {
+ chan = &dw->chan[off + i];
+ scoped_guard(spinlock_irqsave, &chan->vc.lock)
+ configured_ll = chan->configured && !chan->non_ll;
+ if (configured_ll &&
+ dw_edma_core_ch_status(chan) == DMA_IN_PROGRESS) {
+ __set_bit(i, busy_mask);
+ tsz[i] = dw_edma_core_ch_transfer_size(chan);
+ }
+ }
+ fsleep(1000);
+ for_each_set_bit(i, busy_mask, cnt) {
+ chan = &dw->chan[off + i];
+ if (dw_edma_core_ch_status(chan) != DMA_IN_PROGRESS ||
+ tsz[i] != dw_edma_core_ch_transfer_size(chan))
+ moving = true;
+ }
+ if (!dw_edma_engine_recovery_needed(rec))
+ goto out_release;
+ if (moving) {
+ dev_warn_ratelimited(dw->chip->dev,
+ "%s engine quiesce timed out with transfers still progressing, retrying\n",
+ dir_name);
+ queue_work(dw->wq, &rec->work);
+ return;
+ }
+
+ dev_warn(dw->chip->dev,
+ "%s engine reset with a channel frozen mid-element\n",
+ dir_name);
+ }
+
+ if (!dw_edma_engine_recovery_needed(rec))
+ goto out_release;
+
+ if (!dw->core->engine_reset(dw, rec->dir)) {
+ /*
+ * Keep channels gated and do not re-enable the engine unless
+ * ENGINE_EN clears.
+ */
+ if (++rec->fails >= DW_EDMA_ENGINE_RESET_MAX_FAILS) {
+ dev_err(dw->chip->dev,
+ "%s engine did not drain after %u attempts; leaving channels gated\n",
+ dir_name, rec->fails);
+ return;
+ }
+ queue_work(dw->wq, &rec->work);
+ return;
+ }
+
+ if (rec->fails)
+ dev_warn(dw->chip->dev, "%s engine drained after %u retries\n",
+ dir_name, rec->fails);
+
+ dw_edma_engine_recovery_sync_irqs(rec);
+
+ /*
+ * Keep the engine disabled until every configured LL channel context is
+ * rebuilt. On the tested integration, re-enabling a preserved context
+ * resumed an old element without a doorbell, causing an IOMMU fault and
+ * stalling the engine again. Reset each ring so it starts from a fresh
+ * context.
+ */
+ for (i = 0; i < cnt; i++) {
+ bool stop;
+
+ chan = &dw->chan[off + i];
+ guard(spinlock_irqsave)(&chan->vc.lock);
+
+ if (chan->configured && !chan->non_ll) {
+ /* Honor EDMA_REQ_STOP and EDMA_REQ_PAUSE instead of republishing. */
+ stop = chan->request == EDMA_REQ_STOP;
+ if (stop)
+ dw_edma_terminate_all_descs(chan);
+
+ dw_edma_core_reset_ll(chan);
+
+ if (stop) {
+ chan->request = EDMA_REQ_NONE;
+ chan->status = EDMA_ST_IDLE;
+ } else if (chan->request == EDMA_REQ_PAUSE) {
+ chan->request = EDMA_REQ_NONE;
+ chan->status = EDMA_ST_PAUSE;
+ }
+ }
+ }
+
+ dw->core->engine_enable(dw, rec->dir);
+ dw_edma_engine_recovery_release(rec);
+
+ dev_warn(dw->chip->dev, "%s engine was reset to recover a stalled channel\n",
+ dir_name);
+ return;
+
+out_release:
+ dw_edma_engine_recovery_release(rec);
+}
+
static void dw_edma_device_caps(struct dma_chan *dchan,
struct dma_slave_caps *caps)
{
@@ -1640,6 +1905,13 @@ int dw_edma_probe(struct dw_edma_chip *chip)
raw_spin_lock_init(&dw->lock);
+ for (i = 0; i < ARRAY_SIZE(dw->eng_recovery); i++) {
+ dw->eng_recovery[i].dw = dw;
+ dw->eng_recovery[i].dir = i;
+ INIT_WORK(&dw->eng_recovery[i].work,
+ dw_edma_engine_recovery_work);
+ }
+
/*
* chip->ll_*_cnt describes the channels exposed by this instance. Keep
* the usable hardware counts separate for partial ownership checks.
@@ -1738,11 +2010,6 @@ int dw_edma_remove(struct dw_edma_chip *chip)
if (!dw)
return -ENODEV;
- if (chip->flags & DW_EDMA_CHIP_PARTIAL)
- err = dw_edma_core_quiesce(dw);
- else
- dw_edma_core_off(dw);
-
/* Free irqs */
for (i = (dw->nr_irqs - 1); i >= 0; i--)
free_irq(chip->ops->irq_vector(dev, i), &dw->irq[i]);
@@ -1751,6 +2018,16 @@ int dw_edma_remove(struct dw_edma_chip *chip)
for (i = 0; i < dw->wr_ch_cnt + dw->rd_ch_cnt; i++)
cancel_work_sync(&dw->chan[i].irq_work);
+ /* Prevent a running recovery worker from requeueing itself. */
+ disable_work_sync(&dw->eng_recovery[0].work);
+ disable_work_sync(&dw->eng_recovery[1].work);
+
+ /* A recovery worker can re-enable the engine, so stop it last. */
+ if (chip->flags & DW_EDMA_CHIP_PARTIAL)
+ err = dw_edma_core_quiesce(dw);
+ else
+ dw_edma_core_off(dw);
+
destroy_workqueue(dw->wq);
/* Deregister eDMA device */
diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index e3b42ad14f78..5520f49a76f1 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -104,6 +104,10 @@ struct dw_edma_chan {
bool ll_irq_stopped;
bool ll_irq_requested;
+ /* Per-channel recovery state. */
+ bool ll_recovery_pending;
+ bool ll_recovering;
+
u32 ll_max; /* Data entries */
struct dw_edma_region ll_region; /* Linked list */
bool ll_valid; /* LL context programmed */
@@ -133,6 +137,18 @@ struct dw_edma_irq {
DECLARE_BITMAP(rd_mask, HDMA_MAX_RD_CH);
};
+/*
+ * Direction-wide recovery state. active prevents duplicate recovery work;
+ * each channel's ll_recovering flag gates publication and doorbells.
+ */
+struct dw_edma_engine_recovery {
+ bool active;
+ unsigned int fails;
+ struct work_struct work;
+ struct dw_edma *dw;
+ enum dw_edma_dir dir;
+};
+
struct dw_edma {
char name[32];
@@ -147,11 +163,13 @@ struct dw_edma {
struct dw_edma_chan *chan;
/*
- * WQ_HIGHPRI keeps completion processing responsive under heavy load;
- * WQ_UNBOUND lets different channels run on different CPUs.
+ * WQ_HIGHPRI keeps completion and recovery work responsive under heavy
+ * load; WQ_UNBOUND lets independent work run on different CPUs.
*/
struct workqueue_struct *wq;
+ struct dw_edma_engine_recovery eng_recovery[2];
+
raw_spinlock_t lock; /* Protect v0 shared registers */
struct dw_edma_chip *chip;
@@ -167,6 +185,10 @@ 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);
+ /*
+ * Recovery is available only when ch_transfer_size, engine_reset, and
+ * engine_enable are all provided.
+ */
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 done,
@@ -180,7 +202,7 @@ struct dw_edma_core_ops {
int (*ll_cur_idx)(struct dw_edma_chan *chan);
/* Called with chan->vc.lock held. */
bool (*ll_irq)(struct dw_edma_desc *desc, u32 i, u32 free);
- /* Reset one direction, discard its IRQ status, and leave it disabled. */
+ /* Reset one direction, clear its IRQ status, and leave it disabled. */
bool (*engine_reset)(struct dw_edma *dw, enum dw_edma_dir dir);
void (*engine_enable)(struct dw_edma *dw, enum dw_edma_dir dir);
void (*ch_doorbell)(struct dw_edma_chan *chan);
@@ -313,7 +335,7 @@ static inline void dw_edma_core_ch_doorbell(struct dw_edma_chan *chan)
static inline void dw_edma_core_ch_enable(struct dw_edma_chan *chan)
{
chan->dw->core->ch_enable(chan);
- if (chan->dw->core->engine_enable)
+ if (!chan->ll_recovering && chan->dw->core->engine_enable)
chan->dw->core->engine_enable(chan->dw, chan->dir);
}
--
2.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v2 16/19] dmaengine: dw-edma: Add engine recovery infrastructure
2026-07-23 8:41 ` [PATCH v2 16/19] dmaengine: dw-edma: Add engine recovery infrastructure Koichiro Den
@ 2026-07-23 9:14 ` sashiko-bot
0 siblings, 0 replies; 37+ messages in thread
From: sashiko-bot @ 2026-07-23 9:14 UTC (permalink / raw)
To: Koichiro Den; +Cc: vkoul, dmaengine, Frank.Li
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [High] Hardware interrupts are disabled via free_irq() before the hardware engine is stopped, risking an interrupt storm.
- [High] Passive stall detection will fail to trigger recovery for sleeping clients, leading to indefinite hangs.
- [High] Queueing work on a destroyed workqueue during driver removal leads to a kernel crash.
- [Low] Mixing goto-based cleanup labels with guard()/scoped_guard() declarations in the same function.
Pre-existing issues:
- [High] IRQs are requested before channels are fully initialized, leading to potential crashes from early interrupts.
--
--- Patch [16]: [PATCH v2 16/19] dmaengine: dw-edma: Add engine recovery infrastructure ---
commit 348674e8eed041726892cbce36b8d539a278c6d1
Author: Koichiro Den <den@valinux.co.jp>
Subject: dmaengine: dw-edma: Add engine recovery infrastructure
This commit adds a worker to coordinate engine recovery in the dw-edma
driver. It gates publication and doorbells, waits for channels to stop,
and rebuilds linked-list contexts after resetting the engine.
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 9305137f85f67..14cd0038dbd27 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
> +static void dw_edma_engine_recovery_work(struct work_struct *work)
> +{
> + struct dw_edma_engine_recovery *rec =
> + container_of(work, struct dw_edma_engine_recovery, work);
> + struct dw_edma *dw = rec->dw;
> + const char *dir_name = str_write_read(rec->dir == EDMA_DIR_WRITE);
> + u16 off = rec->dir == EDMA_DIR_WRITE ? 0 : dw->wr_ch_cnt;
> + u16 cnt = rec->dir == EDMA_DIR_WRITE ? dw->wr_ch_cnt : dw->rd_ch_cnt;
> + unsigned long timeout =
> + jiffies + msecs_to_jiffies(DW_EDMA_ENGINE_QUIESCE_TIMEOUT_MS);
> + struct dw_edma_chan *chan;
> + bool configured_ll;
> + bool busy;
> + u16 i;
> +
> + if (!dw_edma_engine_recovery_needed(rec)) {
> + if (READ_ONCE(rec->active))
> + dw_edma_engine_recovery_release(rec);
> + return;
> + }
> +
> + WRITE_ONCE(rec->active, true);
> +
> + /* Gate each channel before inspecting or resetting the direction. */
> + for (i = 0; i < cnt; i++) {
> + chan = &dw->chan[off + i];
> + guard(spinlock_irqsave)(&chan->vc.lock);
[Severity: Low]
Does this code violate the cleanup subsystem guidelines by mixing goto-based
cleanup labels with scope-based guards?
The cleanup subsystem guide explicitly states that functions must not mix
goto-based cleanup labels with guard() or scoped_guard() declarations. This
function uses a goto out_release; label further down, while declaring
guard(spinlock_irqsave) here in the loop.
[ ... ]
> + if (moving) {
> + dev_warn_ratelimited(dw->chip->dev,
> + "%s engine quiesce timed out with transfers still progressing, retrying\n",
> + dir_name);
> + queue_work(dw->wq, &rec->work);
> + return;
> + }
> +
> + dev_warn(dw->chip->dev,
> + "%s engine reset with a channel frozen mid-element\n",
> + dir_name);
> + }
> +
> + if (!dw_edma_engine_recovery_needed(rec))
> + goto out_release;
[ ... ]
> + if (!dw->core->engine_reset(dw, rec->dir)) {
> + /*
> + * Keep channels gated and do not re-enable the engine unless
> + * ENGINE_EN clears.
> + */
> + if (++rec->fails >= DW_EDMA_ENGINE_RESET_MAX_FAILS) {
> + dev_err(dw->chip->dev,
> + "%s engine did not drain after %u attempts; leaving channels gated\n",
> + dir_name, rec->fails);
> + return;
> + }
> + queue_work(dw->wq, &rec->work);
[Severity: High]
Can passive stall detection fail to trigger this recovery for clients that
are sleeping?
The stall check relies on dw_edma_ll_stall_check() reading timeouts, but
it appears to only be invoked when dw_edma_core_ch_maybe_doorbell() is
called. If a hardware channel freezes and generates no further interrupts
while a client waits via wait_for_completion(), could the stall go
completely undetected because the status is never actively polled?
[ ... ]
> @@ -1640,6 +1905,13 @@ int dw_edma_probe(struct dw_edma_chip *chip)
>
> raw_spin_lock_init(&dw->lock);
>
> + for (i = 0; i < ARRAY_SIZE(dw->eng_recovery); i++) {
> + dw->eng_recovery[i].dw = dw;
> + dw->eng_recovery[i].dir = i;
> + INIT_WORK(&dw->eng_recovery[i].work,
> + dw_edma_engine_recovery_work);
> + }
> +
[Severity: High]
This is a pre-existing issue, but does dw_edma_probe() register interrupts
before the driver's internal channel state is initialized?
In dw_edma_probe(), the interrupt handler is registered by calling
dw_edma_irq_request() before the channel structures and work items are
fully initialized via dw_edma_channel_setup().
If a spurious or shared interrupt fires immediately after registration:
dw_edma_probe()
dw_edma_irq_request(dw, &wr_alloc, &rd_alloc);
...
dw_edma_channel_setup(dw, wr_alloc, rd_alloc);
Could this attempt to access or queue work using zeroed memory, leading to
a kernel crash?
[ ... ]
> @@ -1738,11 +2010,6 @@ int dw_edma_remove(struct dw_edma_chip *chip)
> if (!dw)
> return -ENODEV;
>
> - if (chip->flags & DW_EDMA_CHIP_PARTIAL)
> - err = dw_edma_core_quiesce(dw);
> - else
> - dw_edma_core_off(dw);
> -
> /* Free irqs */
> for (i = (dw->nr_irqs - 1); i >= 0; i--)
> free_irq(chip->ops->irq_vector(dev, i), &dw->irq[i]);
> @@ -1751,6 +2018,16 @@ int dw_edma_remove(struct dw_edma_chip *chip)
> for (i = 0; i < dw->wr_ch_cnt + dw->rd_ch_cnt; i++)
> cancel_work_sync(&dw->chan[i].irq_work);
>
> + /* Prevent a running recovery worker from requeueing itself. */
> + disable_work_sync(&dw->eng_recovery[0].work);
> + disable_work_sync(&dw->eng_recovery[1].work);
> +
> + /* A recovery worker can re-enable the engine, so stop it last. */
> + if (chip->flags & DW_EDMA_CHIP_PARTIAL)
> + err = dw_edma_core_quiesce(dw);
> + else
> + dw_edma_core_off(dw);
> +
[Severity: High]
Will moving dw_edma_core_off() after free_irq() risk an interrupt storm?
By tearing down the interrupt handler before stopping the hardware, the
hardware remains fully active while its handler is unregistered. If the
engine generates an interrupt, a shared interrupt handler would return
IRQ_NONE. Because the hardware interrupt is never cleared, won't the CPU
get stuck in an interrupt loop until the kernel disables the shared IRQ
line completely?
> destroy_workqueue(dw->wq);
>
> /* Deregister eDMA device */
[Severity: High]
Could destroying the workqueue before unregistering the DMA device cause
a use-after-free crash?
Since destroy_workqueue() is called before dma_async_device_unregister(),
the device is still visible to clients. If a client software calls
dmaengine_tx_status() during driver unload, it can invoke
dw_edma_core_ch_maybe_doorbell(). With this patch, the doorbell logic now
eventually hits dw_edma_ll_stall_check(), which can call
queue_work(chan->dw->wq, &rec->work).
Could this attempt to queue work on the already-destroyed workqueue?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260723084150.521366-1-den@valinux.co.jp?part=16
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v2 17/19] dmaengine: dw-edma: Detect and recover a stalled eDMA engine
2026-07-23 8:41 [PATCH v2 00/19] dmaengine: dw-edma: Support dynamic LL appends Koichiro Den
` (15 preceding siblings ...)
2026-07-23 8:41 ` [PATCH v2 16/19] dmaengine: dw-edma: Add engine recovery infrastructure Koichiro Den
@ 2026-07-23 8:41 ` Koichiro Den
2026-07-23 9:12 ` sashiko-bot
2026-07-23 8:41 ` [PATCH v2 18/19] dmaengine: dw-edma: Dynamically append requests while running Koichiro Den
2026-07-23 8:41 ` [PATCH v2 19/19] dmaengine: dw-edma: Add trace support Koichiro Den
18 siblings, 1 reply; 37+ messages in thread
From: Koichiro Den @ 2026-07-23 8:41 UTC (permalink / raw)
To: Manivannan Sadhasivam, Vinod Koul, Frank Li, Gustavo Pimentel,
Kees Cook, Krzysztof Wilczyński, Kishon Vijay Abraham I,
Bjorn Helgaas, Christoph Hellwig, Serge Semin, Cai Huoqing,
Niklas Cassel
Cc: Devendra K Verma, dmaengine, linux-kernel
Under sustained multi-channel traffic on one legacy eDMA integration, a
write channel occasionally stopped with pending work, no progress, and
no error. Repeated doorbells did not restart it; clearing ENGINE_EN did.
Start a stall window when the common doorbell path sees a stopped channel
with pending entries. If a later check still finds no progress after 30
ms, reconcile the stopped LLP boundary and queue direction recovery.
Clear the stall state whenever progress advances. Another doorbell-path
check, not a timer, confirms the stall.
Only providers with the recovery operations opt in. Exclude HDMA
compatibility mode because it has no ENGINE_EN. The trigger and timeout
remain empirical and need coverage on other integrations.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
- New patch.
drivers/dma/dw-edma/dw-edma-core.c | 46 ++++++++++++++++++++++++++++++
drivers/dma/dw-edma/dw-edma-core.h | 4 ++-
2 files changed, 49 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 14cd0038dbd2..1b9ba34b25c8 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -24,6 +24,7 @@
#include "../dmaengine.h"
#include "../virt-dma.h"
+#define DW_EDMA_LL_STALL_TIMEOUT_MS 30
#define DW_EDMA_ENGINE_QUIESCE_TIMEOUT_MS 250
#define DW_EDMA_ENGINE_RESET_MAX_FAILS 5
#define DW_EDMA_MAX_DIR_CH MAX(HDMA_MAX_WR_CH, HDMA_MAX_RD_CH)
@@ -129,6 +130,7 @@ static void dw_edma_core_reset_ll(struct dw_edma_chan *chan)
chan->ll_head = 0;
chan->ll_done = 0;
dw_edma_ll_irq_idx_discard(chan);
+ chan->ll_stall_valid = false;
chan->ll_recovery_pending = false;
/* Drop stale CB bits before reusing the circular LL ring. */
for (i = 0; i < chan->ll_max; i++)
@@ -183,11 +185,52 @@ static bool dw_edma_ll_advance(struct dw_edma_chan *chan, int idx, u32 *old_done
*old_done = chan->ll_done;
chan->ll_done = idx;
+ chan->ll_stall_valid = false;
chan->ll_recovery_pending = false;
return true;
}
+/*
+ * Called with vc.lock held for a stopped channel with LL work pending. Queue
+ * direction recovery if repeated doorbells show no progress for the stall
+ * timeout.
+ */
+static void dw_edma_ll_stall_check(struct dw_edma_chan *chan)
+{
+ struct dw_edma_engine_recovery *rec;
+
+ /*
+ * DWC PCIe Controller Databook 6.10a-lca06, "Legacy DMA and HDMA
+ * Software Compatibility":
+ *
+ * "HDMA does not implement engine enable, that is
+ * DMA_[WRITE|READ]_ENGINE_EN_OFF.DMA_[WRITE|READ]_ENGINE field."
+ */
+ if (chan->dw->chip->mf == EDMA_MF_HDMA_COMPAT)
+ return;
+
+ if (!chan->dw->core->engine_reset ||
+ !chan->dw->core->engine_enable ||
+ !chan->dw->core->ch_transfer_size)
+ return;
+
+ if (!chan->ll_stall_valid) {
+ chan->ll_stall_valid = true;
+ chan->ll_stall_since = jiffies;
+ return;
+ }
+
+ if (!time_after(jiffies, chan->ll_stall_since +
+ msecs_to_jiffies(DW_EDMA_LL_STALL_TIMEOUT_MS)))
+ return;
+
+ rec = &chan->dw->eng_recovery[chan->dir];
+ chan->ll_recovery_pending = true;
+ if (!READ_ONCE(rec->active))
+ queue_work(chan->dw->wq, &rec->work);
+}
+
static bool dw_edma_ll_pending(struct dw_edma_chan *chan)
{
return dw_edma_core_get_used_num(chan);
@@ -492,10 +535,13 @@ static void dw_edma_core_ch_maybe_doorbell(struct dw_edma_chan *chan)
if (dw_edma_core_ch_status(chan) == DMA_IN_PROGRESS)
return;
+ /* Reconcile a lost tail completion before declaring a stall. */
dw_edma_ll_reconcile_and_refill(chan);
if (!dw_edma_ll_pending(chan))
return;
+ dw_edma_ll_stall_check(chan);
+
dw_edma_core_ch_doorbell(chan);
}
diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index 5520f49a76f1..f67232110f3c 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -104,7 +104,9 @@ struct dw_edma_chan {
bool ll_irq_stopped;
bool ll_irq_requested;
- /* Per-channel recovery state. */
+ /* Per-channel stall and recovery state. */
+ unsigned long ll_stall_since;
+ bool ll_stall_valid;
bool ll_recovery_pending;
bool ll_recovering;
--
2.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v2 17/19] dmaengine: dw-edma: Detect and recover a stalled eDMA engine
2026-07-23 8:41 ` [PATCH v2 17/19] dmaengine: dw-edma: Detect and recover a stalled eDMA engine Koichiro Den
@ 2026-07-23 9:12 ` sashiko-bot
0 siblings, 0 replies; 37+ messages in thread
From: sashiko-bot @ 2026-07-23 9:12 UTC (permalink / raw)
To: Koichiro Den; +Cc: vkoul, Frank.Li, dmaengine
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] A race condition in the channel stall recovery mechanism can leave a channel permanently stalled because `queue_work()` is manually bypassed while `rec->active` is true during the worker's cleanup phase.
--
--- Patch [17]: [PATCH v2 17/19] dmaengine: dw-edma: Detect and recover a stalled eDMA engine ---
commit 9ca900fbc673ab7db9a251eb4f96dc2eddcf8833
Author: Koichiro Den <den@valinux.co.jp>
dmaengine: dw-edma: Detect and recover a stalled eDMA engine
This commit introduces a mechanism to detect and recover stalled legacy
eDMA engines. It adds a 30ms stall window during the doorbell path and
queues a recovery work if a channel stops making progress.
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 14cd0038dbd27..1b9ba34b25c80 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
> @@ -183,11 +185,52 @@ static bool dw_edma_ll_advance(struct dw_edma_chan *chan, int idx, u32 *old_done
>
> *old_done = chan->ll_done;
> chan->ll_done = idx;
> + chan->ll_stall_valid = false;
> chan->ll_recovery_pending = false;
>
> return true;
> }
>
> +/*
> + * Called with vc.lock held for a stopped channel with LL work pending. Queue
> + * direction recovery if repeated doorbells show no progress for the stall
> + * timeout.
> + */
> +static void dw_edma_ll_stall_check(struct dw_edma_chan *chan)
> +{
[ ... ]
> + if (!time_after(jiffies, chan->ll_stall_since +
> + msecs_to_jiffies(DW_EDMA_LL_STALL_TIMEOUT_MS)))
> + return;
> +
> + rec = &chan->dw->eng_recovery[chan->dir];
> + chan->ll_recovery_pending = true;
> + if (!READ_ONCE(rec->active))
> + queue_work(chan->dw->wq, &rec->work);
> +}
[Severity: High]
Does this custom active check bypass queue_work()'s built-in deduplication,
potentially leaving a channel permanently stalled?
In dw_edma_engine_recovery_release(), rec->active is still true when it
calls dw_edma_core_ch_maybe_doorbell() for all channels:
drivers/dma/dw-edma/dw-edma-core.c:dw_edma_engine_recovery_release() {
...
for (i = 0; i < cnt; i++) {
chan = &dw->chan[off + i];
...
dw_edma_core_ch_maybe_doorbell(chan);
}
WRITE_ONCE(rec->active, false);
}
If another channel also stalled during the recovery wait, this sequence
would trigger dw_edma_ll_stall_check() while rec->active is still true.
The manual check here skips queue_work(), and when the release function
finishes by setting rec->active to false, no new worker is queued.
Since queue_work() already safely handles WORK_STRUCT_PENDING, could
this check be safely removed to ensure we don't drop recovery work?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260723084150.521366-1-den@valinux.co.jp?part=17
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v2 18/19] dmaengine: dw-edma: Dynamically append requests while running
2026-07-23 8:41 [PATCH v2 00/19] dmaengine: dw-edma: Support dynamic LL appends Koichiro Den
` (16 preceding siblings ...)
2026-07-23 8:41 ` [PATCH v2 17/19] dmaengine: dw-edma: Detect and recover a stalled eDMA engine Koichiro Den
@ 2026-07-23 8:41 ` Koichiro Den
2026-07-23 8:41 ` [PATCH v2 19/19] dmaengine: dw-edma: Add trace support Koichiro Den
18 siblings, 0 replies; 37+ messages in thread
From: Koichiro Den @ 2026-07-23 8:41 UTC (permalink / raw)
To: Manivannan Sadhasivam, Vinod Koul, Frank Li, Gustavo Pimentel,
Kees Cook, Krzysztof Wilczyński, Kishon Vijay Abraham I,
Bjorn Helgaas, Christoph Hellwig, Serge Semin, Cai Huoqing,
Niklas Cassel
Cc: Devendra K Verma, dmaengine, linux-kernel
From: Frank Li <Frank.Li@nxp.com>
Publish issued descriptors into free LL entries while the channel runs.
Skip descriptors already fully published and continue with later ones.
For LL channels, dw_edma_start_transfer() reports only whether this pass
published entries. Derive BUSY or IDLE from pending ring work instead.
Let issue_pending() feed a BUSY LL channel when neither EDMA_REQ_STOP nor
EDMA_REQ_PAUSE is pending; non-LL channels remain IDLE-only.
All kicks still use the common doorbell path, so running channels continue
without another kick and stopped channels use the reconciliation and
recovery paths.
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>
---
Changes in v2:
- Enable live append only after the progress and recovery support.
- Keep non-LL channels on the IDLE-only path.
- Base resume state on pending LL entries, not whether that pass
published new entries.
drivers/dma/dw-edma/dw-edma-core.c | 47 ++++++++++++++++++++----------
1 file changed, 32 insertions(+), 15 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index 1b9ba34b25c8..1ccd3783181e 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -323,25 +323,38 @@ static int dw_edma_start_transfer(struct dw_edma_chan *chan)
{
struct dw_edma_desc *desc;
struct virt_dma_desc *vd;
+ int ret = 0;
- /* Stop publishing until recovery can rebuild and republish the ring. */
- if (!chan->non_ll && chan->ll_recovering)
- return 0;
+ if (chan->non_ll) {
+ vd = vchan_next_desc(&chan->vc);
+ if (!vd)
+ return 0;
- vd = vchan_next_desc(&chan->vc);
- if (!vd)
- return 0;
+ dw_edma_core_start(vd2dw_edma_desc(vd));
+ return 1;
+ }
- desc = vd2dw_edma_desc(vd);
- if (!desc)
+ /* Stop publishing until recovery can rebuild and republish the ring. */
+ if (chan->ll_recovering)
return 0;
- if (!chan->non_ll && !chan->ll_valid)
+ if (!chan->ll_valid)
dw_edma_core_reset_ll(chan);
- dw_edma_core_start(desc);
+ list_for_each_entry(vd, &chan->vc.desc_issued, node) {
+ if (!dw_edma_core_get_free_num(chan))
+ break;
- return 1;
+ desc = vd2dw_edma_desc(vd);
+ /* A fully published descriptor may still be pending in hardware. */
+ if (desc->start_burst == desc->nburst)
+ continue;
+
+ dw_edma_core_start(desc);
+ ret = 1;
+ }
+
+ return ret;
}
static void dw_edma_terminate_vdesc(struct virt_dma_desc *vd)
@@ -908,6 +921,7 @@ static int dw_edma_device_pause(struct dma_chan *dchan)
static int dw_edma_device_resume(struct dma_chan *dchan)
{
struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
+ bool active;
int err = 0;
guard(spinlock_irqsave)(&chan->vc.lock);
@@ -919,9 +933,10 @@ static int dw_edma_device_resume(struct dma_chan *dchan)
} else if (chan->request != EDMA_REQ_NONE) {
err = -EPERM;
} else {
- chan->status = EDMA_ST_BUSY;
- if (!dw_edma_start_transfer(chan))
- chan->status = EDMA_ST_IDLE;
+ active = dw_edma_start_transfer(chan);
+ if (!chan->non_ll)
+ active = dw_edma_ll_pending(chan);
+ chan->status = active ? EDMA_ST_BUSY : EDMA_ST_IDLE;
dw_edma_core_ch_maybe_doorbell(chan);
}
@@ -994,9 +1009,11 @@ static void dw_edma_device_issue_pending(struct dma_chan *dchan)
unsigned long flags;
spin_lock_irqsave(&chan->vc.lock, flags);
+ /* Only LL channels can accept work while already running. */
if (chan->configured && vchan_issue_pending(&chan->vc) &&
chan->request == EDMA_REQ_NONE &&
- chan->status == EDMA_ST_IDLE) {
+ (chan->non_ll ? chan->status == EDMA_ST_IDLE :
+ chan->status != EDMA_ST_PAUSE)) {
if (!chan->non_ll && !dw_edma_ll_pending(chan))
dw_edma_ll_irq_idx_discard(chan);
chan->status = EDMA_ST_BUSY;
--
2.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* [PATCH v2 19/19] dmaengine: dw-edma: Add trace support
2026-07-23 8:41 [PATCH v2 00/19] dmaengine: dw-edma: Support dynamic LL appends Koichiro Den
` (17 preceding siblings ...)
2026-07-23 8:41 ` [PATCH v2 18/19] dmaengine: dw-edma: Dynamically append requests while running Koichiro Den
@ 2026-07-23 8:41 ` Koichiro Den
2026-07-23 9:11 ` sashiko-bot
18 siblings, 1 reply; 37+ messages in thread
From: Koichiro Den @ 2026-07-23 8:41 UTC (permalink / raw)
To: Manivannan Sadhasivam, Vinod Koul, Frank Li, Gustavo Pimentel,
Kees Cook, Krzysztof Wilczyński, Kishon Vijay Abraham I,
Bjorn Helgaas, Christoph Hellwig, Serge Semin, Cai Huoqing,
Niklas Cassel
Cc: Devendra K Verma, dmaengine, linux-kernel
From: Frank Li <Frank.Li@nxp.com>
The circular LL ring has separate producer and consumer positions. Add
tracepoints for LL publication, descriptor append and completion,
IRQ-paired progress, and engine recovery. The common events cover eDMA
and HDMA.
The recovery event records configured LL channels before a direction
reset. When disabled, static keys leave only a branch at each trace site.
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>
---
Changes in v2:
- Drop the obsolete tx_status event and ll_end fields.
- Rename the start event for append/refill and add stopped/request state
to the IRQ event.
- Add the engine recovery event.
- Match fields to their source types and simplify the Kbuild wiring.
drivers/dma/dw-edma/Makefile | 2 +
drivers/dma/dw-edma/dw-edma-core.c | 54 ++++++++
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 | 187 ++++++++++++++++++++++++++++
5 files changed, 249 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 1ccd3783181e..4b30fa5383d0 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -293,6 +293,12 @@ static void dw_edma_core_ll_start(struct dw_edma_desc *desc)
chan->ll_head, chan->cb,
dw_edma_core_enable_ll_irq(desc, i, free));
+ 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) {
@@ -350,6 +356,7 @@ static int dw_edma_start_transfer(struct dw_edma_chan *chan)
if (desc->start_burst == desc->nburst)
continue;
+ trace_edma_append_desc(desc);
dw_edma_core_start(desc);
ret = 1;
}
@@ -421,6 +428,7 @@ static void dw_edma_ll_clean_pending(struct dw_edma_chan *chan, u32 old_done)
/* Hardware has consumed this descriptor's LL entries. */
dw_hdma_set_callback_result(vd, DMA_TRANS_NOERROR);
list_del(&vd->node);
+ trace_edma_complete_desc(desc);
vchan_cookie_complete(vd);
}
}
@@ -647,6 +655,45 @@ dw_edma_engine_recovery_sync_irqs(struct dw_edma_engine_recovery *rec)
}
}
+static void
+dw_edma_trace_engine_recovery(struct dw_edma_engine_recovery *rec)
+{
+ struct dw_edma *dw = rec->dw;
+ u16 off = rec->dir == EDMA_DIR_WRITE ? 0 : dw->wr_ch_cnt;
+ u16 cnt = rec->dir == EDMA_DIR_WRITE ? dw->wr_ch_cnt : dw->rd_ch_cnt;
+ struct dw_edma_chan *chan;
+ u16 i;
+
+ if (!trace_edma_engine_recovery_enabled())
+ return;
+
+ for (i = 0; i < cnt; i++) {
+ enum dw_edma_request request;
+ enum dw_edma_status status;
+ bool configured_ll;
+ unsigned int pending;
+ u32 ll_head, ll_done;
+
+ chan = &dw->chan[off + i];
+ scoped_guard(spinlock_irqsave, &chan->vc.lock) {
+ configured_ll = chan->configured && !chan->non_ll;
+ if (configured_ll) {
+ request = chan->request;
+ status = chan->status;
+ ll_head = chan->ll_head;
+ ll_done = chan->ll_done;
+ pending = dw_edma_ll_pending(chan);
+ }
+ }
+ if (!configured_ll)
+ continue;
+
+ trace_edma_engine_recovery(chan, dw_edma_core_ch_status(chan),
+ request, status, ll_head, ll_done,
+ pending);
+ }
+}
+
static void dw_edma_engine_recovery_work(struct work_struct *work)
{
struct dw_edma_engine_recovery *rec =
@@ -747,6 +794,8 @@ static void dw_edma_engine_recovery_work(struct work_struct *work)
if (!dw_edma_engine_recovery_needed(rec))
goto out_release;
+ dw_edma_trace_engine_recovery(rec);
+
if (!dw->core->engine_reset(dw, rec->dir)) {
/*
* Keep channels gated and do not re-enable the engine unless
@@ -1322,6 +1371,9 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
bool active;
spin_lock_irqsave(&chan->vc.lock, flags);
+ if (!chan->non_ll)
+ trace_edma_irq(chan, chan->ll_irq_idx, false,
+ chan->ll_irq_stopped);
if (chan->status == EDMA_ST_PAUSE)
goto out;
@@ -1394,6 +1446,8 @@ static void dw_edma_progress_interrupt(struct dw_edma_chan *chan)
guard(spinlock_irqsave)(&chan->vc.lock);
+ trace_edma_irq(chan, chan->ll_irq_idx, true,
+ chan->ll_irq_stopped);
if (chan->request == EDMA_REQ_NONE && chan->status != EDMA_ST_PAUSE) {
if (dw_edma_ll_consume_progress(chan))
dw_edma_start_transfer(chan);
diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index f67232110f3c..c174fa0d8447 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -179,6 +179,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 *, bool);
struct dw_edma_core_ops {
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..9d6e8fce6032
--- /dev/null
+++ b/drivers/dma/dw-edma/dw-edma-trace.h
@@ -0,0 +1,187 @@
+/* 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)
+);
+
+DECLARE_EVENT_CLASS(edma_ll_info,
+ TP_PROTO(struct dw_edma_chan *chan, int idx, bool progress,
+ bool stopped),
+ TP_ARGS(chan, idx, progress, stopped),
+ TP_STRUCT__entry(
+ __field(u32, head)
+ __field(u32, done)
+ __field(u32, total)
+ __field(int, index)
+ __field(dma_cookie_t, completed_cookie)
+ __field(dma_cookie_t, cookie)
+ __field(u32, id)
+ __field(u8, dir)
+ __field(bool, progress)
+ __field(bool, stopped)
+ __field(bool, requested)
+ ),
+ TP_fast_assign(
+ __entry->head = chan->ll_head;
+ __entry->done = chan->ll_done;
+ __entry->total = chan->ll_max;
+ __entry->index = idx;
+ __entry->completed_cookie = chan->vc.chan.completed_cookie;
+ __entry->cookie = chan->vc.chan.cookie;
+ __entry->id = chan->id;
+ __entry->dir = chan->dir;
+ __entry->progress = progress;
+ __entry->stopped = stopped;
+ __entry->requested = chan->ll_irq_requested;
+ ),
+ TP_printk("chan %u%c event %s stopped %u requested %u head %u done %u total %u index %d complete cookie %d cookie %d",
+ __entry->id,
+ __entry->dir ? 'R' : 'W',
+ __entry->progress ? "progress" : "done",
+ __entry->stopped,
+ __entry->requested,
+ __entry->head,
+ __entry->done,
+ __entry->total,
+ __entry->index,
+ __entry->completed_cookie,
+ __entry->cookie)
+);
+
+DEFINE_EVENT(edma_ll_info, edma_irq,
+ TP_PROTO(struct dw_edma_chan *chan, int idx, bool progress,
+ bool stopped),
+ TP_ARGS(chan, idx, progress, stopped)
+);
+
+TRACE_EVENT(edma_engine_recovery,
+ TP_PROTO(struct dw_edma_chan *chan, enum dma_status hw_status,
+ enum dw_edma_request request, enum dw_edma_status status,
+ u32 ll_head, u32 ll_done, u32 pending),
+ TP_ARGS(chan, hw_status, request, status, ll_head, ll_done, pending),
+ TP_STRUCT__entry(
+ __field(u32, id)
+ __field(u32, head)
+ __field(u32, done)
+ __field(u32, pending)
+ __field(int, hw_status)
+ __field(int, request)
+ __field(int, status)
+ __field(u8, dir)
+ ),
+ TP_fast_assign(
+ __entry->id = chan->id;
+ __entry->dir = chan->dir;
+ __entry->hw_status = hw_status;
+ __entry->request = request;
+ __entry->status = status;
+ __entry->head = ll_head;
+ __entry->done = ll_done;
+ __entry->pending = pending;
+ ),
+ TP_printk("chan %u%c hw %d request %d status %d head %u done %u pending %u",
+ __entry->id,
+ __entry->dir ? 'R' : 'W',
+ __entry->hw_status,
+ __entry->request,
+ __entry->status,
+ __entry->head,
+ __entry->done,
+ __entry->pending)
+);
+
+DECLARE_EVENT_CLASS(edma_log_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)
+);
+
+DEFINE_EVENT(edma_log_ll, 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)
+);
+
+#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.51.0
^ permalink raw reply related [flat|nested] 37+ messages in thread* Re: [PATCH v2 19/19] dmaengine: dw-edma: Add trace support
2026-07-23 8:41 ` [PATCH v2 19/19] dmaengine: dw-edma: Add trace support Koichiro Den
@ 2026-07-23 9:11 ` sashiko-bot
0 siblings, 0 replies; 37+ messages in thread
From: sashiko-bot @ 2026-07-23 9:11 UTC (permalink / raw)
To: Koichiro Den; +Cc: vkoul, Frank.Li, dmaengine
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Data truncation of the pending elements count in engine recovery tracepoint.
--
--- Patch [19]: [PATCH v2 19/19] dmaengine: dw-edma: Add trace support ---
commit 13ac2a18e4baa8bbe3759195a89d17ff4f9df2eb
Author: Frank Li <Frank.Li@nxp.com>
dmaengine: dw-edma: Add trace support
The circular LL ring has separate producer and consumer positions. Add
tracepoints for LL publication, descriptor append and completion,
IRQ-paired progress, and engine recovery. The common events cover eDMA
and HDMA.
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 1ccd3783181ec..4b30fa5383d0a 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
> @@ -647,6 +655,45 @@ dw_edma_engine_recovery_sync_irqs(struct dw_edma_engine_recovery *rec)
> }
> }
>
> +static void
> +dw_edma_trace_engine_recovery(struct dw_edma_engine_recovery *rec)
> +{
> + struct dw_edma *dw = rec->dw;
> + u16 off = rec->dir == EDMA_DIR_WRITE ? 0 : dw->wr_ch_cnt;
> + u16 cnt = rec->dir == EDMA_DIR_WRITE ? dw->wr_ch_cnt : dw->rd_ch_cnt;
> + struct dw_edma_chan *chan;
> + u16 i;
> +
> + if (!trace_edma_engine_recovery_enabled())
> + return;
> +
> + for (i = 0; i < cnt; i++) {
> + enum dw_edma_request request;
> + enum dw_edma_status status;
> + bool configured_ll;
> + unsigned int pending;
> + u32 ll_head, ll_done;
> +
> + chan = &dw->chan[off + i];
> + scoped_guard(spinlock_irqsave, &chan->vc.lock) {
> + configured_ll = chan->configured && !chan->non_ll;
> + if (configured_ll) {
> + request = chan->request;
> + status = chan->status;
> + ll_head = chan->ll_head;
> + ll_done = chan->ll_done;
> + pending = dw_edma_ll_pending(chan);
[Severity: Medium]
Will this truncate the actual count of pending entries to 0 or 1?
Looking at dw_edma_ll_pending() in dw-edma-core.c, it is defined to return a
boolean. Assigning its result to the unsigned int pending variable means the
tracepoint will only ever record 0 or 1 instead of the true integer count of
pending items.
Could this use dw_edma_core_get_used_num(chan) directly to ensure the correct
statistics are logged?
> + }
> + }
> + if (!configured_ll)
> + continue;
> +
> + trace_edma_engine_recovery(chan, dw_edma_core_ch_status(chan),
> + request, status, ll_head, ll_done,
> + pending);
> + }
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260723084150.521366-1-den@valinux.co.jp?part=19
^ permalink raw reply [flat|nested] 37+ messages in thread