RE: [PATCH] linux-2.6.0-test4_cyclone-hpet-fix_A0

["Pallipadi, Venkatesh" <venkatesh.pallipadi@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 1413 bytes --]



> -----Original Message-----
> From: Andrew Morton [mailto:akpm@osdl.org] 
> 
> john stultz <johnstul@us.ibm.com> wrote:
> >
> > Anyway, the HPET changes made calibrate_tsc() static, which 
> it probably
> > should be, but it broke the timer_cyclone code. This patch 
> fixes it back
> > up by re-implementing calibrate_tsc() locally as it was done in
> > timer_hpet.c 
> 
> <stdrant>
> Of course if some bozo had stuck this:
> 
> 	extern unsigned long calibrate_tsc(void);
> 
> in a header file rather than in a .c file (timer_cyclone.c), 
> this problem
> would not have occurred.  Nevereverever put extern 
> declarations in .c files!
> </stdrant>


My fault. I should have atleast test compiled cyclone timer code.


> Can we not we avoid the cut-n-paste coding?
> 
> There is also timer_tsc.c:calibrate_tsc_hpet() which is 
> almost the same as
> timer_hpet.c:calibrate_tsc().  Seem to me that we could tweak
> calibrate_tsc_hpet() a bit, unstaticalise 
> timer_tsc.c:calibrate_tsc() and
> have two functions rather than four?
> 


How about the attached patch (against mm4), that moves all 
calibrate tsc functions into a common file, avoiding the duplication. 
This time I could successfully compile cyclone timer too :). However,
I had to do an unrelated one line change in fixmap (last chunk in 
the patch) to compile for summit.


Thanks,
-Venkatesh


