Re: [PATCH v5 4/5] dt-bindings: dmaengine: sun50i-a64-dma: Add allwinner,sun60i-a733-dma compatible string

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Loop bounds calculation in `sun6i_dma_interrupt` truncates unaligned channel counts, causing interrupts for higher channels to be permanently missed.
- [High] Incorrect indexing of the physical channel array in `sun6i_dma_interrupt` maps status bits from subsequent interrupt registers to the wrong physical channels.
--

commit 42af4e68ee14f82cbb1311fbd3ef28760a3616bc
Author: Yuanshen Cao <alex.caoys@gmail.com>

dt-bindings: dmaengine: sun50i-a64-dma: Add allwinner,sun60i-a733-dma compatible string
    
Add allwinner,sun60i-a733-dma to the list of compatible strings for the
sun50i-a64-dma dtbinding documentation.

> diff --git a/Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml b/Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml
> index c3e14eb6cfff9..1cc3304b74149 100644
> --- a/Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml
> +++ b/Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml
> @@ -25,6 +25,7 @@ properties:
>            - allwinner,sun50i-a64-dma
>            - allwinner,sun50i-a100-dma
>            - allwinner,sun50i-h6-dma
> +          - allwinner,sun60i-a733-dma

[Severity: High]
This isn't a bug introduced by this patch, but adding a new compatible
string here might expose existing issues in the driver if the A733 DMA
controller uses unaligned channel counts or more than 8 channels.

Looking at sun6i_dma_interrupt() in drivers/dma/sun6i-dma.c, does this loop
condition fail to round up unaligned channel counts?

drivers/dma/sun6i-dma.c:sun6i_dma_interrupt() {
...
	for (i = 0; i < sdev->num_pchans / sdev->cfg->num_channels_per_reg; i++) {
		status = sun6i_read_irq_stat(sdev, i);
...
}

If a device tree specifies a dma-channels value that is not a multiple of
num_channels_per_reg (8), this loop bound evaluates incorrectly. For example,
with 12 channels, it evaluates to 1 instead of 2.

Will this cause interrupts for the trailing physical channels (e.g., 8-11)
to be permanently ignored, resulting in DMA transfers on those channels
hanging? This pattern was fixed with DIV_ROUND_UP in
sun6i_dma_dump_com_regs(), but appears to be missed here.

[Severity: High]
This is a pre-existing issue, but I also noticed a potential problem with how
the physical channel array is indexed in the inner loop of the same function:

drivers/dma/sun6i-dma.c:sun6i_dma_interrupt() {
...
		for (j = 0; (j < sdev->cfg->num_channels_per_reg) && status; j++) {
			pchan = sdev->pchans + j;
...
}

Since this inner loop index 'j' is added directly to 'sdev->pchans' without
the '(i * sdev->cfg->num_channels_per_reg)' offset, will status bits from
subsequent interrupt registers (where i > 0) be mapped to the wrong physical
channels (0-7)?

Could this cause premature completion of DMA requests on low-numbered channels
while the actual high-numbered channels hang forever?

[ ... ]

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