From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail.thorsis.com ([92.198.35.195]) by metis.ext.pengutronix.de with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1iXL4t-0003Fs-TC for ptxdist@pengutronix.de; Wed, 20 Nov 2019 09:11:08 +0100 Received: from localhost (localhost [127.0.0.1]) by mail.thorsis.com (Postfix) with ESMTP id C9A41516D for ; Wed, 20 Nov 2019 09:12:17 +0100 (CET) Received: from mail.thorsis.com ([127.0.0.1]) by localhost (mail.thorsis.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id JnIxZvS0zPnx for ; Wed, 20 Nov 2019 09:12:17 +0100 (CET) From: Alexander Dahl Date: Wed, 20 Nov 2019 09:10:58 +0100 Message-ID: <9844490.fM4Dk9IfLu@ada> In-Reply-To: <20191119164002.5288-1-bruno.thomsen@gmail.com> References: <20191119164002.5288-1-bruno.thomsen@gmail.com> Subject: Re: [ptxdist] [PATCH v3] u-boot: generate environment image List-Id: PTXdist Development Mailing List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: ptxdist@pengutronix.de MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ptxdist-bounces@pengutronix.de Sender: "ptxdist" To: ptxdist@pengutronix.de Cc: Denis OSTERLAND , Bruno Thomsen , bth@kamstrup.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 > Cc: Alexander Dahl > Signed-off-by: Bruno Thomsen > --- > 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