[U-Boot] [PATCH 0/3] env_embedded related fixes/improvements

[U-Boot] [PATCH 0/3] env_embedded related fixes/improvements

[Thomas Petazzoni]

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

-- 
2.9.4

[U-Boot] [PATCH 1/3] arch/sh: don't bring common/env_embedded.o into the link

[Thomas Petazzoni]

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
-- 
2.9.4

[U-Boot] [PATCH 1/3] arch/sh: don't bring common/env_embedded.o into the link

[Nobuhiro Iwamatsu]

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
>



-- 
Nobuhiro Iwamatsu
   iwamatsu at {nigauri.org / debian.org}
   GPG ID: 40AD1FA6

[U-Boot] [PATCH 1/3] arch/sh: don't bring common/env_embedded.o into the link

[Thomas Petazzoni]

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[U-Boot] [PATCH 2/3] common/env_embedded.c: drop support for CONFIG_SYS_USE_PPCENV

[Thomas Petazzoni]

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

[U-Boot] [U-Boot, 2/3] common/env_embedded.c: drop support for CONFIG_SYS_USE_PPCENV

[Tom Rini]

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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170807/e7e12dd2/attachment.sig>

[U-Boot] [PATCH 3/3] common/env_embedded.c: rename PPCENV/PPCTEXT macros

[Thomas Petazzoni]

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

[U-Boot] [U-Boot, 3/3] common/env_embedded.c: rename PPCENV/PPCTEXT macros

[Tom Rini]

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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170807/1ffbe2d6/attachment.sig>