[PATCH 0/5] dmaengine: sun6i-dma: Add support for Allwinner A733 DMA controller

[PATCH 0/5] dmaengine: sun6i-dma: Add support for Allwinner A733 DMA controller

[Yuanshen Cao]

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:
1. It supports higher address (up to 32G).
2. It uses a different interrupt register layout and mapping.
3. It has a different number of channels per interrupt register.

To support these differences without introducing complex conditional
logic throughout the driver, this series first refactors the
`sun6i_dma_config` structure. By moving interrupt handling, register
dumping, and address configuration into function pointers within the
configuration structure. This allows the driver to support the A733 
and future hardware revisions. It also aligns with the DMA drivers in
Radxa BSP Package[1].

The series is organized as follows:
- Refactors the configuration structure to include function pointers for
  interrupt and register operations.
- Moves address setting logic into the configuration structure to handle
  varying address widths.
- Adds support for variable channels per interrupt register.
- Implements the A733-specific configuration and register mappings.
- Updates the device tree bindings documentation.

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>
---
Yuanshen Cao (5):
      dmaengine: sun6i-dma: Refactor to support A733 interrupt and register handling
      dmaengine: sun6i-dma: Add set_addr function pointer for variable address widths
      dmaengine: sun6i-dma: Add num_channels_per_reg for flexible interrupt mapping
      dmaengine: sun6i-dma: Implement support for Allwinner A733 DMA controller
      dt-bindings: dma: sun50i-a64-dma: Update device tree bindings documentation for A733

 .../bindings/dma/allwinner,sun50i-a64-dma.yaml     |   2 +
 drivers/dma/sun6i-dma.c                            | 223 +++++++++++++++++++--
 2 files changed, 203 insertions(+), 22 deletions(-)
---
base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6
change-id: 20260619-sun60i-a733-dma-c2455149165d

Best regards,
--  
Yuanshen Cao <alex.caoys@gmail.com>

[PATCH 1/5] dmaengine: sun6i-dma: Refactor to support A733 interrupt and register handling

[Yuanshen Cao]

This patch is the first step in a refactoring effort to support the
Allwinner A733 DMA controller. Currently, the `sun6i-dma` driver has
several functions related to interrupt handling (reading/writing
interrupt enable and status registers) and register dumping that are
hardcoded.

To support the A733, which has different register layouts and interrupt
handling logic, these functions are being moved into the
`sun6i_dma_config` structure as function pointers. This allows the
driver to use a polymorphic approach where the specific implementation
is determined by the hardware configuration assigned during device
probing.

Changes:
- Added function pointers to `struct sun6i_dma_config` for:
    - `dump_com_regs`
    - `read_irq_en`
    - `write_irq_en`
    - `read_irq_stat`
    - `write_irq_stat`
- Implemented generic `sun6i_read/write_irq_*` functions for existing
  hardware.
- Updated existing `sun6i_dma_config` instances (A31, A23, H3, A64,
  A100, H6, V3S) to use these new function pointers.

Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
---
 drivers/dma/sun6i-dma.c | 74 +++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 69 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index a9a254dbf8cb..d92e702320d9 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -138,6 +138,11 @@ struct sun6i_dma_config {
 	void (*set_burst_length)(u32 *p_cfg, s8 src_burst, s8 dst_burst);
 	void (*set_drq)(u32 *p_cfg, s8 src_drq, s8 dst_drq);
 	void (*set_mode)(u32 *p_cfg, s8 src_mode, s8 dst_mode);
+	void (*dump_com_regs)(struct sun6i_dma_dev *sdev);
+	u32 (*read_irq_en)(struct sun6i_dma_dev *sdev, u32 chan_num);
+	void (*write_irq_en)(struct sun6i_dma_dev *sdev, u32 chan_num, u32 irq_val);
+	u32 (*read_irq_stat)(struct sun6i_dma_dev *sdev, u32 chan_num);
+	void (*write_irq_stat)(struct sun6i_dma_dev *sdev, u32 chan_num, u32 status);
 	u32 src_burst_lengths;
 	u32 dst_burst_lengths;
 	u32 src_addr_widths;
@@ -347,6 +352,25 @@ static void sun6i_set_mode_h6(u32 *p_cfg, s8 src_mode, s8 dst_mode)
 		  DMA_CHAN_CFG_DST_MODE_H6(dst_mode);
 }
 
+static u32 sun6i_read_irq_en(struct sun6i_dma_dev *sdev, u32 chan_num)
+{
+	return readl(sdev->base + DMA_IRQ_EN(chan_num));
+}
+
+static void sun6i_write_irq_en(struct sun6i_dma_dev *sdev, u32 chan_num, u32 irq_val)
+{
+	writel(irq_val, sdev->base + DMA_IRQ_EN(chan_num));
+}
+static u32 sun6i_read_irq_stat(struct sun6i_dma_dev *sdev, u32 chan_num)
+{
+	return readl(sdev->base + DMA_IRQ_STAT(chan_num));
+}
+
+static void sun6i_write_irq_stat(struct sun6i_dma_dev *sdev, u32 chan_num, u32 status)
+{
+	writel(status, sdev->base + DMA_IRQ_STAT(chan_num));
+}
+
 static size_t sun6i_get_chan_size(struct sun6i_pchan *pchan)
 {
 	struct sun6i_desc *txd = pchan->desc;
@@ -460,16 +484,16 @@ 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 = sdev->cfg->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));
+	sdev->cfg->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);
 
-	sun6i_dma_dump_com_regs(sdev);
+	sdev->cfg->dump_com_regs(sdev);
 	sun6i_dma_dump_chan_regs(sdev, pchan);
 
 	return 0;
