Re: [PATCH v5 01/14] ASoC: dt-bindings: sound: Add DT binding for RZ/G3E sound

[Krzysztof Kozlowski <krzk@kernel.org>]

On Wed, Apr 15, 2026 at 12:47:18PM +0000, John Madieu wrote:
> Add a standalone device tree binding for the Renesas RZ/G3E (R9A09G047)
> sound controller.
> 
> The RZ/G3E sound IP is based on R-Car Sound but differs in several ways:
> - Uses unprefixed sub-node names (ssi, ssiu, src, dvc, mix, ctu) instead
>   of R-Car's rcar_sound,xxx prefixed names.
> - Supports up to 5 DMA controllers per direction, allowing multiple DMA
>   entries with repeated channel names in SSIU, SRC and DVC sub-nodes.
> - Has 47 clocks including per-SSI ADG clocks (adg.ssi.0-9), SCU clocks
>   (scu, scu_x2, scu_supply), SSIF supply clock, AUDMAC peri-peri clock,
>   and ADG clock.
> - Has 14 reset lines including SCU, ADG and AUDMAC peri-peri resets.
> - SSI operates exclusively in BUSIF mode.
> 
> These differences make the RZ/G3E binding incompatible with the existing
> renesas,rsnd.yaml, so it is added as a separate standalone binding with
> its own $ref to dai-common.yaml.
> 
> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>

DCO/author mismatch.

Did you run checkpatch?

> ---
> 
> Changes:
>  
> v5:
>  - Drop the two-patch rsnd.yaml split approach from v4.
>    Replace with a single self-contained standalone binding that does
>    not touch renesas,rsnd.yaml at all.
>  - Remove select: false, redundant blanket properties (compatible: true,
>    reg: true, etc.) and pointless patternProperties per Krzystof's review
>  - Add missing #clock-cells and #sound-dai-cells constraints
>  - Add hardware description text instead of "Binding for ..." phrasing
>  - Move G3E-specific DMA comment into the binding itself rather than
>    relying on a shared schema
>  - Use unprefixed sub-node names (ssi, ssiu, src, dvc, mix, ctu) to
>    reflect the actual RZ/G3E DT binding
> 
> v4: No changes
> v3: No changes
> v2:
>  - Introduce RZ/G3E sound binding as a standalone schema
> 
>  .../sound/renesas,r9a09g047-sound.yaml        | 770 ++++++++++++++++++
>  1 file changed, 770 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/renesas,r9a09g047-sound.yaml
> 
> diff --git a/Documentation/devicetree/bindings/sound/renesas,r9a09g047-sound.yaml b/Documentation/devicetree/bindings/sound/renesas,r9a09g047-sound.yaml
> new file mode 100644
> index 000000000000..b7e5348636bb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/renesas,r9a09g047-sound.yaml
> @@ -0,0 +1,770 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/renesas,r9a09g047-sound.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas RZ/G3E Sound Controller
> +
> +maintainers:
> +  - Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> +  - John Madieu <john.madieu.xa@bp.renesas.com>
> +
> +description:
> +  The RZ/G3E (R9A09G047) sound controller is based on R-Car Sound IP
> +  with extended DMA channel support (up to 5 DMACs per direction),
> +  additional clock domains (47 clocks including per-SSI ADG clocks),
> +  and additional reset lines (14 including SCU, ADG and Audio DMAC
> +  peri-peri resets). SSI operates exclusively in BUSIF mode with
> +  2-4 BUSIF channels per SSI.
> +
> +allOf:
> +  - $ref: dai-common.yaml#
> +
> +properties:
> +  compatible:
> +    const: renesas,r9a09g047-sound
> +
> +  reg:
> +    maxItems: 5
> +
> +  reg-names:
> +    items:
> +      - const: scu
> +      - const: adg
> +      - const: ssiu
> +      - const: ssi
> +      - const: audmapp
> +
> +  "#sound-dai-cells":
> +    enum: [0, 1]

Why is this flexible? That's a defined device meaning you have one XOR more
DAIs. Not "1 and more".

> +
> +  "#clock-cells":
> +    const: 0
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +  clocks:
> +    maxItems: 47
> +
> +  clock-names:
> +    items:
> +      - const: ssi-all
> +      - const: ssi.9


Use consistently -

> +      - const: ssi.8
> +      - const: ssi.7
> +      - const: ssi.6
> +      - const: ssi.5
> +      - const: ssi.4
> +      - const: ssi.3
> +      - const: ssi.2
> +      - const: ssi.1
> +      - const: ssi.0
> +      - const: src.9
> +      - const: src.8
> +      - const: src.7
> +      - const: src.6
> +      - const: src.5
> +      - const: src.4
> +      - const: src.3
> +      - const: src.2
> +      - const: src.1
> +      - const: src.0
> +      - const: mix.1
> +      - const: mix.0
> +      - const: ctu.1
> +      - const: ctu.0
> +      - const: dvc.0
> +      - const: dvc.1
> +      - const: clk_a

And here as well

name "clk_a" is half useless, because this cannot be anything else than
clk, thus basically you said "a". What is a?


