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@xxxxxxxxx> > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > > 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
