* [PATCH v4 0/5] dmaengine: sun6i-dma: Add support for Allwinner A733 DMA controller
@ 2026-08-26 1:30 Yuanshen Cao
2026-08-26 1:30 ` [PATCH v4 1/5] dmaengine: sun6i-dma: Refactor to support A733 interrupt and register handling Yuanshen Cao
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Yuanshen Cao @ 2026-08-26 1:30 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maxime Ripard
Cc: Yuanshen Cao, dmaengine, linux-arm-kernel, linux-sunxi,
linux-kernel, devicetree, Frank Li, Krzysztof Kozlowski
Hi everyone,
This patch series introduces support for the Allwinner A733 DMA
controller in the `sun6i-dma` driver.
The A733 DMA controller differs from previous generations in several key
ways:
- Support for higher addresses (up to 32G).
- A different interrupt register layout and mapping.
- A different number of channels per interrupt register.
To support these differences without introducing complex conditional
logic throughout the driver, this series refactors the
`sun6i_dma_config` structure by adding hardware-specific parameters such
as interrupt register offsets, address masks, and channel counts per
register. This allows the driver to support the A733 and future hardware
revisions. This approach also achieves the same functionality as the DMA
drivers in Radxa BSP Package[1].
The series is organized as follows:
1. Refactor the configuration structure for interrupt and register operations.
2. Add support for variable address widths using masks.
3. Add support for variable channels per interrupt register.
4. Update the device tree bindings documentation.
5. Implement the A733-specific configuration and register mappings.
Tested on Radxa Cubie A7Z.
[1] https://github.com/radxa/allwinner-bsp/blob/cubie-aiot-v1.4.8/drivers/dma/sunxi-dma.c
Thanks!
Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
---
Changes in v4:
- Refactor configuration structure to use irq stride and offsets instead
of function pointers.
- Use GENMASK and field_prep in set_addr for different high addresses.
- Link to v3: https://patch.msgid.link/20260622-sun60i-a733-dma-v3-0-f697ef296cbc@gmail.com
Changes in v3:
- Reword patches for clarity.
- Link to v2: https://patch.msgid.link/20260621-sun60i-a733-dma-v2-0-340f205891cc@gmail.com
Changes in v2:
- Implement SUN6I_DMA_IRQ_A31_COMMON_OPS macro to avoid duplicate.
- Move set_addr into helper function and revert back sun6i_dma_set_addr.
- Rename chan_num to irq_req to avoid misleading name as suggested by
sashiko.
- Reorder and reword the dtbinding patch for more clarity.
- Link to v1: https://patch.msgid.link/20260619-sun60i-a733-dma-v1-0-da4b649fc72a@gmail.com
To: Vinod Koul <vkoul@kernel.org>
To: Frank Li <Frank.Li@kernel.org>
To: Chen-Yu Tsai <wens@kernel.org>
To: Jernej Skrabec <jernej.skrabec@gmail.com>
To: Samuel Holland <samuel@sholland.org>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Maxime Ripard <mripard@kernel.org>
Cc: dmaengine@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-sunxi@lists.linux.dev
Cc: linux-kernel@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Yuanshen Cao (5):
dmaengine: sun6i-dma: Refactor to support A733 interrupt and register handling
dmaengine: sun6i-dma: Support variable address widths using masks
dmaengine: sun6i-dma: Add num_channels_per_reg for flexible interrupt mapping
dt-bindings: dmaengine: sun50i-a64-dma: Add allwinner,sun60i-a733-dma compatible string
dmaengine: sun6i-dma: Add support for Allwinner A733 DMA controller
.../bindings/dma/allwinner,sun50i-a64-dma.yaml | 2 +
drivers/dma/sun6i-dma.c | 153 ++++++++++++++++-----
2 files changed, 122 insertions(+), 33 deletions(-)
---
base-commit: 8d3ae59288f1e7d58d76558a6ee96d533bc5019f
change-id: 20260619-sun60i-a733-dma-c2455149165d
Best regards,
--
Yuanshen Cao <alex.caoys@gmail.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v4 1/5] dmaengine: sun6i-dma: Refactor to support A733 interrupt and register handling
2026-08-26 1:30 [PATCH v4 0/5] dmaengine: sun6i-dma: Add support for Allwinner A733 DMA controller Yuanshen Cao
@ 2026-08-26 1:30 ` Yuanshen Cao
2026-08-26 1:45 ` sashiko-bot
2026-08-26 1:30 ` [PATCH v4 2/5] dmaengine: sun6i-dma: Support variable address widths using masks Yuanshen Cao
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Yuanshen Cao @ 2026-08-26 1:30 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maxime Ripard
Cc: Yuanshen Cao, dmaengine, linux-arm-kernel, linux-sunxi,
linux-kernel, devicetree
The `sun6i-dma` driver currently uses hardcoded logic for interrupt
handling (reading/writing interrupt enable and status registers) and
register dumping.
To support the Allwinner A733, which has a different register layout and
interrupt handling logic, add `irq_stride`, `irq_en_offset`, and
`irq_stat_offset` to `struct sun6i_dma_config`. Implement generic
`sun6i_read/write_irq_*` functions using these new configuration fields
to accommodate different hardware revisions.
Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
---
drivers/dma/sun6i-dma.c | 87 +++++++++++++++++++++++++++++++++++++------------
1 file changed, 67 insertions(+), 20 deletions(-)
diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index f47a326dd7ff..f305fbfb3545 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -27,7 +27,6 @@
/*
* Common registers
*/
-#define DMA_IRQ_EN(x) ((x) * 0x04)
#define DMA_IRQ_HALF BIT(0)
#define DMA_IRQ_PKG BIT(1)
#define DMA_IRQ_QUEUE BIT(2)
@@ -36,8 +35,6 @@
#define DMA_IRQ_CHAN_WIDTH 4
-#define DMA_IRQ_STAT(x) ((x) * 0x04 + 0x10)
-
#define DMA_STAT 0x30
/* Offset between DMA_IRQ_EN and DMA_IRQ_STAT limits number of channels */
@@ -52,6 +49,14 @@
#define SUNXI_H3_SECURE_REG 0x20
#define SUNXI_H3_DMA_GATE 0x28
#define SUNXI_H3_DMA_GATE_ENABLE 0x4
+
+/*
+ * Interrupts specific registers
+ */
+#define DMA_IRQ_STRIDE_A31 0x04
+#define DMA_IRQ_EN_OFFSET_A31 0x00
+#define DMA_IRQ_STAT_OFFSET_A31 0x10
+
/*
* Channels specific registers
*/
@@ -144,6 +149,9 @@ struct sun6i_dma_config {
u32 dst_addr_widths;
bool has_high_addr;
bool has_mbus_clk;
+ u32 irq_stride;
+ u32 irq_en_offset;
+ u32 irq_stat_offset;
};
/*
@@ -234,19 +242,43 @@ to_sun6i_desc(struct dma_async_tx_descriptor *tx)
return container_of(tx, struct sun6i_desc, vd.tx);
}
+static u32 sun6i_read_irq_en(struct sun6i_dma_dev *sdev, u32 irq_reg)
+{
+ return readl(sdev->base + irq_reg * sdev->cfg->irq_stride + sdev->cfg->irq_en_offset);
+}
+
+static void sun6i_write_irq_en(struct sun6i_dma_dev *sdev, u32 irq_reg, u32 irq_val)
+{
+ writel(irq_val, sdev->base + irq_reg * sdev->cfg->irq_stride + sdev->cfg->irq_en_offset);
+}
+
+static u32 sun6i_read_irq_stat(struct sun6i_dma_dev *sdev, u32 irq_reg)
+{
+ return readl(sdev->base + irq_reg * sdev->cfg->irq_stride + sdev->cfg->irq_stat_offset);
+}
+
+static void sun6i_write_irq_stat(struct sun6i_dma_dev *sdev, u32 irq_reg, u32 status)
+{
+ writel(status, sdev->base + irq_reg * sdev->cfg->irq_stride + sdev->cfg->irq_stat_offset);
+}
+
static inline void sun6i_dma_dump_com_regs(struct sun6i_dma_dev *sdev)
{
- dev_dbg(sdev->slave.dev, "Common register:\n"
- "\tmask0(%04x): 0x%08x\n"
- "\tmask1(%04x): 0x%08x\n"
- "\tpend0(%04x): 0x%08x\n"
- "\tpend1(%04x): 0x%08x\n"
- "\tstats(%04x): 0x%08x\n",
- DMA_IRQ_EN(0), readl(sdev->base + DMA_IRQ_EN(0)),
- DMA_IRQ_EN(1), readl(sdev->base + DMA_IRQ_EN(1)),
- DMA_IRQ_STAT(0), readl(sdev->base + DMA_IRQ_STAT(0)),
- DMA_IRQ_STAT(1), readl(sdev->base + DMA_IRQ_STAT(1)),
- DMA_STAT, readl(sdev->base + DMA_STAT));
+ int i;
+
+ for (i = 0; i < 2; i++) {
+ dev_dbg(sdev->slave.dev, "Common register:\n"
+ "chan num %d\n"
+ "\tmask(%04x): 0x%08x\n"
+ "\tpend(%04x): 0x%08x\n"
+ "\tstats(%04x): 0x%08x\n",
+ i,
+ i * sdev->cfg->irq_stride + sdev->cfg->irq_en_offset,
+ sun6i_read_irq_en(sdev, i),
+ i * sdev->cfg->irq_stride + sdev->cfg->irq_stat_offset,
+ sun6i_read_irq_stat(sdev, i),
+ DMA_STAT, readl(sdev->base + DMA_STAT));
+ }
}
static inline void sun6i_dma_dump_chan_regs(struct sun6i_dma_dev *sdev,
@@ -460,11 +492,11 @@ static int sun6i_dma_start_desc(struct sun6i_vchan *vchan)
vchan->irq_type = vchan->cyclic ? DMA_IRQ_PKG : DMA_IRQ_QUEUE;
- irq_val = readl(sdev->base + DMA_IRQ_EN(irq_reg));
+ irq_val = sun6i_read_irq_en(sdev, irq_reg);
irq_val &= ~((DMA_IRQ_HALF | DMA_IRQ_PKG | DMA_IRQ_QUEUE) <<
(irq_offset * DMA_IRQ_CHAN_WIDTH));
irq_val |= vchan->irq_type << (irq_offset * DMA_IRQ_CHAN_WIDTH);
- writel(irq_val, sdev->base + DMA_IRQ_EN(irq_reg));
+ sun6i_write_irq_en(sdev, irq_reg, irq_val);
writel(pchan->desc->p_lli, pchan->base + DMA_CHAN_LLI_ADDR);
writel(DMA_CHAN_ENABLE_START, pchan->base + DMA_CHAN_ENABLE);
@@ -549,14 +581,14 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
u32 status;
for (i = 0; i < sdev->num_pchans / DMA_IRQ_CHAN_NR; i++) {
- status = readl(sdev->base + DMA_IRQ_STAT(i));
+ status = sun6i_read_irq_stat(sdev, i);
if (!status)
continue;
dev_dbg(sdev->slave.dev, "DMA irq status %s: 0x%x\n",
str_high_low(i), status);
- writel(status, sdev->base + DMA_IRQ_STAT(i));
+ sun6i_write_irq_stat(sdev, i, status);
for (j = 0; (j < DMA_IRQ_CHAN_NR) && status; j++) {
pchan = sdev->pchans + j;
@@ -1072,9 +1104,11 @@ static struct dma_chan *sun6i_dma_of_xlate(struct of_phandle_args *dma_spec,
static inline void sun6i_kill_tasklet(struct sun6i_dma_dev *sdev)
{
+ int i;
+
/* Disable all interrupts from DMA */
- writel(0, sdev->base + DMA_IRQ_EN(0));
- writel(0, sdev->base + DMA_IRQ_EN(1));
+ for (i = 0; i < 2; i++)
+ sun6i_write_irq_en(sdev, i, 0);
/* Prevent spurious interrupts from scheduling the tasklet */
atomic_inc(&sdev->tasklet_shutdown);
@@ -1098,6 +1132,11 @@ static inline void sun6i_dma_free(struct sun6i_dma_dev *sdev)
}
}
+#define SUN6I_DMA_IRQ_A31_COMMON_CFG \
+ .irq_stride = DMA_IRQ_STRIDE_A31, \
+ .irq_en_offset = DMA_IRQ_EN_OFFSET_A31, \
+ .irq_stat_offset = DMA_IRQ_STAT_OFFSET_A31,
+
/*
* For A31:
*
@@ -1129,6 +1168,7 @@ static struct sun6i_dma_config sun6i_a31_dma_cfg = {
.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES),
+ SUN6I_DMA_IRQ_A31_COMMON_CFG
};
/*
@@ -1152,6 +1192,7 @@ static struct sun6i_dma_config sun8i_a23_dma_cfg = {
.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES),
+ SUN6I_DMA_IRQ_A31_COMMON_CFG
};
static struct sun6i_dma_config sun8i_a83t_dma_cfg = {
@@ -1170,6 +1211,7 @@ static struct sun6i_dma_config sun8i_a83t_dma_cfg = {
.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES),
+ SUN6I_DMA_IRQ_A31_COMMON_CFG
};
/*
@@ -1197,6 +1239,7 @@ static struct sun6i_dma_config sun8i_h3_dma_cfg = {
BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_8_BYTES),
+ SUN6I_DMA_IRQ_A31_COMMON_CFG
};
/*
@@ -1218,6 +1261,7 @@ static struct sun6i_dma_config sun50i_a64_dma_cfg = {
BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_8_BYTES),
+ SUN6I_DMA_IRQ_A31_COMMON_CFG
};
/*
@@ -1241,6 +1285,7 @@ static struct sun6i_dma_config sun50i_a100_dma_cfg = {
BIT(DMA_SLAVE_BUSWIDTH_8_BYTES),
.has_high_addr = true,
.has_mbus_clk = true,
+ SUN6I_DMA_IRQ_A31_COMMON_CFG
};
/*
@@ -1263,6 +1308,7 @@ static struct sun6i_dma_config sun50i_h6_dma_cfg = {
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_8_BYTES),
.has_mbus_clk = true,
+ SUN6I_DMA_IRQ_A31_COMMON_CFG
};
/*
@@ -1286,6 +1332,7 @@ static struct sun6i_dma_config sun8i_v3s_dma_cfg = {
.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES),
+ SUN6I_DMA_IRQ_A31_COMMON_CFG
};
static const struct of_device_id sun6i_dma_match[] = {
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v4 2/5] dmaengine: sun6i-dma: Support variable address widths using masks
2026-08-26 1:30 [PATCH v4 0/5] dmaengine: sun6i-dma: Add support for Allwinner A733 DMA controller Yuanshen Cao
2026-08-26 1:30 ` [PATCH v4 1/5] dmaengine: sun6i-dma: Refactor to support A733 interrupt and register handling Yuanshen Cao
@ 2026-08-26 1:30 ` Yuanshen Cao
2026-08-26 1:40 ` sashiko-bot
2026-08-26 1:30 ` [PATCH v4 3/5] dmaengine: sun6i-dma: Add num_channels_per_reg for flexible interrupt mapping Yuanshen Cao
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Yuanshen Cao @ 2026-08-26 1:30 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maxime Ripard
Cc: Yuanshen Cao, dmaengine, linux-arm-kernel, linux-sunxi,
linux-kernel, devicetree
The Allwinner A733 DMA controller supports higher addresses (up to
32G) compared to previous generations. The existing `sun6i_dma_set_addr`
function uses hardcoded logic for setting the high-address bits in the
LLI parameters.
Add `src_high_addr_mask` and `dst_high_addr_mask` to `struct
sun6i_dma_config` to handle different high-address bitfield mappings.
Update `sun6i_dma_set_addr` to use these masks via `field_prep()`,
allowing the driver to support variable address widths.
Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
---
drivers/dma/sun6i-dma.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index f305fbfb3545..53c54161e32a 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -24,6 +24,10 @@
#include "virt-dma.h"
+/* Non-constant mask variant of FIELD_GET() and FIELD_PREP() */
+#define field_get(_mask, _reg) (((_reg) & (_mask)) >> (ffs(_mask) - 1))
+#define field_prep(_mask, _val) (((_val) << (ffs(_mask) - 1)) & (_mask))
+
/*
* Common registers
*/
@@ -103,8 +107,8 @@
* The LLI link physical address is also mangled, but we avoid dealing
* with that by allocating LLIs from the DMA32 zone.
*/
-#define SRC_HIGH_ADDR(x) (((x) & 0x3U) << 16)
-#define DST_HIGH_ADDR(x) (((x) & 0x3U) << 18)
+#define SRC_HIGH_ADDR_MASK GENMASK(17, 16)
+#define DST_HIGH_ADDR_MASK GENMASK(19, 18)
/*
* Various hardware related defines
@@ -147,7 +151,8 @@ struct sun6i_dma_config {
u32 dst_burst_lengths;
u32 src_addr_widths;
u32 dst_addr_widths;
- bool has_high_addr;
+ u32 src_high_addr_mask;
+ u32 dst_high_addr_mask;
bool has_mbus_clk;
u32 irq_stride;
u32 irq_en_offset;
@@ -687,9 +692,10 @@ static inline void sun6i_dma_set_addr(struct sun6i_dma_dev *sdev,
v_lli->src = lower_32_bits(src);
v_lli->dst = lower_32_bits(dst);
- if (sdev->cfg->has_high_addr)
- v_lli->para |= SRC_HIGH_ADDR(upper_32_bits(src)) |
- DST_HIGH_ADDR(upper_32_bits(dst));
+ if (sdev->cfg->src_high_addr_mask)
+ v_lli->para |=
+ field_prep(sdev->cfg->src_high_addr_mask, upper_32_bits(src)) |
+ field_prep(sdev->cfg->dst_high_addr_mask, upper_32_bits(dst));
}
static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_memcpy(
@@ -1283,7 +1289,8 @@ static struct sun6i_dma_config sun50i_a100_dma_cfg = {
BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_8_BYTES),
- .has_high_addr = true,
+ .src_high_addr_mask = SRC_HIGH_ADDR_MASK,
+ .dst_high_addr_mask = DST_HIGH_ADDR_MASK,
.has_mbus_clk = true,
SUN6I_DMA_IRQ_A31_COMMON_CFG
};
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v4 3/5] dmaengine: sun6i-dma: Add num_channels_per_reg for flexible interrupt mapping
2026-08-26 1:30 [PATCH v4 0/5] dmaengine: sun6i-dma: Add support for Allwinner A733 DMA controller Yuanshen Cao
2026-08-26 1:30 ` [PATCH v4 1/5] dmaengine: sun6i-dma: Refactor to support A733 interrupt and register handling Yuanshen Cao
2026-08-26 1:30 ` [PATCH v4 2/5] dmaengine: sun6i-dma: Support variable address widths using masks Yuanshen Cao
@ 2026-08-26 1:30 ` Yuanshen Cao
2026-08-26 1:43 ` sashiko-bot
2026-08-26 1:30 ` [PATCH v4 4/5] dt-bindings: dmaengine: sun50i-a64-dma: Add allwinner,sun60i-a733-dma compatible string Yuanshen Cao
2026-08-26 1:30 ` [PATCH v4 5/5] dmaengine: sun6i-dma: Add support for Allwinner A733 DMA controller Yuanshen Cao
4 siblings, 1 reply; 11+ messages in thread
From: Yuanshen Cao @ 2026-08-26 1:30 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maxime Ripard
Cc: Yuanshen Cao, dmaengine, linux-arm-kernel, linux-sunxi,
linux-kernel, devicetree
The `sun6i-dma` driver previously assumed a fixed number of channels
per interrupt register. For example, `sun6i_kill_tasklet` was hardcoded
to disable interrupts only for registers 0 and 1. `DMA_MAX_CHANNELS` was
also not in used previously, and the old SoCs never has more than 16
channels.
The A733 has a different interrupt structure where the number of
channels per register may differ. Add `num_channels_per_reg` to `struct
sun6i_dma_config` to make the interrupt handling logic handware-agnostic
Update `sun6i_dma_interrupt`, `sun6i_dma_start_desc`, and
`sun6i_kill_tasklet` to use this value.
Additionally, set `DMA_MAX_CHANNELS` to 16 to ensure loops over
interrupts are correctly bounded, aligning with the hardware
specifications.
Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
---
drivers/dma/sun6i-dma.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 53c54161e32a..b96d7d90f6b5 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -35,14 +35,13 @@
#define DMA_IRQ_PKG BIT(1)
#define DMA_IRQ_QUEUE BIT(2)
-#define DMA_IRQ_CHAN_NR 8
#define DMA_IRQ_CHAN_WIDTH 4
#define DMA_STAT 0x30
/* Offset between DMA_IRQ_EN and DMA_IRQ_STAT limits number of channels */
-#define DMA_MAX_CHANNELS (DMA_IRQ_CHAN_NR * 0x10 / 4)
+#define DMA_MAX_CHANNELS 16
/*
* sun8i specific registers
@@ -61,6 +60,8 @@
#define DMA_IRQ_EN_OFFSET_A31 0x00
#define DMA_IRQ_STAT_OFFSET_A31 0x10
+#define DMA_IRQ_CHAN_NR_A31 8
+
/*
* Channels specific registers
*/
@@ -157,6 +158,7 @@ struct sun6i_dma_config {
u32 irq_stride;
u32 irq_en_offset;
u32 irq_stat_offset;
+ u32 num_channels_per_reg;
};
/*
@@ -271,7 +273,7 @@ static inline void sun6i_dma_dump_com_regs(struct sun6i_dma_dev *sdev)
{
int i;
- for (i = 0; i < 2; i++) {
+ for (i = 0; i < DIV_ROUND_UP(sdev->num_pchans, sdev->cfg->num_channels_per_reg); i++) {
dev_dbg(sdev->slave.dev, "Common register:\n"
"chan num %d\n"
"\tmask(%04x): 0x%08x\n"
@@ -492,8 +494,8 @@ static int sun6i_dma_start_desc(struct sun6i_vchan *vchan)
sun6i_dma_dump_lli(vchan, pchan->desc->v_lli, pchan->desc->p_lli);
- irq_reg = pchan->idx / DMA_IRQ_CHAN_NR;
- irq_offset = pchan->idx % DMA_IRQ_CHAN_NR;
+ irq_reg = pchan->idx / sdev->cfg->num_channels_per_reg;
+ irq_offset = pchan->idx % sdev->cfg->num_channels_per_reg;
vchan->irq_type = vchan->cyclic ? DMA_IRQ_PKG : DMA_IRQ_QUEUE;
@@ -585,7 +587,7 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
int i, j, ret = IRQ_NONE;
u32 status;
- for (i = 0; i < sdev->num_pchans / DMA_IRQ_CHAN_NR; i++) {
+ for (i = 0; i < sdev->num_pchans / sdev->cfg->num_channels_per_reg; i++) {
status = sun6i_read_irq_stat(sdev, i);
if (!status)
continue;
@@ -595,7 +597,7 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
sun6i_write_irq_stat(sdev, i, status);
- for (j = 0; (j < DMA_IRQ_CHAN_NR) && status; j++) {
+ for (j = 0; (j < sdev->cfg->num_channels_per_reg) && status; j++) {
pchan = sdev->pchans + j;
vchan = pchan->vchan;
if (vchan && (status & vchan->irq_type)) {
@@ -1113,7 +1115,7 @@ static inline void sun6i_kill_tasklet(struct sun6i_dma_dev *sdev)
int i;
/* Disable all interrupts from DMA */
- for (i = 0; i < 2; i++)
+ for (i = 0; i < DMA_MAX_CHANNELS / sdev->cfg->num_channels_per_reg; i++)
sun6i_write_irq_en(sdev, i, 0);
/* Prevent spurious interrupts from scheduling the tasklet */
@@ -1141,7 +1143,8 @@ static inline void sun6i_dma_free(struct sun6i_dma_dev *sdev)
#define SUN6I_DMA_IRQ_A31_COMMON_CFG \
.irq_stride = DMA_IRQ_STRIDE_A31, \
.irq_en_offset = DMA_IRQ_EN_OFFSET_A31, \
- .irq_stat_offset = DMA_IRQ_STAT_OFFSET_A31,
+ .irq_stat_offset = DMA_IRQ_STAT_OFFSET_A31, \
+ .num_channels_per_reg = DMA_IRQ_CHAN_NR_A31,
/*
* For A31:
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v4 4/5] dt-bindings: dmaengine: sun50i-a64-dma: Add allwinner,sun60i-a733-dma compatible string
2026-08-26 1:30 [PATCH v4 0/5] dmaengine: sun6i-dma: Add support for Allwinner A733 DMA controller Yuanshen Cao
` (2 preceding siblings ...)
2026-08-26 1:30 ` [PATCH v4 3/5] dmaengine: sun6i-dma: Add num_channels_per_reg for flexible interrupt mapping Yuanshen Cao
@ 2026-08-26 1:30 ` Yuanshen Cao
2026-08-26 1:30 ` [PATCH v4 5/5] dmaengine: sun6i-dma: Add support for Allwinner A733 DMA controller Yuanshen Cao
4 siblings, 0 replies; 11+ messages in thread
From: Yuanshen Cao @ 2026-08-26 1:30 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maxime Ripard
Cc: Yuanshen Cao, dmaengine, linux-arm-kernel, linux-sunxi,
linux-kernel, devicetree, Frank Li, Krzysztof Kozlowski
Add `allwinner,sun60i-a733-dma` to the list of compatible strings for the
`sun50i-a64-dma` dtbinding documentation.
While the A733 DMA controller shares many similarities with the sun50i-a64
DMA controller, it requires a specific configuration due to differences in:
- Interrupt register layout and mapping.
- Number of channels per interrupt register.
- Support for higher (32G) address widths in LLI parameters.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
---
Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml b/Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml
index c3e14eb6cfff..1cc3304b7414 100644
--- a/Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml
+++ b/Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml
@@ -25,6 +25,7 @@ properties:
- allwinner,sun50i-a64-dma
- allwinner,sun50i-a100-dma
- allwinner,sun50i-h6-dma
+ - allwinner,sun60i-a733-dma
- items:
- const: allwinner,sun8i-r40-dma
- const: allwinner,sun50i-a64-dma
@@ -70,6 +71,7 @@ if:
- allwinner,sun20i-d1-dma
- allwinner,sun50i-a100-dma
- allwinner,sun50i-h6-dma
+ - allwinner,sun60i-a733-dma
then:
properties:
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v4 5/5] dmaengine: sun6i-dma: Add support for Allwinner A733 DMA controller
2026-08-26 1:30 [PATCH v4 0/5] dmaengine: sun6i-dma: Add support for Allwinner A733 DMA controller Yuanshen Cao
` (3 preceding siblings ...)
2026-08-26 1:30 ` [PATCH v4 4/5] dt-bindings: dmaengine: sun50i-a64-dma: Add allwinner,sun60i-a733-dma compatible string Yuanshen Cao
@ 2026-08-26 1:30 ` Yuanshen Cao
2026-08-26 1:43 ` sashiko-bot
2026-08-26 16:26 ` Frank Li
4 siblings, 2 replies; 11+ messages in thread
From: Yuanshen Cao @ 2026-08-26 1:30 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maxime Ripard
Cc: Yuanshen Cao, dmaengine, linux-arm-kernel, linux-sunxi,
linux-kernel, devicetree
Implement support for the Allwinner A733 DMA controller by defining
A733-specific register offsets, bitfield masks, and the
`sun60i_a733_dma_cfg` configuration structure.
This includes the IRQ stride, offsets, and channel counts specific to the
A733, as well as the 32G mask for high-address fields in the LLI. Add
`sun60i_a733_dma_cfg`, which ties all the refactored functionality
together for A733.
Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
---
drivers/dma/sun6i-dma.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index b96d7d90f6b5..fb371b57d792 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -57,10 +57,14 @@
* Interrupts specific registers
*/
#define DMA_IRQ_STRIDE_A31 0x04
+#define DMA_IRQ_STRIDE_A733 0x40
#define DMA_IRQ_EN_OFFSET_A31 0x00
+#define DMA_IRQ_EN_OFFSET_A733 0x134
#define DMA_IRQ_STAT_OFFSET_A31 0x10
+#define DMA_IRQ_STAT_OFFSET_A733 0x138
#define DMA_IRQ_CHAN_NR_A31 8
+#define DMA_IRQ_CHAN_NR_A733 1
/*
* Channels specific registers
@@ -110,6 +114,8 @@
*/
#define SRC_HIGH_ADDR_MASK GENMASK(17, 16)
#define DST_HIGH_ADDR_MASK GENMASK(19, 18)
+#define SRC_HIGH_ADDR_32G_MASK GENMASK(13, 11)
+#define DST_HIGH_ADDR_32G_MASK GENMASK(17, 15)
/*
* Various hardware related defines
@@ -1321,6 +1327,29 @@ static struct sun6i_dma_config sun50i_h6_dma_cfg = {
SUN6I_DMA_IRQ_A31_COMMON_CFG
};
+/*
+ * The A733 binding uses the number of dma channels from the
+ * device tree node.
+ */
+static struct sun6i_dma_config sun60i_a733_dma_cfg = {
+ .clock_autogate_enable = sun6i_enable_clock_autogate_h3,
+ .src_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
+ .dst_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
+ .src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
+ BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
+ BIT(DMA_SLAVE_BUSWIDTH_4_BYTES),
+ .dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
+ BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
+ BIT(DMA_SLAVE_BUSWIDTH_4_BYTES),
+ .src_high_addr_mask = SRC_HIGH_ADDR_32G_MASK,
+ .dst_high_addr_mask = DST_HIGH_ADDR_32G_MASK,
+ .has_mbus_clk = true,
+ .irq_stride = DMA_IRQ_STRIDE_A733,
+ .irq_en_offset = DMA_IRQ_EN_OFFSET_A733,
+ .irq_stat_offset = DMA_IRQ_STAT_OFFSET_A733,
+ .num_channels_per_reg = DMA_IRQ_CHAN_NR_A733,
+};
+
/*
* The V3s have only 8 physical channels, a maximum DRQ port id of 23,
* and a total of 24 usable source and destination endpoints.
@@ -1355,6 +1384,7 @@ static const struct of_device_id sun6i_dma_match[] = {
{ .compatible = "allwinner,sun50i-a64-dma", .data = &sun50i_a64_dma_cfg },
{ .compatible = "allwinner,sun50i-a100-dma", .data = &sun50i_a100_dma_cfg },
{ .compatible = "allwinner,sun50i-h6-dma", .data = &sun50i_h6_dma_cfg },
+ { .compatible = "allwinner,sun60i-a733-dma", .data = &sun60i_a733_dma_cfg },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, sun6i_dma_match);
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v4 2/5] dmaengine: sun6i-dma: Support variable address widths using masks
2026-08-26 1:30 ` [PATCH v4 2/5] dmaengine: sun6i-dma: Support variable address widths using masks Yuanshen Cao
@ 2026-08-26 1:40 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-08-26 1:40 UTC (permalink / raw)
To: Yuanshen Cao
Cc: linux-sunxi, vkoul, dmaengine, conor+dt, Frank.Li, robh,
devicetree
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Local redefinition of standard kernel macros `field_get` and `field_prep` instead of using the generic implementations from `<linux/bitfield.h>`.
- [Low] The newly introduced `field_get` macro is never used.
--
commit f2482b50c417768080bd7de168d315dbd88b3be7
Author: Yuanshen Cao <alex.caoys@gmail.com>
dmaengine: sun6i-dma: Support variable address widths using masks
This patch updates sun6i_dma_set_addr() to use masks for high-address bits,
allowing the driver to support variable address widths for newer Allwinner
SoCs. It also introduces local field_get and field_prep macros.
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index f305fbfb35453..53c54161e32a9 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -24,6 +24,10 @@
>
> #include "virt-dma.h"
>
> +/* Non-constant mask variant of FIELD_GET() and FIELD_PREP() */
> +#define field_get(_mask, _reg) (((_reg) & (_mask)) >> (ffs(_mask) - 1))
[Severity: Low]
Is this macro actually needed?
Looking through the rest of the patch, field_get() doesn't seem to be used
anywhere.
> +#define field_prep(_mask, _val) (((_val) << (ffs(_mask) - 1)) & (_mask))
[Severity: Low]
Could we use the generic implementations from <linux/bitfield.h> instead of
defining field_prep() and field_get() locally?
Redefining these core kernel macros creates a namespace collision, bypasses
the standard bitfield type-safety checks, and risks a build failure if
<linux/bitfield.h> is ever included in this driver.
Additionally, unlike the standard kernel macros, these custom versions
evaluate their arguments multiple times.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260826-sun60i-a733-dma-v4-0-dc2f745f40dc@gmail.com?part=2
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 3/5] dmaengine: sun6i-dma: Add num_channels_per_reg for flexible interrupt mapping
2026-08-26 1:30 ` [PATCH v4 3/5] dmaengine: sun6i-dma: Add num_channels_per_reg for flexible interrupt mapping Yuanshen Cao
@ 2026-08-26 1:43 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-08-26 1:43 UTC (permalink / raw)
To: Yuanshen Cao
Cc: linux-sunxi, devicetree, dmaengine, robh, conor+dt, vkoul,
Frank.Li
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Integer division truncation in `sun6i_dma_interrupt` loop bound silently drops interrupts for higher-numbered channels.
- [High] Incorrect physical channel indexing in `sun6i_dma_interrupt` misroutes interrupts for higher channels.
- [High] TOCTOU race in `sun6i_dma_interrupt` leads to a NULL pointer dereference on channel teardown.
--
commit 0015717509538d0aa3c622981170b196ad6d4d64
Author: Yuanshen Cao <alex.caoys@gmail.com>
dmaengine: sun6i-dma: Add num_channels_per_reg for flexible interrupt mapping
The `sun6i-dma` driver previously assumed a fixed number of channels per
interrupt register. The A733 has a different interrupt structure where the
number of channels per register may differ, so add `num_channels_per_reg` to
`struct sun6i_dma_config` to make the interrupt handling logic hardware-agnostic.
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index 53c54161e32a9..b96d7d90f6b53 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
[ ... ]
> @@ -585,7 +587,7 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
> int i, j, ret = IRQ_NONE;
> u32 status;
>
> - for (i = 0; i < sdev->num_pchans / DMA_IRQ_CHAN_NR; i++) {
> + for (i = 0; i < sdev->num_pchans / sdev->cfg->num_channels_per_reg; i++) {
[Severity: High]
This isn't a bug introduced by this patch, but does this loop silently ignore
higher-numbered channels in sun6i_dma_interrupt() due to integer division
truncation?
If the total number of channels is not perfectly divisible by the channels per
register (e.g. Allwinner H3 has 12 channels, 8 per register), this calculation
truncates the remainder. For H3, 12 / 8 truncates to 1, causing the loop to
only process i = 0 (channels 0-7) and skip i = 1 (channels 8-11), stalling
those higher channels.
The patch correctly uses DIV_ROUND_UP() in sun6i_dma_dump_com_regs(), should
it be used here as well?
> status = sun6i_read_irq_stat(sdev, i);
> if (!status)
> continue;
> @@ -595,7 +597,7 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
>
> sun6i_write_irq_stat(sdev, i, status);
>
> - for (j = 0; (j < DMA_IRQ_CHAN_NR) && status; j++) {
> + for (j = 0; (j < sdev->cfg->num_channels_per_reg) && status; j++) {
> pchan = sdev->pchans + j;
[Severity: High]
This is a pre-existing issue, but does this array indexing misroute interrupts
for channels in subsequent registers?
The index calculation in sun6i_dma_interrupt() completely ignores the register
index `i`. For instance, if an interrupt fires on channel 8 (which is in
register i=1, offset j=0), this will retrieve sdev->pchans[0] instead of
sdev->pchans[8], erroneously completing channel 0's active transfer.
Shouldn't this incorporate i * sdev->cfg->num_channels_per_reg?
> vchan = pchan->vchan;
> if (vchan && (status & vchan->irq_type)) {
[Severity: High]
This is a pre-existing issue, but does this lockless check introduce a TOCTOU
race leading to a NULL pointer dereference on channel teardown?
If a client driver concurrently terminates the channel (e.g., during device
removal) and sun6i_dma_terminate_all() sets pchan->desc to NULL, this code
can evaluate the following block where pchan->desc->vd is dereferenced:
if (vchan->cyclic) {
vchan_cyclic_callback(&pchan->desc->vd);
} else {
spin_lock(&vchan->vc.lock);
vchan_cookie_complete(&pchan->desc->vd);
Since the cyclic path takes no locks and the non-cyclic path doesn't re-verify
pchan->desc after taking the lock, could this crash when the pointer becomes
NULL mid-execution?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260826-sun60i-a733-dma-v4-0-dc2f745f40dc@gmail.com?part=3
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 5/5] dmaengine: sun6i-dma: Add support for Allwinner A733 DMA controller
2026-08-26 1:30 ` [PATCH v4 5/5] dmaengine: sun6i-dma: Add support for Allwinner A733 DMA controller Yuanshen Cao
@ 2026-08-26 1:43 ` sashiko-bot
2026-08-26 16:26 ` Frank Li
1 sibling, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-08-26 1:43 UTC (permalink / raw)
To: Yuanshen Cao
Cc: robh, devicetree, Frank.Li, dmaengine, vkoul, linux-sunxi,
conor+dt
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Missing mandatory function pointers in sun60i_a733_dma_cfg leads to unconditionally dereferencing NULL pointers during DMA preparation.
Pre-existing issues:
- [High] Incorrect physical channel indexing in the DMA interrupt handler maps interrupts for all registers to the first register's channels.
- [High] Missed interrupts due to integer division truncation in the DMA interrupt handler causes higher channel interrupts to be ignored.
--
commit 9fc6c319ac37c3c86b13dd65e2affac0589606b0
Author: Yuanshen Cao <alex.caoys@gmail.com>
dmaengine: sun6i-dma: Add support for Allwinner A733 DMA controller
Implement support for the Allwinner A733 DMA controller by defining
A733-specific register offsets, bitfield masks, and the
sun60i_a733_dma_cfg configuration structure.
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index b96d7d90f6b53..fb371b57d792a 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
[ ... ]
> @@ -1321,6 +1327,29 @@ static struct sun6i_dma_config sun50i_h6_dma_cfg = {
> SUN6I_DMA_IRQ_A31_COMMON_CFG
> };
>
> +/*
> + * The A733 binding uses the number of dma channels from the
> + * device tree node.
> + */
> +static struct sun6i_dma_config sun60i_a733_dma_cfg = {
> + .clock_autogate_enable = sun6i_enable_clock_autogate_h3,
> + .src_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
> + .dst_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
> + .src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> + BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
> + BIT(DMA_SLAVE_BUSWIDTH_4_BYTES),
> + .dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> + BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
> + BIT(DMA_SLAVE_BUSWIDTH_4_BYTES),
> + .src_high_addr_mask = SRC_HIGH_ADDR_32G_MASK,
> + .dst_high_addr_mask = DST_HIGH_ADDR_32G_MASK,
> + .has_mbus_clk = true,
> + .irq_stride = DMA_IRQ_STRIDE_A733,
> + .irq_en_offset = DMA_IRQ_EN_OFFSET_A733,
> + .irq_stat_offset = DMA_IRQ_STAT_OFFSET_A733,
> + .num_channels_per_reg = DMA_IRQ_CHAN_NR_A733,
> +};
> +
[Severity: High]
Does sun60i_a733_dma_cfg omit the set_burst_length, set_drq, and set_mode
function pointers?
When sun6i_dma_prep_dma_memcpy(), sun6i_dma_prep_slave_sg(), or
sun6i_dma_prep_dma_cyclic() are called, they unconditionally invoke these
function pointers:
sdev->cfg->set_burst_length(&v_lli->cfg, burst, burst);
sdev->cfg->set_drq(&v_lli->cfg, DRQ_SDRAM, DRQ_SDRAM);
sdev->cfg->set_mode(&v_lli->cfg, LINEAR_MODE, LINEAR_MODE);
Could this result in a NULL pointer dereference on the A733 platform?
[Severity: High]
This is a pre-existing issue, but does setting num_channels_per_reg to 1 for
the A733 platform expose a physical channel indexing issue in
sun6i_dma_interrupt()?
When an interrupt occurs on a secondary IRQ register, sun6i_dma_interrupt()
reads the correct status register i, but calculates the physical channel
pointer as:
pchan = sdev->pchans + j;
Since it omits the i * sdev->cfg->num_channels_per_reg offset, hardware
interrupts for higher channels appear to be routed to lower channels. On
A733, this would map all channel interrupts to channel 0.
[Severity: High]
This is a pre-existing issue, but does the loop bound in
sun6i_dma_interrupt() truncate due to integer division on platforms with
non-multiple channel counts?
for (i = 0; i < sdev->num_pchans / sdev->cfg->num_channels_per_reg; i++) {
status = sun6i_read_irq_stat(sdev, i);
If a platform has 12 physical channels and 8 channels per register (like H3),
the division evaluates to 1, causing the loop to only read interrupts for
channels 0-7 and ignore the final register. Could this cause transfers on the
highest channels to time out?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260826-sun60i-a733-dma-v4-0-dc2f745f40dc@gmail.com?part=5
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 1/5] dmaengine: sun6i-dma: Refactor to support A733 interrupt and register handling
2026-08-26 1:30 ` [PATCH v4 1/5] dmaengine: sun6i-dma: Refactor to support A733 interrupt and register handling Yuanshen Cao
@ 2026-08-26 1:45 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-08-26 1:45 UTC (permalink / raw)
To: Yuanshen Cao
Cc: dmaengine, linux-sunxi, conor+dt, devicetree, Frank.Li, robh,
vkoul
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Preexisting logic errors in `sun6i_dma_interrupt` regarding outer loop integer division truncation and inner loop physical channel indexing lead to dropped interrupts, unhandled interrupt storms, and use-after-free conditions for upper channels (>= 8).
- [High] Deadlock risk due to acquiring `sdev->lock` without disabling softirqs in process context.
- [High] Lockless read of `vchan->phy` causes TOCTOU race that can pause or stop unrelated DMA channels.
- [High] Data race on IRQ_EN register during driver remove can cause an unhandled IRQ storm.
--
commit c54e18d9ab8b9baf10207d467edd0a2daf494d43
Author: Yuanshen Cao <alex.caoys@gmail.com>
dmaengine: sun6i-dma: Refactor to support A733 interrupt and register handling
The `sun6i-dma` driver currently uses hardcoded logic for interrupt
handling (reading/writing interrupt enable and status registers) and
register dumping.
To support the Allwinner A733, which has a different register layout and
interrupt handling logic, add `irq_stride`, `irq_en_offset`, and
`irq_stat_offset` to `struct sun6i_dma_config`. Implement generic
`sun6i_read/write_irq_*` functions using these new configuration fields
to accommodate different hardware revisions.
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index f47a326dd7ffa..f305fbfb35453 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
[ ... ]
> @@ -549,14 +581,14 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
> u32 status;
>
> for (i = 0; i < sdev->num_pchans / DMA_IRQ_CHAN_NR; i++) {
> - status = readl(sdev->base + DMA_IRQ_STAT(i));
> + status = sun6i_read_irq_stat(sdev, i);
> if (!status)
> continue;
>
> dev_dbg(sdev->slave.dev, "DMA irq status %s: 0x%x\n",
> str_high_low(i), status);
>
> - writel(status, sdev->base + DMA_IRQ_STAT(i));
> + sun6i_write_irq_stat(sdev, i, status);
>
> for (j = 0; (j < DMA_IRQ_CHAN_NR) && status; j++) {
> pchan = sdev->pchans + j;
[Severity: Critical]
This is a pre-existing issue, but does this outer loop truncation and inner
loop physical channel indexing lead to dropped interrupts and unhandled
interrupt storms?
For the outer loop, integer division truncates the number of loops for SoCs
with non-multiple-of-8 channel counts (e.g., H3 with 12 channels). This
ignores interrupts for channels 8-11, causing unhandled hardware interrupts
and CPU hangs.
For the inner loop, does the array offset use only `j` without `i`? For
channels in higher registers (like channel 8, where `i=1`), this incorrectly
attributes the interrupt to channels 0-7 instead of
`i * num_channels_per_reg + j`.
This causes the DMA framework to prematurely complete active transfers on the
wrong channels, leading to use-after-free conditions, data corruption, and
dropped interrupts for the true channel.
Note that for the newly supported A733 SoC, which uses
`num_channels_per_reg = 1`, all interrupts for channels 1-15 will incorrectly
map to channel 0, rendering the DMA controller completely non-functional.
[ ... ]
> @@ -1072,9 +1104,11 @@ static struct dma_chan *sun6i_dma_of_xlate(struct of_phandle_args *dma_spec,
>
> static inline void sun6i_kill_tasklet(struct sun6i_dma_dev *sdev)
> {
> + int i;
> +
> /* Disable all interrupts from DMA */
> - writel(0, sdev->base + DMA_IRQ_EN(0));
> - writel(0, sdev->base + DMA_IRQ_EN(1));
> + for (i = 0; i < 2; i++)
> + sun6i_write_irq_en(sdev, i, 0);
[Severity: High]
This is a pre-existing issue, but can this unlocked write to the IRQ_EN
register race with the tasklet's read-modify-write and leave interrupts
enabled?
During driver unload, sun6i_kill_tasklet() writes 0 to the IRQ_EN register
to disable hardware interrupts without any lock. Concurrently, if a channel
is still completing, sun6i_dma_tasklet() can run on another CPU, calling
sun6i_dma_start_desc() which performs an unprotected read-modify-write on
the same IRQ_EN register.
The tasklet can overwrite the 0 written by sun6i_kill_tasklet() with a
non-zero interrupt enable mask. Because the hardware interrupt is left
enabled but the IRQ handler is immediately freed by devm_free_irq(), any
subsequent DMA interrupt assertion will trigger an unhandled IRQ storm,
permanently hanging the CPU.
[ ... ]
Additionally, there are a few other pre-existing issues in this file:
[Severity: High]
This is a pre-existing issue, but does acquiring sdev->lock without
disabling softirqs in process context create a deadlock risk?
Functions like sun6i_dma_terminate_all() and sun6i_dma_pause() can be
called from process context, acquiring sdev->lock using plain spin_lock():
drivers/dma/sun6i-dma.c:sun6i_dma_terminate_all() {
...
spin_lock(&sdev->lock);
list_del_init(&vchan->node);
spin_unlock(&sdev->lock);
...
}
Concurrently, sun6i_dma_tasklet() executes in softirq context and acquires
the same lock using spin_lock_irq(&sdev->lock). If a hardware interrupt
occurs on the same CPU while the process context holds sdev->lock, the
returning interrupt will trigger do_softirq().
The sun6i_dma_tasklet() will then execute, attempting to acquire sdev->lock
and spinning forever, causing a CPU deadlock.
[Severity: High]
This is a pre-existing issue, but does a lockless read of vchan->phy cause
a TOCTOU race that can pause or stop unrelated DMA channels?
In sun6i_dma_pause(), sun6i_dma_terminate_all(), sun6i_dma_resume(), and
sun6i_dma_tx_status(), vchan->phy is read into a local pchan variable
before any locks are acquired:
drivers/dma/sun6i-dma.c:sun6i_dma_pause() {
...
struct sun6i_pchan *pchan = vchan->phy;
...
}
Concurrently, sun6i_dma_tasklet() can finish a transfer, clear vchan->phy,
and immediately reallocate the physical channel to a completely different
virtual channel. The original functions then proceed to use the stale
pchan pointer without holding locks, inadvertently stopping, pausing, or
querying an unrelated active DMA transfer. Can this lead to data corruption
and hardware timeouts?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260826-sun60i-a733-dma-v4-0-dc2f745f40dc@gmail.com?part=1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 5/5] dmaengine: sun6i-dma: Add support for Allwinner A733 DMA controller
2026-08-26 1:30 ` [PATCH v4 5/5] dmaengine: sun6i-dma: Add support for Allwinner A733 DMA controller Yuanshen Cao
2026-08-26 1:43 ` sashiko-bot
@ 2026-08-26 16:26 ` Frank Li
1 sibling, 0 replies; 11+ messages in thread
From: Frank Li @ 2026-08-26 16:26 UTC (permalink / raw)
To: Yuanshen Cao
Cc: Vinod Koul, Frank Li, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maxime Ripard, dmaengine, linux-arm-kernel, linux-sunxi,
linux-kernel, devicetree
On Wed, Aug 26, 2026 at 01:30:07AM +0000, Yuanshen Cao wrote:
> Implement support for the Allwinner A733 DMA controller by defining
> A733-specific register offsets, bitfield masks, and the
> `sun60i_a733_dma_cfg` configuration structure.
>
> This includes the IRQ stride, offsets, and channel counts specific to the
> A733, as well as the 32G mask for high-address fields in the LLI. Add
> `sun60i_a733_dma_cfg`, which ties all the refactored functionality
> together for A733.
>
> Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
> ---
> drivers/dma/sun6i-dma.c | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index b96d7d90f6b5..fb371b57d792 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -57,10 +57,14 @@
> * Interrupts specific registers
> */
> #define DMA_IRQ_STRIDE_A31 0x04
> +#define DMA_IRQ_STRIDE_A733 0x40
> #define DMA_IRQ_EN_OFFSET_A31 0x00
> +#define DMA_IRQ_EN_OFFSET_A733 0x134
> #define DMA_IRQ_STAT_OFFSET_A31 0x10
> +#define DMA_IRQ_STAT_OFFSET_A733 0x138
>
> #define DMA_IRQ_CHAN_NR_A31 8
> +#define DMA_IRQ_CHAN_NR_A733 1
>
> /*
> * Channels specific registers
> @@ -110,6 +114,8 @@
> */
> #define SRC_HIGH_ADDR_MASK GENMASK(17, 16)
> #define DST_HIGH_ADDR_MASK GENMASK(19, 18)
> +#define SRC_HIGH_ADDR_32G_MASK GENMASK(13, 11)
> +#define DST_HIGH_ADDR_32G_MASK GENMASK(17, 15)
>
> /*
> * Various hardware related defines
> @@ -1321,6 +1327,29 @@ static struct sun6i_dma_config sun50i_h6_dma_cfg = {
> SUN6I_DMA_IRQ_A31_COMMON_CFG
> };
>
> +/*
> + * The A733 binding uses the number of dma channels from the
> + * device tree node.
> + */
> +static struct sun6i_dma_config sun60i_a733_dma_cfg = {
> + .clock_autogate_enable = sun6i_enable_clock_autogate_h3,
> + .src_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
> + .dst_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
> + .src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> + BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
> + BIT(DMA_SLAVE_BUSWIDTH_4_BYTES),
> + .dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> + BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
> + BIT(DMA_SLAVE_BUSWIDTH_4_BYTES),
> + .src_high_addr_mask = SRC_HIGH_ADDR_32G_MASK,
> + .dst_high_addr_mask = DST_HIGH_ADDR_32G_MASK,
> + .has_mbus_clk = true,
> + .irq_stride = DMA_IRQ_STRIDE_A733,
> + .irq_en_offset = DMA_IRQ_EN_OFFSET_A733,
> + .irq_stat_offset = DMA_IRQ_STAT_OFFSET_A733,
> + .num_channels_per_reg = DMA_IRQ_CHAN_NR_A733,
Nit: if only use once, needn't define macro, you can put value to here.
Also please check previous prepare patch.
Frank
> +};
> +
> /*
> * The V3s have only 8 physical channels, a maximum DRQ port id of 23,
> * and a total of 24 usable source and destination endpoints.
> @@ -1355,6 +1384,7 @@ static const struct of_device_id sun6i_dma_match[] = {
> { .compatible = "allwinner,sun50i-a64-dma", .data = &sun50i_a64_dma_cfg },
> { .compatible = "allwinner,sun50i-a100-dma", .data = &sun50i_a100_dma_cfg },
> { .compatible = "allwinner,sun50i-h6-dma", .data = &sun50i_h6_dma_cfg },
> + { .compatible = "allwinner,sun60i-a733-dma", .data = &sun60i_a733_dma_cfg },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, sun6i_dma_match);
>
> --
> 2.55.0
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-08-26 16:27 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-26 1:30 [PATCH v4 0/5] dmaengine: sun6i-dma: Add support for Allwinner A733 DMA controller Yuanshen Cao
2026-08-26 1:30 ` [PATCH v4 1/5] dmaengine: sun6i-dma: Refactor to support A733 interrupt and register handling Yuanshen Cao
2026-08-26 1:45 ` sashiko-bot
2026-08-26 1:30 ` [PATCH v4 2/5] dmaengine: sun6i-dma: Support variable address widths using masks Yuanshen Cao
2026-08-26 1:40 ` sashiko-bot
2026-08-26 1:30 ` [PATCH v4 3/5] dmaengine: sun6i-dma: Add num_channels_per_reg for flexible interrupt mapping Yuanshen Cao
2026-08-26 1:43 ` sashiko-bot
2026-08-26 1:30 ` [PATCH v4 4/5] dt-bindings: dmaengine: sun50i-a64-dma: Add allwinner,sun60i-a733-dma compatible string Yuanshen Cao
2026-08-26 1:30 ` [PATCH v4 5/5] dmaengine: sun6i-dma: Add support for Allwinner A733 DMA controller Yuanshen Cao
2026-08-26 1:43 ` sashiko-bot
2026-08-26 16:26 ` Frank Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox