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

[sashiko-bot@kernel.org]

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