[coreboot-gerrit] Patch set updated for coreboot: romstage_handoff: remove code duplication

Aaron Durbin (adurbin@chromium.org) gerrit at coreboot.org
Wed Nov 30 05:55:16 CET 2016


Aaron Durbin (adurbin at chromium.org) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17646

-gerrit

commit 21ad886755af4e7bf7343ac09e59be51596600f9
Author: Aaron Durbin <adurbin at chromium.org>
Date:   Tue Nov 29 17:43:04 2016 -0600

    romstage_handoff: remove code duplication
    
    The same pattern was being used throughout the code base
    for initializing the romstage handoff structure. Provide
    a helper function to initialize the structure with the S3
    resume state then utilize it at all the existing call sites.
    
    Change-Id: I1e9d588ab6b9ace67757387dbb5963ae31ceb252
    Signed-off-by: Aaron Durbin <adurbin at chromium.org>
---
 src/cpu/amd/car/post_cache_as_ram.c            |  9 +-----
 src/cpu/intel/haswell/romstage.c               |  7 +----
 src/drivers/intel/fsp1_1/romstage.c            | 10 ++----
 src/drivers/intel/fsp2_0/memory_init.c         |  7 +----
 src/include/romstage_handoff.h                 | 43 +++++++++++++++++++++++---
 src/northbridge/intel/sandybridge/early_init.c |  8 +----
 src/soc/intel/baytrail/romstage/romstage.c     |  7 +----
 src/soc/intel/broadwell/romstage/romstage.c    |  9 +-----
 src/soc/intel/fsp_baytrail/romstage/romstage.c |  7 +----
 9 files changed, 47 insertions(+), 60 deletions(-)

diff --git a/src/cpu/amd/car/post_cache_as_ram.c b/src/cpu/amd/car/post_cache_as_ram.c
index 296adc9..88b8637 100644
--- a/src/cpu/amd/car/post_cache_as_ram.c
+++ b/src/cpu/amd/car/post_cache_as_ram.c
@@ -129,14 +129,7 @@ void post_cache_as_ram(void)
 
 	prepare_romstage_ramstack(s3resume);
 
