[U-Boot] CONFIG_SYS_XIP_BOOT for when it's a choice?

Tom Rini trini at ti.com
Tue Apr 8 20:00:29 CEST 2014


On Tue, Apr 08, 2014 at 03:02:44PM +0200, Wolfgang Denk wrote:
> Dear Tom,
> 
> In message <20140407195731.GL23803 at bill-the-cat> you wrote:
> > 
> > Maybe, maybe not.  But it's beside the point.  The point is we have SoC
> > init code that needs to be run once.  Sometimes this is true when
> > CONFIG_SPL_BUILD=y, sometimes this is true when other options are set
> > instead.  What should we call the option to denote one-time init?
> > Should we extend CONFIG_SKIP_LOWLEVEL_INIT to cover this case?
> 
> Um... I think, if we build (and use) the SPL, then the low level init
> code would always be run (if at all) in the SPL; without SPL, it would
> be run in the normal U-Boot code.  CONFIG_SKIP_LOWLEVEL_INIT is
> intended to skip running this low level init code.  To me this looks
> independent of where this init code is located - it is yet another
> orthogonal decision.
> 
> Which sort of extension do you have in mind here?

Maybe talking about this in patches will be clearer.  If instead of
adding a new CONFIG option, I use CONFIG_SKIP_LOWLEVEL_INIT like this!
diff --git a/arch/arm/cpu/armv7/am33xx/board.c b/arch/arm/cpu/armv7/am33xx/board.c
index fb44cc8..28c16f8 100644
--- a/arch/arm/cpu/armv7/am33xx/board.c
+++ b/arch/arm/cpu/armv7/am33xx/board.c
@@ -142,7 +142,7 @@ int arch_misc_init(void)
 	return 0;
 }
 
-#if defined(CONFIG_SPL_BUILD) || defined(CONFIG_NOR_BOOT)
+#ifndef CONFIG_SKIP_LOWLEVEL_INIT
 /*
  * This function is the place to do per-board things such as ramp up the
  * MPU clock frequency.
@@ -200,9 +200,7 @@ static void watchdog_disable(void)
 	while (readl(&wdtimer->wdtwwps) != 0x0)
 		;
 }
-#endif
 
-#if defined(CONFIG_SPL_BUILD) || defined(CONFIG_NOR_BOOT)
 void s_init(void)
 {
 	/*
diff --git a/include/configs/ti_am335x_common.h b/include/configs/ti_am335x_common.h
index 50c3203..89d14c8 100644
--- a/include/configs/ti_am335x_common.h
+++ b/include/configs/ti_am335x_common.h
@@ -69,7 +69,8 @@
  * Since SPL did pll and ddr initialization for us,
  * we don't need to do it twice.
  */
-#if !defined(CONFIG_SPL_BUILD) && !defined(CONFIG_NOR_BOOT)
+#if !defined(CONFIG_SPL_BUILD) && !defined(CONFIG_NOR_BOOT) && \
+	!defined(CONFIG_QSPI_BOOT)
 #define CONFIG_SKIP_LOWLEVEL_INIT
 #endif

The following is true:
a) am335x_evm build, SPL calls s_init, does DDR init, u-boot.img
continues to not do so, just like today.
b) am335x_evm_norboot, _no_ SPL, u-boot.bin calls s_init, does DDR init,
just like today.
c) am43xx_evm build, SPL and u-boot.img work just like am335x_evm
d) am43xx_evm_qspiboot, _no_ SPL, u-boot.bin calls s_init, does DDR
init, just like today with the patch series that adds the target.

If instead we add a new option (for the moment, CONFIG_SYS_XIP_BOOT):

diff --git a/arch/arm/cpu/armv7/am33xx/board.c b/arch/arm/cpu/armv7/am33xx/board.c
index fb44cc8..133ca1c 100644
--- a/arch/arm/cpu/armv7/am33xx/board.c
+++ b/arch/arm/cpu/armv7/am33xx/board.c
@@ -142,7 +142,7 @@ int arch_misc_init(void)
 	return 0;
 }
 
