New patch to review for coreboot: 83b0037 pc80/mc146818rtc: Return an error code rather than an integer
Alexandru Gagniuc (mr.nuke.me@gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/4265
-gerrit
commit 83b0037e8413c97deab96bd63bb5562d71c10238 Author: Alexandru Gagniuc mr.nuke.me@gmail.com Date: Sat Nov 23 18:54:44 2013 -0600
pc80/mc146818rtc: Return an error code rather than an integer
Do not return hardcoded numerical values to communicate succes/failure, but instead use an enumeration.
Change-Id: I742b08796adf136dce5984b702533f91640846dd Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com --- src/drivers/pc80/mc146818rtc.c | 48 ++++++++++++++++++++++-------------------- src/include/pc80/mc146818rtc.h | 15 ++++++++----- src/include/types.h | 7 ++++++ 3 files changed, 42 insertions(+), 28 deletions(-)
diff --git a/src/drivers/pc80/mc146818rtc.c b/src/drivers/pc80/mc146818rtc.c index be52454..c469091 100644 --- a/src/drivers/pc80/mc146818rtc.c +++ b/src/drivers/pc80/mc146818rtc.c @@ -150,9 +150,10 @@ void rtc_init(int invalid) length = number of bits to include in the value ret = a character pointer to where the value is to be returned output the value placed in ret - returns 0 = successful, -1 = an error occurred + returns CB_SUCCESS = successful, cb_err code if an error occurred */ -static int get_cmos_value(unsigned long bit, unsigned long length, void *vret) +static enum cb_err get_cmos_value(unsigned long bit, unsigned long length, + void *vret) { unsigned char *ret; unsigned long byte,byte_bit; @@ -176,10 +177,10 @@ static int get_cmos_value(unsigned long bit, unsigned long length, void *vret) ret[i]=cmos_read(byte); } } - return 0; + return CB_SUCCESS; }
-int get_option(void *dest, const char *name) +enum cb_err get_option(void *dest, const char *name) { struct cmos_option_table *ct; struct cmos_entries *ce; @@ -195,7 +196,7 @@ int get_option(void *dest, const char *name) if (!ct) { printk(BIOS_ERR, "RTC: cmos_layout.bin could not be found. " "Options are disabled\n"); - return(-2); + return CB_CMOS_LAYOUT_NOT_FOUND; } ce=(struct cmos_entries*)((unsigned char *)ct + ct->header_length); for(;ce->tag==LB_TAG_OPTION; @@ -207,18 +208,18 @@ int get_option(void *dest, const char *name) } if(!found) { printk(BIOS_DEBUG, "WARNING: No CMOS option '%s'.\n", name); - return(-2); + return CB_CMOS_OPTION_NOT_FOUND; }
- if(get_cmos_value(ce->bit, ce->length, dest)) - return(-3); - if(!rtc_checksum_valid(LB_CKS_RANGE_START, - LB_CKS_RANGE_END,LB_CKS_LOC)) - return(-4); - return(0); + if(get_cmos_value(ce->bit, ce->length, dest) != CB_SUCCESS) + return CB_CMOS_ACCESS_ERROR; + if(!rtc_checksum_valid(LB_CKS_RANGE_START, LB_CKS_RANGE_END,LB_CKS_LOC)) + return CB_CMOS_CHECKSUM_INVALID; + return CB_SUCCESS; }
-static int set_cmos_value(unsigned long bit, unsigned long length, void *vret) +static enum cb_err set_cmos_value(unsigned long bit, unsigned long length, + void *vret) { unsigned char *ret; unsigned long byte,byte_bit; @@ -241,7 +242,7 @@ static int set_cmos_value(unsigned long bit, unsigned long length, void *vret) chksum_update_needed = 1; } else { /* more that one byte so transfer the whole bytes */ if (byte_bit || length % 8) - return -1; + return CB_ERR_ARG;
for(i=0; length; i++, length-=8, byte++) cmos_write(ret[i], byte); @@ -253,11 +254,11 @@ static int set_cmos_value(unsigned long bit, unsigned long length, void *vret) rtc_set_checksum(LB_CKS_RANGE_START, LB_CKS_RANGE_END,LB_CKS_LOC); } - return 0; + return CB_SUCCESS; }
-int set_option(const char *name, void *value) +enum cb_err set_option(const char *name, void *value) { struct cmos_option_table *ct; struct cmos_entries *ce; @@ -273,7 +274,7 @@ int set_option(const char *name, void *value) CBFS_COMPONENT_CMOS_LAYOUT); if (!ct) { printk(BIOS_ERR, "cmos_layout.bin could not be found. Options are disabled\n"); - return(-2); + return CB_CMOS_LAYOUT_NOT_FOUND; } ce=(struct cmos_entries*)((unsigned char *)ct + ct->header_length); for(;ce->tag==LB_TAG_OPTION; @@ -285,21 +286,22 @@ int set_option(const char *name, void *value) } if(!found) { printk(BIOS_DEBUG, "WARNING: No CMOS option '%s'.\n", name); - return(-2); + return CB_CMOS_OPTION_NOT_FOUND; }
length = ce->length; if (ce->config == 's') { length = MAX(strlen((const char *)value) * 8, ce->length - 8); /* make sure the string is null terminated */ - if ((set_cmos_value(ce->bit + ce->length - 8, 8, &(u8[]){0}))) - return (-3); + if (set_cmos_value(ce->bit + ce->length - 8, 8, &(u8[]){0}) + != CB_SUCCESS) + return (CB_CMOS_ACCESS_ERROR); }
- if ((set_cmos_value(ce->bit, length, value))) - return (-3); + if (set_cmos_value(ce->bit, length, value) != CB_SUCCESS) + return (CB_CMOS_ACCESS_ERROR);
- return 0; + return CB_SUCCESS; } #endif /* CONFIG_USE_OPTION_TABLE */
diff --git a/src/include/pc80/mc146818rtc.h b/src/include/pc80/mc146818rtc.h index 170a433..ef48a36 100644 --- a/src/include/pc80/mc146818rtc.h +++ b/src/include/pc80/mc146818rtc.h @@ -1,6 +1,8 @@ #ifndef PC80_MC146818RTC_H #define PC80_MC146818RTC_H
+#include <types.h> + #ifndef RTC_BASE_PORT #define RTC_BASE_PORT 0x70 #endif @@ -171,13 +173,16 @@ static inline void cmos_write32(u8 offset, u32 value) void rtc_init(int invalid); void rtc_check_update_cmos_date(u8 has_century); #if CONFIG_USE_OPTION_TABLE -int set_option(const char *name, void *val); -int get_option(void *dest, const char *name); +enum cb_err set_option(const char *name, void *val); +enum cb_err get_option(void *dest, const char *name); unsigned read_option_lowlevel(unsigned start, unsigned size, unsigned def); #else -static inline int set_option(const char *name __attribute__((unused)), void *val __attribute__((unused))) { return -2; }; -static inline int get_option(void *dest __attribute__((unused)), - const char *name __attribute__((unused))) { return -2; } +static inline enum cb_err set_option(const char *name __attribute__((unused)), + void *val __attribute__((unused))) + { return CB_CMOS_OTABLE_DISABLED; }; +static inline enum cb_err get_option(void *dest __attribute__((unused)), + const char *name __attribute__((unused))) + { return CB_CMOS_OTABLE_DISABLED; } #define read_option_lowlevel(start, size, def) def #endif #else diff --git a/src/include/types.h b/src/include/types.h index 180fa3a..d1f991e 100644 --- a/src/include/types.h +++ b/src/include/types.h @@ -34,6 +34,13 @@ enum cb_err { CB_SUCCESS = 0, /**< Call completed succesfully */ CB_ERR = -1, /**< Generic error code */ CB_ERR_ARG = -2, /**< Invalid argument */ + + /* NVRAM/CMOS errors */ + CB_CMOS_OTABLE_DISABLED = -100, /**< Option table disabled */ + CB_CMOS_LAYOUT_NOT_FOUND = -101, /**< Layout file not found */ + CB_CMOS_OPTION_NOT_FOUND = -102, /**< Option string not found */ + CB_CMOS_ACCESS_ERROR = -103, /**< CMOS access error */ + CB_CMOS_CHECKSUM_INVALID = -104, /**< CMOS checksum is invalid */ };
#endif /* __TYPES_H */
-
Alexandru Gagniuc