Hi Marek,
Thanks for the feedback.
Subject: Re: [PATCH 2/2] arm: rmobile: Add HopeRun HiHope RZ/G2M board support
On 9/19/20 2:18 PM, Biju Das wrote:
Hi,
[...]
+++ 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
Can you try fixing that with WEAK(s_init) in the lowlevel_init_gen3.S ? I think that would be much better, if anyone needs to override the function, then they can, otherwise empty WEAK function would be used.
OK. Will add weak function in lowlevel_init_gen3.S.
+#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)
Maybe it would be better to use imply BOARD_EARLY_INIT_F , and then disable it on boards which don't need it (RZA1 and RZG2)
OK Will fix this in V3.
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)
Look at the 'imply' keyword, that might be even better.
Will send separate patch for this.
[...]
+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.
Thanks
+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???
I think if you can start U-Boot on either core, then the reset function should handle both, yes.
OK. Will fix this in V3.
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