[PATCH 2/2] arm: rmobile: Add HopeRun HiHope RZ/G2M board support

[Biju Das]

Hi Marek,

Thanks for the feedback.

> Subject: Re: [PATCH 2/2] arm: rmobile: Add HopeRun HiHope RZ/G2M board
> support
>
> On 9/18/20 6:03 PM, Biju Das wrote:
>
> [...]
>
> > +++ b/board/hoperun/hihope-rzg2/hihope-rzg2.c
> > @@ -0,0 +1,80 @@
> [...]
> > +#define RST_BASE0xE6160000
> > +#define RST_CA57RESCNT(RST_BASE + 0x40)
> > +#define RST_CODE0xA5A5000F
> > +
> > +/* If the firmware passed a device tree use it for U-Boot DRAM setup.
> > +*/ extern u64 rcar_atf_boot_args[];
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +void s_init(void)
> > +{
> > +}
>
> Is this empty function needed ?

Yes Compilation error otherwise. This function is called from lowlevel_init_gen3.S. I believe it is place holder for doing some early initialisation such as watchdog
That could be the reason  all rcar gen3 boards have this empty function for eg:-[1], please correct me if you think otherwise.
[1] board/renesas/salvator-x/Salvator-x.c

> > +#ifdef CONFIG_BOARD_EARLY_INIT_F
> > +/* Kconfig forces this on, so just return 0 */
>
> I think BOARD_EARLY_INIT_F should really be disabled in Kconfig rather than
> implementing empty function here.
>

Ok, will fix in v2.

For eg:- file arch/arm/Kconfig
 Select BOARD_EARLY_INIT_FUNCTION if !(RZA1 || TARGET_HIHOPE_RZG2)


I also noticed other boards in board/renesas directory with empty function(for eg:- ebisu,condor etc).
For completeness, do you want me to fix that as well in  separate patch and removing empty functions.
Select BOARD_EARLY_INIT_FUNCTION if !(RZA1 || TARGET_HIHOPE_RZG2|| TARGET_EBISU || TARGET_CONDOR)

> > +int board_early_init_f(void)
> > +{
> > +return 0;
> > +}
> > +#endif
>
>
> [...]
>
> > +int fdtdec_board_setup(const void *fdt_blob) {
> > +void *atf_fdt_blob = (void *)(rcar_atf_boot_args[1]);
> > +
> > +if (fdt_magic(atf_fdt_blob) == FDT_MAGIC)
> > +fdt_overlay_apply_node((void *)fdt_blob, 0, atf_fdt_blob,
> 0);
> > +
> > +return 0;
> > +}
>
> Please use board/renesas/rcar-common/common.c , no need to duplicate
> the code.

OK will fix in V3.

> > +int dram_init(void)
> > +{
> > +return fdtdec_setup_mem_size_base(); }
> > +
> > +int dram_init_banksize(void)
> > +{
> > +return fdtdec_setup_memory_banksize(); }
> > +
> > +void reset_cpu(ulong addr)
> > +{
> > +writel(RST_CODE, RST_CA57RESCNT);
> > +}
>
> Isn't there CA53 core in the RZG2 too ?

Yes, big little CPU 2xCA57 + 4 xCA53. Do you want me to add reset code for in case of CA53 boot mode???

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