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

Pali Rohár
2021-08-08T11:37:05Z

Christophe, could you please review this change? According to your commit
https://source.denx.de/u-boot/u-boot/-/commit/907208c452999427cb2f43450308045bf8b42958
mpc8xx code will have to be maintained until at least 2027.

On Sunday 08 August 2021 13:20:38 Pali Rohár wrote:
> It is unknown why version string is placed at specific position on these
> powerpc mpc platforms.

Also, could you look at this, why version string is at specific location
of u-boot.bin header?

> 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".
>
> 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/mpc83xx/start.S b/arch/powerpc/cpu/mpc83xx/start.S
> index 9da22ce486a9..87747f2b2275 100644
> --- a/arch/powerpc/cpu/mpc83xx/start.S
> +++ b/arch/powerpc/cpu/mpc83xx/start.S
> @@ -13,7 +13,6 @@
> #include <asm-offsets.h>
> #include <config.h>
> #include <mpc83xx.h>
> -#include <version.h>
>
> #define CONFIG_83XX 1 /* needed for Linux kernel header files*/
>
> @@ -76,7 +75,7 @@
> * times so the processor can fetch it out of flash whether the flash
> * is 8, 16, 32, or 64 bits wide (hardware trickery).
> */
> - .text
> + .section .text_pre
> #define _HRCW_TABLE_ENTRY(w) \
> .fill 8,1,(((w)>>24)&0xff); \
> .fill 8,1,(((w)>>16)&0xff); \
> @@ -92,12 +91,9 @@
> */
> .long 0x27051956 /* U-Boot Magic Number */
>
> - .globl version_string
> -version_string:
> - .ascii U_BOOT_VERSION_STRING, "\0"
> -
> - .align 2
> +/* U-Boot version string is filled at this place by linker script */
>
> + .text
> .globl enable_addr_trans
> enable_addr_trans:
> /* enable address translation */
> diff --git a/arch/powerpc/cpu/mpc83xx/u-boot.lds b/arch/powerpc/cpu/mpc83xx/u-boot.lds
> index d10f528da4c4..309082bc3df5 100644
> --- a/arch/powerpc/cpu/mpc83xx/u-boot.lds
> +++ b/arch/powerpc/cpu/mpc83xx/u-boot.lds
> @@ -10,6 +10,9 @@ SECTIONS
> /* Read-only sections, merged into text segment: */
> .text :
> {
> + arch/powerpc/cpu/mpc83xx/start.o (.text_pre)
> + *(.text_version_string)
> + . = ALIGN(2);
> arch/powerpc/cpu/mpc83xx/start.o (.text*)
> *(.text*)
> . = ALIGN(16);
> diff --git a/arch/powerpc/cpu/mpc85xx/start.S b/arch/powerpc/cpu/mpc85xx/start.S
> index f41e82ad189f..5ba26d3c449b 100644
> --- a/arch/powerpc/cpu/mpc85xx/start.S
> +++ b/arch/powerpc/cpu/mpc85xx/start.S
> @@ -14,7 +14,6 @@
> #include <asm-offsets.h>
> #include <config.h>
> #include <mpc85xx.h>
> -#include <version.h>
>
> #include <ppc_asm.tmpl>
> #include <ppc_defs.h>
> @@ -1134,15 +1133,14 @@ switch_as:
> blr
> #endif
>
> - .text
> + .section .text_pre
> .globl _start
> _start:
> .long 0x27051956 /* U-BOOT Magic Number */
> - .globl version_string
> -version_string:
> - .ascii U_BOOT_VERSION_STRING, "\0"
>
> - .align 4
> +/* U-Boot version string is filled at this place by linker script */
> +
> + .text
> .globl _start_cont
> _start_cont:
> /* Setup the stack in initial RAM,could be L2-as-SRAM or L1 dcache*/
> diff --git a/arch/powerpc/cpu/mpc85xx/u-boot-nand.lds b/arch/powerpc/cpu/mpc85xx/u-boot-nand.lds
> index 75b0285e4e51..6e48223380cb 100644
> --- a/arch/powerpc/cpu/mpc85xx/u-boot-nand.lds
> +++ b/arch/powerpc/cpu/mpc85xx/u-boot-nand.lds
> @@ -25,6 +25,10 @@ SECTIONS
> .interp : { *(.interp) }
> .text :
> {
> + arch/powerpc/cpu/mpc85xx/start.o (.text_pre)
> + *(.text_version_string)
> + . = ALIGN(4);
> + arch/powerpc/cpu/mpc85xx/start.o (.text*)
> *(.text*)
> } :text
> _etext = .;
> diff --git a/arch/powerpc/cpu/mpc85xx/u-boot-spl.lds b/arch/powerpc/cpu/mpc85xx/u-boot-spl.lds
> index 27a5fe6306a3..2312cd47f11d 100644
> --- a/arch/powerpc/cpu/mpc85xx/u-boot-spl.lds
> +++ b/arch/powerpc/cpu/mpc85xx/u-boot-spl.lds
> @@ -20,6 +20,10 @@ SECTIONS
> {
> . = IMAGE_TEXT_BASE;
> .text : {
> + arch/powerpc/cpu/mpc85xx/start.o (.text_pre)
> + *(.text_version_string)
> + . = ALIGN(4);
> + arch/powerpc/cpu/mpc85xx/start.o (.text*)
> *(.text*)
> }
> _etext = .;
> diff --git a/arch/powerpc/cpu/mpc85xx/u-boot.lds b/arch/powerpc/cpu/mpc85xx/u-boot.lds
> index 22bbac51aa33..40d181ef2caa 100644
> --- a/arch/powerpc/cpu/mpc85xx/u-boot.lds
> +++ b/arch/powerpc/cpu/mpc85xx/u-boot.lds
> @@ -31,6 +31,10 @@ SECTIONS
> .interp : { *(.interp) }
> .text :
> {
> + arch/powerpc/cpu/mpc85xx/start.o (.text_pre)
> + *(.text_version_string)
> + . = ALIGN(4);
> + arch/powerpc/cpu/mpc85xx/start.o (.text*)
> *(.text*)
> } :text
> _etext = .;
> 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 */
> - .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*)
> --
> 2.20.1
>