Hi Krzysztof, On Wed, 9 Apr 2025 at 08:37, Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote: > On 09/04/2025 03:05, Kuninori Morimoto wrote: > > Renesas MSIOF (Clock-Synchronized Serial Interface with FIFO) can work as > > both SPI and I2S. MSIOF-I2S will use Audio Graph Card/Card2 driver which > > uses Of-Graph in DT. > > > MSIOF-SPI/I2S are using same DT compatible properties. > > MSIOF-I2S uses Of-Graph for Audio-Graph-Card/Card2, > > MSIOF-SPI doesn't use Of-Graph. > > > > Adds MSIOF-I2S documentation for Sound. > > > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx> > > --- > > .../bindings/sound/renesas,msiof.yaml | 112 ++++++++++++++++++ > > 1 file changed, 112 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/sound/renesas,msiof.yaml > > > > diff --git a/Documentation/devicetree/bindings/sound/renesas,msiof.yaml b/Documentation/devicetree/bindings/sound/renesas,msiof.yaml > > new file mode 100644 > > index 000000000000..5173e80698fb > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/sound/renesas,msiof.yaml > > @@ -0,0 +1,112 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/sound/renesas,msiof.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Renesas MSIOF I2S controller > > + > > +maintainers: > > + - Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx> > > + > > +# sharing with MSIOF SPI > > +# see > > +# ${LINUX}/Documentation/devicetree/bindings/spi/renesas,sh-msiof.yaml http://devicetree.org/schemas/spi/renesas,sh-msiof.yaml > > +select: > > + properties: > > + compatible: > > + contains: > > + pattern: "renesas,.*-msiof" > > + required: > > + - compatible > > + - port > > Drop entire select. This is needed to avoid matching when using the device in SPI mode. > > +properties: > > + compatible: > > + items: > > + - const: renesas,msiof-r8a779g0 # R-Car V4H > > Use expected format of all soc compatibles. It has been always: SoC-module. This is a pre-existing compatible value, so it cannot be changed. > > + - const: renesas,rcar-gen4-msiof # generic R-Car Gen4 > > If you have duplicated compatibles then: > 1. It rarely makes sense because you claim that two different devices > are using the same compatible. Different device, different compatible. > 2. Or if this is really same device, then only one schema. This the same device, but it can be used in two (actually more) different modes: SPI and I2S. Hence it has two separate DT binding documents. If this needs to be merged (the result is gonna be ugly): where to fit it in the DT binding doc hierarchy? > > + reg: > > + minItems: 1 > > + maxItems: 2 > > Drop these two. > > > + oneOf: > > Why is this flexible? I am not sure where this is coming from (an old SH part?). The SPI bindings have the same construct. As this binding supports R-Car Gen4 only, a single reg should be fine. > > > + - items: > > + - description: CPU and DMA engine registers > > + - items: > > + - description: CPU registers > > + - description: DMA engine registers > > + dmas: > > + minItems: 2 > > + maxItems: 4 > > Why flexible? > > > + > > + dma-names: > > + minItems: 2 > > + maxItems: 4 > > + items: > > + enum: [ tx, rx ] > > How would that work? tx rx tx rx? And then driver requests 'tx' (by > name) and what is supposed to be returned? The module may be connected to one or more DMA controllers (see below). > > + > > + msiof1: serial@e6ea0000 { > > serial means UART controller. You need name matching the class of the > device. > Node names should be generic. See also an explanation and list of > examples (not exhaustive) in DT specification: > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation What is the recommend generic node name for a flexible serial device that can operate as (a.o.) either SPI or I2S controller? > > + compatible = "renesas,msiof-r8a779g0", > > + "renesas,rcar-gen4-msiof"; > > + reg = <0 0xe6ea0000 0 0x0064>; > > + interrupts = <GIC_SPI 240 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&cpg CPG_MOD 619>; > > + dmas = <&dmac0 0x43>, <&dmac0 0x42>, > > + <&dmac1 0x43>, <&dmac1 0x42>; > > + dma-names = "tx", "rx", "tx", "rx"; > > So test it now - get DMA by name 'tx'. What do you get? A handle to either <&dmac0 0x43> or <&dmac1 0x43>; which one is random. It's been working like that for ages. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
