Re: [PATCH v3 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:
- [Critical] The interrupt handler reads the status for higher physical channels but incorrectly applies completions to channels 0-7, causing cross-channel corruption and dropping interrupts.
- [High] The interrupt handler loop bound truncates remainders, skipping the last interrupt register for SoCs whose channel count is not a multiple of `num_channels_per_reg`.
--

commit 949f13d1f9dc8ae54064e512fd45f644694422c7
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: Critical]
This is a pre-existing issue, but does the interrupt handler in
sun6i_dma_interrupt() correctly handle the physical channel calculation?

Looking at drivers/dma/sun6i-dma.c:sun6i_dma_interrupt():

    pchan = sdev->pchans + j;

The physical channel is derived from the inner loop index j alone, completely
ignoring the outer loop register index i. For the newly added sun60i-a733
(where num_channels_per_reg is 1) or SoCs with more than 8 channels, an
interrupt on a higher channel will incorrectly apply the completion to channel
0.

Can this corrupt the state by signaling spurious completions for active
transfers on lower channels while permanently hanging the actual completing
channels?

[Severity: High]
This is also a pre-existing issue. Is the interrupt handler skipping registers
for SoCs whose channel count is not a multiple of num_channels_per_reg?

Looking at the loop condition in sun6i_dma_interrupt():

    for (i = 0; i < sdev->num_pchans / sdev->cfg->num_channels_per_reg; i++) {

The loop bound uses integer division. For SoCs like the H3 (which has 12
channels and 8 channels per register), 12 / 8 evaluates to 1.

Will this cause the loop to only check the first register (channels 0-7) and
completely skip the second register (channels 8-11), permanently dropping
interrupts for channels 8 and above? Would using DIV_ROUND_UP() be more
appropriate here?

[ ... ]

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