diff mbox series

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

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

Commit Message

Yuanshen Cao June 22, 2026, 1:36 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>
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 June 22, 2026, 1:49 a.m. UTC | #1 
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?

[ ... ]
Krzysztof Kozlowski June 22, 2026, 10:41 a.m. UTC | #2 
On Mon, Jun 22, 2026 at 01:36:26AM +0000, Yuanshen Cao wrote:
> 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>
> Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
> ---
>  Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof
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: