RE: [PATCH v6 01/16] ASoC: dt-bindings: sound: Add DT binding for RZ/G3E sound

[John Madieu]

Hi Rob,

Thanks for your review.

> -----Original Message-----
> From: Rob Herring <robh@xxxxxxxxxx>
> Sent: Donnerstag, 14. Mai 2026 16:28
> To: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> Subject: Re: [PATCH v6 01/16] ASoC: dt-bindings: sound: Add DT binding for
> RZ/G3E sound
> 
> On Tue, May 12, 2026 at 06:26:16PM +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@xxxxxxxxxxxxxx>
> > ---
> >
> > Changes:
> >
> > v6:
> >  - Rename all indexed clock-names and reset-names from the dotted
> >    form (ssi.0, src.0, adg.ssi.0, clk_a, clk_b, clk_c, clk_i) to
> >    the hyphenated form (ssi-0, src-0, adg-ssi-0, audio-clka,
> >    audio-clkb, audio-clkc, audio-clki) so the new binding follows
> >    the standard DT naming convention.
> >  - Tighten #sound-dai-cells to const: 1.
> >  - Drop unused properties: clock-frequency, clkout-lr-asynchronous.
> >  - Simplify the ports/endpoint schema (single ports object with
> >    port@N children referencing audio-graph-port.yaml), drop the
> >    separate top-level dai patternProperties block.
> >  - Move additionalProperties: false to the top of each sub-object
> >    (dvc, mix, ctu, src, ssiu, ssi).
> >  - Reorder example clocks/resets to match the new ordinal-ascending
> >    name order.
> >
> > 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 Krzysztof'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        | 743 ++++++++++++++++++
> >  1 file changed, 743 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..0b651214bd61
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/sound/renesas,r9a09g047-sound.
> > +++ yaml
> > @@ -0,0 +1,743 @@
> > +# 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@xxxxxxxxxxx>
> > +  - John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> > +
> > +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":
> > +    const: 1
> > +
> > +  "#clock-cells":
> > +    const: 0
> > +
> > +  "#address-cells":
> > +    const: 1
> > +
> > +  "#size-cells":
> > +    const: 0
> > +
> > +  clocks:
> > +    maxItems: 47
> > +
> > +  clock-names:
> > +    items:
> > +      - const: ssi-all
> > +      - const: ssi-0
> > +      - const: ssi-1
> > +      - const: ssi-2
> > +      - const: ssi-3
> > +      - const: ssi-4
> > +      - const: ssi-5
> > +      - const: ssi-6
> > +      - const: ssi-7
> > +      - const: ssi-8
> > +      - const: ssi-9
> > +      - const: src-0
> > +      - const: src-1
> > +      - const: src-2
> > +      - const: src-3
> > +      - const: src-4
> > +      - const: src-5
> > +      - const: src-6
> > +      - const: src-7
> > +      - const: src-8
> > +      - const: src-9
> > +      - const: mix-0
> > +      - const: mix-1
> > +      - const: ctu-0
> > +      - const: ctu-1
> > +      - const: dvc-0
> > +      - const: dvc-1
> > +      - const: audio-clka
> > +      - const: audio-clkb
> > +      - const: audio-clkc
> > +      - const: audio-clki
> > +      - const: ssif_supply
> > +      - const: scu
> > +      - const: scu_x2
> > +      - const: scu_supply
> > +      - const: adg-ssi-0
> > +      - const: adg-ssi-1
> > +      - const: adg-ssi-2
> > +      - const: adg-ssi-3
> > +      - const: adg-ssi-4
> > +      - const: adg-ssi-5
> > +      - const: adg-ssi-6
> > +      - const: adg-ssi-7
> > +      - const: adg-ssi-8
> > +      - const: adg-ssi-9
> > +      - const: audmapp
> > +      - const: adg
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  resets:
> > +    maxItems: 14
> > +
> > +  reset-names:
> > +    items:
> > +      - const: ssi-all
> > +      - const: ssi-0
> > +      - const: ssi-1
> > +      - const: ssi-2
> > +      - const: ssi-3
> > +      - const: ssi-4
> > +      - const: ssi-5
> > +      - const: ssi-6
> > +      - const: ssi-7
> > +      - const: ssi-8
> > +      - const: ssi-9
> > +      - const: scu
> > +      - const: adg
> > +      - const: audmapp
> > +
> > +  dvc:
> > +    type: object
> > +    additionalProperties: false
> 
> blank line
> 
> > +    patternProperties:
> > +      "^dvc-[0-1]$":
> > +        type: object
> > +        additionalProperties: false
> 
> blank line
> 
> > +        properties:
> > +          dmas:
> > +            maxItems: 5
> 
> blank line

Will add, here and in the equivalent src, ssiu, ssi blocks.

> 
> > +          dma-names:
> > +            maxItems: 5
> > +            allOf:
> 
> Don't need allOf.

