[Buildroot] [RFC v2] boot/uboot: add option to install custom environment file

[Yann E. MORIN]

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 at walle.cc>
> Signed-off-by: Heiko Thiery <heiko.thiery at 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 at buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'