[coreboot-gerrit] New patch to review for coreboot: 83b0037 pc80/mc146818rtc: Return an error code rather than an integer

Alexandru Gagniuc (mr.nuke.me@gmail.com) gerrit at coreboot.org
Sun Nov 24 02:24:49 CET 2013


Alexandru Gagniuc (mr.nuke.me at 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 at 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 at 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 */



More information about the coreboot-gerrit mailing list