Re: [ptxdist] [PATCH v3] u-boot: generate environment image

[Alexander Dahl <ada@thorsis.com>]

Hello Bruno,

thanks for your effort. I guess some folks have custom rules for generating 
env images, so do we. It's good to have a common approach for ordinary use 
cases to prevent reinventing the wheel. I have some remarks however, see 
below. ;-)

Am Dienstag, 19. November 2019, 17:40:02 CET schrieb Bruno Thomsen:
> Add possiblity to generate a default or a custom environment
> image. Image can be used during manufacturing to avoid bootloader
> console usage and speed up first boot. Other image use-cases
> include device development edition, device demonstration
> mode, etc.
> 
> Custom environment image is generated from an user provided
> config file with one 'var=value' per line format. Input config
> file name is configurable.
> 
> Cc: Denis OSTERLAND <denis.osterland@diehl.com>
> Cc: Alexander Dahl <ada@thorsis.com>
> Signed-off-by: Bruno Thomsen <bruno.thomsen@gmail.com>
> ---
> v3:
> - remove multiple env images support
> - use static image names
> - rebase patches on top of: u-boot: Build out-of-tree
> v2:
> - remove HOST_U_BOOT_TOOLS dependency
> - configurable default env image name
> - add custom env image generation option
> - move all options to sub menu
> 
>  platforms/u-boot.in | 57 +++++++++++++++++++++++++++++++++++++++++++++
>  rules/u-boot.make   | 22 +++++++++++++++++
>  2 files changed, 79 insertions(+)
> 
> diff --git a/platforms/u-boot.in b/platforms/u-boot.in
> index 9bac4a758..1dfb80098 100644
> --- a/platforms/u-boot.in
> +++ b/platforms/u-boot.in
> @@ -71,6 +71,63 @@ config U_BOOT_CONFIG
> 
>  endif
> 
> +choice
> +	prompt "Generate environment image"
> +	default U_BOOT_NONE_ENV_IMAGE
> +
> +config U_BOOT_NONE_ENV_IMAGE
> +	prompt "none"
> +	bool
> +	help
> +	  Don't generate an U-Boot environment image.
> +
> +config U_BOOT_DEFAULT_ENV_IMAGE
> +	prompt "default"
> +	bool
> +	help
> +	  Use U-Boot's mkenvimage to compile a default U-Boot environment
> +	  image for use in e.g. device manufacturing or development.
> +
> +config U_BOOT_CUSTOM_ENV_IMAGE
> +	prompt "custom"
> +	bool
> +	help
> +	  Use U-Boot's mkenvimage to compile a custom U-Boot environment
> +	  image based on the text file in U_BOOT_CUSTOM_ENV_IMAGE_SOURCE
> +	  for use in e.g. device manufacturing or development.
> +
> +endchoice
> +
> +config U_BOOT_CUSTOM_ENV_IMAGE_SOURCE
> +	prompt "Custom environment source"
> +	string
> +	default "custom_env.config"
> +	depends on U_BOOT_CUSTOM_ENV_IMAGE
> +	help
> +	  Text file in PTXDIST_WORKSPACE describing the custom environment.
> +	  The file should have lines in the form var=value, one per line.
> +	  Blank lines and lines starting with a # are ignored.
> +
> +if !U_BOOT_NONE_ENV_IMAGE
> +
> +config U_BOOT_ENV_IMAGE_SIZE
> +	prompt "Environment image size"
> +	string
> +	default "0x2000"
> +	help
> +	  Enter the U-Boot environment size for generation of image.
> +	  Size can be prefixed with 0x for hexadecimal values.
> +	  Must match size defined in target config and "/etc/fw_env.config".

Kconfig knows the type "hex" which you could use here. It would prevent 
invalid user input.

