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

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

Commit Message

Yuanshen Cao June 21, 2026, 9:40 p.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.

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 21, 2026, 9:52 p.m. UTC | #1
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The IRQ handler loop skips trailing DMA channels if the total number of physical channels is not an exact multiple of the channels per register.
--

commit 1a8550cc9da2ef939f555f682f0d5ea9606e1907
Author: Yuanshen Cao <alex.caoys@gmail.com>

dt-bindings: dma: 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.

>     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.

[Severity: High]
This isn't a bug introduced by this patch, but could this truncating
integer division skip trailing DMA channels if the total number of physical
channels is not an exact multiple of the channels per register?

In drivers/dma/sun6i-dma.c:sun6i_dma_interrupt(), the loop condition
determines which interrupt registers to poll:

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

For a controller with 12 physical channels (e.g., sun8i_h3_dma_cfg sets
nr_max_channels = 12) and 8 channels per register, 12 / 8 evaluates to 1.

Consequently, the loop only iterates once (for i=0), checking interrupts
for the first 8 channels and completely ignoring the remaining 4 channels.
Any DMA transfers on channels 8-11 will never have their completion
interrupts handled, leading to system hangs or driver timeouts.

Could this be updated to use DIV_ROUND_UP() to ensure all applicable
interrupt registers are processed?

>     - Support for higher (32G) address widths in LLI parameters.
>     
>     Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
  
Frank Li June 21, 2026, 10:19 p.m. UTC | #2
On Sun, Jun 21, 2026 at 09:40:57PM +0000, Yuanshen Cao wrote:

subject dt-bindings: dmaengine: ....

> 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.
>
> Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
> ---

After fix subject tags,

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml | 2 ++
>  1 file changed, 2 insertions(+)
>
> 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:
>
> --
> 2.54.0
>
  
Conor Dooley June 22, 2026, 4:40 p.m. UTC | #3
On Sun, Jun 21, 2026 at 09:52:34PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The IRQ handler loop skips trailing DMA channels if the total number of physical channels is not an exact multiple of the channels per register.
> --
> 
> commit 1a8550cc9da2ef939f555f682f0d5ea9606e1907
> Author: Yuanshen Cao <alex.caoys@gmail.com>
> 
> dt-bindings: dma: 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.
> 
> >     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.
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but could this truncating
> integer division skip trailing DMA channels if the total number of physical
> channels is not an exact multiple of the channels per register?

What has this got to do with the binding?

> 
> In drivers/dma/sun6i-dma.c:sun6i_dma_interrupt(), the loop condition
> determines which interrupt registers to poll:
> 
>     for (i = 0; i < sdev->num_pchans / sdev->cfg->num_channels_per_reg; i++) {
> 
> For a controller with 12 physical channels (e.g., sun8i_h3_dma_cfg sets
> nr_max_channels = 12) and 8 channels per register, 12 / 8 evaluates to 1.
> 
> Consequently, the loop only iterates once (for i=0), checking interrupts
> for the first 8 channels and completely ignoring the remaining 4 channels.
> Any DMA transfers on channels 8-11 will never have their completion
> interrupts handled, leading to system hangs or driver timeouts.
> 
> Could this be updated to use DIV_ROUND_UP() to ensure all applicable
> interrupt registers are processed?
> 
> >     - Support for higher (32G) address widths in LLI parameters.
> >     
> >     Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
> 
> -- 
> Sashiko AI review ยท https://sashiko.dev/#/patchset/20260621-sun60i-a733-dma-v2-0-340f205891cc@gmail.com?part=4
  
Conor Dooley June 22, 2026, 4:42 p.m. UTC | #4
On Sun, Jun 21, 2026 at 05:19:59PM -0500, Frank Li wrote:
> On Sun, Jun 21, 2026 at 09:40:57PM +0000, Yuanshen Cao wrote:
> 
> subject dt-bindings: dmaengine: ....
> 
> > 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.
> >
> > Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
> > ---
> 
> After fix subject tags,

Do not change this unless you're respinning for another reason. dma v
dmaengine is not worth resubmission, especially since dma is far more
commonly used and is the directory name.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

> 
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> 
> >  Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > 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:
> >
> > --
> > 2.54.0
> >
  

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: