[U-Boot] [PATCH 0/3] env_embedded related fixes/improvements
Hello,
Here is a small patch series that fixes the build on SuperH when CONFIG_ENV_IS_NOWHERE is set (first patch) and then does some follow-up cleanups in common/env_embedded.c.
However, I would like to ask for a careful review to be made here, since the whole environment handling logic is still not clear to me.
For example, I don't quite get why common/Makefile looks like this:
obj-$(CONFIG_ENV_IS_IN_DATAFLASH) += env_dataflash.o obj-$(CONFIG_ENV_IS_IN_EEPROM) += env_eeprom.o extra-$(CONFIG_ENV_IS_EMBEDDED) += env_embedded.o obj-$(CONFIG_ENV_IS_IN_EEPROM) += env_embedded.o extra-$(CONFIG_ENV_IS_IN_FLASH) += env_embedded.o
Why when CONFIG_ENV_IS_IN_FLASH is used, env_embedded is built, but not integrated at link time ? Is it the responsibility of the linker script to include it ? Why is it done so ? Very few linker scripts in U-Boot actually include env_embedded.o.
Thanks,
Thomas
Thomas Petazzoni (3): arch/sh: don't bring common/env_embedded.o into the link common/env_embedded.c: drop support for CONFIG_SYS_USE_PPCENV common/env_embedded.c: rename PPCENV/PPCTEXT macros
arch/sh/cpu/u-boot.lds | 4 ---- common/env_embedded.c | 23 +++++++---------------- include/env_default.h | 2 +- 3 files changed, 8 insertions(+), 21 deletions(-)
The linker script for SuperH brings the .ppcenv and .ppcenvr section of common/env_embedded.o into the .text section. However, the .ppcenv section is only ever filled in by env_embedded.o when CONFIG_SYS_USE_PPCENV is defined, but no platforms in mainline U-Boot use this.
In addition, common/env_embedded.o is not always built (when you use CONFIG_ENV_IS_NOWHERE for example), which causes the following build failure:
Fixes:
LD u-boot /home/thomas/sh4aeb-linux-musl/bin/sh4aeb-linux-ld.bfd: cannot find common/env_embedded.o
We fix this by no longer adding the .ppcenv and .ppcenvr sections from common/env_embedded.o into the .text section.
Signed-off-by: Thomas Petazzoni thomas.petazzoni@free-electrons.com --- arch/sh/cpu/u-boot.lds | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/arch/sh/cpu/u-boot.lds b/arch/sh/cpu/u-boot.lds index f185b4d..2f2bdb5 100644 --- a/arch/sh/cpu/u-boot.lds +++ b/arch/sh/cpu/u-boot.lds @@ -38,10 +38,6 @@ SECTIONS KEEP(CONFIG_BOARDDIR/lowlevel_init.o (.text .spiboot1.text)) KEEP(*(.spiboot2.text)) . = ALIGN(8192); - common/env_embedded.o (.ppcenv) - . = ALIGN(8192); - common/env_embedded.o (.ppcenvr) - . = ALIGN(8192); *(.text) . = ALIGN(4); } >ram =0xFF
Hi,
2017-07-29 6:46 GMT+09:00 Thomas Petazzoni thomas.petazzoni@free-electrons.com:
The linker script for SuperH brings the .ppcenv and .ppcenvr section of common/env_embedded.o into the .text section. However, the .ppcenv section is only ever filled in by env_embedded.o when CONFIG_SYS_USE_PPCENV is defined, but no platforms in mainline U-Boot use this.
In addition, common/env_embedded.o is not always built (when you use CONFIG_ENV_IS_NOWHERE for example), which causes the following build failure:
Fixes:
LD u-boot /home/thomas/sh4aeb-linux-musl/bin/sh4aeb-linux-ld.bfd: cannot find common/env_embedded.o
We fix this by no longer adding the .ppcenv and .ppcenvr sections from common/env_embedded.o into the .text section.
Signed-off-by: Thomas Petazzoni thomas.petazzoni@free-electrons.com
Acked-by: Nobuhiro Iwamatsu iwamatsu@nigauri.org
arch/sh/cpu/u-boot.lds | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/arch/sh/cpu/u-boot.lds b/arch/sh/cpu/u-boot.lds index f185b4d..2f2bdb5 100644 --- a/arch/sh/cpu/u-boot.lds +++ b/arch/sh/cpu/u-boot.lds @@ -38,10 +38,6 @@ SECTIONS KEEP(CONFIG_BOARDDIR/lowlevel_init.o (.text .spiboot1.text)) KEEP(*(.spiboot2.text)) . = ALIGN(8192);
common/env_embedded.o (.ppcenv). = ALIGN(8192);common/env_embedded.o (.ppcenvr). = ALIGN(8192); *(.text) . = ALIGN(4); } >ram =0xFF-- 2.9.4
Hello,
On Thu, 3 Aug 2017 08:20:20 +0900, Nobuhiro Iwamatsu wrote:
Hi,
2017-07-29 6:46 GMT+09:00 Thomas Petazzoni thomas.petazzoni@free-electrons.com:
The linker script for SuperH brings the .ppcenv and .ppcenvr section of common/env_embedded.o into the .text section. However, the .ppcenv section is only ever filled in by env_embedded.o when CONFIG_SYS_USE_PPCENV is defined, but no platforms in mainline U-Boot use this.
In addition, common/env_embedded.o is not always built (when you use CONFIG_ENV_IS_NOWHERE for example), which causes the following build failure:
Fixes:
LD u-boot /home/thomas/sh4aeb-linux-musl/bin/sh4aeb-linux-ld.bfd: cannot find common/env_embedded.o
We fix this by no longer adding the .ppcenv and .ppcenvr sections from common/env_embedded.o into the .text section.
Signed-off-by: Thomas Petazzoni thomas.petazzoni@free-electrons.com
Acked-by: Nobuhiro Iwamatsu iwamatsu@nigauri.org
Thanks for your review! Are you going to apply the patch, as you did for "arch/sh: allow building in big-endian mode" ?
Thanks!
Thomas
CONFIG_SYS_USE_PPCENV is no longer used anywhere. It was used to put the environment in the special .ppcenv section, but the last architecture using this section (SuperH) has been changed to not use it.
Therefore, this commit drops support for CONFIG_SYS_USE_PPCENV entirely. We only handle two cases:
- We're building the host tool tools/envcrc, in which case the environment is place with no special section attribute (so it depends up in .data)
- We're building U-Boot itself, in which case the environnement is placed in the .text section.
Signed-off-by: Thomas Petazzoni thomas.petazzoni@free-electrons.com --- common/env_embedded.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/common/env_embedded.c b/common/env_embedded.c index b368fda..4532589 100644 --- a/common/env_embedded.c +++ b/common/env_embedded.c @@ -30,18 +30,11 @@ */ #if defined(ENV_IS_EMBEDDED) || defined(CONFIG_BUILD_ENVCRC) /* - * Only put the environment in it's own section when we are building + * Put the environment in the .text section when we are building * U-Boot proper. The host based program "tools/envcrc" does not need - * a seperate section. Note that ENV_CRC is only defined when building - * U-Boot itself. + * a seperate section. */ -#if defined(CONFIG_SYS_USE_PPCENV) && \ - defined(ENV_CRC) /* Environment embedded in U-Boot .ppcenv section */ -/* XXX - This only works with GNU C */ -# define __PPCENV__ __attribute__ ((section(".ppcenv"))) -# define __PPCTEXT__ __attribute__ ((section(".text"))) - -#elif defined(USE_HOSTCC) /* Native for 'tools/envcrc' */ +#if defined(USE_HOSTCC) /* Native for 'tools/envcrc' */ # define __PPCENV__ /*XXX DO_NOT_DEL_THIS_COMMENT*/ # define __PPCTEXT__ /*XXX DO_NOT_DEL_THIS_COMMENT*/
On Fri, Jul 28, 2017 at 11:46:37PM +0200, Thomas Petazzoni wrote:
CONFIG_SYS_USE_PPCENV is no longer used anywhere. It was used to put the environment in the special .ppcenv section, but the last architecture using this section (SuperH) has been changed to not use it.
Therefore, this commit drops support for CONFIG_SYS_USE_PPCENV entirely. We only handle two cases:
Applied to u-boot/master, thanks!
The environment has pretty much nothing to do with just "PPC", so rename the macros to just __UBOOT_ENV_SECTION__ which is more readable.
In addition, only a single macro is needed: the environment now goes either to the default section (USE_HOSTCC is defined) or in the .text section.
Signed-off-by: Thomas Petazzoni thomas.petazzoni@free-electrons.com --- common/env_embedded.c | 10 ++++------ include/env_default.h | 2 +- 2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/common/env_embedded.c b/common/env_embedded.c index 4532589..43694db 100644 --- a/common/env_embedded.c +++ b/common/env_embedded.c @@ -35,13 +35,11 @@ * a seperate section. */ #if defined(USE_HOSTCC) /* Native for 'tools/envcrc' */ -# define __PPCENV__ /*XXX DO_NOT_DEL_THIS_COMMENT*/ -# define __PPCTEXT__ /*XXX DO_NOT_DEL_THIS_COMMENT*/ +# define __UBOOT_ENV_SECTION__ /*XXX DO_NOT_DEL_THIS_COMMENT*/
#else /* Environment is embedded in U-Boot's .text section */ /* XXX - This only works with GNU C */ -# define __PPCENV__ __attribute__ ((section(".text"))) -# define __PPCTEXT__ __attribute__ ((section(".text"))) +# define __UBOOT_ENV_SECTION__ __attribute__ ((section(".text"))) #endif
/* @@ -72,7 +70,7 @@ #include <env_default.h>
#ifdef CONFIG_ENV_ADDR_REDUND -env_t redundand_environment __PPCENV__ = { +env_t redundand_environment __UBOOT_ENV_SECTION__ = { 0, /* CRC Sum: invalid */ 0, /* Flags: invalid */ { @@ -89,7 +87,7 @@ env_t redundand_environment __PPCENV__ = { * .data/.sdata section. * */ -unsigned long env_size __PPCTEXT__ = sizeof(env_t); +unsigned long env_size __UBOOT_ENV_SECTION__ = sizeof(env_t);
/* * Add in absolutes. diff --git a/include/env_default.h b/include/env_default.h index ea6704a..acd4198 100644 --- a/include/env_default.h +++ b/include/env_default.h @@ -11,7 +11,7 @@ #include <env_callback.h>
#ifdef DEFAULT_ENV_INSTANCE_EMBEDDED -env_t environment __PPCENV__ = { +env_t environment __UBOOT_ENV_SECTION__ = { ENV_CRC, /* CRC Sum */ #ifdef CONFIG_SYS_REDUNDAND_ENVIRONMENT 1, /* Flags: valid */
On Fri, Jul 28, 2017 at 11:46:38PM +0200, Thomas Petazzoni wrote:
The environment has pretty much nothing to do with just "PPC", so rename the macros to just __UBOOT_ENV_SECTION__ which is more readable.
In addition, only a single macro is needed: the environment now goes either to the default section (USE_HOSTCC is defined) or in the .text section.
Signed-off-by: Thomas Petazzoni thomas.petazzoni@free-electrons.com
Applied to u-boot/master, thanks!
participants (3)
-
Nobuhiro Iwamatsu -
Thomas Petazzoni -
Tom Rini