[U-Boot] [PATCH 1/3] ARmv7: Add a soc_init hook to start.S

[Hans de Goede]

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 <hdegoede at redhat.com>
>> ---
>> 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