Re: [PATCH] arm64: dts: renesas: r8a77980: Add V3H2 Condor-I boards

[Geert Uytterhoeven <geert@linux-m68k.org>]

Hi Morimoto-san,

On Mon, Aug 22, 2022 at 4:34 AM Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> This patch adds r8a77980A V3H2 (= r8a77980 ES2) Condor-I board basic
> support. It is using r8a77980 Condor board setting as-is for now,
> but should care ES2.0 specific feature in the future.
>
> Signed-off-by: Andrey Dolnikov <andrey.dolnikov@cogentembedded.com>
> Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
> Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@renesas.com>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Thanks for your patch!

> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/r8a77980-es2-condor.dts

I think this should be "r8a77980a-condor-i.dts"

> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Device Tree Source for the Condor-I board on r8a77980A (ES2.0)
> + *
> + * Copyright (C) 2018 Renesas Electronics Corp.
> + * Copyright (C) 2018 Cogent Embedded, Inc.
> + */
> +
> +#include "r8a77980-condor.dts"

R-Car V3H2 is known to differ slightly from R-Car V3H.
Hence I suggest adding "r8a77980a.dtsi", which includes
"r8a77980.dtsi", and adds/removes/overrides what is different.

I have no idea what are the differences between the Condor and Condor-I
boards, but I suggest to move the base or common parts to "condor.dtsi"
or "condor-common.dtsi", like we did before for the Draak/Ebisu and
Salvator-X(S) boards.

Then "r8a77980a-condor-i.dts" can include both "r8a77980a.dtsi" and
"condor.dtsi"/"condor-common.dtsi", and add the pieces that are
different.

> +
> +/ {
> +       model = "Renesas Condor-I board based on r8a77980A (ES2.0)";

I think this needs to override the compatible value, too:
  1. Based on previous discussions about how to handle R-Car V3H2,
     "renesas,r8a77980a" should be added to the top-level compatible
     value, so we can distinguish between R-Car V3H and V3H2.
     All other compatible values can probably stay the same
    (cfr. R-Car Gen3e),
  2. As the board is different, "renesas,condor-i" should be added,
  3. Depending on the differences between Condor and Condor-I, the
     "reneas,condor" value should be dropped (or not).

So I expect the override to become:

    compatible = "renesas,condor-i", "renesas,condor",
"renesas,r8a77980a", "renesas,r8a77980".

or

    compatible = "renesas,condor-i", "renesas,r8a77980a", "renesas,r8a77980".

Obviously, any new compatible values must be documented.

> +};

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