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

21 Sep 2020


      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

    