| Message ID | 20210305133703.42179-1-geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | [RFC] dt-bindings: arm: renesas: Document Renesas Falcon sub-boards | expand |
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@glider.be> > --- > 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@glider.be > --- > 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: >
On Fri, Mar 05, 2021 at 02:37:03PM +0100, 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. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > 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). > > 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. > > 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. I think we just don't want to put sub-board compatibles in the root compatible at least if they are optional, peripheral components like this case seems to be. For something like a SoM plus baseboard I tend to feel differently. Do you really need it? I'd assume you could just check for the components? Or we define connectors and under the connector we define a top level compatible for the sub-board. This sounds like an eval or validation board? Those tend to have every possible option and I'm not sure we want to solve that before solving the simple cases. Rob
Hi Rob, On Mon, Mar 8, 2021 at 11:16 PM Rob Herring <robh@kernel.org> wrote: > On Fri, Mar 05, 2021 at 02:37:03PM +0100, 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. > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > --- > > 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). > > > > 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. > > > > 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. > > I think we just don't want to put sub-board compatibles in the root > compatible at least if they are optional, peripheral components like > this case seems to be. For something like a SoM plus baseboard I tend to > feel differently. OK, makes sense. > Do you really need it? I'd assume you could just check for the Just being cautious. We once (actually 5 times) needed a quirk for boards with regulators keeping shared IRQS asserted[1]. Something like that might happen with an expansion board, too. > components? Or we define connectors and under the connector we define a > top level compatible for the sub-board. This sounds like an eval or > validation board? Those tend to have every possible option and I'm not > sure we want to solve that before solving the simple cases. Let's do without for now. We can still check for main board compatible value + components when needed. Thanks! [1] arch/arm/mach-shmobile/regulator-quirk-rcar-gen2.c Gr{oetje,eeting}s, Geert
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:
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. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- 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). 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. 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. Thanks for your comments! [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@glider.be --- Documentation/devicetree/bindings/arm/renesas.yaml | 7 +++++++ 1 file changed, 7 insertions(+)