[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/19/20 8:35 PM, Biju Das wrote:
>
> Hi,
>
> [...]
>
> >>>>> +static const struct udevice_id *of_soc_match_compatible(void) {
> >>>>> +const struct udevice_id *of_match = soc_ids; int i;
> >>>>> +
> >>>>> +for (i = 0; i < ARRAY_SIZE(soc_ids); i++) { if
> >>>>> +(!fdt_node_check_compatible(gd->fdt_blob, 0,
> >>>>> +       of_match->compatible))
> >>>>> +return of_match;
> >>>>> +of_match++;
> >>>>> +}
> >>>>> +
> >>>>> +return NULL;
> >>>>> +}
> >>>>
> >>>> This should rather be a generic function, I think this is something
> >>>> that already exists in Linux common code too, right ?
> >>>
> >>> No.  I have seen some other SoC's uses similar logic [1]& [2] .
> >>
> >> I mean, this looks like Linux's soc_device_match() , so such a
> >> function is likely generic code, there is nothing platform specific to it, is
> there ?
> >
> > I agree, we need to have a new generic api for such purpose. The Linux/U-
> boot soc_device_match  is for adding quirks with in different ES version of
> same SoC.
> >
> > What we here need is similar to of_match_compatible for  array of
> different SoC's.
> > Can you please confirm  [1] drivers/soc/soc-uclass.c is the right place for
> such generic api?
>
> Can you use of_machine_is_compatible() ?
Yes, will use that one.
> [...]
>
> >>>  So can you please check whether there might
> >>>> be some way to tell the two SoCs apart ?
> >>>
> >>> At present there is no way other than matching the SoC compatible
> string.
> >>
> >> Thinking about it a bit more, if you were to use the compatible
> >> string psssed from TFA in the / node, you could iterate over
> >> soc_ids[] array and return RMOBILE_CPU_TYPE_x , which could be stored
> there as .data .
> >> Then you won't even need the SOC_RZG2 and it would all be faster, as
> >> all you would need is a single pass over a smaller array.
> >
> > Good point. Ok will get rid of SOC_RZG2,  will use smaller array forRZG2.
> >
> > Are you suggesting to modify "arch_misc_init" directly set "platform"
> > environment variable using match logic, which use a smaller array
> Compared to rmobile_cpuinfo.
> >
> > Basically we match the compatible string from TFA, .data from "
> RMOBILE_CPU_TYPE_x" matched against PRR values and set the platform
> type .
>
> 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;
+}

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