On Wed, Apr 09, 2025 at 09:01:22AM GMT, Geert Uytterhoeven wrote:
> > > +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.
Which you need to avoid, so drop the select. One device, one schema.
>
> > > +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):
... then next time don't post incomplete bindings. I know we do not have
time machine, but any mess is on contributors who posted some limited
scope/view of the hardware entirely ignoring the rest of interfaces.
> where to fit it in the DT binding doc hierarchy?
Does not matter, whatever fits better in overal picture/purpose of this
device.
>
> > > + 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).
Yes, but how the implementation would work?
Anyway, this needs to be strictly ordered, not random rx rx tx tx or rx
rx rx rx.
>
> > > +
> > > + 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?
i2s
or even not so generic msiof, but definitely not serial because that is
reserved for UART.
>
> > > + 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.
Interesting. And is this expected behavior? Driver does not care which
RX and which TX it gets? Like RX from dmac0 and TX from dmac1?
Best regards,
Krzysztof
