[U-Boot] [Patch v3 1/2] common/board_f: Preserve early global data

[Simon Glass]

Hi York,

On 30 April 2014 18:01, York Sun <yorksun at freescale.com> wrote:
> Some platforms (including mpc85xx, mpc86xx) use global data before calling
> function baord_inti_f(). This data should not be cleared later. Any arch
> which needs to preserve global data should define CONFIG_SYS_EARLY_GD.
>
> Signed-off-by: York Sun <yorksun at freescale.com>
> CC: Scott Wood <scottwood at freescale.com>
> CC: Simon Glass <sjg at chromium.org>
> ---
> Change log
>
>  v3: Introduce CONFIG_SYS_EARLY_GD instead of using a list
>  v2: Instead of adding back gd init for all PPC, preserve gd for mpc85xx and mpc86xx.
>
>  README                            |    3 +++
>  arch/powerpc/include/asm/config.h |    2 ++
>  arch/x86/include/asm/config.h     |    1 +
>  common/board_f.c                  |   10 +++++-----
>  4 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/README b/README
> index 12758dc..1a51b11 100644
> --- a/README
> +++ b/README
> @@ -440,6 +440,9 @@ The following options need to be configured:
>                 supported, core will start to execute uboot when wakes up.
>
>  - Generic CPU options:
> +               CONFIG_SYS_EARLY_GD
> +               Defines global data is used before board_init_f().
> +

This looks like a good solution, but I believe that 'early' is in fact
where we are headed. So perhaps you should create an inverted option
and define it for archs that don't support it?

For what you have, I think CONFIG_SYS_EARLY_GLOBAL_DATA might be more
descriptive. Also I think you should describe the purpose more
precisely here - it's not just the use that you need to discuss.

- The CPU init code must set it up and zero it
- Then board_init_f() doesn't zero it

>                 CONFIG_SYS_BIG_ENDIAN, CONFIG_SYS_LITTLE_ENDIAN
>
>                 Defines the endianess of the CPU. Implementation of those
> diff --git a/arch/powerpc/include/asm/config.h b/arch/powerpc/include/asm/config.h
> index 423a6fb..4b6103b 100644
> --- a/arch/powerpc/include/asm/config.h
> +++ b/arch/powerpc/include/asm/config.h
> @@ -10,11 +10,13 @@
>  #ifdef CONFIG_MPC85xx
>  #include <asm/config_mpc85xx.h>
>  #define CONFIG_SYS_FSL_DDR
> +#define CONFIG_SYS_EARLY_GD
>  #endif
>
>  #ifdef CONFIG_MPC86xx
>  #include <asm/config_mpc86xx.h>
>  #define CONFIG_SYS_FSL_DDR
> +#define CONFIG_SYS_EARLY_GD
>  #endif
>
>  #ifdef CONFIG_MPC83xx
> diff --git a/arch/x86/include/asm/config.h b/arch/x86/include/asm/config.h
> index f06a15c..940461d 100644
> --- a/arch/x86/include/asm/config.h
> +++ b/arch/x86/include/asm/config.h
> @@ -7,5 +7,6 @@
>  #ifndef _ASM_CONFIG_H_
>  #define _ASM_CONFIG_H_
>
> +#define CONFIG_SYS_EARLY_GD
>  #define CONFIG_SYS_GENERIC_BOARD
>  #endif
> diff --git a/common/board_f.c b/common/board_f.c
> index cbdf06f..0823bed 100644
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -970,20 +970,20 @@ static init_fnc_t init_sequence_f[] = {
>
>  void board_init_f(ulong boot_flags)
>  {
> -#ifndef CONFIG_X86
> +       /*
> +        * For some archtecture, global data is initialized and used before
> +        * calling this function. The data should be preserved.
> +        */
> +#ifndef CONFIG_SYS_EARLY_GD
>         gd_t data;
>
>         gd = &data;
> -#endif
>
>         /*
>          * Clear global data before it is accessed at debug print
>          * in initcall_run_list. Otherwise the debug print probably
>          * get the wrong vaule of gd->have_console.
>          */
> -#if !defined(CONFIG_CPM2) && !defined(CONFIG_MPC512X) && \
> -               !defined(CONFIG_MPC83xx) && !defined(CONFIG_MPC85xx) && \
> -               !defined(CONFIG_MPC86xx) && !defined(CONFIG_X86)
>         zero_global_data();
>  #endif

As a separate issue/patch, I think this should be defined for ARM also.

Regards,
Simon