diff mbox series

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

Message ID 20260826-sun60i-a733-dma-v5-4-abc5229b441e@gmail.com (mailing list archive)
State New
Headers
Series dmaengine: sun6i-dma: Add support for Allwinner A733 DMA controller |

Commit Message

Yuanshen Cao Aug. 26, 2026, 2:40 a.m. UTC 
Add `allwinner,sun60i-a733-dma` to the list of compatible strings for the
`sun50i-a64-dma` dtbinding documentation.

While the A733 DMA controller shares many similarities with the sun50i-a64
DMA controller, it requires a specific configuration due to differences in:
- Interrupt register layout and mapping.
- Number of channels per interrupt register.
- Support for higher (32G) address widths in LLI parameters.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
---
 Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml | 2 ++
 1 file changed, 2 insertions(+)

Comments

sashiko-bot@kernel.org Aug. 26, 2026, 2:57 a.m. UTC | #1 
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?

[ ... ]
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml b/Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml
index c3e14eb6cfff..1cc3304b7414 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
       - items:
           - const: allwinner,sun8i-r40-dma
           - const: allwinner,sun50i-a64-dma
@@ -70,6 +71,7 @@  if:
           - allwinner,sun20i-d1-dma
           - allwinner,sun50i-a100-dma
           - allwinner,sun50i-h6-dma
+          - allwinner,sun60i-a733-dma
 
 then:
   properties: