Re: [PATCH v2] powerpc: mpc: Put U-Boot version string at correct place by linker script

Christophe Leroy
2021-08-24T05:28:17Z

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

I think we should probably investigate the reason for the version at that specific place if other
architectures don't do the same.

>
> Signed-off-by: Pali Rohár <pali@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 ?

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