| Message ID | 20230803121003.160501-1-heiko.thiery@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [RFC,v2] boot/uboot: add option to install custom environment file | expand |
Heiko, All, On 2023-08-03 14:10 +0200, Heiko Thiery spake thusly: > U-Boot has the capability to set the environment variables via a text file. > The text file has to be located in the U-Boot board source directory and > selected via the CONFIG_ENV_SOURCE_FILE option. The CONFIG_ENV_SOURCE_FILE > must only contain the filename without the '.env' suffix. > > Thus the buildroot option BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT is added > that needs the information about the source of the file in the buildroot > environment (BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT_SOURCE). > > Since the environment file must be located in the U-Boot board source > directory. This directory is <SRC>/board/<VENDOR>/<BOARDNAME>. > > Thes information about vendor name and board name are available in the > U-Boot .config file and can be extracted from there to determine the > destination directoy. > > Cc: Michael Walle <michael@walle.cc> > Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com> > --- [--SNIP--] > diff --git a/boot/uboot/Config.in b/boot/uboot/Config.in > index 8b726eaa57..894a0bd3b2 100644 > --- a/boot/uboot/Config.in > +++ b/boot/uboot/Config.in > @@ -607,6 +607,22 @@ config BR2_TARGET_UBOOT_CUSTOM_DTS_PATH > > endif > > +config BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT > + bool "custom environment" > + help > + Provide a custom u-boot environment file. This will be > + copied to the U-Boot source path and enabled via the > + U-Boot config option CONFIG_ENV_SOURCE_FILE. The target > + path will be determined based on the U-Boot configuration. > + > +if BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT > +config BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT_SOURCE > + string "Custom environment source file" > + help > + Path to U-Boot custom environment file. > + > +endif We don't really need a boolean option to guard a single string option. I know we tend to do that a lot, but I find that to be an anti-pattern. In this case, the empty string is as good as saying "no" to the boolean option, so we can just live with the sting option, and then (see below)... (of course, if the empty string _has_ a special meaning, then we'd need a boolean, but that's usually not the case in such situations). > config BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS > string "Custom make options" > help > diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk > index b3d26b16fe..35e26ade2d 100644 > --- a/boot/uboot/uboot.mk > +++ b/boot/uboot/uboot.mk > @@ -181,6 +181,26 @@ UBOOT_PRE_BUILD_HOOKS += UBOOT_COPY_ATF_FIRMWARE > endif > endif > > +# > +# Prepare for custom environment > +# > +ifeq ($(BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT),y) > +ifeq ($(call qstrip,$(BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT_SOURCE)),) > +$(error No custom environment source file specified, check your BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT_SOURCE setting) > +endif ... we don't need to check the sanity of the settings: empty means don't use a custom env file, set means use that file as custom env file. > +define UBOOT_COPY_CUSTOM_ENVIRONMENT_FILE > + cp -f $(BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT_SOURCE) $(@D)/board/$(shell grep CONFIG_SYS_VENDOR $(@D)/.config | sed 's/.*=//' | sed 's/"//g')/$(shell grep CONFIG_SYS_BOARD $(@D)/.config | sed 's/.*=//' | sed 's/"//g')/ Please split this line into easier-to-parse construct, see below... > +endef > +UBOOT_PRE_BUILD_HOOKS += UBOOT_COPY_CUSTOM_ENVIRONMENT_FILE Is it a pre-build or a post-configure hook? I would think it should be a post-configure one... > + > + > +UBOOT_ENV_FILE_NAME=$(subst .env,,$(notdir $(call qstrip,$(BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT_SOURCE)))) $(patsubst %.env,%,$(notdir $(UBOOT_CUSTOM_ENVIRONMENT_SOURCE))) > +define UBOOT_KCONFIG_CUSTOM_ENV_SOURCE > + $(call KCONFIG_SET_OPT,CONFIG_ENV_SOURCE_FILE,"$(UBOOT_ENV_FILE_NAME)") > +endef > +endif So, the .mk code would look like; UBOOT_CUSTOM_ENVIRONMENT_SOURCE = $(call qstrip,$(BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT_SOURCE))) UBOOT_ENV_FILE_NAME=$(patsubst %.env,%,$(notdir $(UBOOT_CUSTOM_ENVIRONMENT_SOURCE))) ifneq ($(UBOOT_CUSTOM_ENVIRONMENT_SOURCE),) define UBOOT_COPY_CUSTOM_ENVIRONMENT_FILE sys_config=$( ./utils/config --file $(@D)/.config -s CONFIG_SYS_VENDOR ); \ sys_board=$( ./utils/config --file $(@D)/.config -s CONFIG_SYS_BOARD ); \ cp -f $(UBOOT_CUSTOM_ENVIRONMENT_SOURCE) $(@D)/board/$${sys_config}/$${sys_board} endef UBOOT_POST_CONFIGURE_HOOKS += UBOOT_COPY_CUSTOM_ENVIRONMENT_FILE define UBOOT_KCONFIG_CUSTOM_ENV_SOURCE $(call KCONFIG_SET_OPT,CONFIG_ENV_SOURCE_FILE,"$(UBOOT_ENV_FILE_NAME)") endef endif # UBOOT_CUSTOM_ENVIRONMENT_SOURCE != "" Regards, Yann E. MORIN. > + > ifeq ($(BR2_TARGET_UBOOT_NEEDS_OPTEE_TEE),y) > UBOOT_DEPENDENCIES += optee-os > UBOOT_MAKE_OPTS += TEE=$(BINARIES_DIR)/tee.elf > @@ -497,6 +517,7 @@ define UBOOT_KCONFIG_FIXUP_CMDS > $(UBOOT_ZYNQMP_KCONFIG_PMUFW) > $(UBOOT_ZYNQMP_KCONFIG_PM_CFG) > $(UBOOT_ZYNQMP_KCONFIG_PSU_INIT) > + $(UBOOT_KCONFIG_CUSTOM_ENV_SOURCE) > endef > > ifeq ($(BR2_TARGET_UBOOT)$(BR_BUILDING),yy) > -- > 2.30.2 > > _______________________________________________ > buildroot mailing list > buildroot@buildroot.org > https://lists.buildroot.org/mailman/listinfo/buildroot
Hi Yann, Am Do., 3. Aug. 2023 um 21:41 Uhr schrieb Yann E. MORIN <yann.morin.1998@free.fr>: > > Heiko, All, > > On 2023-08-03 14:10 +0200, Heiko Thiery spake thusly: > > U-Boot has the capability to set the environment variables via a text file. > > The text file has to be located in the U-Boot board source directory and > > selected via the CONFIG_ENV_SOURCE_FILE option. The CONFIG_ENV_SOURCE_FILE > > must only contain the filename without the '.env' suffix. > > > > Thus the buildroot option BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT is added > > that needs the information about the source of the file in the buildroot > > environment (BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT_SOURCE). > > > > Since the environment file must be located in the U-Boot board source > > directory. This directory is <SRC>/board/<VENDOR>/<BOARDNAME>. > > > > Thes information about vendor name and board name are available in the > > U-Boot .config file and can be extracted from there to determine the > > destination directoy. > > > > Cc: Michael Walle <michael@walle.cc> > > Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com> > > --- > [--SNIP--] > > diff --git a/boot/uboot/Config.in b/boot/uboot/Config.in > > index 8b726eaa57..894a0bd3b2 100644 > > --- a/boot/uboot/Config.in > > +++ b/boot/uboot/Config.in > > @@ -607,6 +607,22 @@ config BR2_TARGET_UBOOT_CUSTOM_DTS_PATH > > > > endif > > > > +config BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT > > + bool "custom environment" > > + help > > + Provide a custom u-boot environment file. This will be > > + copied to the U-Boot source path and enabled via the > > + U-Boot config option CONFIG_ENV_SOURCE_FILE. The target > > + path will be determined based on the U-Boot configuration. > > + > > +if BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT > > +config BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT_SOURCE > > + string "Custom environment source file" > > + help > > + Path to U-Boot custom environment file. > > + > > +endif > > We don't really need a boolean option to guard a single string option. I > know we tend to do that a lot, but I find that to be an anti-pattern. > > In this case, the empty string is as good as saying "no" to the boolean > option, so we can just live with the sting option, and then (see below)... > > (of course, if the empty string _has_ a special meaning, then we'd need > a boolean, but that's usually not the case in such situations). Ok, makes sense. Previously I had 2 config options for source and destination and removed the destination in v2. > > config BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS > > string "Custom make options" > > help > > diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk > > index b3d26b16fe..35e26ade2d 100644 > > --- a/boot/uboot/uboot.mk > > +++ b/boot/uboot/uboot.mk > > @@ -181,6 +181,26 @@ UBOOT_PRE_BUILD_HOOKS += UBOOT_COPY_ATF_FIRMWARE > > endif > > endif > > > > +# > > +# Prepare for custom environment > > +# > > +ifeq ($(BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT),y) > > +ifeq ($(call qstrip,$(BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT_SOURCE)),) > > +$(error No custom environment source file specified, check your BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT_SOURCE setting) > > +endif > > ... we don't need to check the sanity of the settings: empty means don't > use a custom env file, set means use that file as custom env file. Ok > > > +define UBOOT_COPY_CUSTOM_ENVIRONMENT_FILE > > + cp -f $(BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT_SOURCE) $(@D)/board/$(shell grep CONFIG_SYS_VENDOR $(@D)/.config | sed 's/.*=//' | sed 's/"//g')/$(shell grep CONFIG_SYS_BOARD $(@D)/.config | sed 's/.*=//' | sed 's/"//g')/ > > Please split this line into easier-to-parse construct, see below... > > > +endef > > +UBOOT_PRE_BUILD_HOOKS += UBOOT_COPY_CUSTOM_ENVIRONMENT_FILE > > Is it a pre-build or a post-configure hook? I would think it should be a > post-configure one... I wanted to do it like the other steps, copy the files to the source folder before building. - UBOOT_COPY_ATF_FIRMWARE - UBOOT_COPY_IMX_FW_FILES > > > + > > + > > +UBOOT_ENV_FILE_NAME=$(subst .env,,$(notdir $(call qstrip,$(BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT_SOURCE)))) > > $(patsubst %.env,%,$(notdir $(UBOOT_CUSTOM_ENVIRONMENT_SOURCE))) > > > +define UBOOT_KCONFIG_CUSTOM_ENV_SOURCE > > + $(call KCONFIG_SET_OPT,CONFIG_ENV_SOURCE_FILE,"$(UBOOT_ENV_FILE_NAME)") > > +endef > > +endif > > So, the .mk code would look like; > > UBOOT_CUSTOM_ENVIRONMENT_SOURCE = $(call qstrip,$(BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT_SOURCE))) > UBOOT_ENV_FILE_NAME=$(patsubst %.env,%,$(notdir $(UBOOT_CUSTOM_ENVIRONMENT_SOURCE))) > ifneq ($(UBOOT_CUSTOM_ENVIRONMENT_SOURCE),) > define UBOOT_COPY_CUSTOM_ENVIRONMENT_FILE > sys_config=$( ./utils/config --file $(@D)/.config -s CONFIG_SYS_VENDOR ); \ Creating a subshell to get the output of uitls/config does not work that way. I have to do $(shell ...). But with that it works ;-) > sys_board=$( ./utils/config --file $(@D)/.config -s CONFIG_SYS_BOARD ); \ > cp -f $(UBOOT_CUSTOM_ENVIRONMENT_SOURCE) $(@D)/board/$${sys_config}/$${sys_board} > endef > UBOOT_POST_CONFIGURE_HOOKS += UBOOT_COPY_CUSTOM_ENVIRONMENT_FILE > > define UBOOT_KCONFIG_CUSTOM_ENV_SOURCE > $(call KCONFIG_SET_OPT,CONFIG_ENV_SOURCE_FILE,"$(UBOOT_ENV_FILE_NAME)") > endef > endif # UBOOT_CUSTOM_ENVIRONMENT_SOURCE != "" > > Regards, > Yann E. MORIN. Many thanks for your comments! I will retest and prepare a v3.
diff --git a/boot/uboot/Config.in b/boot/uboot/Config.in index 8b726eaa57..894a0bd3b2 100644 --- a/boot/uboot/Config.in +++ b/boot/uboot/Config.in @@ -607,6 +607,22 @@ config BR2_TARGET_UBOOT_CUSTOM_DTS_PATH endif +config BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT + bool "custom environment" + help + Provide a custom u-boot environment file. This will be + copied to the U-Boot source path and enabled via the + U-Boot config option CONFIG_ENV_SOURCE_FILE. The target + path will be determined based on the U-Boot configuration. + +if BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT +config BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT_SOURCE + string "Custom environment source file" + help + Path to U-Boot custom environment file. + +endif + config BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS string "Custom make options" help diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk index b3d26b16fe..35e26ade2d 100644 --- a/boot/uboot/uboot.mk +++ b/boot/uboot/uboot.mk @@ -181,6 +181,26 @@ UBOOT_PRE_BUILD_HOOKS += UBOOT_COPY_ATF_FIRMWARE endif endif +# +# Prepare for custom environment +# +ifeq ($(BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT),y) +ifeq ($(call qstrip,$(BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT_SOURCE)),) +$(error No custom environment source file specified, check your BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT_SOURCE setting) +endif + +define UBOOT_COPY_CUSTOM_ENVIRONMENT_FILE + cp -f $(BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT_SOURCE) $(@D)/board/$(shell grep CONFIG_SYS_VENDOR $(@D)/.config | sed 's/.*=//' | sed 's/"//g')/$(shell grep CONFIG_SYS_BOARD $(@D)/.config | sed 's/.*=//' | sed 's/"//g')/ +endef +UBOOT_PRE_BUILD_HOOKS += UBOOT_COPY_CUSTOM_ENVIRONMENT_FILE + + +UBOOT_ENV_FILE_NAME=$(subst .env,,$(notdir $(call qstrip,$(BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT_SOURCE)))) +define UBOOT_KCONFIG_CUSTOM_ENV_SOURCE + $(call KCONFIG_SET_OPT,CONFIG_ENV_SOURCE_FILE,"$(UBOOT_ENV_FILE_NAME)") +endef +endif + ifeq ($(BR2_TARGET_UBOOT_NEEDS_OPTEE_TEE),y) UBOOT_DEPENDENCIES += optee-os UBOOT_MAKE_OPTS += TEE=$(BINARIES_DIR)/tee.elf @@ -497,6 +517,7 @@ define UBOOT_KCONFIG_FIXUP_CMDS $(UBOOT_ZYNQMP_KCONFIG_PMUFW) $(UBOOT_ZYNQMP_KCONFIG_PM_CFG) $(UBOOT_ZYNQMP_KCONFIG_PSU_INIT) + $(UBOOT_KCONFIG_CUSTOM_ENV_SOURCE) endef ifeq ($(BR2_TARGET_UBOOT)$(BR_BUILDING),yy)
U-Boot has the capability to set the environment variables via a text file. The text file has to be located in the U-Boot board source directory and selected via the CONFIG_ENV_SOURCE_FILE option. The CONFIG_ENV_SOURCE_FILE must only contain the filename without the '.env' suffix. Thus the buildroot option BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT is added that needs the information about the source of the file in the buildroot environment (BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT_SOURCE). Since the environment file must be located in the U-Boot board source directory. This directory is <SRC>/board/<VENDOR>/<BOARDNAME>. Thes information about vendor name and board name are available in the U-Boot .config file and can be extracted from there to determine the destination directoy. Cc: Michael Walle <michael@walle.cc> Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com> --- v2: Drop the BR2_TARGET_UBOOT_CUSTOM_ENVIRONMENT_TARGET option and determine the value from the applied u-boot defconfig (.config). Adopt the commit message according these changes. boot/uboot/Config.in | 16 ++++++++++++++++ boot/uboot/uboot.mk | 21 +++++++++++++++++++++ 2 files changed, 37 insertions(+)