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.przyw...@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. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