-#if defined(CONFIG_SPL_BUILD) || defined(CONFIG_NOR_BOOT)
+#if defined(CONFIG_SPL_BUILD) || defined(CONFIG_SYS_XIP_BOOT)
 /*
  * This function is the place to do per-board things such as ramp up the
  * MPU clock frequency.
@@ -200,9 +200,7 @@ static void watchdog_disable(void)
 	while (readl(&wdtimer->wdtwwps) != 0x0)
 		;
 }
-#endif
 
-#if defined(CONFIG_SPL_BUILD) || defined(CONFIG_NOR_BOOT)
 void s_init(void)
 {
 	/*
diff --git a/boards.cfg b/boards.cfg
index b4203f1..b5a79d0 100644
--- a/boards.cfg
+++ b/boards.cfg
@@ -267,7 +267,7 @@ Active  arm         armv7          am33xx      silica          pengwyn
 Active  arm         armv7          am33xx      ti              am335x              am335x_boneblack                     am335x_evm:SERIAL1,CONS_INDEX=1,EMMC_BOOT                                                                                         Tom Rini <trini at ti.com>
 Active  arm         armv7          am33xx      ti              am335x              am335x_evm                           am335x_evm:SERIAL1,CONS_INDEX=1,NAND                                                                                              Tom Rini <trini at ti.com>
 Active  arm         armv7          am33xx      ti              am335x              am335x_evm_nor                       am335x_evm:SERIAL1,CONS_INDEX=1,NAND,NOR                                                                                          Tom Rini <trini at ti.com>
-Active  arm         armv7          am33xx      ti              am335x              am335x_evm_norboot                   am335x_evm:SERIAL1,CONS_INDEX=1,NOR,NOR_BOOT                                                                                      Tom Rini <trini at ti.com>
+Active  arm         armv7          am33xx      ti              am335x              am335x_evm_norboot                   am335x_evm:SERIAL1,CONS_INDEX=1,NOR,NOR_BOOT,SYS_XIP_BOOT                                                                         Tom Rini <trini at ti.com>
 Active  arm         armv7          am33xx      ti              am335x              am335x_evm_spiboot                   am335x_evm:SERIAL1,CONS_INDEX=1,SPI_BOOT                                                                                          Tom Rini <trini at ti.com>
 Active  arm         armv7          am33xx      ti              am335x              am335x_evm_uart1                     am335x_evm:SERIAL2,CONS_INDEX=2,NAND                                                                                              Tom Rini <trini at ti.com>
 Active  arm         armv7          am33xx      ti              am335x              am335x_evm_uart2                     am335x_evm:SERIAL3,CONS_INDEX=3,NAND                                                                                              Tom Rini <trini at ti.com>
diff --git a/include/configs/ti_am335x_common.h b/include/configs/ti_am335x_common.h
index 50c3203..1092c25 100644
--- a/include/configs/ti_am335x_common.h
+++ b/include/configs/ti_am335x_common.h
@@ -69,7 +69,8 @@
  * Since SPL did pll and ddr initialization for us,
  * we don't need to do it twice.
  */
-#if !defined(CONFIG_SPL_BUILD) && !defined(CONFIG_NOR_BOOT)
+#if !defined(CONFIG_SPL_BUILD) && !defined(CONFIG_NOR_BOOT) && \
+	!defined(CONFIG_QSPI_BOOT)
 #define CONFIG_SKIP_LOWLEVEL_INIT
 #endif
 
Pasting it out, I like using CONFIG_SKIP_LOWLEVEL_INIT more to cover
this code.  Does that work for you?

> > > If you use the memory mapped mode, then it looks just like any
> > > other ROM, and we should not need special code either.
> >
> > Well, for CONFIG_NOR_BOOT we must have it on am335x (haven't checked
> > am43xx) to finish doing pinmux as only 12KiB is mapped, in addition to
> > env related options.  On am43xx and QSPI it's just used to denote when
> > we are doing one-time init in U-Boot proper rather than SPL.
> 
> Sorry, I lost you.  what is the "it" in both sentences referring to?

First it is the running binary (in this case, U-Boot).  The second it is
CONFIG_QSPI_BOOT.

> Also, I'm not sure what exactly you mean by "one-time"init - all
> initialization steps in init_sequence[] are executed only once.

Right, but we don't initalize DDR twice, especially if we're running
from DDR.

> These settings you are referring to - can these me made here, or is
> this stuff that is running before board_init_f() ?
> 
> 
> Um... I just ran over this in "arch/arm/lib/spl.c":
> 
>  21  * In the context of SPL, board_init_f must ensure that any clocks/etc for
>  22  * DDR are enabled, ensure that the stack pointer is valid, clear the BSS
>  23  * and call board_init_f.  We provide this version by default but mark it
>            ^^^^^^^^^^^^^^^^^
>  24  * as __weak to allow for platforms to do this in their own way if needed.
>  25  */
>  26 void __weak board_init_f(ulong dummy)   
>  27 {
>  28         /* Clear the BSS. */
>  29         memset(__bss_start, 0, __bss_end - __bss_start);
>  30
>  31         /* Set global data pointer. */
>  32         gd = &gdata;
>  33 
>  34         board_init_r(NULL, 0);
>             ^^^^^^^^^^^^
> 
> The comment says _f, the code has _r - which is right?

Oops, the comment should say "_r" as board_init_r is common SPL code.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140408/0fa750e2/attachment.pgp>


More information about the U-Boot mailing list