Patchworkβ Clean up error code definitions

login
register
about
Submitter Carl-Daniel Hailfinger
Date 2011-11-14 12:22:33
Message ID <4EC10809.5010109@gmx.net>
Download mbox | patch
Permalink /patch/3458/
State New
Headers show

Comments

Carl-Daniel Hailfinger - 2011-11-14 12:22:33
Am 14.11.2011 09:02 schrieb Stefan Tauner:
> On Mon, 14 Nov 2011 08:54:47 +0100
> Stefan Tauner <stefan.tauner@student.tuwien.ac.at> wrote:
>
>> On Mon, 14 Nov 2011 01:24:38 +0100
>> Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote:
>>
>>> This is split off the bus pirate buffer management revamp patch where it
>>> didn't really belong.
>> can we please also integrate the ones from ich_descriptors.h (and
>> maybe others*)? including the value indicating success would be nice too
>> imho. i am willing to help with this if you tell me how...

#define RET_OK    0

 
> SPI_GENERIC_ERROR etc. from spi.h shows up on my radar... and of course
> most of the exit(1) calls should be reviewed and changed after this is
> merged. serprog.c i am looking at you!

Heh. serprog.c is already a bit better since my last programmer
registration patch, but it still needs work, as do other places in the code.

Here are the error codes I found, partially renamed. This is not for
merge, we first have to get the names right.

Regards,
Carl-Daniel

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
Stefan Tauner - 2012-01-12 01:15:04
*push*

i dont care much for the exact names, but i wish for a few things to be
considered:
 - all of them should have a common prefix. this is mostly a problem
   for the value of "OK". often projects use "ERR_" as prefix which
   leads to ERR_OK macros or similar, which isnt too bad. i would
   prefer that over having different prefixes. if we want a very short
   prefix ERR_ is probably best because most return values will be
   some error. but there is also OK and some warnings, so i would favor
   something more neutral like RET_ or so.
 - do we need a "flashrom" prefix too? iirc these are internal
   values so we can ignore external users, right? we may want to define
   some external values too then? could be postponed...
 - all of them should be somewhat readable in context and have
   approximately the same length.

Patch

Index: flashrom-errorcodes/flash.h
===================================================================
--- flashrom-errorcodes/flash.h	(Revision 1463)
+++ flashrom-errorcodes/flash.h	(Arbeitskopie)
@@ -33,10 +33,29 @@ 
 #undef max
 #endif
 
+/* Error code list, specific errors first, generic errors second. */
+#define ERROR_GENERIC	-1
+#define SPI_INVALID_OPCODE	-2
+#define SPI_INVALID_ADDRESS	-3
+#define SPI_INVALID_LENGTH	-4
+#define ERROR_FLASHROM_BUG	-5
+#define ERROR_PROGRAMMER	-6
+#define ICH_RET_OK	0
+#define ICH_RET_ERR	-1
+#define ICH_RET_WARN	-2
+#define ICH_RET_PARAM	-3
+#define ICH_RET_OOB	-4
+/* Out of memory */
+#define ERROR_OOM	-100
+/* Timeout */
+#define ERROR_TIMEOUT	-101
+/* Something happened that shouldn't happen, but we can go on. */
+#define ERROR_NONFATAL	-110
+/* Something happened that shouldn't happen, we'll abort. */
+#define ERROR_FATAL	-111
+/* For functions which can only return pointers */
 #define ERROR_PTR ((void*)-1)
 
-/* Error codes */
-#define TIMEOUT_ERROR	-101
 
 typedef unsigned long chipaddr;
 
@@ -224,12 +243,6 @@ 
 #define OK 0
 #define NT 1    /* Not tested */
 
-/* Something happened that shouldn't happen, but we can go on. */
-#define ERROR_NONFATAL 0x100
-
-/* Something happened that shouldn't happen, we'll abort. */
-#define ERROR_FATAL -0xee
-
 /* cli_output.c */
 /* Let gcc and clang check for correct printf-style format strings. */
 int print(int type, const char *fmt, ...) __attribute__((format(printf, 2, 3)));
