diff mbox series

[01/14] dt-bindings: arm: renesas: Document R-Car H3e-2G and M3e-2G SoCs and boards

Message ID f79841c1881f8b9a2c10fadb3d3ad6cb5fccc6a5.1623315732.git.geert+renesas@glider.be (mailing list archive)
State New
Delegated to: Geert Uytterhoeven
Headers show
Series arm64: renesas: Add support for R Car H3e 2G-and M3e-2G | expand

Commit Message

Geert Uytterhoeven June 10, 2021, 9:37 a.m. UTC
Document the compatible values for the R-Car H3e-2G (R8A779M1) and
M3e-2G (R8A779M3) SoCs.  These are different gradings of the R-Car H3
ES3.0 (R8A77951) and M3-W+ (R8A77961) SoCs.

All R-Car Gen3e on-SoC devices are identical to the devices on the
corresponding R-Car Gen3 SoCs, and thus just use the compatible values
for the latter.  The root compatible properties do gain an additional
value, to sort out integration issues if they ever arise.

Document the use of these SoCs on the Salvator-XS and ULCB (with and
without Kingfisher) development boards.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 .../devicetree/bindings/arm/renesas.yaml      | 50 +++++++++++++++----
 1 file changed, 39 insertions(+), 11 deletions(-)

Comments

Laurent Pinchart June 13, 2021, 1:13 a.m. UTC | #1
Hi Geert,

Thank you for the patch.

On Thu, Jun 10, 2021 at 11:37:14AM +0200, Geert Uytterhoeven wrote:
> Document the compatible values for the R-Car H3e-2G (R8A779M1) and
> M3e-2G (R8A779M3) SoCs.  These are different gradings of the R-Car H3
> ES3.0 (R8A77951) and M3-W+ (R8A77961) SoCs.
> 
> All R-Car Gen3e on-SoC devices are identical to the devices on the
> corresponding R-Car Gen3 SoCs, and thus just use the compatible values
> for the latter.  The root compatible properties do gain an additional
> value, to sort out integration issues if they ever arise.
> 
> Document the use of these SoCs on the Salvator-XS and ULCB (with and
> without Kingfisher) development boards.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

(Copying a comment from another e-mail)

I however wonder if we haven't messed up the board compatible strings
somehow (unrelated to this patch). Aren't compatible strings supposed to
be ordered from most specific to most generic, with a more specific
compatible string being a strict subset of a more generic string ?
Looking at, for example,

        compatible = "renesas,salvator-xs", "renesas,r8a779m1", "renesas,r8a7795";

the rule is upheld by renesas,r8a779m1 being a subset of the more
generic renesas,r8a7795, but that's not the case for
renesas,salvator-xs.

