On 08/09/16 11:02, Marek Vasut wrote: > On 09/07/2016 07:48 PM, Paul Burton wrote: >> Add an implementation of arch_cpu_init that covers all MIPS boards, in >> preparation for performing various pieces of initialisation there in >> later patches. >> >> In order to allow the ath79 code to continue performing initialisation >> at this point in the boot, it's converted to use a new mach_cpu_init >> function which is called from arch_cpu_init. >> >> Signed-off-by: Paul Burton <paul.bur...@imgtec.com> > > [...] > > Hi! > >> diff --git a/arch/mips/include/asm/u-boot-mips.h >> b/arch/mips/include/asm/u-boot-mips.h >> index 1f527bb..0eaf32b 100644 >> --- a/arch/mips/include/asm/u-boot-mips.h >> +++ b/arch/mips/include/asm/u-boot-mips.h >> @@ -5,4 +5,16 @@ >> #ifndef _U_BOOT_MIPS_H_ >> #define _U_BOOT_MIPS_H_ >> >> +/** >> + * mach_cpu_init() - Perform SoC/machine-specific CPU initialisation >> + * >> + * This is called from arch_cpu_init() to allow for SoC/machine-level code >> to >> + * perform any CPU initialisation it may require. > > Just call this function from arch_cpu_init() in various places instead > of replacing arch_cpu_init() with it all over the place. Also rename it > to arch_cpu_init_common() to make it more obvious what this does .
Hi Marek, That's "backwards" compared with what this code actually does - arch_cpu_init becomes the common function (ie. is arch-wide as in the patch subject) and mach_cpu_init is for use by machines/SoCs - ie. code under arch/mips/mach-*. >> + */ >> +#ifdef CONFIG_MACH_CPU_INIT >> +void mach_cpu_init(void); >> +#else >> +static inline void mach_cpu_init(void) {} > > Implement this as __weak int and you can nuke the ifdefery . I could make it weak, but then we don't let the compiler optimise it out entirely for builds that don't need it (ie. everything except ath79). I'd say that since the #ifdefery here is confined to this one case in the header (which is a pretty common approach) it's ugliness is kept minimal & its cost on binaries & runtime is as low as it can be at zero. Daniel - do you have a preference? > >> +#endif >> + >> #endif /* _U_BOOT_MIPS_H_ */ >> diff --git a/arch/mips/mach-ath79/cpu.c b/arch/mips/mach-ath79/cpu.c >> index 5756a06..a556b73 100644 >> --- a/arch/mips/mach-ath79/cpu.c >> +++ b/arch/mips/mach-ath79/cpu.c >> @@ -46,7 +46,7 @@ static const struct ath79_soc_desc desc[] = { >> {ATH79_SOC_QCA9561, "9561", REV_ID_MAJOR_QCA9561, 0}, >> }; >> >> -int arch_cpu_init(void) >> +void mach_cpu_init(void) > > See above. > >> { >> void __iomem *base; >> enum ath79_soc_type soc = ATH79_SOC_UNKNOWN; >> @@ -98,7 +98,6 @@ int arch_cpu_init(void) >> gd->arch.soc = soc; >> gd->arch.rev = rev; >> gd->arch.ver = ver; >> - return 0; > > That's a nope, keep the return value. The implementation always returns zero, so I see no point. Thanks, Paul > >> } >> >> int print_cpuinfo(void) >> > > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot