[U-Boot,04/24] armv8: add lowlevel_init.S

For boards that call s_init() when the SPL runs, we are expected to
setup an early stack before calling this C function.
Implement the proper AArch64 version of this based on the ARMv7 code.
This allows sunxi boards to setup the basic peripherals even on with a
64-bit SPL.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/cpu/armv8/Makefile        |  1 +
 arch/arm/cpu/armv8/lowlevel_init.S | 44 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)
 create mode 100644 arch/arm/cpu/armv8/lowlevel_init.S

On 20/11/2016 15:56, Andre Przywara wrote:
> For boards that call s_init() when the SPL runs, we are expected to
> setup an early stack before calling this C function.
> Implement the proper AArch64 version of this based on the ARMv7 code.
> This allows sunxi boards to setup the basic peripherals even on with a
> 64-bit SPL.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Isn't that what _main in ./arch/arm/lib/crt0_64.S is supposed to do? 
That should be used for the SPL flow too, no?

> ---
>  arch/arm/cpu/armv8/Makefile        |  1 +
>  arch/arm/cpu/armv8/lowlevel_init.S | 44 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 45 insertions(+)
>  create mode 100644 arch/arm/cpu/armv8/lowlevel_init.S
>
> diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile
> index dea1465..799a752 100644
> --- a/arch/arm/cpu/armv8/Makefile
> +++ b/arch/arm/cpu/armv8/Makefile
> @@ -25,3 +25,4 @@ obj-$(CONFIG_FSL_LAYERSCAPE) += fsl-layerscape/
>  obj-$(CONFIG_S32V234) += s32v234/
>  obj-$(CONFIG_ARCH_ZYNQMP) += zynqmp/
>  obj-$(CONFIG_TARGET_HIKEY) += hisilicon/
> +obj-$(CONFIG_ARCH_SUNXI) += lowlevel_init.o
> diff --git a/arch/arm/cpu/armv8/lowlevel_init.S b/arch/arm/cpu/armv8/lowlevel_init.S
> new file mode 100644
> index 0000000..189e35f
> --- /dev/null
> +++ b/arch/arm/cpu/armv8/lowlevel_init.S
> @@ -0,0 +1,44 @@
> +/*
> + * A lowlevel_init function that sets up the stack to call a C function to
> + * perform further init.
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#include <asm-offsets.h>
> +#include <config.h>
> +#include <linux/linkage.h>
> +
> +ENTRY(lowlevel_init)
> +	/*
> +	 * Setup a temporary stack. Global data is not available yet.
> +	 */
> +#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_STACK)
> +	ldr	w0, =CONFIG_SPL_STACK
> +#else
> +	ldr	w0, =CONFIG_SYS_INIT_SP_ADDR
> +#endif
> +	bic	sp, x0, #0xf	/* 16-byte alignment for ABI compliance */
> +
> +	/*
> +	 * Save the old LR(passed in x29) and the current LR to stack
> +	 */
> +	stp	x29, x30, [sp, #-16]!
> +
> +	/*
> +	 * Call the very early init function. This should do only the
> +	 * absolute bare minimum to get started. It should not:
> +	 *
> +	 * - set up DRAM
> +	 * - use global_data
> +	 * - clear BSS
> +	 * - try to start a console
> +	 *
> +	 * For boards with SPL this should be empty since SPL can do all of
> +	 * this init in the SPL board_init_f() function which is called
> +	 * immediately after this.

So this comment says that s_init shouldn't even be used for SPL and 
instead we should use board_init_f which is what crt0 calls.


Alex

Hi Alex,

thanks for having a look!

On 21/11/16 15:34, Alexander Graf wrote:
> 
> 
> On 20/11/2016 15:56, Andre Przywara wrote:
>> For boards that call s_init() when the SPL runs, we are expected to
>> setup an early stack before calling this C function.
>> Implement the proper AArch64 version of this based on the ARMv7 code.
>> This allows sunxi boards to setup the basic peripherals even on with a
>> 64-bit SPL.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> 
> Isn't that what _main in ./arch/arm/lib/crt0_64.S is supposed to do?
> That should be used for the SPL flow too, no?

I saw that too, but apparently this one here is a some shortcut for the
SPL. TBH I didn't dare to look too closely, as this seems to be a bit
entangled.

....

>> ---
>>  arch/arm/cpu/armv8/Makefile        |  1 +
>>  arch/arm/cpu/armv8/lowlevel_init.S | 44
>> ++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 45 insertions(+)
>>  create mode 100644 arch/arm/cpu/armv8/lowlevel_init.S
>>
>> diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile
>> index dea1465..799a752 100644
>> --- a/arch/arm/cpu/armv8/Makefile
>> +++ b/arch/arm/cpu/armv8/Makefile
>> @@ -25,3 +25,4 @@ obj-$(CONFIG_FSL_LAYERSCAPE) += fsl-layerscape/
>>  obj-$(CONFIG_S32V234) += s32v234/
>>  obj-$(CONFIG_ARCH_ZYNQMP) += zynqmp/
>>  obj-$(CONFIG_TARGET_HIKEY) += hisilicon/
>> +obj-$(CONFIG_ARCH_SUNXI) += lowlevel_init.o
>> diff --git a/arch/arm/cpu/armv8/lowlevel_init.S
>> b/arch/arm/cpu/armv8/lowlevel_init.S
>> new file mode 100644
>> index 0000000..189e35f
>> --- /dev/null
>> +++ b/arch/arm/cpu/armv8/lowlevel_init.S
>> @@ -0,0 +1,44 @@
>> +/*
>> + * A lowlevel_init function that sets up the stack to call a C
>> function to
>> + * perform further init.
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#include <asm-offsets.h>
>> +#include <config.h>
>> +#include <linux/linkage.h>
>> +
>> +ENTRY(lowlevel_init)
>> +    /*
>> +     * Setup a temporary stack. Global data is not available yet.
>> +     */
>> +#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_STACK)
>> +    ldr    w0, =CONFIG_SPL_STACK
>> +#else
>> +    ldr    w0, =CONFIG_SYS_INIT_SP_ADDR
>> +#endif
>> +    bic    sp, x0, #0xf    /* 16-byte alignment for ABI compliance */
>> +
>> +    /*
>> +     * Save the old LR(passed in x29) and the current LR to stack
>> +     */
>> +    stp    x29, x30, [sp, #-16]!
>> +
>> +    /*
>> +     * Call the very early init function. This should do only the
>> +     * absolute bare minimum to get started. It should not:
>> +     *
>> +     * - set up DRAM
>> +     * - use global_data
>> +     * - clear BSS
>> +     * - try to start a console
>> +     *
>> +     * For boards with SPL this should be empty since SPL can do all of
>> +     * this init in the SPL board_init_f() function which is called
>> +     * immediately after this.
> 
> So this comment says that s_init shouldn't even be used for SPL and
> instead we should use board_init_f which is what crt0 calls.

Yes, that is a good point. So the sunxi port seemed to ignore this
recommendation and do it in its own way (TM).
Frankly I didn't want to fix this one too at this point, so I just went
ahead with filling the 64-bit gaps.
But I agree that this should be fixed eventually. The line above:
+obj-$(CONFIG_ARCH_SUNXI) += lowlevel_init.o
hints already that something is not really right here, as SUNXI is the
only user of this rather generic function.
If I find some spare time (haha), I might take a look, otherwise: be my
guest ;-)

Cheers,
Andre.

On 21/11/2016 16:49, Andre Przywara wrote:
> Hi Alex,
>
> thanks for having a look!
>
> On 21/11/16 15:34, Alexander Graf wrote:
>>
>>
>> On 20/11/2016 15:56, Andre Przywara wrote:
>>> For boards that call s_init() when the SPL runs, we are expected to
>>> setup an early stack before calling this C function.
>>> Implement the proper AArch64 version of this based on the ARMv7 code.
>>> This allows sunxi boards to setup the basic peripherals even on with a
>>> 64-bit SPL.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>
>> Isn't that what _main in ./arch/arm/lib/crt0_64.S is supposed to do?
>> That should be used for the SPL flow too, no?
>
> I saw that too, but apparently this one here is a some shortcut for the
> SPL. TBH I didn't dare to look too closely, as this seems to be a bit
> entangled.
>
> ....
>
>>> ---
>>>  arch/arm/cpu/armv8/Makefile        |  1 +
>>>  arch/arm/cpu/armv8/lowlevel_init.S | 44
>>> ++++++++++++++++++++++++++++++++++++++
>>>  2 files changed, 45 insertions(+)
>>>  create mode 100644 arch/arm/cpu/armv8/lowlevel_init.S
>>>
>>> diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile
>>> index dea1465..799a752 100644
>>> --- a/arch/arm/cpu/armv8/Makefile
>>> +++ b/arch/arm/cpu/armv8/Makefile
>>> @@ -25,3 +25,4 @@ obj-$(CONFIG_FSL_LAYERSCAPE) += fsl-layerscape/
>>>  obj-$(CONFIG_S32V234) += s32v234/
>>>  obj-$(CONFIG_ARCH_ZYNQMP) += zynqmp/
>>>  obj-$(CONFIG_TARGET_HIKEY) += hisilicon/
>>> +obj-$(CONFIG_ARCH_SUNXI) += lowlevel_init.o
>>> diff --git a/arch/arm/cpu/armv8/lowlevel_init.S
>>> b/arch/arm/cpu/armv8/lowlevel_init.S
>>> new file mode 100644
>>> index 0000000..189e35f
>>> --- /dev/null
>>> +++ b/arch/arm/cpu/armv8/lowlevel_init.S
>>> @@ -0,0 +1,44 @@
>>> +/*
>>> + * A lowlevel_init function that sets up the stack to call a C
>>> function to
>>> + * perform further init.
>>> + *
>>> + * SPDX-License-Identifier:    GPL-2.0+
>>> + */
>>> +
>>> +#include <asm-offsets.h>
>>> +#include <config.h>
>>> +#include <linux/linkage.h>
>>> +
>>> +ENTRY(lowlevel_init)
>>> +    /*
>>> +     * Setup a temporary stack. Global data is not available yet.
>>> +     */
>>> +#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_STACK)
>>> +    ldr    w0, =CONFIG_SPL_STACK
>>> +#else
>>> +    ldr    w0, =CONFIG_SYS_INIT_SP_ADDR
>>> +#endif
>>> +    bic    sp, x0, #0xf    /* 16-byte alignment for ABI compliance */
>>> +
>>> +    /*
>>> +     * Save the old LR(passed in x29) and the current LR to stack
>>> +     */
>>> +    stp    x29, x30, [sp, #-16]!
>>> +
>>> +    /*
>>> +     * Call the very early init function. This should do only the
>>> +     * absolute bare minimum to get started. It should not:
>>> +     *
>>> +     * - set up DRAM
>>> +     * - use global_data
>>> +     * - clear BSS
>>> +     * - try to start a console
>>> +     *
>>> +     * For boards with SPL this should be empty since SPL can do all of
>>> +     * this init in the SPL board_init_f() function which is called
>>> +     * immediately after this.
>>
>> So this comment says that s_init shouldn't even be used for SPL and
>> instead we should use board_init_f which is what crt0 calls.
>
> Yes, that is a good point. So the sunxi port seemed to ignore this
> recommendation and do it in its own way (TM).
> Frankly I didn't want to fix this one too at this point, so I just went
> ahead with filling the 64-bit gaps.
> But I agree that this should be fixed eventually. The line above:
> +obj-$(CONFIG_ARCH_SUNXI) += lowlevel_init.o
> hints already that something is not really right here, as SUNXI is the
> only user of this rather generic function.
> If I find some spare time (haha), I might take a look, otherwise: be my
> guest ;-)

IIRC in my SPL port (that I can't find anymore) I just called s_init 
from board_init_f.


Alex