[PATCH v2 05/30] global: Use CONFIG_XPL_BUILD instead of CONFIG_SPL_BUILD

Simon Glass sjg at chromium.org
Sun Sep 29 13:48:53 CEST 2024


Hi Jonas,

On Sun, 29 Sept 2024 at 13:00, Jonas Karlman <jonas at kwiboo.se> wrote:
>
> Hi Simon,
>
> On 2024-09-28 22:00, Simon Glass wrote:
> > Use the new symbol to refer to any 'SPL' build, including TPL and VPL
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> > (no changes since v1)
> >
>
> [snip]
>
> > diff --git a/arch/arm/include/asm/arch-rockchip/boot0.h b/arch/arm/include/asm/arch-rockchip/boot0.h
> > index 0c375e543a5..edb2a31c348 100644
> > --- a/arch/arm/include/asm/arch-rockchip/boot0.h
> > +++ b/arch/arm/include/asm/arch-rockchip/boot0.h
> > @@ -12,7 +12,7 @@
> >   * To make life easier for everyone, we build the SPL binary with
> >   * space for this 4-byte header already included in the binary.
> >   */
> > -#ifdef CONFIG_SPL_BUILD
> > +#ifdef CONFIG_XPL_BUILD
> >       /*
> >        * We need to add 4 bytes of space for the 'RK33' at the
> >        * beginning of the executable.  However, as we want to keep
> > @@ -39,7 +39,7 @@ entry_counter:
> >       .word   0
> >  #endif
> >
> > -#if (defined(CONFIG_SPL_BUILD) || defined(CONFIG_ARM64))
> > +#if (defined(CONFIG_XPL_BUILD) || defined(CONFIG_ARM64))
> >       /* U-Boot proper of armv7 do not need this */
> >       b reset
> >  #endif
> > @@ -54,7 +54,7 @@ _start:
> >       ARM_VECTORS
> >  #endif
> >
> > -#if !defined(CONFIG_TPL_BUILD) && defined(CONFIG_SPL_BUILD) && \
> > +#if !defined(CONFIG_TPL_BUILD) && defined(CONFIG_XPL_BUILD) && \
>
> This is meant to be for SPL where TF-A is loaded into part of SRAM,
> and is only needed/used when CONFIG_TPL=n, so no need to replace it with
> XPL_BUILD.

I'm not sure if there is a misunderstanding here.

XPL_BUILD is the new SPL_BUILD, i.e. it means the same as SPL_BUILD
used to. So all occurrences of SPL_BUILD need to change to XPL_BUILD.

Before it was a bit unclear and murky as to what was intended,
particularly as apparently many people were not too sure of the real
meaning of SPL_BUILD. Resolving that is the intent of this rename.

Of course I think follow-up patches could tidy things up, now that we
have a 'real' SPL_BUILD (which means SPL and not also TPL, VPL). I
included some of those in the v1 series, but of course then people
wonder why they can't be squashed into the rename, so it just confuses
the whole process.

So my goal with this patch is to completely rename everything.