I tried dropping it and dt_binding_check rejects the bare form
("items: {const/enum} is not of type 'array'" from
string-array.yaml). The items-as-list form doesn't fit either,
because the length varies (up to 5 AUDMAC controllers per
direction), so maxItems is needed alongside the per-entry
constraint, and the meta-schema rejects maxItems together with
an items list.

The allOf wrapper is what renesas,rsnd.yaml uses on src, ssiu and
ssi for the same kind of constraint. Is there a cleaner construct
for "string array, maxItems N, every entry must match this enum"?
Happy to switch if so.

> 
> > +              - items:
> > +                  enum:
> > +                    - tx
> 
> Is 5 entries of 'tx' really what you want?

Yes. On RZ/G3E each direction is fanned out across up to 5 AUDMAC
controllers. Each dmas entry points at a different AUDMAC, and all
5 share the name "tx" so the dma-engine core's match-string lookup
picks the first matching entry, falls through to the next when
that provider's .of_xlate() can't satisfy the request, and so on.
The driver just calls dma_request_chan(dev, "tx") and gets back
whichever AUDMAC had a free channel, without needing to know the
topology.

Will expand the commit message and add a paragraph in the
binding's top-level description making the fallback explicit
so it doesn't read as a mistake.

> 
> blank line
> 
> > +        required:
> > +          - dmas
> > +          - dma-names
> > +
> > +  mix:
> > +    type: object
> > +    additionalProperties: false
> > +    patternProperties:
> > +      "^mix-[0-1]$":
> > +        type: object
> > +        additionalProperties: false
> 
> There is little point in empty nodes.

The rsnd driver enumerates MIX and CTU instances via
of_get_child_by_name(), and DT labels are attached to these
sub-nodes for the playback/capture phandle routing arrays. Without
the nodes there is nothing to label and nothing for the driver to
iterate. The same empty patternProperties exist in
renesas,rsnd.yaml today.

Will add a description block on the mix and ctu objects so the
purpose of the empty patternProperties is clear. Is there a tidier
way to express "this node exists for labelling and enumeration
only" that you have in mind?

> 
> > +
> > +  ctu:
> > +    type: object
> > +    additionalProperties: false
> > +    patternProperties:
> > +      "^ctu-[0-7]$":
> > +        type: object
> > +        additionalProperties: false
> > +
> > +  src:
> > +    type: object
> > +    additionalProperties: false
> > +    patternProperties:
> > +      "^src-[0-9]$":
> > +        type: object
> > +        additionalProperties: false
> > +        properties:
> > +          interrupts:
> > +            maxItems: 1
> > +          dmas:
> > +            maxItems: 10
> > +          dma-names:
> > +            maxItems: 10
> > +            allOf:
> 
> Don't need allOf.

Same situation as the dvc-N case above; will follow whatever
resolution we land on there.

> 
> > +              - items:
> > +                  enum:
> > +                    - tx
> > +                    - rx
> 
> 10 entries of any combination of tx and rx?

Same reasoning as dvc-N but for both directions: up to 5 "tx" plus
up to 5 "rx", each duplicate name being a separate AUDMAC fallback
target for the dma-engine core to try. The same description
paragraph at the top of the binding will cover this case.

If you'd prefer a tighter constraint (enforcing exactly 5 "tx" +
5 "rx" in some specific order, or splitting the property into
separate tx-only and rx-only halves), let me know.

> 
> > +
> > +  ssiu:
> > +    type: object
> > +    additionalProperties: false
> > +    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
> > +
> > +  ssi:
> > +    type: object
> > +    additionalProperties: false
> > +    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
> > +
> > +  ports:
> > +    $ref: audio-graph-port.yaml#/definitions/port-base
> > +    unevaluatedProperties: false
> > +    patternProperties:
> > +      '^port@[0-9a-f]+$':
> > +        $ref: audio-graph-port.yaml#/definitions/port-base
> > +        unevaluatedProperties: false
> > +        properties:
> > +          reg:
> > +            maxItems: 1
> > +          endpoint:
> > +            $ref: audio-graph-port.yaml#/definitions/endpoint-base
> > +            unevaluatedProperties: false
> > +            properties:
> > +              playback:
> > +                $ref: /schemas/types.yaml#/definitions/phandle-array
> > +              capture:
> > +                $ref: /schemas/types.yaml#/definitions/phandle-array
> 
> This is odd. The graph should really just point to another endpoint along
> with any properties for the connection. These probably belong elsewhere.
> What do these point to? Missing any sort of description or constraints.

Each phandle in these arrays references one of the in-node ssi-N
/ src-N / ctu-N / mix-N / dvc-N sub-nodes; they describe the
in-SoC DAI module chain that the rsnd driver wires up for each
direction. The remote-endpoint phandle on the same endpoint
describes the off-SoC connection. The shape was carried over from
the existing renesas,rsnd binding, which has the same properties
on the audio-graph endpoint, and is where asoc-audio-graph-card
and the rsnd parser read them from.

I'll add proper type definitions, item constraints and
descriptions either way. Where would you like the properties to
live? Or is there any other approach I should have followed ?

Regards,
John