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

[John Madieu]

Hi Geert,

Thank you for the review.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: Freitag, 24. April 2026 08:24
> To: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> Subject: Re: [PATCH v5 01/14] ASoC: dt-bindings: sound: Add DT binding for
> RZ/G3E sound
> 
> Hi John,
> 
> On Fri, 24 Apr 2026 at 03:39, John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> wrote:
> > > From: Krzysztof Kozlowski <krzk@xxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> 
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/sound/renesas,r9a09g047-
> sound.
> > > > +++ yaml
> 
> > > > +  "#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".
> >
> > The IP exposes ten independent SSI interfaces, and a board can
> > reasonably wire either a single SSI to one codec or several SSIs to
> > several codecs. The cells value follows that wiring: 0 when the
> > phandle is `<&rcar_sound>` for the single-DAI case, 1 when it is
> > `<&rcar_sound N>` selecting a specific DAI index.
> 
> How does this work if both types of wiring are present?
> e.g. SSI1 wired to one codec, and SSI2-4 wired to several codecs?
> 

I don't think both forms can really coexist on the same provider:
as `#sound-dai-cells` is declared once on the sound node, so on
a given board all references have to use the same cell count
anyway. I might be wrong. For your mixed example, the board would
end up with `#sound-dai-cells = <1>` and all consumer references
would carry an index (the SSI1 one included).

The reason I kept both values in the enum was not to allow mixing
within a DTS, but to leave room for the two different kinds of board
that will plausibly use this IP:

  - a minimal board with one SSI wired to one codec, which can pick
    `#sound-dai-cells = <0>` and write the reference as
    `<&snd_rzg3e>`, without an index;

  - a board with multiple codecs (or one that wants to keep the door
    open for more), which picks `#sound-dai-cells = <1>` and writes
    all references as `<&snd_rzg3e N>`.

So the enum was meant as "pick one at board level" rather than
"both allowed at once".

This follows what the existing R-Car sound binding does, and the
convention is also spelled out as a comment on the sound node in the
SoC dtsi:

    /*
     * #sound-dai-cells is required
     *
     * Single DAI : #sound-dai-cells = <0>; <&rcar_sound>;
     * Multi  DAI : #sound-dai-cells = <1>; <&rcar_sound N>;
     */

That said, I'm happy to tighten this to `const: 1` if you (or
Krzysztof) think the flexibility isn't worth keeping. It would
just mean single-codec boards also have to write the index.
Would that be preferable?

> > > > +  clock-names:
> > > > +    items:
> > > > +      - const: ssi-all
> > > > +      - const: ssi.9
> > >
> > > Use consistently -
> >
> > Agreed, I'll switch to hyphens for all indexed entries in both lists
> > (ssi-0..9, src-0..9, mix-0..1, ctu-0..7, dvc-0..1, adg-ssi-0..9).
> >
> > > > +      - 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
> 
> Why are these listed in descending order...
> 
> > > > +      - const: dvc.0
> > > > +      - const: dvc.1
> 
> ... and these in ascending order?

That's inconsistent and there's no real reason for it.
I'll reorder every block ascending (0 -> N) so ssi, src, mix,
ctu and dvc all follow the same convention

Regards,
John