Change in ...coreboot[master]: arch/x86: Remove weak tsc_freq_mhz() implementation

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

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

Change subject: arch/x86: Remove weak tsc_freq_mhz() implementation
......................................................................

arch/x86: Remove weak tsc_freq_mhz() implementation

Build with TSC_CONSTANT_RATE must fail when this function
is not implemented for the platform. Weak implementation
causes division by zero in timer_monotonic_get() and
turns udelay() into no delay.

Change-Id: Id3b105ea3aac37cd0cba18ce2fb06d87a055486f
Signed-off-by: Kyösti Mälkki 
---
M src/arch/x86/timestamp.c
1 file changed, 3 insertions(+), 8 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/30762/1

diff --git a/src/arch/x86/timestamp.c b/src/arch/x86/timestamp.c
index 928d7d7..cbc7bac 100644
--- a/src/arch/x86/timestamp.c
+++ b/src/arch/x86/timestamp.c
@@ -21,15 +21,10 @@
    return rdtscll();
 }

-unsigned long __weak tsc_freq_mhz(void)
-{
-	/* Default to not knowing TSC frequency. cbmem will have to fallback
-	 * on trying to determine it in userspace. */
-	return 0;
-}
-
 int timestamp_tick_freq_mhz(void)
 {
    /* Chipsets that have a constant TSC provide this value correctly. */
-	return tsc_freq_mhz();
+	if (IS_ENABLED(CONFIG_TSC_CONSTANT_RATE))
+		return tsc_freq_mhz();
+	return 0;
 }

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30762 )

Change subject: arch/x86: Remove weak tsc_freq_mhz() implementation
......................................................................

Patch Set 6:

This seems to remove the possibility for a platforms not using UDELAY_TSC to have a working implementation of this function using a tsc_freq_mhz() implementation. Is that intended?

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30762 )

Change subject: arch/x86: Remove weak tsc_freq_mhz() implementation
......................................................................

Patch Set 6:
> This seems to remove the possibility for a platforms not using UDELAY_TSC to have a working implementation of this function using a tsc_freq_mhz() implementation. Is that intended?

I guess no. I don't see why TSC_CONSTANT_RATE should depend on UDELAY_TSC.

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

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30762 )

Change subject: arch/x86: Remove weak tsc_freq_mhz() implementation
......................................................................

Patch Set 6:
> Patch Set 6:
> > This seems to remove the possibility for a platforms not using UDELAY_TSC to have a working implementation of this function using a tsc_freq_mhz() implementation. Is that intended?
>
> I guess no. I don't see why TSC_CONSTANT_RATE should depend on UDELAY_TSC.

cpu/x86/tsc/Makefilei.inc is ... interesting

I welcome the change to cleanup the Kconfig dependency.