> ---
>  .../devicetree/bindings/arm/renesas.yaml      | 50 +++++++++++++++----
>  1 file changed, 39 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/renesas.yaml b/Documentation/devicetree/bindings/arm/renesas.yaml
> index 5fd0696a9f91f383..a01dd064bf16632a 100644
> --- a/Documentation/devicetree/bindings/arm/renesas.yaml
> +++ b/Documentation/devicetree/bindings/arm/renesas.yaml
> @@ -238,17 +238,29 @@ properties:
>            - const: renesas,r8a77961
>  
>        - description: Kingfisher (SBEV-RCAR-KF-M03)
> -        items:
> -          - const: shimafuji,kingfisher
> -          - enum:
> -              - renesas,h3ulcb
> -              - renesas,m3ulcb
> -              - renesas,m3nulcb
> -          - enum:
> -              - renesas,r8a7795
> -              - renesas,r8a7796
> -              - renesas,r8a77961
> -              - renesas,r8a77965
> +        oneOf:
> +          - items:
> +              - const: shimafuji,kingfisher
> +              - enum:
> +                  - renesas,h3ulcb
> +                  - renesas,m3ulcb
> +                  - renesas,m3nulcb
> +              - enum:
> +                  - renesas,r8a7795
> +                  - renesas,r8a7796
> +                  - renesas,r8a77961
> +                  - renesas,r8a77965
> +          - items:
> +              - const: shimafuji,kingfisher
> +              - enum:
> +                  - renesas,h3ulcb
> +                  - renesas,m3ulcb
> +              - enum:
> +                  - renesas,r8a779m1
> +                  - renesas,r8a779m3
> +              - enum:
> +                  - renesas,r8a7795
> +                  - renesas,r8a77961
>  
>        - description: R-Car M3-N (R8A77965)
>          items:
> @@ -296,6 +308,22 @@ properties:
>            - const: renesas,falcon-cpu
>            - const: renesas,r8a779a0
>  
> +      - description: R-Car H3e-2G (R8A779M1)
> +        items:
> +          - enum:
> +              - renesas,h3ulcb      # H3ULCB (R-Car Starter Kit Premier)
> +              - renesas,salvator-xs # Salvator-XS (Salvator-X 2nd version)
> +          - const: renesas,r8a779m1
> +          - const: renesas,r8a7795
> +
> +      - description: R-Car M3e-2G (R8A779M3)
> +        items:
> +          - enum:
> +              - renesas,m3ulcb      # M3ULCB (R-Car Starter Kit Pro)
> +              - renesas,salvator-xs # Salvator-XS (Salvator-X 2nd version)
> +          - const: renesas,r8a779m3
> +          - const: renesas,r8a77961
> +
>        - description: RZ/N1D (R9A06G032)
>          items:
>            - enum:
Geert Uytterhoeven June 14, 2021, 11:25 a.m. UTC | #2
Hi Laurent,

On Sun, Jun 13, 2021 at 3:13 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Thu, Jun 10, 2021 at 11:37:14AM +0200, Geert Uytterhoeven wrote:
> > Document the compatible values for the R-Car H3e-2G (R8A779M1) and
> > M3e-2G (R8A779M3) SoCs.  These are different gradings of the R-Car H3
> > ES3.0 (R8A77951) and M3-W+ (R8A77961) SoCs.
> >
> > All R-Car Gen3e on-SoC devices are identical to the devices on the
> > corresponding R-Car Gen3 SoCs, and thus just use the compatible values
> > for the latter.  The root compatible properties do gain an additional
> > value, to sort out integration issues if they ever arise.
> >
> > Document the use of these SoCs on the Salvator-XS and ULCB (with and
> > without Kingfisher) development boards.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks!

> I however wonder if we haven't messed up the board compatible strings
> somehow (unrelated to this patch). Aren't compatible strings supposed to
> be ordered from most specific to most generic, with a more specific
> compatible string being a strict subset of a more generic string ?
> Looking at, for example,
>
>         compatible = "renesas,salvator-xs", "renesas,r8a779m1", "renesas,r8a7795";
>
> the rule is upheld by renesas,r8a779m1 being a subset of the more
> generic renesas,r8a7795, but that's not the case for
> renesas,salvator-xs.

That's a very interesting comment.  Originally, we had lists like:

    compatible = "renesas,koelsch", "renesas,r8a7791";

with the Koelsch board indeed being a specialization of an R-Car
M2-W-based system. Later, we reused that system for the Salvator-X
board with an R-Car H3 SiP:

    compatible = "renesas,salvator-x", "renesas,r8a7795";

That scheme became "broken" with the introduction of the R-Car M3-W
SiP, which was also mounted on a Salvator-X board, leading to:

    compatible = "renesas,salvator-x", "renesas,r8a7796";

Note that we did have a similar case for R-Car M2-W and R-Car M2-N on
the Koelsch resp. Gose boards: from the schematics (I haven't seen
a Gose), it looks identical to Koelsch, with parts not supported by
R-Car M2-N (like the second SDRAM bank) marked "Do not stuff".
But in this case the boards were assigned different names, thus
leading to different compatible values.

With Salvator-X(S), it was easier to support multiple SoCs, as they
are mounted on SiPs, with differences like the different number of
memory channels hidden in the SiP, and handled at a different level
(these days memory layout information flows from ATF to U-Boot to
the DTB passed to the kernel).

Would you feel more comfortable if we had introduced more
board-specific compatible values, like "renesas,r8a7796-salvator-x",
and had used

    compatible = "renesas,r8a7795-salvator-x", "renesas,salvator-x",
