[U-Boot] [PATCH 0/10] Make more use of board_init_f_mem()

[U-Boot] [PATCH 0/10] Make more use of board_init_f_mem()

[Simon Glass]

Adding the code to support early malloc and global data setup to every
arch's assembler start-up is a pain. Also this code is not actually
architecture-specific. We can use common code for all architectures and
with a bit of care we can write this code in C.

This code is now in board_init_f_mem() but is only used on a few archs.
This series expand the use of this function to ARM and adds TODOs for
a few other archs. It als fixes a few problems that have come up, e.g.
on Zynq.

Later work will enable use of this function on x86 also.


Simon Glass (10):
  Move board_init_f_mem() into a common location
  board_init_f_mem(): Don't require memset()
  board_init_f_mem(): Round down for stack alignment
  board_init_f_mem(): Don't create an unused early malloc() area
  arm: Adjust start-up code to use board_init_f_mem()
  arm64: Adjust start-up code to use board_init_f_mem()
  x86: Add a TODO to call board_init_f_mem()
  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"

 Makefile                           |  1 +
 arch/arm/cpu/armv7/zynq/spl.c      |  2 +-
 arch/arm/lib/crt0.S                | 20 ++++--------------
 arch/arm/lib/crt0_64.S             | 19 ++++++-----------
 arch/microblaze/cpu/start.S        |  2 ++
 arch/x86/cpu/start.S               |  1 +
 common/board_f.c                   | 22 +-------------------
 common/init/Makefile               |  7 +++++++
 common/init/global_data.c          | 42 ++++++++++++++++++++++++++++++++++++++
 configs/zynq_microzed_defconfig    |  1 -
 configs/zynq_zc70x_defconfig       |  1 -
 configs/zynq_zc770_xm010_defconfig |  1 -
 configs/zynq_zc770_xm012_defconfig |  1 -
 configs/zynq_zc770_xm013_defconfig |  1 -
 configs/zynq_zed_defconfig         |  1 -
 configs/zynq_zybo_defconfig        |  1 -
 scripts/Makefile.spl               |  1 +
 17 files changed, 66 insertions(+), 58 deletions(-)
 create mode 100644 common/init/Makefile
 create mode 100644 common/init/global_data.c

-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 01/10] 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.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 Makefile                  |  1 +
 common/board_f.c          | 22 +---------------------
 common/init/Makefile      |  7 +++++++
 common/init/global_data.c | 33 +++++++++++++++++++++++++++++++++
 scripts/Makefile.spl      |  1 +
 5 files changed, 43 insertions(+), 21 deletions(-)
 create mode 100644 common/init/Makefile
 create mode 100644 common/init/global_data.c

diff --git a/Makefile b/Makefile
index 343f416..7f6af72 100644
--- a/Makefile
+++ b/Makefile
@@ -659,6 +659,7 @@ libs-y += drivers/usb/musb-new/
 libs-y += drivers/usb/phy/
 libs-y += drivers/usb/ulpi/
 libs-y += common/
+libs-y += common/init/
 libs-$(CONFIG_API) += api/
 libs-$(CONFIG_HAS_POST) += post/
 libs-y += test/
diff --git a/common/board_f.c b/common/board_f.c
index 775df14..e6cec30 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -1087,24 +1087,4 @@ void board_init_f_r(void)
 	/* NOTREACHED - board_init_r() does not return */
 	hang();
 }
