On Tuesday 24 August 2021 07:28:07 Christophe Leroy wrote:
> Le 08/08/2021 à 13:20, Pali Rohár a écrit :
> > It is unknown why version string is placed at specific position on these
> > powerpc mpc platforms. But there is no need to overload version_string
> > symbol. Just use common definition of version_string and modify linker
> > script to put it at "correct place".
>
> What's the benefit of this change ?
Removal of the _weak_ property of version_string symbol to cleanup
U-Boot code, which allows U-Boot build system on second build run to
recompile only files which were really changed.
> I see more insertions than deletions, it
> doesn't remove any redundancy either it seems. And it provides a strange
> look to u-boot.lds
It is only to ensure that version_string is defined only at one place in
U-Boot sources. And for this kind of things it is required to use linker
hacks.
> I think we should probably investigate the reason for the version at that
> specific place if other architectures don't do the same.
I agree. This is really the only platform which has version_string at
specific place.
> >
> > Signed-off-by: Pali Rohár <p...@kernel.org>
> >
> > ---
> > Changes in v2:
> > * Put explicit ".section" keyword before declaring ".text_pre" section as
> > some gcc versions cannot recognize specifying custom section without it.
> > * Tested compilation for:
> > $ make CROSS_COMPILE=powerpc-linux-gnu- MCR3000_defconfig u-boot.bin
> > and checked that u-boot.bin header is same with and without this patch
> > ---
> > arch/powerpc/cpu/mpc83xx/start.S | 10 +++-------
> > arch/powerpc/cpu/mpc83xx/u-boot.lds | 3 +++
> > arch/powerpc/cpu/mpc85xx/start.S | 10 ++++------
> > arch/powerpc/cpu/mpc85xx/u-boot-nand.lds | 4 ++++
> > arch/powerpc/cpu/mpc85xx/u-boot-spl.lds | 4 ++++
> > arch/powerpc/cpu/mpc85xx/u-boot.lds | 4 ++++
> > arch/powerpc/cpu/mpc8xx/start.S | 9 ++++-----
> > board/cssi/MCR3000/u-boot.lds | 2 ++
> > 8 files changed, 28 insertions(+), 18 deletions(-)
> >
> > diff --git a/arch/powerpc/cpu/mpc8xx/start.S
> > b/arch/powerpc/cpu/mpc8xx/start.S
> > index ed735cdee005..09628a0d60f0 100644
> > --- a/arch/powerpc/cpu/mpc8xx/start.S
> > +++ b/arch/powerpc/cpu/mpc8xx/start.S
> > @@ -23,7 +23,6 @@
> > #include <asm-offsets.h>
> > #include <config.h>
> > #include <mpc8xx.h>
> > -#include <version.h>
> > #include <ppc_asm.tmpl>
> > #include <ppc_defs.h>
> > @@ -60,12 +59,12 @@
> > * r3 - 1st arg to board_init(): IMMP pointer
> > * r4 - 2nd arg to board_init(): boot flag
> > */
> > - .text
> > + .section .text_pre
> > .long 0x27051956 /* U-Boot Magic Number
> > */
>
> Instead of creating a new artificial section, maybe you can add the magic
> number in front of the version string directly in u-boot.lds ?
Well, this patch series is about version_string. I do not want to touch
other parts, like magic numbers in this patch series as it is not
related to version string in build system.
> > - .globl version_string
> > -version_string:
> > - .ascii U_BOOT_VERSION_STRING, "\0"
> > +/* U-Boot version string is filled at this place by linker script */
> > +
> > + .text
> > . = EXC_OFF_SYS_RESET
> > .globl _start
> > _start:
> > diff --git a/board/cssi/MCR3000/u-boot.lds b/board/cssi/MCR3000/u-boot.lds
> > index 70aef3241c8e..9b2ead29b4a5 100644
> > --- a/board/cssi/MCR3000/u-boot.lds
> > +++ b/board/cssi/MCR3000/u-boot.lds
> > @@ -16,6 +16,8 @@ SECTIONS
> > . = + SIZEOF_HEADERS;
> > .text :
> > {
> > + arch/powerpc/cpu/mpc8xx/start.o (.text_pre)
> > + *(.text_version_string)
> > arch/powerpc/cpu/mpc8xx/start.o (.text)
> > arch/powerpc/cpu/mpc8xx/traps.o (.text*)
> > arch/powerpc/lib/built-in.o (.text*)
> >