"renesas,r8a7795";

or

    compatible = "renesas,r8a7795-salvator-x", "renesas,r8a7795";

?

If the need ever arises, Linux can still identify the exact combination
by checking for both the board- and the SoC-specific values.
So far we didn't have that need for Salvator-X(S) yet (we do have
board-specific checks in
arch/arm/mach-shmobile/regulator-quirk-rcar-gen2.c).

> > --- a/Documentation/devicetree/bindings/arm/renesas.yaml
> > +++ b/Documentation/devicetree/bindings/arm/renesas.yaml
> > @@ -238,17 +238,29 @@ properties:
> >            - const: renesas,r8a77961
> >
> >        - description: Kingfisher (SBEV-RCAR-KF-M03)
> > -        items:
> > -          - const: shimafuji,kingfisher
> > -          - enum:
> > -              - renesas,h3ulcb
> > -              - renesas,m3ulcb
> > -              - renesas,m3nulcb
> > -          - enum:
> > -              - renesas,r8a7795
> > -              - renesas,r8a7796
> > -              - renesas,r8a77961
> > -              - renesas,r8a77965
> > +        oneOf:
> > +          - items:
> > +              - const: shimafuji,kingfisher
> > +              - enum:
> > +                  - renesas,h3ulcb
> > +                  - renesas,m3ulcb
> > +                  - renesas,m3nulcb
> > +              - enum:
> > +                  - renesas,r8a7795
> > +                  - renesas,r8a7796
> > +                  - renesas,r8a77961
> > +                  - renesas,r8a77965
> > +          - items:
> > +              - const: shimafuji,kingfisher
> > +              - enum:
> > +                  - renesas,h3ulcb
> > +                  - renesas,m3ulcb
> > +              - enum:
> > +                  - renesas,r8a779m1
> > +                  - renesas,r8a779m3
> > +              - enum:
> > +                  - renesas,r8a7795
> > +                  - renesas,r8a77961
> >
> >        - description: R-Car M3-N (R8A77965)
> >          items:
> > @@ -296,6 +308,22 @@ properties:
> >            - const: renesas,falcon-cpu
> >            - const: renesas,r8a779a0
> >
> > +      - description: R-Car H3e-2G (R8A779M1)
> > +        items:
> > +          - enum:
> > +              - renesas,h3ulcb      # H3ULCB (R-Car Starter Kit Premier)
> > +              - renesas,salvator-xs # Salvator-XS (Salvator-X 2nd version)
> > +          - const: renesas,r8a779m1
> > +          - const: renesas,r8a7795
> > +
> > +      - description: R-Car M3e-2G (R8A779M3)
> > +        items:
> > +          - enum:
> > +              - renesas,m3ulcb      # M3ULCB (R-Car Starter Kit Pro)
> > +              - renesas,salvator-xs # Salvator-XS (Salvator-X 2nd version)
> > +          - const: renesas,r8a779m3
> > +          - const: renesas,r8a77961
> > +
> >        - description: RZ/N1D (R9A06G032)
> >          items:
> >            - enum:

Gr{oetje,eeting}s,

                        Geert
Laurent Pinchart June 14, 2021, 6:24 p.m. UTC | #3
Hi Geert,

