Re: [PATCH v2 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:
- [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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260621-sun60i-a733-dma-v2-0-340f205891cc@gmail.com?part=3