diff mbox

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

Message ID 1429146849-11994-2-git-send-email-sjg@chromium.org
State Deferred
Delegated to: Tom Rini
Headers show

Commit Message

Simon Glass April 16, 2015, 1:14 a.m. UTC 
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

Comments

Jeroen Hofstee April 16, 2015, 9:32 a.m. UTC | #1 
Hello Simon,

On 16-04-15 03:14, Simon Glass wrote:
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +ulong board_init_f_mem(ulong top)
> +{
> +	/* TODO(sjg@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
Masahiro Yamada April 17, 2015, 4:37 p.m. UTC | #2 
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 ?
Simon Glass April 17, 2015, 10:55 p.m. UTC | #3 
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
Simon Glass April 17, 2015, 10:56 p.m. UTC | #4 
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@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
diff mbox

Patch

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@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/