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

Sun Aug 8 13:36:51 CEST 2021

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