[PATCH RFC] dt-bindings: arm: renesas: Document Renesas Falcon sub-boards

[PATCH RFC] dt-bindings: arm: renesas: Document Renesas Falcon sub-boards

[Geert Uytterhoeven]

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(+)

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:
-- 
2.25.1

Re: [PATCH RFC] dt-bindings: arm: renesas: Document Renesas Falcon sub-boards

[Kieran Bingham]

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:
> 

Re: [PATCH RFC] dt-bindings: arm: renesas: Document Renesas Falcon sub-boards

[Rob Herring]

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

Re: [PATCH RFC] dt-bindings: arm: renesas: Document Renesas Falcon sub-boards

[Geert Uytterhoeven]

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

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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