Hi Jonas, On Mon, 17 Feb 2025 at 14:17, Jonas Karlman <jo...@kwiboo.se> wrote: > > Hi Simon, > > On 2025-02-09 22:14, Simon Glass wrote: > > Now that we have the same option for SPL and VPL, simplify the logic for > > I suspect you mean TPL here?
Yes > > > determining the initial stack. > > > > Signed-off-by: Simon Glass <s...@chromium.org> > > Suggested-by: Tom Rini <tr...@konsulko.com> > > --- > > > > Changes in v2: > > - Add new patch to use CONFIG_VAL() to obtain the SPL stack > > > > arch/arm/cpu/armv7/lowlevel_init.S | 4 ++-- > > arch/arm/cpu/armv7/start.S | 4 ++-- > > arch/arm/lib/crt0.S | 6 ++---- > > arch/arm/lib/crt0_64.S | 6 ++---- > > arch/riscv/cpu/start.S | 4 ++-- > > 5 files changed, 10 insertions(+), 14 deletions(-) > > > > diff --git a/arch/arm/cpu/armv7/lowlevel_init.S > > b/arch/arm/cpu/armv7/lowlevel_init.S > > index c2a0fe061a3..72b7b7d082c 100644 > > --- a/arch/arm/cpu/armv7/lowlevel_init.S > > +++ b/arch/arm/cpu/armv7/lowlevel_init.S > > @@ -26,8 +26,8 @@ WEAK(lowlevel_init) > > /* > > * Setup a temporary stack. Global data is not available yet. > > */ > > -#if defined(CONFIG_XPL_BUILD) && defined(CONFIG_SPL_HAVE_INIT_STACK) > > - ldr sp, =CONFIG_SPL_STACK > > +#if CONFIG_IS_ENABLED(HAVE_INIT_STACK) > > + ldr sp, =CONFIG_VAL(STACK) > > #else > > ldr sp, =SYS_INIT_SP_ADDR > > #endif > > diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S > > index 3394db46953..959251957de 100644 > > --- a/arch/arm/cpu/armv7/start.S > > +++ b/arch/arm/cpu/armv7/start.S > > @@ -279,8 +279,8 @@ ENTRY(cpu_init_cp15) > > orr r2, r4, r2 @ r2 has combined CPU variant + > > revision > > > > /* Early stack for ERRATA that needs into call C code */ > > -#if defined(CONFIG_XPL_BUILD) && defined(CONFIG_SPL_HAVE_INIT_STACK) > > - ldr r0, =(CONFIG_SPL_STACK) > > +#if CONFIG_IS_ENABLED(HAVE_INIT_STACK) > > + ldr r0, =CONFIG_VAL(STACK) > > #else > > ldr r0, =(SYS_INIT_SP_ADDR) > > #endif > > diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S > > index 007e89407a8..8ef18ce17e4 100644 > > --- a/arch/arm/lib/crt0.S > > +++ b/arch/arm/lib/crt0.S > > @@ -100,10 +100,8 @@ ENTRY(_main) > > * Set up initial C runtime environment and call board_init_f(0). > > */ > > > > -#if defined(CONFIG_TPL_BUILD) && defined(CONFIG_TPL_HAVE_INIT_STACK) > > - ldr r0, =(CONFIG_TPL_STACK) > > -#elif defined(CONFIG_XPL_BUILD) && defined(CONFIG_SPL_HAVE_INIT_STACK) > > - ldr r0, =(CONFIG_SPL_STACK) > > +#if CONFIG_IS_ENABLED(HAVE_INIT_STACK) > > + ldr r0, =CONFIG_VAL(STACK) > > This changes behavior, prior to this the the SPL_STACK would be fallback > for TPL. Now this will only fallback to SYS_INIT_SP_ADDR. Is this > intentional and/or will this affect any board? I didn't see that, actually. But I think it is a bug, so yes my change is intentional. I'll update the commit message. > > > #else > > ldr r0, =(SYS_INIT_SP_ADDR) > > #endif > > diff --git a/arch/arm/lib/crt0_64.S b/arch/arm/lib/crt0_64.S > > index 1b6782b06ed..30950ddaf9b 100644 > > --- a/arch/arm/lib/crt0_64.S > > +++ b/arch/arm/lib/crt0_64.S > > @@ -69,10 +69,8 @@ ENTRY(_main) > > /* > > * Set up initial C runtime environment and call board_init_f(0). > > */ > > -#if defined(CONFIG_TPL_BUILD) && defined(CONFIG_TPL_HAVE_INIT_STACK) > > - ldr x0, =(CONFIG_TPL_STACK) > > -#elif defined(CONFIG_XPL_BUILD) && defined(CONFIG_SPL_HAVE_INIT_STACK) > > - ldr x0, =(CONFIG_SPL_STACK) > > +#if CONFIG_IS_ENABLED(HAVE_INIT_STACK) > > + ldr x0, =CONFIG_VAL(STACK) > > Same here, this changes behavior and should be mentioned in the commit > message. > > Also this and two preceding patches deserve their own series, especially > since this changes behavior. OK > > Regards, > Jonas > > > #elif defined(CONFIG_INIT_SP_RELATIVE) > > #if CONFIG_POSITION_INDEPENDENT > > adrp x0, __bss_start /* x0 <- Runtime &__bss_start */ > > diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S > > index c8526dd82fd..7bafdfd390a 100644 > > --- a/arch/riscv/cpu/start.S > > +++ b/arch/riscv/cpu/start.S > > @@ -90,8 +90,8 @@ _start: > > * Set stackpointer in internal/ex RAM to call board_init_f > > */ > > call_board_init_f: > > -#if defined(CONFIG_XPL_BUILD) && defined(CONFIG_SPL_HAVE_INIT_STACK) > > - li t0, CONFIG_SPL_STACK > > +#if CONFIG_IS_ENABLED(HAVE_INIT_STACK) > > + li t0, CONFIG_VAL(STACK) > > #else > > li t0, SYS_INIT_SP_ADDR > > #endif > Regards, Simon