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

[Bruno Thomsen <bruno.thomsen@gmail.com>]

Hi Alexander,

> 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. ;-)

Thanks for review.

We just realized that around 75% of our rules was custom but not
project specific
in any way. So we might as well upstream them if possible :)

> > +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.

Ok, will fix in next version.

> > +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?

I will update help to reflect this limitation using final version as shipping
with release candidates is just bad practice(tm) ;)

Running an old version of U-Boot has many limitation like poor support of new
gcc versions and boards not using Kconfig. So don't enable this option if you
use an ancient U-Boot version. Evolution has to continue.

>
> > +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?

U-boot run-time will by default pad with 0x00 and mkenvimage pad with
0xFF by default. U-boot does not care what pad is used.
So it was just to mimic what normal U-boot run-time does, but we can use
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?
>

Okay, rebuild in case of source file changes and move it to platformconfigdir,
so it's stored next to the kernel configuration.

Have not tested the below section with ptxdist layers, but seems to work
in my setup without layers. According to docs[1] it should search from top
layer and down.

ifdef PTXCONF_U_BOOT_CUSTOM_ENV_IMAGE
U_BOOT_CUSTOM_ENV_SRC := $(call ptx/in-platformconfigdir, \
    $(PTXCONF_U_BOOT_CUSTOM_ENV_IMAGE_SOURCE))
$(STATEDIR)/u-boot.compile: $(call remove_quotes, $(U_BOOT_CUSTOM_ENV_SRC))
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.

Yes, I will add them to cleanup including a missing cleanup of imx dtb image.

/Bruno

[1] https://www.ptxdist.org/doc/dev_manual.html#writing-layer-aware-rules

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de