-	if (IS_ENABLED(CONFIG_EARLY_CBMEM_INIT)) {
-		struct romstage_handoff *handoff;
-		handoff = romstage_handoff_find_or_add();
-		if (handoff != NULL)
-			handoff->s3_resume = s3resume;
-		else
-			printk(BIOS_DEBUG, "Romstage handoff structure not added!\n");
-	}
+	romstage_handoff_init(s3resume);
 
 	/* from here don't store more data in CAR */
 	if (family >= 0x1f && family <= 0x3f) {
diff --git a/src/cpu/intel/haswell/romstage.c b/src/cpu/intel/haswell/romstage.c
index 48920b3..7eb115c 100644
--- a/src/cpu/intel/haswell/romstage.c
+++ b/src/cpu/intel/haswell/romstage.c
@@ -172,7 +172,6 @@ void romstage_common(const struct romstage_params *params)
 {
 	int boot_mode;
 	int wake_from_s3;
-	struct romstage_handoff *handoff;
 
 	timestamp_init(get_initial_timestamp());
 	timestamp_add_now(TS_START_ROMSTAGE);
@@ -245,11 +244,7 @@ void romstage_common(const struct romstage_params *params)
 	#endif
 	}
 
-	handoff = romstage_handoff_find_or_add();
-	if (handoff != NULL)
-		handoff->s3_resume = wake_from_s3;
-	else
-		printk(BIOS_DEBUG, "Romstage handoff structure not added!\n");
+	romstage_handoff_init(wake_from_s3);
 
 	post_code(0x3f);
 	if (IS_ENABLED(CONFIG_LPC_TPM)) {
diff --git a/src/drivers/intel/fsp1_1/romstage.c b/src/drivers/intel/fsp1_1/romstage.c
index 97379b2..b222082 100644
--- a/src/drivers/intel/fsp1_1/romstage.c
+++ b/src/drivers/intel/fsp1_1/romstage.c
@@ -99,7 +99,6 @@ void *cache_as_ram_stage_main(FSP_INFO_HEADER *fih)
 void romstage_common(struct romstage_params *params)
 {
 	const struct mrc_saved_data *cache;
-	struct romstage_handoff *handoff;
 	struct pei_data *pei_data;
 
 	post_code(0x32);
@@ -165,14 +164,9 @@ void romstage_common(struct romstage_params *params)
 	mainboard_save_dimm_info(params);
 
 	/* Create romstage handof information */
-	handoff = romstage_handoff_find_or_add();
-	if (handoff != NULL)
-		handoff->s3_resume = (params->power_state->prev_sleep_state ==
-				      ACPI_S3);
-	else {
-		printk(BIOS_DEBUG, "Romstage handoff structure not added!\n");
+	if (romstage_handoff_init(
+			params->power_state->prev_sleep_state == ACPI_S3) < 0)
 		hard_reset();
-	}
 
 	/*
 	 * Initialize the TPM, unless the TPM was already initialized
diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c
index b833561..c0cb5d3 100644
--- a/src/drivers/intel/fsp2_0/memory_init.c
+++ b/src/drivers/intel/fsp2_0/memory_init.c
@@ -117,7 +117,6 @@ static void save_memory_training_data(bool s3wake, uint32_t fsp_version)
 static void do_fsp_post_memory_init(bool s3wake, uint32_t fsp_version)
 {
 	struct range_entry fsp_mem;
-	struct romstage_handoff *handoff;
 
 	if (fsp_find_reserved_memory(&fsp_mem))
 		die("Failed to find FSP_RESERVED_MEMORY_RESOURCE_HOB!\n");
@@ -144,11 +143,7 @@ static void do_fsp_post_memory_init(bool s3wake, uint32_t fsp_version)
 	save_memory_training_data(s3wake, fsp_version);
 
 	/* Create romstage handof information */
-	handoff = romstage_handoff_find_or_add();
-	if (handoff != NULL)
-		handoff->s3_resume = s3wake;
-	else
-		printk(BIOS_SPEW, "Romstage handoff structure not added!\n");
+	romstage_handoff_init(s3wake);
 }
 
 static const char *mrc_cache_get_region_name(void)
diff --git a/src/include/romstage_handoff.h b/src/include/romstage_handoff.h
index 4aba2ce..175cf09 100644
--- a/src/include/romstage_handoff.h
+++ b/src/include/romstage_handoff.h
@@ -18,6 +18,8 @@
 #include <stdint.h>
 #include <string.h>
 #include <cbmem.h>
+#include <console/console.h>
+#include <rules.h>
 
 /* It is the chipset's responsibility for maintaining the integrity of this
  * structure in CBMEM. For instance, if chipset code adds this structure
@@ -34,6 +36,15 @@ struct romstage_handoff {
 	uint8_t reserved[2];
 };
 
+static inline int cbmem_available(void)
+{
+	/* No cbmem available prior to ramstage on non-early cbmem platforms. */
+	if (!ENV_RAMSTAGE && !IS_ENABLED(CONFIG_EARLY_CBMEM_INIT))
+		return 0;
+
+	return 1;
+}
+
 /* The romstage_handoff_find_or_add() function provides the necessary logic
  * for initializing the romstage_handoff structure in cbmem. Different components
  * of the romstage may be responsible for setting up different fields. Therefore
@@ -43,18 +54,40 @@ static inline struct romstage_handoff *romstage_handoff_find_or_add(void)
 {
 	struct romstage_handoff *handoff;
 
+	if (!cbmem_available())
+		return NULL;
+
 	/* cbmem_add() first does a find and uses the old location before the
 	 * real add. However, it is important to know when the structure is not
 	 * found so it can be initialized to 0. */
 	handoff = cbmem_find(CBMEM_ID_ROMSTAGE_INFO);
 
-	if (handoff == NULL) {
-		handoff = cbmem_add(CBMEM_ID_ROMSTAGE_INFO, sizeof(*handoff));
-		if (handoff != NULL)
-			memset(handoff, 0, sizeof(*handoff));
-	}
+	if (handoff)
+		return handoff;
+
+	handoff = cbmem_add(CBMEM_ID_ROMSTAGE_INFO, sizeof(*handoff));
+
+	if (handoff != NULL)
+		memset(handoff, 0, sizeof(*handoff));
+	else
+		printk(BIOS_DEBUG, "Romstage handoff structure not added!\n");
 
 	return handoff;
 }
 
+/* Returns 0 if initialized. Else < 0 if handoff structure not added. */
+static inline int romstage_handoff_init(int is_s3_resume)
+{
+	struct romstage_handoff *handoff;
+
+	handoff = romstage_handoff_find_or_add();
+
+	if (handoff == NULL)
+		return -1;
+
+	handoff->s3_resume = is_s3_resume;
+
+	return 0;
+}
+
 #endif /* ROMSTAGE_HANDOFF_H */
