Make more use of board_init_f_mem()

PATCH New Simon Glass 4 participants 24 messages Apr 16, 2015 – Apr 24, 2015

Subsystems (5)

Arm (32-bit) x86 Kbuild Arm/Zynq architecture +1 more subsystems

lore

Share & tools

lore

Simon Glass

author

2015-04-16 01:13 UTC #0 lore raw

cover Make more use of board_init_f_mem()

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

Signature · 1 line

2.2.0.rc0.207.ga3a616c

Simon Glass

author

2015-04-16 01:14 UTC #1 lore raw

PATCH 1/10 Move board_init_f_mem() into a common location

This function will be used by both SPL and U-Boot proper. So move it into a common place.

Signed-off-by Simon Glass <[email protected]>

---

 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

Makefile +1 -0

index 343f416..7f6af72 100644
@@ -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/

common/board_f.c +1 ~~-21~~

index 775df14..e6cec30 100644
@@ -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 */

common/init/Makefile new file +7 -0

new file mode 100644
index 0000000..fadcc61
@@ -0,0 +1,7 @@
+#
+# (C) Copyright 2015 Google, Inc
+#
+# SPDX-License-Identifier:	GPL-2.0+
+#
+
+obj-y += global_data.o

common/init/global_data.c new file +33 -0

