Hi Geert, On 05/03/2021 13:37, Geert Uytterhoeven wrote: > Add device tree bindings documentation for the Renesas R-Car V3U Falcon > CSI/DSI and Ethernet sub-boards. These are plugged into the Falcon > BreakOut board to form the full Falcon board stack. Aha, so this answers my questions this week ;-) > Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx> > --- > Marked as RFC > > The Falcon board stack consists of 4 boards: > 1. CPU board, containing the R-Car V3U SoC, and core system parts like > RAM, console, eMMC, > 2. BreakOut board, providing power, an Ethernet PHY, and a backplane > where boards 1, 3, and 4 are plugged in, > 3. CSI/DSI sub-board, providing 2 GMSL displays and 12 GMSL cameras, > 4. Ethernet sub-board, providing 6 Ethernet PHYs. > > As the BreakOut board provides power, the CPU board cannot be used > without the BreakOut board. However, both the CSI/DSI and Ethernet > sub-boards are optional. So that means we have to support 4 stacks of > board combinations (1+2, 1+2+3, 1+2+4, 1+2+3+4). Presumably however - of course 2 could itself also be optional, with /another/ board being designed to fit in place. Which I assume is why 1 is separated at all. But that only comes when someone adds the DTB for that 'alternative' breakout. > That sounds like a good target for fdtoverlay, right? > > For now[1] the Falcon include hierarchy looks like this (supporting only > the full stack 1+2+3+4): > > r8a779a0-falcon.dts > |-- r8a779a0-falcon-cpu.dtsi > | `-- r8a779a0.dtsi > |-- r8a779a0-falcon-csi-dsi.dtsi > `-- r8a779a0-falcon-ethernet.dtsi > > Traditionally, we augmented the "model" and "compatible" properties of > the root node in each additional layer: > > r8a779a0.dtsi: > compatible = "renesas,r8a779a0"; > > r8a779a0-falcon-cpu.dtsi: > model = "Renesas Falcon CPU board"; > compatible = "renesas,falcon-cpu", "renesas,r8a779a0"; > > r8a779a0-falcon.dts: > model = "Renesas Falcon CPU and Breakout boards based on r8a779a0"; > compatible = "renesas,falcon-breakout", "renesas,falcon-cpu", "renesas,r8a779a0"; > > (Note: I haven't done that yet for the CSI/DSI and Ethernet sub-boards) > > With a stack of 4 boards, some optional, this becomes a bit cumbersome. > But it is still doable when using .dts and .dtsi files, by just adding 3 > more r8a779a0-falcon*.dts files. > > So we can add model/compatible properties to the sub-boards: > > r8a779a0-falcon-csi-dsi.dtsi > model = "Renesas Falcon CSI/DSI sub-board"; > compatible = "renesas,falcon-csi-dsi"; > > r8a779a0-falcon-ethernet.dtsi: > model = "Renesas Falcon Ethernet sub-board"; > compatible = "renesas,falcon-ethernet"; > > and update r8a779a0-falcon*dts to augment the properties. > > However, this is currently not possible when using overlays, as applying > an overlay would override the properties in the underlying DTB, not > augment them. > > Questions: > a. Should we document all possible combinations in the bindings file? > After this patch, we only have 1, 1+2, and 1+2+3+4 documented. Will '1' on it's own be of any use? Or will there always need to be a '2'? (which might be undefined / other board / '5'). > b. How to handle "model" and "compatible" properties for (sub)boards? > Perhaps fdtoverlay could combine the "model" and "compatible" > properties in the root nodes? However, that is not always desired. > Combining the models could get ... long and repetitive couldn't it ? I guess this is one advantage of having a separate .dts file and setting it explicitly... I guess there's no way to easily pass that in as a parameter of sorts :-( > Thanks for your comments! I'm looking forward to seeing this in action, and develop further. Especially for cleaning up some of the GMSL add on boards we have. -- Kieran > > [1] [PATCH v2 0/3] arm64: dts: renesas: falcon: Add I2C EEPROMs and sub-boards > https://lore.kernel.org/linux-renesas-soc/20210304153257.4059277-1-geert+renesas@xxxxxxxxx > --- > Documentation/devicetree/bindings/arm/renesas.yaml | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/Documentation/devicetree/bindings/arm/renesas.yaml b/Documentation/devicetree/bindings/arm/renesas.yaml > index 5fd0696a9f91f383..08ba12632f52c317 100644 > --- a/Documentation/devicetree/bindings/arm/renesas.yaml > +++ b/Documentation/devicetree/bindings/arm/renesas.yaml > @@ -296,6 +296,13 @@ properties: > - const: renesas,falcon-cpu > - const: renesas,r8a779a0 > > + - items: > + - const: renesas,falcon-breakout # Falcon BreakOut board (RTP0RC779A0BOB0010S) > + - const: renesas,falcon-csi-dsi # Falcon CSI/DSI sub-board (RTP0RC779A0DCS0010S) > + - const: renesas,falcon-ethernet # Falcon Ethernet sub-board (RTP0RC779A0ETS0010S) > + - const: renesas,falcon-cpu > + - const: renesas,r8a779a0 > + > - description: RZ/N1D (R9A06G032) > items: > - enum: >