From mboxrd@z Thu Jan  1 00:00:00 1970
From: Albert ARIBAUD <albert.u.boot@aribaud.net>
Date: Sun, 13 Oct 2013 10:03:22 +0200
Subject: [U-Boot] [PATCH v1] arm: remove unneeded symbol offsets and
	_TEXT_BASE
In-Reply-To: <1381648228-24373-1-git-send-email-albert.u.boot@aribaud.net>
References: <1381648228-24373-1-git-send-email-albert.u.boot@aribaud.net>
Message-ID: <20131013100322.5a15798b@lilith>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Sun, 13 Oct 2013 09:10:28 +0200, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:

> Remove the last uses of symbol offsets in ARM U-Boot.
> Remove some needless uses of _TEXT_BASE.
> Remove all _TEXT_BASE definitions.
> 
> Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
> ---
> _TEXT_BASE was only used by ARM to allow resolution of
> symbol offsets, themselves only needed due to absolute
> relocations.
> 
> In some places, _TEXT_BASE was locally defined only
> to provide a literal for CONFIG_SYS_TEXT_BASE when the
> latter could have been used directly.
> 
> Sometimes even, _TEXT_BASE was defined but unused.
> 
> Since all relocations in ARM are relative, offsets,
> _TEXT_BASE and CONFIG_SYS_SYM_OFFSETS can be completely
> removed, and their uses can be replaced with adequate
> use of compiler-generated symbols from sections.c file.

Of course, I've done some non-regression testing before submitting, but
this should undergo heavier testing by people with various ARM
hardware. Considering the changes, checks should concentrate on:

- monitor_flash_len;
- gd fields mon_len, fdt_blob, and reloc_off;
- targets the lowlevel_init.S of which was moified.

Also note two things I've noticed only after posting v1:

> diff --git a/README b/README
> index 0bb1ad5..1f9e16f 100644
> --- a/README
> +++ b/README
> @@ -3522,12 +3522,6 @@ Configuration Settings:
>  	its config.mk file). If you find problems enabling this option on
>  	your board please report the problem and send patches!
>  
> -- CONFIG_SYS_SYM_OFFSETS
> -	This is set by architectures that use offsets for link symbols
> -	instead of absolute values. So bss_start is obtained using an
> -	offset _bss_start_ofs from CONFIG_SYS_TEXT_BASE, rather than
> -	directly. You should not need to touch this setting.

The removal of CONFIG_SYS_SYM_OFFSETS from the codebase is not mentioned
in the commit message. Will add in v2.

> diff --git a/arch/arm/cpu/arm926ejs/at91/lowlevel_init.S b/arch/arm/cpu/arm926ejs/at91/lowlevel_init.S
> index 39de2f4..7d1a633 100644
> --- a/arch/arm/cpu/arm926ejs/at91/lowlevel_init.S
> +++ b/arch/arm/cpu/arm926ejs/at91/lowlevel_init.S
> @@ -26,27 +26,18 @@
>  #define CONFIG_SYS_MATRIX_EBICSA_VAL CONFIG_SYS_MATRIX_EBI0CSA_VAL
>  #endif
>  
> -_TEXT_BASE:
> -	.word	CONFIG_SYS_TEXT_BASE
> -
>  .globl lowlevel_init
>  .type lowlevel_init,function
>  lowlevel_init:
>  
> -	mov	r5, pc		/* r5 = POS1 + 4 current */
>  POS1:
> +	adr	r5, POS1	/* r5 = POS1 run time */
>  	ldr	r0, =POS1	/* r0 = POS1 compile */

This could in fact use the existing lowlevel_init label rather than
introduce POS1 for this purpose. Will fix in v2.

Amicalement,
-- 
Albert.