@@ -549,14 +573,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 = sdev->cfg->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));
+		sdev->cfg->write_irq_stat(sdev, i, status);
 
 		for (j = 0; (j < DMA_IRQ_CHAN_NR) && status; j++) {
 			pchan = sdev->pchans + j;
@@ -1124,6 +1148,11 @@ static struct sun6i_dma_config sun6i_a31_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_a31,
 	.set_drq          = sun6i_set_drq_a31,
 	.set_mode         = sun6i_set_mode_a31,
+	.dump_com_regs    = sun6i_dma_dump_com_regs,
+	.read_irq_en      = sun6i_read_irq_en,
+	.write_irq_en     = sun6i_write_irq_en,
+	.read_irq_stat    = sun6i_read_irq_stat,
+	.write_irq_stat   = sun6i_write_irq_stat,
 	.src_burst_lengths = BIT(1) | BIT(8),
 	.dst_burst_lengths = BIT(1) | BIT(8),
 	.src_addr_widths   = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
@@ -1147,6 +1176,11 @@ static struct sun6i_dma_config sun8i_a23_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_a31,
 	.set_drq          = sun6i_set_drq_a31,
 	.set_mode         = sun6i_set_mode_a31,
+	.dump_com_regs    = sun6i_dma_dump_com_regs,
+	.read_irq_en      = sun6i_read_irq_en,
+	.write_irq_en     = sun6i_write_irq_en,
+	.read_irq_stat    = sun6i_read_irq_stat,
+	.write_irq_stat   = sun6i_write_irq_stat,
 	.src_burst_lengths = BIT(1) | BIT(8),
 	.dst_burst_lengths = BIT(1) | BIT(8),
 	.src_addr_widths   = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
@@ -1165,6 +1199,11 @@ static struct sun6i_dma_config sun8i_a83t_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_a31,
 	.set_drq          = sun6i_set_drq_a31,
 	.set_mode         = sun6i_set_mode_a31,
+	.dump_com_regs    = sun6i_dma_dump_com_regs,
+	.read_irq_en      = sun6i_read_irq_en,
+	.write_irq_en     = sun6i_write_irq_en,
+	.read_irq_stat    = sun6i_read_irq_stat,
+	.write_irq_stat   = sun6i_write_irq_stat,
 	.src_burst_lengths = BIT(1) | BIT(8),
 	.dst_burst_lengths = BIT(1) | BIT(8),
 	.src_addr_widths   = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
@@ -1190,6 +1229,11 @@ static struct sun6i_dma_config sun8i_h3_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_h3,
 	.set_drq          = sun6i_set_drq_a31,
 	.set_mode         = sun6i_set_mode_a31,
+	.dump_com_regs    = sun6i_dma_dump_com_regs,
+	.read_irq_en      = sun6i_read_irq_en,
+	.write_irq_en     = sun6i_write_irq_en,
+	.read_irq_stat    = sun6i_read_irq_stat,
+	.write_irq_stat   = sun6i_write_irq_stat,
 	.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) |
@@ -1211,6 +1255,11 @@ static struct sun6i_dma_config sun50i_a64_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_h3,
 	.set_drq          = sun6i_set_drq_a31,
 	.set_mode         = sun6i_set_mode_a31,
+	.dump_com_regs    = sun6i_dma_dump_com_regs,
+	.read_irq_en      = sun6i_read_irq_en,
+	.write_irq_en     = sun6i_write_irq_en,
+	.read_irq_stat    = sun6i_read_irq_stat,
+	.write_irq_stat   = sun6i_write_irq_stat,
 	.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) |
@@ -1232,6 +1281,11 @@ static struct sun6i_dma_config sun50i_a100_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_h3,
 	.set_drq          = sun6i_set_drq_h6,
 	.set_mode         = sun6i_set_mode_h6,
+	.dump_com_regs    = sun6i_dma_dump_com_regs,
+	.read_irq_en      = sun6i_read_irq_en,
+	.write_irq_en     = sun6i_write_irq_en,
+	.read_irq_stat    = sun6i_read_irq_stat,
+	.write_irq_stat   = sun6i_write_irq_stat,
 	.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) |
@@ -1255,6 +1309,11 @@ static struct sun6i_dma_config sun50i_h6_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_h3,
 	.set_drq          = sun6i_set_drq_h6,
 	.set_mode         = sun6i_set_mode_h6,
+	.dump_com_regs    = sun6i_dma_dump_com_regs,
+	.read_irq_en      = sun6i_read_irq_en,
+	.write_irq_en     = sun6i_write_irq_en,
+	.read_irq_stat    = sun6i_read_irq_stat,
+	.write_irq_stat   = sun6i_write_irq_stat,
 	.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) |
@@ -1281,6 +1340,11 @@ static struct sun6i_dma_config sun8i_v3s_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_a31,
 	.set_drq          = sun6i_set_drq_a31,
 	.set_mode         = sun6i_set_mode_a31,
+	.dump_com_regs    = sun6i_dma_dump_com_regs,
+	.read_irq_en      = sun6i_read_irq_en,
+	.write_irq_en     = sun6i_write_irq_en,
+	.read_irq_stat    = sun6i_read_irq_stat,
+	.write_irq_stat   = sun6i_write_irq_stat,
 	.src_burst_lengths = BIT(1) | BIT(8),
 	.dst_burst_lengths = BIT(1) | BIT(8),
 	.src_addr_widths   = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |

-- 
2.54.0

Re: [PATCH 1/5] dmaengine: sun6i-dma: Refactor to support A733 interrupt and register handling

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [Low] The parameter `chan_num` in the newly introduced `read_irq_en` and `write_irq_en` callbacks is highly misleading because it expects a register index, not a channel number.