[-- Attachment #2: mm4-hpet-fix.patch --]
[-- Type: application/octet-stream, Size: 13007 bytes --]

diff -purN linux-2.6.0-test4-mm4/arch/i386/kernel/timers/common.c linux-2.6.0-test4-mm4-fix/arch/i386/kernel/timers/common.c
--- linux-2.6.0-test4-mm4/arch/i386/kernel/timers/common.c	1969-12-31 16:00:00.000000000 -0800
+++ linux-2.6.0-test4-mm4-fix/arch/i386/kernel/timers/common.c	2003-09-04 11:57:47.000000000 -0700
@@ -0,0 +1,138 @@
+/*	
+ *	Common functions used across the timers go here
+ */
+
+#include <linux/init.h>
+#include <linux/timex.h>
+#include <linux/errno.h>
+
+#include <asm/timer.h>
+#include <asm/hpet.h>
+
+#include "mach_timer.h"
+
+/* ------ Calibrate the TSC ------- 
+ * Return 2^32 * (1 / (TSC clocks per usec)) for do_fast_gettimeoffset().
+ * Too much 64-bit arithmetic here to do this cleanly in C, and for
+ * accuracy's sake we want to keep the overhead on the CTC speaker (channel 2)
+ * output busy loop as low as possible. We avoid reading the CTC registers
+ * directly because of the awkward 8-bit access mechanism of the 82C54
+ * device.
+ */
+
+#define CALIBRATE_TIME	(5 * 1000020/HZ)
+
+unsigned long __init calibrate_tsc(void)
+{
+	mach_prepare_counter();
+
+	{
+		unsigned long startlow, starthigh;
+		unsigned long endlow, endhigh;
+		unsigned long count;
+
+		rdtsc(startlow,starthigh);
+		mach_countup(&count);
+		rdtsc(endlow,endhigh);
+
+
+		/* Error: ECTCNEVERSET */
+		if (count <= 1)
+			goto bad_ctc;
+
+		/* 64-bit subtract - gcc just messes up with long longs */
+		__asm__("subl %2,%0\n\t"
+			"sbbl %3,%1"
+			:"=a" (endlow), "=d" (endhigh)
+			:"g" (startlow), "g" (starthigh),
+			 "0" (endlow), "1" (endhigh));
+
+		/* Error: ECPUTOOFAST */
+		if (endhigh)
+			goto bad_ctc;
+
+		/* Error: ECPUTOOSLOW */
+		if (endlow <= CALIBRATE_TIME)
+			goto bad_ctc;
+
+		__asm__("divl %2"
+			:"=a" (endlow), "=d" (endhigh)
+			:"r" (endlow), "0" (0), "1" (CALIBRATE_TIME));
+
+		return endlow;
+	}
+
+	/*
+	 * The CTC wasn't reliable: we got a hit on the very first read,
+	 * or the CPU was so fast/slow that the quotient wouldn't fit in
+	 * 32 bits..
+	 */
+bad_ctc:
+	return 0;
+}
+
+#ifdef CONFIG_HPET_TIMER
+/* ------ Calibrate the TSC using HPET -------
+ * Return 2^32 * (1 / (TSC clocks per usec)) for getting the CPU freq.
+ * Second output is parameter 1 (when non NULL)
+ * Set 2^32 * (1 / (tsc per HPET clk)) for delay_hpet().
+ * calibrate_tsc() calibrates the processor TSC by comparing
+ * it to the HPET timer of known frequency.
+ * Too much 64-bit arithmetic here to do this cleanly in C
+ */
+#define CALIBRATE_CNT_HPET 	(5 * hpet_tick)
+#define CALIBRATE_TIME_HPET 	(5 * KERNEL_TICK_USEC)
+
+unsigned long __init calibrate_tsc_hpet(unsigned long *tsc_hpet_quotient_ptr)
+{
+	unsigned long tsc_startlow, tsc_starthigh;
+	unsigned long tsc_endlow, tsc_endhigh;
+	unsigned long hpet_start, hpet_end;
+	unsigned long result, remain;
+
+	hpet_start = hpet_readl(HPET_COUNTER);
+	rdtsc(tsc_startlow, tsc_starthigh);
+	do {
+		hpet_end = hpet_readl(HPET_COUNTER);
+	} while ((hpet_end - hpet_start) < CALIBRATE_CNT_HPET);
+	rdtsc(tsc_endlow, tsc_endhigh);
+
+	/* 64-bit subtract - gcc just messes up with long longs */
+	__asm__("subl %2,%0\n\t"
+		"sbbl %3,%1"
+		:"=a" (tsc_endlow), "=d" (tsc_endhigh)
+		:"g" (tsc_startlow), "g" (tsc_starthigh),
+		 "0" (tsc_endlow), "1" (tsc_endhigh));
+
+	/* Error: ECPUTOOFAST */
+	if (tsc_endhigh)
+		goto bad_calibration;
+
+	/* Error: ECPUTOOSLOW */
+	if (tsc_endlow <= CALIBRATE_TIME_HPET)
+		goto bad_calibration;
+
+	ASM_DIV64_REG(result, remain, tsc_endlow, 0, CALIBRATE_TIME_HPET);
+	if (remain > (tsc_endlow >> 1))
+		result++; /* rounding the result */
+
+	if (tsc_hpet_quotient_ptr) {
+		unsigned long tsc_hpet_quotient;
+
+		ASM_DIV64_REG(tsc_hpet_quotient, remain, tsc_endlow, 0,
+			CALIBRATE_CNT_HPET);
+		if (remain > (tsc_endlow >> 1))
+			tsc_hpet_quotient++; /* rounding the result */
+		*tsc_hpet_quotient_ptr = tsc_hpet_quotient;
+	}
+
+	return result;
+bad_calibration:
+	/*
+	 * the CPU was so fast/slow that the quotient wouldn't fit in
+	 * 32 bits..
+	 */
+	return 0;
+}
+#endif
+
diff -purN linux-2.6.0-test4-mm4/arch/i386/kernel/timers/Makefile linux-2.6.0-test4-mm4-fix/arch/i386/kernel/timers/Makefile
--- linux-2.6.0-test4-mm4/arch/i386/kernel/timers/Makefile	2003-09-02 17:56:48.000000000 -0700
+++ linux-2.6.0-test4-mm4-fix/arch/i386/kernel/timers/Makefile	2003-09-04 11:33:04.000000000 -0700
@@ -2,7 +2,7 @@
 # Makefile for x86 timers
 #
 
-obj-y := timer.o timer_none.o timer_tsc.o timer_pit.o
+obj-y := timer.o timer_none.o timer_tsc.o timer_pit.o common.o
 
 obj-$(CONFIG_X86_CYCLONE_TIMER)	+= timer_cyclone.o
 obj-$(CONFIG_HPET_TIMER)	+= timer_hpet.o
diff -purN linux-2.6.0-test4-mm4/arch/i386/kernel/timers/timer_cyclone.c linux-2.6.0-test4-mm4-fix/arch/i386/kernel/timers/timer_cyclone.c
--- linux-2.6.0-test4-mm4/arch/i386/kernel/timers/timer_cyclone.c	2003-08-22 16:57:49.000000000 -0700
+++ linux-2.6.0-test4-mm4-fix/arch/i386/kernel/timers/timer_cyclone.c	2003-09-04 11:57:42.000000000 -0700
@@ -19,7 +19,6 @@
 #include <asm/fixmap.h>
 
 extern spinlock_t i8253_lock;
-extern unsigned long calibrate_tsc(void);
 
 /* Number of usecs that the last interrupt was delayed */
 static int delay_at_last_interrupt;
diff -purN linux-2.6.0-test4-mm4/arch/i386/kernel/timers/timer_hpet.c linux-2.6.0-test4-mm4-fix/arch/i386/kernel/timers/timer_hpet.c
--- linux-2.6.0-test4-mm4/arch/i386/kernel/timers/timer_hpet.c	2003-09-02 17:56:48.000000000 -0700
+++ linux-2.6.0-test4-mm4-fix/arch/i386/kernel/timers/timer_hpet.c	2003-09-04 11:57:38.000000000 -0700
@@ -131,63 +131,6 @@ void delay_hpet(unsigned long loops)
 	} while ((hpet_end - hpet_start) < (loops));
 }
 
