[coreboot-gerrit] Patch set updated for coreboot: drivers/intel/fsp2_0: FSP driver handles all FSP errors

Lee Leahy (leroy.p.leahy@intel.com) gerrit at coreboot.org
Sun Jul 31 02:17:47 CEST 2016


Lee Leahy (leroy.p.leahy at intel.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/15853

-gerrit

commit 99de3054cbf3b2d941dd2d63fad623fbac2e91b3
Author: Lee Leahy <leroy.p.leahy at intel.com>
Date:   Sun Jul 24 18:18:52 2016 -0700

    drivers/intel/fsp2_0: FSP driver handles all FSP errors
    
    Move all FSP error handling into the FSP 2.0 driver.  This removes the
    need to implement error handling within the SOC code.
    
    TEST=Build and run on Galileo Gen2
    
    Change-Id: I4d548b4c90d369d3857c24f50f93e7db7e9d3028
    Signed-off-by: Lee Leahy <leroy.p.leahy at intel.com>
---
 src/drivers/intel/fsp2_0/include/fsp/api.h |  6 +++---
 src/drivers/intel/fsp2_0/memory_init.c     | 29 ++++++++++++-----------------
 src/drivers/intel/fsp2_0/notify.c          | 11 +++++++----
 src/drivers/intel/fsp2_0/silicon_init.c    | 27 ++++++++++++---------------
 src/soc/intel/apollolake/chip.c            | 10 +++-------
 src/soc/intel/apollolake/romstage.c        |  6 +-----
 6 files changed, 38 insertions(+), 51 deletions(-)

diff --git a/src/drivers/intel/fsp2_0/include/fsp/api.h b/src/drivers/intel/fsp2_0/include/fsp/api.h
index bc55c50..1348ead 100644
--- a/src/drivers/intel/fsp2_0/include/fsp/api.h
+++ b/src/drivers/intel/fsp2_0/include/fsp/api.h
@@ -60,9 +60,9 @@ enum fsp_notify_phase {
 
 
 /* Main FSP stages */
-enum fsp_status fsp_memory_init(bool s3wake);
-enum fsp_status fsp_silicon_init(void);
-enum fsp_status fsp_notify(enum fsp_notify_phase phase);
+void fsp_memory_init(bool s3wake);
+void fsp_silicon_init(void);
+void fsp_notify(enum fsp_notify_phase phase);
 
 /* Callbacks for updating stage-specific parameters */
 void platform_fsp_memory_init_params_cb(struct FSPM_UPD *mupd);
diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c
index 78e3d81..0677e74 100644
--- a/src/drivers/intel/fsp2_0/memory_init.c
+++ b/src/drivers/intel/fsp2_0/memory_init.c
@@ -61,7 +61,7 @@ static void save_memory_training_data(bool s3wake, uint32_t fsp_version)
  */
 #define MRC_DEAD_VERSION		(0xdeaddead)
 
-static enum fsp_status do_fsp_post_memory_init(void *hob_list_ptr, bool s3wake,
+static void do_fsp_post_memory_init(void *hob_list_ptr, bool s3wake,
 						uint32_t fsp_version)
 {
 	struct range_entry fsp_mem;
@@ -85,7 +85,7 @@ static enum fsp_status do_fsp_post_memory_init(void *hob_list_ptr, bool s3wake,
 	/* make sure FSP memory is reserved in cbmem */
 	if (range_entry_base(&fsp_mem) !=
 		(uintptr_t)cbmem_find(CBMEM_ID_FSP_RESERVED_MEMORY))
-		die("Failed to accommodate FSP reserved memory request");
+		die("Failed to accommodate FSP reserved memory request!\n");
 
 	/* Now that CBMEM is up, save the list so ramstage can use it */
 	fsp_save_hob_list(hob_list_ptr);
@@ -101,8 +101,6 @@ static enum fsp_status do_fsp_post_memory_init(void *hob_list_ptr, bool s3wake,
 		handoff->s3_resume = s3wake;
 	else
 		printk(BIOS_DEBUG, "Romstage handoff structure not added!\n");
-
-	return FSP_SUCCESS;
 }
 
 static void fsp_fill_mrc_cache(struct FSPM_ARCH_UPD *arch_upd, bool s3wake,
@@ -182,7 +180,7 @@ static enum cb_err fsp_fill_common_arch_params(struct FSPM_ARCH_UPD *arch_upd,
 	return CB_SUCCESS;
 }
 
-static enum fsp_status do_fsp_memory_init(struct fsp_header *hdr, bool s3wake,
+static void do_fsp_memory_init(struct fsp_header *hdr, bool s3wake,
 					const struct memranges *memmap)
 {
 	enum fsp_status status;
@@ -196,8 +194,7 @@ static enum fsp_status do_fsp_memory_init(struct fsp_header *hdr, bool s3wake,
 	upd = (struct FSPM_UPD *)(hdr->cfg_region_offset + hdr->image_base);
 
 	if (upd->FspUpdHeader.Signature != FSPM_UPD_SIGNATURE) {
-		printk(BIOS_ERR, "Invalid FSPM signature\n");
-		return FSP_INCOMPATIBLE_VERSION;
+		die("Invalid FSPM signature!\n");
 	}
 
 	/* Copy the default values from the UPD area */
@@ -211,7 +208,7 @@ static enum fsp_status do_fsp_memory_init(struct fsp_header *hdr, bool s3wake,
 	/* Fill common settings on behalf of chipset. */
 	if (fsp_fill_common_arch_params(arch_upd, s3wake, hdr->fsp_revision,
 					memmap) != CB_SUCCESS)
-		return FSP_NOT_FOUND;
+		die("FSPM_ARCH_UPD not found!\n");
 
 	/* Give SoC and mainboard a chance to update the UPD */
 	platform_fsp_memory_init_params_cb(&fspm_upd);
@@ -229,13 +226,12 @@ static enum fsp_status do_fsp_memory_init(struct fsp_header *hdr, bool s3wake,
 
 	fsp_debug_memory_init(status, hob_list_ptr);
 
-	/* Handle any resets requested by FSPM. */
+	/* Handle any errors returned by FspMemoryInit */
 	fsp_handle_reset(status);
-
 	if (status != FSP_SUCCESS)
-		return status;
+		die("FspMemoryInit returned an error!\n");
 
-	return do_fsp_post_memory_init(hob_list_ptr, s3wake, hdr->fsp_revision);
+	do_fsp_post_memory_init(hob_list_ptr, s3wake, hdr->fsp_revision);
 }
 
 /* Load the binary into the memory specified by the info header. */
@@ -286,7 +282,7 @@ static enum cb_err load_fspm_xip(struct fsp_header *hdr,
 	return CB_SUCCESS;
 }
 
-enum fsp_status fsp_memory_init(bool s3wake)
+void fsp_memory_init(bool s3wake)
 {
 	struct fsp_header hdr;
 	enum cb_err status;
@@ -298,7 +294,7 @@ enum fsp_status fsp_memory_init(bool s3wake)
 
 	if (cbfs_boot_locate(&file_desc, name, NULL)) {
 		printk(BIOS_ERR, "Could not locate %s in CBFS\n", name);
-		return FSP_NOT_FOUND;
+		die("FSPM not available!\n");
 	}
 
 	cbfs_file_data(&file_data, &file_desc);
@@ -315,12 +311,11 @@ enum fsp_status fsp_memory_init(bool s3wake)
 		status = load_fspm_xip(&hdr, &file_data);
 
 	if (status != CB_SUCCESS) {
-		printk(BIOS_ERR, "Loading FSPM failed.\n");
-		return FSP_NOT_FOUND;
+		die("Loading FSPM failed!\n");
 	}
 
 	/* Signal that FSP component has been loaded. */
 	prog_segment_loaded(hdr.image_base, hdr.image_size, SEG_FINAL);
 
-	return do_fsp_memory_init(&hdr, s3wake, &memmap);
+	do_fsp_memory_init(&hdr, s3wake, &memmap);
 }
diff --git a/src/drivers/intel/fsp2_0/notify.c b/src/drivers/intel/fsp2_0/notify.c
index 139e172..b002cf8 100644
--- a/src/drivers/intel/fsp2_0/notify.c
+++ b/src/drivers/intel/fsp2_0/notify.c
@@ -17,14 +17,14 @@
 #include <string.h>
 #include <timestamp.h>
 
-enum fsp_status fsp_notify(enum fsp_notify_phase phase)
+void fsp_notify(enum fsp_notify_phase phase)
 {
 	enum fsp_status ret;
 	fsp_notify_fn fspnotify;
 	struct fsp_notify_params notify_params = { .phase = phase };
 
-	if (!fsps_hdr.silicon_init_entry_offset)
-		return FSP_NOT_FOUND;
+	if (!fsps_hdr.notify_phase_entry_offset)
+		die("Notify_phase_entry_offset is zero!\n");
 
 	fspnotify = (void*) (fsps_hdr.image_base +
 			    fsps_hdr.notify_phase_entry_offset);
@@ -49,5 +49,8 @@ enum fsp_status fsp_notify(enum fsp_notify_phase phase)
 	}
 	fsp_debug_notify(ret);
 
-	return ret;
+	/* Handle any errors returned by FspNotify */
+	fsp_handle_reset(ret);
+	if (ret != FSP_SUCCESS)
+		die("FspNotify returned an error!\n");
 }
diff --git a/src/drivers/intel/fsp2_0/silicon_init.c b/src/drivers/intel/fsp2_0/silicon_init.c
index b9fea66..c539db6 100644
--- a/src/drivers/intel/fsp2_0/silicon_init.c
+++ b/src/drivers/intel/fsp2_0/silicon_init.c
@@ -23,7 +23,7 @@
 
 struct fsp_header fsps_hdr;
 
-static enum fsp_status do_silicon_init(struct fsp_header *hdr)
+static void do_silicon_init(struct fsp_header *hdr)
 {
 	struct FSPS_UPD upd, *supd;
 	fsp_silicon_init_fn silicon_init;
@@ -32,8 +32,7 @@ static enum fsp_status do_silicon_init(struct fsp_header *hdr)
 	supd = (struct FSPS_UPD *) (hdr->cfg_region_offset + hdr->image_base);
 
 	if (supd->FspUpdHeader.Signature != FSPS_UPD_SIGNATURE) {
-		printk(BIOS_ERR, "Invalid FSPS signature\n");
-		return FSP_INCOMPATIBLE_VERSION;
+		die("Invalid FSPS signature\n");
 	}
 
 	memcpy(&upd, supd, sizeof(upd));
@@ -54,13 +53,13 @@ static enum fsp_status do_silicon_init(struct fsp_header *hdr)
 
 	fsp_debug_silicon_init(status);
 
-	/* Handle any resets requested by FSPS. */
+	/* Handle any errors returned by FspSiliconInit */
 	fsp_handle_reset(status);
-
-	return status;
+	if (status != FSP_SUCCESS)
+		die("FspSiliconINit returned an error!\n");
 }
 
-enum fsp_status fsp_silicon_init(void)
+void fsp_silicon_init(void)
 {
 	struct fsp_header *hdr = &fsps_hdr;
 	struct cbfsf file_desc;
@@ -71,7 +70,7 @@ enum fsp_status fsp_silicon_init(void)
 
 	if (cbfs_boot_locate(&file_desc, name, NULL)) {
 		printk(BIOS_ERR, "Could not locate %s in CBFS\n", name);
-		return FSP_NOT_FOUND;
+		die("FSPS not available!\n");
 	}
 
 	cbfs_file_data(&rdev, &file_desc);
@@ -81,26 +80,24 @@ enum fsp_status fsp_silicon_init(void)
 	dest = cbmem_add(CBMEM_ID_REFCODE, size);
 
 	if (dest == NULL) {
-		printk(BIOS_ERR, "Could not add FSPS to CBMEM.\n");
-		return FSP_NOT_FOUND;
+		die("Could not add FSPS to CBMEM!\n");
 	}
 
 	if (rdev_readat(&rdev, dest, 0, size) < 0)
-		return FSP_NOT_FOUND;
+		die("Failed to read FSPS!\n");
 
 	if (fsp_component_relocate((uintptr_t)dest, dest, size) < 0) {
-		printk(BIOS_ERR, "Unable to relocate FSPS.\n");
-		return FSP_NOT_FOUND;
+		die("Unable to relocate FSPS!\n");
 	}
 
 	/* Create new region device in memory after relocation. */
 	rdev_chain(&rdev, &addrspace_32bit.rdev, (uintptr_t)dest, size);
 
 	if (fsp_validate_component(hdr, &rdev) != CB_SUCCESS)
-		return FSP_NOT_FOUND;
+		die("Invalid FSPS header!\n");
 
 	/* Signal that FSP component has been loaded. */
 	prog_segment_loaded(hdr->image_base, hdr->image_size, SEG_FINAL);
 
-	return do_silicon_init(hdr);
+	do_silicon_init(hdr);
 }
diff --git a/src/soc/intel/apollolake/chip.c b/src/soc/intel/apollolake/chip.c
index 10b71fb..812f663 100644
--- a/src/soc/intel/apollolake/chip.c
+++ b/src/soc/intel/apollolake/chip.c
@@ -200,8 +200,7 @@ static void soc_init(void *data)
 	 * default policy that doesn't honor boards' requirements. */
 	itss_snapshot_irq_polarities(GPIO_IRQ_START, GPIO_IRQ_END);
 
-	if (fsp_silicon_init() != FSP_SUCCESS)
-		die("FSP silicon init failed. Giving up.");
+	fsp_silicon_init();
 
 	/* Restore GPIO IRQ polarities back to previous settings. */
 	itss_restore_irq_polarities(GPIO_IRQ_START, GPIO_IRQ_END);
@@ -411,12 +410,9 @@ static void fsp_notify_dummy(void *arg)
 {
 
 	enum fsp_notify_phase ph = (enum fsp_notify_phase) arg;
-	enum fsp_status ret;
 
-	if ((ret = fsp_notify(ph)) != FSP_SUCCESS) {
-		printk(BIOS_CRIT, "FspNotify failed, ret = %x!\n", ret);
-		fsp_handle_reset(ret);
-	}
+	fsp_notify(ph);
+
 	/* Call END_OF_FIRMWARE Notify after READY_TO_BOOT Notify */
 	if (ph == READY_TO_BOOT) {
 		fsp_notify_dummy((void *)END_OF_FIRMWARE);
diff --git a/src/soc/intel/apollolake/romstage.c b/src/soc/intel/apollolake/romstage.c
index da7fcf1..8f17fdd 100644
--- a/src/soc/intel/apollolake/romstage.c
+++ b/src/soc/intel/apollolake/romstage.c
@@ -114,11 +114,7 @@ asmlinkage void car_stage_entry(void)
 	console_init();
 
 	s3wake = fill_power_state(ps) == ACPI_S3;
-
-	if (fsp_memory_init(s3wake) != FSP_SUCCESS) {
-		die("FSP memory init failed. Giving up.");
-	}
-
+	fsp_memory_init(s3wake);
 	if (postcar_frame_init(&pcf, 1*KiB))
 		die("Unable to initialize postcar frame.\n");
 



More information about the coreboot-gerrit mailing list