diff --git a/src/northbridge/intel/sandybridge/early_init.c b/src/northbridge/intel/sandybridge/early_init.c
index ef97a1a..16ea29d 100644
--- a/src/northbridge/intel/sandybridge/early_init.c
+++ b/src/northbridge/intel/sandybridge/early_init.c
@@ -227,13 +227,7 @@ void sandybridge_early_initialization(int chipset_type)
 
 void northbridge_romstage_finalize(int s3resume)
 {
-	struct romstage_handoff *handoff;
-
 	MCHBAR16(SSKPD) = 0xCAFE;
 
-	handoff = romstage_handoff_find_or_add();
-	if (handoff != NULL)
-		handoff->s3_resume = s3resume;
-	else
-		printk(BIOS_DEBUG, "Romstage handoff structure not added!\n");
+	romstage_handoff_init(s3resume);
 }
diff --git a/src/soc/intel/baytrail/romstage/romstage.c b/src/soc/intel/baytrail/romstage/romstage.c
index a61b571..124eb6e 100644
--- a/src/soc/intel/baytrail/romstage/romstage.c
+++ b/src/soc/intel/baytrail/romstage/romstage.c
@@ -210,7 +210,6 @@ static int chipset_prev_sleep_state(struct chipset_power_state *ps)
 /* Entry from the mainboard. */
 void romstage_common(struct romstage_params *params)
 {
-	struct romstage_handoff *handoff;
 	struct chipset_power_state *ps;
 	int prev_sleep_state;
 
@@ -232,11 +231,7 @@ void romstage_common(struct romstage_params *params)
 
 	timestamp_add_now(TS_AFTER_INITRAM);
 
-	handoff = romstage_handoff_find_or_add();
-	if (handoff != NULL)
-		handoff->s3_resume = (prev_sleep_state == ACPI_S3);
-	else
-		printk(BIOS_DEBUG, "Romstage handoff structure not added!\n");
+	romstage_handoff_init(prev_sleep_state == ACPI_S3);
 
 	if (IS_ENABLED(CONFIG_LPC_TPM)) {
 		init_tpm(prev_sleep_state == ACPI_S3);
diff --git a/src/soc/intel/broadwell/romstage/romstage.c b/src/soc/intel/broadwell/romstage/romstage.c
index 44df88c..849c55d 100644
--- a/src/soc/intel/broadwell/romstage/romstage.c
+++ b/src/soc/intel/broadwell/romstage/romstage.c
@@ -89,8 +89,6 @@ void * asmlinkage romstage_main(unsigned long bist,
 /* Entry from the mainboard. */
 void romstage_common(struct romstage_params *params)
 {
-	struct romstage_handoff *handoff;
-
 	post_code(0x32);
 
 	timestamp_add_now(TS_BEFORE_INITRAM);
@@ -114,12 +112,7 @@ void romstage_common(struct romstage_params *params)
 
 	timestamp_add_now(TS_AFTER_INITRAM);
 
-	handoff = romstage_handoff_find_or_add();
-	if (handoff != NULL)
-		handoff->s3_resume = (params->power_state->prev_sleep_state ==
-				      ACPI_S3);
-	else
-		printk(BIOS_DEBUG, "Romstage handoff structure not added!\n");
+	romstage_handoff_init(params->power_state->prev_sleep_state == ACPI_S3);
 
 #if CONFIG_LPC_TPM
 	init_tpm(params->power_state->prev_sleep_state == ACPI_S3);
diff --git a/src/soc/intel/fsp_baytrail/romstage/romstage.c b/src/soc/intel/fsp_baytrail/romstage/romstage.c
index a7ed414..9d204d1 100644
--- a/src/soc/intel/fsp_baytrail/romstage/romstage.c
+++ b/src/soc/intel/fsp_baytrail/romstage/romstage.c
@@ -218,7 +218,6 @@ void romstage_main_continue(EFI_STATUS status, void *hob_list_ptr)
 	int cbmem_was_initted;
 	void *cbmem_hob_ptr;
 	uint32_t prev_sleep_state;
-	struct romstage_handoff *handoff;
 
 	timestamp_add_now(TS_AFTER_INITRAM);
 
@@ -257,11 +256,7 @@ void romstage_main_continue(EFI_STATUS status, void *hob_list_ptr)
 	*(u32*)cbmem_hob_ptr = (u32)hob_list_ptr;
 	post_code(0x4e);
 
-	handoff = romstage_handoff_find_or_add();
-	if (handoff != NULL)
-		handoff->s3_resume = (prev_sleep_state == ACPI_S3);
-	else
-		printk(BIOS_DEBUG, "Romstage handoff structure not added!\n");
+	romstage_handoff_init(prev_sleep_state == ACPI_S3);
 
 	post_code(0x4f);
 



More information about the coreboot-gerrit mailing list