[Skiboot] [PATCH RFC] flash: Rework error paths and messages for multiple flash controllers

Michael Neuling mikey at neuling.org
Tue Jul 5 21:36:36 AEST 2016


The current flash code was written with only one flash chip, which is
a system_flash (ie. the PNOR image), in mind.

Now that we have mambo bogusdisk flash, we can have many flash chips.
This is resulting in some confusing output messages.

This reworks some of the error paths and warnings to make this more
coherent when we have multiple flash chips.

We assume everything can be a system flash, so I've removed the
is_system_flash parameter from flash_register().  We'll use the first
system flash we find and warn if we find another since discovery order
is not a guaranteed API.

Signed-off-by: Michael Neuling <mikey at neuling.org>
---
 core/flash.c              | 45 +++++++++++++++++++++++----------------------
 include/skiboot.h         |  2 +-
 libflash/libffs.c         |  2 +-
 platforms/astbmc/pnor.c   |  2 +-
 platforms/mambo/mambo.c   |  2 +-
 platforms/rhesus/rhesus.c |  2 +-
 6 files changed, 28 insertions(+), 27 deletions(-)

diff --git a/core/flash.c b/core/flash.c
index 430114e..f783bc6 100644
--- a/core/flash.c
+++ b/core/flash.c
@@ -208,31 +208,25 @@ static void setup_system_flash(struct flash *flash, struct dt_node *node,
 {
 	char *path;
 
-	if (system_flash) {
-		/**
-		 * @fwts-label SystemFlashDuplicate
-		 * @fwts-advice More than one flash device was registered
-		 *  as the system flash device. Check for duplicate calls
-		 *  to flash_register(..., true).
-		 */
-		prlog(PR_WARNING, "FLASH: attempted to register a second "
-				"system flash device %s\n", name);
+	if (!ffs)
 		return;
-	}
 
-	if (!ffs) {
+	if (system_flash) {
 		/**
-		 * @fwts-label SystemFlashNoPartitionTable
-		 * @fwts-advice OPAL Could not read a partition table on
-		 *    system flash. Since we've still booted the machine (which
-		 *    requires flash), check that we're registering the proper
-		 *    system flash device.
+		 * @fwts-label SystemFlashMultiple
+		 * @fwts-advice OPAL Found multiple system flash.
+		 *    Since we've already found a system flash we are
+		 *    going to use that one but this ordering is not
+		 *    guaranteed so may change in future.
 		 */
-		prlog(PR_WARNING, "FLASH: attempted to register system flash "
-				"%s, which has no partition info\n", name);
+		prlog(PR_WARNING, "FLASH: Attempted to register multiple system "
+		      "flash: %s\n", name);
 		return;
 	}
 
+	prlog(PR_NOTICE, "FLASH: Found system flash: %s id:%i\n",
+	      name, flash->id);
+
 	system_flash = flash;
 	path = dt_get_path(node);
 	dt_add_property_string(dt_chosen, "ibm,system-flash", path);
@@ -254,7 +248,7 @@ static int num_flashes(void)
 	return i;
 }
 
-int flash_register(struct blocklevel_device *bl, bool is_system_flash)
+int flash_register(struct blocklevel_device *bl)
 {
 	uint32_t size, block_size;
 	struct ffs_handle *ffs;
@@ -303,8 +297,7 @@ int flash_register(struct blocklevel_device *bl, bool is_system_flash)
 
 	node = flash_add_dt_node(flash, flash->id);
 
-	if (is_system_flash)
-		setup_system_flash(flash, node, name, ffs);
+	setup_system_flash(flash, node, name, ffs);
 
 	if (ffs)
 		ffs_close(ffs);
@@ -546,8 +539,16 @@ static int flash_load_resource(enum resource_id id, uint32_t subid,
 
 	lock(&flash_lock);
 
-	if (!system_flash)
+	if (!system_flash) {
+		/**
+		 * @fwts-label SystemFlashNotFound
+		 * @fwts-advice No system flash was found. Check for missing
+		 * calls flash_register(...).
+		 */
+		prlog(PR_WARNING, "FLASH: Can't load resource id:%i. "
+		      "No system flash found\n", id);
 		goto out_unlock;
+	}
 
 	flash = system_flash;
 
diff --git a/include/skiboot.h b/include/skiboot.h
index ded6bb8..6f9438b 100644
--- a/include/skiboot.h
+++ b/include/skiboot.h
@@ -213,7 +213,7 @@ extern void lpc_rtc_init(void);
 
 /* flash support */
 struct flash_chip;
-extern int flash_register(struct blocklevel_device *bl, bool is_system_flash);
+extern int flash_register(struct blocklevel_device *bl);
 extern int flash_start_preload_resource(enum resource_id id, uint32_t subid,
 					void *buf, size_t *len);
 extern int flash_resource_loaded(enum resource_id id, uint32_t idx);
diff --git a/libflash/libffs.c b/libflash/libffs.c
index 8134962..1765560 100644
--- a/libflash/libffs.c
+++ b/libflash/libffs.c
@@ -133,7 +133,7 @@ int ffs_init(uint32_t offset, uint32_t max_size, struct blocklevel_device *bl,
 	/* Convert and check flash header */
 	rc = ffs_check_convert_header(&f->hdr, &hdr);
 	if (rc) {
-		FL_ERR("FFS: Error %d checking flash header\n", rc);
+		FL_INF("FFS: Flash header not found. Code: %d\n", rc);
 		goto out;
 	}
 
diff --git a/platforms/astbmc/pnor.c b/platforms/astbmc/pnor.c
index 74353ea..de4cafa 100644
--- a/platforms/astbmc/pnor.c
+++ b/platforms/astbmc/pnor.c
@@ -52,7 +52,7 @@ int pnor_init(void)
 		goto fail;
 	}
 
-	rc = flash_register(bl, true);
+	rc = flash_register(bl);
 	if (!rc)
 		return 0;
 
diff --git a/platforms/mambo/mambo.c b/platforms/mambo/mambo.c
index 154c734..49071aa 100644
--- a/platforms/mambo/mambo.c
+++ b/platforms/mambo/mambo.c
@@ -207,7 +207,7 @@ static void bogus_disk_flash_init(void)
 		bl->priv = bdi;
 		bl->erase_mask = BD_SECT_SZ - 1;
 
-		rc = flash_register(bl, true);
+		rc = flash_register(bl);
 		if (rc)
 			prerror("mambo: Failed to register bogus disk: %li\n",
 				id);
diff --git a/platforms/rhesus/rhesus.c b/platforms/rhesus/rhesus.c
index 3e2c41b..08df218 100644
--- a/platforms/rhesus/rhesus.c
+++ b/platforms/rhesus/rhesus.c
@@ -135,7 +135,7 @@ static int rhesus_pnor_init(void)
 		goto fail;
 	}
 
-	rc = flash_register(bl, true);
+	rc = flash_register(bl);
 	if (!rc)
 		return 0;
 
-- 
2.7.4



More information about the Skiboot mailing list