Message ID | 1479653838-3574-5-git-send-email-andre.przywara@arm.com |
---|---|
State | Superseded |
Delegated to: | Jagannadha Sutradharudu Teki |
Headers | show |
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
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. + */ + bl s_init + ldp x29, x30, [sp] + ret +ENDPROC(lowlevel_init)
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