diff mbox series

[RFC] pflash: Print libflash error strings

Message ID 20180227052023.16598-1-oohall@gmail.com
State New
Headers show
Series [RFC] pflash: Print libflash error strings | expand

Commit Message

Oliver O'Halloran Feb. 27, 2018, 5:20 a.m. UTC 
Seems like something we might want. Currently most errors just give a
little bit of context and spit an error code at you which requires
looking inside of libflash to work out the meaning of.

Inspired-by: https://github.com/open-power/skiboot/issues/151
Cc: cyrilbur@gmail.com
Cc: ppaidipe@linux.vnet.ibm.com

---
 external/pflash/pflash.c | 26 +++++++++++++++-----------
 libflash/errors.h        |  7 +++++--
 libflash/libflash.c      | 31 +++++++++++++++++++++++++++++++
 libflash/libflash.h      |  5 +++++
 4 files changed, 56 insertions(+), 13 deletions(-)

Comments

Stewart Smith Feb. 27, 2018, 6:24 a.m. UTC | #1 
Oliver O'Halloran <oohall@gmail.com> writes:
> Seems like something we might want. Currently most errors just give a
> little bit of context and spit an error code at you which requires
> looking inside of libflash to work out the meaning of.
>
> Inspired-by: https://github.com/open-power/skiboot/issues/151
> Cc: cyrilbur@gmail.com
> Cc: ppaidipe@linux.vnet.ibm.com

this does look like something we want.

Why RFC?
Cyril Bur Feb. 27, 2018, 6:50 a.m. UTC | #2 
On Tue, 2018-02-27 at 17:24 +1100, Stewart Smith wrote:
> Oliver O'Halloran <oohall@gmail.com> writes:
> > Seems like something we might want. Currently most errors just give a
> > little bit of context and spit an error code at you which requires
> > looking inside of libflash to work out the meaning of.
> > 
> > Inspired-by: https://github.com/open-power/skiboot/issues/151
> > Cc: cyrilbur@gmail.com
> > Cc: ppaidipe@linux.vnet.ibm.com
> 
> this does look like something we want.
> 

I haven't had much of a look, something like this would be great

> Why RFC?

Maybe because it breaks tests ;)

>
Oliver O'Halloran Feb. 27, 2018, 7:46 a.m. UTC | #3 
On Tue, Feb 27, 2018 at 5:50 PM, Cyril Bur <cyrilbur@gmail.com> wrote:
> On Tue, 2018-02-27 at 17:24 +1100, Stewart Smith wrote:
>> Oliver O'Halloran <oohall@gmail.com> writes:
>> > Seems like something we might want. Currently most errors just give a
>> > little bit of context and spit an error code at you which requires
>> > looking inside of libflash to work out the meaning of.
>> >
>> > Inspired-by: https://github.com/open-power/skiboot/issues/151
>> > Cc: cyrilbur@gmail.com
>> > Cc: ppaidipe@linux.vnet.ibm.com
>>
>> this does look like something we want.
>>
>
> I haven't had much of a look, something like this would be great
>
>> Why RFC?

Largely because it doesn't even try and address the libffs error codes
and I only did a half-assed pass over pflash to find places where we
were printing them. Odds are there are places in libflash that would
need fixing up too.

>
> Maybe because it breaks tests ;)

Hell I'm not even sure it compiles.

>
>>
diff mbox series

Patch

diff --git a/external/pflash/pflash.c b/external/pflash/pflash.c
index 3ac948496e24..419461b2e34d 100644
--- a/external/pflash/pflash.c
+++ b/external/pflash/pflash.c
@@ -149,7 +149,7 @@  static struct ffs_handle *open_ffs(struct flash_details *flash)
 	rc = ffs_init(flash->toc, flash->total_size,
 			flash->bl, &ffsh, 0);
 	if (rc) {
-		fprintf(stderr, "Error %d opening ffs !\n", rc);
+		fprintf(stderr, "Error %s (%d) opening ffs !\n", flash_error_str(rc), rc);
 		if (flash->toc) {
 			fprintf(stderr, "You specified 0x%" PRIx64 " as the libffs TOC\n"
 				   	"Looks like it doesn't exist\n", flash->toc);
@@ -293,7 +293,8 @@  static int erase_chip(struct flash_details *flash)
 	}
 	progress_end();
 	if (rc) {
-		fprintf(stderr, "Error %d erasing chip\n", rc);
+		fprintf(stderr, "Error %s (%d) erasing chip\n",
+			flash_error_str(rc), rc);
 		return rc;
 	}
 
@@ -334,7 +335,8 @@  static int erase_range(struct flash_details *flash,
 
 		rc = blocklevel_smart_erase(flash->bl, start, first_size);
 		if (rc) {
-			fprintf(stderr, "Failed to blocklevel_smart_erase(): %d\n", rc);
+			fprintf(stderr, "Failed to blocklevel_smart_erase(): %s (%d)\n",
+					flash_error_str(rc), rc);
 			return 1;
 		}
 		size -= first_size;
@@ -345,7 +347,8 @@  static int erase_range(struct flash_details *flash,
 	while (size & ~(erase_mask)) {
 		rc = blocklevel_smart_erase(flash->bl, start, flash->erase_granule);
 		if (rc) {
-			fprintf(stderr, "Failed to blocklevel_smart_erase(): %d\n", rc);
+			fprintf(stderr, "Failed to blocklevel_smart_erase(): %s (%d)\n",
+					flash_error_str(rc), rc);
 			return 1;
 		}
 		start += flash->erase_granule;
@@ -356,7 +359,8 @@  static int erase_range(struct flash_details *flash,
 	if (size) {
 		rc = blocklevel_smart_erase(flash->bl, start, size);
 		if (rc) {
-			fprintf(stderr, "Failed to blocklevel_smart_erase(): %d\n", rc);
+			fprintf(stderr, "Failed to blocklevel_smart_erase(): %s (%d)\n",
+					flash_error_str(rc), rc);
 			return 1;
 		}
 		done += size;
@@ -456,8 +460,8 @@  static int program_file(struct blocklevel_device *bl,
 				fprintf(stderr, "Verification failed for"
 					" chunk at 0x%08x\n", start);
 			else
-				fprintf(stderr, "Flash write error %d for"
-					" chunk at 0x%08x\n", rc, start);
+				fprintf(stderr, "Flash write error %s (%d) for"
+					" chunk at 0x%08x\n", flash_error_str(rc), rc, start);
 			goto out;
 		}
 		start += len;
@@ -531,7 +535,7 @@  static int enable_4B_addresses(struct blocklevel_device *bl)
 		if (rc == -1) {
 			fprintf(stderr, "Switching address mode not available on this architecture\n");
 		} else {
-			fprintf(stderr, "Error %d enabling 4b mode\n", rc);
+			fprintf(stderr, "Error %s (%d) enabling 4b mode\n", flash_error_str(rc), rc);
 		}
 	}
 
@@ -549,7 +553,7 @@  static int disable_4B_addresses(struct blocklevel_device *bl)
 		if (rc == -1) {
 			fprintf(stderr, "Switching address mode not available on this architecture\n");
 		} else {
-			fprintf(stderr, "Error %d enabling 4b mode\n", rc);
+			fprintf(stderr, "Error %s (%d) enabling 4b mode\n", flash_error_str(rc), rc);
 		}
 	}
 
@@ -566,8 +570,8 @@  static void print_partition_detail(struct ffs_handle *ffsh, uint32_t part_id)
 	rc = ffs_part_info(ffsh, part_id, &ent_name, &start, &size,
 			&act, NULL);
 	if (rc) {
-		fprintf(stderr, "Partition with ID %d doesn't exist error: %d\n",
-				part_id, rc);
+		fprintf(stderr, "Partition with ID %d doesn't exist error: %s (%d)\n",
+				part_id, flash_error_str(rc), rc);
 		goto out;
 	}
 
diff --git a/libflash/errors.h b/libflash/errors.h
index d144e097ea0e..1ed6c02e635a 100644
--- a/libflash/errors.h
+++ b/libflash/errors.h
@@ -16,6 +16,7 @@ 
 #ifndef __LIBFLASH_ERRORS_H
 #define __LIBFLASH_ERRORS_H
 
+#define FLASH_ERR_SUCCESS		0
 #define FLASH_ERR_MALLOC_FAILED		1
 #define FLASH_ERR_CHIP_UNKNOWN		2
 #define FLASH_ERR_PARM_ERROR		3
@@ -31,8 +32,10 @@ 
 #define FLASH_ERR_CTRL_TIMEOUT		13
 #define FLASH_ERR_ECC_INVALID		14
 #define FLASH_ERR_BAD_READ		15
-#define FLASH_ERR_DEVICE_GONE	16
-#define FLASH_ERR_AGAIN	17
+#define FLASH_ERR_DEVICE_GONE		16
+#define FLASH_ERR_AGAIN			17
+
+#define FLASH_ERR_LAST			18 /* increment when needed */
 
 #ifdef __SKIBOOT__
 #include <skiboot.h>
diff --git a/libflash/libflash.c b/libflash/libflash.c
index ad25b613357a..6bcd056d20a9 100644
--- a/libflash/libflash.c
+++ b/libflash/libflash.c
@@ -23,6 +23,37 @@ 
 #include "ecc.h"
 #include "blocklevel.h"
 
+static const char *error_str[FLASH_ERR_LAST] = {
+	[FLASH_ERR_SUCCESS] = "success",
+	[FLASH_ERR_MALLOC_FAILED] = "malloc failed",
+	[FLASH_ERR_CHIP_UNKNOWN] = "chip unknown",
+	[FLASH_ERR_PARM_ERROR] = "parameter error",
+	[FLASH_ERR_ERASE_BOUNDARY] = "erase boundary error",
+	[FLASH_ERR_WREN_TIMEOUT] = "wren timeout",
+	[FLASH_ERR_WIP_TIMEOUT] = "wip timeout",
+	[FLASH_ERR_BAD_PAGE_SIZE] = "bad page size",
+	[FLASH_ERR_VERIFY_FAILURE] = "verify failure",
+	[FLASH_ERR_4B_NOT_SUPPORTED] = "4b mode unsupported",
+	[FLASH_ERR_CTRL_CONFIG_MISMATCH] = "controller is misconfigured",
+	[FLASH_ERR_CHIP_ER_NOT_SUPPORTED] = "chip doesn't support erase",
+	[FLASH_ERR_CTRL_CMD_UNSUPPORTED] = "unsupported controller command",
+	[FLASH_ERR_CTRL_TIMEOUT] = "controller timeout",
+	[FLASH_ERR_ECC_INVALID] = "ecc invalid",
+	[FLASH_ERR_BAD_READ] = "bad read",
+	[FLASH_ERR_DEVICE_GONE] = "device gone",
+	[FLASH_ERR_AGAIN] = "try again"
+};
+
+const char *flash_error_str(int rc)
+{
+	if (rc >= FLASH_ERR_LAST)
+		return "unknown error";
+	if (rc < 0)
+		return "backend error";
+
+	return error_str[rc];
+}
+
 static const struct flash_info flash_info[] = {
 	{ 0xc22018, 0x01000000, FL_ERASE_ALL | FL_CAN_4B, "Macronix MXxxL12835F"},
 	{ 0xc22019, 0x02000000, FL_ERASE_ALL | FL_CAN_4B, "Macronix MXxxL25635F"},
diff --git a/libflash/libflash.h b/libflash/libflash.h
index 01b4d6052e7d..2d3b74a1a992 100644
--- a/libflash/libflash.h
+++ b/libflash/libflash.h
@@ -28,6 +28,11 @@ 
  */
 #include <libflash/errors.h>
 
+/*
+ * Translates a libflash return code into something sensible.
+ */
+const char *flash_error_str(int rc) __attribute__((const));
+
 #ifndef MIN
 #define MIN(a, b)	((a) < (b) ? (a) : (b))
 #endif