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

[Biju Das]

Hi Marek,

> -----Original Message-----
> From: Biju Das
> Sent: 21 September 2020 18:30
> To: Marek Vasut <marek.vasut at gmail.com>; Nobuhiro Iwamatsu
> <iwamatsu at nigauri.org>
> Cc: u-boot at lists.denx.de; Chris Paterson <Chris.Paterson2 at renesas.com>;
> Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj at bp.renesas.com>
> Subject: RE: [PATCH 1/2] arm: rmobile: Add RZ/G2M SoC
>
>
> 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 ?
>

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.

Cheers,
Biju

> 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