On Mon, Jun 14, 2021 at 01:25:49PM +0200, Geert Uytterhoeven wrote:
> On Sun, Jun 13, 2021 at 3:13 AM Laurent Pinchart wrote:
> > On Thu, Jun 10, 2021 at 11:37:14AM +0200, Geert Uytterhoeven wrote:
> > > Document the compatible values for the R-Car H3e-2G (R8A779M1) and
> > > M3e-2G (R8A779M3) SoCs.  These are different gradings of the R-Car H3
> > > ES3.0 (R8A77951) and M3-W+ (R8A77961) SoCs.
> > >
> > > All R-Car Gen3e on-SoC devices are identical to the devices on the
> > > corresponding R-Car Gen3 SoCs, and thus just use the compatible values
> > > for the latter.  The root compatible properties do gain an additional
> > > value, to sort out integration issues if they ever arise.
> > >
> > > Document the use of these SoCs on the Salvator-XS and ULCB (with and
> > > without Kingfisher) development boards.
> > >
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Thanks!
> 
> > I however wonder if we haven't messed up the board compatible strings
> > somehow (unrelated to this patch). Aren't compatible strings supposed to
> > be ordered from most specific to most generic, with a more specific
> > compatible string being a strict subset of a more generic string ?
> > Looking at, for example,
> >
> >         compatible = "renesas,salvator-xs", "renesas,r8a779m1", "renesas,r8a7795";
> >
> > the rule is upheld by renesas,r8a779m1 being a subset of the more
> > generic renesas,r8a7795, but that's not the case for
> > renesas,salvator-xs.
> 
> That's a very interesting comment.  Originally, we had lists like:
> 
>     compatible = "renesas,koelsch", "renesas,r8a7791";
> 
> with the Koelsch board indeed being a specialization of an R-Car
> M2-W-based system. Later, we reused that system for the Salvator-X
> board with an R-Car H3 SiP:
> 
>     compatible = "renesas,salvator-x", "renesas,r8a7795";
> 
> That scheme became "broken" with the introduction of the R-Car M3-W
> SiP, which was also mounted on a Salvator-X board, leading to:
> 
>     compatible = "renesas,salvator-x", "renesas,r8a7796";
> 
> Note that we did have a similar case for R-Car M2-W and R-Car M2-N on
> the Koelsch resp. Gose boards: from the schematics (I haven't seen
> a Gose), it looks identical to Koelsch, with parts not supported by
> R-Car M2-N (like the second SDRAM bank) marked "Do not stuff".
> But in this case the boards were assigned different names, thus
> leading to different compatible values.
> 
> With Salvator-X(S), it was easier to support multiple SoCs, as they
> are mounted on SiPs, with differences like the different number of
> memory channels hidden in the SiP, and handled at a different level
> (these days memory layout information flows from ATF to U-Boot to
> the DTB passed to the kernel).
> 
> Would you feel more comfortable if we had introduced more
> board-specific compatible values, like "renesas,r8a7796-salvator-x",
> and had used
> 
>     compatible = "renesas,r8a7795-salvator-x", "renesas,salvator-x",
> "renesas,r8a7795";
> 
> or
> 
>     compatible = "renesas,r8a7795-salvator-x", "renesas,r8a7795";
> 
> ?

I think I would prefer that, yes. The idea of specifying separate board
and an SoC identifiers is interesting and, I believe, useful, but it
seems to be a bit of an abuse of what the "compatible" property is meant
for.

All of this will probably never be handled by code not specific to
Renesas, so it's probably a theoretical issue only.

