On 09/08/2016 01:21 PM, Paul Burton wrote: > > > On 08/09/16 11:47, Marek Vasut wrote: >> On 09/08/2016 12:36 PM, Paul Burton wrote: >>> 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, >> >> Hi, >> >>> 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-*. >> >> For machines, we already have board_cpu_init() , for SoCs we have >> arch_cpu_init() . Reading through the patchset, I understand the >> purpose, but then the content of mach_cpu_init() looks like some >> common arch init bit to me. > > Hi Marek, > > The content of the only implementation of mach_cpu_init is some > ath79-specific chip ID stuff. I'm not sure how you think that's "common > arch init"? > > Right now without this patch you're right - arch_cpu_init is used by > SoCs (well, one SoC - ath79). This patch changes that so that > arch_cpu_init is used by the arch code & mach_cpu_init is used by the > SoC/machine code under arch/mips/mach-*. That seems like the most > logical way to handle it to me.
It also introduces discrepancy with ARM and other architectures though. > I get that arch_cpu_init is also used by SoCs on some other > architectures (arm & x86 seemingly), but not on all - there's precedent > for an arch-wide implementation in arc, avr32, blackfin, nios2 or xtensa. Most of which are dead/unmaintained though. Seems like this might need some further cleanup/clarification then. >> >>>>> + */ >>>>> +#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). >> >> Did you try it ? > > I didn't, but the compiler will have to emit the call as it won't be > known whether there's an implementation of the function until link time > so it will obviously have a small cost (unless LTO is used). Having said > that I see the U-Boot wiki seems to indicate that weak is preferred so > I'm ok with changing it. btw if you want to go ahead with adding the mach_cpu_init(), you might want to consider adding it into the board_f.c initcalls , so other arches can convert to it. -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot