On Mon, Oct 31, 2022 at 11:51:45AM +0100, Max Krummenacher wrote: > Hi > > On Fri, Oct 28, 2022 at 6:40 PM Pali Rohár <p...@kernel.org> wrote: > > > > Hello! This is really much better solution! Few comments are below. > > > > On Friday 28 October 2022 18:18:49 Max Krummenacher wrote: > > > From: Max Krummenacher <max.krummenac...@toradex.com> > > > > > > With LTO enabled the U-Boot initial environment is no longer stored > > > in an easy accessible section in env/common.o. I.e. the section name > > > changes from build to build, its content maybe compressed and it is > > > annotated with additional data. > > > > > > Drop trying to read the initial env with elf tools from the compiler > > > specific object file in favour of adding and using a host tool with > > > the only functionality of printing the initial env to stdout. > > > > > > See also: > > > https://lore.kernel.org/all/927b122e-1f62-e790-f5ca-30bae4332...@foss.st.com/ > > > > > > Signed-off-by: Max Krummenacher <max.krummenac...@toradex.com> > > > > > > --- > > > > > > Changes in v2: > > > - reworked to build a host tool which prints the configured > > > environment as proposed by Pali Rohár > > > > > > https://lore.kernel.org/u-boot/20221018174827.1393211-1-max.oss...@gmail.com/ > > > - renamed patch, v1 used "Makefile: fix u-boot-initial-env target if lto > > > is enabled" > > > > > > Makefile | 7 ++++--- > > > scripts/.gitignore | 1 + > > > scripts/Makefile | 5 +++++ > > > scripts/printinitialenv.c | 44 +++++++++++++++++++++++++++++++++++++++ > > > 4 files changed, 54 insertions(+), 3 deletions(-) > > > create mode 100644 scripts/printinitialenv.c > > > > > > diff --git a/Makefile b/Makefile > > > index 0f1174718f7..2f97d63c398 100644 > > > --- a/Makefile > > > +++ b/Makefile > > > @@ -2442,9 +2442,10 @@ endif > > > $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost > > > > > > quiet_cmd_genenv = GENENV $@ > > > -cmd_genenv = $(OBJCOPY) --dump-section .rodata.default_environment=$@ > > > env/common.o; \ > > > - sed --in-place -e 's/\x00/\x0A/g' $@; sed --in-place -e '/^\s*$$/d' > > > $@; \ > > > - sort --field-separator== -k1,1 --stable $@ -o $@ > > > +cmd_genenv = \ > > > + scripts/printinitialenv | \ > > > + sed -e 's/\x00/\x0A/g' -e '/^\s*$$/d' | \ > > > > I think that you do not need this sed anymore as you print newline in > > host tool. > > Missed that one, will change in a V3. > > > > > > + sort --field-separator== -k1,1 --stable -o $@ > > > > > > u-boot-initial-env: u-boot.bin > > > > It is needed to update dependencies for u-boot-initial-env target. Now > > it does not depend on u-boot.bin but rather on printinitialenv tool. > > I'm unsure if that is the best way forward. The initial solution would > also not need to depend on u-boot.bin but rather on env/common.o. > > I guess that the intention was that the U-Boot binary and the > u-boot-initial-env file should not be out of sync. > > Opinions? > > > > > > $(call if_changed,genenv) > > > diff --git a/scripts/.gitignore b/scripts/.gitignore > > > index 08cc824bac3..6068724a0d4 100644 > > > --- a/scripts/.gitignore > > > +++ b/scripts/.gitignore > > > @@ -2,3 +2,4 @@ > > > # Generated files > > > # > > > bin2c > > > +printinitialenv > > > diff --git a/scripts/Makefile b/scripts/Makefile > > > index 8731e6cecd7..ba993820571 100644 > > > --- a/scripts/Makefile > > > +++ b/scripts/Makefile > > > @@ -5,8 +5,13 @@ > > > # > > > --------------------------------------------------------------------------- > > > > > > hostprogs-$(CONFIG_BUILD_BIN2C) += bin2c > > > +hostprogs-y += printinitialenv > > > > I'm not sure if the scripts/ is the correct directory for the this tool. > > Maybe it should be in tools? Lets wait what maintainers or Tom say. > > According to tools/Makefile tools should be used for tools which > are not dependent on any boards. This here is a helper during > the build of a particular U-Boot configuration. That is why I put it > into scripts/.
I think it should go to tools/ as the comment there isn't an absolute (other env things are there too). -- Tom
signature.asc
Description: PGP signature
