Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38194 )
Change subject: drivers/pc80/rtc: Remove duplicate cmos_chksum_valid() ......................................................................
drivers/pc80/rtc: Remove duplicate cmos_chksum_valid()
This moves reference to LB_CKS_xxx to a file that is only built with USE_OPTION_TABLE and where option_table.h is always generated.
Change-Id: I5a4b86921876c24cd1d310b674119b960c3d2fd6 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/drivers/pc80/rtc/mc146818rtc.c M src/drivers/pc80/rtc/mc146818rtc_boot.c M src/drivers/pc80/rtc/option.c M src/include/option.h 4 files changed, 9 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/38194/1
diff --git a/src/drivers/pc80/rtc/mc146818rtc.c b/src/drivers/pc80/rtc/mc146818rtc.c index fe6900f..d6a6279 100644 --- a/src/drivers/pc80/rtc/mc146818rtc.c +++ b/src/drivers/pc80/rtc/mc146818rtc.c @@ -26,18 +26,6 @@ #include <security/vboot/vbnv_layout.h> #include <types.h>
-/* There's no way around this include guard. option_table.h is autogenerated */ -#if CONFIG(USE_OPTION_TABLE) -#include "option_table.h" -#else -#define LB_CKS_RANGE_START 0 -#define LB_CKS_RANGE_END 0 -#define LB_CKS_LOC 0 -#endif - -/* Don't warn for checking >= LB_CKS_RANGE_START even though it may be 0. */ -#pragma GCC diagnostic ignored "-Wtype-limits" - static void cmos_reset_date(void) { /* Now setup a default date equals to the build date */ @@ -154,8 +142,7 @@
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); + checksum_invalid = !cmos_lb_cks_valid(); if (checksum_invalid) printk(BIOS_DEBUG, "RTC: coreboot checksum invalid\n");
diff --git a/src/drivers/pc80/rtc/mc146818rtc_boot.c b/src/drivers/pc80/rtc/mc146818rtc_boot.c index 307b1cf..89b2be3 100644 --- a/src/drivers/pc80/rtc/mc146818rtc_boot.c +++ b/src/drivers/pc80/rtc/mc146818rtc_boot.c @@ -17,30 +17,9 @@ #include <pc80/mc146818rtc.h> #include <fallback.h>
-#if CONFIG(USE_OPTION_TABLE) -#include <option_table.h> - -static int cmos_chksum_valid(void) -{ - unsigned char addr; - u16 sum, old_sum; - - sum = 0; - /* Compute the cmos checksum */ - for (addr = LB_CKS_RANGE_START; addr <= LB_CKS_RANGE_END; addr++) - sum += cmos_read(addr); - - /* Read the stored checksum */ - old_sum = cmos_read(LB_CKS_LOC) << 8; - old_sum |= cmos_read(LB_CKS_LOC + 1); - - return sum == old_sum; -} - void sanitize_cmos(void) { - if (cmos_error() || !cmos_chksum_valid() || - CONFIG(STATIC_OPTION_TABLE)) { + if (cmos_error() || !cmos_lb_cks_valid() || CONFIG(STATIC_OPTION_TABLE)) { size_t length = 128; const unsigned char *cmos_default = cbfs_boot_map_with_leak("cmos.default", @@ -54,7 +33,6 @@ } } } -#endif
#if CONFIG_MAX_REBOOT_CNT > 15 #error "CONFIG_MAX_REBOOT_CNT too high" @@ -84,7 +62,7 @@ { unsigned char byte;
- if (cmos_error() || (CONFIG(USE_OPTION_TABLE) && !cmos_chksum_valid())) { + if (cmos_error() || (CONFIG(USE_OPTION_TABLE) && !cmos_lb_cks_valid())) { /* Invalid CMOS checksum detected! * Force fallback boot... */ diff --git a/src/drivers/pc80/rtc/option.c b/src/drivers/pc80/rtc/option.c index 65f75ef..0f4e553 100644 --- a/src/drivers/pc80/rtc/option.c +++ b/src/drivers/pc80/rtc/option.c @@ -233,3 +233,8 @@ rdev_munmap(&rdev, ct); return CB_SUCCESS; } + +int cmos_lb_cks_valid(void) +{ + return cmos_checksum_valid(LB_CKS_RANGE_START, LB_CKS_RANGE_END, LB_CKS_LOC); +} diff --git a/src/include/option.h b/src/include/option.h index 4bd5dee..e3736c9 100644 --- a/src/include/option.h +++ b/src/include/option.h @@ -17,6 +17,7 @@ #include <types.h>
void sanitize_cmos(void); +int cmos_lb_cks_valid(void);
/* * FIXME: get_option() needs to be abstracted better so that other non-volatile
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38194
to look at the new patch set (#3).
Change subject: drivers/pc80/rtc: Remove duplicate cmos_chksum_valid() ......................................................................
drivers/pc80/rtc: Remove duplicate cmos_chksum_valid()
This moves reference to LB_CKS_xxx to a file that is only built with USE_OPTION_TABLE and where option_table.h is always generated.
Change-Id: I5a4b86921876c24cd1d310b674119b960c3d2fd6 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/drivers/pc80/rtc/mc146818rtc.c M src/drivers/pc80/rtc/mc146818rtc_boot.c M src/drivers/pc80/rtc/option.c M src/include/option.h 4 files changed, 9 insertions(+), 37 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/38194/3
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38194
to look at the new patch set (#5).
Change subject: drivers/pc80/rtc: Remove duplicate cmos_chksum_valid() ......................................................................
drivers/pc80/rtc: Remove duplicate cmos_chksum_valid()
This moves reference to LB_CKS_xxx to a file that is only built with USE_OPTION_TABLE and where option_table.h is always generated.
Change-Id: I5a4b86921876c24cd1d310b674119b960c3d2fd6 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/drivers/pc80/rtc/mc146818rtc.c M src/drivers/pc80/rtc/mc146818rtc_boot.c M src/include/pc80/mc146818rtc.h 3 files changed, 11 insertions(+), 22 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/38194/5
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38194
to look at the new patch set (#6).
Change subject: drivers/pc80/rtc: Remove duplicate cmos_chksum_valid() ......................................................................
drivers/pc80/rtc: Remove duplicate cmos_chksum_valid()
This moves reference to LB_CKS_xxx to a file that is only built with USE_OPTION_TABLE and where option_table.h is always generated.
Change-Id: I5a4b86921876c24cd1d310b674119b960c3d2fd6 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/drivers/pc80/rtc/Makefile.inc M src/drivers/pc80/rtc/mc146818rtc.c M src/drivers/pc80/rtc/mc146818rtc_boot.c M src/include/pc80/mc146818rtc.h 4 files changed, 13 insertions(+), 23 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/38194/6
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38194 )
Change subject: drivers/pc80/rtc: Remove duplicate cmos_chksum_valid() ......................................................................
Patch Set 6: Code-Review+1
Hello Paul Menzel, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38194
to look at the new patch set (#7).
Change subject: drivers/pc80/rtc: Remove duplicate cmos_chksum_valid() ......................................................................
drivers/pc80/rtc: Remove duplicate cmos_chksum_valid()
Change-Id: I5a4b86921876c24cd1d310b674119b960c3d2fd6 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/drivers/pc80/rtc/Makefile.inc M src/drivers/pc80/rtc/mc146818rtc.c M src/drivers/pc80/rtc/mc146818rtc_boot.c M src/include/pc80/mc146818rtc.h 4 files changed, 13 insertions(+), 23 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/38194/7
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38194 )
Change subject: drivers/pc80/rtc: Remove duplicate cmos_chksum_valid() ......................................................................
Patch Set 8: Code-Review+2
Kyösti Mälkki has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38194 )
Change subject: drivers/pc80/rtc: Remove duplicate cmos_chksum_valid() ......................................................................
drivers/pc80/rtc: Remove duplicate cmos_chksum_valid()
Change-Id: I5a4b86921876c24cd1d310b674119b960c3d2fd6 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/38194 Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/drivers/pc80/rtc/Makefile.inc M src/drivers/pc80/rtc/mc146818rtc.c M src/drivers/pc80/rtc/mc146818rtc_boot.c M src/include/pc80/mc146818rtc.h 4 files changed, 13 insertions(+), 23 deletions(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved
diff --git a/src/drivers/pc80/rtc/Makefile.inc b/src/drivers/pc80/rtc/Makefile.inc index 998b7e7..749306b 100644 --- a/src/drivers/pc80/rtc/Makefile.inc +++ b/src/drivers/pc80/rtc/Makefile.inc @@ -1,6 +1,7 @@ ifeq ($(CONFIG_ARCH_X86),y)
-bootblock-$(CONFIG_DRIVERS_MC146818) += mc146818rtc_boot.c +all-$(CONFIG_DRIVERS_MC146818) += mc146818rtc_boot.c + bootblock-$(CONFIG_DRIVERS_MC146818) += mc146818rtc.c postcar-$(CONFIG_DRIVERS_MC146818) += mc146818rtc.c romstage-$(CONFIG_DRIVERS_MC146818) += mc146818rtc.c diff --git a/src/drivers/pc80/rtc/mc146818rtc.c b/src/drivers/pc80/rtc/mc146818rtc.c index b870da2..715e440 100644 --- a/src/drivers/pc80/rtc/mc146818rtc.c +++ b/src/drivers/pc80/rtc/mc146818rtc.c @@ -53,7 +53,7 @@ rtc_set(&time); }
-static int cmos_checksum_valid(int range_start, int range_end, int cks_loc) +int cmos_checksum_valid(int range_start, int range_end, int cks_loc) { int i; u16 sum, old_sum; @@ -69,7 +69,7 @@ return sum == old_sum; }
-static void cmos_set_checksum(int range_start, int range_end, int cks_loc) +void cmos_set_checksum(int range_start, int range_end, int cks_loc) { int i; u16 sum; @@ -149,8 +149,7 @@
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); + checksum_invalid = !cmos_lb_cks_valid(); if (checksum_invalid) printk(BIOS_DEBUG, "RTC: coreboot checksum invalid\n");
diff --git a/src/drivers/pc80/rtc/mc146818rtc_boot.c b/src/drivers/pc80/rtc/mc146818rtc_boot.c index 2998c73..e599aff 100644 --- a/src/drivers/pc80/rtc/mc146818rtc_boot.c +++ b/src/drivers/pc80/rtc/mc146818rtc_boot.c @@ -19,27 +19,14 @@ #if CONFIG(USE_OPTION_TABLE) #include <option_table.h>
-int cmos_chksum_valid(void) +int cmos_lb_cks_valid(void) { - unsigned char addr; - u16 sum, old_sum; - - sum = 0; - /* Compute the cmos checksum */ - for (addr = LB_CKS_RANGE_START; addr <= LB_CKS_RANGE_END; addr++) - sum += cmos_read(addr); - - /* Read the stored checksum */ - old_sum = cmos_read(LB_CKS_LOC) << 8; - old_sum |= cmos_read(LB_CKS_LOC + 1); - - return sum == old_sum; + return cmos_checksum_valid(LB_CKS_RANGE_START, LB_CKS_RANGE_END, LB_CKS_LOC); }
void sanitize_cmos(void) { - if (cmos_error() || !cmos_chksum_valid() || - CONFIG(STATIC_OPTION_TABLE)) { + if (cmos_error() || !cmos_lb_cks_valid() || CONFIG(STATIC_OPTION_TABLE)) { size_t length = 128; const unsigned char *cmos_default = cbfs_boot_map_with_leak("cmos.default", @@ -83,7 +70,7 @@ { unsigned char byte;
- if (!CONFIG(USE_OPTION_TABLE) || cmos_error() || !cmos_chksum_valid()) { + if (!CONFIG(USE_OPTION_TABLE) || cmos_error() || !cmos_lb_cks_valid()) { /* Invalid CMOS checksum detected! * Force fallback boot... */ diff --git a/src/include/pc80/mc146818rtc.h b/src/include/pc80/mc146818rtc.h index 5fd0729..91413d1 100644 --- a/src/include/pc80/mc146818rtc.h +++ b/src/include/pc80/mc146818rtc.h @@ -176,7 +176,10 @@ void cmos_init(bool invalid); void cmos_check_update_date(void); int cmos_error(void); -int cmos_chksum_valid(void); +int cmos_lb_cks_valid(void); + +int cmos_checksum_valid(int range_start, int range_end, int cks_loc); +void cmos_set_checksum(int range_start, int range_end, int cks_loc);
enum cb_err set_option(const char *name, void *val); enum cb_err get_option(void *dest, const char *name);
-
Angel Pons (Code Review) -
Kyösti Mälkki (Code Review)
-
Paul Menzel (Code Review)