From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Date: Thu, 22 Jan 2015 14:29:51 +0100 Subject: [U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S In-Reply-To: <87twzjyevg.fsf@nbsps.com> References: <1421870607-22916-1-git-send-email-hdegoede@redhat.com> <87twzjyevg.fsf@nbsps.com> Message-ID: <54C0FB4F.1070304@redhat.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi, On 21-01-15 22:59, Bill Pringlemeir wrote: > On 21 Jan 2015, hdegoede at redhat.com wrote: > >> On some SoCs / ARMv7 CPU cores we need to do some setup before >> enabling the icache, etc. Add a soc_init hook with a weak default >> which just calls cpu_init_cp15. >> >> This way different implementations can be provided to do some extra >> work before or after cpu_init_cp15, or completely replacing >> cpu_init_cp15. >> >> Signed-off-by: Hans de Goede >> --- >> arch/arm/cpu/armv7/start.S | 18 +++++++++++++++++- >> 1 file changed, 17 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S >> index fdc05b9..9882b20 100644 >> --- a/arch/arm/cpu/armv7/start.S >> +++ b/arch/arm/cpu/armv7/start.S >>>> -64,7 +64,7 @@ reset: >> >> /* the mask ROM code should have PLL and others stable */ >> #ifndef CONFIG_SKIP_LOWLEVEL_INIT >> - bl cpu_init_cp15 >> + bl soc_init >> bl cpu_init_crit >> #endif >> >>>> -102,6 +102,22 @@ ENDPROC(save_boot_params) >> /************************************************************************* >> * >> + * void soc_init(void) >> + * __attribute__((weak)); >> + * >> + * Stack pointer is not yet initialized at this moment >> + * Don't save anything to stack even if compiled with -O0 >> + * >> + *************************************************************************/ >> +ENTRY(soc_init) >> + mov r9, lr >> + bl cpu_init_cp15 >> + mov pc, r9 @ back to my caller >> +ENDPROC(soc_init) >> + .weak soc_init > > You could just use a 'tail call' and make this, > > +ENTRY(soc_init) > + b cpu_init_cp15 > +ENDPROC(soc_init) True, although I think that actually saving lr to r9 is useful as an example for boards who want to override this, and that we can spare the 2 extra instructions :) > Or even as the code follows just add a duplicate label, so 'soc_init' is > a weak version of 'cpu_init_cp15'? This gives no additional code in a > final binary. I guess it depends on how the generic 'soc_init' might be > modified in the future? I think that having a separate entry for it is much more clear, otherwise the entire symbol will be hard to find / grok for people trying to get into the code. Albert what do you want to do here? > If we put code after the 'cpu_init_cp15' in the generic version, then we > should keep saving the 'lr' into 'r9'; also 'cpu_init_cp15' should never > use 'r9'. This maybe good to document (or someone may break your sunxi > code). A valid point, once I've some more feedback I'll do a v2 adding a comment to the top of cpu_init_cp15 that it must not touch r9 as it caller may have that in use. > >> + >> +/************************************************************************* >> + * >> * cpu_init_cp15 >> * >> * Setup CP15 registers (cache, MMU, TLBs). The I-cache is turned on unless Regards, Hans