[U-Boot] [PATCH 4/8] x86: fsp: Rename shared_data to fsp_config_data

[Simon Glass]

Hi Bin,

On 2 December 2015 at 01:59, Bin Meng <bmeng.cn at gmail.com> wrote:
> FSP has several config data like UPD, HDA verb table which can be
> overridden or provided by bootloader. Currently in U-Boot only UPD
> is handled via struct shared_data. To accommodate any platform, we
> rename shared_data to fsp_config_data and move the definition from
> common place fsp_support.h to platform-specific place fsp_configs.h.
>
> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
> ---
>
>  arch/x86/include/asm/arch-baytrail/fsp/fsp_configs.h  | 17 +++++++++++++++++
>  arch/x86/include/asm/arch-queensbay/fsp/fsp_configs.h | 17 +++++++++++++++++
>  arch/x86/include/asm/fsp/fsp_support.h                |  8 +-------
>  arch/x86/lib/fsp/fsp_support.c                        | 10 +++++-----
>  4 files changed, 40 insertions(+), 12 deletions(-)
>  create mode 100644 arch/x86/include/asm/arch-baytrail/fsp/fsp_configs.h
>  create mode 100644 arch/x86/include/asm/arch-queensbay/fsp/fsp_configs.h
>
> diff --git a/arch/x86/include/asm/arch-baytrail/fsp/fsp_configs.h b/arch/x86/include/asm/arch-baytrail/fsp/fsp_configs.h
> new file mode 100644
> index 0000000..2397553
> --- /dev/null
> +++ b/arch/x86/include/asm/arch-baytrail/fsp/fsp_configs.h
> @@ -0,0 +1,17 @@
> +/*
> + * Copyright (C) 2015, Bin Meng <bmeng.cn at gmail.com>
> + *
> + * SPDX-License-Identifier:    Intel
> + */
> +
> +#ifndef __FSP_CONFIGS_H__
> +#define __FSP_CONFIGS_H__

Does this justify its own header file? I suppose it does...we have too
many fsp header files and I never know which file contains a
particular struct definition.

> +
> +struct fsp_config_data {
> +       struct fsp_header       *fsp_hdr;
> +       u32                     stack_top;
> +       u32                     boot_mode;
> +       struct upd_region       fsp_upd;

These are common - should we have a common struct as the first member?

I'm just struggling to understand the benefit of duplicating this
common thing into separate files.

> +};
> +
> +#endif /* __FSP_CONFIGS_H__ */
> diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_configs.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_configs.h
> new file mode 100644
> index 0000000..2397553
> --- /dev/null
> +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_configs.h
> @@ -0,0 +1,17 @@
> +/*
> + * Copyright (C) 2015, Bin Meng <bmeng.cn at gmail.com>
> + *
> + * SPDX-License-Identifier:    Intel
> + */
> +
> +#ifndef __FSP_CONFIGS_H__
> +#define __FSP_CONFIGS_H__
> +
> +struct fsp_config_data {
> +       struct fsp_header       *fsp_hdr;
> +       u32                     stack_top;
> +       u32                     boot_mode;
> +       struct upd_region       fsp_upd;
> +};
> +
> +#endif /* __FSP_CONFIGS_H__ */
> diff --git a/arch/x86/include/asm/fsp/fsp_support.h b/arch/x86/include/asm/fsp/fsp_support.h
> index 18e2d21..39b2864 100644
> --- a/arch/x86/include/asm/fsp/fsp_support.h
> +++ b/arch/x86/include/asm/fsp/fsp_support.h
> @@ -17,13 +17,7 @@
>  #include "fsp_infoheader.h"
>  #include "fsp_bootmode.h"
>  #include <asm/arch/fsp/fsp_vpd.h>
> -
> -struct shared_data {
> -       struct fsp_header       *fsp_hdr;
> -       u32                     stack_top;
> -       u32                     boot_mode;
> -       struct upd_region       fsp_upd;
> -};
> +#include <asm/arch/fsp/fsp_configs.h>
>
>  #define FSP_LOWMEM_BASE                0x100000UL
>  #define FSP_HIGHMEM_BASE       0x100000000ULL
> diff --git a/arch/x86/lib/fsp/fsp_support.c b/arch/x86/lib/fsp/fsp_support.c
> index 083d855..9da720b 100644
> --- a/arch/x86/lib/fsp/fsp_support.c
> +++ b/arch/x86/lib/fsp/fsp_support.c
> @@ -99,7 +99,7 @@ void fsp_continue(u32 status, void *hob_list)
>
>  void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
>  {
> -       struct shared_data shared_data;
> +       struct fsp_config_data config_data;
>         fsp_init_f init;
>         struct fsp_init_params params;
>         struct fspinit_rtbuf rt_buf;
> @@ -118,7 +118,7 @@ void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
>                 panic("Invalid FSP header");
>         }
>
> -       fsp_upd = &shared_data.fsp_upd;
> +       fsp_upd = &config_data.fsp_upd;
>         memset(&rt_buf, 0, sizeof(struct fspinit_rtbuf));
>
>         /* Reserve a gap in stack top */
> @@ -151,9 +151,9 @@ void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
>         init = (fsp_init_f)(fsp_hdr->img_base + fsp_hdr->fsp_init);
>         params_ptr = ¶ms;
>
> -       shared_data.fsp_hdr = fsp_hdr;
> -       shared_data.stack_top = stack_top;
> -       shared_data.boot_mode = boot_mode;
> +       config_data.fsp_hdr = fsp_hdr;
> +       config_data.stack_top = stack_top;
> +       config_data.boot_mode = boot_mode;
>
>         post_code(POST_PRE_MRC);
>
> --
> 1.8.2.1
>

Regards,
Simon