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

[Albert ARIBAUD <albert.u.boot@aribaud.net>]

Hello Thomas,

On Thu, 12 Nov 2015 13:59:28 +0800, Thomas Chou <thomas@wytron.com.tw>
wrote:
> Hi Albert,
> 
> On 2015?11?11? 02:30, 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>
> > ---
> > For NIOS2, this patch hopefully contains all manual
> > fixes by Thomas.
> >
> > 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            | 20 +++++++++++++---
> >   arch/arm/lib/crt0.S             | 10 ++++++--
> >   arch/arm/lib/crt0_64.S          | 10 ++++++--
> >   arch/microblaze/cpu/start.S     |  4 ++--
> >   arch/nios2/cpu/start.S          | 17 ++++++++++++--
> >   arch/powerpc/cpu/ppc4xx/start.S | 18 ++++++++++----
> >   arch/x86/cpu/start.S            | 10 ++++++--
> >   arch/x86/lib/fsp/fsp_common.c   |  4 ++--
> >   common/init/board_init.c        | 31 ++++++++++++++----------
> >   include/common.h                | 52 +++++++++++++++++++++++++----------------
> >   10 files changed, 125 insertions(+), 51 deletions(-)
> >
> 
> Additional fixes,
> ------------------------------------------------------------------------
> diff --git a/common/init/board_init.c b/common/init/board_init.c
> index 8839a4a..703e6d8 100644
> --- a/common/init/board_init.c
> +++ b/common/init/board_init.c
> @@ -46,6 +46,7 @@ void board_init_f_gd(struct global_data *gd_ptr)
>   	for (ptr = (int *)gd_ptr; ptr < (int *)(gd_ptr + 1); )
>   		*ptr++ = 0;
>   #endif
> +	arch_setup_gd(gd_ptr);

Correct -- in ARM (Thumb-1 at least) we cannot use arch_setup_gd() so
we set GD (in r9) from within arch/arm/lib/crt0.S, but for NIOS2 it
might (and apparently does) work. Where is GD stored in NIOS2?


 
>   }
> 
>   ulong board_init_f_malloc_size(void)
> --------------------------------------------------------------------------
> diff --git a/arch/nios2/cpu/start.S b/arch/nios2/cpu/start.S
> index c163ce1..0adff46 100644
> --- a/arch/nios2/cpu/start.S
> +++ b/arch/nios2/cpu/start.S
> @@ -110,7 +110,7 @@ _reloc:
>   	movhi	r2, %hi(board_init_f_gd_size at h)
>   	ori	r2, r2, %lo(board_init_f_gd_size at h)
>   	callr	r2
> -	sub	sp, sp, r4
> +	sub	sp, sp, r2

Sorry, didn't know / realize the NIOS2 ABI has r2 iass the function
value return register, not r4.

>   	mov	r4, sp
>   	movhi	r2, %hi(board_init_f_gd at h)
>   	ori	r2, r2, %lo(board_init_f_gd at h)
> @@ -119,16 +119,12 @@ _reloc:
>   	movhi	r2, %hi(board_init_f_malloc_size at h)
>   	ori	r2, r2, %lo(board_init_f_malloc_size at h)
>   	callr	r2
> -	sub	sp, sp, r4
> +	sub	sp, sp, r2

Ditto.

>   	mov	r4, sp
>   	movhi	r2, %hi(board_init_f_malloc at h)
>   	ori	r2, r2, %lo(board_init_f_malloc at h)
>   	callr	r2
> 
> -	/* Update stack- and frame-pointers */
> -	mov	sp, r2
> -	mov	fp, sp
> -

Oops.

>   	/* Call board_init_f -- never returns */
>   	mov	r4, r0
>   	movhi	r2, %hi(board_init_f at h)
> ----------------------------------------------------------------------------
> Otherwise,
> 
> Tested-by: Thomas Chou <thomas@wytron.com.tw>
> Acked-by: Thomas Chou <thomas@wytron.com.tw>
>
> Thanks.

Thanks to you; Will send a v3 to account for your and Simon's comments.

> Best regards,
> Thomas

Amicalement,
-- 
Albert.