[U-Boot] [PATCH v4] Fix board init code to use a valid C runtime environment

Alexey Brodkin
2015-11-16T13:12:15Z

Hi Albert,

On Sun, 2015-11-15 at 19:25 +0100, Albert ARIBAUD wrote:
> board_init_f_mem() alters the C runtime environment's
> stack it ls actually already using. This is not a valid
> C runtime environment.
>
> Split board_init_f_mem into C functions which do not
> alter their own stack and therefore run in a valid C
> runtime environment.
>
> Signed-off-by: Albert ARIBAUD <albert.u.boot@aribaud.net>
> ---
> Cc:ing custodians for all architectures which this
> patch affects.
>
> This patch has been build-tested for all there
> architectures, and run-tested on ARM OpenRD Client.
>
> For NIOS2, this patch contains all manual v1 and v2
> fixes by Thomas.
>
> For x86, this patch contains all manual v2 fixes by Bin.
>
> Changes in v4:
> - Add comments on reserving GD at the lowest location
> - Add comments on post-incrementing base for each "chunk"
>
> Changes in v3:
> - Rebase after commit 9ac4fc82
> - Simplify malloc conditional as per commit 9ac4fc82
> - Fix NIOS2 return value register (r2, not r4)
> - Fix x86 subl argument inversion
> - Group allocations in a single function
> - Group initializations in a single function
>
> Changes in v2:
> - Fix all checkpatch issues
> - Fix board_init_f_malloc prototype mismatch
> - Fix board_init_[f_]xxx typo in NIOS2
> - Fix aarch64 asm 'sub' syntax error
>
> arch/arc/lib/start.S | 12 +++--
> arch/arm/lib/crt0.S | 6 ++-
> arch/arm/lib/crt0_64.S | 6 ++-
> arch/microblaze/cpu/start.S | 4 +-
> arch/nios2/cpu/start.S | 13 +++--
> arch/powerpc/cpu/ppc4xx/start.S | 10 ++--
> arch/x86/cpu/start.S | 5 +-
> arch/x86/lib/fsp/fsp_common.c | 4 +-
> common/init/board_init.c | 108 +++++++++++++++++++++++++++++++++++-----
> include/common.h | 33 +++++-------
> 10 files changed, 144 insertions(+), 57 deletions(-)
>
> diff --git a/arch/arc/lib/start.S b/arch/arc/lib/start.S
> index 26a5934..2a99193 100644
> --- a/arch/arc/lib/start.S
> +++ b/arch/arc/lib/start.S
> @@ -54,14 +54,16 @@ ENTRY(_start)
> mov %sp, CONFIG_SYS_INIT_SP_ADDR
> mov %fp, %sp
>
> - /* Allocate and zero GD, update SP */
> - mov %r0, %sp
> - bl board_init_f_mem
> -
> + /* Get reserved area size, update SP and FP */
> + bl board_init_f_get_reserve_size
> /* Update stack- and frame-pointers */

I think we don't need to mention SP/FP update in comments twice here.
I.e. either strip ", update SP and FP" from your introduced comment or
which I really like more remove following line with comment entirely:
---------->8----------
/* Update stack- and frame-pointers */
---------->8----------

Not sure if this tiny nitpick worth respinning thought.

Otherwise build and run tested on free nSIM,
see "Running U-Boot on ARC in Free nSIM" section in
http://www.denx.de/wiki/U-Boot/ARCNotes if of any interest how to do it
yourself.

Feel free to add
---------->8----------
Acked-by: Alexey Brodkin <abrodkin@synopsys.com>
---------->8----------

-Alexey