[v2,3/5] dmaengine: sun6i-dma: Add num_channels_per_reg for flexible interrupt mapping
Commit Message
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(-)
Comments
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [Critical] The interrupt handler calculates the physical channel pointer without accounting for the register index `i`, erroneously aliasing all interrupts from secondary registers to the first group of channels.
- [High] The interrupt handler uses integer division to determine the number of IRQ registers to poll, entirely missing trailing channels if `sdev->num_pchans` is not a multiple of `num_channels_per_reg`.
--
commit 8ce409f21992b8890893dbba369b1293cac22679
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 and
updates DMA interrupt handling to avoid hardcoded register limits. It also
changes DMA_MAX_CHANNELS to 16 to align with the new BSP code.
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index 9984b9033cbb9..196a0d73b2212 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
[ ... ]
> @@ -575,7 +576,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 boundary condition truncate
the number of required registers to check?
In drivers/dma/sun6i-dma.c:sun6i_dma_interrupt(), the loop boundary condition
sdev->num_pchans / sdev->cfg->num_channels_per_reg uses integer division.
For configurations like sun8i_h3_dma_cfg, num_pchans is 12 and
num_channels_per_reg is 8. The division evaluates to 1, meaning the handler
will only check the first register (channels 0-7) and skip checking the
secondary register (channels 8-11).
If an interrupt triggers on channels 8-11, could the level-sensitive IRQ
remain asserted without being cleared, causing a continuous interrupt storm
that freezes the CPU core? Would using DIV_ROUND_UP prevent this truncation?
> status = sdev->cfg->read_irq_stat(sdev, i);
> if (!status)
> continue;
> @@ -585,7 +586,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 inner loop calculate the physical
channel pointer correctly for secondary registers?
In sun6i_dma_interrupt(), the pchan pointer is resolved using only the
inner loop index j. This ignores the outer loop's register index i. If an
interrupt fires for a channel in a secondary register (for example, channel
8, where i = 1 and j = 0), the handler appears to erroneously fetch
sdev->pchans[0] instead of sdev->pchans[8].
Could this premature completion of an active transfer on channel 0 lead to
a use-after-free if the client frees the buffer while hardware DMA is still
running, while the actual requesting channel hangs indefinitely? Should the
index calculation include the register offset?
On Sun, Jun 21, 2026 at 09:40:56PM +0000, Yuanshen Cao wrote:
> 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>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.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 9984b9033cbb..196a0d73b221 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;
> };
>
> /*
> @@ -482,8 +483,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;
>
> @@ -575,7 +576,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;
> @@ -585,7 +586,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)) {
> @@ -1116,9 +1117,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);
> @@ -1181,6 +1184,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,
> SUN6I_DMA_IRQ_A31_COMMON_OPS
> };
>
> @@ -1206,6 +1210,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,
> SUN6I_DMA_IRQ_A31_COMMON_OPS
> };
>
> @@ -1226,6 +1231,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,
> SUN6I_DMA_IRQ_A31_COMMON_OPS
> };
>
> @@ -1255,6 +1261,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,
> SUN6I_DMA_IRQ_A31_COMMON_OPS
> };
>
> @@ -1278,6 +1285,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,
> SUN6I_DMA_IRQ_A31_COMMON_OPS
> };
>
> @@ -1301,6 +1309,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,
> SUN6I_DMA_IRQ_A31_COMMON_OPS
> };
> @@ -1325,6 +1334,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,
> SUN6I_DMA_IRQ_A31_COMMON_OPS
> };
> @@ -1351,6 +1361,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,
> SUN6I_DMA_IRQ_A31_COMMON_OPS
> };
>
>
> --
> 2.54.0
>
@@ -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;
};
/*
@@ -482,8 +483,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;
@@ -575,7 +576,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;
@@ -585,7 +586,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)) {
@@ -1116,9 +1117,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);
@@ -1181,6 +1184,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,
SUN6I_DMA_IRQ_A31_COMMON_OPS
};
@@ -1206,6 +1210,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,
SUN6I_DMA_IRQ_A31_COMMON_OPS
};
@@ -1226,6 +1231,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,
SUN6I_DMA_IRQ_A31_COMMON_OPS
};
@@ -1255,6 +1261,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,
SUN6I_DMA_IRQ_A31_COMMON_OPS
};
@@ -1278,6 +1285,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,
SUN6I_DMA_IRQ_A31_COMMON_OPS
};
@@ -1301,6 +1309,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,
SUN6I_DMA_IRQ_A31_COMMON_OPS
};
@@ -1325,6 +1334,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,
SUN6I_DMA_IRQ_A31_COMMON_OPS
};
@@ -1351,6 +1361,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,
SUN6I_DMA_IRQ_A31_COMMON_OPS
};