-#endif /* CONFIG_X86 */
-
-#ifndef CONFIG_X86
-ulong board_init_f_mem(ulong top)
-{
-	/* Leave space for the stack we are running with now */
-	top -= 0x40;
-
-	top -= sizeof(struct global_data);
-	top = ALIGN(top, 16);
-	gd = (struct global_data *)top;
-	memset((void *)gd, '\0', sizeof(*gd));
-
-#ifdef CONFIG_SYS_MALLOC_F_LEN
-	top -= CONFIG_SYS_MALLOC_F_LEN;
-	gd->malloc_base = top;
-#endif
-
-	return top;
-}
-#endif /* !CONFIG_X86 */
+#endif /* CONFIG_X86 || CONFIG_ARC */
diff --git a/common/init/Makefile b/common/init/Makefile
new file mode 100644
index 0000000..fadcc61
--- /dev/null
+++ b/common/init/Makefile
@@ -0,0 +1,7 @@
+#
+# (C) Copyright 2015 Google, Inc
+#
+# SPDX-License-Identifier:	GPL-2.0+
+#
+
+obj-y += global_data.o
diff --git a/common/init/global_data.c b/common/init/global_data.c
new file mode 100644
index 0000000..2633f0d
--- /dev/null
+++ b/common/init/global_data.c
@@ -0,0 +1,33 @@
+/*
+ * 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;
+
+ulong board_init_f_mem(ulong top)
+{
+	/* TODO(sjg at chromium.org): Figure out how x86 can use this */
+#ifndef CONFIG_X86
+	/* Leave space for the stack we are running with now */
+	top -= 0x40;
+
+	top -= sizeof(struct global_data);
+	top = ALIGN(top, 16);
+	gd = (struct global_data *)top;
+	memset((void *)gd, '\0', sizeof(*gd));
+
+#ifdef CONFIG_SYS_MALLOC_F_LEN
+	top -= CONFIG_SYS_MALLOC_F_LEN;
+	gd->malloc_base = top;
+#endif
+#endif
+
+	return top;
+}
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index fcacb7f..60042ea 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -51,6 +51,7 @@ HAVE_VENDOR_COMMON_LIB = $(if $(wildcard $(srctree)/board/$(VENDOR)/common/Makef
 libs-y += $(if $(BOARDDIR),board/$(BOARDDIR)/)
 libs-$(HAVE_VENDOR_COMMON_LIB) += board/$(VENDOR)/common/
 
+libs-y += common/init/
 libs-$(CONFIG_SPL_FRAMEWORK) += common/spl/
 libs-$(CONFIG_SPL_LIBCOMMON_SUPPORT) += common/
 libs-$(CONFIG_SPL_LIBDISK_SUPPORT) += disk/
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 02/10] 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>
---

 common/init/global_data.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/common/init/global_data.c b/common/init/global_data.c
index 2633f0d..ef055c4 100644
--- a/common/init/global_data.c
+++ b/common/init/global_data.c
@@ -21,7 +21,15 @@ ulong board_init_f_mem(ulong top)
 	top -= sizeof(struct global_data);
 	top = ALIGN(top, 16);
 	gd = (struct global_data *)top;
+#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBGENERIC_SUPPORT)
 	memset((void *)gd, '\0', sizeof(*gd));
+#else
+	int *ptr = (int *)gd;
+	int *end = (int *)(gd + 1);
+
+	while (ptr < end)
+		*ptr++ = 0;
+#endif
 
 #ifdef CONFIG_SYS_MALLOC_F_LEN
 	top -= CONFIG_SYS_MALLOC_F_LEN;
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 03/10] board_init_f_mem(): Round down for stack alignment

[Simon Glass]

We have plenty of stack space for this little function, but just to be safe,
round the stack pointer down instead of up. This may also reduce confusion
for later readers of the code.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/init/global_data.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/init/global_data.c b/common/init/global_data.c
index ef055c4..d2d7baa 100644
--- a/common/init/global_data.c
+++ b/common/init/global_data.c
@@ -19,7 +19,7 @@ ulong board_init_f_mem(ulong top)
 	top -= 0x40;
 
 	top -= sizeof(struct global_data);
-	top = ALIGN(top, 16);
+	top = round_down(top, 16);
 	gd = (struct global_data *)top;
 #if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBGENERIC_SUPPORT)
 	memset((void *)gd, '\0', sizeof(*gd));
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 04/10] 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>
---

 common/init/global_data.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/common/init/global_data.c b/common/init/global_data.c
index d2d7baa..dd91e98 100644
--- a/common/init/global_data.c
+++ b/common/init/global_data.c
@@ -31,7 +31,8 @@ ulong board_init_f_mem(ulong top)
 		*ptr++ = 0;
 #endif
 
-#ifdef CONFIG_SYS_MALLOC_F_LEN
+#if defined(CONFIG_SYS_MALLOC_F_LEN) && \
+	(!defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SYS_SPL_MALLOC_START))
 	top -= CONFIG_SYS_MALLOC_F_LEN;
 	gd->malloc_base = top;
 #endif
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 05/10] arm: Adjust start-up code to use board_init_f_mem()

[Simon Glass]

We can use this C function to do the early memory layout and init.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/arm/lib/crt0.S | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
index 92d3732..871e2b0 100644
--- a/arch/arm/lib/crt0.S
+++ b/arch/arm/lib/crt0.S
@@ -66,23 +66,11 @@ ENTRY(_main)
 #else
 	ldr	sp, =(CONFIG_SYS_INIT_SP_ADDR)
 #endif
-	bic	sp, sp, #7	/* 8-byte alignment for ABI compliance */
-	mov	r2, sp
-	sub	sp, sp, #GD_SIZE	/* allocate one GD above SP */
-	bic	sp, sp, #7	/* 8-byte alignment for ABI compliance */
-	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 */
-	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.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 06/10] arm64: Adjust start-up code to use board_init_f_mem()

[Simon Glass]