Pre-existing issues:
- [Critical] `sun6i_dma_interrupt` incorrectly maps interrupts for higher channels to the wrong physical channel structs because it ignores the outer register index.
- [High] Integer division truncation in the `sun6i_dma_interrupt` loop bound completely ignores interrupts for higher channels on SoCs where the channel count is not a multiple of the channels per register.
- [High] `sun6i_dma_terminate_all` and `sun6i_dma_pause`/`resume` acquire `sdev->lock` without disabling softirqs, creating a deadlock with the DMA tasklet.
- [High] `sun6i_dma_interrupt` accesses `vchan->cyclic` and dereferences `pchan->desc` without holding `vc.lock`, racing with channel teardown.
--

commit 47f7d45b0543fc62a4eb37d06b46eeffe4dd06c6
Author: Yuanshen Cao <alex.caoys@gmail.com>

dmaengine: sun6i-dma: Refactor to support A733 interrupt and register handling

This patch refactors the sun6i-dma driver to support the Allwinner A733 DMA
controller. It moves interrupt handling and register dumping functions into
the sun6i_dma_config structure to handle different register layouts.

> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index a9a254dbf8cb9..d92e702320d9f 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -138,6 +138,11 @@ struct sun6i_dma_config {
>  	void (*set_burst_length)(u32 *p_cfg, s8 src_burst, s8 dst_burst);
>  	void (*set_drq)(u32 *p_cfg, s8 src_drq, s8 dst_drq);
>  	void (*set_mode)(u32 *p_cfg, s8 src_mode, s8 dst_mode);
> +	void (*dump_com_regs)(struct sun6i_dma_dev *sdev);
> +	u32 (*read_irq_en)(struct sun6i_dma_dev *sdev, u32 chan_num);
> +	void (*write_irq_en)(struct sun6i_dma_dev *sdev, u32 chan_num, u32 irq_val);

[Severity: Low]
Is the chan_num parameter name misleading here?

Looking at sun6i_dma_start_desc(), the value passed to these callbacks is
actually the register index (irq_reg), not the physical channel number.

Could this cause confusion for future implementers who might try to use it
as a literal channel index?

[ ... ]

> @@ -549,14 +573,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++) {

[Severity: High]
This is a pre-existing issue, but does this loop bound properly handle
platforms where the channel count is not a multiple of the channels per
register?

Because integer division truncates, a platform like sun8i-h3 (which defines
nr_max_channels = 12) will evaluate 12 / 8 as 1.

Does this mean the loop only runs for i = 0 (handling channels 0-7) and
completely drops interrupts for channels 8-11, potentially leading to an
unhandled hardware interrupt storm? Should this use DIV_ROUND_UP?

> -		status = readl(sdev->base + DMA_IRQ_STAT(i));
> +		status = sdev->cfg->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));
> +		sdev->cfg->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 correctly calculate the physical
channel pointer for higher channels?

Inside this interrupt loop, i represents the interrupt register index and j
is the local bit offset within that register. By calculating the physical
channel as sdev->pchans + j, it completely ignores the outer register index
offset.

If an interrupt triggers on physical channel 8 (i=1, j=0), won't the code
incorrectly assign it to sdev->pchans + 0?

Could this incorrectly complete the active transfer for channel 0, causing
use-after-free issues and DMA state corruption? Should the assignment be
sdev->pchans + j + i * sdev->cfg->num_channels_per_reg?


[Severity: High]
This is a pre-existing issue, but does sun6i_dma_interrupt() have a race
condition with channel teardown?

In the hardirq handler sun6i_dma_interrupt(), vchan->cyclic is checked
locklessly:

drivers/dma/sun6i-dma.c:sun6i_dma_interrupt() {
	...
	if (vchan->cyclic) {
		vchan_cyclic_callback(&pchan->desc->vd);
	}
	...
}

Concurrently, sun6i_dma_terminate_all() can run on another CPU, acquire
vchan->vc.lock, set vchan->cyclic = false, set pchan->desc = NULL, and free
the descriptors:

drivers/dma/sun6i-dma.c:sun6i_dma_terminate_all() {
	...
	vchan->cyclic = false;
	...
}

If the IRQ handler checks vchan->cyclic just before it is modified, could it
proceed to dereference pchan->desc after it has been nulled or freed?


[Severity: High]
This is a pre-existing issue, but do sun6i_dma_terminate_all(),
sun6i_dma_pause(), and sun6i_dma_resume() have a deadlock window?

These functions acquire sdev->lock using a plain spin_lock() from process
context:

drivers/dma/sun6i-dma.c:sun6i_dma_terminate_all() {
	...
	spin_lock(&sdev->lock);
	...
}

However, sdev->lock is also acquired by sun6i_dma_tasklet(), which runs in
softirq context.

If a hard interrupt occurs on the same CPU while this lock is held, and the
interrupt handler returns and processes pending softirqs (including the
scheduled sun6i_dma_tasklet()), won't the tasklet attempt to acquire
sdev->lock using spin_lock_irq(), resulting in a same-CPU ABBA deadlock?

Should process context use spin_lock_bh() or spin_lock_irqsave() when locking
resources shared with softirqs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260619-sun60i-a733-dma-v1-0-da4b649fc72a@gmail.com?part=1

[PATCH 2/5] dmaengine: sun6i-dma: Add set_addr function pointer for variable address widths

[Yuanshen Cao]

The A733 DMA controller supports higher address (up to 32G) compared to
previous generations. The existing `sun6i_dma_set_addr` function uses a
hardcoded logic for setting the high-address bits in the LLI parameters.

