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
