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

Support Allwinner A733 DMA controller. Define new register offsets,
bitfield mappings and dma_config required for the A733, which slightly
differs from the older `sun6i` DMA controllers.

Changes:
- New register macros for A733 interrupt enable `DMA_IRQ_EN_A733`,
  status `DMA_IRQ_STAT_A733`, and channel count `DMA_IRQ_CHAN_NR_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_cfg`, 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 | 87 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 87 insertions(+)
  

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

New issues:
- [High] Hardcoded loop bound in `sun6i_kill_tasklet` leads to out-of-bounds MMIO register writes.

Pre-existing issues:
- [Critical] Incorrect physical channel indexing in `sun6i_dma_interrupt` misroutes hardware interrupts to the wrong software channels.
- [High] Integer division truncation in IRQ register loops silently drops interrupts for leftover channels.
- [High] A TOCTOU race exists between `sun6i_dma_interrupt` and `sun6i_dma_terminate_all`, leading to a NULL pointer dereference.
--

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

dmaengine: sun6i-dma: Add support for Allwinner A733 DMA controller
    
Support Allwinner A733 DMA controller. Define new register offsets,
bitfield mappings and dma_config required for the A733, which slightly
differs from the older sun6i DMA controllers.

> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index 196a0d73b2212..4808015934ccb 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -257,6 +268,23 @@ static inline void sun6i_dma_dump_com_regs(struct sun6i_dma_dev *sdev)
[ ... ]
> +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++) {

[Severity: High]
This is a pre-existing issue, but I noticed this loop limit might cause integer
division truncation.

If the total number of channels is not an exact multiple of the channels
per register, the fractional remainder is truncated. For example, if a
platform has 12 channels and 8 per register, the loop only iterates once,
completely ignoring the second register. 

Does this mean interrupts for the leftover channels are never read or
cleared, leading to an unhandled interrupt storm? Should this use 
DIV_ROUND_UP instead? This buggy logic also seems to be present in
sun6i_dma_interrupt().

> +		dev_dbg(sdev->slave.dev, "Common register:\n"
[ ... ]
> @@ -1339,6 +1398,33 @@ static struct sun6i_dma_config sun50i_h6_dma_cfg = {
[ ... ]
> +static struct sun6i_dma_config sun60i_a733_dma_cfg = {
> +	.clock_autogate_enable = sun6i_enable_clock_autogate_h3,
[ ... ]
> +	.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 setting num_channels_per_reg to 1 for
the A733 controller appears to completely break the DMA controller due
to an issue in sun6i_dma_interrupt().

In sun6i_dma_interrupt(), the physical channel is resolved as:

    pchan = sdev->pchans + j;

Since num_channels_per_reg is 1, j is always 0. Does this route every
hardware interrupt to software channel 0, regardless of the actual
triggering channel? Should the calculation instead be updated to properly
account for the register index:

    pchan = sdev->pchans + i * sdev->cfg->num_channels_per_reg + j;

[Severity: High]
I also noticed an issue in sun6i_kill_tasklet() that might be triggered
by this configuration.

sun6i_kill_tasklet() calculates its iteration bounds using:

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

With DMA_MAX_CHANNELS set to 16 and num_channels_per_reg set to 1, this
loop will unconditionally execute 16 times. If the dynamically initialized
channel count for the SoC is fewer than 16, will these writes target
unmapped peripheral memory space and potentially trigger a synchronous
external abort? Should the loop bound depend on the actual initialized
channel count (sdev->num_pchans) instead?

> +	.has_mbus_clk = true,
> +};

[Severity: High]
This is a pre-existing issue, but could there be a TOCTOU race condition
between sun6i_dma_interrupt() and sun6i_dma_terminate_all() that leads to
a NULL pointer dereference?

In sun6i_dma_terminate_all(), the channel is stopped and pointers are cleared:

    vchan->phy = NULL;
    pchan->vchan = NULL;
    pchan->desc = NULL;
    pchan->done = NULL;

However, in sun6i_dma_interrupt(), the pointers are accessed like this:

    pchan = sdev->pchans + j;
    vchan = pchan->vchan;
    if (vchan && (status & vchan->irq_type)) {
        if (vchan->cyclic) {
            vchan_cyclic_callback(&pchan->desc->vd);
        } else {
            spin_lock(&vchan->vc.lock);
            vchan_cookie_complete(&pchan->desc->vd);

Since the interrupt handler checks vchan = pchan->vchan outside the lock,
could a concurrent call to sun6i_dma_terminate_all() clear pchan->desc
before it is dereferenced inside the interrupt handler, resulting in a
near-NULL pointer dereference?
  

On Mon, Jun 22, 2026 at 01:36:27AM +0000, Yuanshen Cao wrote:
> Support Allwinner A733 DMA controller. Define new register offsets,
> bitfield mappings and dma_config required for the A733, which slightly
> differs from the older `sun6i` DMA controllers.
>
> Changes:
> - New register macros for A733 interrupt enable `DMA_IRQ_EN_A733`,
>   status `DMA_IRQ_STAT_A733`, and channel count `DMA_IRQ_CHAN_NR_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_cfg`, 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 | 87 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 87 insertions(+)
>
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index 196a0d73b221..4808015934cc 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)

Because the previous code use this pattern, I provide my reviewed-by tags.
I suggest change to use GEN_MASK and FIELD_PREP macro in future.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
>
>  /*
>   * 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,21 +388,41 @@ static u32 sun6i_read_irq_en(struct sun6i_dma_dev *sdev, u32 irq_reg)
>  	return readl(sdev->base + DMA_IRQ_EN(irq_reg));
>  }
>
> +static u32 sun6i_read_irq_en_a733(struct sun6i_dma_dev *sdev, u32 irq_reg)
> +{
> +	return readl(sdev->base + DMA_IRQ_EN_A733(irq_reg));
> +}
> +
>  static void sun6i_write_irq_en(struct sun6i_dma_dev *sdev, u32 irq_reg, u32 irq_val)
>  {
>  	writel(irq_val, sdev->base + DMA_IRQ_EN(irq_reg));
>  }
>
> +static void sun6i_write_irq_en_a733(struct sun6i_dma_dev *sdev, u32 irq_reg, u32 irq_val)
> +{
> +	writel(irq_val, sdev->base + DMA_IRQ_EN_A733(irq_reg));
> +}
> +
>  static u32 sun6i_read_irq_stat(struct sun6i_dma_dev *sdev, u32 irq_reg)
>  {
>  	return readl(sdev->base + DMA_IRQ_STAT(irq_reg));
>  }
>
> +static u32 sun6i_read_irq_stat_a733(struct sun6i_dma_dev *sdev, u32 irq_reg)
> +{
> +	return readl(sdev->base + DMA_IRQ_STAT_A733(irq_reg));
> +}
> +
>  static void sun6i_write_irq_stat(struct sun6i_dma_dev *sdev, u32 irq_reg, u32 status)
>  {
>  	writel(status, sdev->base + DMA_IRQ_STAT(irq_reg));
>  }
>
> +static void sun6i_write_irq_stat_a733(struct sun6i_dma_dev *sdev, u32 irq_reg, u32 status)
> +{
> +	writel(status, sdev->base + DMA_IRQ_STAT_A733(irq_reg));
> +}
> +
>  static size_t sun6i_get_chan_size(struct sun6i_pchan *pchan)
>  {
>  	struct sun6i_desc *txd = pchan->desc;
> @@ -695,6 +743,17 @@ static void sun6i_dma_set_addr_a100(struct sun6i_dma_dev *sdev,
>  				DST_HIGH_ADDR(upper_32_bits(dst));
>  }
>
> +static 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 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)
> @@ -1339,6 +1398,33 @@ static struct sun6i_dma_config sun50i_h6_dma_cfg = {
>  	SUN6I_DMA_IRQ_A31_COMMON_OPS
>  };
>
> +/*
> + * 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.
> @@ -1375,6 +1461,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
>