[PATCH v2 01/35] tpl: Rename TPL_NEEDS_SEPARATE_STACK to TPL_HAVE_INIT_STACK
Simon Glass
sjg at chromium.org
Thu Feb 27 17:26:35 CET 2025
Hi Jonas,
On Mon, 17 Feb 2025 at 13:28, Jonas Karlman <jonas at kwiboo.se> wrote:
>
> Hi Simon,
>
> On 2025-02-09 22:14, Simon Glass wrote:
> > The most common word for features that make a platform work is to use
> > 'HAVE_xxx'. Rename this option to match.
>
> We already have HAS_CUSTOM_SYS_INIT_SP_ADDR related to the init stack
> pointer address, maybe the new symbol should be named something similar
> to that? e.g. HAS_xPL_INIT_SP_ADDR
That would be quite confusing, since we still need SYS_INIT_SP_ADDR
that option to work.
The name would have to be xPL_HAS_SP_INIT_SP_ADDR to fit with the
naming convention and allow CONFIG_IS_ENABLED() and avoid more
confusion. Plus, SYS_INIT_SP_ADDR is actually a fallback for when the
xPL builds don't provide their own address.
So I think this needs to be separate, particularly as Tom is talking
of splitting the deconfig files, at which point we wouldn't be able to
make xPL use SYS_INIT_SP_ADDR as a default.
>
> >
> > Update the help to use the word 'phase' rather than 'stage', since
> > that is the current terminology. Also clarify that, absent this setting,
> > the stack pointer generally comes from the value used by U-Boot proper,
> > rather than SPL.
>
> The last statement is not fully correct, see below.
>
> >
> > Move the option just above TPL_STACK which depends on it.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> > Changes in v2:
> > - Add new patch to rename TPL_NEEDS_SEPARATE_STACK
> >
> > arch/arm/lib/crt0.S | 2 +-
> > arch/arm/lib/crt0_64.S | 2 +-
> > arch/arm/mach-rockchip/Kconfig | 14 +++++++-------
> > common/spl/Kconfig.tpl | 19 ++++++++++---------
> > configs/mk808_defconfig | 2 +-
> > 5 files changed, 20 insertions(+), 19 deletions(-)
> >
> > diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
> > index a691d844847..a7e6f50d39a 100644
> > --- a/arch/arm/lib/crt0.S
> > +++ b/arch/arm/lib/crt0.S
> > @@ -100,7 +100,7 @@ ENTRY(_main)
> > * Set up initial C runtime environment and call board_init_f(0).
> > */
> >
> > -#if defined(CONFIG_TPL_BUILD) && defined(CONFIG_TPL_NEEDS_SEPARATE_STACK)
> > +#if defined(CONFIG_TPL_BUILD) && defined(CONFIG_TPL_HAVE_INIT_STACK)
> > ldr r0, =(CONFIG_TPL_STACK)
> > #elif defined(CONFIG_XPL_BUILD) && defined(CONFIG_SPL_STACK)
> > ldr r0, =(CONFIG_SPL_STACK)
>
> Here we can see that SPL_STACK is used as the first fallback for
> XPL_BUILD's.
Oh dear. I believe that is unintended, but I'm not sure which way to
take it. For now I'll update the comment.
>
> > diff --git a/arch/arm/lib/crt0_64.S b/arch/arm/lib/crt0_64.S
> > index 32401f544a7..62a0abe1fed 100644
> > --- a/arch/arm/lib/crt0_64.S
> > +++ b/arch/arm/lib/crt0_64.S
> > @@ -69,7 +69,7 @@ ENTRY(_main)
> > /*
> > * Set up initial C runtime environment and call board_init_f(0).
> > */
> > -#if defined(CONFIG_TPL_BUILD) && defined(CONFIG_TPL_NEEDS_SEPARATE_STACK)
> > +#if defined(CONFIG_TPL_BUILD) && defined(CONFIG_TPL_HAVE_INIT_STACK)
> > ldr x0, =(CONFIG_TPL_STACK)
> > #elif defined(CONFIG_XPL_BUILD) && defined(CONFIG_SPL_STACK)
> > ldr x0, =(CONFIG_SPL_STACK)
> > diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
> > index 4d3157b2edd..4c515593718 100644
> > --- a/arch/arm/mach-rockchip/Kconfig
> > +++ b/arch/arm/mach-rockchip/Kconfig
> > @@ -9,7 +9,7 @@ config ROCKCHIP_PX30
> > select SPL
> > select TPL
> > select TPL_TINY_FRAMEWORK if TPL
> > - select TPL_NEEDS_SEPARATE_STACK if TPL
> > + select TPL_HAVE_INIT_STACK if TPL
> > imply SPL_SEPARATE_BSS
> > select SPL_SERIAL
> > select TPL_SERIAL
> > @@ -106,7 +106,7 @@ config ROCKCHIP_RK322X
> > select TPL
> > select TPL_DM
> > select TPL_OF_LIBFDT
> > - select TPL_NEEDS_SEPARATE_STACK if TPL
> > + select TPL_HAVE_INIT_STACK if TPL
> > select SPL_DRIVERS_MISC
> > imply ROCKCHIP_COMMON_BOARD
> > imply SPL_SERIAL
> > @@ -139,7 +139,7 @@ config ROCKCHIP_RK3288
> > imply TPL_DRIVERS_MISC
> > imply TPL_LIBCOMMON_SUPPORT
> > imply TPL_LIBGENERIC_SUPPORT
> > - imply TPL_NEEDS_SEPARATE_STACK
> > + imply TPL_HAVE_INIT_STACK
> > imply TPL_OF_CONTROL
> > imply TPL_OF_PLATDATA
> > imply TPL_RAM
> > @@ -197,7 +197,7 @@ config ROCKCHIP_RK3328
> > select SPL
> > select SUPPORT_TPL
> > select TPL
> > - select TPL_NEEDS_SEPARATE_STACK if TPL
> > + select TPL_HAVE_INIT_STACK if TPL
> > imply ARMV8_CRYPTO
> > imply ARMV8_SET_SMPEN
> > imply MISC
> > @@ -225,7 +225,7 @@ config ROCKCHIP_RK3368
> > select ARM64
> > select SUPPORT_SPL
> > select SUPPORT_TPL
> > - select TPL_NEEDS_SEPARATE_STACK if TPL
> > + select TPL_HAVE_INIT_STACK if TPL
> > imply ROCKCHIP_COMMON_BOARD
> > imply SPL_ROCKCHIP_COMMON_BOARD
> > imply SPL_SEPARATE_BSS
> > @@ -257,7 +257,7 @@ config ROCKCHIP_RK3399
> > select SPL_RAM if SPL
> > select SPL_REGMAP if SPL
> > select SPL_SYSCON if SPL
> > - select TPL_NEEDS_SEPARATE_STACK if TPL
> > + select TPL_HAVE_INIT_STACK if TPL
> > select SPL_SEPARATE_BSS
> > select CLK
> > select FIT
> > @@ -392,7 +392,7 @@ config ROCKCHIP_RV1126
> > select SKIP_LOWLEVEL_INIT_ONLY
> > select TPL
> > select SUPPORT_TPL
> > - select TPL_NEEDS_SEPARATE_STACK
> > + select TPL_HAVE_INIT_STACK
> > select TPL_ROCKCHIP_BACK_TO_BROM
> > select SPL
> > select SUPPORT_SPL
> > diff --git a/common/spl/Kconfig.tpl b/common/spl/Kconfig.tpl
> > index 22ca7016453..980d6da202a 100644
> > --- a/common/spl/Kconfig.tpl
> > +++ b/common/spl/Kconfig.tpl
> > @@ -106,12 +106,6 @@ config TPL_LDSCRIPT
> > May be left empty to trigger the Makefile infrastructure to
> > fall back to the linker-script used for the SPL stage.
> >
> > -config TPL_NEEDS_SEPARATE_STACK
> > - bool "TPL needs a separate initial stack-pointer"
> > - help
> > - Enable, if the TPL stage should not inherit its initial
> > - stack-pointer from the settings for the SPL stage.
> > -
> > config TPL_POWER
> > bool "Support power drivers"
> > help
> > @@ -140,11 +134,18 @@ config TPL_MAX_SIZE
> > help
> > The maximum size (in bytes) of the TPL stage.
> >
> > +config TPL_HAVE_INIT_STACK
> > + bool "TPL requires a initial, fixed, stack-pointer location"
> > + help
> > + Enable if the TPL phase should not use inherit its initial
>
> "should not use inherit its initial" does not sound correct.
Fixed
>
> > + stack-pointer from the settings for U-Boot proper, but should set
>
> Here you are changing from SPL to proper, however the SPL address will
> be used as the fallback (if it is defined) and then SYS_INIT_SP_ADDR.
OK, I'll update that.
>
> > + its own value.
> > +
> > config TPL_STACK
> > - hex "Address of the initial stack-pointer for the TPL stage"
> > - depends on TPL_NEEDS_SEPARATE_STACK
> > + hex "Address of the initial stack-pointer for the TPL phase"
> > + depends on TPL_HAVE_INIT_STACK
> > help
> > - The address of the initial stack-pointer for the TPL stage.
> > + The address of the initial stack-pointer for the TPL phase
>
> Did you remove the . (period) on purpose?
Yes, as it is not a sentence
>
> Regards,
> Jonas
>
> > Usually this will be the (aligned) top-of-stack.
> >
> > config TPL_READ_ONLY
> > diff --git a/configs/mk808_defconfig b/configs/mk808_defconfig
> > index e47d0b594f3..a1f089a23ea 100644
> > --- a/configs/mk808_defconfig
> > +++ b/configs/mk808_defconfig
> > @@ -48,7 +48,7 @@ CONFIG_SPL_NO_BSS_LIMIT=y
> > CONFIG_SPL_SEPARATE_BSS=y
> > CONFIG_SPL_FS_EXT4=y
> > CONFIG_SYS_MMCSD_FS_BOOT_PARTITION=2
> > -CONFIG_TPL_NEEDS_SEPARATE_STACK=y
> > +CONFIG_TPL_HAVE_INIT_STACK=y
> > # CONFIG_BOOTM_PLAN9 is not set
> > # CONFIG_BOOTM_RTEMS is not set
> > # CONFIG_BOOTM_VXWORKS is not set
>
Regards,
Simon
More information about the U-Boot
mailing list