[U-Boot] [PATCH] armv8: spl: Call board_init_r from crt0_64 in SPL

[Simon Glass]

Hi Jeremy,

On 11 July 2016 at 16:55, Jeremy Hunt <Jeremy.Hunt at deshawresearch.com> wrote:
> As part of the startup process for boards using the SPL, the
> meaning of board_init_f changed such that it should return normally
> rather than calling board_init_r directly.
> (see db910353a126d84fe8dff7a694ea792f50fcfb6a)
> This was fixed in 32-bit arm, but broke when SPL was added to
> 64 bit arm. This fixes crt0_64 so that it calls board_init_r
> durring the SPL and removes the direct call from board_init_f
> from the arm SPL example.
>
> Signed-off-by: Jeremy Hunt <Jeremy.Hunt at DEShawResearch.com>
>
> ---
>
>  arch/arm/lib/crt0_64.S |  3 +--
>  arch/arm/lib/spl.c     | 18 +++++++++---------
>  2 files changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/lib/crt0_64.S b/arch/arm/lib/crt0_64.S
> index cad22c7..91b19e0 100644
> --- a/arch/arm/lib/crt0_64.S
> +++ b/arch/arm/lib/crt0_64.S
> @@ -108,6 +108,7 @@ relocation_return:
>   * Set up final (full) environment
>   */
>         bl      c_runtime_cpu_setup             /* still call old routine */
> +#endif /* !CONFIG_SPL_BUILD */
>
>  /* TODO: For SPL, call spl_relocate_stack_gd() to alloc stack relocation */
>
> @@ -130,6 +131,4 @@ clear_loop:
>
>         /* NOTREACHED - board_init_r() does not return */
>
> -#endif /* !CONFIG_SPL_BUILD */
> -
>  ENDPROC(_main)
> diff --git a/arch/arm/lib/spl.c b/arch/arm/lib/spl.c
> index e428868..b188834 100644
> --- a/arch/arm/lib/spl.c
> +++ b/arch/arm/lib/spl.c
> @@ -25,22 +25,22 @@ gd_t gdata __attribute__ ((section(".data")));
>  #endif
>
>  /*
> - * In the context of SPL, board_init_f must ensure that any clocks/etc for
> - * DDR are enabled, ensure that the stack pointer is valid, clear the BSS
> - * and call board_init_r.  We provide this version by default but mark it
> - * as __weak to allow for platforms to do this in their own way if needed.
> + * In the context of SPL, board_init_f() prepares the hardware for execution
> + * from system RAM (DRAM, DDR...). As system RAM may not be available yet,
> + * board_init_f() must use the current GD to store any data which must be
> + * passed on to later stages. These data include the relocation destination,
> + * the future stack, and the future GD location. BSS is cleared after this
> + * function (and therefore must be accessible).
> + *
> + * We provide this version by default but mark it as __weak to allow for
> + * platforms to do this in their own way if needed.

Can you please add a comment pointing to the README also ("Board
Initialisation Flow")?

>   */
>  void __weak board_init_f(ulong dummy)
>  {
> -       /* Clear the BSS. */
> -       memset(__bss_start, 0, __bss_end - __bss_start);
> -
>  #ifndef CONFIG_SPL_DM
>         /* TODO: Remove settings of the global data pointer here */
>         gd = &gdata;
>  #endif
> -

This is a good change, but if you are dropping the call to
board_init_r() then you should drop the above code also (along with
the gdata variable). I doubt any more is using it now as it has been a
while. So this weak function should be empty.

> -       board_init_r(NULL, 0);
>  }
>
>  /*
> --
> 2.4.4
>

Regards,
Simon