Change in coreboot[master]: Revert "cpu/intel/common: Extend FSB detection to cover TSC"

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

Hello Aaron Durbin, Patrick Rudolph, HAOUAS Elyes, Arthur Heymans, Paul Menzel, [e-mail], build bot (Jenkins), Nico Huber, Patrick Georgi, Furquan Shaikh,

I'd like you to do a code review. Please visit

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

to review the following change.

Change subject: Revert "cpu/intel/common: Extend FSB detection to cover TSC"
......................................................................

Revert "cpu/intel/common: Extend FSB detection to cover TSC"

This reverts commit ecea91679f3193b308eabcd5f1f82525f0f5669f.

Reason for revert: reported reset loops on model 6fx.

Change-Id: I9756213260e6e9e3f3e1c9ab91c28ec295fd9f96
Signed-off-by: Kyösti Mälkki 
---
M src/cpu/intel/common/fsb.c
1 file changed, 16 insertions(+), 54 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/35564/1

diff --git a/src/cpu/intel/common/fsb.c b/src/cpu/intel/common/fsb.c
index 5ad98d4..2d86abd 100644
--- a/src/cpu/intel/common/fsb.c
+++ b/src/cpu/intel/common/fsb.c
@@ -14,7 +14,6 @@
 #include <arch/early_variables.h>
 #include <cpu/cpu.h>
 #include <cpu/x86/msr.h>
-#include <cpu/x86/tsc.h>
 #include <cpu/intel/speedstep.h>
 #include <cpu/intel/fsb.h>
 #include <console/console.h>
@@ -22,18 +21,15 @@
 #include <delay.h>

static u32 g_timer_fsb CAR_GLOBAL;
-static u32 g_timer_tsc CAR_GLOBAL;

-/* This is not an architectural MSR. */
-#define MSR_PLATFORM_INFO 0xce
-
-static int get_fsb_tsc(int *fsb, int *ratio)
+static int get_fsb(void)
 {
    struct cpuinfo_x86 c;
    static const short core_fsb[8] = { -1, 133, -1, 166, -1, 100, -1, -1 };
    static const short core2_fsb[8] = { 266, 133, 200, 166, 333, 100, 400, -1 };
    static const short f2x_fsb[8] = { 100, 133, 200, 166, 333, -1, -1, -1 };
    msr_t msr;
+	int ret = -2;

get_fms(&c, cpuid_eax(1));
    switch (c.x86) {
@@ -41,75 +37,53 @@
    	switch (c.x86_model) {
    	case 0xe:  /* Core Solo/Duo */
    	case 0x1c: /* Atom */
-			*fsb = core_fsb[rdmsr(MSR_FSB_FREQ).lo & 7];
-			*ratio = rdmsr(MSR_EBC_FREQUENCY_ID).lo >> 24;
+			ret = core_fsb[rdmsr(MSR_FSB_FREQ).lo & 7];
    		break;
    	case 0xf:  /* Core 2 or Xeon */
    	case 0x17: /* Enhanced Core */
-			*fsb = core2_fsb[rdmsr(MSR_FSB_FREQ).lo & 7];
-			*ratio = rdmsr(MSR_EBC_FREQUENCY_ID).lo >> 24;
+			ret = core2_fsb[rdmsr(MSR_FSB_FREQ).lo & 7];
    		break;
    	case 0x25: /* Nehalem BCLK fixed at 133MHz */
-			*fsb = 133;
-			*ratio = (rdmsr(MSR_PLATFORM_INFO).lo >> 8) & 0xff;
+			ret = 133;
    		break;
    	case 0x2a: /* SandyBridge BCLK fixed at 100MHz */
    	case 0x3a: /* IvyBridge BCLK fixed at 100MHz */
    	case 0x3c: /* Haswell BCLK fixed at 100MHz */
    	case 0x45: /* Haswell-ULT BCLK fixed at 100MHz */
    	case 0x4d: /* Rangeley BCLK fixed at 100MHz */
-			*fsb = 100;
-			*ratio = (rdmsr(MSR_PLATFORM_INFO).lo >> 8) & 0xff;
+			ret = 100;
    		break;
-		default:
-			return -2;
    	}
    	break;
    case 0xf: /* Netburst */
    	msr = rdmsr(MSR_EBC_FREQUENCY_ID);
-		*ratio = msr.lo >> 24;
    	switch (c.x86_model) {
    	case 0x2:
-			*fsb = f2x_fsb[(msr.lo >> 16) & 7];
+			ret = f2x_fsb[(msr.lo >> 16) & 7];
    		break;
    	case 0x3:
    	case 0x4:
    	case 0x6:
-			*fsb = core2_fsb[(msr.lo >> 16) & 7];
+			ret = core2_fsb[(msr.lo >> 16) & 7];
    		break;
-		default:
-			return -2;
    	}
-		break;
-	default:
-		return -2;
    }
-	if (*fsb > 0)
-		return 0;
-	return -1;
+	return ret;
 }

-static void resolve_timebase(void)
+static int set_timer_fsb(void)
 {
-	int ret, fsb, ratio;
+	int ret = get_fsb();

-	ret = get_fsb_tsc(&fsb, &ratio);
-	if (ret == 0) {
-		u32 tsc = 100 * DIV_ROUND_CLOSEST(ratio * fsb, 100);
-		car_set_var(g_timer_fsb, fsb);
-		car_set_var(g_timer_tsc, tsc);
-		return;
+	if (ret > 0) {
+		car_set_var(g_timer_fsb, ret);
+		return 0;
    }
-
    if (ret == -1)
    	printk(BIOS_ERR, "FSB not found\n");
    if (ret == -2)
    	printk(BIOS_ERR, "CPU not supported\n");
-
-	/* Set some semi-ridiculous defaults. */
-	car_set_var(g_timer_fsb, 500);
-	car_set_var(g_timer_tsc, 5000);
-	return;
+	return -1;
 }

u32 get_timer_fsb(void)
@@ -120,22 +94,10 @@
    if (fsb > 0)
    	return fsb;

-	resolve_timebase();
+	set_timer_fsb();
    return car_get_var(g_timer_fsb);
 }

-unsigned long tsc_freq_mhz(void)
-{
-	u32 tsc;
-
-	tsc = car_get_var(g_timer_tsc);
-	if (tsc > 0)
-		return tsc;
-
-	resolve_timebase();
-	return car_get_var(g_timer_tsc);
-}
-
 /**
  * @brief Returns three times the FSB clock in MHz
  *

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

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

Change subject: Revert "cpu/intel/common: Extend FSB detection to cover TSC"
......................................................................

Patch Set 1:

CB:35548 is candidate to fix this instead of revert. Apparently we hit bad docs and I don't have hardware to test core/core2/atom.

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

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

Change subject: Revert "cpu/intel/common: Extend FSB detection to cover TSC"
......................................................................

Abandoned