This is split off the bus pirate buffer management revamp patch where it didn't really belong. Thanks to Stefan Tauner for the suggestion.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-errorcodes/flash.h =================================================================== --- flashrom-errorcodes/flash.h (Revision 1463) +++ flashrom-errorcodes/flash.h (Arbeitskopie) @@ -33,10 +33,18 @@ #undef max #endif
+/* Error code list, specific errors first, generic errors second. */ +/* 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 +232,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); }
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...
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...
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!
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
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 */
*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.
-
Carl-Daniel Hailfinger -
Stefan Tauner