By moving `set_addr` into the `sun6i_dma_config` structure, we can
provide specialized implementations for different hardware. This allows
the A733 to use a version of `set_addr` that correctly handles its
specific `SRC_HIGH_ADDR_32G` and `DST_HIGH_ADDR_32G` in the `set_addr`
register later in the series.

Changes:
- Added `set_addr` function pointer to `struct sun6i_dma_config`.
- Refactored `sun6i_dma_set_addr` and introduced
  `sun6i_dma_set_addr_a100` (keeping the logic for high address
  support).
- Updated all existing configuration structs to include the new
  `set_addr` pointer.
- Removed `has_high_addr` since the logic is replaced by
  `sun6i_dma_set_addr_a100`.

Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
---
 drivers/dma/sun6i-dma.c | 36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index d92e702320d9..059455425e19 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -112,6 +112,7 @@
 
 /* forward declaration */
 struct sun6i_dma_dev;
+struct sun6i_dma_lli;
 
 /*
  * Hardware channels / ports representation
@@ -138,6 +139,8 @@ struct sun6i_dma_config {
 	void (*set_burst_length)(u32 *p_cfg, s8 src_burst, s8 dst_burst);
 	void (*set_drq)(u32 *p_cfg, s8 src_drq, s8 dst_drq);
 	void (*set_mode)(u32 *p_cfg, s8 src_mode, s8 dst_mode);
+	void (*set_addr)(struct sun6i_dma_dev *sdev, struct sun6i_dma_lli *v_lli,
+		dma_addr_t src, dma_addr_t dst);
 	void (*dump_com_regs)(struct sun6i_dma_dev *sdev);
 	u32 (*read_irq_en)(struct sun6i_dma_dev *sdev, u32 chan_num);
 	void (*write_irq_en)(struct sun6i_dma_dev *sdev, u32 chan_num, u32 irq_val);
@@ -147,7 +150,6 @@ struct sun6i_dma_config {
 	u32 dst_burst_lengths;
 	u32 src_addr_widths;
 	u32 dst_addr_widths;
-	bool has_high_addr;
 	bool has_mbus_clk;
 };
 
@@ -675,13 +677,20 @@ static int set_config(struct sun6i_dma_dev *sdev,
 static inline void sun6i_dma_set_addr(struct sun6i_dma_dev *sdev,
 				      struct sun6i_dma_lli *v_lli,
 				      dma_addr_t src, dma_addr_t dst)
+{
+	v_lli->src = lower_32_bits(src);
+	v_lli->dst = lower_32_bits(dst);
+}
+
+static inline void sun6i_dma_set_addr_a100(struct sun6i_dma_dev *sdev,
+				      struct sun6i_dma_lli *v_lli,
+				      dma_addr_t src, dma_addr_t dst)
 {
 	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));
+	v_lli->para |= SRC_HIGH_ADDR(upper_32_bits(src)) |
+				DST_HIGH_ADDR(upper_32_bits(dst));
 }
 
 static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_memcpy(
@@ -714,7 +723,7 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_memcpy(
 
 	v_lli->len = len;
 	v_lli->para = NORMAL_WAIT;
-	sun6i_dma_set_addr(sdev, v_lli, src, dest);
+	sdev->cfg->set_addr(sdev, v_lli, src, dest);
 
 	burst = convert_burst(8);
 	width = convert_buswidth(DMA_SLAVE_BUSWIDTH_4_BYTES);
@@ -773,7 +782,7 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_slave_sg(
 		v_lli->para = NORMAL_WAIT;
 
 		if (dir == DMA_MEM_TO_DEV) {
-			sun6i_dma_set_addr(sdev, v_lli,
+			sdev->cfg->set_addr(sdev, v_lli,
 					   sg_dma_address(sg),
 					   sconfig->dst_addr);
 			v_lli->cfg = lli_cfg;
@@ -787,7 +796,7 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_slave_sg(
 				sg_dma_len(sg), flags);
 
 		} else {
-			sun6i_dma_set_addr(sdev, v_lli,
+			sdev->cfg->set_addr(sdev, v_lli,
 					   sconfig->src_addr,
 					   sg_dma_address(sg));
 			v_lli->cfg = lli_cfg;
@@ -858,7 +867,7 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_cyclic(
 		v_lli->para = NORMAL_WAIT;
 
 		if (dir == DMA_MEM_TO_DEV) {
-			sun6i_dma_set_addr(sdev, v_lli,
+			sdev->cfg->set_addr(sdev, v_lli,
 					   buf_addr + period_len * i,
 					   sconfig->dst_addr);
 			v_lli->cfg = lli_cfg;
@@ -870,7 +879,7 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_cyclic(
 				&sconfig->dst_addr, &buf_addr,
 				buf_len, flags);
 		} else {
-			sun6i_dma_set_addr(sdev, v_lli,
+			sdev->cfg->set_addr(sdev, v_lli,
 					   sconfig->src_addr,
 					   buf_addr + period_len * i);
 			v_lli->cfg = lli_cfg;
@@ -1148,6 +1157,7 @@ static struct sun6i_dma_config sun6i_a31_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_a31,
 	.set_drq          = sun6i_set_drq_a31,
 	.set_mode         = sun6i_set_mode_a31,
+	.set_addr         = sun6i_dma_set_addr,
 	.dump_com_regs    = sun6i_dma_dump_com_regs,
 	.read_irq_en      = sun6i_read_irq_en,
 	.write_irq_en     = sun6i_write_irq_en,
@@ -1176,6 +1186,7 @@ static struct sun6i_dma_config sun8i_a23_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_a31,
 	.set_drq          = sun6i_set_drq_a31,
 	.set_mode         = sun6i_set_mode_a31,
+	.set_addr         = sun6i_dma_set_addr,
 	.dump_com_regs    = sun6i_dma_dump_com_regs,
 	.read_irq_en      = sun6i_read_irq_en,
 	.write_irq_en     = sun6i_write_irq_en,
@@ -1199,6 +1210,7 @@ static struct sun6i_dma_config sun8i_a83t_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_a31,
 	.set_drq          = sun6i_set_drq_a31,
 	.set_mode         = sun6i_set_mode_a31,
+	.set_addr         = sun6i_dma_set_addr,
 	.dump_com_regs    = sun6i_dma_dump_com_regs,
 	.read_irq_en      = sun6i_read_irq_en,
 	.write_irq_en     = sun6i_write_irq_en,
@@ -1229,6 +1241,7 @@ static struct sun6i_dma_config sun8i_h3_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_h3,
 	.set_drq          = sun6i_set_drq_a31,
 	.set_mode         = sun6i_set_mode_a31,
+	.set_addr         = sun6i_dma_set_addr,
 	.dump_com_regs    = sun6i_dma_dump_com_regs,
 	.read_irq_en      = sun6i_read_irq_en,
 	.write_irq_en     = sun6i_write_irq_en,
@@ -1255,6 +1268,7 @@ static struct sun6i_dma_config sun50i_a64_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_h3,
 	.set_drq          = sun6i_set_drq_a31,
 	.set_mode         = sun6i_set_mode_a31,
+	.set_addr         = sun6i_dma_set_addr,
 	.dump_com_regs    = sun6i_dma_dump_com_regs,
 	.read_irq_en      = sun6i_read_irq_en,
 	.write_irq_en     = sun6i_write_irq_en,
@@ -1281,6 +1295,7 @@ static struct sun6i_dma_config sun50i_a100_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_h3,
 	.set_drq          = sun6i_set_drq_h6,
 	.set_mode         = sun6i_set_mode_h6,
+	.set_addr         = sun6i_dma_set_addr_a100,
 	.dump_com_regs    = sun6i_dma_dump_com_regs,
 	.read_irq_en      = sun6i_read_irq_en,
 	.write_irq_en     = sun6i_write_irq_en,
@@ -1296,7 +1311,6 @@ 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,
 	.has_mbus_clk = true,
 };
 
@@ -1309,6 +1323,7 @@ static struct sun6i_dma_config sun50i_h6_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_h3,
 	.set_drq          = sun6i_set_drq_h6,
 	.set_mode         = sun6i_set_mode_h6,
+	.set_addr         = sun6i_dma_set_addr,
 	.dump_com_regs    = sun6i_dma_dump_com_regs,
 	.read_irq_en      = sun6i_read_irq_en,
 	.write_irq_en     = sun6i_write_irq_en,
@@ -1340,6 +1355,7 @@ static struct sun6i_dma_config sun8i_v3s_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_a31,
 	.set_drq          = sun6i_set_drq_a31,
 	.set_mode         = sun6i_set_mode_a31,
+	.set_addr         = sun6i_dma_set_addr,
 	.dump_com_regs    = sun6i_dma_dump_com_regs,
 	.read_irq_en      = sun6i_read_irq_en,
 	.write_irq_en     = sun6i_write_irq_en,

-- 
2.54.0

Re: [PATCH 2/5] dmaengine: sun6i-dma: Add set_addr function pointer for variable address widths

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Use-after-free vulnerability in the error handling paths of `sun6i_dma_prep_slave_sg` and `sun6i_dma_prep_dma_cyclic` when freeing DMA pool items.
--

commit ea99a7147f9100709dbaea854d96eccdbc89f235
Author: Yuanshen Cao <alex.caoys@gmail.com>

dmaengine: sun6i-dma: Add set_addr function pointer for variable address widths

The A733 DMA controller supports higher address (up to 32G) compared to
previous generations. By moving set_addr into the sun6i_dma_config
structure, we can provide specialized implementations for different hardware.

> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index d92e702320d9f..059455425e19c 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
[ ... ]
> @@ -787,7 +796,7 @@ static struct dma_async_tx_descriptor *sun6i_dma_prep_slave_sg(
>  				sg_dma_len(sg), flags);
>  
>  		} else {
> -			sun6i_dma_set_addr(sdev, v_lli,
> +			sdev->cfg->set_addr(sdev, v_lli,
>  					   sconfig->src_addr,
>  					   sg_dma_address(sg));
>  			v_lli->cfg = lli_cfg;

[Severity: High]
This isn't a bug introduced by this patch, but looking at the error handling
paths for sun6i_dma_prep_slave_sg() and sun6i_dma_prep_dma_cyclic(), is there
a use-after-free when freeing DMA pool items?

In the err_lli_free path, the code iterates over the linked list of items:

err_lli_free:
	for (p_lli = txd->p_lli, v_lli = txd->v_lli; v_lli;
	     p_lli = v_lli->p_lli_next, v_lli = v_lli->v_lli_next)
		dma_pool_free(sdev->pool, v_lli, p_lli);

When dma_pool_free() executes, the memory associated with v_lli is returned
to the pool. The loop's update step then accesses v_lli->p_lli_next and
v_lli->v_lli_next to advance the pointers.

Under memory pressure, another thread might allocate this newly freed item
and overwrite its contents, potentially causing an invalid pointer
dereference. Should this loop use a safe iteration method that reads the
next pointers before calling dma_pool_free()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260619-sun60i-a733-dma-v1-0-da4b649fc72a@gmail.com?part=2

[PATCH 3/5] dmaengine: sun6i-dma: Add num_channels_per_reg for flexible interrupt mapping

[Yuanshen Cao]

The previous implementation of `sun6i-dma` had some implicit assumptions
about the number of channels per interrupt register. Specifically,
functions like `sun6i_kill_tasklet` were hardcoded to only disable
interrupts for IRQ 0 and 1. `DMA_MAX_CHANNELS` is also not in used in
the past, 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. This patch introduces
`num_channels_per_reg` to the `sun6i_dma_config`, similar to BSP, to
make the interrupt handling logic hardware-agnostic. It also sets
`DMA_MAX_CHANNELS` to 16 to align with the new BSP code and ensure loops
over interrupts are correctly bounded.

Changes:
- Change `DMA_MAX_CHANNELS` definition to 16.
- Added `num_channels_per_reg` to `struct sun6i_dma_config`.
- Replaced hardcoded IRQ register calculations with values from
  `sdev->cfg->num_channels_per_reg`.
- Updated `sun6i_kill_tasklet` to loop through all possible interrupt
  registers based on `DMA_MAX_CHANNELS` and the configuration.

Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
---
 drivers/dma/sun6i-dma.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 059455425e19..fb1c1a28744b 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -41,7 +41,7 @@
 #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
@@ -151,6 +151,7 @@ struct sun6i_dma_config {
 	u32 src_addr_widths;
 	u32 dst_addr_widths;
 	bool has_mbus_clk;
+	u32 num_channels_per_reg;
 };
 
 /*
@@ -481,8 +482,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;
 
@@ -574,7 +575,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 = sdev->cfg->read_irq_stat(sdev, i);
 		if (!status)
 			continue;
@@ -584,7 +585,7 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
 
 		sdev->cfg->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)) {
@@ -1108,9 +1109,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 < DMA_MAX_CHANNELS / sdev->cfg->num_channels_per_reg; i++)
+		sdev->cfg->write_irq_en(sdev, i, 0);
 
 	/* Prevent spurious interrupts from scheduling the tasklet */
 	atomic_inc(&sdev->tasklet_shutdown);
