Change in coreboot[master]: drivers/pc80/rtc: Refactor some USE_OPTION_TABLE

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38183 )

Change subject: drivers/pc80/rtc: Refactor some USE_OPTION_TABLE
......................................................................

drivers/pc80/rtc: Refactor some USE_OPTION_TABLE

Change-Id: I3a5004db021af6127de2f058bec9d84a985bae67
Signed-off-by: Kyösti Mälkki 
---
M src/drivers/pc80/rtc/mc146818rtc_boot.c
M src/include/pc80/mc146818rtc.h
2 files changed, 4 insertions(+), 8 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/38183/1

diff --git a/src/drivers/pc80/rtc/mc146818rtc_boot.c b/src/drivers/pc80/rtc/mc146818rtc_boot.c
index fd0095e..c009d67 100644
--- a/src/drivers/pc80/rtc/mc146818rtc_boot.c
+++ b/src/drivers/pc80/rtc/mc146818rtc_boot.c
@@ -16,13 +16,12 @@
 #include <option.h>
 #include <pc80/mc146818rtc.h>
 #include <fallback.h>
+
 #if CONFIG(USE_OPTION_TABLE)
 #include <option_table.h>
-#endif

-int cmos_chksum_valid(void)
+static int cmos_chksum_valid(void)
 {
-#if CONFIG(USE_OPTION_TABLE)
    unsigned char addr;
    u16 sum, old_sum;

@@ -36,9 +35,6 @@
    old_sum |= cmos_read(LB_CKS_LOC + 1);

return sum == old_sum;
-#else
-	return 0;
-#endif
 }

void sanitize_cmos(void)
@@ -58,6 +54,7 @@
    	}
    }
 }
+#endif

#if  CONFIG_MAX_REBOOT_CNT > 15
 #error "CONFIG_MAX_REBOOT_CNT too high"
@@ -87,7 +84,7 @@
 {
    unsigned char byte;

-	if (cmos_error() || !cmos_chksum_valid()) {
+	if (!CONFIG(USE_OPTION_TABLE) || cmos_error() || !cmos_chksum_valid()) {
    	/* Invalid CMOS checksum detected!
    	 * Force fallback boot...
    	 */
diff --git a/src/include/pc80/mc146818rtc.h b/src/include/pc80/mc146818rtc.h
index e3bd63a..b064461 100644
--- a/src/include/pc80/mc146818rtc.h
+++ b/src/include/pc80/mc146818rtc.h
@@ -176,7 +176,6 @@
 void cmos_init(bool invalid);
 void cmos_check_update_date(void);
 int cmos_error(void);
-int cmos_chksum_valid(void);

enum cb_err set_option(const char *name, void *val);
 enum cb_err get_option(void *dest, const char *name);

[Kyösti Mälkki (Code Review)]

Hello build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/38183

to look at the new patch set (#4).

Change subject: drivers/pc80/rtc: Refactor some USE_OPTION_TABLE
......................................................................

drivers/pc80/rtc: Refactor some USE_OPTION_TABLE

Change-Id: I3a5004db021af6127de2f058bec9d84a985bae67
Signed-off-by: Kyösti Mälkki 
---
M src/drivers/pc80/rtc/mc146818rtc_boot.c
M src/include/pc80/mc146818rtc.h
2 files changed, 3 insertions(+), 9 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/38183/4

[Kyösti Mälkki (Code Review)]

Hello build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/38183

to look at the new patch set (#5).

Change subject: drivers/pc80/rtc: Refactor some USE_OPTION_TABLE
......................................................................

drivers/pc80/rtc: Refactor some USE_OPTION_TABLE

Change-Id: I3a5004db021af6127de2f058bec9d84a985bae67
Signed-off-by: Kyösti Mälkki 
---
M src/drivers/pc80/rtc/mc146818rtc_boot.c
1 file changed, 2 insertions(+), 7 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/38183/5

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38183 )

Change subject: drivers/pc80/rtc: Refactor some USE_OPTION_TABLE
......................................................................

Patch Set 6: Code-Review+2

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38183 )

Change subject: drivers/pc80/rtc: Refactor some USE_OPTION_TABLE
......................................................................

Patch Set 6: Code-Review+1

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38183 )

Change subject: drivers/pc80/rtc: Refactor some USE_OPTION_TABLE
......................................................................

Patch Set 7: Code-Review+2

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38183 )

Change subject: drivers/pc80/rtc: Refactor some USE_OPTION_TABLE
......................................................................

drivers/pc80/rtc: Refactor some USE_OPTION_TABLE

Change-Id: I3a5004db021af6127de2f058bec9d84a985bae67
Signed-off-by: Kyösti Mälkki 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/38183
Reviewed-by: Angel Pons 
Reviewed-by: Patrick Rudolph 
Reviewed-by: Paul Menzel 
Tested-by: build bot (Jenkins) 
---
M src/drivers/pc80/rtc/mc146818rtc_boot.c
1 file changed, 2 insertions(+), 7 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Paul Menzel: Looks good to me, but someone else must approve
  Patrick Rudolph: Looks good to me, approved
  Angel Pons: Looks good to me, approved

diff --git a/src/drivers/pc80/rtc/mc146818rtc_boot.c b/src/drivers/pc80/rtc/mc146818rtc_boot.c
index a52e222..2998c73 100644
--- a/src/drivers/pc80/rtc/mc146818rtc_boot.c
+++ b/src/drivers/pc80/rtc/mc146818rtc_boot.c
@@ -15,13 +15,12 @@
 #include <cbfs.h>
 #include <pc80/mc146818rtc.h>
 #include <fallback.h>
+
 #if CONFIG(USE_OPTION_TABLE)
 #include <option_table.h>
-#endif

int cmos_chksum_valid(void)
 {
-#if CONFIG(USE_OPTION_TABLE)
    unsigned char addr;
    u16 sum, old_sum;

@@ -35,12 +34,8 @@
    old_sum |= cmos_read(LB_CKS_LOC + 1);

return sum == old_sum;
-#else
-	return 0;
-#endif
 }

-#if CONFIG(USE_OPTION_TABLE)
 void sanitize_cmos(void)
 {
    if (cmos_error() || !cmos_chksum_valid() ||
@@ -88,7 +83,7 @@
 {
    unsigned char byte;

-	if (cmos_error() || !cmos_chksum_valid()) {
+	if (!CONFIG(USE_OPTION_TABLE) || cmos_error() || !cmos_chksum_valid()) {
    	/* Invalid CMOS checksum detected!
    	 * Force fallback boot...
    	 */