> +      - const: clk_b
> +      - const: clk_c
> +      - const: clk_i
> +      - const: ssif_supply
> +      - const: scu
> +      - const: scu_x2
> +      - const: scu_supply
> +      - const: adg.ssi.9
> +      - const: adg.ssi.8
> +      - const: adg.ssi.7
> +      - const: adg.ssi.6
> +      - const: adg.ssi.5
> +      - const: adg.ssi.4
> +      - const: adg.ssi.3
> +      - const: adg.ssi.2
> +      - const: adg.ssi.1
> +      - const: adg.ssi.0
> +      - const: audmapp
> +      - const: adg
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  resets:
> +    maxItems: 14
> +
> +  reset-names:
> +    items:
> +      - const: ssi-all
> +      - const: ssi.9

s/./-/

> +      - const: ssi.8
> +      - const: ssi.7
> +      - const: ssi.6
> +      - const: ssi.5
> +      - const: ssi.4
> +      - const: ssi.3
> +      - const: ssi.2
> +      - const: ssi.1
> +      - const: ssi.0
> +      - const: scu
> +      - const: adg
> +      - const: audmapp
> +
> +  clock-frequency:
> +    description: Audio clock output frequency.

Drop, this is a legacy property, not really allowed for new devices
which are not I2C buses.

> +
> +  clkout-lr-asynchronous:

Missing vendor prefix.

> +    description: audio_clkoutn is asynchronous with lr-clock.
> +    $ref: /schemas/types.yaml#/definitions/flag
> +
> +  dvc:

Mixing nodes with and without addressing is discouraged. Why do you have
such mixup?

This node looks empty, so just define dvc-[01] directly. Same for other
cases.

> +    type: object
> +    patternProperties:
> +      "^dvc-[0-1]$":
> +        type: object
> +        additionalProperties: false
> +        properties:
> +          dmas:
> +            maxItems: 5
> +          dma-names:
> +            maxItems: 5
> +            allOf:
> +              - items:
> +                  enum:
> +                    - tx
> +        required:
> +          - dmas
> +          - dma-names
> +    additionalProperties: false
> +
> +  mix:
> +    type: object
> +    patternProperties:
> +      "^mix-[0-1]$":
> +        type: object
> +        additionalProperties: false
> +    additionalProperties: false
> +
> +  ctu:
> +    type: object
> +    patternProperties:
> +      "^ctu-[0-7]$":
> +        type: object
> +        additionalProperties: false
> +    additionalProperties: false
> +
> +  src:
> +    type: object
> +    patternProperties:
> +      "^src-[0-9]$":
> +        type: object
> +        additionalProperties: false
> +        properties:
> +          interrupts:
> +            maxItems: 1
> +          dmas:
> +            maxItems: 10
> +          dma-names:
> +            maxItems: 10
> +            allOf:
> +              - items:
> +                  enum:
> +                    - tx
> +                    - rx
> +    additionalProperties: false
> +
> +  ssiu:
> +    type: object
> +    patternProperties:
> +      "^ssiu-[0-9]+$":
> +        type: object
> +        additionalProperties: false
> +        properties:
> +          dmas:
> +            maxItems: 10
> +          dma-names:
> +            maxItems: 10
> +            allOf:
> +              - items:
> +                  enum:
> +                    - tx
> +                    - rx
> +        required:
> +          - dmas
> +          - dma-names
> +    additionalProperties: false
> +
> +  ssi:
> +    type: object
> +    patternProperties:
> +      "^ssi-[0-9]$":
> +        type: object
> +        additionalProperties: false
> +        properties:
> +          interrupts:
> +            maxItems: 1
> +          dmas: true
> +          dma-names: true
> +          shared-pin:
> +            description: Shared clock pin.
> +            $ref: /schemas/types.yaml#/definitions/flag
> +        required:
> +          - interrupts
> +    additionalProperties: false
> +
> +  port:
> +    $ref: audio-graph-port.yaml#/definitions/port-base
> +    unevaluatedProperties: false
> +    patternProperties:
> +      "^endpoint(@[0-9a-f]+)?$":
> +        $ref: audio-graph-port.yaml#/definitions/endpoint-base
> +        properties:
> +          playback:
> +            $ref: /schemas/types.yaml#/definitions/phandle-array
> +          capture:
> +            $ref: /schemas/types.yaml#/definitions/phandle-array
> +        unevaluatedProperties: false
> +
> +patternProperties:
> +  '^dai(@[0-9a-f]+)?$':

Why node addressing is optional?

> +    type: object
> +    patternProperties:
> +      "^dai([0-9]+)?$":

You did not verify your DTS, you have warnings here. And I really do not
understand why dai@0 has dai@0 again.


> +        type: object
> +        additionalProperties: false
> +        properties:
> +          playback:
> +            $ref: /schemas/types.yaml#/definitions/phandle-array
> +          capture:
> +            $ref: /schemas/types.yaml#/definitions/phandle-array
> +        anyOf:
> +          - required:
> +              - playback
> +          - required:
> +              - capture
> +    additionalProperties: false
> +
> +  'ports(@[0-9a-f]+)?$':

Why ports even have addferssing?

> +    $ref: audio-graph-port.yaml#/definitions/port-base

So this is port base or ports? How port-base could have one more port as
a child? Open the port-base definition and look there.

> +    unevaluatedProperties: false
> +    patternProperties:
> +      '^port(@[0-9a-f]+)?$':

No, you must define exactly what the ports are.

> +        $ref: "#/properties/port"
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - clocks
> +  - clock-names
> +  - resets
> +  - reset-names

Best regards,
Krzysztof