We can use this C function to do the early memory layout and init.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/arm/lib/crt0_64.S | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/arch/arm/lib/crt0_64.S b/arch/arm/lib/crt0_64.S
index 1654011..33432f2 100644
--- a/arch/arm/lib/crt0_64.S
+++ b/arch/arm/lib/crt0_64.S
@@ -61,19 +61,12 @@ ENTRY(_main)
 /*
  * Set up initial C runtime environment and call board_init_f(0).
  */
-	ldr	x0, =(CONFIG_SYS_INIT_SP_ADDR)
-	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)
-	sub	x0, x18, #CONFIG_SYS_MALLOC_F_LEN
-	str	x0, [x18, #GD_MALLOC_BASE]
-#endif
-	bic	sp, x0, #0xf	/* 16-byte alignment for ABI compliance */
+	ldr	sp, =(CONFIG_SYS_INIT_SP_ADDR)
+
+	mov	x0, sp
+	bl	board_init_f_mem
+
+	mov	sp, x0
 	mov	x0, #0
 	bl	board_init_f
 
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 07/10] x86: 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. For
now, add a TODO, as it involves some trickery.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/x86/cpu/start.S | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/cpu/start.S b/arch/x86/cpu/start.S
index 2e5f9da..82fdba7 100644
--- a/arch/x86/cpu/start.S
+++ b/arch/x86/cpu/start.S
@@ -132,6 +132,7 @@ car_init_ret:
 	andl	$0xfffffff0, %esp
 	movl	%esp, %ecx
 
+	/* TODO: Redo this code to call board_init_f_mem() */
 #if defined(CONFIG_SYS_MALLOC_F_LEN)
 	subl	$CONFIG_SYS_MALLOC_F_LEN, %esp
 	movl	%eax, %edx
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 08/10] 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>
---

 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.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 09/10] 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>
---

 arch/arm/cpu/armv7/zynq/spl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/cpu/armv7/zynq/spl.c b/arch/arm/cpu/armv7/zynq/spl.c
index b80c357..19e1365 100644
--- a/arch/arm/cpu/armv7/zynq/spl.c
+++ b/arch/arm/cpu/armv7/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.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 10/10] 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>
---

 configs/zynq_microzed_defconfig    | 1 -
 configs/zynq_zc70x_defconfig       | 1 -
 configs/zynq_zc770_xm010_defconfig | 1 -
 configs/zynq_zc770_xm012_defconfig | 1 -
 configs/zynq_zc770_xm013_defconfig | 1 -
 configs/zynq_zed_defconfig         | 1 -
 configs/zynq_zybo_defconfig        | 1 -
 7 files changed, 7 deletions(-)

diff --git a/configs/zynq_microzed_defconfig b/configs/zynq_microzed_defconfig
index 95cfe89..f5c1dff 100644
--- a/configs/zynq_microzed_defconfig
+++ b/configs/zynq_microzed_defconfig
@@ -3,7 +3,6 @@ CONFIG_ARM=y
 CONFIG_ZYNQ=y
 CONFIG_TARGET_ZYNQ_MICROZED=y
 CONFIG_OF_CONTROL=y
-# CONFIG_SYS_MALLOC_F is not set
 CONFIG_FIT=y
 CONFIG_FIT_VERBOSE=y
 CONFIG_FIT_SIGNATURE=y
diff --git a/configs/zynq_zc70x_defconfig b/configs/zynq_zc70x_defconfig
index 81fb4af..a599bb1 100644
--- a/configs/zynq_zc70x_defconfig
+++ b/configs/zynq_zc70x_defconfig
@@ -4,7 +4,6 @@ CONFIG_ZYNQ=y
 CONFIG_TARGET_ZYNQ_ZC70X=y
 CONFIG_OF_CONTROL=y
 CONFIG_DEFAULT_DEVICE_TREE="zynq-zc702"
-# CONFIG_SYS_MALLOC_F is not set
 CONFIG_FIT=y
 CONFIG_FIT_VERBOSE=y
 CONFIG_FIT_SIGNATURE=y
diff --git a/configs/zynq_zc770_xm010_defconfig b/configs/zynq_zc770_xm010_defconfig
index fc39cca..92bc76e 100644
--- a/configs/zynq_zc770_xm010_defconfig
+++ b/configs/zynq_zc770_xm010_defconfig
@@ -5,7 +5,6 @@ CONFIG_ZYNQ=y
 CONFIG_TARGET_ZYNQ_ZC770=y
 CONFIG_OF_CONTROL=y
 CONFIG_DEFAULT_DEVICE_TREE="zynq-zc770-xm010"
