[U-Boot] [PATCH v3 0/8] arm: Tidy up early init
This series collects the previous RFT patches I sent out.
https://patchwork.ozlabs.org/patch/508167/ https://patchwork.ozlabs.org/patch/508168/
It turns out that I originally sent a version of these in April:
https://patchwork.ozlabs.org/patch/461687/ https://patchwork.ozlabs.org/patch/461690/
so this series mirrors that one and includes the Zynq patches from that series.
I have tested this on a few ARM platforms: Zynq Zybo, Beaglebone Black, pcduino3 (sunxi), Jetson-TK1 (tegra).
Changes in v3: - Rebase to master - Rebase to master
Changes in v2: - Put this into common/init/ and just Makefiles accordingly - Add comments as to why this is needed, deal with arch-specific memset()
Simon Glass (8): Move board_init_f_mem() into a common location board_init_f_mem(): Don't require memset() board_init_f_mem(): Don't create an unused early malloc() area arm: Switch aarch64 to using generic global_data setup arm: Switch 32-bit ARM to using generic global_data setup microblaze: Add a TODO to call board_init_f_mem() zynq: Move SPL console init out of board_init_f() Revert "ARM: zynq: disable CONFIG_SYS_MALLOC_F to fix MMC boot"
arch/arm/lib/crt0.S | 28 +++------------------ arch/arm/lib/crt0_64.S | 15 +++-------- arch/arm/mach-zynq/spl.c | 2 +- arch/microblaze/cpu/start.S | 2 ++ common/Makefile | 1 + common/board_f.c | 29 --------------------- common/init/Makefile | 7 ++++++ common/init/board_init.c | 60 ++++++++++++++++++++++++++++++++++++++++++++ configs/zynq_zc702_defconfig | 1 - scripts/Makefile.spl | 1 + 10 files changed, 79 insertions(+), 67 deletions(-) create mode 100644 common/init/Makefile create mode 100644 common/init/board_init.c
This function will be used by both SPL and U-Boot proper. So move it into a common place. Also change the #ifdef so that the early malloc() area is not set up in SPL if CONFIG_SYS_SPL_MALLOC_START is defined. In that case it would never actually be used, and just chews up stack space.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v3: None Changes in v2: - Put this into common/init/ and just Makefiles accordingly
common/Makefile | 1 + common/board_f.c | 29 ----------------------------- common/init/Makefile | 7 +++++++ common/init/board_init.c | 41 +++++++++++++++++++++++++++++++++++++++++ scripts/Makefile.spl | 1 + 5 files changed, 50 insertions(+), 29 deletions(-) create mode 100644 common/init/Makefile create mode 100644 common/init/board_init.c
diff --git a/common/Makefile b/common/Makefile index 491c565..e2f9401 100644 --- a/common/Makefile +++ b/common/Makefile @@ -7,6 +7,7 @@
# core ifndef CONFIG_SPL_BUILD +obj-y += init/ obj-y += main.o obj-y += exports.o obj-y += hash.o diff --git a/common/board_f.c b/common/board_f.c index 613332e..62570ab 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -1030,32 +1030,3 @@ void board_init_f_r(void) hang(); } #endif /* CONFIG_X86 */ - -/* Unfortunately x86 can't compile this code as gd cannot be assigned */ -#ifndef CONFIG_X86 -__weak void arch_setup_gd(struct global_data *gd_ptr) -{ - gd = gd_ptr; -} -#endif /* !CONFIG_X86 */ - -ulong board_init_f_mem(ulong top) -{ - struct global_data *gd_ptr; - - /* Leave space for the stack we are running with now */ - top -= 0x40; - - top -= sizeof(struct global_data); - top = ALIGN(top, 16); - gd_ptr = (struct global_data *)top; - memset(gd_ptr, '\0', sizeof(*gd)); - arch_setup_gd(gd_ptr); - -#ifdef CONFIG_SYS_MALLOC_F_LEN - top -= CONFIG_SYS_MALLOC_F_LEN; - gd->malloc_base = top; -#endif - - return top; -} diff --git a/common/init/Makefile b/common/init/Makefile new file mode 100644 index 0000000..4902635 --- /dev/null +++ b/common/init/Makefile @@ -0,0 +1,7 @@ +# +# Copyright (c) 2015 Google, Inc +# +# SPDX-License-Identifier: GPL-2.0+ +# + +obj-y += board_init.o diff --git a/common/init/board_init.c b/common/init/board_init.c new file mode 100644 index 0000000..e7ebca7 --- /dev/null +++ b/common/init/board_init.c @@ -0,0 +1,41 @@ +/* + * Code shared between SPL and U-Boot proper + * + * Copyright (c) 2015 Google, Inc + * Written by Simon Glass sjg@chromium.org + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> + +DECLARE_GLOBAL_DATA_PTR; + +/* Unfortunately x86 can't compile this code as gd cannot be assigned */ +#ifndef CONFIG_X86 +__weak void arch_setup_gd(struct global_data *gd_ptr) +{ + gd = gd_ptr; +} +#endif /* !CONFIG_X86 */ + +ulong board_init_f_mem(ulong top) +{ + struct global_data *gd_ptr; + + /* Leave space for the stack we are running with now */ + top -= 0x40; + + top -= sizeof(struct global_data); + top = ALIGN(top, 16); + gd_ptr = (struct global_data *)top; + memset(gd_ptr, '\0', sizeof(*gd)); + arch_setup_gd(gd_ptr); + +#if defined(CONFIG_SYS_MALLOC_F) + top -= CONFIG_SYS_MALLOC_F_LEN; + gd->malloc_base = top; +#endif + + return top; +} diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index 58442f1..2df93c8 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -52,6 +52,7 @@ libs-y += $(if $(BOARDDIR),board/$(BOARDDIR)/) libs-$(HAVE_VENDOR_COMMON_LIB) += board/$(VENDOR)/common/
libs-$(CONFIG_SPL_FRAMEWORK) += common/spl/ +libs-y += common/init/ libs-$(CONFIG_SPL_LIBCOMMON_SUPPORT) += common/ libs-$(CONFIG_SPL_LIBDISK_SUPPORT) += disk/ libs-y += drivers/
Unfortunately memset() is not always available, so provide a substitute when needed.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v3: None Changes in v2: - Add comments as to why this is needed, deal with arch-specific memset()
common/init/board_init.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/common/init/board_init.c b/common/init/board_init.c index e7ebca7..c113a80 100644 --- a/common/init/board_init.c +++ b/common/init/board_init.c @@ -11,6 +11,16 @@
DECLARE_GLOBAL_DATA_PTR;
+/* + * It isn't trivial to figure out whether memcpy() exists. The arch-specific + * memcpy() is not normally available in SPL due to code size. + */ +#if !defined(CONFIG_SPL_BUILD) || \ + (defined(CONFIG_SPL_LIBGENERIC_SUPPORT) && \ + !defined(CONFIG_USE_ARCH_MEMSET)) +#define _USE_MEMCPY +#endif + /* Unfortunately x86 can't compile this code as gd cannot be assigned */ #ifndef CONFIG_X86 __weak void arch_setup_gd(struct global_data *gd_ptr) @@ -22,6 +32,9 @@ __weak void arch_setup_gd(struct global_data *gd_ptr) ulong board_init_f_mem(ulong top) { struct global_data *gd_ptr; +#ifndef _USE_MEMCPY + int *ptr, *end; +#endif
/* Leave space for the stack we are running with now */ top -= 0x40; @@ -29,7 +42,12 @@ ulong board_init_f_mem(ulong top) top -= sizeof(struct global_data); top = ALIGN(top, 16); gd_ptr = (struct global_data *)top; +#ifdef _USE_MEMCPY memset(gd_ptr, '\0', sizeof(*gd)); +#else + for (ptr = (int *)gd_ptr, end = (int *)(gd_ptr + 1); ptr < end; ) + *ptr++ = 0; +#endif arch_setup_gd(gd_ptr);
#if defined(CONFIG_SYS_MALLOC_F)
Hello Simon,
On Sat, 17 Oct 2015 15:06:55 -0600, Simon Glass sjg@chromium.org wrote:
Unfortunately memset() is not always available, so provide a substitute when needed.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3: None Changes in v2:
- Add comments as to why this is needed, deal with arch-specific memset()
common/init/board_init.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/common/init/board_init.c b/common/init/board_init.c index e7ebca7..c113a80 100644 --- a/common/init/board_init.c +++ b/common/init/board_init.c @@ -11,6 +11,16 @@
DECLARE_GLOBAL_DATA_PTR;
+/*
- It isn't trivial to figure out whether memcpy() exists. The arch-specific
- memcpy() is not normally available in SPL due to code size.
- */
+#if !defined(CONFIG_SPL_BUILD) || \
(defined(CONFIG_SPL_LIBGENERIC_SUPPORT) && \!defined(CONFIG_USE_ARCH_MEMSET))+#define _USE_MEMCPY +#endif
/* Unfortunately x86 can't compile this code as gd cannot be assigned */ #ifndef CONFIG_X86 __weak void arch_setup_gd(struct global_data *gd_ptr) @@ -22,6 +32,9 @@ __weak void arch_setup_gd(struct global_data *gd_ptr) ulong board_init_f_mem(ulong top) { struct global_data *gd_ptr; +#ifndef _USE_MEMCPY
- int *ptr, *end;
+#endif
/* Leave space for the stack we are running with now */ top -= 0x40; @@ -29,7 +42,12 @@ ulong board_init_f_mem(ulong top) top -= sizeof(struct global_data); top = ALIGN(top, 16); gd_ptr = (struct global_data *)top; +#ifdef _USE_MEMCPY memset(gd_ptr, '\0', sizeof(*gd)); +#else
- for (ptr = (int *)gd_ptr, end = (int *)(gd_ptr + 1); ptr < end; )
Nitpick here: There is little point in naming a variable just for it to be set and used once. Without 'end', the compiler will be just as fine if ptr is directly tested against (int *)(gd_ptr + 1), and human readers won't wonder why 'end' was created.
*ptr++ = 0;+#endif arch_setup_gd(gd_ptr);
#if defined(CONFIG_SYS_MALLOC_F)
2.6.0.rc2.230.g3dd15c0
Amicalement,
Hi Albert,
On 18 October 2015 at 10:28, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
Hello Simon,
On Sat, 17 Oct 2015 15:06:55 -0600, Simon Glass sjg@chromium.org wrote:
Unfortunately memset() is not always available, so provide a substitute when needed.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3: None Changes in v2:
- Add comments as to why this is needed, deal with arch-specific memset()
common/init/board_init.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/common/init/board_init.c b/common/init/board_init.c index e7ebca7..c113a80 100644 --- a/common/init/board_init.c +++ b/common/init/board_init.c @@ -11,6 +11,16 @@
DECLARE_GLOBAL_DATA_PTR;
+/*
- It isn't trivial to figure out whether memcpy() exists. The arch-specific
- memcpy() is not normally available in SPL due to code size.
- */
+#if !defined(CONFIG_SPL_BUILD) || \
(defined(CONFIG_SPL_LIBGENERIC_SUPPORT) && \!defined(CONFIG_USE_ARCH_MEMSET))+#define _USE_MEMCPY +#endif
/* Unfortunately x86 can't compile this code as gd cannot be assigned */ #ifndef CONFIG_X86 __weak void arch_setup_gd(struct global_data *gd_ptr) @@ -22,6 +32,9 @@ __weak void arch_setup_gd(struct global_data *gd_ptr) ulong board_init_f_mem(ulong top) { struct global_data *gd_ptr; +#ifndef _USE_MEMCPY
int *ptr, *end;+#endif
/* Leave space for the stack we are running with now */ top -= 0x40;@@ -29,7 +42,12 @@ ulong board_init_f_mem(ulong top) top -= sizeof(struct global_data); top = ALIGN(top, 16); gd_ptr = (struct global_data *)top; +#ifdef _USE_MEMCPY memset(gd_ptr, '\0', sizeof(*gd)); +#else
for (ptr = (int *)gd_ptr, end = (int *)(gd_ptr + 1); ptr < end; )Nitpick here: There is little point in naming a variable just for it to be set and used once. Without 'end', the compiler will be just as fine if ptr is directly tested against (int *)(gd_ptr + 1), and human readers won't wonder why 'end' was created.
Well it makes it clear that the ptr goes from the start to the end. But it's probably clear enough just doing what you suggest, so I can change it.
*ptr++ = 0;+#endif arch_setup_gd(gd_ptr);
#if defined(CONFIG_SYS_MALLOC_F)
2.6.0.rc2.230.g3dd15c0
Amicalement,
Albert.
Regards, Simon
Hello Simon,
On Sun, 18 Oct 2015 14:38:06 -0600, Simon Glass sjg@chromium.org wrote:
Hi Albert,
On 18 October 2015 at 10:28, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
Hello Simon,
On Sat, 17 Oct 2015 15:06:55 -0600, Simon Glass sjg@chromium.org wrote:
Unfortunately memset() is not always available, so provide a substitute when needed.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3: None Changes in v2:
- Add comments as to why this is needed, deal with arch-specific memset()
common/init/board_init.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/common/init/board_init.c b/common/init/board_init.c index e7ebca7..c113a80 100644 --- a/common/init/board_init.c +++ b/common/init/board_init.c @@ -11,6 +11,16 @@
DECLARE_GLOBAL_DATA_PTR;
+/*
- It isn't trivial to figure out whether memcpy() exists. The arch-specific
- memcpy() is not normally available in SPL due to code size.
- */
+#if !defined(CONFIG_SPL_BUILD) || \
(defined(CONFIG_SPL_LIBGENERIC_SUPPORT) && \!defined(CONFIG_USE_ARCH_MEMSET))+#define _USE_MEMCPY +#endif
/* Unfortunately x86 can't compile this code as gd cannot be assigned */ #ifndef CONFIG_X86 __weak void arch_setup_gd(struct global_data *gd_ptr) @@ -22,6 +32,9 @@ __weak void arch_setup_gd(struct global_data *gd_ptr) ulong board_init_f_mem(ulong top) { struct global_data *gd_ptr; +#ifndef _USE_MEMCPY
int *ptr, *end;+#endif
/* Leave space for the stack we are running with now */ top -= 0x40;@@ -29,7 +42,12 @@ ulong board_init_f_mem(ulong top) top -= sizeof(struct global_data); top = ALIGN(top, 16); gd_ptr = (struct global_data *)top; +#ifdef _USE_MEMCPY memset(gd_ptr, '\0', sizeof(*gd)); +#else
for (ptr = (int *)gd_ptr, end = (int *)(gd_ptr + 1); ptr < end; )Nitpick here: There is little point in naming a variable just for it to be set and used once. Without 'end', the compiler will be just as fine if ptr is directly tested against (int *)(gd_ptr + 1), and human readers won't wonder why 'end' was created.
Well it makes it clear that the ptr goes from the start to the end. But it's probably clear enough just doing what you suggest, so I can change it.
Please do, thanks!
Amicalement,
Change the #ifdef so that the early malloc() area is not set up in SPL if CONFIG_SYS_SPL_MALLOC_START is defined. In that case it would never actually be used, and just chews up stack space.
Signed-off-by: Simon Glass sjg@chromium.org Tested-by: Masahiro Yamada yamada.masahiro@socionext.com ---
Changes in v3: None Changes in v2: None
common/init/board_init.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/common/init/board_init.c b/common/init/board_init.c index c113a80..040baca 100644 --- a/common/init/board_init.c +++ b/common/init/board_init.c @@ -50,7 +50,8 @@ ulong board_init_f_mem(ulong top) #endif arch_setup_gd(gd_ptr);
-#if defined(CONFIG_SYS_MALLOC_F) +#if defined(CONFIG_SYS_MALLOC_F) && \ + (!defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SYS_SPL_MALLOC_START)) top -= CONFIG_SYS_MALLOC_F_LEN; gd->malloc_base = top; #endif
There is quite a bit of assembler code that can be removed if we use the generic global_data setup. Less arch-specific code makes it easier to add new features and maintain the start-up code.
Drop the unneeded code and adjust the hooks in board_f.c to cope.
Tested on LS2085ARDB and LS2085AQDS (armv8 SoC). Tested-by: York Sun yorksun@freescale.com
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v3: None Changes in v2: None
arch/arm/lib/crt0_64.S | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/arch/arm/lib/crt0_64.S b/arch/arm/lib/crt0_64.S index 8b34e04..cef1c71 100644 --- a/arch/arm/lib/crt0_64.S +++ b/arch/arm/lib/crt0_64.S @@ -74,19 +74,10 @@ ENTRY(_main) #else ldr x0, =(CONFIG_SYS_INIT_SP_ADDR) #endif - sub x18, x0, #GD_SIZE /* allocate one GD above SP */ - bic x18, x18, #0x7 /* 8-byte alignment for GD */ -zero_gd: - sub x0, x0, #0x8 - str xzr, [x0] - cmp x0, x18 - b.gt zero_gd -#if defined(CONFIG_SYS_MALLOC_F_LEN) - ldr x0, =CONFIG_SYS_MALLOC_F_LEN - sub x0, x18, x0 - str x0, [x18, #GD_MALLOC_BASE] -#endif bic sp, x0, #0xf /* 16-byte alignment for ABI compliance */ + bl board_init_f_mem + mov sp, x0 + mov x0, #0 bl board_init_f
There is quite a bit of assembler code that can be removed if we use the generic global_data setup. Less arch-specific code makes it easier to add new features and maintain the start-up code.
Drop the unneeded code and adjust the hooks in board_f.c to cope.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v3: None Changes in v2: None
arch/arm/lib/crt0.S | 28 ++++------------------------ 1 file changed, 4 insertions(+), 24 deletions(-)
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S index 4c3a94a..80548eb 100644 --- a/arch/arm/lib/crt0.S +++ b/arch/arm/lib/crt0.S @@ -82,31 +82,11 @@ ENTRY(_main) #else bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ #endif - mov r2, sp - sub sp, sp, #GD_SIZE /* allocate one GD above SP */ -#if defined(CONFIG_CPU_V7M) /* v7M forbids using SP as BIC destination */ - mov r3, sp - bic r3, r3, #7 - mov sp, r3 -#else - bic sp, sp, #7 /* 8-byte alignment for ABI compliance */ -#endif - mov r9, sp /* GD is above SP */ - mov r1, sp + mov r0, sp + bl board_init_f_mem + mov sp, r0 + mov r0, #0 -clr_gd: - cmp r1, r2 /* while not at end of GD */ -#if defined(CONFIG_CPU_V7M) - itt lo -#endif - strlo r0, [r1] /* clear 32-bit GD word */ - addlo r1, r1, #4 /* move to next */ - blo clr_gd -#if defined(CONFIG_SYS_MALLOC_F_LEN) - sub sp, sp, #CONFIG_SYS_MALLOC_F_LEN - str sp, [r9, #GD_MALLOC_BASE] -#endif - /* mov r0, #0 not needed due to above code */ bl board_init_f
#if ! defined(CONFIG_SPL_BUILD)
This C function should be used to do the early memory layout and init. This is beyond my powers, so just add a TODO for the maintainer.
Signed-off-by: Simon Glass sjg@chromium.org Acked-by: Michal Simek michal.simek@xilinx.com ---
Changes in v3: None Changes in v2: None
arch/microblaze/cpu/start.S | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/arch/microblaze/cpu/start.S b/arch/microblaze/cpu/start.S index 953d3a1..14f46a8 100644 --- a/arch/microblaze/cpu/start.S +++ b/arch/microblaze/cpu/start.S @@ -25,6 +25,7 @@ _start:
addi r8, r0, __end mts rslr, r8 + /* TODO: Redo this code to call board_init_f_mem() */ #if defined(CONFIG_SPL_BUILD) addi r1, r0, CONFIG_SPL_STACK_ADDR mts rshr, r1 @@ -141,6 +142,7 @@ _start: ori r12, r12, 0x1a0 mts rmsr, r12
+ /* TODO: Redo this code to call board_init_f_mem() */ clear_bss: /* clear BSS segments */ addi r5, r0, __bss_start
We should not init the console this early and there is no need to. If we want to do early init it can be done in spl_board_init(). Move the preloader_console_init() call from board_init_f() to board_init_r().
Signed-off-by: Simon Glass sjg@chromium.org Tested-by: Masahiro Yamada yamada.masahiro@socionext.com Tested-by: Michal Simek michal.simek@xilinx.com ---
Changes in v3: - Rebase to master
Changes in v2: None
arch/arm/mach-zynq/spl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-zynq/spl.c b/arch/arm/mach-zynq/spl.c index e7df6d3..7bdac3b 100644 --- a/arch/arm/mach-zynq/spl.c +++ b/arch/arm/mach-zynq/spl.c @@ -20,7 +20,6 @@ void board_init_f(ulong dummy) /* Clear the BSS. */ memset(__bss_start, 0, __bss_end - __bss_start);
- preloader_console_init(); arch_cpu_init(); board_init_r(NULL, 0); } @@ -28,6 +27,7 @@ void board_init_f(ulong dummy) #ifdef CONFIG_SPL_BOARD_INIT void spl_board_init(void) { + preloader_console_init(); board_init(); } #endif
Hello Simon,
On Sat, 17 Oct 2015 15:07:00 -0600, Simon Glass sjg@chromium.org wrote:
We should not init the console this early and there is no need to. If we want to do early init it can be done in spl_board_init(). Move the preloader_console_init() call from board_init_f() to board_init_r().
Signed-off-by: Simon Glass sjg@chromium.org Tested-by: Masahiro Yamada yamada.masahiro@socionext.com Tested-by: Michal Simek michal.simek@xilinx.com
I personally think that we should, almost must in fact, initialize the console as early as possible.
What exactly is the drawaback of initializing the console here?
Amicalement,
Hi Albert,
On 18 October 2015 at 10:36, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
Hello Simon,
On Sat, 17 Oct 2015 15:07:00 -0600, Simon Glass sjg@chromium.org wrote:
We should not init the console this early and there is no need to. If we want to do early init it can be done in spl_board_init(). Move the preloader_console_init() call from board_init_f() to board_init_r().
Signed-off-by: Simon Glass sjg@chromium.org Tested-by: Masahiro Yamada yamada.masahiro@socionext.com Tested-by: Michal Simek michal.simek@xilinx.com
I personally think that we should, almost must in fact, initialize the console as early as possible.
What exactly is the drawaback of initializing the console here?
This is described in the README now for SPL. The console is not available until driver model is ready, which cannot be before global_data is ready.
My second zynq series removes the next few lines from board_init_f(), so in effect there is very little difference in timing - the console still is set up very early. It is just that we must not set it up before driver model is running.
There is also a debug UART which can be used to make printf() work before global_data is available. But I'm trying to remove the global_data hacks that exist in early board code.
Regards, Simon
Hello Simon,
On Sun, 18 Oct 2015 14:37:03 -0600, Simon Glass sjg@chromium.org wrote:
Hi Albert,
On 18 October 2015 at 10:36, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
Hello Simon,
On Sat, 17 Oct 2015 15:07:00 -0600, Simon Glass sjg@chromium.org wrote:
We should not init the console this early and there is no need to. If we want to do early init it can be done in spl_board_init(). Move the preloader_console_init() call from board_init_f() to board_init_r().
Signed-off-by: Simon Glass sjg@chromium.org Tested-by: Masahiro Yamada yamada.masahiro@socionext.com Tested-by: Michal Simek michal.simek@xilinx.com
I personally think that we should, almost must in fact, initialize the console as early as possible.
What exactly is the drawaback of initializing the console here?
This is described in the README now for SPL. The console is not available until driver model is ready, which cannot be before global_data is ready.
Makes sense now.
Maybe the commit message could explicitly mention that the cause of the "should not" is the driver model? This commit might be viewed in isolation and the reader may need a clue on how to relate that commit to the driver model or the README.
My second zynq series removes the next few lines from board_init_f(), so in effect there is very little difference in timing - the console still is set up very early. It is just that we must not set it up before driver model is running.
There is also a debug UART which can be used to make printf() work before global_data is available. But I'm trying to remove the global_data hacks that exist in early board code.
Then my initial concern is address (and again, maybe a small note in the commit message could remind the reader about the debug UART not being affected by the commit). Thanks!
Regards, Simon
Amicalement,
This reverts commit 321f86e18d6aae9f7b7ba3ef1eb0cec769481874.
The original bug has been fixed.
Signed-off-by: Simon Glass sjg@chromium.org Tested-on: Zedboard and ZC706 board Tested-by: Masahiro Yamada yamada.masahiro@socionext.com Tested-on: zc702 Tested-by: Michal Simek michal.simek@xilinx.com ---
Changes in v3: - Rebase to master
Changes in v2: None
configs/zynq_zc702_defconfig | 1 - 1 file changed, 1 deletion(-)
diff --git a/configs/zynq_zc702_defconfig b/configs/zynq_zc702_defconfig index 8a388f3..0abb7a8 100644 --- a/configs/zynq_zc702_defconfig +++ b/configs/zynq_zc702_defconfig @@ -1,6 +1,5 @@ CONFIG_ARM=y CONFIG_ARCH_ZYNQ=y -# CONFIG_SYS_MALLOC_F is not set CONFIG_DEFAULT_DEVICE_TREE="zynq-zc702" CONFIG_SPL=y CONFIG_FIT=y
participants (2)
-
Albert ARIBAUD -
Simon Glass