[PATCH] tools: Remove #include <version.h>

[Pali Rohár]

On Monday 01 February 2021 16:20:18 Tom Rini wrote:
> On Wed, Jan 27, 2021 at 04:34:24PM +0100, Pali Rohár wrote:
> 
> > Header file version.h includes also autogenerated file timestamp.h which
> > is recompiled on every time when SOURCE_DATE_EPOCH change.
> > 
> > Tools do not use build time therefore they do not have to include
> > timestamp.h file.
> > 
> > This change prevents recompiling tools every time when SOURCE_DATE_EPOCH
> > changes.
> > 
> > Signed-off-by: Pali Rohár <pali at kernel.org>
> > ---
> >  tools/dumpimage.c  | 2 +-
> >  tools/mkenvimage.c | 2 +-
> >  tools/mkimage.c    | 2 +-
> >  3 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/dumpimage.c b/tools/dumpimage.c
> > index e5481435a7..54c2517c9e 100644
> > --- a/tools/dumpimage.c
> > +++ b/tools/dumpimage.c
> > @@ -7,7 +7,7 @@
> >  
> >  #include "dumpimage.h"
> >  #include <image.h>
> > -#include <version.h>
> > +#include "generated/version_autogenerated.h"
> >  
> >  static void usage(void);
> >  
> 
> I don't know if I really like this approach.  It also seems inconsistent
> as we have for example tools/fit-image.c that has content from
> generated/version_autogenerated.h but wasn't changed by this patch (nor
> the rest of the series).
> 
> I think a bit more invasive approach is required here.  I like the
> <version_string.h> approach you used elsewhere, and as best I can see,
> the only place U_BOOT_VERSION_STRING, which is where we have the
> timestamp itself, is used is cmd/version.c (and defined in version.h).
> We should isolate the file that has the date such that it's only
> in one file and referenced in one file.  This means probably removing
> the version_string global from arch/{powerpc,m68k} and dropping __weak
> from cmd/version.c and seeing what happens, as well.

Hello! I have looked at dropping this weak property from version_string
but seem it is not so easy... powerpc platforms have following code:

file arch/powerpc/cpu/mpc85xx/start.S:

       .globl  _start
_start:
       .long   0x27051956              /* U-BOOT Magic Number */
       .globl  version_string
version_string:
       .ascii U_BOOT_VERSION_STRING, "\0"

I do not know how it is used, but for me it looks like that on these
platforms is somehow null-term U-Boot version string important to be put
after some magic number at specific location (start of the binary?).

How could we handle this situation? I do not have any of those powerpc
platforms and I do not know any details about them.

If we cannot remove version string from this location, it is possible to
tweak linker to put at this location version string from compiled
cmd/version.o object file (at the time when building final u-boot
binary)?

CCing powerpc maintainers.

> Do you see where I'm thinking with this, or do I need to try and explain
> a bit more?  Thanks!
> 
> -- 
> Tom