| Message ID | f79841c1881f8b9a2c10fadb3d3ad6cb5fccc6a5.1623315732.git.geert+renesas@glider.be |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm64: renesas: Add support for R Car H3e 2G-and M3e-2G | expand |
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: -- Regards, Laurent Pinchart
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 -- 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
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: -- Regards, Laurent Pinchart
On Thu, 10 Jun 2021 11:37:14 +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> > --- > .../devicetree/bindings/arm/renesas.yaml | 50 +++++++++++++++---- > 1 file changed, 39 insertions(+), 11 deletions(-) > Acked-by: Rob Herring <robh@kernel.org>
Hi Geert-san, > From: Geert Uytterhoeven, Sent: Thursday, June 10, 2021 6:37 PM > > 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> Thank you for the patch! Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Best regards, Yoshihiro Shimoda
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:
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(-)