Index: flashrom-errorcodes/spi25.c
===================================================================
--- flashrom-errorcodes/spi25.c	(Revision 1463)
+++ flashrom-errorcodes/spi25.c	(Arbeitskopie)
@@ -769,7 +769,7 @@ 
 	while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP) {
 		if (++i > 490) {
 			msg_cerr("Error: WIP bit after WRSR never cleared\n");
-			return TIMEOUT_ERROR;
+			return ERROR_TIMEOUT;
 		}
 		programmer_delay(10 * 1000);
 	}
@@ -817,7 +817,7 @@ 
 	while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP) {
 		if (++i > 490) {
 			msg_cerr("Error: WIP bit after WRSR never cleared\n");
-			return TIMEOUT_ERROR;
+			return ERROR_TIMEOUT;
 		}
 		programmer_delay(10 * 1000);
 	}
@@ -1128,7 +1128,7 @@ 
 		msg_cerr("%s: start address not even! Please report a bug at "
 			 "flashrom@flashrom.org\n", __func__);
 		if (spi_chip_write_1(flash, buf, start, start % 2))
-			return SPI_GENERIC_ERROR;
+			return ERROR_GENERIC;
 		pos += start % 2;
 		cmds[1].writearr = (const unsigned char[]){
 					JEDEC_AAI_WORD_PROGRAM,
@@ -1139,14 +1139,14 @@ 
 					buf[pos - start + 1]
 				};
 		/* Do not return an error for now. */
-		//return SPI_GENERIC_ERROR;
+		//return ERROR_GENERIC;
 	}
 	/* The data sheet requires total AAI write length to be even. */
 	if (len % 2) {
 		msg_cerr("%s: total write length not even! Please report a "
 			 "bug at flashrom@flashrom.org\n", __func__);
 		/* Do not return an error for now. */
-		//return SPI_GENERIC_ERROR;
+		//return ERROR_GENERIC;
 	}
 
 
@@ -1182,7 +1182,7 @@ 
 	/* Write remaining byte (if any). */
 	if (pos < start + len) {
 		if (spi_chip_write_1(flash, buf + pos - start, pos, pos % 2))
-			return SPI_GENERIC_ERROR;
+			return ERROR_GENERIC;
 		pos += pos % 2;
 	}
 
Index: flashrom-errorcodes/buspirate_spi.c
===================================================================
--- flashrom-errorcodes/buspirate_spi.c	(Revision 1463)
+++ flashrom-errorcodes/buspirate_spi.c	(Arbeitskopie)
@@ -323,22 +323,22 @@ 
 
 	if (ret) {
 		msg_perr("Bus Pirate communication error!\n");
-		return SPI_GENERIC_ERROR;
+		return ERROR_GENERIC;
 	}
 
 	if (buf[0] != 0x01) {
 		msg_perr("Protocol error while lowering CS#!\n");
-		return SPI_GENERIC_ERROR;
+		return ERROR_GENERIC;
 	}
 
 	if (buf[1] != 0x01) {
 		msg_perr("Protocol error while reading/writing SPI!\n");
-		return SPI_GENERIC_ERROR;
+		return ERROR_GENERIC;
 	}
 
 	if (buf[i - 1] != 0x01) {
 		msg_perr("Protocol error while raising CS#!\n");
-		return SPI_GENERIC_ERROR;
+		return ERROR_GENERIC;
 	}
 
 	/* Skip CS#, length, writearr. */
Index: flashrom-errorcodes/ft2232_spi.c
===================================================================
--- flashrom-errorcodes/ft2232_spi.c	(Revision 1463)
+++ flashrom-errorcodes/ft2232_spi.c	(Arbeitskopie)
@@ -363,7 +363,7 @@ 
 		if (!buf) {
 			msg_perr("Out of memory!\n");
 			/* TODO: What to do with buf? */
-			return SPI_GENERIC_ERROR;
+			return ERROR_GENERIC;
 		}
 		oldbufsize = bufsize;
 	}
