| Message ID | 20180320103845.2404-1-rasmus.villemoes@prevas.dk |
|---|---|
| State | Accepted |
| Commit | f3d8f7dd73ac5dde258eb786d4a01869395b56d7 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot,v2] Allow providing default environment from file | expand |
Hi Rasmus, > Modifying the default environment via CONFIG_EXTRA_ENV_SETTINGS is > somewhat inflexible, partly because the cpp language does not allow > appending to an existing macro. This prevents reuse of "environment > fragments" for different boards, which in turn makes maintaining that > environment consistently tedious and error-prone. It is also possible to build boot.scr image, which is afterwards read from e.g. vfat, from text file. As a reference and example please look into ./boards/samsung/common/bootscripts/*.cmd > > This implements a Kconfig option for allowing one to define the entire > default environment in an external file, which can then, for example, > be generated programmatically as part of a Yocto recipe, Is this yocto generation upstreamed? Or this is some kind of internal patch? > or simply be > kept in version control separately from the U-boot repository. > > Tested-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk> > Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk> > --- > v2: > * rename CONFIG_DEFAULT_ENV_FROM_FILE -> CONFIG_USE_DEFAULT_ENV_FILE > * add Tested-by > * provide a little more rationale (example use case instead of just > "sometimes be useful") > * rebase to current master (v2018.03-189-gda773532cd) > > This adds xxd as a build-time requirement. Not sure whether that needs > mentioning in the Kconfig help text. On my Ubuntu 16.04, it is > provided by the vim-common package, while more recent Ubuntu and > Debian seem to have split it to a separate package. > > Makefile | 16 ++++++++++++++++ > env/Kconfig | 18 ++++++++++++++++++ > include/env_default.h | 4 ++++ > 3 files changed, 38 insertions(+) > > diff --git a/Makefile b/Makefile > index 5fa14789d9..867b189c41 100644 > --- a/Makefile > +++ b/Makefile > @@ -423,6 +423,7 @@ endif > > version_h := include/generated/version_autogenerated.h > timestamp_h := include/generated/timestamp_autogenerated.h > +defaultenv_h := include/generated/defaultenv_autogenerated.h > > no-dot-config-targets := clean clobber mrproper distclean \ > help %docs check% coccicheck \ > @@ -1387,6 +1388,10 @@ ifeq ($(wildcard $(LDSCRIPT)),) > @/bin/false > endif > > +ifeq ($(CONFIG_USE_DEFAULT_ENV_FILE),y) > +prepare1: $(defaultenv_h) > +endif > + > archprepare: prepare1 scripts_basic > > prepare0: archprepare FORCE > @@ -1434,12 +1439,23 @@ define filechk_timestamp.h > fi) > endef > > +define filechk_defaultenv.h > + (grep -v '^#' | \ > + grep -v '^$$' | \ > + tr '\n' '\0' | \ > + sed -e 's/\\\x0/\n/' | \ > + xxd -i ; echo ", 0x00" ; ) > +endef > + > $(version_h): include/config/uboot.release FORCE > $(call filechk,version.h) > > $(timestamp_h): $(srctree)/Makefile FORCE > $(call filechk,timestamp.h) > > +$(defaultenv_h): $(CONFIG_DEFAULT_ENV_FILE:"%"=%) FORCE > + $(call filechk,defaultenv.h) > + > # > --------------------------------------------------------------------------- > quiet_cmd_cpp_lds = LDS $@ cmd_cpp_lds = $(CPP) > -Wp,-MD,$(depfile) $(cpp_flags) $(LDPPFLAGS) \ diff --git > a/env/Kconfig b/env/Kconfig index a3c6298273..e8e21dcfc6 100644 > --- a/env/Kconfig > +++ b/env/Kconfig > @@ -486,4 +486,22 @@ config ENV_SIZE > > endif > > +config USE_DEFAULT_ENV_FILE > + bool "Create default environment from file" > + help > + Normally, the default environment is automatically > generated > + based on the settings of various CONFIG_* options, as well > + as the CONFIG_EXTRA_ENV_SETTINGS. By selecting this option, > + you can instead define the entire default environment in an > + external file. > + > +config DEFAULT_ENV_FILE > + string "Path to default environment file" > + depends on USE_DEFAULT_ENV_FILE > + help > + The path containing the default environment. The format is > + the same as accepted by the mkenvimage tool: lines > + containing key=value pairs, blank lines and lines beginning > + with # are ignored. > + > endmenu > diff --git a/include/env_default.h b/include/env_default.h > index b574345af2..1fbeb92f38 100644 > --- a/include/env_default.h > +++ b/include/env_default.h > @@ -22,6 +22,7 @@ static char default_environment[] = { > #else > const uchar default_environment[] = { > #endif > +#ifndef CONFIG_USE_DEFAULT_ENV_FILE > #ifdef CONFIG_ENV_CALLBACK_LIST_DEFAULT > ENV_CALLBACK_VAR "=" CONFIG_ENV_CALLBACK_LIST_DEFAULT "\0" > #endif > @@ -108,6 +109,9 @@ const uchar default_environment[] = { > CONFIG_EXTRA_ENV_SETTINGS > #endif > "\0" > +#else /* CONFIG_USE_DEFAULT_ENV_FILE */ > +#include "generated/defaultenv_autogenerated.h" > +#endif > #ifdef DEFAULT_ENV_INSTANCE_EMBEDDED > } > #endif The patch looks ok. Reviewed-by: Lukasz Majewski <lukma@denx.de> Best regards, Lukasz Majewski -- 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@denx.de
On 2018-03-20 15:20, Lukasz Majewski wrote: > Hi Rasmus, > >> Modifying the default environment via CONFIG_EXTRA_ENV_SETTINGS is >> somewhat inflexible, partly because the cpp language does not allow >> appending to an existing macro. This prevents reuse of "environment >> fragments" for different boards, which in turn makes maintaining that >> environment consistently tedious and error-prone. > > It is also possible to build boot.scr image, which is afterwards read > from e.g. vfat, from text file. > > As a reference and example please look > into ./boards/samsung/common/bootscripts/*.cmd > >> >> This implements a Kconfig option for allowing one to define the entire >> default environment in an external file, which can then, for example, >> be generated programmatically as part of a Yocto recipe, > > Is this yocto generation upstreamed? Or this is some kind of internal > patch? Neither, for now, we're just using the "entire environment in file maintained elsewhere" model. But I can easily see us using the ability to amend the environment with a simple "echo ... >> foo.env" in some pre/postfunc, or having foo.env contain some __PLACEHOLDER__ which we fix up with sed using values from .conf files before the U-boot build. Stuff like tftp server ip address is nice to be able to override easily to point at one's own laptop while doing development. > > The patch looks ok. > > Reviewed-by: Lukasz Majewski <lukma@denx.de> Thanks. Rasmus
On 2018-03-20 15:47, Rasmus Villemoes wrote: > On 2018-03-20 15:20, Lukasz Majewski wrote: >> Hi Rasmus, >> >>> Modifying the default environment via CONFIG_EXTRA_ENV_SETTINGS is >>> somewhat inflexible, partly because the cpp language does not allow >>> appending to an existing macro. This prevents reuse of "environment >>> fragments" for different boards, which in turn makes maintaining that >>> environment consistently tedious and error-prone. [...] >> >> The patch looks ok. >> >> Reviewed-by: Lukasz Majewski <lukma@denx.de> Any chance this could get picked up, or is there anything more needed on my end? Thanks, Rasmus
On Tue, Mar 20, 2018 at 11:38:45AM +0100, Rasmus Villemoes wrote: > Modifying the default environment via CONFIG_EXTRA_ENV_SETTINGS is > somewhat inflexible, partly because the cpp language does not allow > appending to an existing macro. This prevents reuse of "environment > fragments" for different boards, which in turn makes maintaining that > environment consistently tedious and error-prone. > > This implements a Kconfig option for allowing one to define the entire > default environment in an external file, which can then, for example, be > generated programmatically as part of a Yocto recipe, or simply be kept > in version control separately from the U-boot repository. > > Tested-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk> > Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk> > Reviewed-by: Lukasz Majewski <lukma@denx.de> Applied to u-boot/master, thanks!
diff --git a/Makefile b/Makefile index 5fa14789d9..867b189c41 100644 --- a/Makefile +++ b/Makefile @@ -423,6 +423,7 @@ endif version_h := include/generated/version_autogenerated.h timestamp_h := include/generated/timestamp_autogenerated.h +defaultenv_h := include/generated/defaultenv_autogenerated.h no-dot-config-targets := clean clobber mrproper distclean \ help %docs check% coccicheck \ @@ -1387,6 +1388,10 @@ ifeq ($(wildcard $(LDSCRIPT)),) @/bin/false endif +ifeq ($(CONFIG_USE_DEFAULT_ENV_FILE),y) +prepare1: $(defaultenv_h) +endif + archprepare: prepare1 scripts_basic prepare0: archprepare FORCE @@ -1434,12 +1439,23 @@ define filechk_timestamp.h fi) endef +define filechk_defaultenv.h + (grep -v '^#' | \ + grep -v '^$$' | \ + tr '\n' '\0' | \ + sed -e 's/\\\x0/\n/' | \ + xxd -i ; echo ", 0x00" ; ) +endef + $(version_h): include/config/uboot.release FORCE $(call filechk,version.h) $(timestamp_h): $(srctree)/Makefile FORCE $(call filechk,timestamp.h) +$(defaultenv_h): $(CONFIG_DEFAULT_ENV_FILE:"%"=%) FORCE + $(call filechk,defaultenv.h) + # --------------------------------------------------------------------------- quiet_cmd_cpp_lds = LDS $@ cmd_cpp_lds = $(CPP) -Wp,-MD,$(depfile) $(cpp_flags) $(LDPPFLAGS) \ diff --git a/env/Kconfig b/env/Kconfig index a3c6298273..e8e21dcfc6 100644 --- a/env/Kconfig +++ b/env/Kconfig @@ -486,4 +486,22 @@ config ENV_SIZE endif +config USE_DEFAULT_ENV_FILE + bool "Create default environment from file" + help + Normally, the default environment is automatically generated + based on the settings of various CONFIG_* options, as well + as the CONFIG_EXTRA_ENV_SETTINGS. By selecting this option, + you can instead define the entire default environment in an + external file. + +config DEFAULT_ENV_FILE + string "Path to default environment file" + depends on USE_DEFAULT_ENV_FILE + help + The path containing the default environment. The format is + the same as accepted by the mkenvimage tool: lines + containing key=value pairs, blank lines and lines beginning + with # are ignored. + endmenu diff --git a/include/env_default.h b/include/env_default.h index b574345af2..1fbeb92f38 100644 --- a/include/env_default.h +++ b/include/env_default.h @@ -22,6 +22,7 @@ static char default_environment[] = { #else const uchar default_environment[] = { #endif +#ifndef CONFIG_USE_DEFAULT_ENV_FILE #ifdef CONFIG_ENV_CALLBACK_LIST_DEFAULT ENV_CALLBACK_VAR "=" CONFIG_ENV_CALLBACK_LIST_DEFAULT "\0" #endif @@ -108,6 +109,9 @@ const uchar default_environment[] = { CONFIG_EXTRA_ENV_SETTINGS #endif "\0" +#else /* CONFIG_USE_DEFAULT_ENV_FILE */ +#include "generated/defaultenv_autogenerated.h" +#endif #ifdef DEFAULT_ENV_INSTANCE_EMBEDDED } #endif