[PATCH 1/2] arm: rmobile: Add RZ/G2M SoC

[Biju Das]

Hi Marek,

Thanks for the feedback.

> Subject: Re: [PATCH 1/2] arm: rmobile: Add RZ/G2M SoC
>
> On 9/21/20 12:30 PM, Biju Das wrote:
>
> [...]
>
> >> I don't think you need to modify anything then, the DT passed from
> >> TFA would contain the correct compatible string in / node, and that
> >> gets merged into the U-Boot control DT early on in fdtdec_board_setup()
> in:
> >> board/renesas/rcar-common/common.c
> >> so all you would have to do is use
> >> of_machine_is_compatible("renesas,r8a7-something-");
> >>
> >> Would that work ?
> >
> >
> > Yes, I have added the below function to get cpu name from small array
> tfa_cpu_table. Will send V3 with this changes.
> >
> > +static const u8* get_cpu_name(void)
> > +{
> > +u32 cpu_type = rmobile_get_cpu_type();
> > +
> > +return is_tfa_soc_match(cpu_type) ?
> > +tfa_cpuinfo[tfa_cpuinfo_idx(cpu_type)].cpu_name :
> > +rmobile_cpuinfo[rmobile_cpuinfo_idx(cpu_type)].cpu_name;
> > +}
> >
> > +static int tfa_cpuinfo_idx(u32 cpu_type) { int i = 0;
> > +
> > +for (; i < ARRAY_SIZE(tfa_cpuinfo); i++) if (tfa_cpuinfo[i].cpu_type
> > +== cpu_type) break;
> > +
> > +return i;
> > +}
> > +
> > +static bool is_tfa_soc_match(u32 cpu_type) { int idx =
> > +tfa_cpuinfo_idx(cpu_type);
> > +
> > +if (idx != ARRAY_SIZE(tfa_cpuinfo) &&
> > +    of_machine_is_compatible(tfa_cpuinfo[idx].compatible))
> > +return true;
> > +
> > +return false;
> > +}
>
> There might be even better way. Look at rmobile_get_cpu_type() , that is a
> weak function. So if you can implement one for RZG2 , then that function can
> return CPU_TYPE_RZG2_something ; and rmobile_get_cpu_type() for RZG2
> can be implemented using the match on /compatible string .
>
> Take a look at how arch/arm/mach-rmobile/cpu_info-rcar.c implements it
> using PRR, you might need cpu_info-rzg.c I think.

As mentioned in the commit message PRR values of both R-Car M3-W and RZ/G2M are identical. So there is no need for separate cpu_info-rzg.c.
I believe it is duplication of code.

We are matching PRR first (device binding)  and then use TFA SoC compatible string to differentiate from R-Car family.
Please see the diff below[3].

> Also, I hope there should already be some function to which you provide a
> compatible string and a table of supported compatible strings (of match
> table), from which it will return the .data field of the matching entry in that
> table. And that .data field can already be the CPU_TYPE_RZG_something , so
> you don't have to implement the table look up again.
>
> What do you think ?

Device binding is important use case, run time you need to match PRR, that is same for both RCar-M3W and RZ/G2E.
In RZ/G2 case, we miss device binding if just go with TFA compatible Approach. So we need both.

What do you think?

[3]
+static const struct {
+char *compatible;
+u16 cpu_type;
+u8 cpu_name[10];
+} tfa_cpuinfo[] = {
+{ "renesas,r8a774a1", RMOBILE_CPU_TYPE_R8A774A1, "R8A774A1" },
+{ },
+};
+
+static int tfa_cpuinfo_idx(u32 cpu_type)
+{
+int i = 0;
+
+for (; i < ARRAY_SIZE(tfa_cpuinfo); i++)
+if (tfa_cpuinfo[i].cpu_type == cpu_type)
+break;
+
+return i;
+}
+
+#if CONFIG_IS_ENABLED(OF_CONTROL)
+static bool is_tfa_soc_match(u32 cpu_type)
+{
+int idx = tfa_cpuinfo_idx(cpu_type);
+
+if (idx != ARRAY_SIZE(tfa_cpuinfo) &&
+    of_machine_is_compatible(tfa_cpuinfo[idx].compatible))
+return true;
+
+return false;
+}
+#else
+static bool __is_tfa_soc_match(u32 cpu_type)
+{
+return false;
+}
+bool is_tfa_soc_match(u32 cpu_type)
+__attribute__((weak, alias("__is_tfa_soc_match")));
+#endif
+
 /* CPU infomation table */
 static const struct {
 u16 cpu_type;
@@ -74,10 +115,9 @@ static const struct {
 { 0x0, "CPU" },
 };

-static int rmobile_cpuinfo_idx(void)
+static int rmobile_cpuinfo_idx(u32 cpu_type)
 {
 int i = 0;
-u32 cpu_type = rmobile_get_cpu_type();

 for (; i < ARRAY_SIZE(rmobile_cpuinfo); i++)
 if (rmobile_cpuinfo[i].cpu_type == cpu_type)
@@ -86,14 +126,24 @@ static int rmobile_cpuinfo_idx(void)
 return i;
 }

+static const u8 *get_cpu_name(void)
+{
+u32 cpu_type = rmobile_get_cpu_type();
+
+return is_tfa_soc_match(cpu_type) ?
+tfa_cpuinfo[tfa_cpuinfo_idx(cpu_type)].cpu_name :
+rmobile_cpuinfo[rmobile_cpuinfo_idx(cpu_type)].cpu_name;
+}
+
 #ifdef CONFIG_ARCH_MISC_INIT
 int arch_misc_init(void)
 {
-int i, idx = rmobile_cpuinfo_idx();
+const u8 *cpu_name = get_cpu_name();
 char cpu[10] = { 0 };
+int i;

 for (i = 0; i < sizeof(cpu); i++)
-cpu[i] = tolower(rmobile_cpuinfo[idx].cpu_name[i]);
+cpu[i] = tolower(cpu_name[i]);

 env_set("platform", cpu);

@@ -103,10 +153,8 @@ int arch_misc_init(void)

 int print_cpuinfo(void)
 {
-int i = rmobile_cpuinfo_idx();
-
 printf("CPU: Renesas Electronics %s rev %d.%d\n",
-rmobile_cpuinfo[i].cpu_name, rmobile_get_cpu_rev_integer(),
+get_cpu_name(), rmobile_get_cpu_rev_integer(),
 rmobile_get_cpu_rev_fraction());

Cheers,
Biju


Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647