Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80320?usp=email )
(
1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: libpayload: timer: Revert timer_hz() return type to 64-bits ......................................................................
libpayload: timer: Revert timer_hz() return type to 64-bits
It seems that reducing the return type of timer_hz() to uint32_t in CB:78888 was a bad idea... some Intel platforms actually use their raw CPU clock for the timestamp counter which can be higher than 4GHz. This patch reverts it back to uint64_t.
Also remove the redundant assertion in timer/generic.c since timer_us() itself already does that check.
Cq-Depend: chromium:5274555 Change-Id: I471c7de7a28aec5bb965b23525ed579481ac8361 Signed-off-by: Julius Werner jwerner@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/80320 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de Reviewed-by: Yidi Lin yidilin@google.com --- M payloads/libpayload/drivers/timer/arm64_arch_timer.c M payloads/libpayload/drivers/timer/generic.c M payloads/libpayload/drivers/timer/rdtsc.c M payloads/libpayload/include/libpayload.h M payloads/libpayload/libc/time.c 5 files changed, 8 insertions(+), 10 deletions(-)
Approvals: Yidi Lin: Looks good to me, approved build bot (Jenkins): Verified Nico Huber: Looks good to me, but someone else must approve
diff --git a/payloads/libpayload/drivers/timer/arm64_arch_timer.c b/payloads/libpayload/drivers/timer/arm64_arch_timer.c index b4d2b86..087d9e7 100644 --- a/payloads/libpayload/drivers/timer/arm64_arch_timer.c +++ b/payloads/libpayload/drivers/timer/arm64_arch_timer.c @@ -32,7 +32,7 @@ #include <arch/lib_helpers.h> #include <libpayload.h>
-uint32_t timer_hz(void) +uint64_t timer_hz(void) { return raw_read_cntfrq_el0(); } diff --git a/payloads/libpayload/drivers/timer/generic.c b/payloads/libpayload/drivers/timer/generic.c index bd5674b..46bc291 100644 --- a/payloads/libpayload/drivers/timer/generic.c +++ b/payloads/libpayload/drivers/timer/generic.c @@ -33,10 +33,8 @@ #include <assert.h> #include <libpayload.h>
-uint32_t timer_hz(void) +uint64_t timer_hz(void) { - /* libc/time.c currently requires all timers to be at least 1MHz. */ - assert(CONFIG_LP_TIMER_GENERIC_HZ >= 1000000); return CONFIG_LP_TIMER_GENERIC_HZ; }
diff --git a/payloads/libpayload/drivers/timer/rdtsc.c b/payloads/libpayload/drivers/timer/rdtsc.c index 952bc0b..d5bf1d2 100644 --- a/payloads/libpayload/drivers/timer/rdtsc.c +++ b/payloads/libpayload/drivers/timer/rdtsc.c @@ -35,10 +35,9 @@ #include <arch/rdtsc.h> #include <assert.h>
-uint32_t timer_hz(void) +uint64_t timer_hz(void) { - assert(UINT32_MAX / 1000 >= lib_sysinfo.cpu_khz); - return lib_sysinfo.cpu_khz * 1000; + return (uint64_t)lib_sysinfo.cpu_khz * 1000; }
uint64_t timer_raw_value(void) diff --git a/payloads/libpayload/include/libpayload.h b/payloads/libpayload/include/libpayload.h index 6da4564..5e34124 100644 --- a/payloads/libpayload/include/libpayload.h +++ b/payloads/libpayload/include/libpayload.h @@ -519,7 +519,7 @@
/* Timer functions. */ /* Defined by each architecture. */ -uint32_t timer_hz(void); +uint64_t timer_hz(void); uint64_t timer_raw_value(void); uint64_t timer_us(uint64_t base); /* Generic. */ diff --git a/payloads/libpayload/libc/time.c b/payloads/libpayload/libc/time.c index 28f2b3e..64de800 100644 --- a/payloads/libpayload/libc/time.c +++ b/payloads/libpayload/libc/time.c @@ -171,14 +171,15 @@
u64 timer_us(u64 base) { - static u32 hz, mult = USECS_PER_SEC; + static u64 hz; + static u32 mult = USECS_PER_SEC; u32 div;
// Only check timer_hz once. Assume it doesn't change. if (hz == 0) { hz = timer_hz(); if (hz < mult) { - printf("Timer frequency %" PRIu32 " is too low, " + printf("Timer frequency %" PRIu64 " is too low, " "must be at least 1MHz.\n", hz); halt(); }