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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Pre-existing logic errors in `sun6i_dma_interrupt` fundamentally break interrupt handling for physical channels >= 8 via integer division truncation and misdirected channel array indexing. This actively breaks the newly added A733 support as well as existing SoCs.
--

commit e5054eef1eb5c40444453238af77d7ab1cfceae5
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 flexible register layouts
and interrupt handling. It introduces irq_stride, irq_en_offset, and
irq_stat_offset to struct sun6i_dma_config to accommodate hardware variations.
These changes specifically enable support for the Allwinner A733 SoC.

> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index f47a326dd7ffa..f305fbfb35453 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
[ ... ]
> @@ -549,14 +581,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 integer division truncate partial
register blocks?

For example, on the 12-channel H3 SoC, 12 / 8 = 1, causing the loop to drop
i=1 and permanently abandon interrupts for channels 8-11. Interrupts for these
upper physical channels are completely ignored, which can lead to unrecoverable
hardware interrupt storms and system hangs when a device allocates them.

> -		status = readl(sdev->base + DMA_IRQ_STAT(i));
> +		status = sun6i_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));
> +		sun6i_write_irq_stat(sdev, i, status);
>  
>  		for (j = 0; (j < DMA_IRQ_CHAN_NR) && status; j++) {
>  			pchan = sdev->pchans + j;

[Severity: High]
This is also a pre-existing issue, but does this indexing fail to include the
outer loop's base channel offset (e.g., i * DMA_IRQ_CHAN_NR)?

This blindly routes interrupts from upper registers (e.g., channels 8-15) to
physical channels 0-7, causing premature transfer completions and cross-channel
data corruption.

Furthermore, for the newly introduced A733, where num_channels_per_reg is 1,
the inner loop always evaluates j as 0. This means every interrupt from any
channel maps to pchans[0], rendering the entire DMA controller non-functional.

I have verified that this is not fixed in any subsequent patch in this series.

>  			vchan = pchan->vchan;

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