@@ -1171,6 +1174,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),
+	.num_channels_per_reg = DMA_IRQ_CHAN_NR,
 };
 
 /*
@@ -1200,6 +1204,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),
+	.num_channels_per_reg = DMA_IRQ_CHAN_NR,
 };
 
 static struct sun6i_dma_config sun8i_a83t_dma_cfg = {
@@ -1224,6 +1229,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),
+	.num_channels_per_reg = DMA_IRQ_CHAN_NR,
 };
 
 /*
@@ -1257,6 +1263,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),
+	.num_channels_per_reg = DMA_IRQ_CHAN_NR,
 };
 
 /*
@@ -1284,6 +1291,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),
+	.num_channels_per_reg = DMA_IRQ_CHAN_NR,
 };
 
 /*
@@ -1311,6 +1319,7 @@ 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),
+	.num_channels_per_reg = DMA_IRQ_CHAN_NR,
 	.has_mbus_clk = true,
 };
 
@@ -1339,6 +1348,7 @@ static struct sun6i_dma_config sun50i_h6_dma_cfg = {
 			     BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
 			     BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) |
 			     BIT(DMA_SLAVE_BUSWIDTH_8_BYTES),
+	.num_channels_per_reg = DMA_IRQ_CHAN_NR,
 	.has_mbus_clk = true,
 };
 
@@ -1369,6 +1379,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),
+	.num_channels_per_reg = DMA_IRQ_CHAN_NR,
 };
 
 static const struct of_device_id sun6i_dma_match[] = {

-- 
2.54.0

Re: [PATCH 3/5] dmaengine: sun6i-dma: Add num_channels_per_reg for flexible interrupt mapping

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Incorrect physical channel indexing in `sun6i_dma_interrupt` ignores the register index, applying status to the wrong physical channel.
- [High] Integer division truncation in the `sun6i_dma_interrupt` loop bound skips polling for trailing channels.
- [High] Time-of-Check to Time-of-Use (TOCTOU) race condition in `sun6i_dma_interrupt` risks a NULL pointer dereference.
--

commit 3bfcb64969bffa1792e80eaa282fc15450f3870b
Author: Yuanshen Cao <alex.caoys@gmail.com>

dmaengine: sun6i-dma: Add num_channels_per_reg for flexible interrupt mapping

This commit introduces num_channels_per_reg to the sun6i_dma_config to
handle SoCs with varying channels per interrupt register. It modifies
the interrupt logic to use this new configuration value and bounds checking.

> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index 059455425e19c..fb1c1a28744bb 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c

[ ... ]

> @@ -574,7 +575,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 is a pre-existing issue, but does this loop condition truncate the
division and miss trailing channels?

If sdev->num_pchans is not perfectly divisible by num_channels_per_reg
(for example, 12 channels with 8 channels per register), the integer
division truncates the remainder. This means the loop will skip the second
register entirely, permanently missing interrupts for the trailing channels.

>  		status = sdev->cfg->read_irq_stat(sdev, i);
>  		if (!status)
>  			continue;
> @@ -584,7 +585,7 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
>  
>  		sdev->cfg->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: Critical]
This is a pre-existing issue, but does this apply the status to the wrong
physical channel?

The array lookup uses only the local offset j, discarding the register
index i. For any register i > 0, it accesses sdev->pchans[j] instead of
sdev->pchans[i * num_channels_per_reg + j].

With the newly added hardware where num_channels_per_reg is 1, j is always
0, which seems like it would cause all channels to incorrectly map to
channel 0.

>  			vchan = pchan->vchan;
>  			if (vchan && (status & vchan->irq_type)) {

[Severity: High]
This is a pre-existing issue, but is there a race condition here that could
lead to a NULL pointer dereference?

The interrupt handler reads vchan = pchan->vchan locklessly, and eventually
blocks to acquire vchan->vc.lock. Concurrently, sun6i_dma_terminate_all()
could acquire the same lock, nullify pchan->desc, and free the descriptors.

When the interrupt handler successfully acquires the lock:

spin_lock(&vchan->vc.lock);
vchan_cookie_complete(&pchan->desc->vd);

it does not re-verify pchan->desc before passing &pchan->desc->vd to
vchan_cookie_complete(), which would crash if pchan->desc was concurrently
set to NULL.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260619-sun60i-a733-dma-v1-0-da4b649fc72a@gmail.com?part=3

[PATCH 4/5] dmaengine: sun6i-dma: Implement support for Allwinner A733 DMA controller

[Yuanshen Cao]

This patch implements the actual support for the Allwinner A733 DMA
controller. It defines the new register offsets and bitfield mappings
required for the A733, which slightly differs from the older `sun6i`
series.

Changes:
- New register macros for A733 interrupt enable `DMA_IRQ_EN_A733` and
  status `DMA_IRQ_STAT_A733`.
- New `SRC_HIGH_ADDR_32G` and `DST_HIGH_ADDR_32G` macro to handle the
  32G high-address field in the LLI.
- Implemented `sun6i_dma_set_addr_a733` and A733-specific interrupt
  register accessors.
- Added `sun60i_a733_dma_config`, which ties all the refactored
  functionality together for this specific hardware.

Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
---
 drivers/dma/sun6i-dma.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index fb1c1a28744b..9585b4a9e00d 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -52,6 +52,15 @@
 #define SUNXI_H3_SECURE_REG		0x20
 #define SUNXI_H3_DMA_GATE		0x28
 #define SUNXI_H3_DMA_GATE_ENABLE	0x4
+
+/*
+ * sun60i specific registers
+ */
+#define DMA_IRQ_EN_A733(x)		((x) * 0x40 + 0x134)
+#define DMA_IRQ_STAT_A733(x)		((x) * 0x40 + 0x138)
+
+#define DMA_IRQ_CHAN_NR_A733		1
+
 /*
  * Channels specific registers
  */
