Alexandru Gagniuc (mr.nuke.me@gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/8306
-gerrit
commit 0c22517d3044ac28d57ba562d1402f1de3036ffc Author: Alexandru Gagniuc mr.nuke.me@gmail.com Date: Fri Jan 30 00:07:12 2015 -0600
drivers/pc80/mc146818rtc: Reduce superfluous preprocessor use
cmos_init() had layers of preprocessor directives, which resulted in a complete mess. Refactor it to make use of the IS_ENABLED() macro. This improves readability significantly.
One of the changes is to remove in inline stub declaration of (get|set)_option. Although that provided the ability for the compiler to optimize out code when USE_OPTION_TABLE is not selected, there is no evidence that such savings are measureable.
Change-Id: I07f00084d809adbb55031b2079f71136ade3028e Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com --- src/drivers/pc80/Makefile.inc | 4 +- src/drivers/pc80/mc146818rtc.c | 101 ++++++++++++++++++++++------------------- src/include/pc80/mc146818rtc.h | 14 ++---- 3 files changed, 58 insertions(+), 61 deletions(-)
diff --git a/src/drivers/pc80/Makefile.inc b/src/drivers/pc80/Makefile.inc index 1d28152..9715949 100644 --- a/src/drivers/pc80/Makefile.inc +++ b/src/drivers/pc80/Makefile.inc @@ -7,9 +7,7 @@ ramstage-$(CONFIG_UDELAY_IO) += udelay_io.c ramstage-y += keyboard.c ramstage-$(CONFIG_SPKMODEM) += spkmodem.c
-ifeq ($(CONFIG_DRIVERS_MC146818),y) -romstage-$(CONFIG_USE_OPTION_TABLE) += mc146818rtc_early.c -endif +romstage-$(CONFIG_DRIVERS_MC146818) += mc146818rtc_early.c
romstage-$(CONFIG_LPC_TPM) += tpm.c romstage-$(CONFIG_SPKMODEM) += spkmodem.c diff --git a/src/drivers/pc80/mc146818rtc.c b/src/drivers/pc80/mc146818rtc.c index fc3adf0..3ce48c8 100644 --- a/src/drivers/pc80/mc146818rtc.c +++ b/src/drivers/pc80/mc146818rtc.c @@ -26,9 +26,15 @@ #include <boot/coreboot_tables.h> #include <rtc.h> #include <string.h> +#include <cbfs.h> + +/* There's no way around this include guard. option_table.h is autogenerated */ #if CONFIG_USE_OPTION_TABLE #include "option_table.h" -#include <cbfs.h> +#else +#define LB_CKS_RANGE_START 0 +#define LB_CKS_RANGE_END 0 +#define LB_CKS_LOC 0 #endif
@@ -48,7 +54,6 @@ static void cmos_reset_date(void) rtc_set(&time); }
-#if CONFIG_USE_OPTION_TABLE static int cmos_checksum_valid(int range_start, int range_end, int cks_loc) { int i; @@ -71,21 +76,20 @@ static void cmos_set_checksum(int range_start, int range_end, int cks_loc) cmos_write(((sum >> 8) & 0x0ff), cks_loc); cmos_write(((sum >> 0) & 0x0ff), cks_loc + 1); } -#endif
#define RTC_CONTROL_DEFAULT (RTC_24H) #define RTC_FREQ_SELECT_DEFAULT (RTC_REF_CLCK_32KHZ | RTC_RATE_1024HZ)
#ifndef __SMM__ -void cmos_init(int invalid) +void cmos_init(bool invalid) { - int cmos_invalid = 0; - int checksum_invalid = 0; -#if CONFIG_USE_OPTION_TABLE - unsigned char x; -#endif + bool cmos_invalid = false; + bool checksum_invalid = false; + bool clear_cmos; + size_t i; + uint8_t x;
-#ifndef __PRE_RAM__ +#if !defined(__PRE_RAM__) && !defined(__SMM__) /* * Avoid clearing pending interrupts and resetting the RTC control * register in the resume path because the Linux kernel relies on @@ -98,38 +102,37 @@ void cmos_init(int invalid)
printk(BIOS_DEBUG, "RTC Init\n");
-#if CONFIG_USE_OPTION_TABLE - /* See if there has been a CMOS power problem. */ - x = cmos_read(RTC_VALID); - cmos_invalid = !(x & RTC_VRT); + if (IS_ENABLED(CONFIG_USE_OPTION_TABLE)) { + /* See if there has been a CMOS power problem. */ + x = cmos_read(RTC_VALID); + cmos_invalid = !(x & RTC_VRT);
- /* See if there is a CMOS checksum error */ - checksum_invalid = !cmos_checksum_valid(PC_CKS_RANGE_START, - PC_CKS_RANGE_END,PC_CKS_LOC); + /* See if there is a CMOS checksum error */ + checksum_invalid = !cmos_checksum_valid(PC_CKS_RANGE_START, + PC_CKS_RANGE_END, PC_CKS_LOC);
-#define CLEAR_CMOS 0 -#else -#define CLEAR_CMOS 1 -#endif + clear_cmos = false; + } else { + clear_cmos = true; + }
if (invalid || cmos_invalid || checksum_invalid) { -#if CLEAR_CMOS - int i; - - cmos_write(0, 0x01); - cmos_write(0, 0x03); - cmos_write(0, 0x05); - for (i = 10; i < 128; i++) - cmos_write(0, i); -#endif + if(clear_cmos) { + cmos_write(0, 0x01); + cmos_write(0, 0x03); + cmos_write(0, 0x05); + for (i = 10; i < 128; i++) + cmos_write(0, i); + } + if (cmos_invalid) cmos_reset_date();
printk(BIOS_WARNING, "RTC:%s%s%s%s\n", - invalid?" Clear requested":"", - cmos_invalid?" Power Problem":"", - checksum_invalid?" Checksum invalid":"", - CLEAR_CMOS?" zeroing cmos":""); + invalid ? " Clear requested":"", + cmos_invalid ? " Power Problem":"", + checksum_invalid ? " Checksum invalid":"", + clear_cmos ? " zeroing cmos":""); }
/* Setup the real time clock */ @@ -139,24 +142,23 @@ void cmos_init(int invalid) /* Ensure all reserved bits are 0 in register D */ cmos_write(RTC_VRT, RTC_VALID);
-#if CONFIG_USE_OPTION_TABLE - /* See if there is a LB CMOS checksum error */ - checksum_invalid = !cmos_checksum_valid(LB_CKS_RANGE_START, - LB_CKS_RANGE_END,LB_CKS_LOC); - if (checksum_invalid) - printk(BIOS_DEBUG, "RTC: coreboot checksum invalid\n"); - - /* Make certain we have a valid checksum */ - cmos_set_checksum(PC_CKS_RANGE_START, PC_CKS_RANGE_END, PC_CKS_LOC); -#endif + if(IS_ENABLED(CONFIG_USE_OPTION_TABLE)) { + /* See if there is a LB CMOS checksum error */ + checksum_invalid = !cmos_checksum_valid(LB_CKS_RANGE_START, + LB_CKS_RANGE_END,LB_CKS_LOC); + if (checksum_invalid) + printk(BIOS_DEBUG, "RTC: coreboot checksum invalid\n"); + + /* Make certain we have a valid checksum */ + cmos_set_checksum(PC_CKS_RANGE_START, PC_CKS_RANGE_END, PC_CKS_LOC); + }
/* Clear any pending interrupts */ cmos_read(RTC_INTR_FLAGS); } -#endif +#endif /* __SMM__ */
-#if CONFIG_USE_OPTION_TABLE /* * This routine returns the value of the requested bits. * input bit = bit count from the beginning of the cmos image @@ -200,6 +202,9 @@ enum cb_err get_option(void *dest, const char *name) size_t namelen; int found = 0;
+ if (!IS_ENABLED(CONFIG_USE_OPTION_TABLE)) + return CB_CMOS_OTABLE_DISABLED; + /* Figure out how long name is */ namelen = strnlen(name, CMOS_MAX_NAME_LENGTH);
@@ -280,6 +285,9 @@ enum cb_err set_option(const char *name, void *value) size_t namelen; int found = 0;
+ if (!IS_ENABLED(CONFIG_USE_OPTION_TABLE)) + return CB_CMOS_OTABLE_DISABLED; + /* Figure out how long name is */ namelen = strnlen(name, CMOS_MAX_NAME_LENGTH);
@@ -318,7 +326,6 @@ enum cb_err set_option(const char *name, void *value)
return CB_SUCCESS; } -#endif /* CONFIG_USE_OPTION_TABLE */
/* * If the CMOS is cleared, the rtc_reg has the invalid date. That diff --git a/src/include/pc80/mc146818rtc.h b/src/include/pc80/mc146818rtc.h index 90ae7ae..c74af66 100644 --- a/src/include/pc80/mc146818rtc.h +++ b/src/include/pc80/mc146818rtc.h @@ -169,21 +169,13 @@ static inline void cmos_write32(u8 offset, u32 value) #endif
#if !defined(__ROMCC__) -void cmos_init(int invalid); +void cmos_init(bool invalid); void cmos_check_update_date(void); -#if CONFIG_USE_OPTION_TABLE + 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 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 /* defined(__ROMCC__) */ #include <drivers/pc80/mc146818rtc_early.c> #endif /* !defined(__ROMCC__) */