>
> >       (CONFIG_ROCKCHIP_SPL_RESERVE_IRAM > 0)
> >       .space CONFIG_ROCKCHIP_SPL_RESERVE_IRAM /* space for the ATF data */
> >  #endif
> > diff --git a/arch/arm/include/asm/arch-rockchip/cru_rv1126.h b/arch/arm/include/asm/arch-rockchip/cru_rv1126.h
> > index 49a1f763795..ae273de3144 100644
> > --- a/arch/arm/include/asm/arch-rockchip/cru_rv1126.h
> > +++ b/arch/arm/include/asm/arch-rockchip/cru_rv1126.h
> > @@ -11,7 +11,7 @@
> >  #define KHz          1000
> >  #define OSC_HZ               (24 * MHz)
> >
> > -#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_KERNEL_BOOT)
> > +#if defined(CONFIG_XPL_BUILD) && defined(CONFIG_SPL_KERNEL_BOOT)
> >  #define APLL_HZ              (1008 * MHz)
> >  #else
> >  #define APLL_HZ              (816 * MHz)
> > @@ -20,7 +20,7 @@
> >  #define CPLL_HZ              (500 * MHz)
> >  #define HPLL_HZ              (1400 * MHz)
> >  #define PCLK_PDPMU_HZ        (100 * MHz)
> > -#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_KERNEL_BOOT)
> > +#if defined(CONFIG_XPL_BUILD) && defined(CONFIG_SPL_KERNEL_BOOT)
> >  #define ACLK_PDBUS_HZ        (396 * MHz)
> >  #else
> >  #define ACLK_PDBUS_HZ        (500 * MHz)
> > @@ -32,7 +32,7 @@
> >  #define HCLK_PDCORE_HZ       (200 * MHz)
> >  #define HCLK_PDAUDIO_HZ      (150 * MHz)
> >  #define CLK_OSC0_DIV_HZ      (32768)
> > -#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_KERNEL_BOOT)
> > +#if defined(CONFIG_XPL_BUILD) && defined(CONFIG_SPL_KERNEL_BOOT)
> >  #define ACLK_PDVI_HZ (297 * MHz)
> >  #define CLK_ISP_HZ   (297 * MHz)
> >  #define ACLK_PDISPP_HZ       (297 * MHz)
> > @@ -324,7 +324,7 @@ enum {
> >       DCLK_VOP_DIV_SHIFT      = 0,
> >       DCLK_VOP_DIV_MASK       = 0xff,
> >
> > -#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_KERNEL_BOOT)
> > +#if defined(CONFIG_XPL_BUILD) && defined(CONFIG_SPL_KERNEL_BOOT)
> >       /* CRU_CLK_SEL49_CON */
> >       ACLK_PDVI_SEL_SHIFT     = 6,
> >       ACLK_PDVI_SEL_MASK      = 0x3 << ACLK_PDVI_SEL_SHIFT,
> > @@ -397,7 +397,7 @@ enum {
> >       CLK_GMAC_SRC_DIV_SHIFT  = 0,
> >       CLK_GMAC_SRC_DIV_MASK   = 0x1f << CLK_GMAC_SRC_DIV_SHIFT,
> >
> > -#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_KERNEL_BOOT)
> > +#if defined(CONFIG_XPL_BUILD) && defined(CONFIG_SPL_KERNEL_BOOT)
> >       /* CRU_CLK_SEL68_CON */
> >       ACLK_PDISPP_SEL_SHIFT   = 6,
> >       ACLK_PDISPP_SEL_MASK    = 0x3 << ACLK_PDISPP_SEL_SHIFT,
>
> I am pretty sure all these are meant for SPL where it is possible to
> SPL_KERNEL_BOOT, so no need to replace these with XPL_BUILD.
>
> [snip]
>
> > diff --git a/arch/arm/mach-rockchip/rk3399/rk3399.c b/arch/arm/mach-rockchip/rk3399/rk3399.c
> > index 2d7d0f82a2f..edccb2a3980 100644
> > --- a/arch/arm/mach-rockchip/rk3399/rk3399.c
> > +++ b/arch/arm/mach-rockchip/rk3399/rk3399.c
> > @@ -51,7 +51,7 @@ static struct mm_region rk3399_mem_map[] = {
> >
> >  struct mm_region *mem_map = rk3399_mem_map;
> >
> > -#ifdef CONFIG_SPL_BUILD
> > +#ifdef CONFIG_XPL_BUILD
> >
> >  #define TIMER_END_COUNT_L    0x00
> >  #define TIMER_END_COUNT_H    0x04
> > @@ -83,7 +83,7 @@ void rockchip_stimer_init(void)
> >  int arch_cpu_init(void)
> >  {
> >
> > -#ifdef CONFIG_SPL_BUILD
> > +#ifdef CONFIG_XPL_BUILD
> >       struct rk3399_pmusgrf_regs *sgrf;
> >       struct rk3399_grf_regs *grf;
> >
> > @@ -136,7 +136,7 @@ void board_debug_uart_init(void)
> >       struct rk3399_pmugrf_regs * const pmugrf = (void *)PMUGRF_BASE;
> >       struct rockchip_gpio_regs * const gpio = (void *)GPIO0_BASE;
> >
> > -     if (IS_ENABLED(CONFIG_SPL_BUILD) &&
> > +     if (IS_ENABLED(CONFIG_XPL_BUILD) &&
>
> This should not be changed to XPL, this part is meant for SPL and does
> not even compile when CONFIG_TPL=y and TPL_BUILD is defined.

The existing code before this series builds fine, though. I did not
change it to SPL_BUILD...it has been that way for a while. I am
changing it to XPL_BUILD which (by the end of this series) means
exactly the same as SPL_BUILD used to.

>
> >           (IS_ENABLED(CONFIG_TARGET_CHROMEBOOK_BOB) ||
> >            IS_ENABLED(CONFIG_TARGET_CHROMEBOOK_KEVIN))) {
> >               rk_setreg(&grf->io_vsel, 1 << 0);
> > @@ -169,7 +169,7 @@ void board_debug_uart_init(void)
> >  }
> >  #endif
> >
> > -#if defined(CONFIG_SPL_BUILD) && !defined(CONFIG_TPL_BUILD)
> > +#if defined(CONFIG_XPL_BUILD) && !defined(CONFIG_TPL_BUILD)
>
> Following is intended for SPL as can be observed from the !TPL_BUILD,
> please do not change this to XPL_BUILD.

These and following are similar comments, so I will skip them for now.

[..]

Regards,
Simon


More information about the U-Boot mailing list