new file mode 100644
index 0000000..2633f0d
@@ -0,0 +1,33 @@
+/*
+ * Code shared between SPL and U-Boot proper
+ *
+ * Copyright (c) 2015 Google, Inc
+ * Written by Simon Glass <[email protected]>
+ *
+ * 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;
+}

scripts/Makefile.spl +1 -0

index fcacb7f..60042ea 100644
@@ -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/

Signature · 1 line

2.2.0.rc0.207.ga3a616c

Jeroen Hofstee

2015-04-16 09:32 UTC #2 lore raw

PATCH 1/10 Move board_init_f_mem() into a common location

Hello Simon,

Quoted 5 lines · Simon Glass

common/init/global_data.c new file +5 -0
+
+DECLARE_GLOBAL_DATA_PTR;
+
+ulong board_init_f_mem(ulong top)
+{

common/init/global_data.c line 25

Quoted 10 lines

common/init/global_data.c new file +10 -0

+	/* 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

Simon Glass

author

2015-04-17 22:56 UTC #3 lore raw

PATCH 1/10 Move board_init_f_mem() into a common location

Hi Jeroen,

Quoted 31 lines · Jeroen Hofstee

Hello Simon,
Quoted 6 lines · Simon Glass
common/init/global_data.c new file +5 -0
+
+DECLARE_GLOBAL_DATA_PTR;
+
+ulong board_init_f_mem(ulong top)
+{
common/init/global_data.c line 25
Quoted 10 lines
common/init/global_data.c new file +10 -0
+       /* 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).

Quoted 7 lines

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

Masahiro Yamada

2015-04-17 16:37 UTC #4 lore raw

PATCH 1/10 Move board_init_f_mem() into a common location

In reply to Simon Glass

Hi Simon,

2015-04-16 10:14 GMT+09:00 Simon Glass <[email protected]>:

Quoted 27 lines

This function will be used by both SPL and U-Boot proper. So move it into a common place.

Signed-off-by Simon Glass <[email protected]>

---

 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

Makefile +1 -0

index 343f416..7f6af72 100644
@@ -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]

scripts/Makefile.spl HAVE_VENDOR_COMMON_LIB = $(if $(wildcard $(srctree)/board/$(VENDOR)/common/Makef · line 57

Quoted 12 lines

scripts/Makefile.spl +1 -0

index fcacb7f..60042ea 100644
@@ -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 ?

Signature · 2 lines

Best Regards
Masahiro Yamada

Simon Glass

author

2015-04-17 22:55 UTC #5 lore raw

PATCH 1/10 Move board_init_f_mem() into a common location

Hi Masahiro,

Quoted 50 lines · Masahiro Yamada

Hi Simon,

2015-04-16 10:14 GMT+09:00 Simon Glass <[email protected]>:

Quoted 27 lines

This function will be used by both SPL and U-Boot proper. So move it into a common place.

Signed-off-by Simon Glass <[email protected]>

---

 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

Makefile +1 -0

index 343f416..7f6af72 100644
@@ -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]

scripts/Makefile.spl HAVE_VENDOR_COMMON_LIB = $(if $(wildcard $(srctree)/board/$(VENDOR)/common/Makef · line 57

Quoted 12 lines

scripts/Makefile.spl +1 -0

index fcacb7f..60042ea 100644
@@ -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.

Quoted 1 line

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

Simon Glass

author

2015-04-16 01:14 UTC #6 lore raw

PATCH 2/10 board_init_f_mem(): Don't require memset()

In reply to Simon Glass

Unfortunately memset() is not always available, so provide a substitute when needed.

Signed-off-by Simon Glass <[email protected]>

---

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

common/init/global_data.c +8 -0

index 2633f0d..ef055c4 100644
@@ -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;

Signature · 1 line

2.2.0.rc0.207.ga3a616c

Wolfgang Denk

2015-04-16 04:25 UTC #7 lore raw

PATCH 2/10 board_init_f_mem(): Don't require memset()

Dear Simon Glass,

common/init/global_data.c ulong board_init_f_mem(ulong top) · line 32

Quoted 12 lines · In message <[email protected]> you wrote:

Unfortunately memset() is not always available, so provide a substitute when needed.

common/init/global_data.c +8 -0

+#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

Signature · 6 lines

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.

Simon Glass

author

2015-04-19 14:22 UTC #8 lore raw

PATCH 2/10 board_init_f_mem(): Don't require memset()

Hi Wolfgang,

Quoted 6 lines · Wolfgang Denk

Dear Simon Glass,
Quoted 2 lines · In message <[email protected]> you wrote:
Unfortunately memset() is not always available, so provide a substitute when needed.

Quoted 11 lines

common/init/global_data.c ulong board_init_f_mem(ulong top) · line 32
Quoted 9 lines
common/init/global_data.c +8 -0
+#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

Masahiro Yamada

2015-04-21 03:42 UTC #9 lore raw

PATCH 2/10 board_init_f_mem(): Don't require memset()

In reply to Simon Glass

Hi Simon,

2015-04-16 10:14 GMT+09:00 Simon Glass <[email protected]>:

Quoted 19 lines

Unfortunately memset() is not always available, so provide a substitute when needed.

Signed-off-by Simon Glass <[email protected]>

---

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

common/init/global_data.c +1 -0

index 2633f0d..ef055c4 100644
@@ -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));

common/init/global_data.c ulong board_init_f_mem(ulong top) · line 35

Quoted 10 lines

common/init/global_data.c +7 -0

+#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.

Signature · 2 lines

Best Regards
Masahiro Yamada

Simon Glass

author

2015-04-21 03:47 UTC #10 lore raw

PATCH 2/10 board_init_f_mem(): Don't require memset()

Hi Masahiro,

Quoted 38 lines · Masahiro Yamada

Hi Simon,
2015-04-16 10:14 GMT+09:00 Simon Glass <[email protected]>:
Quoted 19 lines
Unfortunately memset() is not always available, so provide a substitute when needed.
Signed-off-by Simon Glass <[email protected]>
---
 common/init/global_data.c | 8 ++++++++
 1 file changed, 8 insertions(+)
common/init/global_data.c +1 -0
index 2633f0d..ef055c4 100644
@@ -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));
common/init/global_data.c ulong board_init_f_mem(ulong top) · line 35
Quoted 10 lines
common/init/global_data.c +7 -0
+#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().

Quoted 5 lines

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

Masahiro Yamada

2015-04-21 04:30 UTC #11 lore raw

PATCH 2/10 board_init_f_mem(): Don't require memset()

2015-04-21 12:47 GMT+09:00 Simon Glass <[email protected]>:

Quoted 42 lines

Hi Masahiro,
Quoted 38 lines · Masahiro Yamada
Hi Simon,
2015-04-16 10:14 GMT+09:00 Simon Glass <[email protected]>:
Quoted 19 lines
Unfortunately memset() is not always available, so provide a substitute when needed.
Signed-off-by Simon Glass <[email protected]>
---
 common/init/global_data.c | 8 ++++++++
 1 file changed, 8 insertions(+)
common/init/global_data.c +1 -0
index 2633f0d..ef055c4 100644
@@ -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));
common/init/global_data.c ulong board_init_f_mem(ulong top) · line 35
Quoted 10 lines
common/init/global_data.c +7 -0
+#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().

Quoted 8 lines

Quoted 5 lines
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.

Quoted 3 lines

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)

Signature · 2 lines

Best Regards
Masahiro Yamada

Simon Glass

author

2015-04-24 04:15 UTC #12 lore raw

PATCH 2/10 board_init_f_mem(): Don't require memset()

Hi Masahiro,

Quoted 64 lines · Masahiro Yamada

2015-04-21 12:47 GMT+09:00 Simon Glass <[email protected]>:
Quoted 42 lines
Hi Masahiro,
Quoted 38 lines · Masahiro Yamada
Hi Simon,
2015-04-16 10:14 GMT+09:00 Simon Glass <[email protected]>:
Quoted 19 lines
Unfortunately memset() is not always available, so provide a substitute when needed.
Signed-off-by Simon Glass <[email protected]>
---
 common/init/global_data.c | 8 ++++++++
 1 file changed, 8 insertions(+)
common/init/global_data.c +1 -0
index 2633f0d..ef055c4 100644
@@ -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));
common/init/global_data.c ulong board_init_f_mem(ulong top) · line 35
Quoted 10 lines
common/init/global_data.c +7 -0
+#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().
Quoted 8 lines
Quoted 5 lines
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.
Quoted 3 lines
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.

Quoted 7 lines

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

Simon Glass

author

2015-04-16 01:14 UTC #13 lore raw

PATCH 3/10 board_init_f_mem(): Round down for stack alignment

In reply to 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 <[email protected]>

---

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

common/init/global_data.c +1 -1

index ef055c4..d2d7baa 100644
@@ -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));

Signature · 1 line

2.2.0.rc0.207.ga3a616c

Simon Glass

author

2015-04-16 01:14 UTC #14 lore raw

PATCH 4/10 board_init_f_mem(): Don't create an unused early malloc() area

In reply to 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 <[email protected]>

---

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

common/init/global_data.c +2 -1

index d2d7baa..dd91e98 100644
@@ -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

Signature · 1 line

2.2.0.rc0.207.ga3a616c

Masahiro Yamada

2015-04-20 12:12 UTC #15 lore raw

PATCH 4/10 board_init_f_mem(): Don't create an unused early malloc() area

2015-04-16 10:14 GMT+09:00 Simon Glass <[email protected]>:

Quoted 15 lines

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 <[email protected]>
---
 common/init/global_data.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
common/init/global_data.c +0 -0
index d2d7baa..dd91e98 100644

common/init/global_data.c ulong board_init_f_mem(ulong top) · line 38

Quoted 9 lines

common/init/global_data.c +2 -1

@@ -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 <[email protected]>

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

Signature · 2 lines

Best Regards
Masahiro Yamada

Simon Glass

author

2015-04-16 01:14 UTC #16 lore raw

PATCH 5/10 arm: Adjust start-up code to use board_init_f_mem()

In reply to Simon Glass

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

Signed-off-by Simon Glass <[email protected]>

---

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

arch/arm/lib/crt0.S +4 ~~-16~~

index 92d3732..871e2b0 100644
@@ -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)

Signature · 1 line

2.2.0.rc0.207.ga3a616c

Simon Glass

author

2015-04-16 01:14 UTC #17 lore raw

PATCH 6/10 arm64: Adjust start-up code to use board_init_f_mem()

In reply to Simon Glass

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

Signed-off-by Simon Glass <[email protected]>

---

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

arch/arm/lib/crt0_64.S +6 ~~-13~~

index 1654011..33432f2 100644
@@ -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

Signature · 1 line

2.2.0.rc0.207.ga3a616c

Simon Glass

author

2015-04-16 01:14 UTC #18 lore raw

PATCH 7/10 x86: Add a TODO to call board_init_f_mem()

In reply to 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 <[email protected]>

---

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

arch/x86/cpu/start.S +1 -0

index 2e5f9da..82fdba7 100644
@@ -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

Signature · 1 line

2.2.0.rc0.207.ga3a616c

Simon Glass

author

2015-04-16 01:14 UTC #19 lore raw

PATCH 8/10 microblaze: Add a TODO to call board_init_f_mem()

In reply to 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 <[email protected]>

---

 arch/microblaze/cpu/start.S | 2 ++
 1 file changed, 2 insertions(+)

arch/microblaze/cpu/start.S +2 -0

index 953d3a1..14f46a8 100644
@@ -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

Signature · 1 line

2.2.0.rc0.207.ga3a616c

Simon Glass

author

2015-04-16 01:14 UTC #20 lore raw

PATCH 9/10 zynq: Move SPL console init out of board_init_f()

In reply to 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 <[email protected]>

---

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

arch/arm/cpu/armv7/zynq/spl.c +1 -1

index b80c357..19e1365 100644
@@ -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

Signature · 1 line

2.2.0.rc0.207.ga3a616c

Masahiro Yamada

2015-04-20 12:15 UTC #21 lore raw

PATCH 9/10 zynq: Move SPL console init out of board_init_f()

2015-04-16 10:14 GMT+09:00 Simon Glass <[email protected]>:

Quoted 7 lines

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 <[email protected]>
---

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

Tested-by Masahiro Yamada <[email protected]>

Signature · 2 lines

Best Regards
Masahiro Yamada

Simon Glass

author

2015-04-16 01:14 UTC #22 lore raw

PATCH 10/10 Revert "ARM: zynq: disable CONFIG_SYS_MALLOC_F to fix MMC boot"

In reply to Simon Glass

This reverts commit 321f86e18d6aae9f7b7ba3ef1eb0cec769481874.

The original bug has been fixed.

Signed-off-by Simon Glass <[email protected]>

---

 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(-)

configs/zynq_microzed_defconfig +0 -1

index 95cfe89..f5c1dff 100644
@@ -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

configs/zynq_zc70x_defconfig +0 -1

index 81fb4af..a599bb1 100644
@@ -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

configs/zynq_zc770_xm010_defconfig +0 -1

index fc39cca..92bc76e 100644
@@ -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

configs/zynq_zc770_xm012_defconfig +0 -1

index 21e52fb..93d32a4 100644
@@ -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

configs/zynq_zc770_xm013_defconfig +0 -1

index 2c38012..9810cb4 100644
@@ -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

configs/zynq_zed_defconfig +0 -1

index d4dc5bb..3200253 100644
@@ -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

configs/zynq_zybo_defconfig +0 -1

index 7d06073..aaa6cec 100644
@@ -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

Signature · 1 line

2.2.0.rc0.207.ga3a616c

Masahiro Yamada

2015-04-20 12:08 UTC #23 lore raw

PATCH 10/10 Revert "ARM: zynq: disable CONFIG_SYS_MALLOC_F to fix MMC boot"

Hi Simon,

2015-04-16 10:14 GMT+09:00 Simon Glass <[email protected]>:

Quoted 4 lines

This reverts commit 321f86e18d6aae9f7b7ba3ef1eb0cec769481874.
The original bug has been fixed.
Signed-off-by Simon Glass <[email protected]>

I confirmed this on Zedboard and ZC706 board. Thanks!

Tested-by Masahiro Yamada <[email protected]>

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

Signature · 2 lines

Best Regards
Masahiro Yamada