Hi Bin,
On 2 December 2015 at 01:59, Bin Meng bmeng.cn@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@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@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@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