[U-Boot] [PATCH v3 0/8] arm: Tidy up early init

[U-Boot] [PATCH v3 0/8] arm: Tidy up early init

[Simon Glass]

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

-- 
2.6.0.rc2.230.g3dd15c0

[U-Boot] [PATCH v3 1/8] Move board_init_f_mem() into a common location

[Simon Glass]

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/
-- 
2.6.0.rc2.230.g3dd15c0

[U-Boot] [PATCH v3 2/8] board_init_f_mem(): Don't require memset()

[Simon Glass]

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)
-- 
2.6.0.rc2.230.g3dd15c0

[U-Boot] [PATCH v3 3/8] board_init_f_mem(): Don't create an unused early malloc() area

[Simon Glass]

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
-- 
2.6.0.rc2.230.g3dd15c0

[U-Boot] [PATCH v3 4/8] arm: Switch aarch64 to using generic global_data setup

[Simon Glass]

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
 
-- 
2.6.0.rc2.230.g3dd15c0

[U-Boot] [PATCH v3 5/8] arm: Switch 32-bit ARM to using generic global_data setup

[Simon Glass]

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)
-- 
2.6.0.rc2.230.g3dd15c0

[U-Boot] [PATCH v3 6/8] microblaze: Add a TODO to call board_init_f_mem()

[Simon Glass]

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
-- 
2.6.0.rc2.230.g3dd15c0

[U-Boot] [PATCH v3 7/8] zynq: Move SPL console init out of board_init_f()

[Simon Glass]

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
-- 
2.6.0.rc2.230.g3dd15c0

[U-Boot] [PATCH v3 8/8] Revert "ARM: zynq: disable CONFIG_SYS_MALLOC_F to fix MMC boot"

[Simon Glass]

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
-- 
2.6.0.rc2.230.g3dd15c0

[U-Boot] [PATCH v3 2/8] board_init_f_mem(): Don't require memset()

[Albert ARIBAUD]

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,
-- 
Albert.

[U-Boot] [PATCH v3 7/8] zynq: Move SPL console init out of board_init_f()

[Albert ARIBAUD]

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,
-- 
Albert.

[U-Boot] [PATCH v3 7/8] zynq: Move SPL console init out of board_init_f()

[Simon Glass]

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

[U-Boot] [PATCH v3 2/8] board_init_f_mem(): Don't require memset()

[Simon Glass]

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

[U-Boot] [PATCH v3 7/8] zynq: Move SPL console init out of board_init_f()

[Albert ARIBAUD]

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,
-- 
Albert.

[U-Boot] [PATCH v3 2/8] board_init_f_mem(): Don't require memset()

[Albert ARIBAUD]

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,
-- 
Albert.