[Skiboot] [RFC] external/pflash: Deal with erase granularities larger than the requested erase range

Cyril Bur cyril.bur at au1.ibm.com
Fri Oct 21 20:42:56 AEDT 2016


There is an implicit assumption in the erase code of flash that the
erase granularity is smaller than that of the erase range, this may not
always be true.

With the increased use of /dev/mtd as a method for accessing the flash
and the kernel drivers being able to expose any erase granularity
supported by the chip, not necessarily always 4K erasing and writing
smaller ffs partitions may be impossible through MTD.

Signed-off-by: Cyril Bur <cyril.bur at au1.ibm.com>
---
I haven't tested this at all, I've only written it in response to
testing my other series and I noticed we're going to need it so lets
get the code out there.

For got to mention in the commit message that theres some cosmetic
fixes too.


 external/pflash/pflash.c | 122 +++++++++++++++++++++++++++++++++++++----------
 libflash/blocklevel.c    |   2 +-
 2 files changed, 99 insertions(+), 25 deletions(-)

diff --git a/external/pflash/pflash.c b/external/pflash/pflash.c
index 52d9497..6c44795 100644
--- a/external/pflash/pflash.c
+++ b/external/pflash/pflash.c
@@ -136,7 +136,7 @@ static void print_flash_info(uint32_t toc)
 	printf("Flash info:\n");
 	printf("-----------\n");
 	printf("Name          = %s\n", fl_name);
-	printf("Total size    = %"PRIu64"dMB \n", fl_total_size >> 20);
+	printf("Total size    = %"PRIu64"MB \n", fl_total_size >> 20);
 	printf("Erase granule = %dKB \n", fl_erase_granule >> 10);
 
 	if (bmc_flash)
@@ -232,6 +232,63 @@ static void erase_chip(void)
 	printf("done !\n");
 }
 
+/*
+ * Ensure this is only called with the range being entirely within a
+ * single erase block
+ */
+static void erase_partial_block(uint32_t start, uint32_t size)
+{
+	int rc;
+	void *buffer;
+	uint32_t erase_start = start & ~(fl_erase_granule - 1);
+
+	buffer = malloc(fl_erase_granule);
+	if (!buffer) {
+		fprintf(stderr, "Couldn't allocate erase buffer\n");
+		exit(1);
+	}
+	rc = blocklevel_read(bl, erase_start, buffer, fl_erase_granule);
+	if (rc) {
+		fprintf(stderr, "Error %d reading 0x%08x for 0x%08x\n",
+				rc, erase_start, fl_erase_granule);
+		exit(1);
+	}
+	rc = blocklevel_erase(bl, erase_start, fl_erase_granule);
+	if (rc) {
+		fprintf(stderr, "Error %d erasing 0x%08x\n", rc, erase_start);
+		exit(1);
+	}
+
+	/* Start could have been fl_erase_granule aligned */
+	if (erase_start != start) {
+		rc = blocklevel_write(bl, erase_start, buffer, start - erase_start);
+		if (rc) {
+			/*
+			 * Oh wow this can't be good... we've done bad things
+			 * to the flash.
+			 */
+			fprintf(stderr, "Error %d writing back buffered region.\n"
+					"Flash probably corrupt at 0x%08x..0x%08x.\n",
+					erase_start, start, rc);
+			exit(1);
+		}
+	}
+
+	/* The range could have been right to the end of an erase granule */
+	if (erase_start + fl_erase_granule != start + size) {
+		rc = blocklevel_write(bl, start + size, buffer + size,
+				(erase_start + fl_erase_granule) - (start + size));
+		if (rc) {
+			fprintf(stderr, "Error %d writing back buffered region.\n"
+					"Flash probably corrupt at 0x%08x..0x%08x.\n",
+					start + size, erase_start + fl_erase_granule, rc);
+			exit(1);
+		}
+	}
+
+	free(buffer);
+}
+
 static void erase_range(uint32_t start, uint32_t size, bool will_program)
 {
 	uint32_t done = 0;
@@ -246,33 +303,50 @@ static void erase_range(uint32_t start, uint32_t size, bool will_program)
 	}
 
 	printf("Erasing...\n");
-	progress_init(size >> 8);
-	while(size) {
-		/* If aligned to 64k and at least 64k, use 64k erase */
-		if ((start & 0xffff) == 0 && size >= 0x10000) {
-			rc = blocklevel_erase(bl, start, 0x10000);
-			if (rc) {
-				fprintf(stderr, "Error %d erasing 0x%08x\n",
-					rc, start);
-				exit(1);
-			}
-			start += 0x10000;
-			size -= 0x10000;
-			done += 0x10000;
+
+	if (fl_erase_granule > size) {
+		uint32_t erase_start = start & ~(fl_erase_granule - 1);
+		uint32_t erase_end = (start + size) & ~(fl_erase_granule - 1);
+
+		if (erase_start == erase_end) {
+			erase_partial_block(start, size);
 		} else {
-			rc = blocklevel_erase(bl, start, 0x1000);
-			if (rc) {
-				fprintf(stderr, "Error %d erasing 0x%08x\n",
-					rc, start);
-				exit(1);
+			/*
+			 * wow seriously, this smaller erase managed to span two
+			 * large erase blocks...
+			 */
+			erase_partial_block(start, erase_end - start);
+			erase_partial_block(erase_end, size - (erase_end - start));
+		}
+	} else {
+		progress_init(size >> 8);
+		while(size) {
+			/* If aligned to 64k and at least 64k, use 64k erase */
+			if ((start & 0xffff) == 0 && size >= 0x10000) {
+				rc = blocklevel_erase(bl, start, 0x10000);
+				if (rc) {
+						fprintf(stderr, "Error %d erasing 0x%08x\n",
+						rc, start);
+					exit(1);
+				}
+				start += 0x10000;
+				size -= 0x10000;
+				done += 0x10000;
+			} else {
+				rc = blocklevel_erase(bl, start, 0x1000);
+				if (rc) {
+					fprintf(stderr, "Error %d erasing 0x%08x\n",
+						rc, start);
+					exit(1);
+				}
+				start += 0x1000;
+				size -= 0x1000;
+				done += 0x1000;
 			}
-			start += 0x1000;
-			size -= 0x1000;
-			done += 0x1000;
+			progress_tick(done >> 8);
 		}
-		progress_tick(done >> 8);
+		progress_end();
 	}
-	progress_end();
 
 	/* If this is a flash partition, mark it empty if we aren't
 	 * going to program over it as well
diff --git a/libflash/blocklevel.c b/libflash/blocklevel.c
index b827045..5bd8ce4 100644
--- a/libflash/blocklevel.c
+++ b/libflash/blocklevel.c
@@ -173,7 +173,7 @@ int blocklevel_erase(struct blocklevel_device *bl, uint64_t pos, uint64_t len)
 
 	/* Programmer may be making a horrible mistake without knowing it */
 	if (len & bl->erase_mask) {
-		fprintf(stderr, "blocklevel_erase: len (0x%"PRIu64") is not erase block (0x%08x) aligned\n",
+		fprintf(stderr, "blocklevel_erase: len (0x%"PRIx64") is not erase block (0x%08x) aligned\n",
 				len, bl->erase_mask + 1);
 		return FLASH_ERR_ERASE_BOUNDARY;
 	}
-- 
2.10.0



More information about the Skiboot mailing list