Index: flashrom-errorcodes/spi.h
===================================================================
--- flashrom-errorcodes/spi.h	(Revision 1463)
+++ flashrom-errorcodes/spi.h	(Arbeitskopie)
@@ -118,12 +118,4 @@ 
 #define JEDEC_AAI_WORD_PROGRAM_CONT_OUTSIZE	0x03
 #define JEDEC_AAI_WORD_PROGRAM_INSIZE		0x00
 
-/* Error codes */
-#define SPI_GENERIC_ERROR	-1
-#define SPI_INVALID_OPCODE	-2
-#define SPI_INVALID_ADDRESS	-3
-#define SPI_INVALID_LENGTH	-4
-#define SPI_FLASHROM_BUG	-5
-#define SPI_PROGRAMMER_ERROR	-6
-
 #endif		/* !__SPI_H__ */
Index: flashrom-errorcodes/sb600spi.c
===================================================================
--- flashrom-errorcodes/sb600spi.c	(Revision 1463)
+++ flashrom-errorcodes/sb600spi.c	(Arbeitskopie)
@@ -141,7 +141,7 @@ 
 	 * the FIFO pointer to the first byte we want to send.
 	 */
 	if (reset_compare_internal_fifo_pointer(writecnt))
-		return SPI_PROGRAMMER_ERROR;
+		return ERROR_PROGRAMMER;
 
 	msg_pspew("Executing: \n");
 	execute_command();
@@ -158,7 +158,7 @@ 
 	 * Usually, the chip will respond with 0x00 or 0xff.
 	 */
 	if (reset_compare_internal_fifo_pointer(writecnt + readcnt))
-		return SPI_PROGRAMMER_ERROR;
+		return ERROR_PROGRAMMER;
 
 	/* Skip the bytes we sent. */
 	msg_pspew("Skipping: ");
@@ -168,7 +168,7 @@ 
 	}
 	msg_pspew("\n");
 	if (compare_internal_fifo_pointer(writecnt))
-		return SPI_PROGRAMMER_ERROR;
+		return ERROR_PROGRAMMER;
 
 	msg_pspew("Reading: ");
 	for (count = 0; count < readcnt; count++, readarr++) {
@@ -177,7 +177,7 @@ 
 	}
 	msg_pspew("\n");
 	if (reset_compare_internal_fifo_pointer(readcnt + writecnt))
-		return SPI_PROGRAMMER_ERROR;
+		return ERROR_PROGRAMMER;
 
 	if (mmio_readb(sb600_spibar + 1) != readwrite) {
 		msg_perr("Unexpected change in SB600 read/write count!\n");
@@ -185,7 +185,7 @@ 
 			 "causes random corruption.\nPlease stop all "
 			 "applications and drivers and IPMI which access the "
 			 "flash chip.\n");
-		return SPI_PROGRAMMER_ERROR;
+		return ERROR_PROGRAMMER;
 	}
 
 	return 0;
Index: flashrom-errorcodes/ich_descriptors.h
===================================================================
--- flashrom-errorcodes/ich_descriptors.h	(Revision 1463)
+++ flashrom-errorcodes/ich_descriptors.h	(Arbeitskopie)
@@ -26,13 +26,6 @@ 
 #include <stdint.h>
 #include "programmer.h" /* for enum ich_chipset */
 
-/* FIXME: Replace with generic return codes */
-#define ICH_RET_OK	0
-#define ICH_RET_ERR	-1
-#define ICH_RET_WARN	-2
-#define ICH_RET_PARAM	-3
-#define ICH_RET_OOB	-4
-
 #define ICH9_REG_FDOC		0xB0	/* 32 Bits Flash Descriptor Observability Control */
 					/* 0-1: reserved */
 #define FDOC_FDSI_OFF		2	/* 2-11: Flash Descriptor Section Index */