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

[Biju Das]

Hi Marek,

> > > 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 ?
> >
>
> Thinking further, we can define CPU_TYPE_RZG_MASK as 0x1000. The value
> CPU_TYPE_RZG_MASK | SOC_PR Is added into tfa_cpu_info table.
>
> We just match  PRR of the SoC  and compatible string from TFA with smaller
> array table. If match found, we set cputype= CPU_TYPE_RZG_MASK |
> SOC_PR.
> Then the remaining logic in the code works as usual and changes are minimal.
>
> I prefer this match function to be vendor specific, iterating cputype in smaller
> array(tfa_cpu_info ) is faster than iterating compatible string from device
> tree.
>
> Please share your views on this.

The new diffs looks like this. I will send V3.

As you suggested, I have used match function which returns RZG2_CPU_ID.

+#if CONFIG_IS_ENABLED(OF_CONTROL)
+static const struct udevice_id tfa_soc_ids[] = {
+{ .compatible = "renesas,r8a774a1", .data = RMOBILE_CPU_TYPE_R8A774A1 },
+{ },
+};
+
+static const u16 get_rzg_soc_cpu_type(void)
+{
+const struct udevice_id *of_match = tfa_soc_ids;
+u32 cpu_type = rmobile_get_cpu_type();
+int i;
+
+for (i = 0; i < ARRAY_SIZE(tfa_soc_ids); i++) {
+if (cpu_type == of_match->data &&
+    of_machine_is_compatible(of_match->compatible))
+return (cpu_type | RZG_CPU_MASK);
+of_match++;
+}
+
+return 0;
+}
+#else
+static const const u16 __get_rzg_soc_cpu_type(void)
+{
+return 0;
+}
+
+static const const u16 get_rzg_soc_cpu_type(void)
+__attribute__((weak, alias("__get_rzg_soc_cpu_type")));
+#endif
+
 /* CPU infomation table */
 static const struct {
 u16 cpu_type;
@@ -59,6 +91,7 @@ static const struct {
 } rmobile_cpuinfo[] = {
 { RMOBILE_CPU_TYPE_SH73A0, "SH73A0" },
 { RMOBILE_CPU_TYPE_R8A7740, "R8A7740" },
+{ RMOBILE_CPU_TYPE_R8A774A1 | RZG_CPU_MASK, "R8A774A1" },
 { RMOBILE_CPU_TYPE_R8A7790, "R8A7790" },
 { RMOBILE_CPU_TYPE_R8A7791, "R8A7791" },
 { RMOBILE_CPU_TYPE_R8A7792, "R8A7792" },
@@ -77,7 +110,8 @@ static const struct {
 static int rmobile_cpuinfo_idx(void)
 {
 int i = 0;
-u32 cpu_type = rmobile_get_cpu_type();
+u16 rzg2_cpu_type = get_rzg_soc_cpu_type();
+u32 cpu_type = rzg2_cpu_type ? rzg2_cpu_type : rmobile_get_cpu_type();


>
> > 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.

Please look at the code above, looks like we don't need generic function. Since we need to compare both CPUID and Compatible string
Iterating through CPU ID is much faster than iterating through compatible string.

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