> If the need ever arises, Linux can still identify the exact combination
> by checking for both the board- and the SoC-specific values.
> So far we didn't have that need for Salvator-X(S) yet (we do have
> board-specific checks in
> arch/arm/mach-shmobile/regulator-quirk-rcar-gen2.c).
> 
> > > --- a/Documentation/devicetree/bindings/arm/renesas.yaml
> > > +++ b/Documentation/devicetree/bindings/arm/renesas.yaml
> > > @@ -238,17 +238,29 @@ properties:
> > >            - const: renesas,r8a77961
> > >
> > >        - description: Kingfisher (SBEV-RCAR-KF-M03)
> > > -        items:
> > > -          - const: shimafuji,kingfisher
> > > -          - enum:
> > > -              - renesas,h3ulcb
> > > -              - renesas,m3ulcb
> > > -              - renesas,m3nulcb
> > > -          - enum:
> > > -              - renesas,r8a7795
> > > -              - renesas,r8a7796
> > > -              - renesas,r8a77961
> > > -              - renesas,r8a77965
> > > +        oneOf:
> > > +          - items:
> > > +              - const: shimafuji,kingfisher
> > > +              - enum:
> > > +                  - renesas,h3ulcb
> > > +                  - renesas,m3ulcb
> > > +                  - renesas,m3nulcb
> > > +              - enum:
> > > +                  - renesas,r8a7795
> > > +                  - renesas,r8a7796
> > > +                  - renesas,r8a77961
> > > +                  - renesas,r8a77965
> > > +          - items:
> > > +              - const: shimafuji,kingfisher
> > > +              - enum:
> > > +                  - renesas,h3ulcb
> > > +                  - renesas,m3ulcb
> > > +              - enum:
> > > +                  - renesas,r8a779m1
> > > +                  - renesas,r8a779m3
> > > +              - enum:
> > > +                  - renesas,r8a7795
> > > +                  - renesas,r8a77961
> > >
> > >        - description: R-Car M3-N (R8A77965)
> > >          items:
> > > @@ -296,6 +308,22 @@ properties:
> > >            - const: renesas,falcon-cpu
> > >            - const: renesas,r8a779a0
> > >
> > > +      - description: R-Car H3e-2G (R8A779M1)
> > > +        items:
> > > +          - enum:
> > > +              - renesas,h3ulcb      # H3ULCB (R-Car Starter Kit Premier)
> > > +              - renesas,salvator-xs # Salvator-XS (Salvator-X 2nd version)
> > > +          - const: renesas,r8a779m1
> > > +          - const: renesas,r8a7795
> > > +
> > > +      - description: R-Car M3e-2G (R8A779M3)
> > > +        items:
> > > +          - enum:
> > > +              - renesas,m3ulcb      # M3ULCB (R-Car Starter Kit Pro)
> > > +              - renesas,salvator-xs # Salvator-XS (Salvator-X 2nd version)
> > > +          - const: renesas,r8a779m3
> > > +          - const: renesas,r8a77961
> > > +
> > >        - description: RZ/N1D (R9A06G032)
> > >          items:
> > >            - enum:
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/arm/renesas.yaml b/Documentation/devicetree/bindings/arm/renesas.yaml
index 5fd0696a9f91f383..a01dd064bf16632a 100644
--- a/Documentation/devicetree/bindings/arm/renesas.yaml
+++ b/Documentation/devicetree/bindings/arm/renesas.yaml
@@ -238,17 +238,29 @@  properties:
           - const: renesas,r8a77961
 
       - description: Kingfisher (SBEV-RCAR-KF-M03)
-        items:
-          - const: shimafuji,kingfisher
-          - enum:
-              - renesas,h3ulcb
-              - renesas,m3ulcb
-              - renesas,m3nulcb
-          - enum:
-              - renesas,r8a7795
-              - renesas,r8a7796
-              - renesas,r8a77961
-              - renesas,r8a77965
+        oneOf:
+          - items:
+              - const: shimafuji,kingfisher
+              - enum:
+                  - renesas,h3ulcb
+                  - renesas,m3ulcb
+                  - renesas,m3nulcb
+              - enum:
+                  - renesas,r8a7795
+                  - renesas,r8a7796
+                  - renesas,r8a77961
+                  - renesas,r8a77965
+          - items:
+              - const: shimafuji,kingfisher
+              - enum:
+                  - renesas,h3ulcb
+                  - renesas,m3ulcb
+              - enum:
+                  - renesas,r8a779m1
+                  - renesas,r8a779m3
+              - enum:
+                  - renesas,r8a7795
+                  - renesas,r8a77961
 
       - description: R-Car M3-N (R8A77965)
         items:
@@ -296,6 +308,22 @@  properties:
           - const: renesas,falcon-cpu
           - const: renesas,r8a779a0
 
+      - description: R-Car H3e-2G (R8A779M1)
+        items:
+          - enum:
+              - renesas,h3ulcb      # H3ULCB (R-Car Starter Kit Premier)
+              - renesas,salvator-xs # Salvator-XS (Salvator-X 2nd version)
+          - const: renesas,r8a779m1
+          - const: renesas,r8a7795
+
+      - description: R-Car M3e-2G (R8A779M3)
+        items:
+          - enum:
+              - renesas,m3ulcb      # M3ULCB (R-Car Starter Kit Pro)
+              - renesas,salvator-xs # Salvator-XS (Salvator-X 2nd version)
+          - const: renesas,r8a779m3
+          - const: renesas,r8a77961
+
       - description: RZ/N1D (R9A06G032)
         items:
           - enum: