LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 1/3] ia64: convert to use clocksource code
@ 2007-04-26 20:26 Peter Keilty
  2007-04-26 20:27 ` [PATCH 2/3] ia64: remove interpolater code Peter Keilty
                   ` (5 more replies)
  0 siblings, 6 replies; 21+ messages in thread
From: Peter Keilty @ 2007-04-26 20:26 UTC (permalink / raw)
  To: linux-ia64; +Cc: John Stultz, Peter Keilty, Eric Whitney, linux-kernel

From: Peter Keilty <peter.keilty@hp.com>

Initial ia64 conversion to the generic timekeeping/clocksource code.

Signed-off-by: Peter Keilty <peter.keilty@hp.com>
Signed-off-by: John Stultz <johnstul@us.ibm.com>

---

 arch/ia64/Kconfig                     |    6 +-
 arch/ia64/configs/bigsur_defconfig    |    2 
 arch/ia64/configs/gensparse_defconfig |    2 
 arch/ia64/configs/sim_defconfig       |    2 
 arch/ia64/configs/sn2_defconfig       |    2 
 arch/ia64/configs/tiger_defconfig     |    2 
 arch/ia64/configs/zx1_defconfig       |    2 
 arch/ia64/defconfig                   |    2 
 arch/ia64/kernel/asm-offsets.c        |   18 ++----
 arch/ia64/kernel/cyclone.c            |   45 +++++++++++-----
 arch/ia64/kernel/fsys.S               |   93 ++++++++++++++--------------------
 arch/ia64/kernel/time.c               |   70 ++++++++++++++++++++++---
 arch/ia64/sn/kernel/sn2/timer.c       |   28 +++++++---
 drivers/acpi/processor_idle.c         |    4 +
 drivers/char/hpet.c                   |   68 +++++++++++++-----------
 include/linux/clocksource.h           |    3 +
 16 files changed, 214 insertions(+), 135 deletions(-)

linux-2.6.21-rc1_timeofday-arch-ia64_C7.patch
============================================
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index d51f0f1..9a7260a 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -56,7 +56,11 @@ config GENERIC_CALIBRATE_DELAY
 	bool
 	default y
 
-config TIME_INTERPOLATION
+config GENERIC_TIME
+	bool
+	default y
+
+config GENERIC_TIME_VSYSCALL
 	bool
 	default y
 
diff --git a/arch/ia64/configs/bigsur_defconfig b/arch/ia64/configs/bigsur_defconfig
index 90e9c2e..9eb48c0 100644
--- a/arch/ia64/configs/bigsur_defconfig
+++ b/arch/ia64/configs/bigsur_defconfig
@@ -85,7 +85,7 @@ CONFIG_MMU=y
 CONFIG_SWIOTLB=y
 CONFIG_RWSEM_XCHGADD_ALGORITHM=y
 CONFIG_GENERIC_CALIBRATE_DELAY=y
-CONFIG_TIME_INTERPOLATION=y
+CONFIG_GENERIC_TIME=y
 CONFIG_EFI=y
 CONFIG_GENERIC_IOMAP=y
 CONFIG_SCHED_NO_NO_OMIT_FRAME_POINTER=y
diff --git a/arch/ia64/configs/gensparse_defconfig b/arch/ia64/configs/gensparse_defconfig
index 0d29aa2..3a9ed95 100644
--- a/arch/ia64/configs/gensparse_defconfig
+++ b/arch/ia64/configs/gensparse_defconfig
@@ -86,7 +86,7 @@ CONFIG_MMU=y
 CONFIG_SWIOTLB=y
 CONFIG_RWSEM_XCHGADD_ALGORITHM=y
 CONFIG_GENERIC_CALIBRATE_DELAY=y
-CONFIG_TIME_INTERPOLATION=y
+CONFIG_GENERIC_TIME=y
 CONFIG_EFI=y
 CONFIG_GENERIC_IOMAP=y
 CONFIG_SCHED_NO_NO_OMIT_FRAME_POINTER=y
diff --git a/arch/ia64/configs/sim_defconfig b/arch/ia64/configs/sim_defconfig
index d9146c3..c420d9f 100644
--- a/arch/ia64/configs/sim_defconfig
+++ b/arch/ia64/configs/sim_defconfig
@@ -86,7 +86,7 @@ CONFIG_MMU=y
 CONFIG_SWIOTLB=y
 CONFIG_RWSEM_XCHGADD_ALGORITHM=y
 CONFIG_GENERIC_CALIBRATE_DELAY=y
-CONFIG_TIME_INTERPOLATION=y
+CONFIG_GENERIC_TIME=y
 CONFIG_EFI=y
 CONFIG_GENERIC_IOMAP=y
 CONFIG_SCHED_NO_NO_OMIT_FRAME_POINTER=y
diff --git a/arch/ia64/configs/sn2_defconfig b/arch/ia64/configs/sn2_defconfig
index 64e951d..4c9ffc4 100644
--- a/arch/ia64/configs/sn2_defconfig
+++ b/arch/ia64/configs/sn2_defconfig
@@ -93,7 +93,7 @@ CONFIG_SWIOTLB=y
 CONFIG_RWSEM_XCHGADD_ALGORITHM=y
 CONFIG_GENERIC_FIND_NEXT_BIT=y
 CONFIG_GENERIC_CALIBRATE_DELAY=y
-CONFIG_TIME_INTERPOLATION=y
+CONFIG_GENERIC_TIME=y
 CONFIG_DMI=y
 CONFIG_EFI=y
 CONFIG_GENERIC_IOMAP=y
diff --git a/arch/ia64/configs/tiger_defconfig b/arch/ia64/configs/tiger_defconfig
index 9d1cffb..5bd072a 100644
--- a/arch/ia64/configs/tiger_defconfig
+++ b/arch/ia64/configs/tiger_defconfig
@@ -86,7 +86,7 @@ CONFIG_MMU=y
 CONFIG_SWIOTLB=y
 CONFIG_RWSEM_XCHGADD_ALGORITHM=y
 CONFIG_GENERIC_CALIBRATE_DELAY=y
-CONFIG_TIME_INTERPOLATION=y
+CONFIG_GENERIC_TIME=y
 CONFIG_EFI=y
 CONFIG_GENERIC_IOMAP=y
 CONFIG_SCHED_NO_NO_OMIT_FRAME_POINTER=y
diff --git a/arch/ia64/configs/zx1_defconfig b/arch/ia64/configs/zx1_defconfig
index 949dc46..17bdfc8 100644
--- a/arch/ia64/configs/zx1_defconfig
+++ b/arch/ia64/configs/zx1_defconfig
@@ -84,7 +84,7 @@ CONFIG_MMU=y
 CONFIG_SWIOTLB=y
 CONFIG_RWSEM_XCHGADD_ALGORITHM=y
 CONFIG_GENERIC_CALIBRATE_DELAY=y
-CONFIG_TIME_INTERPOLATION=y
+CONFIG_GENERIC_TIME=y
 CONFIG_EFI=y
 CONFIG_GENERIC_IOMAP=y
 CONFIG_SCHED_NO_NO_OMIT_FRAME_POINTER=y
diff --git a/arch/ia64/defconfig b/arch/ia64/defconfig
index 9001b3f..61ad76a 100644
--- a/arch/ia64/defconfig
+++ b/arch/ia64/defconfig
@@ -86,7 +86,7 @@ CONFIG_MMU=y
 CONFIG_SWIOTLB=y
 CONFIG_RWSEM_XCHGADD_ALGORITHM=y
 CONFIG_GENERIC_CALIBRATE_DELAY=y
-CONFIG_TIME_INTERPOLATION=y
+CONFIG_GENERIC_TIME=y
 CONFIG_EFI=y
 CONFIG_GENERIC_IOMAP=y
 CONFIG_SCHED_NO_NO_OMIT_FRAME_POINTER=y
diff --git a/arch/ia64/kernel/asm-offsets.c b/arch/ia64/kernel/asm-offsets.c
index 75a2a2c..3e4039e 100644
--- a/arch/ia64/kernel/asm-offsets.c
+++ b/arch/ia64/kernel/asm-offsets.c
@@ -7,6 +7,7 @@
 #define ASM_OFFSETS_C 1
 
 #include <linux/sched.h>
+#include <linux/clocksource.h>
 
 #include <asm-ia64/processor.h>
 #include <asm-ia64/ptrace.h>
@@ -255,17 +256,10 @@ #endif
 	BLANK();
 
 	/* used by fsys_gettimeofday in arch/ia64/kernel/fsys.S */
-	DEFINE(IA64_TIME_INTERPOLATOR_ADDRESS_OFFSET, offsetof (struct time_interpolator, addr));
-	DEFINE(IA64_TIME_INTERPOLATOR_SOURCE_OFFSET, offsetof (struct time_interpolator, source));
-	DEFINE(IA64_TIME_INTERPOLATOR_SHIFT_OFFSET, offsetof (struct time_interpolator, shift));
-	DEFINE(IA64_TIME_INTERPOLATOR_NSEC_OFFSET, offsetof (struct time_interpolator, nsec_per_cyc));
-	DEFINE(IA64_TIME_INTERPOLATOR_OFFSET_OFFSET, offsetof (struct time_interpolator, offset));
-	DEFINE(IA64_TIME_INTERPOLATOR_LAST_CYCLE_OFFSET, offsetof (struct time_interpolator, last_cycle));
-	DEFINE(IA64_TIME_INTERPOLATOR_LAST_COUNTER_OFFSET, offsetof (struct time_interpolator, last_counter));
-	DEFINE(IA64_TIME_INTERPOLATOR_JITTER_OFFSET, offsetof (struct time_interpolator, jitter));
-	DEFINE(IA64_TIME_INTERPOLATOR_MASK_OFFSET, offsetof (struct time_interpolator, mask));
-	DEFINE(IA64_TIME_SOURCE_CPU, TIME_SOURCE_CPU);
-	DEFINE(IA64_TIME_SOURCE_MMIO64, TIME_SOURCE_MMIO64);
-	DEFINE(IA64_TIME_SOURCE_MMIO32, TIME_SOURCE_MMIO32);
 	DEFINE(IA64_TIMESPEC_TV_NSEC_OFFSET, offsetof (struct timespec, tv_nsec));
+	DEFINE(IA64_CLOCKSOURCE_MASK_OFFSET, offsetof (struct clocksource, mask));
+	DEFINE(IA64_CLOCKSOURCE_MULT_OFFSET, offsetof (struct clocksource, mult));
+	DEFINE(IA64_CLOCKSOURCE_SHIFT_OFFSET, offsetof (struct clocksource, shift));
+	DEFINE(IA64_CLOCKSOURCE_MMIO_PTR_OFFSET, offsetof (struct clocksource, fsys_mmio_ptr));
+	DEFINE(IA64_CLOCKSOURCE_CYCLE_LAST_OFFSET, offsetof (struct clocksource, cycle_last));
 }
diff --git a/arch/ia64/kernel/cyclone.c b/arch/ia64/kernel/cyclone.c
index e00b215..280383b 100644
--- a/arch/ia64/kernel/cyclone.c
+++ b/arch/ia64/kernel/cyclone.c
@@ -3,6 +3,7 @@ #include <linux/smp.h>
 #include <linux/time.h>
 #include <linux/errno.h>
 #include <linux/timex.h>
+#include <linux/clocksource.h>
 #include <asm/io.h>
 
 /* IBM Summit (EXA) Cyclone counter code*/
@@ -18,13 +19,21 @@ void __init cyclone_setup(void)
 	use_cyclone = 1;
 }
 
+static void __iomem *cyclone_mc_ptr;
 
-struct time_interpolator cyclone_interpolator = {
-	.source =	TIME_SOURCE_MMIO64,
-	.shift =	16,
-	.frequency =	CYCLONE_TIMER_FREQ,
-	.drift =	-100,
-	.mask =		(1LL << 40) - 1
+static cycle_t read_cyclone(void)
+{
+	return (cycle_t)readq((void __iomem *)cyclone_mc_ptr);
+}
+
+static struct clocksource clocksource_cyclone = {
+        .name           = "cyclone",
+        .rating         = 300,
+        .read           = read_cyclone,
+        .mask           = (1LL << 40) - 1,
+        .mult           = 0, /*to be caluclated*/
+        .shift          = 16,
+        .is_continuous  = 1,
 };
 
 int __init init_cyclone_clock(void)
@@ -44,13 +53,15 @@ int __init init_cyclone_clock(void)
 	offset = (CYCLONE_CBAR_ADDR);
 	reg = (u64*)ioremap_nocache(offset, sizeof(u64));
 	if(!reg){
-		printk(KERN_ERR "Summit chipset: Could not find valid CBAR register.\n");
+		printk(KERN_ERR "Summit chipset: Could not find valid CBAR"
+				" register.\n");
 		use_cyclone = 0;
 		return -ENODEV;
 	}
 	base = readq(reg);
 	if(!base){
-		printk(KERN_ERR "Summit chipset: Could not find valid CBAR value.\n");
+		printk(KERN_ERR "Summit chipset: Could not find valid CBAR"
+				" value.\n");
 		use_cyclone = 0;
 		return -ENODEV;
 	}
@@ -60,7 +71,8 @@ int __init init_cyclone_clock(void)
 	offset = (base + CYCLONE_PMCC_OFFSET);
 	reg = (u64*)ioremap_nocache(offset, sizeof(u64));
 	if(!reg){
-		printk(KERN_ERR "Summit chipset: Could not find valid PMCC register.\n");
+		printk(KERN_ERR "Summit chipset: Could not find valid PMCC"
+				" register.\n");
 		use_cyclone = 0;
 		return -ENODEV;
 	}
@@ -71,7 +83,8 @@ int __init init_cyclone_clock(void)
 	offset = (base + CYCLONE_MPCS_OFFSET);
 	reg = (u64*)ioremap_nocache(offset, sizeof(u64));
 	if(!reg){
-		printk(KERN_ERR "Summit chipset: Could not find valid MPCS register.\n");
+		printk(KERN_ERR "Summit chipset: Could not find valid MPCS"
+				" register.\n");
 		use_cyclone = 0;
 		return -ENODEV;
 	}
@@ -82,7 +95,8 @@ int __init init_cyclone_clock(void)
 	offset = (base + CYCLONE_MPMC_OFFSET);
 	cyclone_timer = (u32*)ioremap_nocache(offset, sizeof(u32));
 	if(!cyclone_timer){
-		printk(KERN_ERR "Summit chipset: Could not find valid MPMC register.\n");
+		printk(KERN_ERR "Summit chipset: Could not find valid MPMC"
+				" register.\n");
 		use_cyclone = 0;
 		return -ENODEV;
 	}
@@ -93,7 +107,8 @@ int __init init_cyclone_clock(void)
 		int stall = 100;
 		while(stall--) barrier();
 		if(readl(cyclone_timer) == old){
-			printk(KERN_ERR "Summit chipset: Counter not counting! DISABLED\n");
+			printk(KERN_ERR "Summit chipset: Counter not counting!"
+					" DISABLED\n");
 			iounmap(cyclone_timer);
 			cyclone_timer = 0;
 			use_cyclone = 0;
@@ -101,8 +116,10 @@ int __init init_cyclone_clock(void)
 		}
 	}
 	/* initialize last tick */
-	cyclone_interpolator.addr = cyclone_timer;
-	register_time_interpolator(&cyclone_interpolator);
+	clocksource_cyclone.fsys_mmio_ptr = cyclone_mc_ptr = cyclone_timer;
+	clocksource_cyclone.mult = clocksource_hz2mult(CYCLONE_TIMER_FREQ,
+						clocksource_cyclone.shift);
+	clocksource_register(&clocksource_cyclone);
 
 	return 0;
 }
diff --git a/arch/ia64/kernel/fsys.S b/arch/ia64/kernel/fsys.S
index 7a05b1c..778ca5f 100644
--- a/arch/ia64/kernel/fsys.S
+++ b/arch/ia64/kernel/fsys.S
@@ -145,13 +145,6 @@ (p8)	br.spnt.many fsys_fallback_syscall
 	FSYS_RETURN
 END(fsys_set_tid_address)
 
-/*
- * Ensure that the time interpolator structure is compatible with the asm code
- */
-#if IA64_TIME_INTERPOLATOR_SOURCE_OFFSET !=0 || IA64_TIME_INTERPOLATOR_SHIFT_OFFSET != 2 \
-	|| IA64_TIME_INTERPOLATOR_JITTER_OFFSET != 3 || IA64_TIME_INTERPOLATOR_NSEC_OFFSET != 4
-#error fsys_gettimeofday incompatible with changes to struct time_interpolator
-#endif
 #define CLOCK_REALTIME 0
 #define CLOCK_MONOTONIC 1
 #define CLOCK_DIVIDE_BY_1000 0x4000
@@ -177,19 +170,18 @@ (p6)    br.cond.spnt.few .fail_einval
 	// r11 = preserved: saved ar.pfs
 	// r12 = preserved: memory stack
 	// r13 = preserved: thread pointer
-	// r14 = address of mask / mask
+	// r14 = address of mask / mask value
 	// r15 = preserved: system call number
 	// r16 = preserved: current task pointer
 	// r17 = wall to monotonic use
-	// r18 = time_interpolator->offset
-	// r19 = address of wall_to_monotonic
-	// r20 = pointer to struct time_interpolator / pointer to time_interpolator->address
-	// r21 = shift factor
-	// r22 = address of time interpolator->last_counter
-	// r23 = address of time_interpolator->last_cycle
-	// r24 = adress of time_interpolator->offset
-	// r25 = last_cycle value
-	// r26 = last_counter value
+	// r19 = address of itc_lastcycle
+	// r20 = struct clocksource / address of first element
+	// r21 = shift value
+	// r22 = address of itc_jitter/ wall_to_monotonic
+	// r23 = address of shift
+	// r24 = address mult factor / cycle_last value
+	// r25 = itc_lastcycle value
+	// r26 = address clocksource cycle_last
 	// r27 = pointer to xtime
 	// r28 = sequence number at the beginning of critcal section
 	// r29 = address of seqlock
@@ -199,9 +191,9 @@ (p6)    br.cond.spnt.few .fail_einval
 	// p6,p7 short term use
 	// p8 = timesource ar.itc
 	// p9 = timesource mmio64
-	// p10 = timesource mmio32
+	// p10 = timesource mmio32 - not used
 	// p11 = timesource not to be handled by asm code
-	// p12 = memory time source ( = p9 | p10)
+	// p12 = memory time source ( = p9 | p10) - not used
 	// p13 = do cmpxchg with time_interpolator_last_cycle
 	// p14 = Divide by 1000
 	// p15 = Add monotonic
@@ -212,61 +204,55 @@ (p6)    br.cond.spnt.few .fail_einval
 	tnat.nz p6,p0 = r31	// branch deferred since it does not fit into bundle structure
 	mov pr = r30,0xc000	// Set predicates according to function
 	add r2 = TI_FLAGS+IA64_TASK_SIZE,r16
-	movl r20 = time_interpolator
+	movl r20 = fsyscall_clock // load fsyscall clocksource address
 	;;
-	ld8 r20 = [r20]		// get pointer to time_interpolator structure
+	add r10 = IA64_CLOCKSOURCE_MMIO_PTR_OFFSET,r20
 	movl r29 = xtime_lock
 	ld4 r2 = [r2]		// process work pending flags
 	movl r27 = xtime
 	;;	// only one bundle here
-	ld8 r21 = [r20]		// first quad with control information
+	add r14 = IA64_CLOCKSOURCE_MASK_OFFSET,r20
+	movl r22 = itc_jitter
+	add r24 = IA64_CLOCKSOURCE_MULT_OFFSET,r20
 	and r2 = TIF_ALLWORK_MASK,r2
 (p6)    br.cond.spnt.few .fail_einval	// deferred branch
 	;;
-	add r10 = IA64_TIME_INTERPOLATOR_ADDRESS_OFFSET,r20
-	extr r3 = r21,32,32	// time_interpolator->nsec_per_cyc
-	extr r8 = r21,0,16	// time_interpolator->source
+	ld8 r30 = [r10]		// clocksource->mmio_ptr
+	movl r19 = itc_lastcycle
+	add r23 = IA64_CLOCKSOURCE_SHIFT_OFFSET,r20
 	cmp.ne p6, p0 = 0, r2	// Fallback if work is scheduled
 (p6)    br.cond.spnt.many fsys_fallback_syscall
 	;;
-	cmp.eq p8,p12 = 0,r8	// Check for cpu timer
-	cmp.eq p9,p0 = 1,r8	// MMIO64 ?
-	extr r2 = r21,24,8	// time_interpolator->jitter
-	cmp.eq p10,p0 = 2,r8	// MMIO32 ?
-	cmp.ltu p11,p0 = 2,r8	// function or other clock
-(p11)	br.cond.spnt.many fsys_fallback_syscall
+	ld8 r14 = [r14]         // clocksource mask value
+	ld4 r2 = [r22]		// itc_jitter value
+	add r26 = IA64_CLOCKSOURCE_CYCLE_LAST_OFFSET,r20 // clock fsyscall_cycle_last
+	ld4 r3 = [r24]		// clocksource->mult value
+	cmp.eq p8,p9 = 0,r30	// Check for cpu timer, no mmio_ptr, set p8, clear p9
 	;;
-	setf.sig f7 = r3	// Setup for scaling of counter
-(p15)	movl r19 = wall_to_monotonic
-(p12)	ld8 r30 = [r10]
-	cmp.ne p13,p0 = r2,r0	// need jitter compensation?
-	extr r21 = r21,16,8	// shift factor
+	setf.sig f7 = r3	// Setup for mult scaling of counter
+(p15)	movl r22 = wall_to_monotonic
+	ld4 r21 = [r23]		// shift value
+(p8)	cmp.ne p13,p0 = r2,r0	// need jitter compensation, set p13
+(p9)	cmp.eq p13,p0 = 0,r30	// if mmio_ptr, clear p13 jitter control
 	;;
 .time_redo:
 	.pred.rel.mutex p8,p9,p10
 	ld4.acq r28 = [r29]	// xtime_lock.sequence. Must come first for locking purposes
 (p8)	mov r2 = ar.itc		// CPU_TIMER. 36 clocks latency!!!
-	add r22 = IA64_TIME_INTERPOLATOR_LAST_COUNTER_OFFSET,r20
 (p9)	ld8 r2 = [r30]		// readq(ti->address). Could also have latency issues..
-(p10)	ld4 r2 = [r30]		// readw(ti->address)
-(p13)	add r23 = IA64_TIME_INTERPOLATOR_LAST_CYCLE_OFFSET,r20
+(p13)	ld8 r25 = [r19]		// get itc_lastcycle value
 	;;			// could be removed by moving the last add upward
-	ld8 r26 = [r22]		// time_interpolator->last_counter
-(p13)	ld8 r25 = [r23]		// time interpolator->last_cycle
-	add r24 = IA64_TIME_INTERPOLATOR_OFFSET_OFFSET,r20
-(p15)	ld8 r17 = [r19],IA64_TIMESPEC_TV_NSEC_OFFSET
  	ld8 r9 = [r27],IA64_TIMESPEC_TV_NSEC_OFFSET
-	add r14 = IA64_TIME_INTERPOLATOR_MASK_OFFSET, r20
+	ld8 r24 = [r26]		// get fsyscall_cycle_last value
+(p15)	ld8 r17 = [r22],IA64_TIMESPEC_TV_NSEC_OFFSET
 	;;
-	ld8 r18 = [r24]		// time_interpolator->offset
 	ld8 r8 = [r27],-IA64_TIMESPEC_TV_NSEC_OFFSET	// xtime.tv_nsec
-(p13)	sub r3 = r25,r2	// Diff needed before comparison (thanks davidm)
+(p13)	sub r3 = r25,r2		// Diff needed before comparison (thanks davidm)
 	;;
-	ld8 r14 = [r14]		// time_interpolator->mask
-(p13)	cmp.gt.unc p6,p7 = r3,r0	// check if it is less than last. p6,p7 cleared
-	sub r10 = r2,r26	// current_counter - last_counter
+(p13)	cmp.gt.unc p6,p7 = r3,r0 // check if it is less than last. p6,p7 cleared
+	sub r10 = r2,r24	// current_counter - last_counter
 	;;
-(p6)	sub r10 = r25,r26	// time we got was less than last_cycle
+(p6)	sub r10 = r25,r24	// time we got was less than last_cycle
 (p7)	mov ar.ccv = r25	// more than last_cycle. Prep for cmpxchg
 	;;
 	and r10 = r10,r14	// Apply mask
@@ -274,22 +260,21 @@ (p7)	mov ar.ccv = r25	// more than last_
 	setf.sig f8 = r10
 	nop.i 123
 	;;
-(p7)	cmpxchg8.rel r3 = [r23],r2,ar.ccv
+(p7)	cmpxchg8.rel r3 = [r19],r2,ar.ccv
 EX(.fail_efault, probe.w.fault r31, 3)	// This takes 5 cycles and we have spare time
 	xmpy.l f8 = f8,f7	// nsec_per_cyc*(counter-last_counter)
 (p15)	add r9 = r9,r17		// Add wall to monotonic.secs to result secs
 	;;
-(p15)	ld8 r17 = [r19],-IA64_TIMESPEC_TV_NSEC_OFFSET
+(p15)	ld8 r17 = [r22],-IA64_TIMESPEC_TV_NSEC_OFFSET
 (p7)	cmp.ne p7,p0 = r25,r3	// if cmpxchg not successful redo
 	// simulate tbit.nz.or p7,p0 = r28,0
 	and r28 = ~1,r28	// Make sequence even to force retry if odd
 	getf.sig r2 = f8
 	mf
-	add r8 = r8,r18		// Add time interpolator offset
 	;;
 	ld4 r10 = [r29]		// xtime_lock.sequence
 (p15)	add r8 = r8, r17	// Add monotonic.nsecs to nsecs
-	shr.u r2 = r2,r21
+	shr.u r2 = r2,r21	// shift by factor
 	;;		// overloaded 3 bundles!
 	// End critical section.
 	add r8 = r8,r2		// Add xtime.nsecs
diff --git a/arch/ia64/kernel/time.c b/arch/ia64/kernel/time.c
index 39e0cd3..dc52c98 100644
--- a/arch/ia64/kernel/time.c
+++ b/arch/ia64/kernel/time.c
@@ -20,6 +20,7 @@ #include <linux/interrupt.h>
 #include <linux/efi.h>
 #include <linux/profile.h>
 #include <linux/timex.h>
+#include <linux/clocksource.h>
 
 #include <asm/machvec.h>
 #include <asm/delay.h>
@@ -29,6 +30,10 @@ #include <asm/sal.h>
 #include <asm/sections.h>
 #include <asm/system.h>
 
+static cycle_t itc_get_cycles(void);
+cycle_t	itc_lastcycle __attribute__((aligned(L1_CACHE_BYTES)));
+int	itc_jitter __attribute__((aligned(L1_CACHE_BYTES)));
+
 volatile int time_keeper_id = 0; /* smp_processor_id() of time-keeper */
 
 #ifdef CONFIG_IA64_DEBUG_IRQ
@@ -38,11 +43,16 @@ EXPORT_SYMBOL(last_cli_ip);
 
 #endif
 
-static struct time_interpolator itc_interpolator = {
-	.shift = 16,
-	.mask = 0xffffffffffffffffLL,
-	.source = TIME_SOURCE_CPU
+static struct clocksource clocksource_itc = {
+        .name           = "itc",
+        .rating         = 350,
+        .read           = itc_get_cycles,
+        .mask           = 0xffffffffffffffffLL,
+        .mult           = 0, /*to be caluclated*/
+        .shift          = 16,
+        .is_continuous  = 1,
 };
+static struct clocksource *clocksource_itc_p;
 
 static irqreturn_t
 timer_interrupt (int irq, void *dev_id)
@@ -211,8 +221,6 @@ ia64_init_itm (void)
 					+ itc_freq/2)/itc_freq;
 
 	if (!(sal_platform_features & IA64_SAL_PLATFORM_FEATURE_ITC_DRIFT)) {
-		itc_interpolator.frequency = local_cpu_data->itc_freq;
-		itc_interpolator.drift = itc_drift;
 #ifdef CONFIG_SMP
 		/* On IA64 in an SMP configuration ITCs are never accurately synchronized.
 		 * Jitter compensation requires a cmpxchg which may limit
@@ -224,15 +232,54 @@ #ifdef CONFIG_SMP
 		 * even going backward) if the ITC offsets between the individual CPUs
 		 * are too large.
 		 */
-		if (!nojitter) itc_interpolator.jitter = 1;
+		if (!nojitter) itc_jitter = 1;
 #endif
-		register_time_interpolator(&itc_interpolator);
 	}
 
 	/* Setup the CPU local timer tick */
 	ia64_cpu_local_tick();
+
+	if (!clocksource_itc_p) {
+        	/* Sort out mult/shift values: */
+		clocksource_itc.mult = clocksource_hz2mult(local_cpu_data->itc_freq,
+						   clocksource_itc.shift);
+        	clocksource_register(&clocksource_itc);
+		clocksource_itc_p = &clocksource_itc;
+	}
+}
+
+
+static cycle_t itc_get_cycles()
+{
+	if (itc_jitter) {
+		u64 lcycle;
+		u64 now;
+
+		do {
+			lcycle = itc_lastcycle;
+			now = get_cycles();
+			if (lcycle && time_after(lcycle, now))
+				return lcycle;
+
+			/* When holding the xtime write lock, there's no need
+			 * to add the overhead of the cmpxchg.  Readers are
+			 * force to retry until the write lock is released.
+			 */
+			if (spin_is_locked(&xtime_lock.lock)) {
+				itc_lastcycle = now;
+				return now;
+			}
+			/* Keep track of the last timer value returned.
+			 * The use of cmpxchg here will cause contention in
+			 * an SMP environment.
+			 */
+		} while (unlikely(cmpxchg(&itc_lastcycle, lcycle, now) != lcycle));
+		return now;
+	} else
+		return get_cycles();
 }
 
+
 static struct irqaction timer_irqaction = {
 	.handler =	timer_interrupt,
 	.flags =	IRQF_DISABLED,
@@ -308,3 +355,10 @@ ia64_setup_printk_clock(void)
 	if (!(sal_platform_features & IA64_SAL_PLATFORM_FEATURE_ITC_DRIFT))
 		ia64_printk_clock = ia64_itc_printk_clock;
 }
+
+struct clocksource fsyscall_clock __attribute__((aligned(L1_CACHE_BYTES)));
+
+void update_vsyscall(struct timespec *wall, struct clocksource *c)
+{
+	fsyscall_clock = *c;
+}
diff --git a/arch/ia64/sn/kernel/sn2/timer.c b/arch/ia64/sn/kernel/sn2/timer.c
index 56a88b6..f6a160b 100644
--- a/arch/ia64/sn/kernel/sn2/timer.c
+++ b/arch/ia64/sn/kernel/sn2/timer.c
@@ -11,6 +11,7 @@ #include <linux/kernel.h>
 #include <linux/sched.h>
 #include <linux/time.h>
 #include <linux/interrupt.h>
+#include <linux/clocksource.h>
 
 #include <asm/hw_irq.h>
 #include <asm/system.h>
@@ -22,11 +23,21 @@ #include <asm/sn/clksupport.h>
 
 extern unsigned long sn_rtc_cycles_per_second;
 
-static struct time_interpolator sn2_interpolator = {
-	.drift = -1,
-	.shift = 10,
-	.mask = (1LL << 55) - 1,
-	.source = TIME_SOURCE_MMIO64
+static void __iomem *sn2_mc_ptr;
+
+static cycle_t read_sn2(void)
+{
+	return (cycle_t)readq(sn2_mc_ptr);
+}
+
+static struct clocksource clocksource_sn2 = {
+        .name           = "sn2_rtc",
+        .rating         = 300,
+        .read           = read_sn2,
+        .mask           = (1LL << 55) - 1,
+        .mult           = 0,
+        .shift          = 10,
+        .is_continuous  = 1,
 };
 
 /*
@@ -47,9 +58,10 @@ ia64_sn_udelay (unsigned long usecs)
 
 void __init sn_timer_init(void)
 {
-	sn2_interpolator.frequency = sn_rtc_cycles_per_second;
-	sn2_interpolator.addr = RTC_COUNTER_ADDR;
-	register_time_interpolator(&sn2_interpolator);
+	clocksource_sn2.fsys_mmio_ptr = sn2_mc_ptr = RTC_COUNTER_ADDR;
+	clocksource_sn2.mult = clocksource_hz2mult(sn_rtc_cycles_per_second,
+							clocksource_sn2.shift);
+	clocksource_register(&clocksource_sn2);
 
 	ia64_udelay = &ia64_sn_udelay;
 }
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 6077300..35ad71f 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -480,10 +480,12 @@ #endif
 		/* Get end time (ticks) */
 		t2 = inl(acpi_gbl_FADT.xpm_timer_block.address);
 
+#ifndef CONFIG_IA64
 #ifdef CONFIG_GENERIC_TIME
 		/* TSC halts in C2, so notify users */
 		mark_tsc_unstable();
 #endif
+#endif
 		/* Re-enable interrupts */
 		local_irq_enable();
 		current_thread_info()->status |= TS_POLLING;
@@ -522,10 +524,12 @@ #endif
 			acpi_set_register(ACPI_BITREG_ARB_DISABLE, 0);
 		}
 
+#ifndef CONFIG_IA64
 #ifdef CONFIG_GENERIC_TIME
 		/* TSC halts in C3, so notify users */
 		mark_tsc_unstable();
 #endif
+#endif
 		/* Re-enable interrupts */
 		local_irq_enable();
 		current_thread_info()->status |= TS_POLLING;
diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index 0be700f..5ea7d3e 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -29,6 +29,7 @@ #include <linux/wait.h>
 #include <linux/bcd.h>
 #include <linux/seq_file.h>
 #include <linux/bitops.h>
+#include <linux/clocksource.h>
 
 #include <asm/current.h>
 #include <asm/uaccess.h>
@@ -51,8 +52,34 @@ #define	HPET_DRIFT	(500)
 
 #define HPET_RANGE_SIZE		1024	/* from HPET spec */
 
+#if BITS_PER_LONG == 64
+#define	write_counter(V, MC)	writeq(V, MC)
+#define	read_counter(MC)	readq(MC)
+#else
+#define	write_counter(V, MC) 	writel(V, MC)
+#define	read_counter(MC)	readl(MC)
+#endif
+
 static u32 hpet_nhpet, hpet_max_freq = HPET_USER_FREQ;
 
+static void __iomem *hpet_mc_ptr;
+
+static cycle_t read_hpet(void)
+{
+	return (cycle_t)read_counter((void __iomem *)hpet_mc_ptr);
+}
+
+static struct clocksource clocksource_hpet = {
+        .name           = "hpet",
+        .rating         = 300,
+        .read           = read_hpet,
+        .mask           = 0xffffffffffffffffLL,
+        .mult           = 0, /*to be caluclated*/
+        .shift          = 10,
+        .is_continuous  = 1,
+};
+static struct clocksource *hpet_clocksource_p;
+
 /* A lock for concurrent access by app and isr hpet activity. */
 static DEFINE_SPINLOCK(hpet_lock);
 /* A lock for concurrent intermodule access to hpet and isr hpet activity. */
@@ -79,7 +106,7 @@ struct hpets {
 	struct hpets *hp_next;
 	struct hpet __iomem *hp_hpet;
 	unsigned long hp_hpet_phys;
-	struct time_interpolator *hp_interpolator;
+	struct clocksource *hp_clocksource;
 	unsigned long long hp_tick_freq;
 	unsigned long hp_delta;
 	unsigned int hp_ntimer;
@@ -94,13 +121,6 @@ #define	HPET_IE			0x0002	/* interrupt en
 #define	HPET_PERIODIC		0x0004
 #define	HPET_SHARED_IRQ		0x0008
 
-#if BITS_PER_LONG == 64
-#define	write_counter(V, MC)	writeq(V, MC)
-#define	read_counter(MC)	readq(MC)
-#else
-#define	write_counter(V, MC) 	writel(V, MC)
-#define	read_counter(MC)	readl(MC)
-#endif
 
 #ifndef readq
 static inline unsigned long long readq(void __iomem *addr)
@@ -737,27 +757,6 @@ static ctl_table dev_root[] = {
 
 static struct ctl_table_header *sysctl_header;
 
-static void hpet_register_interpolator(struct hpets *hpetp)
-{
-#ifdef	CONFIG_TIME_INTERPOLATION
-	struct time_interpolator *ti;
-
-	ti = kzalloc(sizeof(*ti), GFP_KERNEL);
-	if (!ti)
-		return;
-
-	ti->source = TIME_SOURCE_MMIO64;
-	ti->shift = 10;
-	ti->addr = &hpetp->hp_hpet->hpet_mc;
-	ti->frequency = hpetp->hp_tick_freq;
-	ti->drift = HPET_DRIFT;
-	ti->mask = -1;
-
-	hpetp->hp_interpolator = ti;
-	register_time_interpolator(ti);
-#endif
-}
-
 /*
  * Adjustment for when arming the timer with
  * initial conditions.  That is, main counter
@@ -909,7 +908,14 @@ int hpet_alloc(struct hpet_data *hdp)
 	}
 
 	hpetp->hp_delta = hpet_calibrate(hpetp);
-	hpet_register_interpolator(hpetp);
+
+	if (!hpet_clocksource_p) {
+        	clocksource_hpet.fsys_mmio_ptr = hpet_mc_ptr = &hpetp->hp_hpet->hpet_mc;
+        	clocksource_hpet.mult = clocksource_hz2mult(hpetp->hp_tick_freq,
+						clocksource_hpet.shift);
+        	clocksource_register(&clocksource_hpet);
+		hpet_clocksource_p = hpetp->hp_clocksource = &clocksource_hpet;
+	}
 
 	return 0;
 }
@@ -995,7 +1001,7 @@ static int hpet_acpi_add(struct acpi_dev
 
 static int hpet_acpi_remove(struct acpi_device *device, int type)
 {
-	/* XXX need to unregister interpolator, dealloc mem, etc */
+	/* XXX need to unregister clocksource, dealloc mem, etc */
 	return -EINVAL;
 }
 
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index daa4940..a20b4d6 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -61,6 +61,9 @@ struct clocksource {
 	u32 shift;
 	unsigned long flags;
 	cycle_t (*vread)(void);
+#ifdef CONFIG_IA64
+	void *fsys_mmio_ptr;	/* used by fsyscall asm code */
+#endif
 
 	/* timekeeping specific data, ignore */
 	cycle_t cycle_last, cycle_interval;

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 2/3] ia64: remove interpolater code
  2007-04-26 20:26 [PATCH 1/3] ia64: convert to use clocksource code Peter Keilty
@ 2007-04-26 20:27 ` Peter Keilty
  2007-04-26 20:52   ` john stultz
  2007-04-26 20:27 ` [PATCH 3/3] ia64: update fsyscall for performance, enable build/run on 2.6.21-rc1 Peter Keilty
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 21+ messages in thread
From: Peter Keilty @ 2007-04-26 20:27 UTC (permalink / raw)
  To: linux-ia64; +Cc: linux-kernel, John Stultz, Peter Keilty, Eric Whitney

From: Peter Keilty <peter.keilty@hp.com>

Remove time_interpolator code.

Signed-off-by: Peter Keilty <peter.keilty@hp.com>
Signed-off-by: John Stultz <johnstul@us.ibm.com>

---

 include/linux/timex.h |   60 ---------------
 kernel/time.c         |    1 
 kernel/timer.c        |  193 --------------------------------------------------
 3 files changed, 254 deletions(-)

linux-2.6.21-rc1_timeofday-remove-interp_C7.patch
============================================
diff --git a/include/linux/timex.h b/include/linux/timex.h
index da929db..37ac3ff 100644
--- a/include/linux/timex.h
+++ b/include/linux/timex.h
@@ -224,66 +224,6 @@ #define shift_right(x, s) ({	\
 	__x < 0 ? -(-__x >> __s) : __x >> __s;	\
 })
 
-
-#ifdef CONFIG_TIME_INTERPOLATION
-
-#define TIME_SOURCE_CPU 0
-#define TIME_SOURCE_MMIO64 1
-#define TIME_SOURCE_MMIO32 2
-#define TIME_SOURCE_FUNCTION 3
-
-/* For proper operations time_interpolator clocks must run slightly slower
- * than the standard clock since the interpolator may only correct by having
- * time jump forward during a tick. A slower clock is usually a side effect
- * of the integer divide of the nanoseconds in a second by the frequency.
- * The accuracy of the division can be increased by specifying a shift.
- * However, this may cause the clock not to be slow enough.
- * The interpolator will self-tune the clock by slowing down if no
- * resets occur or speeding up if the time jumps per analysis cycle
- * become too high.
- *
- * Setting jitter compensates for a fluctuating timesource by comparing
- * to the last value read from the timesource to insure that an earlier value
- * is not returned by a later call. The price to pay
- * for the compensation is that the timer routines are not as scalable anymore.
- */
-
-struct time_interpolator {
-	u16 source;			/* time source flags */
-	u8 shift;			/* increases accuracy of multiply by shifting. */
-				/* Note that bits may be lost if shift is set too high */
-	u8 jitter;			/* if set compensate for fluctuations */
-	u32 nsec_per_cyc;		/* set by register_time_interpolator() */
-	void *addr;			/* address of counter or function */
-	cycles_t mask;			/* mask the valid bits of the counter */
-	unsigned long offset;		/* nsec offset at last update of interpolator */
-	u64 last_counter;		/* counter value in units of the counter at last update */
-	cycles_t last_cycle;		/* Last timer value if TIME_SOURCE_JITTER is set */
-	u64 frequency;			/* frequency in counts/second */
-	long drift;			/* drift in parts-per-million (or -1) */
-	unsigned long skips;		/* skips forward */
-	unsigned long ns_skipped;	/* nanoseconds skipped */
-	struct time_interpolator *next;
-};
-
-extern void register_time_interpolator(struct time_interpolator *);
-extern void unregister_time_interpolator(struct time_interpolator *);
-extern void time_interpolator_reset(void);
-extern unsigned long time_interpolator_get_offset(void);
-extern void time_interpolator_update(long delta_nsec);
-
-#else /* !CONFIG_TIME_INTERPOLATION */
-
-static inline void time_interpolator_reset(void)
-{
-}
-
-static inline void time_interpolator_update(long delta_nsec)
-{
-}
-
-#endif /* !CONFIG_TIME_INTERPOLATION */
-
 #define TICK_LENGTH_SHIFT	32
 
 #ifdef CONFIG_NO_HZ
diff --git a/kernel/time.c b/kernel/time.c
index c6c80ea..53b73fd 100644
--- a/kernel/time.c
+++ b/kernel/time.c
@@ -134,7 +134,6 @@ static inline void warp_clock(void)
 	write_seqlock_irq(&xtime_lock);
 	wall_to_monotonic.tv_sec -= sys_tz.tz_minuteswest * 60;
 	xtime.tv_sec += sys_tz.tz_minuteswest * 60;
-	time_interpolator_reset();
 	write_sequnlock_irq(&xtime_lock);
 	clock_was_set();
 }
diff --git a/kernel/timer.c b/kernel/timer.c
index cb1b86a..f812456 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -751,7 +751,6 @@ struct timespec wall_to_monotonic __attr
 
 EXPORT_SYMBOL(xtime);
 
-
 /* XXX - all of this timekeeping code should be later moved to time.c */
 #include <linux/clocksource.h>
 static struct clocksource *clock; /* pointer to current clocksource */
@@ -1156,10 +1155,6 @@ #endif
 			second_overflow();
 		}
 
-		/* interpolator bits */
-		time_interpolator_update(clock->xtime_interval
-						>> clock->shift);
-
 		/* accumulate error between NTP and clock interval */
 		clock->error += current_tick_length();
 		clock->error -= clock->xtime_interval << (TICK_LENGTH_SHIFT - clock->shift);
@@ -1708,194 +1703,6 @@ void __init init_timers(void)
 	open_softirq(TIMER_SOFTIRQ, run_timer_softirq, NULL);
 }
 
-#ifdef CONFIG_TIME_INTERPOLATION
-
-struct time_interpolator *time_interpolator __read_mostly;
-static struct time_interpolator *time_interpolator_list __read_mostly;
-static DEFINE_SPINLOCK(time_interpolator_lock);
-
-static inline cycles_t time_interpolator_get_cycles(unsigned int src)
-{
-	unsigned long (*x)(void);
-
-	switch (src)
-	{
-		case TIME_SOURCE_FUNCTION:
-			x = time_interpolator->addr;
-			return x();
-
-		case TIME_SOURCE_MMIO64	:
-			return readq_relaxed((void __iomem *)time_interpolator->addr);
-
-		case TIME_SOURCE_MMIO32	:
-			return readl_relaxed((void __iomem *)time_interpolator->addr);
-
-		default: return get_cycles();
-	}
-}
-
-static inline u64 time_interpolator_get_counter(int writelock)
-{
-	unsigned int src = time_interpolator->source;
-
-	if (time_interpolator->jitter)
-	{
-		cycles_t lcycle;
-		cycles_t now;
-
-		do {
-			lcycle = time_interpolator->last_cycle;
-			now = time_interpolator_get_cycles(src);
-			if (lcycle && time_after(lcycle, now))
-				return lcycle;
-
-			/* When holding the xtime write lock, there's no need
-			 * to add the overhead of the cmpxchg.  Readers are
-			 * force to retry until the write lock is released.
-			 */
-			if (writelock) {
-				time_interpolator->last_cycle = now;
-				return now;
-			}
-			/* Keep track of the last timer value returned. The use of cmpxchg here
-			 * will cause contention in an SMP environment.
-			 */
-		} while (unlikely(cmpxchg(&time_interpolator->last_cycle, lcycle, now) != lcycle));
-		return now;
-	}
-	else
-		return time_interpolator_get_cycles(src);
-}
-
-void time_interpolator_reset(void)
-{
-	time_interpolator->offset = 0;
-	time_interpolator->last_counter = time_interpolator_get_counter(1);
-}
-
-#define GET_TI_NSECS(count,i) (((((count) - i->last_counter) & (i)->mask) * (i)->nsec_per_cyc) >> (i)->shift)
-
-unsigned long time_interpolator_get_offset(void)
-{
-	/* If we do not have a time interpolator set up then just return zero */
-	if (!time_interpolator)
-		return 0;
-
-	return time_interpolator->offset +
-		GET_TI_NSECS(time_interpolator_get_counter(0), time_interpolator);
-}
-
-#define INTERPOLATOR_ADJUST 65536
-#define INTERPOLATOR_MAX_SKIP 10*INTERPOLATOR_ADJUST
-
-void time_interpolator_update(long delta_nsec)
-{
-	u64 counter;
-	unsigned long offset;
-
-	/* If there is no time interpolator set up then do nothing */
-	if (!time_interpolator)
-		return;
-
-	/*
-	 * The interpolator compensates for late ticks by accumulating the late
-	 * time in time_interpolator->offset. A tick earlier than expected will
-	 * lead to a reset of the offset and a corresponding jump of the clock
-	 * forward. Again this only works if the interpolator clock is running
-	 * slightly slower than the regular clock and the tuning logic insures
-	 * that.
-	 */
-
-	counter = time_interpolator_get_counter(1);
-	offset = time_interpolator->offset +
-			GET_TI_NSECS(counter, time_interpolator);
-
-	if (delta_nsec < 0 || (unsigned long) delta_nsec < offset)
-		time_interpolator->offset = offset - delta_nsec;
-	else {
-		time_interpolator->skips++;
-		time_interpolator->ns_skipped += delta_nsec - offset;
-		time_interpolator->offset = 0;
-	}
-	time_interpolator->last_counter = counter;
-
-	/* Tuning logic for time interpolator invoked every minute or so.
-	 * Decrease interpolator clock speed if no skips occurred and an offset is carried.
-	 * Increase interpolator clock speed if we skip too much time.
-	 */
-	if (jiffies % INTERPOLATOR_ADJUST == 0)
-	{
-		if (time_interpolator->skips == 0 && time_interpolator->offset > tick_nsec)
-			time_interpolator->nsec_per_cyc--;
-		if (time_interpolator->ns_skipped > INTERPOLATOR_MAX_SKIP && time_interpolator->offset == 0)
-			time_interpolator->nsec_per_cyc++;
-		time_interpolator->skips = 0;
-		time_interpolator->ns_skipped = 0;
-	}
-}
-
-static inline int
-is_better_time_interpolator(struct time_interpolator *new)
-{
-	if (!time_interpolator)
-		return 1;
-	return new->frequency > 2*time_interpolator->frequency ||
-	    (unsigned long)new->drift < (unsigned long)time_interpolator->drift;
-}
-
-void
-register_time_interpolator(struct time_interpolator *ti)
-{
-	unsigned long flags;
-
-	/* Sanity check */
-	BUG_ON(ti->frequency == 0 || ti->mask == 0);
-
-	ti->nsec_per_cyc = ((u64)NSEC_PER_SEC << ti->shift) / ti->frequency;
-	spin_lock(&time_interpolator_lock);
-	write_seqlock_irqsave(&xtime_lock, flags);
-	if (is_better_time_interpolator(ti)) {
-		time_interpolator = ti;
-		time_interpolator_reset();
-	}
-	write_sequnlock_irqrestore(&xtime_lock, flags);
-
-	ti->next = time_interpolator_list;
-	time_interpolator_list = ti;
-	spin_unlock(&time_interpolator_lock);
-}
-
-void
-unregister_time_interpolator(struct time_interpolator *ti)
-{
-	struct time_interpolator *curr, **prev;
-	unsigned long flags;
-
-	spin_lock(&time_interpolator_lock);
-	prev = &time_interpolator_list;
-	for (curr = *prev; curr; curr = curr->next) {
-		if (curr == ti) {
-			*prev = curr->next;
-			break;
-		}
-		prev = &curr->next;
-	}
-
-	write_seqlock_irqsave(&xtime_lock, flags);
-	if (ti == time_interpolator) {
-		/* we lost the best time-interpolator: */
-		time_interpolator = NULL;
-		/* find the next-best interpolator */
-		for (curr = time_interpolator_list; curr; curr = curr->next)
-			if (is_better_time_interpolator(curr))
-				time_interpolator = curr;
-		time_interpolator_reset();
-	}
-	write_sequnlock_irqrestore(&xtime_lock, flags);
-	spin_unlock(&time_interpolator_lock);
-}
-#endif /* CONFIG_TIME_INTERPOLATION */
-
 /**
  * msleep - sleep safely even with waitqueue interruptions
  * @msecs: Time in milliseconds to sleep for

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 3/3] ia64: update fsyscall for performance, enable build/run on 2.6.21-rc1
  2007-04-26 20:26 [PATCH 1/3] ia64: convert to use clocksource code Peter Keilty
  2007-04-26 20:27 ` [PATCH 2/3] ia64: remove interpolater code Peter Keilty
@ 2007-04-26 20:27 ` Peter Keilty
  2007-04-26 20:41 ` [PATCH 1/3] ia64: convert to use clocksource code Sam Ravnborg
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 21+ messages in thread
From: Peter Keilty @ 2007-04-26 20:27 UTC (permalink / raw)
  To: linux-ia64; +Cc: John Stultz, linux-kernel, Eric Whitney, Peter Keilty

From: Peter Keilty <peter.keilty@hp.com>

Update ia64 conversion to the generic timekeeping/clocksource code.
Modified fast syscall path for gettimeofday to get performance equal
to orginal code and handle clocksource change at clock hz time.

Performance measurements for single calls (ITC cycles):

A. 32 way Intel IA64 SMP system 8640 (montecito)
A.1. Current code itc cmpxchg
gettimeofday cycles: 39 37 37 37 37 37 37 37 37 37 
clock_gettime(REAL) cycles: 49 35 35 35 35 35 35 35 35 35
clock_gettime(MONO) cycles: 42 36 36 36 36 36 36 36 36 36

A.2 New code itc cmpxchg
gettimeofday cycles: 39 38 37 37 38 37 38 37 37 37
clock_gettime(REAL) cycles: 53 35 35 36 35 35 35 35 35 35
clock_gettime(MONO) cycles: 44 38 36 36 36 36 36 36 36 36

A.3 New code itc cmpxchg switched off (nojitter kernel option)
gettimeofday cycles: 35 35 36 35 36 35 36 35 35 35
clock_gettime(REAL) cycles: 49 33 33 34 33 33 33 33 33 33
clock_gettime(MONO) cycles: 38 33 33 33 33 33 33 33 33 33

A.4 New code with hpet as clocksource, mmio space read
gettimeofday cycles: 183 183 181 180 182 185 182 183 184 182
clock_gettime(REAL) cycles: 196 180 179 179 183 181 182 183 183 181
clock_gettime(MONO) cycles: 185 180 182 180 182 182 179 179 179 181

Signed-off-by: Peter keilty <peter.keilty@hp.com>

---

 arch/ia64/kernel/asm-offsets.c        |   15 ++++---
 arch/ia64/kernel/cyclone.c            |    2 -
 arch/ia64/kernel/fsys.S               |   64 ++++++++++++++++++----------------
 arch/ia64/kernel/fsyscall_gtod_data.h |   18 +++++++++
 arch/ia64/kernel/time.c               |   37 +++++++++++++------
 arch/ia64/sn/kernel/sn2/timer.c       |    2 -
 drivers/char/hpet.c                   |    2 -
 kernel/time/ntp.c                     |   10 -----
 8 files changed, 90 insertions(+), 60 deletions(-)

Index: Linux/arch/ia64/kernel/asm-offsets.c
===================================================================
--- Linux.orig/arch/ia64/kernel/asm-offsets.c	2007-04-24 11:40:44.000000000 -0400
+++ Linux/arch/ia64/kernel/asm-offsets.c	2007-04-24 12:58:49.000000000 -0400
@@ -16,6 +16,7 @@
 #include <asm-ia64/mca.h>
 
 #include "../kernel/sigframe.h"
+#include "../kernel/fsyscall_gtod_data.h"
 
 #define DEFINE(sym, val) \
         asm volatile("\n->" #sym " %0 " #val : : "i" (val))
@@ -256,10 +257,12 @@ void foo(void)
 	BLANK();
 
 	/* used by fsys_gettimeofday in arch/ia64/kernel/fsys.S */
-	DEFINE(IA64_TIMESPEC_TV_NSEC_OFFSET, offsetof (struct timespec, tv_nsec));
-	DEFINE(IA64_CLOCKSOURCE_MASK_OFFSET, offsetof (struct clocksource, mask));
-	DEFINE(IA64_CLOCKSOURCE_MULT_OFFSET, offsetof (struct clocksource, mult));
-	DEFINE(IA64_CLOCKSOURCE_SHIFT_OFFSET, offsetof (struct clocksource, shift));
-	DEFINE(IA64_CLOCKSOURCE_MMIO_PTR_OFFSET, offsetof (struct clocksource, fsys_mmio_ptr));
-	DEFINE(IA64_CLOCKSOURCE_CYCLE_LAST_OFFSET, offsetof (struct clocksource, cycle_last));
+	DEFINE(IA64_GTOD_LOCK_OFFSET, offsetof (struct fsyscall_gtod_data_t, lock));
+	DEFINE(IA64_CLKSRC_MASK_OFFSET, offsetof (struct fsyscall_gtod_data_t, clk_mask));
+	DEFINE(IA64_CLKSRC_MULT_OFFSET, offsetof (struct fsyscall_gtod_data_t, clk_mult));
+	DEFINE(IA64_CLKSRC_SHIFT_OFFSET, offsetof (struct fsyscall_gtod_data_t, clk_shift));
+	DEFINE(IA64_CLKSRC_MMIO_PTR_OFFSET, offsetof (struct fsyscall_gtod_data_t, clk_fsys_mmio_ptr));
+	DEFINE(IA64_CLKSRC_CYCLE_LAST_OFFSET, offsetof (struct fsyscall_gtod_data_t, clk_cycle_last));
+	DEFINE(IA64_ITC_LASTCYCLE_OFFSET, offsetof (struct fsyscall_gtod_data_t, itc_lastcycle));
+	DEFINE(IA64_ITC_JITTER_OFFSET, offsetof (struct fsyscall_gtod_data_t, itc_jitter));
 }
Index: Linux/arch/ia64/kernel/cyclone.c
===================================================================
--- Linux.orig/arch/ia64/kernel/cyclone.c	2007-04-24 11:40:44.000000000 -0400
+++ Linux/arch/ia64/kernel/cyclone.c	2007-04-24 12:31:20.000000000 -0400
@@ -33,7 +33,7 @@ static struct clocksource clocksource_cy
         .mask           = (1LL << 40) - 1,
         .mult           = 0, /*to be caluclated*/
         .shift          = 16,
-        .is_continuous  = 1,
+        .flags          = CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
 int __init init_cyclone_clock(void)
Index: Linux/arch/ia64/kernel/fsys.S
===================================================================
--- Linux.orig/arch/ia64/kernel/fsys.S	2007-04-24 11:40:44.000000000 -0400
+++ Linux/arch/ia64/kernel/fsys.S	2007-04-25 16:39:25.000000000 -0400
@@ -145,6 +145,9 @@ ENTRY(fsys_set_tid_address)
 	FSYS_RETURN
 END(fsys_set_tid_address)
 
+#if IA64_GTOD_LOCK_OFFSET !=0
+#error fsys_gettimeofday incompatible with changes to struct fsyscall_gtod_data_t
+#endif
 #define CLOCK_REALTIME 0
 #define CLOCK_MONOTONIC 1
 #define CLOCK_DIVIDE_BY_1000 0x4000
@@ -175,10 +178,10 @@ ENTRY(fsys_gettimeofday)
 	// r16 = preserved: current task pointer
 	// r17 = wall to monotonic use
 	// r19 = address of itc_lastcycle
-	// r20 = struct clocksource / address of first element
-	// r21 = shift value
-	// r22 = address of itc_jitter/ wall_to_monotonic
-	// r23 = address of shift
+	// r20 = struct fsyscall_gtod_data  / address of first element
+	// r21 = address of mmio_ptr
+	// r22 = address of  wall_to_monotonic
+	// r23 = address of shift/ value
 	// r24 = address mult factor / cycle_last value
 	// r25 = itc_lastcycle value
 	// r26 = address clocksource cycle_last
@@ -204,46 +207,47 @@ ENTRY(fsys_gettimeofday)
 	tnat.nz p6,p0 = r31	// branch deferred since it does not fit into bundle structure
 	mov pr = r30,0xc000	// Set predicates according to function
 	add r2 = TI_FLAGS+IA64_TASK_SIZE,r16
-	movl r20 = fsyscall_clock // load fsyscall clocksource address
+	movl r20 = fsyscall_gtod_data // load fsyscall gettimeofday data address
 	;;
-	add r10 = IA64_CLOCKSOURCE_MMIO_PTR_OFFSET,r20
-	movl r29 = xtime_lock
-	ld4 r2 = [r2]		// process work pending flags
+	add r29 = IA64_ITC_JITTER_OFFSET,r20
 	movl r27 = xtime
-	;;	// only one bundle here
-	add r14 = IA64_CLOCKSOURCE_MASK_OFFSET,r20
-	movl r22 = itc_jitter
-	add r24 = IA64_CLOCKSOURCE_MULT_OFFSET,r20
+	ld4 r2 = [r2]		// process work pending flags
+(p15)	movl r22 = wall_to_monotonic
+	;;
+	add r21 = IA64_CLKSRC_MMIO_PTR_OFFSET,r20
+	add r19 = IA64_ITC_LASTCYCLE_OFFSET,r20
 	and r2 = TIF_ALLWORK_MASK,r2
 (p6)    br.cond.spnt.few .fail_einval	// deferred branch
 	;;
-	ld8 r30 = [r10]		// clocksource->mmio_ptr
-	movl r19 = itc_lastcycle
-	add r23 = IA64_CLOCKSOURCE_SHIFT_OFFSET,r20
+	add r26 = IA64_CLKSRC_CYCLE_LAST_OFFSET,r20 // clksrc_cycle_last
 	cmp.ne p6, p0 = 0, r2	// Fallback if work is scheduled
 (p6)    br.cond.spnt.many fsys_fallback_syscall
+	;; // get lock.seq here new code, outer loop2!
+.time_redo:
+	ld4.acq r28 = [r20]	// gtod_lock.sequence, Must be first in struct
+	ld8 r30 = [r21]		// clocksource->mmio_ptr
+	add r24 = IA64_CLKSRC_MULT_OFFSET,r20
+	ld4 r2 = [r29]		// itc_jitter value
+	add r23 = IA64_CLKSRC_SHIFT_OFFSET,r20
+	add r14 = IA64_CLKSRC_MASK_OFFSET,r20
 	;;
+	ld4 r3 = [r24]		// clocksource mult value
 	ld8 r14 = [r14]         // clocksource mask value
-	ld4 r2 = [r22]		// itc_jitter value
-	add r26 = IA64_CLOCKSOURCE_CYCLE_LAST_OFFSET,r20 // clock fsyscall_cycle_last
-	ld4 r3 = [r24]		// clocksource->mult value
 	cmp.eq p8,p9 = 0,r30	// Check for cpu timer, no mmio_ptr, set p8, clear p9
 	;;
 	setf.sig f7 = r3	// Setup for mult scaling of counter
-(p15)	movl r22 = wall_to_monotonic
-	ld4 r21 = [r23]		// shift value
-(p8)	cmp.ne p13,p0 = r2,r0	// need jitter compensation, set p13
+(p8)	cmp.ne p13,p0 = r2,r0	// need itc_jitter compensation, set p13
+	ld4 r23 = [r23]		// clocksource shift value
+	ld8 r24 = [r26]		// get clksrc_cycle_last value
 (p9)	cmp.eq p13,p0 = 0,r30	// if mmio_ptr, clear p13 jitter control
-	;;
-.time_redo:
-	.pred.rel.mutex p8,p9,p10
-	ld4.acq r28 = [r29]	// xtime_lock.sequence. Must come first for locking purposes
+	;; // old position for lock seq, new inner loop1!
+.cmpxchg_redo:
+	.pred.rel.mutex p8,p9
 (p8)	mov r2 = ar.itc		// CPU_TIMER. 36 clocks latency!!!
 (p9)	ld8 r2 = [r30]		// readq(ti->address). Could also have latency issues..
 (p13)	ld8 r25 = [r19]		// get itc_lastcycle value
 	;;			// could be removed by moving the last add upward
  	ld8 r9 = [r27],IA64_TIMESPEC_TV_NSEC_OFFSET
-	ld8 r24 = [r26]		// get fsyscall_cycle_last value
 (p15)	ld8 r17 = [r22],IA64_TIMESPEC_TV_NSEC_OFFSET
 	;;
 	ld8 r8 = [r27],-IA64_TIMESPEC_TV_NSEC_OFFSET	// xtime.tv_nsec
@@ -265,21 +269,23 @@ EX(.fail_efault, probe.w.fault r31, 3)	/
 	xmpy.l f8 = f8,f7	// nsec_per_cyc*(counter-last_counter)
 (p15)	add r9 = r9,r17		// Add wall to monotonic.secs to result secs
 	;;
+	// End cmpxchg critical section loop1
 (p15)	ld8 r17 = [r22],-IA64_TIMESPEC_TV_NSEC_OFFSET
 (p7)	cmp.ne p7,p0 = r25,r3	// if cmpxchg not successful redo
+(p7)	br.cond.dpnt.few .cmpxchg_redo	// inner loop1
 	// simulate tbit.nz.or p7,p0 = r28,0
 	and r28 = ~1,r28	// Make sequence even to force retry if odd
 	getf.sig r2 = f8
 	mf
 	;;
-	ld4 r10 = [r29]		// xtime_lock.sequence
+	ld4 r10 = [r20]		// gtod_lock.sequence, old xtime_lock.sequence
 (p15)	add r8 = r8, r17	// Add monotonic.nsecs to nsecs
-	shr.u r2 = r2,r21	// shift by factor
+	shr.u r2 = r2,r23	// shift by factor
 	;;		// overloaded 3 bundles!
 	// End critical section.
 	add r8 = r8,r2		// Add xtime.nsecs
 	cmp4.ne.or p7,p0 = r28,r10
-(p7)	br.cond.dpnt.few .time_redo	// sequence number changed ?
+(p7)	br.cond.dpnt.few .time_redo	// sequence number changed, outer loop2
 	// Now r8=tv->tv_nsec and r9=tv->tv_sec
 	mov r10 = r0
 	movl r2 = 1000000000
Index: Linux/arch/ia64/kernel/fsyscall_gtod_data.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ Linux/arch/ia64/kernel/fsyscall_gtod_data.h	2007-04-24 12:54:52.000000000 -0400
@@ -0,0 +1,18 @@
+/*
+ * (c) Copyright 2007 Hewlett-Packard Development Company, L.P.
+ *        Contributed by Peter Keilty <peter.keilty@hp.com>
+ *
+ * fsyscall gettimeofday data
+ */
+
+struct fsyscall_gtod_data_t {
+        seqlock_t lock;
+        cycle_t   clk_mask;
+        u32       clk_mult;
+        u32       clk_shift;
+        void     *clk_fsys_mmio_ptr;
+        cycle_t   clk_cycle_last;
+        cycle_t   itc_lastcycle;
+        int       itc_jitter;
+} __attribute__ ((aligned (L1_CACHE_BYTES)));
+
Index: Linux/arch/ia64/kernel/time.c
===================================================================
--- Linux.orig/arch/ia64/kernel/time.c	2007-04-24 11:40:44.000000000 -0400
+++ Linux/arch/ia64/kernel/time.c	2007-04-24 12:41:28.000000000 -0400
@@ -30,9 +30,13 @@
 #include <asm/sections.h>
 #include <asm/system.h>
 
+#include "fsyscall_gtod_data.h"
+
 static cycle_t itc_get_cycles(void);
-cycle_t	itc_lastcycle __attribute__((aligned(L1_CACHE_BYTES)));
-int	itc_jitter __attribute__((aligned(L1_CACHE_BYTES)));
+
+struct fsyscall_gtod_data_t fsyscall_gtod_data = {
+	.lock = SEQLOCK_UNLOCKED,
+};
 
 volatile int time_keeper_id = 0; /* smp_processor_id() of time-keeper */
 
@@ -50,7 +54,7 @@ static struct clocksource clocksource_it
         .mask           = 0xffffffffffffffffLL,
         .mult           = 0, /*to be caluclated*/
         .shift          = 16,
-        .is_continuous  = 1,
+        .flags          = CLOCK_SOURCE_IS_CONTINUOUS,
 };
 static struct clocksource *clocksource_itc_p;
 
@@ -232,7 +236,7 @@ ia64_init_itm (void)
 		 * even going backward) if the ITC offsets between the individual CPUs
 		 * are too large.
 		 */
-		if (!nojitter) itc_jitter = 1;
+		if (!nojitter) fsyscall_gtod_data.itc_jitter = 1;
 #endif
 	}
 
@@ -248,15 +252,14 @@ ia64_init_itm (void)
 	}
 }
 
-
 static cycle_t itc_get_cycles()
 {
-	if (itc_jitter) {
+	if (fsyscall_gtod_data.itc_jitter) {
 		u64 lcycle;
 		u64 now;
 
 		do {
-			lcycle = itc_lastcycle;
+			lcycle = fsyscall_gtod_data.itc_lastcycle;
 			now = get_cycles();
 			if (lcycle && time_after(lcycle, now))
 				return lcycle;
@@ -266,14 +269,14 @@ static cycle_t itc_get_cycles()
 			 * force to retry until the write lock is released.
 			 */
 			if (spin_is_locked(&xtime_lock.lock)) {
-				itc_lastcycle = now;
+				fsyscall_gtod_data.itc_lastcycle = now;
 				return now;
 			}
 			/* Keep track of the last timer value returned.
 			 * The use of cmpxchg here will cause contention in
 			 * an SMP environment.
 			 */
-		} while (unlikely(cmpxchg(&itc_lastcycle, lcycle, now) != lcycle));
+		} while (likely(cmpxchg(&fsyscall_gtod_data.itc_lastcycle, lcycle, now) != lcycle));
 		return now;
 	} else
 		return get_cycles();
@@ -356,9 +359,19 @@ ia64_setup_printk_clock(void)
 		ia64_printk_clock = ia64_itc_printk_clock;
 }
 
-struct clocksource fsyscall_clock __attribute__((aligned(L1_CACHE_BYTES)));
-
 void update_vsyscall(struct timespec *wall, struct clocksource *c)
 {
-	fsyscall_clock = *c;
+        unsigned long flags;
+
+        write_seqlock_irqsave(&fsyscall_gtod_data.lock, flags);
+
+        /* copy fsyscall clock data */
+        fsyscall_gtod_data.clk_mask = c->mask;
+        fsyscall_gtod_data.clk_mult = c->mult;
+        fsyscall_gtod_data.clk_shift = c->shift;
+        fsyscall_gtod_data.clk_fsys_mmio_ptr = c->fsys_mmio_ptr;
+        fsyscall_gtod_data.clk_cycle_last = c->cycle_last;
+
+        write_sequnlock_irqrestore(&fsyscall_gtod_data.lock, flags);
 }
+
Index: Linux/arch/ia64/sn/kernel/sn2/timer.c
===================================================================
--- Linux.orig/arch/ia64/sn/kernel/sn2/timer.c	2007-04-24 11:40:44.000000000 -0400
+++ Linux/arch/ia64/sn/kernel/sn2/timer.c	2007-04-24 12:32:33.000000000 -0400
@@ -37,7 +37,7 @@ static struct clocksource clocksource_sn
         .mask           = (1LL << 55) - 1,
         .mult           = 0,
         .shift          = 10,
-        .is_continuous  = 1,
+        .flags          = CLOCK_SOURCE_IS_CONTINUOUS,
 };
 
 /*
Index: Linux/drivers/char/hpet.c
===================================================================
--- Linux.orig/drivers/char/hpet.c	2007-04-24 11:40:44.000000000 -0400
+++ Linux/drivers/char/hpet.c	2007-04-26 09:36:31.000000000 -0400
@@ -76,7 +76,7 @@ static struct clocksource clocksource_hp
         .mask           = 0xffffffffffffffffLL,
         .mult           = 0, /*to be caluclated*/
         .shift          = 10,
-        .is_continuous  = 1,
+        .flags          = CLOCK_SOURCE_IS_CONTINUOUS,
 };
 static struct clocksource *hpet_clocksource_p;
 
Index: Linux/kernel/time/ntp.c
===================================================================
--- Linux.orig/kernel/time/ntp.c	2007-04-24 11:40:44.000000000 -0400
+++ Linux/kernel/time/ntp.c	2007-04-24 12:33:24.000000000 -0400
@@ -114,11 +114,6 @@ void second_overflow(void)
 		if (xtime.tv_sec % 86400 == 0) {
 			xtime.tv_sec--;
 			wall_to_monotonic.tv_sec++;
-			/*
-			 * The timer interpolator will make time change
-			 * gradually instead of an immediate jump by one second
-			 */
-			time_interpolator_update(-NSEC_PER_SEC);
 			time_state = TIME_OOP;
 			clock_was_set();
 			printk(KERN_NOTICE "Clock: inserting leap second "
@@ -129,11 +124,6 @@ void second_overflow(void)
 		if ((xtime.tv_sec + 1) % 86400 == 0) {
 			xtime.tv_sec++;
 			wall_to_monotonic.tv_sec--;
-			/*
-			 * Use of time interpolator for a gradual change of
-			 * time
-			 */
-			time_interpolator_update(NSEC_PER_SEC);
 			time_state = TIME_WAIT;
 			clock_was_set();
 			printk(KERN_NOTICE "Clock: deleting leap second "

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/3] ia64: convert to use clocksource code
  2007-04-26 20:26 [PATCH 1/3] ia64: convert to use clocksource code Peter Keilty
  2007-04-26 20:27 ` [PATCH 2/3] ia64: remove interpolater code Peter Keilty
  2007-04-26 20:27 ` [PATCH 3/3] ia64: update fsyscall for performance, enable build/run on 2.6.21-rc1 Peter Keilty
@ 2007-04-26 20:41 ` Sam Ravnborg
  2007-04-26 20:48   ` john stultz
  2007-04-26 21:07 ` Daniel Walker
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 21+ messages in thread
From: Sam Ravnborg @ 2007-04-26 20:41 UTC (permalink / raw)
  To: Peter Keilty; +Cc: linux-ia64, John Stultz, Eric Whitney, linux-kernel

On Thu, Apr 26, 2007 at 04:26:32PM -0400, Peter Keilty wrote:
> From: Peter Keilty <peter.keilty@hp.com>
> 
> Initial ia64 conversion to the generic timekeeping/clocksource code.
> 
> Signed-off-by: Peter Keilty <peter.keilty@hp.com>
> Signed-off-by: John Stultz <johnstul@us.ibm.com>

The "Signed-off-by:" should reflect the order in which a path is processed
with the last submitter at the bottom of the list.
So if this patch came from John then he should be first in the list
and since the patch passes you and you submit it you should be in the bottom
of the list.

And "Signed-off-by:" tell the path that the patch takes. It is not to be used
to let others say "I have seen it and I think the patch is ok".
For the latter we have "Acked-by:".

I did not look at the code so ne feedback there.

	Sam

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/3] ia64: convert to use clocksource code
  2007-04-26 20:41 ` [PATCH 1/3] ia64: convert to use clocksource code Sam Ravnborg
@ 2007-04-26 20:48   ` john stultz
  0 siblings, 0 replies; 21+ messages in thread
From: john stultz @ 2007-04-26 20:48 UTC (permalink / raw)
  To: Sam Ravnborg; +Cc: Peter Keilty, linux-ia64, Eric Whitney, linux-kernel

On Thu, 2007-04-26 at 22:41 +0200, Sam Ravnborg wrote:
> On Thu, Apr 26, 2007 at 04:26:32PM -0400, Peter Keilty wrote:
> > From: Peter Keilty <peter.keilty@hp.com>
> > 
> > Initial ia64 conversion to the generic timekeeping/clocksource code.
> > 
> > Signed-off-by: Peter Keilty <peter.keilty@hp.com>
> > Signed-off-by: John Stultz <johnstul@us.ibm.com>
> 
> The "Signed-off-by:" should reflect the order in which a path is processed
> with the last submitter at the bottom of the list.
> So if this patch came from John then he should be first in the list
> and since the patch passes you and you submit it you should be in the bottom
> of the list.
> 
> And "Signed-off-by:" tell the path that the patch takes. It is not to be used
> to let others say "I have seen it and I think the patch is ok".
> For the latter we have "Acked-by:".

Yea. Its a little odd in this case, because Peter sent me the signed off
code, then I've made tweaks to it and signed it off, then peter picked
that up and has made further improvements.

So I don't think it is inaccurate, but I see how it could be confusing.
Maybe Peter should add an extra signed-off so its more clear?

thanks
-john





^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/3] ia64: remove interpolater code
  2007-04-26 20:27 ` [PATCH 2/3] ia64: remove interpolater code Peter Keilty
@ 2007-04-26 20:52   ` john stultz
  2007-04-26 21:47     ` David Miller
  0 siblings, 1 reply; 21+ messages in thread
From: john stultz @ 2007-04-26 20:52 UTC (permalink / raw)
  To: Peter Keilty; +Cc: linux-ia64, linux-kernel, Eric Whitney, davem

On Thu, 2007-04-26 at 16:27 -0400, Peter Keilty wrote:
> From: Peter Keilty <peter.keilty@hp.com>
> 
> Remove time_interpolator code.
> 
> Signed-off-by: Peter Keilty <peter.keilty@hp.com>
> Signed-off-by: John Stultz <johnstul@us.ibm.com>
> 
> ---
> 
>  include/linux/timex.h |   60 ---------------
>  kernel/time.c         |    1 
>  kernel/timer.c        |  193 --------------------------------------------------
>  3 files changed, 254 deletions(-)

This specific patch may need to be held off until Dave Miller pushes his
SPARC64 GENERIC_TIME conversion (as SPARC64 currently uses the
time_interpolator code). 

I think Dave intends to push it soon, so it shouldn't be a long wait.

thanks
-john



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/3] ia64: convert to use clocksource code
  2007-04-26 20:26 [PATCH 1/3] ia64: convert to use clocksource code Peter Keilty
                   ` (2 preceding siblings ...)
  2007-04-26 20:41 ` [PATCH 1/3] ia64: convert to use clocksource code Sam Ravnborg
@ 2007-04-26 21:07 ` Daniel Walker
  2007-04-27 14:38   ` Peter Keilty
  2007-04-26 21:18 ` john stultz
  2007-04-27  1:02 ` Chris Wright
  5 siblings, 1 reply; 21+ messages in thread
From: Daniel Walker @ 2007-04-26 21:07 UTC (permalink / raw)
  To: Peter Keilty; +Cc: linux-ia64, John Stultz, Eric Whitney, linux-kernel

On Thu, 2007-04-26 at 16:26 -0400, Peter Keilty wrote:

> +        .mask           = (1LL << 40) - 1,
> +        .mult           = 0, /*to be caluclated*/
> +        .shift          = 16,
> +        .is_continuous  = 1,
>  };

You should use CLOCKSOURCE_MASK() here ..


>  
> diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
> index daa4940..a20b4d6 100644
> --- a/include/linux/clocksource.h
> +++ b/include/linux/clocksource.h
> @@ -61,6 +61,9 @@ struct clocksource {
>  	u32 shift;
>  	unsigned long flags;
>  	cycle_t (*vread)(void);
> +#ifdef CONFIG_IA64
> +	void *fsys_mmio_ptr;	/* used by fsyscall asm code */
> +#endif

Could you explain in detail why this is needed?

Daniel


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/3] ia64: convert to use clocksource code
  2007-04-26 20:26 [PATCH 1/3] ia64: convert to use clocksource code Peter Keilty
                   ` (3 preceding siblings ...)
  2007-04-26 21:07 ` Daniel Walker
@ 2007-04-26 21:18 ` john stultz
  2007-04-27 12:54   ` Peter Keilty
  2007-04-27  1:02 ` Chris Wright
  5 siblings, 1 reply; 21+ messages in thread
From: john stultz @ 2007-04-26 21:18 UTC (permalink / raw)
  To: Peter Keilty; +Cc: linux-ia64, Eric Whitney, linux-kernel

On Thu, 2007-04-26 at 16:26 -0400, Peter Keilty wrote:
> From: Peter Keilty <peter.keilty@hp.com>
> 
> Initial ia64 conversion to the generic timekeeping/clocksource code.
> 
> Signed-off-by: Peter Keilty <peter.keilty@hp.com>
> Signed-off-by: John Stultz <johnstul@us.ibm.com>

Peter,
	Thanks so much for pushing this on! I suspect this patch needs to be
updated a touch, as I'm not sure if it still compiles in light of recent
changes...


> diff --git a/arch/ia64/kernel/cyclone.c b/arch/ia64/kernel/cyclone.c
> index e00b215..280383b 100644
> --- a/arch/ia64/kernel/cyclone.c
> +++ b/arch/ia64/kernel/cyclone.c
> @@ -3,6 +3,7 @@ #include <linux/smp.h>
>  #include <linux/time.h>
>  #include <linux/errno.h>
>  #include <linux/timex.h>
> +#include <linux/clocksource.h>
>  #include <asm/io.h>
> 
>  /* IBM Summit (EXA) Cyclone counter code*/
> @@ -18,13 +19,21 @@ void __init cyclone_setup(void)
>  	use_cyclone = 1;
>  }
> 
> +static void __iomem *cyclone_mc_ptr;
> 
> -struct time_interpolator cyclone_interpolator = {
> -	.source =	TIME_SOURCE_MMIO64,
> -	.shift =	16,
> -	.frequency =	CYCLONE_TIMER_FREQ,
> -	.drift =	-100,
> -	.mask =		(1LL << 40) - 1
> +static cycle_t read_cyclone(void)
> +{
> +	return (cycle_t)readq((void __iomem *)cyclone_mc_ptr);
> +}
> +
> +static struct clocksource clocksource_cyclone = {
> +        .name           = "cyclone",
> +        .rating         = 300,
> +        .read           = read_cyclone,
> +        .mask           = (1LL << 40) - 1,

Daniel Walker pointed out to me on IRC that CLOCKSOURCE_MASK() should
probably be used here.

> +        .mult           = 0, /*to be caluclated*/
> +        .shift          = 16,
> +        .is_continuous  = 1,

.is_continuous no longer exists. 

You want to use:
.flags = CLOCK_SOURCE_IS_CONTINUOUS 

This holds for all the clocksources introduced.


thanks
-john



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/3] ia64: remove interpolater code
  2007-04-26 20:52   ` john stultz
@ 2007-04-26 21:47     ` David Miller
  0 siblings, 0 replies; 21+ messages in thread
From: David Miller @ 2007-04-26 21:47 UTC (permalink / raw)
  To: johnstul; +Cc: peter.keilty, linux-ia64, linux-kernel, eric.whitney

From: john stultz <johnstul@us.ibm.com>
Date: Thu, 26 Apr 2007 13:52:47 -0700

> This specific patch may need to be held off until Dave Miller pushes his
> SPARC64 GENERIC_TIME conversion (as SPARC64 currently uses the
> time_interpolator code). 
> 
> I think Dave intends to push it soon, so it shouldn't be a long wait.

Feel free to break the tree meanwhile in order to delete this,
I won't mind as I'll have it fixed as soon as I merge my
sparc tree to Linus.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/3] ia64: convert to use clocksource code
  2007-04-26 20:26 [PATCH 1/3] ia64: convert to use clocksource code Peter Keilty
                   ` (4 preceding siblings ...)
  2007-04-26 21:18 ` john stultz
@ 2007-04-27  1:02 ` Chris Wright
  2007-04-27 13:35   ` Peter Keilty
  5 siblings, 1 reply; 21+ messages in thread
From: Chris Wright @ 2007-04-27  1:02 UTC (permalink / raw)
  To: Peter Keilty; +Cc: linux-ia64, John Stultz, Eric Whitney, linux-kernel

* Peter Keilty (peter.keilty@hp.com) wrote:
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index 6077300..35ad71f 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -480,10 +480,12 @@ #endif
>  		/* Get end time (ticks) */
>  		t2 = inl(acpi_gbl_FADT.xpm_timer_block.address);
>  
> +#ifndef CONFIG_IA64
>  #ifdef CONFIG_GENERIC_TIME
>  		/* TSC halts in C2, so notify users */
>  		mark_tsc_unstable();
>  #endif
> +#endif

Is this a better description of the dependency?

  #if defined (CONFIG_GENERIC_TIME) && defined (CONFIG_X86_TSC)

>  		/* Re-enable interrupts */
>  		local_irq_enable();
>  		current_thread_info()->status |= TS_POLLING;
> @@ -522,10 +524,12 @@ #endif
>  			acpi_set_register(ACPI_BITREG_ARB_DISABLE, 0);
>  		}
>  
> +#ifndef CONFIG_IA64
>  #ifdef CONFIG_GENERIC_TIME
>  		/* TSC halts in C3, so notify users */
>  		mark_tsc_unstable();
>  #endif
> +#endif

ditto

>  		/* Re-enable interrupts */
>  		local_irq_enable();
>  		current_thread_info()->status |= TS_POLLING;
> diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
> index 0be700f..5ea7d3e 100644
> --- a/drivers/char/hpet.c
> +++ b/drivers/char/hpet.c
> @@ -29,6 +29,7 @@ #include <linux/wait.h>
>  #include <linux/bcd.h>
>  #include <linux/seq_file.h>
>  #include <linux/bitops.h>
> +#include <linux/clocksource.h>
>  
>  #include <asm/current.h>
>  #include <asm/uaccess.h>
> @@ -51,8 +52,34 @@ #define	HPET_DRIFT	(500)
>  
>  #define HPET_RANGE_SIZE		1024	/* from HPET spec */
>  
> +#if BITS_PER_LONG == 64
> +#define	write_counter(V, MC)	writeq(V, MC)
> +#define	read_counter(MC)	readq(MC)
> +#else
> +#define	write_counter(V, MC) 	writel(V, MC)
> +#define	read_counter(MC)	readl(MC)
> +#endif
> +
>  static u32 hpet_nhpet, hpet_max_freq = HPET_USER_FREQ;
>  
> +static void __iomem *hpet_mc_ptr;

CodingStyle nit: we don't need all this _ptr...

> +static cycle_t read_hpet(void)
> +{
> +	return (cycle_t)read_counter((void __iomem *)hpet_mc_ptr);
> +}
> +
> +static struct clocksource clocksource_hpet = {
> +        .name           = "hpet",
> +        .rating         = 300,
> +        .read           = read_hpet,
> +        .mask           = 0xffffffffffffffffLL,
> +        .mult           = 0, /*to be caluclated*/
> +        .shift          = 10,
> +        .is_continuous  = 1,
> +};
> +static struct clocksource *hpet_clocksource_p;

and _p naming.

> +
>  /* A lock for concurrent access by app and isr hpet activity. */
>  static DEFINE_SPINLOCK(hpet_lock);
>  /* A lock for concurrent intermodule access to hpet and isr hpet activity. */
> @@ -79,7 +106,7 @@ struct hpets {
>  	struct hpets *hp_next;
>  	struct hpet __iomem *hp_hpet;
>  	unsigned long hp_hpet_phys;
> -	struct time_interpolator *hp_interpolator;
> +	struct clocksource *hp_clocksource;
>  	unsigned long long hp_tick_freq;
>  	unsigned long hp_delta;
>  	unsigned int hp_ntimer;
> @@ -94,13 +121,6 @@ #define	HPET_IE			0x0002	/* interrupt en
>  #define	HPET_PERIODIC		0x0004
>  #define	HPET_SHARED_IRQ		0x0008
>  
> -#if BITS_PER_LONG == 64
> -#define	write_counter(V, MC)	writeq(V, MC)
> -#define	read_counter(MC)	readq(MC)
> -#else
> -#define	write_counter(V, MC) 	writel(V, MC)
> -#define	read_counter(MC)	readl(MC)
> -#endif
>  
>  #ifndef readq
>  static inline unsigned long long readq(void __iomem *addr)
> @@ -737,27 +757,6 @@ static ctl_table dev_root[] = {
>  
>  static struct ctl_table_header *sysctl_header;
>  
> -static void hpet_register_interpolator(struct hpets *hpetp)
> -{
> -#ifdef	CONFIG_TIME_INTERPOLATION
> -	struct time_interpolator *ti;
> -
> -	ti = kzalloc(sizeof(*ti), GFP_KERNEL);
> -	if (!ti)
> -		return;
> -
> -	ti->source = TIME_SOURCE_MMIO64;
> -	ti->shift = 10;
> -	ti->addr = &hpetp->hp_hpet->hpet_mc;
> -	ti->frequency = hpetp->hp_tick_freq;
> -	ti->drift = HPET_DRIFT;
> -	ti->mask = -1;
> -
> -	hpetp->hp_interpolator = ti;
> -	register_time_interpolator(ti);
> -#endif
> -}
> -
>  /*
>   * Adjustment for when arming the timer with
>   * initial conditions.  That is, main counter
> @@ -909,7 +908,14 @@ int hpet_alloc(struct hpet_data *hdp)
>  	}
>  
>  	hpetp->hp_delta = hpet_calibrate(hpetp);
> -	hpet_register_interpolator(hpetp);
> +
> +	if (!hpet_clocksource_p) {
> +        	clocksource_hpet.fsys_mmio_ptr = hpet_mc_ptr = &hpetp->hp_hpet->hpet_mc;
> +        	clocksource_hpet.mult = clocksource_hz2mult(hpetp->hp_tick_freq,
> +						clocksource_hpet.shift);
> +        	clocksource_register(&clocksource_hpet);
> +		hpet_clocksource_p = hpetp->hp_clocksource = &clocksource_hpet;
> +	}

This looks like a change in behaviour for non-ia64.  Now i386 and x86_64
will get this clocksource twice (and this one has a higher rating).
Doesn't look like this will even compile since fsys_mmio_ptr (more
extraneous _ptr suffix) is ia64 only?  And sparc64 is left out in
the cold.

>  	return 0;
>  }
> @@ -995,7 +1001,7 @@ static int hpet_acpi_add(struct acpi_dev
>  
>  static int hpet_acpi_remove(struct acpi_device *device, int type)
>  {
> -	/* XXX need to unregister interpolator, dealloc mem, etc */
> +	/* XXX need to unregister clocksource, dealloc mem, etc */
>  	return -EINVAL;
>  }
>  
> diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
> index daa4940..a20b4d6 100644
> --- a/include/linux/clocksource.h
> +++ b/include/linux/clocksource.h
> @@ -61,6 +61,9 @@ struct clocksource {
>  	u32 shift;
>  	unsigned long flags;
>  	cycle_t (*vread)(void);
> +#ifdef CONFIG_IA64
> +	void *fsys_mmio_ptr;	/* used by fsyscall asm code */
> +#endif
>  
>  	/* timekeeping specific data, ignore */
>  	cycle_t cycle_last, cycle_interval;

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/3] ia64: convert to use clocksource code
  2007-04-26 21:18 ` john stultz
@ 2007-04-27 12:54   ` Peter Keilty
  2007-05-02 17:50     ` john stultz
  0 siblings, 1 reply; 21+ messages in thread
From: Peter Keilty @ 2007-04-27 12:54 UTC (permalink / raw)
  To: john stultz; +Cc: linux-ia64, Eric Whitney, linux-kernel

john stultz wrote:

>On Thu, 2007-04-26 at 16:26 -0400, Peter Keilty wrote:
>  
>
>>From: Peter Keilty <peter.keilty@hp.com>
>>
>>Initial ia64 conversion to the generic timekeeping/clocksource code.
>>
>>Signed-off-by: Peter Keilty <peter.keilty@hp.com>
>>Signed-off-by: John Stultz <johnstul@us.ibm.com>
>>    
>>
>
>Peter,
>	Thanks so much for pushing this on! I suspect this patch needs to be
>updated a touch, as I'm not sure if it still compiles in light of recent
>changes...
>
>  
>
John,

You are correct, you need patch 3/3 for it to compile and run.
I did a patch to the orginal patch, thought that was correct thing to do.
But I can make a new patch 1 from the orginal of ours and my #3.
I would also make an update to the #2 patch from #3 for ntp correct change.
That would be just 2 patches then, the enable_ia64 and remove_interpolater.
What do you think?

>  
>
>>diff --git a/arch/ia64/kernel/cyclone.c b/arch/ia64/kernel/cyclone.c
>>index e00b215..280383b 100644
>>--- a/arch/ia64/kernel/cyclone.c
>>+++ b/arch/ia64/kernel/cyclone.c
>>@@ -3,6 +3,7 @@ #include <linux/smp.h>
>> #include <linux/time.h>
>> #include <linux/errno.h>
>> #include <linux/timex.h>
>>+#include <linux/clocksource.h>
>> #include <asm/io.h>
>>
>> /* IBM Summit (EXA) Cyclone counter code*/
>>@@ -18,13 +19,21 @@ void __init cyclone_setup(void)
>> 	use_cyclone = 1;
>> }
>>
>>+static void __iomem *cyclone_mc_ptr;
>>
>>-struct time_interpolator cyclone_interpolator = {
>>-	.source =	TIME_SOURCE_MMIO64,
>>-	.shift =	16,
>>-	.frequency =	CYCLONE_TIMER_FREQ,
>>-	.drift =	-100,
>>-	.mask =		(1LL << 40) - 1
>>+static cycle_t read_cyclone(void)
>>+{
>>+	return (cycle_t)readq((void __iomem *)cyclone_mc_ptr);
>>+}
>>+
>>+static struct clocksource clocksource_cyclone = {
>>+        .name           = "cyclone",
>>+        .rating         = 300,
>>+        .read           = read_cyclone,
>>+        .mask           = (1LL << 40) - 1,
>>    
>>
>
>Daniel Walker pointed out to me on IRC that CLOCKSOURCE_MASK() should
>probably be used here.
>
>  
>
>>+        .mult           = 0, /*to be caluclated*/
>>+        .shift          = 16,
>>+        .is_continuous  = 1,
>>    
>>
>
>.is_continuous no longer exists. 
>
>You want to use:
>.flags = CLOCK_SOURCE_IS_CONTINUOUS 
>
>This holds for all the clocksources introduced.
>
>
>thanks
>-john
>
>
>  
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/3] ia64: convert to use clocksource code
  2007-04-27  1:02 ` Chris Wright
@ 2007-04-27 13:35   ` Peter Keilty
  0 siblings, 0 replies; 21+ messages in thread
From: Peter Keilty @ 2007-04-27 13:35 UTC (permalink / raw)
  To: Chris Wright; +Cc: linux-ia64, John Stultz, Eric Whitney, linux-kernel

Chris Wright wrote:

>* Peter Keilty (peter.keilty@hp.com) wrote:
>  
>
>>diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
>>index 6077300..35ad71f 100644
>>--- a/drivers/acpi/processor_idle.c
>>+++ b/drivers/acpi/processor_idle.c
>>@@ -480,10 +480,12 @@ #endif
>> 		/* Get end time (ticks) */
>> 		t2 = inl(acpi_gbl_FADT.xpm_timer_block.address);
>> 
>>+#ifndef CONFIG_IA64
>> #ifdef CONFIG_GENERIC_TIME
>> 		/* TSC halts in C2, so notify users */
>> 		mark_tsc_unstable();
>> #endif
>>+#endif
>>    
>>
>
>Is this a better description of the dependency?
>
>  #if defined (CONFIG_GENERIC_TIME) && defined (CONFIG_X86_TSC)
>  
>
yes, ok.

>  
>
>> 		/* Re-enable interrupts */
>> 		local_irq_enable();
>> 		current_thread_info()->status |= TS_POLLING;
>>@@ -522,10 +524,12 @@ #endif
>> 			acpi_set_register(ACPI_BITREG_ARB_DISABLE, 0);
>> 		}
>> 
>>+#ifndef CONFIG_IA64
>> #ifdef CONFIG_GENERIC_TIME
>> 		/* TSC halts in C3, so notify users */
>> 		mark_tsc_unstable();
>> #endif
>>+#endif
>>    
>>
>
>ditto
>
>  
>
>> 		/* Re-enable interrupts */
>> 		local_irq_enable();
>> 		current_thread_info()->status |= TS_POLLING;
>>diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
>>index 0be700f..5ea7d3e 100644
>>--- a/drivers/char/hpet.c
>>+++ b/drivers/char/hpet.c
>>@@ -29,6 +29,7 @@ #include <linux/wait.h>
>> #include <linux/bcd.h>
>> #include <linux/seq_file.h>
>> #include <linux/bitops.h>
>>+#include <linux/clocksource.h>
>> 
>> #include <asm/current.h>
>> #include <asm/uaccess.h>
>>@@ -51,8 +52,34 @@ #define	HPET_DRIFT	(500)
>> 
>> #define HPET_RANGE_SIZE		1024	/* from HPET spec */
>> 
>>+#if BITS_PER_LONG == 64
>>+#define	write_counter(V, MC)	writeq(V, MC)
>>+#define	read_counter(MC)	readq(MC)
>>+#else
>>+#define	write_counter(V, MC) 	writel(V, MC)
>>+#define	read_counter(MC)	readl(MC)
>>+#endif
>>+
>> static u32 hpet_nhpet, hpet_max_freq = HPET_USER_FREQ;
>> 
>>+static void __iomem *hpet_mc_ptr;
>>    
>>
>
>CodingStyle nit: we don't need all this _ptr...
>
>  
>
>>+static cycle_t read_hpet(void)
>>+{
>>+	return (cycle_t)read_counter((void __iomem *)hpet_mc_ptr);
>>+}
>>+
>>+static struct clocksource clocksource_hpet = {
>>+        .name           = "hpet",
>>+        .rating         = 300,
>>+        .read           = read_hpet,
>>+        .mask           = 0xffffffffffffffffLL,
>>+        .mult           = 0, /*to be caluclated*/
>>+        .shift          = 10,
>>+        .is_continuous  = 1,
>>+};
>>+static struct clocksource *hpet_clocksource_p;
>>    
>>
>
>and _p naming.
>
>  
>
>>+
>> /* A lock for concurrent access by app and isr hpet activity. */
>> static DEFINE_SPINLOCK(hpet_lock);
>> /* A lock for concurrent intermodule access to hpet and isr hpet activity. */
>>@@ -79,7 +106,7 @@ struct hpets {
>> 	struct hpets *hp_next;
>> 	struct hpet __iomem *hp_hpet;
>> 	unsigned long hp_hpet_phys;
>>-	struct time_interpolator *hp_interpolator;
>>+	struct clocksource *hp_clocksource;
>> 	unsigned long long hp_tick_freq;
>> 	unsigned long hp_delta;
>> 	unsigned int hp_ntimer;
>>@@ -94,13 +121,6 @@ #define	HPET_IE			0x0002	/* interrupt en
>> #define	HPET_PERIODIC		0x0004
>> #define	HPET_SHARED_IRQ		0x0008
>> 
>>-#if BITS_PER_LONG == 64
>>-#define	write_counter(V, MC)	writeq(V, MC)
>>-#define	read_counter(MC)	readq(MC)
>>-#else
>>-#define	write_counter(V, MC) 	writel(V, MC)
>>-#define	read_counter(MC)	readl(MC)
>>-#endif
>> 
>> #ifndef readq
>> static inline unsigned long long readq(void __iomem *addr)
>>@@ -737,27 +757,6 @@ static ctl_table dev_root[] = {
>> 
>> static struct ctl_table_header *sysctl_header;
>> 
>>-static void hpet_register_interpolator(struct hpets *hpetp)
>>-{
>>-#ifdef	CONFIG_TIME_INTERPOLATION
>>-	struct time_interpolator *ti;
>>-
>>-	ti = kzalloc(sizeof(*ti), GFP_KERNEL);
>>-	if (!ti)
>>-		return;
>>-
>>-	ti->source = TIME_SOURCE_MMIO64;
>>-	ti->shift = 10;
>>-	ti->addr = &hpetp->hp_hpet->hpet_mc;
>>-	ti->frequency = hpetp->hp_tick_freq;
>>-	ti->drift = HPET_DRIFT;
>>-	ti->mask = -1;
>>-
>>-	hpetp->hp_interpolator = ti;
>>-	register_time_interpolator(ti);
>>-#endif
>>-}
>>-
>> /*
>>  * Adjustment for when arming the timer with
>>  * initial conditions.  That is, main counter
>>@@ -909,7 +908,14 @@ int hpet_alloc(struct hpet_data *hdp)
>> 	}
>> 
>> 	hpetp->hp_delta = hpet_calibrate(hpetp);
>>-	hpet_register_interpolator(hpetp);
>>+
>>+	if (!hpet_clocksource_p) {
>>+        	clocksource_hpet.fsys_mmio_ptr = hpet_mc_ptr = &hpetp->hp_hpet->hpet_mc;
>>+        	clocksource_hpet.mult = clocksource_hz2mult(hpetp->hp_tick_freq,
>>+						clocksource_hpet.shift);
>>+        	clocksource_register(&clocksource_hpet);
>>+		hpet_clocksource_p = hpetp->hp_clocksource = &clocksource_hpet;
>>+	}
>>    
>>
>
>This looks like a change in behaviour for non-ia64.  Now i386 and x86_64
>will get this clocksource twice (and this one has a higher rating).
>Doesn't look like this will even compile since fsys_mmio_ptr (more
>  
>
Yes need to change setting for  fsys_mmio just for ia64.
I will check the load multiple clocksources, before it was limited.

>extraneous _ptr suffix) is ia64 only?  And sparc64 is left out in
>the cold.
>
>  
>
>> 	return 0;
>> }
>>@@ -995,7 +1001,7 @@ static int hpet_acpi_add(struct acpi_dev
>> 
>> static int hpet_acpi_remove(struct acpi_device *device, int type)
>> {
>>-	/* XXX need to unregister interpolator, dealloc mem, etc */
>>+	/* XXX need to unregister clocksource, dealloc mem, etc */
>> 	return -EINVAL;
>> }
>> 
>>diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
>>index daa4940..a20b4d6 100644
>>--- a/include/linux/clocksource.h
>>+++ b/include/linux/clocksource.h
>>@@ -61,6 +61,9 @@ struct clocksource {
>> 	u32 shift;
>> 	unsigned long flags;
>> 	cycle_t (*vread)(void);
>>+#ifdef CONFIG_IA64
>>+	void *fsys_mmio_ptr;	/* used by fsyscall asm code */
>>+#endif
>> 
>> 	/* timekeeping specific data, ignore */
>> 	cycle_t cycle_last, cycle_interval;
>>    
>>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/3] ia64: convert to use clocksource code
  2007-04-26 21:07 ` Daniel Walker
@ 2007-04-27 14:38   ` Peter Keilty
  2007-04-27 15:35     ` Daniel Walker
  0 siblings, 1 reply; 21+ messages in thread
From: Peter Keilty @ 2007-04-27 14:38 UTC (permalink / raw)
  To: Daniel Walker; +Cc: linux-ia64, John Stultz, Eric Whitney, linux-kernel

Daniel Walker wrote:

>On Thu, 2007-04-26 at 16:26 -0400, Peter Keilty wrote:
>
>  
>
>>+        .mask           = (1LL << 40) - 1,
>>+        .mult           = 0, /*to be caluclated*/
>>+        .shift          = 16,
>>+        .is_continuous  = 1,
>> };
>>    
>>
>
>You should use CLOCKSOURCE_MASK() here ..
>
>  
>
It is correct in patch 3, I believe.

>  
>
>> 
>>diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
>>index daa4940..a20b4d6 100644
>>--- a/include/linux/clocksource.h
>>+++ b/include/linux/clocksource.h
>>@@ -61,6 +61,9 @@ struct clocksource {
>> 	u32 shift;
>> 	unsigned long flags;
>> 	cycle_t (*vread)(void);
>>+#ifdef CONFIG_IA64
>>+	void *fsys_mmio_ptr;	/* used by fsyscall asm code */
>>+#endif
>>    
>>
>
>Could you explain in detail why this is needed?
>  
>
This ptr is needed to hold the mmio address to read the cycle value.
The fast ia64 path utilizies a special gate page which can allow user
code to execute small amount of kernel code, normal calling a function
not done and so the address is need. The fast syscall path executes on
user stack, between user/kernel state. And if the the fast path has to 
fallback
to the slow syscall code the vread will be needed and used.
Hope this helps.

>Daniel
>
>  
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/3] ia64: convert to use clocksource code
  2007-04-27 14:38   ` Peter Keilty
@ 2007-04-27 15:35     ` Daniel Walker
  2007-04-27 15:42       ` Peter Keilty
  0 siblings, 1 reply; 21+ messages in thread
From: Daniel Walker @ 2007-04-27 15:35 UTC (permalink / raw)
  To: Peter Keilty; +Cc: linux-ia64, John Stultz, Eric Whitney, linux-kernel

On Fri, 2007-04-27 at 10:38 -0400, Peter Keilty wrote:
> Daniel Walker wrote:
> 
> >On Thu, 2007-04-26 at 16:26 -0400, Peter Keilty wrote:
> >
> >  
> >
> >>+        .mask           = (1LL << 40) - 1,
> >>+        .mult           = 0, /*to be caluclated*/
> >>+        .shift          = 16,
> >>+        .is_continuous  = 1,
> >> };
> >>    
> >>
> >
> >You should use CLOCKSOURCE_MASK() here ..
> >
> >  
> >
> It is correct in patch 3, I believe.