-# CONFIG_SYS_MALLOC_F is not set
 CONFIG_FIT=y
 CONFIG_FIT_VERBOSE=y
 CONFIG_FIT_SIGNATURE=y
diff --git a/configs/zynq_zc770_xm012_defconfig b/configs/zynq_zc770_xm012_defconfig
index 21e52fb..93d32a4 100644
--- a/configs/zynq_zc770_xm012_defconfig
+++ b/configs/zynq_zc770_xm012_defconfig
@@ -5,7 +5,6 @@ CONFIG_ZYNQ=y
 CONFIG_TARGET_ZYNQ_ZC770=y
 CONFIG_OF_CONTROL=y
 CONFIG_DEFAULT_DEVICE_TREE="zynq-zc770-xm012"
-# CONFIG_SYS_MALLOC_F is not set
 CONFIG_FIT=y
 CONFIG_FIT_VERBOSE=y
 CONFIG_FIT_SIGNATURE=y
diff --git a/configs/zynq_zc770_xm013_defconfig b/configs/zynq_zc770_xm013_defconfig
index 2c38012..9810cb4 100644
--- a/configs/zynq_zc770_xm013_defconfig
+++ b/configs/zynq_zc770_xm013_defconfig
@@ -5,7 +5,6 @@ CONFIG_ZYNQ=y
 CONFIG_TARGET_ZYNQ_ZC770=y
 CONFIG_OF_CONTROL=y
 CONFIG_DEFAULT_DEVICE_TREE="zynq-zc770-xm013"
-# CONFIG_SYS_MALLOC_F is not set
 CONFIG_FIT=y
 CONFIG_FIT_VERBOSE=y
 CONFIG_FIT_SIGNATURE=y
diff --git a/configs/zynq_zed_defconfig b/configs/zynq_zed_defconfig
index d4dc5bb..3200253 100644
--- a/configs/zynq_zed_defconfig
+++ b/configs/zynq_zed_defconfig
@@ -4,7 +4,6 @@ CONFIG_ZYNQ=y
 CONFIG_TARGET_ZYNQ_ZED=y
 CONFIG_OF_CONTROL=y
 CONFIG_DEFAULT_DEVICE_TREE="zynq-zed"
-# CONFIG_SYS_MALLOC_F is not set
 CONFIG_FIT=y
 CONFIG_FIT_VERBOSE=y
 CONFIG_FIT_SIGNATURE=y
diff --git a/configs/zynq_zybo_defconfig b/configs/zynq_zybo_defconfig
index 7d06073..aaa6cec 100644
--- a/configs/zynq_zybo_defconfig
+++ b/configs/zynq_zybo_defconfig
@@ -4,7 +4,6 @@ CONFIG_ZYNQ=y
 CONFIG_TARGET_ZYNQ_ZYBO=y
 CONFIG_OF_CONTROL=y
 CONFIG_DEFAULT_DEVICE_TREE="zynq-zybo"
-# CONFIG_SYS_MALLOC_F is not set
 CONFIG_FIT=y
 CONFIG_FIT_VERBOSE=y
 CONFIG_FIT_SIGNATURE=y
-- 
2.2.0.rc0.207.ga3a616c

[U-Boot] [PATCH 02/10] board_init_f_mem(): Don't require memset()

[Wolfgang Denk]

Dear Simon Glass,

In message <1429146849-11994-3-git-send-email-sjg@chromium.org> you wrote:
> Unfortunately memset() is not always available, so provide a substitute when
> needed.

> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBGENERIC_SUPPORT)
>  	memset((void *)gd, '\0', sizeof(*gd));
> +#else
> +	int *ptr = (int *)gd;
> +	int *end = (int *)(gd + 1);
> +
> +	while (ptr < end)
> +		*ptr++ = 0;
> +#endif

Please don't declare variables in the middle of the code.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Command, n.:
            Statement presented by a human and accepted by a computer
in such a manner as to make the human feel as if he is in control.

[U-Boot] [PATCH 01/10] Move board_init_f_mem() into a common location

[Jeroen Hofstee]

Hello Simon,