> +
> +config U_BOOT_ENV_IMAGE_REDUNDANT
> +	prompt "Environment image with redundant copy"
> +	bool
> +	help
> +	  Use to generate a redundant environment in the image.
> +	  Must match target config and "/etc/fw_env.config".
> +
> +endif
> +
>  config U_BOOT_BOOT_SCRIPT
>  	prompt "Compile U-Boot boot script"
>  	bool
> diff --git a/rules/u-boot.make b/rules/u-boot.make
> index e3c2c2389..932b4e4af 100644
> --- a/rules/u-boot.make
> +++ b/rules/u-boot.make
> @@ -100,6 +100,20 @@ ifdef PTXCONF_U_BOOT_BOOT_SCRIPT
>  	@$(U_BOOT_BUILD_DIR)/tools/mkimage -T script -C none \
>  		-d $(U_BOOT_BOOT_SCRIPT_TXT) \
>  		$(U_BOOT_BUILD_DIR)/boot.scr.uimg
> +endif
> +ifdef PTXCONF_U_BOOT_DEFAULT_ENV_IMAGE
> +	$(U_BOOT_MAKE_ENV) $(U_BOOT_DIR)/scripts/get_default_envs.sh
> $(U_BOOT_BUILD_DIR) | \ +		$(U_BOOT_BUILD_DIR)/tools/mkenvimage -p 0x0 \
> +		$(call ptx/ifdef,PTXCONF_U_BOOT_ENV_IMAGE_REDUNDANT,-r,) \
> +		-s $(PTXCONF_U_BOOT_ENV_IMAGE_SIZE) \
> +		-o $(U_BOOT_BUILD_DIR)/u-boot-env.img -

This way of calling scripts/get_default_envs.sh is possible since U-Boot 
v2018.03-rc3, it will probably fail with older U-Boot. We allow arbitrary U-
Boot versions with this package and can not easily test on that. Nevertheless 
I would keep it in, but add a hint in the menu on this requirement. Users with 
older U-Boot can just deactivate it.

With U-Boot v2018.01 this gives:

  File 'env_common.o' not found!

The resulting platform-v7a/images/u-boot-env.img is just the checksum and all 
zeros then.

The script itself exists since U-Boot v2016.11-rc2, but without support for 
separate build dir, and it might be broken somewhere in between. Before 
v2016.11 we would get such an error:

  /bin/bash: /home/adahl/Work/bsp/tt-generic/platform-v7a/build-target/u-
boot-2016.09.01/scripts/get_default_envs.sh: No such file or directory

The resulting platform-v7a/images/u-boot-env.img is also just the checksum and 
all zeros then.

I'm not sure if we can bail out here or just add something to the help text? 
Maybe add it to the prompt?

> +endif
> +ifdef PTXCONF_U_BOOT_CUSTOM_ENV_IMAGE
> +	$(U_BOOT_BUILD_DIR)/tools/mkenvimage -p 0x0 \

Why -p 0x0? What's the benefit over stuffing with 0xFF by default?

> +		$(call ptx/ifdef,PTXCONF_U_BOOT_ENV_IMAGE_REDUNDANT,-r,) \
> +		-s $(PTXCONF_U_BOOT_ENV_IMAGE_SIZE) \
> +		-o $(U_BOOT_BUILD_DIR)/u-boot-custom-env.img \
> +		$(PTXDIST_WORKSPACE)/$(PTXCONF_U_BOOT_CUSTOM_ENV_IMAGE_SOURCE)

I'm not sure about that path. I would put my input file to e.g. [workspace]/
configs/platform-v7a/env_source.config and use of PTXDIST_PLATFORMCONFIGDIR is 
not possible this way. I would have to set 
PTXCONF_U_BOOT_CUSTOM_ENV_IMAGE_SOURCE it like this:

  configs/platform-${PTXCONF_PLATFORM}/my-env-source.config

besides: I had a similar problem with dts files and layers. You can not use 
the macros from the make files in those config variables for automatically 
locating that in lower layers. :-/

Anyways, if I set it to a working path, it works.

You might add a make dependency on the input file however, so the stage gets 
rebuild automatically if the custom env input changes?

>  endif
>  	@$(call touch)
> 
> @@ -147,6 +161,14 @@ ifdef PTXCONF_U_BOOT_INSTALL_U_BOOT_WITH_SPL_PBL
>  	@install -v -D -m644 $(U_BOOT_BUILD_DIR)/u-boot-with-spl-pbl.bin \
>  		$(IMAGEDIR)/u-boot-with-spl-pbl.bin
>  endif
> +ifdef PTXCONF_U_BOOT_DEFAULT_ENV_IMAGE
> +	@install -v -D -m644 $(U_BOOT_BUILD_DIR)/u-boot-env.img \
> +		$(IMAGEDIR)/u-boot-env.img
> +endif
> +ifdef PTXCONF_U_BOOT_CUSTOM_ENV_IMAGE
> +	@install -v -D -m644 $(U_BOOT_BUILD_DIR)/u-boot-custom-env.img \
> +		$(IMAGEDIR)/u-boot-custom-env.img
> +endif
> 
>  ifdef PTXCONF_U_BOOT_BOOT_SCRIPT
>  	@$(call install_init, u-boot)

Removing those images in clean stage is missing.

Greets
Alex


_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de