[U-Boot] [PATCH v1] arm: remove unneeded symbol offsets and _TEXT_BASE

Albert ARIBAUD
2013-10-13T08:03:22Z

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.