There's another spot that should use CLOCKSOURE_MASK() in the hpet I
think .

> >  
> >
> >> 
> >>diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
> >>index daa4940..a20b4d6 100644
> >>--- a/include/linux/clocksource.h
> >>+++ b/include/linux/clocksource.h
> >>@@ -61,6 +61,9 @@ struct clocksource {
> >> 	u32 shift;
> >> 	unsigned long flags;
> >> 	cycle_t (*vread)(void);
> >>+#ifdef CONFIG_IA64
> >>+	void *fsys_mmio_ptr;	/* used by fsyscall asm code */
> >>+#endif
> >>    
> >>
> >
> >Could you explain in detail why this is needed?
> >  
> >
> This ptr is needed to hold the mmio address to read the cycle value.
> The fast ia64 path utilizies a special gate page which can allow user
> code to execute small amount of kernel code, normal calling a function
> not done and so the address is need. The fast syscall path executes on
> user stack, between user/kernel state. And if the the fast path has to 
> fallback
> to the slow syscall code the vread will be needed and used.

There is a read(), and a vread() did you modify the slow syscall path to
use the vread()?

Daniel


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/3] ia64: convert to use clocksource code
  2007-04-27 15:35     ` Daniel Walker
@ 2007-04-27 15:42       ` Peter Keilty
  2007-04-27 15:48         ` Daniel Walker
  0 siblings, 1 reply; 21+ messages in thread
From: Peter Keilty @ 2007-04-27 15:42 UTC (permalink / raw)
  To: Daniel Walker; +Cc: linux-ia64, John Stultz, Eric Whitney, linux-kernel

Daniel Walker wrote:

>On Fri, 2007-04-27 at 10:38 -0400, Peter Keilty wrote:
>  
>
>>Daniel Walker wrote:
>>
>>    
>>
>>>On Thu, 2007-04-26 at 16:26 -0400, Peter Keilty wrote:
>>>
>>> 
>>>
>>>      
>>>
>>>>+        .mask           = (1LL << 40) - 1,
>>>>+        .mult           = 0, /*to be caluclated*/
>>>>+        .shift          = 16,
>>>>+        .is_continuous  = 1,
>>>>};
>>>>   
>>>>
>>>>        
>>>>
>>>You should use CLOCKSOURCE_MASK() here ..
>>>
>>> 
>>>
>>>      
>>>
>>It is correct in patch 3, I believe.
>>    
>>
>
>There's another spot that should use CLOCKSOURE_MASK() in the hpet I
>think .
>  
>
Correct that was change in patch 3 also.

>  
>
>>> 
>>>
>>>      
>>>
>>>>diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
>>>>index daa4940..a20b4d6 100644
>>>>--- a/include/linux/clocksource.h
>>>>+++ b/include/linux/clocksource.h
>>>>@@ -61,6 +61,9 @@ struct clocksource {
>>>>	u32 shift;
>>>>	unsigned long flags;
>>>>	cycle_t (*vread)(void);
>>>>+#ifdef CONFIG_IA64
>>>>+	void *fsys_mmio_ptr;	/* used by fsyscall asm code */
>>>>+#endif
>>>>   
>>>>
>>>>        
>>>>
>>>Could you explain in detail why this is needed?
>>> 
>>>
>>>      
>>>
>>This ptr is needed to hold the mmio address to read the cycle value.
>>The fast ia64 path utilizies a special gate page which can allow user
>>code to execute small amount of kernel code, normal calling a function
>>not done and so the address is need. The fast syscall path executes on
>>user stack, between user/kernel state. And if the the fast path has to 
>>fallback
>>to the slow syscall code the vread will be needed and used.
>>    
>>
>
>There is a read(), and a vread() did you modify the slow syscall path to
>use the vread()?
>  
>
I miss type, read().

>Daniel
>
>  
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/3] ia64: convert to use clocksource code
  2007-04-27 15:42       ` Peter Keilty