-/* ------ Calibrate the TSC -------
- * Return 2^32 * (1 / (TSC clocks per usec)) for getting the CPU freq.
- * Set 2^32 * (1 / (tsc per HPET clk)) for delay_hpet().
- * calibrate_tsc() calibrates the processor TSC by comparing
- * it to the HPET timer of known frequency.
- * Too much 64-bit arithmetic here to do this cleanly in C
- */
-#define CALIBRATE_CNT_HPET 	(5 * hpet_tick)
-#define CALIBRATE_TIME_HPET 	(5 * KERNEL_TICK_USEC)
-
-static unsigned long __init calibrate_tsc(void)
-{
-	unsigned long tsc_startlow, tsc_starthigh;
-	unsigned long tsc_endlow, tsc_endhigh;
-	unsigned long hpet_start, hpet_end;
-	unsigned long result, remain;
-
-	hpet_start = hpet_readl(HPET_COUNTER);
-	rdtsc(tsc_startlow, tsc_starthigh);
-	do {
-		hpet_end = hpet_readl(HPET_COUNTER);
-	} while ((hpet_end - hpet_start) < CALIBRATE_CNT_HPET);
-	rdtsc(tsc_endlow, tsc_endhigh);
-
-	/* 64-bit subtract - gcc just messes up with long longs */
-	__asm__("subl %2,%0\n\t"
-		"sbbl %3,%1"
-		:"=a" (tsc_endlow), "=d" (tsc_endhigh)
-		:"g" (tsc_startlow), "g" (tsc_starthigh),
-		 "0" (tsc_endlow), "1" (tsc_endhigh));
-
-	/* Error: ECPUTOOFAST */
-	if (tsc_endhigh)
-		goto bad_calibration;
-
-	/* Error: ECPUTOOSLOW */
-	if (tsc_endlow <= CALIBRATE_TIME_HPET)
-		goto bad_calibration;
-
-	ASM_DIV64_REG(result, remain, tsc_endlow, 0, CALIBRATE_TIME_HPET);
-	if (remain > (tsc_endlow >> 1))
-		result++; /* rounding the result */
-
-	ASM_DIV64_REG(tsc_hpet_quotient, remain, tsc_endlow, 0,
-			CALIBRATE_CNT_HPET);
-	if (remain > (tsc_endlow >> 1))
-		tsc_hpet_quotient++; /* rounding the result */
-
-	return result;
-bad_calibration:
-	/*
-	 * the CPU was so fast/slow that the quotient wouldn't fit in
-	 * 32 bits..
-	 */
-	return 0;
-}
-
 static int __init init_hpet(char* override)
 {
 	unsigned long result, remain;
@@ -201,7 +144,7 @@ static int __init init_hpet(char* overri
 
 	printk("Using HPET for gettimeofday\n");
 	if (cpu_has_tsc) {
-		unsigned long tsc_quotient = calibrate_tsc();
+		unsigned long tsc_quotient = calibrate_tsc_hpet(&tsc_hpet_quotient);
 		if (tsc_quotient) {
 			/* report CPU clock rate in Hz.
 			 * The formula is (10^6 * 2^32) / (2^32 * 1 / (clocks/us)) =
diff -purN linux-2.6.0-test4-mm4/arch/i386/kernel/timers/timer_tsc.c linux-2.6.0-test4-mm4-fix/arch/i386/kernel/timers/timer_tsc.c
--- linux-2.6.0-test4-mm4/arch/i386/kernel/timers/timer_tsc.c	2003-09-02 17:56:48.000000000 -0700
+++ linux-2.6.0-test4-mm4-fix/arch/i386/kernel/timers/timer_tsc.c	2003-09-04 11:57:35.000000000 -0700
@@ -254,67 +254,6 @@ static void delay_tsc(unsigned long loop
 	} while ((now-bclock) < loops);
 }
 
-/* ------ Calibrate the TSC ------- 
- * Return 2^32 * (1 / (TSC clocks per usec)) for do_fast_gettimeoffset().
- * Too much 64-bit arithmetic here to do this cleanly in C, and for
- * accuracy's sake we want to keep the overhead on the CTC speaker (channel 2)
- * output busy loop as low as possible. We avoid reading the CTC registers
- * directly because of the awkward 8-bit access mechanism of the 82C54
- * device.
- */
-
-#define CALIBRATE_TIME	(5 * 1000020/HZ)
-
-static unsigned long __init calibrate_tsc(void)
-{
-	mach_prepare_counter();
-
-	{
-		unsigned long startlow, starthigh;
-		unsigned long endlow, endhigh;
-		unsigned long count;
-
-		rdtsc(startlow,starthigh);
-		mach_countup(&count);
-		rdtsc(endlow,endhigh);
-
-		last_tsc_low = endlow;
-
-		/* Error: ECTCNEVERSET */
-		if (count <= 1)
-			goto bad_ctc;
-
-		/* 64-bit subtract - gcc just messes up with long longs */
-		__asm__("subl %2,%0\n\t"
-			"sbbl %3,%1"
-			:"=a" (endlow), "=d" (endhigh)
-			:"g" (startlow), "g" (starthigh),
-			 "0" (endlow), "1" (endhigh));
-
-		/* Error: ECPUTOOFAST */
-		if (endhigh)
-			goto bad_ctc;
-
-		/* Error: ECPUTOOSLOW */
-		if (endlow <= CALIBRATE_TIME)
-			goto bad_ctc;
-
-		__asm__("divl %2"
-			:"=a" (endlow), "=d" (endhigh)
-			:"r" (endlow), "0" (0), "1" (CALIBRATE_TIME));
-
-		return endlow;
-	}
-
-	/*
-	 * The CTC wasn't reliable: we got a hit on the very first read,
-	 * or the CPU was so fast/slow that the quotient wouldn't fit in
-	 * 32 bits..
-	 */
-bad_ctc:
-	return 0;
-}
-
 #ifdef CONFIG_HPET_TIMER
 static void mark_offset_tsc_hpet(void)
 {
@@ -363,58 +302,6 @@ static void mark_offset_tsc_hpet(void)
 	ASM_MUL64_REG(temp, delay_at_last_interrupt,
 			hpet_usec_quotient, delay_at_last_interrupt);
 }
-
-/* ------ Calibrate the TSC based on HPET timer -------
- * Return 2^32 * (1 / (TSC clocks per usec)) for do_fast_gettimeoffset().
- * calibrate_tsc() calibrates the processor TSC by comparing
- * it to the HPET timer of known frequency.
- * Too much 64-bit arithmetic here to do this cleanly in C
- */
-
-#define CALIBRATE_CNT_HPET 	(5 * hpet_tick)
-#define CALIBRATE_TIME_HPET 	(5 * KERNEL_TICK_USEC)
-
-unsigned long __init calibrate_tsc_hpet(void)
-{
-	unsigned long tsc_startlow, tsc_starthigh;
-	unsigned long tsc_endlow, tsc_endhigh;
-	unsigned long hpet_start, hpet_end;
-	unsigned long result, remain;
-
-	hpet_start = hpet_readl(HPET_COUNTER);
-	rdtsc(tsc_startlow, tsc_starthigh);
-	do {
-		hpet_end = hpet_readl(HPET_COUNTER);
-	} while ((hpet_end - hpet_start) < CALIBRATE_CNT_HPET);
-	rdtsc(tsc_endlow, tsc_endhigh);
-
-	/* 64-bit subtract - gcc just messes up with long longs */
-	__asm__("subl %2,%0\n\t"
-		"sbbl %3,%1"
-		:"=a" (tsc_endlow), "=d" (tsc_endhigh)
-		:"g" (tsc_startlow), "g" (tsc_starthigh),
-		 "0" (tsc_endlow), "1" (tsc_endhigh));
-
-	/* Error: ECPUTOOFAST */
-	if (tsc_endhigh)
-		goto bad_calibration;
-
-	/* Error: ECPUTOOSLOW */
-	if (tsc_endlow <= CALIBRATE_TIME_HPET)
-		goto bad_calibration;
-
-	ASM_DIV64_REG(result, remain, tsc_endlow, 0, CALIBRATE_TIME_HPET);
-	if (remain > (tsc_endlow >> 1))
-		result++; /* rounding the result */
-
-	return result;
-bad_calibration:
-	/*
-	 * the CPU was so fast/slow that the quotient wouldn't fit in
-	 * 32 bits..
-	 */
-	return 0;
-}
 #endif
 
 #ifdef CONFIG_CPU_FREQ
@@ -515,7 +402,7 @@ static int __init init_tsc(char* overrid
 		if (is_hpet_enabled()){
 			unsigned long result, remain;
 			printk("Using TSC for gettimeofday\n");
-			tsc_quotient = calibrate_tsc_hpet();
+			tsc_quotient = calibrate_tsc_hpet(NULL);
 			timer_tsc.mark_offset = &mark_offset_tsc_hpet;
 			/*
 			 * Math to calculate hpet to usec multiplier
diff -purN linux-2.6.0-test4-mm4/include/asm-i386/timer.h linux-2.6.0-test4-mm4-fix/include/asm-i386/timer.h
--- linux-2.6.0-test4-mm4/include/asm-i386/timer.h	2003-09-02 17:56:48.000000000 -0700
+++ linux-2.6.0-test4-mm4-fix/include/asm-i386/timer.h	2003-09-04 11:21:24.000000000 -0700
@@ -38,8 +38,10 @@ extern struct timer_opts timer_tsc;
 extern struct timer_opts timer_cyclone;
 #endif
 
+extern unsigned long calibrate_tsc(void);
 #ifdef CONFIG_HPET_TIMER
 extern struct timer_opts timer_hpet;
+extern unsigned long calibrate_tsc_hpet(unsigned long *tsc_hpet_quotient_ptr);
 #endif
 
 #endif
diff -purN linux-2.6.0-test4-mm4/include/asm-i386/fixmap.h linux-2.6.0-test4-mm4-fix/include/asm-i386/fixmap.h
--- linux-2.6.0-test4-mm4/include/asm-i386/fixmap.h	2003-09-02 17:56:48.000000000 -0700
+++ linux-2.6.0-test4-mm4-fix/include/asm-i386/fixmap.h	2003-09-04 11:42:13.000000000 -0700
@@ -73,7 +73,7 @@ enum fixed_addresses {
 	FIX_TSS_0,
 	FIX_ENTRY_TRAMPOLINE_1,
 	FIX_ENTRY_TRAMPOLINE_0,
-#ifdef CONFIG_X86_CYCLONE
+#ifdef CONFIG_X86_CYCLONE_TIMER
 	FIX_CYCLONE_TIMER, /*cyclone timer register*/
 	FIX_VSTACK_HOLE_2,
 #endif