[PATCH v2 1/1] Makefile: rework u-boot-initial-env target

[Tom Rini]

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 <pali at 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.krummenacher at 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-30bae4332c77@foss.st.com/
> > >
> > > Signed-off-by: Max Krummenacher <max.krummenacher at 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.09@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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20221101/86118120/attachment.sig>