@@ -100,6 +109,8 @@
  */
 #define SRC_HIGH_ADDR(x)		(((x) & 0x3U) << 16)
 #define DST_HIGH_ADDR(x)		(((x) & 0x3U) << 18)
+#define SRC_HIGH_ADDR_32G(x)	(((x) & 0x7U) << 11)
+#define DST_HIGH_ADDR_32G(x)	(((x) & 0x7U) << 15)
 
 /*
  * Various hardware related defines
@@ -257,6 +268,23 @@ static inline void sun6i_dma_dump_com_regs(struct sun6i_dma_dev *sdev)
 		DMA_STAT, readl(sdev->base + DMA_STAT));
 }
 
+static inline void sun6i_dma_dump_com_regs_a733(struct sun6i_dma_dev *sdev)
+{
+	int i;
+
+	for (i = 0; i < 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"
+			"\tpend(%04x): 0x%08x\n"
+			"\tstats(%04x): 0x%08x\n",
+			i,
+			DMA_IRQ_EN_A733(i), readl(sdev->base + DMA_IRQ_EN_A733(i)),
+			DMA_IRQ_STAT_A733(i), readl(sdev->base + DMA_IRQ_STAT_A733(i)),
+			DMA_STAT, readl(sdev->base + DMA_STAT));
+	}
+}
+
 static inline void sun6i_dma_dump_chan_regs(struct sun6i_dma_dev *sdev,
 					    struct sun6i_pchan *pchan)
 {
@@ -360,20 +388,41 @@ static u32 sun6i_read_irq_en(struct sun6i_dma_dev *sdev, u32 chan_num)
 	return readl(sdev->base + DMA_IRQ_EN(chan_num));
 }
 
+static u32 sun6i_read_irq_en_a733(struct sun6i_dma_dev *sdev, u32 chan_num)
+{
+	return readl(sdev->base + DMA_IRQ_EN_A733(chan_num));
+}
+
 static void sun6i_write_irq_en(struct sun6i_dma_dev *sdev, u32 chan_num, u32 irq_val)
 {
 	writel(irq_val, sdev->base + DMA_IRQ_EN(chan_num));
 }
+
+static void sun6i_write_irq_en_a733(struct sun6i_dma_dev *sdev, u32 chan_num, u32 irq_val)
+{
+	writel(irq_val, sdev->base + DMA_IRQ_EN_A733(chan_num));
+}
+
 static u32 sun6i_read_irq_stat(struct sun6i_dma_dev *sdev, u32 chan_num)
 {
 	return readl(sdev->base + DMA_IRQ_STAT(chan_num));
 }
 
+static u32 sun6i_read_irq_stat_a733(struct sun6i_dma_dev *sdev, u32 chan_num)
+{
+	return readl(sdev->base + DMA_IRQ_STAT_A733(chan_num));
+}
+
 static void sun6i_write_irq_stat(struct sun6i_dma_dev *sdev, u32 chan_num, u32 status)
 {
 	writel(status, sdev->base + DMA_IRQ_STAT(chan_num));
 }
 
+static void sun6i_write_irq_stat_a733(struct sun6i_dma_dev *sdev, u32 chan_num, u32 status)
+{
+	writel(status, sdev->base + DMA_IRQ_STAT_A733(chan_num));
+}
+
 static size_t sun6i_get_chan_size(struct sun6i_pchan *pchan)
 {
 	struct sun6i_desc *txd = pchan->desc;
@@ -694,6 +743,17 @@ static inline void sun6i_dma_set_addr_a100(struct sun6i_dma_dev *sdev,
 				DST_HIGH_ADDR(upper_32_bits(dst));
 }
 
+static inline void sun6i_dma_set_addr_a733(struct sun6i_dma_dev *sdev,
+				      struct sun6i_dma_lli *v_lli,
+				      dma_addr_t src, dma_addr_t dst)
+{
+	v_lli->src = lower_32_bits(src);
+	v_lli->dst = lower_32_bits(dst);
+
+	v_lli->para |= SRC_HIGH_ADDR_32G(upper_32_bits(src)) |
+				DST_HIGH_ADDR_32G(upper_32_bits(dst));
+}
+
 static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_memcpy(
 		struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
 		size_t len, unsigned long flags)
@@ -1352,6 +1412,33 @@ static struct sun6i_dma_config sun50i_h6_dma_cfg = {
 	.has_mbus_clk = true,
 };
 
+/*
+ * 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,
+	.set_burst_length = sun6i_set_burst_length_h3,
+	.set_drq          = sun6i_set_drq_h6,
+	.set_mode         = sun6i_set_mode_h6,
+	.set_addr         = sun6i_dma_set_addr_a733,
+	.dump_com_regs    = sun6i_dma_dump_com_regs_a733,
+	.read_irq_en      = sun6i_read_irq_en_a733,
+	.write_irq_en     = sun6i_write_irq_en_a733,
+	.read_irq_stat    = sun6i_read_irq_stat_a733,
+	.write_irq_stat   = sun6i_write_irq_stat_a733,
+	.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),
+	.num_channels_per_reg = DMA_IRQ_CHAN_NR_A733,
+	.has_mbus_clk = true,
+};
+
 /*
  * The V3s have only 8 physical channels, a maximum DRQ port id of 23,
  * and a total of 24 usable source and destination endpoints.
@@ -1392,6 +1479,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.54.0

Re: [PATCH 4/5] dmaengine: sun6i-dma: Implement support for Allwinner A733 DMA controller

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The DMA interrupt handler incorrectly resolves the physical channel (`pchan`) from the interrupt status bit by ignoring the outer loop register index `i`.
- [High] Integer division truncation in loop bounds causes upper hardware channels to be completely ignored, and introduces out-of-bounds MMIO accesses.
--

commit e328a2b422946f5a9b0337a1058273c7077c1ac4
Author: Yuanshen Cao <alex.caoys@gmail.com>

dmaengine: sun6i-dma: Implement support for Allwinner A733 DMA controller

This patch implements the actual support for the Allwinner A733 DMA
controller. It defines the new register offsets and bitfield mappings
required for the A733, which slightly differs from the older sun6i
series.

> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index fb1c1a28744bb..9585b4a9e00de 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c

[ ... ]

> @@ -1352,6 +1412,33 @@ static struct sun6i_dma_config sun50i_h6_dma_cfg = {
>  	.has_mbus_clk = true,
>  };
>  
> +/*
> + * 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,
> +	.set_burst_length = sun6i_set_burst_length_h3,
> +	.set_drq          = sun6i_set_drq_h6,
> +	.set_mode         = sun6i_set_mode_h6,
> +	.set_addr         = sun6i_dma_set_addr_a733,
> +	.dump_com_regs    = sun6i_dma_dump_com_regs_a733,
> +	.read_irq_en      = sun6i_read_irq_en_a733,
> +	.write_irq_en     = sun6i_write_irq_en_a733,
> +	.read_irq_stat    = sun6i_read_irq_stat_a733,
> +	.write_irq_stat   = sun6i_write_irq_stat_a733,
> +	.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),
> +	.num_channels_per_reg = DMA_IRQ_CHAN_NR_A733,

[Severity: Critical]
This is a pre-existing issue, but does setting num_channels_per_reg to 1
(DMA_IRQ_CHAN_NR_A733) break interrupt handling for all A733 channels > 0?

In sun6i_dma_interrupt(), the physical channel is calculated without the outer
loop index i:

    for (i = 0; i < sdev->num_pchans / sdev->cfg->num_channels_per_reg; i++) {
        ...
        for (j = 0; (j < sdev->cfg->num_channels_per_reg) && status; j++) {
            pchan = sdev->pchans + j;

Since num_channels_per_reg is 1, j is always 0. As a result, every interrupt
on every channel will incorrectly resolve to sdev->pchans[0]. Doesn't this
completely break DMA functionality for all A733 channels except channel 0 by
causing spurious completions on channel 0 and hanging all others?

[Severity: High]
This is a pre-existing issue, but does using integer division truncation for
loop bounds ignore remainder channels and cause out-of-bounds MMIO accesses?

In sun6i_dma_interrupt(), the loop bound is:

    i < sdev->num_pchans / sdev->cfg->num_channels_per_reg

For SoCs where the number of channels is not a perfect multiple of
num_channels_per_reg (like H3 with 12 channels and 8 channels per register),
this division truncates (12 / 8 = 1), causing the remainder channels to be
entirely ignored so their interrupts are never serviced.

Additionally, this patch series introduces a similar loop in
sun6i_kill_tasklet():

    for (i = 0; i < DMA_MAX_CHANNELS / sdev->cfg->num_channels_per_reg; i++)

Since DMA_MAX_CHANNELS is 16 and num_channels_per_reg is 1 for A733, the loop
evaluates to 16. If the A733 hardware possesses fewer than 16 channels, won't
this cause out-of-bounds MMIO writes which can trigger bus aborts?

> +	.has_mbus_clk = true,
> +};
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260619-sun60i-a733-dma-v1-0-da4b649fc72a@gmail.com?part=4

[PATCH 5/5] dt-bindings: dma: sun50i-a64-dma: Update device tree bindings documentation for A733

[Yuanshen Cao]

To complete the support for the A733 DMA controller, added
`allwinner,sun60i-a733-dma` to the list of compatible strings for
`allwinner,sun50i-a64-dma` dt-binding documentations..

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.54.0