Re: [PATCH v5 3/5] dmaengine: sun6i-dma: Add num_channels_per_reg for flexible interrupt mapping

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Integer division truncates the interrupt register loop bound, permanently ignoring interrupts for channels that do not divide evenly into num_channels_per_reg.
- [High] Incorrect array indexing for physical channels misroutes interrupts to the wrong DMA channels, breaking functionality and A733 support.
--

commit 4af47032928a84468d9e66febfa1d354c72ba8f6
Author: Yuanshen Cao <alex.caoys@gmail.com>

dmaengine: sun6i-dma: Add num_channels_per_reg for flexible interrupt mapping

This commit updates the sun6i-dma driver to use a flexible 
num_channels_per_reg configuration rather than a hardcoded value. This makes 
the interrupt handling hardware-agnostic, enabling support for the A733 SoC 
which has a different interrupt structure.

> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index 1ffd870d95942..ffb63212bea71 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c

[ ... ]

> @@ -582,7 +584,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 isn't a bug introduced by this patch, but it appears to be a pre-existing
issue where integer division truncates the interrupt register loop bound. On
hardware where the number of physical channels is not an exact multiple of
num_channels_per_reg, will this loop prematurely exit and abandon unaligned
physical channels?

For example, if a SoC has 12 physical channels and num_channels_per_reg is 8,
the bound evaluates to 12 / 8 = 1, meaning the second interrupt register is
never checked. Should this be using DIV_ROUND_UP(sdev->num_pchans,
sdev->cfg->num_channels_per_reg) to match how it is handled in
sun6i_dma_dump_com_regs()?

>  		status = sun6i_read_irq_stat(sdev, i);
>  		if (!status)
>  			continue;
> @@ -592,7 +594,7 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
>  
>  		sun6i_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: High]
This is also a pre-existing issue, but the incorrect array indexing here
becomes particularly critical with the newly introduced A733 support. Does
this array access ignore the outer register index i, aliasing all
higher-channel interrupts to the first register's channels?

Since the newly supported A733 SoC has num_channels_per_reg = 1, j will
always evaluate to 0. Will this catastrophically misroute all interrupts
across all channels to channel 0?

It looks like the correct calculation needs to include the register offset,
such as: pchan = sdev->pchans + (i * sdev->cfg->num_channels_per_reg) + j;

Note that this might also require an explicit array bounds check against
sdev->num_pchans to prevent out-of-bounds access for trailing unaligned
channels.

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