@ 2007-04-27 15:48         ` Daniel Walker
  2007-04-27 16:11           ` Peter Keilty
  0 siblings, 1 reply; 21+ messages in thread
From: Daniel Walker @ 2007-04-27 15:48 UTC (permalink / raw)
  To: Peter Keilty; +Cc: linux-ia64, John Stultz, Eric Whitney, linux-kernel

On Fri, 2007-04-27 at 11:42 -0400, Peter Keilty wrote:

> >
> >There is a read(), and a vread() did you modify the slow syscall path to
> >use the vread()?
> >  
> >
> I miss type, read().

John mentioned that he thought fsys_mmio_ptr could be held in the vread
pointer. vread() is used in x86 for vsyscalls. It looks like you've used
the update_vsyscall() which is also used for vsyscalls. So vread could
also be used .. Have you considered that at all?

Daniel


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/3] ia64: convert to use clocksource code
  2007-04-27 15:48         ` Daniel Walker
@ 2007-04-27 16:11           ` Peter Keilty
  2007-04-27 16:32             ` Daniel Walker
  2007-05-02 17:58             ` john stultz
  0 siblings, 2 replies; 21+ messages in thread
From: Peter Keilty @ 2007-04-27 16:11 UTC (permalink / raw)
  To: Daniel Walker; +Cc: linux-ia64, John Stultz, Eric Whitney, linux-kernel

Daniel Walker wrote:

>On Fri, 2007-04-27 at 11:42 -0400, Peter Keilty wrote:
>
>  
>
>>>There is a read(), and a vread() did you modify the slow syscall path to
>>>use the vread()?
>>> 
>>>
>>>      
>>>
>>I miss type, read().
>>    
>>
>
>John mentioned that he thought fsys_mmio_ptr could be held in the vread
>pointer. vread() is used in x86 for vsyscalls. It looks like you've used
>the update_vsyscall() which is also used for vsyscalls. So vread could
>also be used .. Have you considered that at all?
>  
>
No, but yes it can be done, overloading the meaning.
It would need to change in the future if vread was needed.
I have no strong argument against  using it.
Although we may still need the IA64 define, I removed 32bit read mmio and
if that is brought back the fast syscall patch call will need to have a 
field in the
clocksource struct that would indicated that. Waiting on comments about 
that...
John and discuss this awhile back felt it was not needed, may prove wrong.

>Daniel
>
>  
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/3] ia64: convert to use clocksource code
  2007-04-27 16:11           ` Peter Keilty
@ 2007-04-27 16:32             ` Daniel Walker
  2007-05-02 17:58             ` john stultz
  1 sibling, 0 replies; 21+ messages in thread
From: Daniel Walker @ 2007-04-27 16:32 UTC (permalink / raw)
  To: Peter Keilty; +Cc: linux-ia64, John Stultz, Eric Whitney, linux-kernel

On Fri, 2007-04-27 at 12:11 -0400, Peter Keilty wrote:

> >
> No, but yes it can be done, overloading the meaning.
> It would need to change in the future if vread was needed.
> I have no strong argument against  using it.
> Although we may still need the IA64 define, I removed 32bit read mmio and
> if that is brought back the fast syscall patch call will need to have a 
> field in the
> clocksource struct that would indicated that. Waiting on comments about 
> that...
> John and discuss this awhile back felt it was not needed, may prove wrong.

The vread field is just a pointer, which should be 64bits on ia64. Why
do you mention 32bits above? The two variables similar enough .. 

Daniel


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/3] ia64: convert to use clocksource code
  2007-04-27 12:54   ` Peter Keilty
@ 2007-05-02 17:50     ` john stultz
  0 siblings, 0 replies; 21+ messages in thread
From: john stultz @ 2007-05-02 17:50 UTC (permalink / raw)
  To: Peter Keilty; +Cc: linux-ia64, Eric Whitney, linux-kernel

On Fri, 2007-04-27 at 08:54 -0400, Peter Keilty wrote:
> john stultz wrote:
> >On Thu, 2007-04-26 at 16:26 -0400, Peter Keilty wrote:
> >>From: Peter Keilty <peter.keilty@hp.com>
> >>
> >>Initial ia64 conversion to the generic timekeeping/clocksource code.
> >>
> >>Signed-off-by: Peter Keilty <peter.keilty@hp.com>
> >>Signed-off-by: John Stultz <johnstul@us.ibm.com>
> >>    
> >	Thanks so much for pushing this on! I suspect this patch needs to be
> >updated a touch, as I'm not sure if it still compiles in light of recent
> >changes...
> >
> You are correct, you need patch 3/3 for it to compile and run.
> I did a patch to the orginal patch, thought that was correct thing to do.
> But I can make a new patch 1 from the orginal of ours and my #3.
> I would also make an update to the #2 patch from #3 for ntp correct change.
> That would be just 2 patches then, the enable_ia64 and remove_interpolater.
> What do you think?

Yep. That sounds like the right path to me. Its a good idea to make sure
your patch set compiles each step of the way, so later folks can
properly bisect through it looking for other issues.

thanks
-john





^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/3] ia64: convert to use clocksource code
  2007-04-27 16:11           ` Peter Keilty
  2007-04-27 16:32             ` Daniel Walker
@ 2007-05-02 17:58             ` john stultz
  2007-05-02 19:08               ` Daniel Walker
  1 sibling, 1 reply; 21+ messages in thread
From: john stultz @ 2007-05-02 17:58 UTC (permalink / raw)
  To: Peter Keilty; +Cc: Daniel Walker, linux-ia64, Eric Whitney, linux-kernel

On Fri, 2007-04-27 at 12:11 -0400, Peter Keilty wrote:
> Daniel Walker wrote:
> >On Fri, 2007-04-27 at 11:42 -0400, Peter Keilty wrote:
> >>>There is a read(), and a vread() did you modify the slow syscall path to
> >>>use the vread()?
> >>> 
> >John mentioned that he thought fsys_mmio_ptr could be held in the vread
> >pointer. vread() is used in x86 for vsyscalls. It looks like you've used
> >the update_vsyscall() which is also used for vsyscalls. So vread could
> >also be used .. Have you considered that at all?
> >  
> >
> No, but yes it can be done, overloading the meaning.

Yea. I'm not really psyched about overloading the vread pointer's use. I
mentioned it could be done if the #ifdef was objected to, but it seems a
bit abusive. The #ifdef isn't great, but I think its something I can
live with for now. At least its explicit.

> It would need to change in the future if vread was needed.
> I have no strong argument against  using it.

Yea. I'd hold off on that for now.

thanks
-john



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/3] ia64: convert to use clocksource code
  2007-05-02 17:58             ` john stultz
@ 2007-05-02 19:08               ` Daniel Walker
  0 siblings, 0 replies; 21+ messages in thread
From: Daniel Walker @ 2007-05-02 19:08 UTC (permalink / raw)
  To: john stultz; +Cc: Peter Keilty, linux-ia64, Eric Whitney, linux-kernel

On Wed, 2007-05-02 at 10:58 -0700, john stultz wrote:
> On Fri, 2007-04-27 at 12:11 -0400, Peter Keilty wrote:
> > Daniel Walker wrote:
> > >On Fri, 2007-04-27 at 11:42 -0400, Peter Keilty wrote:
> > >>>There is a read(), and a vread() did you modify the slow syscall path to
> > >>>use the vread()?
> > >>> 
> > >John mentioned that he thought fsys_mmio_ptr could be held in the vread
> > >pointer. vread() is used in x86 for vsyscalls. It looks like you've used
> > >the update_vsyscall() which is also used for vsyscalls. So vread could
> > >also be used .. Have you considered that at all?
> > >  
> > >
> > No, but yes it can be done, overloading the meaning.
> 
> Yea. I'm not really psyched about overloading the vread pointer's use. I
> mentioned it could be done if the #ifdef was objected to, but it seems a
> bit abusive. The #ifdef isn't great, but I think its something I can
> live with for now. At least its explicit.

Use of config options like that is a bad precedence I think, which is
why I commented on it .. Since we have a vread pointer that exists
already, and it's used a very similar purpose it seems like bloat to
just add another pointer.. 

We could change the vread to be a plain void pointer, then let each
architecture use it however they want.

Daniel


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2007-05-02 19:10 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-04-26 20:26 [PATCH 1/3] ia64: convert to use clocksource code Peter Keilty
2007-04-26 20:27 ` [PATCH 2/3] ia64: remove interpolater code Peter Keilty
2007-04-26 20:52   ` john stultz
2007-04-26 21:47     ` David Miller
2007-04-26 20:27 ` [PATCH 3/3] ia64: update fsyscall for performance, enable build/run on 2.6.21-rc1 Peter Keilty
2007-04-26 20:41 ` [PATCH 1/3] ia64: convert to use clocksource code Sam Ravnborg
2007-04-26 20:48   ` john stultz
2007-04-26 21:07 ` Daniel Walker
2007-04-27 14:38   ` Peter Keilty
2007-04-27 15:35     ` Daniel Walker
2007-04-27 15:42       ` Peter Keilty
2007-04-27 15:48         ` Daniel Walker
2007-04-27 16:11           ` Peter Keilty
2007-04-27 16:32             ` Daniel Walker
2007-05-02 17:58             ` john stultz
2007-05-02 19:08               ` Daniel Walker
2007-04-26 21:18 ` john stultz
2007-04-27 12:54   ` Peter Keilty
2007-05-02 17:50     ` john stultz
2007-04-27  1:02 ` Chris Wright
2007-04-27 13:35   ` Peter Keilty

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).