On 16-04-15 03:14, Simon Glass wrote:
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +ulong board_init_f_mem(ulong top)
> +{
> +	/* TODO(sjg at chromium.org): Figure out how x86 can use this */
> +#ifndef CONFIG_X86
> +	/* Leave space for the stack we are running with now */
> +	top -= 0x40;
> +
> +	top -= sizeof(struct global_data);
> +	top = ALIGN(top, 16);
> +	gd = (struct global_data *)top;
> +	memset((void *)gd, '\0', sizeof(*gd));
> +

Above piece of code is on my TODO list as well. Like x86, clang cannot
directly assign gd. What I still need to check is why this reassignment is
needed in the first place. Typically, at least for ARM, allocating an 
initial
gd in _main and copying it over in relocate suffices for common boards.

This doesn't work if there is a valid use case for needing gd before calling
main, but I am not aware of any (but haven't found time to google for any
as well, so it doesn't mean there isn't any).

That said, if there is valid reason to reassign gd, clang could do that 
if there
was a macro e.g. set_gd(new_gd) instead of a direct assignment. Since 
this is a
cross arch patchset, that might be something to consider (and likely 
solves the
"Figure out how x86 can use this" as well).

Regards,
Jeroen

[U-Boot] [PATCH 01/10] Move board_init_f_mem() into a common location

[Masahiro Yamada]

Hi Simon,


2015-04-16 10:14 GMT+09:00 Simon Glass <sjg@chromium.org>:
> This function will be used by both SPL and U-Boot proper. So move it into
> a common place.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  Makefile                  |  1 +
>  common/board_f.c          | 22 +---------------------
>  common/init/Makefile      |  7 +++++++
>  common/init/global_data.c | 33 +++++++++++++++++++++++++++++++++
>  scripts/Makefile.spl      |  1 +
>  5 files changed, 43 insertions(+), 21 deletions(-)
>  create mode 100644 common/init/Makefile
>  create mode 100644 common/init/global_data.c
>
> diff --git a/Makefile b/Makefile
> index 343f416..7f6af72 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -659,6 +659,7 @@ libs-y += drivers/usb/musb-new/
>  libs-y += drivers/usb/phy/
>  libs-y += drivers/usb/ulpi/
>  libs-y += common/
> +libs-y += common/init/
>  libs-$(CONFIG_API) += api/
>  libs-$(CONFIG_HAS_POST) += post/
>  libs-y += test/

[snip]

> diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
> index fcacb7f..60042ea 100644
> --- a/scripts/Makefile.spl
> +++ b/scripts/Makefile.spl
> @@ -51,6 +51,7 @@ HAVE_VENDOR_COMMON_LIB = $(if $(wildcard $(srctree)/board/$(VENDOR)/common/Makef
>  libs-y += $(if $(BOARDDIR),board/$(BOARDDIR)/)
>  libs-$(HAVE_VENDOR_COMMON_LIB) += board/$(VENDOR)/common/
>
> +libs-y += common/init/
>  libs-$(CONFIG_SPL_FRAMEWORK) += common/spl/
>  libs-$(CONFIG_SPL_LIBCOMMON_SUPPORT) += common/
>  libs-$(CONFIG_SPL_LIBDISK_SUPPORT) += disk/


Please do not increase the top-level entry any more.

How about adding "obj-y += init/" into common/Makefile ?




-- 
Best Regards
Masahiro Yamada

[U-Boot] [PATCH 01/10] Move board_init_f_mem() into a common location

[Simon Glass]

Hi Masahiro,

On 17 April 2015 at 10:37, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> Hi Simon,
>
>
> 2015-04-16 10:14 GMT+09:00 Simon Glass <sjg@chromium.org>:
>> This function will be used by both SPL and U-Boot proper. So move it into
>> a common place.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>>  Makefile                  |  1 +
>>  common/board_f.c          | 22 +---------------------
>>  common/init/Makefile      |  7 +++++++
>>  common/init/global_data.c | 33 +++++++++++++++++++++++++++++++++
>>  scripts/Makefile.spl      |  1 +
>>  5 files changed, 43 insertions(+), 21 deletions(-)
>>  create mode 100644 common/init/Makefile
>>  create mode 100644 common/init/global_data.c
>>
>> diff --git a/Makefile b/Makefile
>> index 343f416..7f6af72 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -659,6 +659,7 @@ libs-y += drivers/usb/musb-new/
>>  libs-y += drivers/usb/phy/
>>  libs-y += drivers/usb/ulpi/
>>  libs-y += common/
>> +libs-y += common/init/
>>  libs-$(CONFIG_API) += api/
>>  libs-$(CONFIG_HAS_POST) += post/
>>  libs-y += test/
>
> [snip]
>
>> diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
>> index fcacb7f..60042ea 100644
>> --- a/scripts/Makefile.spl
>> +++ b/scripts/Makefile.spl
>> @@ -51,6 +51,7 @@ HAVE_VENDOR_COMMON_LIB = $(if $(wildcard $(srctree)/board/$(VENDOR)/common/Makef
>>  libs-y += $(if $(BOARDDIR),board/$(BOARDDIR)/)
>>  libs-$(HAVE_VENDOR_COMMON_LIB) += board/$(VENDOR)/common/
>>
>> +libs-y += common/init/
>>  libs-$(CONFIG_SPL_FRAMEWORK) += common/spl/
>>  libs-$(CONFIG_SPL_LIBCOMMON_SUPPORT) += common/
>>  libs-$(CONFIG_SPL_LIBDISK_SUPPORT) += disk/
>
>
> Please do not increase the top-level entry any more.
>
> How about adding "obj-y += init/" into common/Makefile ?

The problem I had was that common/ is not included for SPL unless you
enable CONFIG_SPL_LIBCOMMON_SUPPORT. Is there another way?

Regards,
Simon

[U-Boot] [PATCH 01/10] Move board_init_f_mem() into a common location

[Simon Glass]

Hi Jeroen,

On 16 April 2015 at 03:32, Jeroen Hofstee <jeroen@myspectrum.nl> wrote:
> Hello Simon,
>
> On 16-04-15 03:14, Simon Glass wrote:
>>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +ulong board_init_f_mem(ulong top)
>> +{
>> +       /* TODO(sjg at chromium.org): Figure out how x86 can use this */
>> +#ifndef CONFIG_X86
>> +       /* Leave space for the stack we are running with now */
>> +       top -= 0x40;
>> +
>> +       top -= sizeof(struct global_data);
>> +       top = ALIGN(top, 16);
>> +       gd = (struct global_data *)top;
>> +       memset((void *)gd, '\0', sizeof(*gd));
>> +
>
>
> Above piece of code is on my TODO list as well. Like x86, clang cannot
> directly assign gd. What I still need to check is why this reassignment is
> needed in the first place. Typically, at least for ARM, allocating an
> initial
> gd in _main and copying it over in relocate suffices for common boards.
>
> This doesn't work if there is a valid use case for needing gd before calling
> main, but I am not aware of any (but haven't found time to google for any
> as well, so it doesn't mean there isn't any).
>
> That said, if there is valid reason to reassign gd, clang could do that if
> there
> was a macro e.g. set_gd(new_gd) instead of a direct assignment. Since this
> is a
> cross arch patchset, that might be something to consider (and likely solves
> the
> "Figure out how x86 can use this" as well).

Yes. I'm fiddling with x86 again so may figure something out here for v2.

Regards,
Simon

[U-Boot] [PATCH 02/10] board_init_f_mem(): Don't require memset()

[Simon Glass]

Hi Wolfgang,

On 15 April 2015 at 22:25, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon Glass,
>
> In message <1429146849-11994-3-git-send-email-sjg@chromium.org> you wrote:
>> Unfortunately memset() is not always available, so provide a substitute when
>> needed.
>
>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBGENERIC_SUPPORT)
>>       memset((void *)gd, '\0', sizeof(*gd));
>> +#else
>> +     int *ptr = (int *)gd;
>> +     int *end = (int *)(gd + 1);
>> +
>> +     while (ptr < end)
>> +             *ptr++ = 0;
>> +#endif
>
> Please don't declare variables in the middle of the code.

OK I'll tidy that up.

Regards,
Simon

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

[Masahiro Yamada]

Hi Simon,


2015-04-16 10:14 GMT+09:00 Simon Glass <sjg@chromium.org>:
> This reverts commit 321f86e18d6aae9f7b7ba3ef1eb0cec769481874.
>
> The original bug has been fixed.
> Signed-off-by: Simon Glass <sjg@chromium.org>


I confirmed this on Zedboard and ZC706 board. Thanks!

Tested-by: Masahiro Yamada <yamada.masahiro@socionext.com>


BTW, please insert a blank line between git-description and Signed-off-by.


-- 
Best Regards
Masahiro Yamada

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

[Masahiro Yamada]

2015-04-16 10:14 GMT+09:00 Simon Glass <sjg@chromium.org>:
> 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>
> ---
>
>  common/init/global_data.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/common/init/global_data.c b/common/init/global_data.c
> index d2d7baa..dd91e98 100644
> --- a/common/init/global_data.c
> +++ b/common/init/global_data.c
> @@ -31,7 +31,8 @@ ulong board_init_f_mem(ulong top)
>                 *ptr++ = 0;
>  #endif
>
> -#ifdef CONFIG_SYS_MALLOC_F_LEN
> +#if defined(CONFIG_SYS_MALLOC_F_LEN) && \
> +       (!defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SYS_SPL_MALLOC_START))
>         top -= CONFIG_SYS_MALLOC_F_LEN;
>         gd->malloc_base = top;
>  #endif


This is a critical one to fix the bug on Zynq.

Tested-by: Masahiro Yamada <yamada.masahiro@socionext.com>


BTW, we should do something with defined(CONFIG_SYS_MALLOC_F_LEN) later.
Note CONFIG_SYS_MALLOC_F_LEN is a hex type config.



-- 
Best Regards
Masahiro Yamada

[U-Boot] [PATCH 09/10] zynq: Move SPL console init out of board_init_f()

[Masahiro Yamada]

2015-04-16 10:14 GMT+09:00 Simon Glass <sjg@chromium.org>:
> 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>
> ---
>

I think this is unrelated to 10/10, but anyway looks good.

Tested-by: Masahiro Yamada <yamada.masahiro@socionext.com>



-- 
Best Regards
Masahiro Yamada

[U-Boot] [PATCH 02/10] board_init_f_mem(): Don't require memset()

[Masahiro Yamada]

Hi Simon,



2015-04-16 10:14 GMT+09:00 Simon Glass <sjg@chromium.org>:
> Unfortunately memset() is not always available, so provide a substitute when
> needed.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  common/init/global_data.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/common/init/global_data.c b/common/init/global_data.c
> index 2633f0d..ef055c4 100644
> --- a/common/init/global_data.c
> +++ b/common/init/global_data.c
> @@ -21,7 +21,15 @@ ulong board_init_f_mem(ulong top)
>         top -= sizeof(struct global_data);
>         top = ALIGN(top, 16);
>         gd = (struct global_data *)top;
> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBGENERIC_SUPPORT)
>         memset((void *)gd, '\0', sizeof(*gd));
> +#else
> +       int *ptr = (int *)gd;
> +       int *end = (int *)(gd + 1);
> +
> +       while (ptr < end)
> +               *ptr++ = 0;
> +#endif
>
>  #ifdef CONFIG_SYS_MALLOC_F_LEN
>         top -= CONFIG_SYS_MALLOC_F_LEN;


This patch implies that all the SPLs should have memset().

Is it better to build lib/ unconditionally?
I posted a patch to do so.

Please consider to use it as a prerequisite
for cleaning up 01/10  and 02/10.



-- 
Best Regards
Masahiro Yamada

[U-Boot] [PATCH 02/10] board_init_f_mem(): Don't require memset()

[Simon Glass]

Hi Masahiro,

On 20 April 2015 at 21:42, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> Hi Simon,
>
>
>
> 2015-04-16 10:14 GMT+09:00 Simon Glass <sjg@chromium.org>:
>> Unfortunately memset() is not always available, so provide a substitute when
>> needed.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>>  common/init/global_data.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/common/init/global_data.c b/common/init/global_data.c
>> index 2633f0d..ef055c4 100644
>> --- a/common/init/global_data.c
>> +++ b/common/init/global_data.c
>> @@ -21,7 +21,15 @@ ulong board_init_f_mem(ulong top)
>>         top -= sizeof(struct global_data);
>>         top = ALIGN(top, 16);
>>         gd = (struct global_data *)top;
>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBGENERIC_SUPPORT)
>>         memset((void *)gd, '\0', sizeof(*gd));
>> +#else
>> +       int *ptr = (int *)gd;
>> +       int *end = (int *)(gd + 1);
>> +
>> +       while (ptr < end)
>> +               *ptr++ = 0;
>> +#endif
>>
>>  #ifdef CONFIG_SYS_MALLOC_F_LEN
>>         top -= CONFIG_SYS_MALLOC_F_LEN;
>
>
> This patch implies that all the SPLs should have memset().
>
> Is it better to build lib/ unconditionally?
> I posted a patch to do so.
>
> Please consider to use it as a prerequisite
> for cleaning up 01/10  and 02/10.

That would be better I think - how did you manage it? I cannot see the
patch you are referring to. Although what about if SPL is very close
to the maximum size and adding memset() makes it too large? I suppose
in that case we would get a build error and notice the problem?
Regards,
Simon

[U-Boot] [PATCH 02/10] board_init_f_mem(): Don't require memset()

[Masahiro Yamada]

2015-04-21 12:47 GMT+09:00 Simon Glass <sjg@chromium.org>:
> Hi Masahiro,
>
> On 20 April 2015 at 21:42, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>> Hi Simon,
>>
>>
>>
>> 2015-04-16 10:14 GMT+09:00 Simon Glass <sjg@chromium.org>:
>>> Unfortunately memset() is not always available, so provide a substitute when
>>> needed.
>>>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>> ---
>>>
>>>  common/init/global_data.c | 8 ++++++++
>>>  1 file changed, 8 insertions(+)
>>>
>>> diff --git a/common/init/global_data.c b/common/init/global_data.c
>>> index 2633f0d..ef055c4 100644
>>> --- a/common/init/global_data.c
>>> +++ b/common/init/global_data.c
>>> @@ -21,7 +21,15 @@ ulong board_init_f_mem(ulong top)
>>>         top -= sizeof(struct global_data);
>>>         top = ALIGN(top, 16);
>>>         gd = (struct global_data *)top;
>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBGENERIC_SUPPORT)
>>>         memset((void *)gd, '\0', sizeof(*gd));
>>> +#else
>>> +       int *ptr = (int *)gd;
>>> +       int *end = (int *)(gd + 1);
>>> +
>>> +       while (ptr < end)
>>> +               *ptr++ = 0;
>>> +#endif
>>>
>>>  #ifdef CONFIG_SYS_MALLOC_F_LEN
>>>         top -= CONFIG_SYS_MALLOC_F_LEN;
>>
>>
>> This patch implies that all the SPLs should have memset().
>>
>> Is it better to build lib/ unconditionally?
>> I posted a patch to do so.
>>
>> Please consider to use it as a prerequisite
>> for cleaning up 01/10  and 02/10.
>
> That would be better I think - how did you manage it? I cannot see the
> patch you are referring to.

It is under moderation because of too many recipients.
(I think you have already received it because you were listed in CC.)

Please wait until it is approved.


> Although what about if SPL is very close
> to the maximum size and adding memset() makes it too large? I suppose
> in that case we would get a build error and notice the problem?

Buildman-test passed, but I am not sure about run-test.

For those boards that define CONFIG_SPL_MAX_SIZE,
CONFIG_SPL_MAX_FOOTPRINT etc., we should notice the problem at the
build time.  (and it did not occur.)

I'd like to encourage the board maintainers to do run-test just in case.
(and also to support such CONFIG options for boards with the limited
memory footprint)



-- 
Best Regards
Masahiro Yamada

[U-Boot] [PATCH 02/10] board_init_f_mem(): Don't require memset()

[Simon Glass]

Hi Masahiro,

On 20 April 2015 at 22:30, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> 2015-04-21 12:47 GMT+09:00 Simon Glass <sjg@chromium.org>:
>> Hi Masahiro,
>>
>> On 20 April 2015 at 21:42, Masahiro Yamada
>> <yamada.masahiro@socionext.com> wrote:
>>> Hi Simon,
>>>
>>>
>>>
>>> 2015-04-16 10:14 GMT+09:00 Simon Glass <sjg@chromium.org>:
>>>> Unfortunately memset() is not always available, so provide a substitute when
>>>> needed.
>>>>
>>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>>> ---
>>>>
>>>>  common/init/global_data.c | 8 ++++++++
>>>>  1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/common/init/global_data.c b/common/init/global_data.c
>>>> index 2633f0d..ef055c4 100644
>>>> --- a/common/init/global_data.c
>>>> +++ b/common/init/global_data.c
>>>> @@ -21,7 +21,15 @@ ulong board_init_f_mem(ulong top)
>>>>         top -= sizeof(struct global_data);
>>>>         top = ALIGN(top, 16);
>>>>         gd = (struct global_data *)top;
>>>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBGENERIC_SUPPORT)
>>>>         memset((void *)gd, '\0', sizeof(*gd));
>>>> +#else
>>>> +       int *ptr = (int *)gd;
>>>> +       int *end = (int *)(gd + 1);
>>>> +
>>>> +       while (ptr < end)
>>>> +               *ptr++ = 0;
>>>> +#endif
>>>>
>>>>  #ifdef CONFIG_SYS_MALLOC_F_LEN
>>>>         top -= CONFIG_SYS_MALLOC_F_LEN;
>>>
>>>
>>> This patch implies that all the SPLs should have memset().
>>>
>>> Is it better to build lib/ unconditionally?
>>> I posted a patch to do so.
>>>
>>> Please consider to use it as a prerequisite
>>> for cleaning up 01/10  and 02/10.
>>
>> That would be better I think - how did you manage it? I cannot see the
>> patch you are referring to.
>
> It is under moderation because of too many recipients.
> (I think you have already received it because you were listed in CC.)
>
> Please wait until it is approved.
>
>
>> Although what about if SPL is very close
>> to the maximum size and adding memset() makes it too large? I suppose
>> in that case we would get a build error and notice the problem?
>
> Buildman-test passed, but I am not sure about run-test.
>
> For those boards that define CONFIG_SPL_MAX_SIZE,
> CONFIG_SPL_MAX_FOOTPRINT etc., we should notice the problem at the
> build time.  (and it did not occur.)
>
> I'd like to encourage the board maintainers to do run-test just in case.
> (and also to support such CONFIG options for boards with the limited
> memory footprint)

OK, I tried it out for code size and it looked fine. I will see if I
can test it on some boards, but I imagine it would be fine.

Regards,
Simon