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

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(-)
  

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?
  

On Fri, Jun 19, 2026 at 04:53:30AM +0000, Yuanshen Cao wrote:
>
> This patch is the first step in a refactoring effort to support the

avoid use "this patch/commit", Just

Refactor ... to support Allwinner A733 DMA controller.

> 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,

Can you define macro like to avoid duplicate these init code. Or set it at
probe if it is NULL.

Frank

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