[RFC][PATCH] Unify interface to persistent CMOS/RTC/whatever clock

[RFC][PATCH] Unify interface to persistent CMOS/RTC/whatever clock

[john stultz]

All,
	Almost every arch has some form of persistent clock (CMOS, RTC, etc)
which is normally used at boot time to initialize xtime and
wall_to_monotonic. As part of the timekeeping consolidation, I propose
the following generic interface to the arch specific persistent clock:

unsigned long read_persistent_clock(void);

Which returns seconds since the epoch.

Having this interface allows for the xtime/wall_to_monotonic
initialization and management to be consolidated along with the rest of
the timekeeping core. Additionally it provides a common interface to a
low-res purely hardware driven clock, which is useful for watchdog
functionality as seen in the hangcheck-timer module.

My first pass at this can be found below. There are a few arches
(specifically: m32r, s390, sparc, sparc64) where I just didn't know what
to do, or where I suspect I didn't get it right, so I've CC'ed those
maintainers for suggestions.

I'll break this patch up before submitting it (using a generic weak
function we can go arch-by-arch), but I wanted to get it out there to
show I'm shooting for 100% coverage and see the initial reaction.

thanks
-john

 arch/alpha/kernel/time.c     |   77 +++++++++++++++++++++----------------------
 arch/cris/kernel/crisksyms.c |    4 +-
 arch/cris/kernel/time.c      |    4 +-
 arch/frv/kernel/time.c       |   10 +++--
 arch/h8300/kernel/time.c     |    8 ++--
 arch/i386/kernel/apm.c       |    6 +--
 arch/i386/kernel/time.c      |   22 +++---------
 arch/ia64/kernel/time.c      |   14 +++----
 arch/m32r/kernel/time.c      |    6 +++
 arch/m68k/kernel/time.c      |   10 +++--
 arch/m68knommu/kernel/time.c |    9 ++---
 arch/mips/kernel/time.c      |   11 ++----
 arch/parisc/kernel/time.c    |   19 +++-------
 arch/powerpc/kernel/time.c   |    4 +-
 arch/ppc/kernel/time.c       |    8 ++++
 arch/s390/kernel/time.c      |    6 +++
 arch/sh/kernel/time.c        |   20 +++++------
 arch/sh64/kernel/time.c      |    8 ++--
 arch/sparc/kernel/time.c     |    6 +++
 arch/sparc64/kernel/time.c   |   44 +++++++++++++-----------
 arch/um/kernel/time.c        |    9 ++---
 arch/v850/kernel/time.c      |    8 +++-
 arch/x86_64/kernel/time.c    |   12 +-----
 arch/xtensa/kernel/time.c    |   24 ++++++-------
 kernel/timer.c               |    8 +++-
 25 files changed, 194 insertions(+), 163 deletions(-)

linux-2.6.18-rc4_timeofday-persistent-clock_C5.patch
============================================
diff --git a/arch/alpha/kernel/time.c b/arch/alpha/kernel/time.c
index b191cc7..647f700 100644
--- a/arch/alpha/kernel/time.c
+++ b/arch/alpha/kernel/time.c
@@ -349,44 +349,6 @@ time_init(void)
 	   bogomips yet, but this is close on a 500Mhz box.  */
 	__delay(1000000);
 
-	sec = CMOS_READ(RTC_SECONDS);
-	min = CMOS_READ(RTC_MINUTES);
-	hour = CMOS_READ(RTC_HOURS);
-	day = CMOS_READ(RTC_DAY_OF_MONTH);
-	mon = CMOS_READ(RTC_MONTH);
-	year = CMOS_READ(RTC_YEAR);
-
-	if (!(CMOS_READ(RTC_CONTROL) & RTC_DM_BINARY) || RTC_ALWAYS_BCD) {
-		BCD_TO_BIN(sec);
-		BCD_TO_BIN(min);
-		BCD_TO_BIN(hour);
-		BCD_TO_BIN(day);
-		BCD_TO_BIN(mon);
-		BCD_TO_BIN(year);
-	}
-
-	/* PC-like is standard; used for year >= 70 */
-	epoch = 1900;
-	if (year < 20)
-		epoch = 2000;
-	else if (year >= 20 && year < 48)
-		/* NT epoch */
-		epoch = 1980;
-	else if (year >= 48 && year < 70)
-		/* Digital UNIX epoch */
-		epoch = 1952;
-
-	printk(KERN_INFO "Using epoch = %d\n", epoch);
-
-	if ((year += epoch) < 1970)
-		year += 100;
-
-	xtime.tv_sec = mktime(year, mon, day, hour, min, sec);
-	xtime.tv_nsec = 0;
-
-        wall_to_monotonic.tv_sec -= xtime.tv_sec;
-        wall_to_monotonic.tv_nsec = 0;
-
 	if (HZ > (1<<16)) {
 		extern void __you_loose (void);
 		__you_loose();
@@ -507,6 +469,45 @@ do_settimeofday(struct timespec *tv)
 
 EXPORT_SYMBOL(do_settimeofday);
 
+/* Read the CMOS clock and convert it to epoch based seconds */
+unsigned long read_persistent_clock(void)
+{
+	unsigned int year, mon, day, hour, min, sec, epoch;
+
+	sec = CMOS_READ(RTC_SECONDS);
+	min = CMOS_READ(RTC_MINUTES);
+	hour = CMOS_READ(RTC_HOURS);
+	day = CMOS_READ(RTC_DAY_OF_MONTH);
+	mon = CMOS_READ(RTC_MONTH);
+	year = CMOS_READ(RTC_YEAR);
+
+	if (!(CMOS_READ(RTC_CONTROL) & RTC_DM_BINARY) || RTC_ALWAYS_BCD) {
+		BCD_TO_BIN(sec);
+		BCD_TO_BIN(min);
+		BCD_TO_BIN(hour);
+		BCD_TO_BIN(day);
+		BCD_TO_BIN(mon);
+		BCD_TO_BIN(year);
+	}
+
+	/* PC-like is standard; used for year >= 70 */
+	epoch = 1900;
+	if (year < 20)
+		epoch = 2000;
+	else if (year >= 20 && year < 48)
+		/* NT epoch */
+		epoch = 1980;
+	else if (year >= 48 && year < 70)
+		/* Digital UNIX epoch */
+		epoch = 1952;
+
+	printk(KERN_INFO "Using epoch = %d\n", epoch);
+
+	if ((year += epoch) < 1970)
+		year += 100;
+
+	return = mktime(year, mon, day, hour, min, sec);
+}
 
 /*
  * In order to set the CMOS clock precisely, set_rtc_mmss has to be
diff --git a/arch/cris/kernel/crisksyms.c b/arch/cris/kernel/crisksyms.c
index 1f20c16..3d702c3 100644
--- a/arch/cris/kernel/crisksyms.c
+++ b/arch/cris/kernel/crisksyms.c
@@ -20,7 +20,7 @@
 #include <asm/pgtable.h>
 #include <asm/fasttimer.h>
 
-extern unsigned long get_cmos_time(void);
+extern unsigned long read_persistent_clock(void);
 extern void __Udiv(void);
 extern void __Umod(void);
 extern void __Div(void);
@@ -32,7 +32,7 @@ extern void iounmap(volatile void * __io
 
 /* Platform dependent support */
 EXPORT_SYMBOL(kernel_thread);
-EXPORT_SYMBOL(get_cmos_time);
+EXPORT_SYMBOL(read_persistent_clock);
 EXPORT_SYMBOL(loops_per_usec);
 
 /* String functions */
diff --git a/arch/cris/kernel/time.c b/arch/cris/kernel/time.c
index 66ba889..329323c 100644
--- a/arch/cris/kernel/time.c
+++ b/arch/cris/kernel/time.c
@@ -168,7 +168,7 @@ int set_rtc_mmss(unsigned long nowtime)
 /* grab the time from the RTC chip */
 
 unsigned long
-get_cmos_time(void)
+read_persistent_clock(void)
 {
 	unsigned int year, mon, day, hour, min, sec;
 
@@ -204,7 +204,7 @@ void
 update_xtime_from_cmos(void)
 {
 	if(have_rtc) {
-		xtime.tv_sec = get_cmos_time();
+		xtime.tv_sec = read_persistent_clock();
 		xtime.tv_nsec = 0;
 	}
 }
diff --git a/arch/frv/kernel/time.c b/arch/frv/kernel/time.c
index d5b64e1..c32659e 100644
--- a/arch/frv/kernel/time.c
+++ b/arch/frv/kernel/time.c
@@ -119,7 +119,8 @@ void time_divisor_init(void)
 	__set_TCSR_DATA(0, base >> 8);
 }
 
-void time_init(void)
+
+unsigned long read_persistent_clock(void)
 {
 	unsigned int year, mon, day, hour, min, sec;
 
@@ -135,9 +136,12 @@ void time_init(void)
 
 	if ((year += 1900) < 1970)
 		year += 100;
-	xtime.tv_sec = mktime(year, mon, day, hour, min, sec);
-	xtime.tv_nsec = 0;
+	return mktime(year, mon, day, hour, min, sec);
+
+}
 
+void time_init(void)
+{
 	/* install scheduling interrupt handler */
 	setup_irq(IRQ_CPU_TIMER0, &timer_irq);
 
diff --git a/arch/h8300/kernel/time.c b/arch/h8300/kernel/time.c
index 688a510..18ea76c 100644
--- a/arch/h8300/kernel/time.c
+++ b/arch/h8300/kernel/time.c
@@ -48,7 +48,7 @@ static void timer_interrupt(int irq, voi
 	profile_tick(CPU_PROFILING, regs);
 }
 
-void time_init(void)
+unsigned long read_persistent_clock(void)
 {
 	unsigned int year, mon, day, hour, min, sec;
 
@@ -62,9 +62,11 @@ void time_init(void)
 
 	if ((year += 1900) < 1970)
 		year += 100;
-	xtime.tv_sec = mktime(year, mon, day, hour, min, sec);
-	xtime.tv_nsec = 0;
+	return mktime(year, mon, day, hour, min, sec);
+}
 
+void time_init(void)
+{
 	platform_timer_setup(timer_interrupt);
 }
 
diff --git a/arch/i386/kernel/apm.c b/arch/i386/kernel/apm.c
index 8591f2f..6d441ae 100644
--- a/arch/i386/kernel/apm.c
+++ b/arch/i386/kernel/apm.c
@@ -233,7 +233,7 @@
 
 #include "io_ports.h"
 
-extern unsigned long get_cmos_time(void);
+extern unsigned long read_persistent_clock(void);
 extern void machine_real_restart(unsigned char *, int);
 
 #if defined(CONFIG_APM_DISPLAY_BLANK) && defined(CONFIG_VT)
@@ -1155,7 +1155,7 @@ out:
 static void set_time(void)
 {
 	if (got_clock_diff) {	/* Must know time zone in order to set clock */
-		xtime.tv_sec = get_cmos_time() + clock_cmos_diff;
+		xtime.tv_sec = read_persistent_clock() + clock_cmos_diff;
 		xtime.tv_nsec = 0; 
 	} 
 }
@@ -1166,7 +1166,7 @@ static void get_time_diff(void)
 	/*
 	 * Estimate time zone so that set_time can update the clock
 	 */
-	clock_cmos_diff = -get_cmos_time();
+	clock_cmos_diff = -read_persistent_clock();
 	clock_cmos_diff += get_seconds();
 	got_clock_diff = 1;
 #endif
diff --git a/arch/i386/kernel/time.c b/arch/i386/kernel/time.c
index edd00f6..571eb6c 100644
--- a/arch/i386/kernel/time.c
+++ b/arch/i386/kernel/time.c
@@ -203,7 +203,7 @@ irqreturn_t timer_interrupt(int irq, voi
 }
 
 /* not static: needed by APM */
-unsigned long get_cmos_time(void)
+unsigned long read_persistent_clock(void)
 {
 	unsigned long retval;
 	unsigned long flags;
@@ -219,7 +219,7 @@ unsigned long get_cmos_time(void)
 
 	return retval;
 }
-EXPORT_SYMBOL(get_cmos_time);
+EXPORT_SYMBOL(read_persistent_clock);
 
 static void sync_cmos_clock(unsigned long dummy);
 
@@ -277,9 +277,9 @@ static int timer_suspend(struct sys_devi
 	/*
 	 * Estimate time zone so that set_time can update the clock
 	 */
-	clock_cmos_diff = -get_cmos_time();
+	clock_cmos_diff = -read_persistent_clock();
 	clock_cmos_diff += get_seconds();
-	sleep_start = get_cmos_time();
+	sleep_start = read_persistent_clock();
 	return 0;
 }
 
@@ -294,8 +294,8 @@ static int timer_resume(struct sys_devic
 		hpet_reenable();
 #endif
 	setup_pit_timer();
-	sec = get_cmos_time() + clock_cmos_diff;
-	sleep_length = (get_cmos_time() - sleep_start) * HZ;
+	sec = read_persistent_clock() + clock_cmos_diff;
+	sleep_length = (read_persistent_clock() - sleep_start) * HZ;
 	write_seqlock_irqsave(&xtime_lock, flags);
 	xtime.tv_sec = sec;
 	xtime.tv_nsec = 0;
@@ -334,11 +334,6 @@ extern void (*late_time_init)(void);
 /* Duplicate of time_init() below, with hpet_enable part added */
 static void __init hpet_time_init(void)
 {
-	xtime.tv_sec = get_cmos_time();
-	xtime.tv_nsec = (INITIAL_JIFFIES % HZ) * (NSEC_PER_SEC / HZ);
-	set_normalized_timespec(&wall_to_monotonic,
-		-xtime.tv_sec, -xtime.tv_nsec);
-
 	if ((hpet_enable() >= 0) && hpet_use_timer) {
 		printk("Using HPET for base-timer\n");
 	}
@@ -359,10 +354,5 @@ void __init time_init(void)
 		return;
 	}
 #endif
-	xtime.tv_sec = get_cmos_time();
-	xtime.tv_nsec = (INITIAL_JIFFIES % HZ) * (NSEC_PER_SEC / HZ);
-	set_normalized_timespec(&wall_to_monotonic,
-		-xtime.tv_sec, -xtime.tv_nsec);
-
 	time_init_hook();
 }
diff --git a/arch/ia64/kernel/time.c b/arch/ia64/kernel/time.c
index 6928ef0..57f119d 100644
--- a/arch/ia64/kernel/time.c
+++ b/arch/ia64/kernel/time.c
@@ -240,18 +240,18 @@ void __devinit ia64_disable_timer(void)
 	ia64_set_itv(1 << 16);
 }
 
+unsigned long read_persistent_clock(void)
+{
+	struct timespec ts;
+	efi_gettimeofday(&ts);
+	return ts.tv_sec;
+}
+
 void __init
 time_init (void)
 {
 	register_percpu_irq(IA64_TIMER_VECTOR, &timer_irqaction);
-	efi_gettimeofday(&xtime);
 	ia64_init_itm();
-
-	/*
-	 * Initialize wall_to_monotonic such that adding it to xtime will yield zero, the
-	 * tv_nsec field must be normalized (i.e., 0 <= nsec < NSEC_PER_SEC).
-	 */
-	set_normalized_timespec(&wall_to_monotonic, -xtime.tv_sec, -xtime.tv_nsec);
 }
 
 /*
diff --git a/arch/m32r/kernel/time.c b/arch/m32r/kernel/time.c
index ded0be0..eebf379 100644
--- a/arch/m32r/kernel/time.c
+++ b/arch/m32r/kernel/time.c
@@ -240,6 +240,12 @@ irqreturn_t timer_interrupt(int irq, voi
 struct irqaction irq0 = { timer_interrupt, IRQF_DISABLED, CPU_MASK_NONE,
 			  "MFT2", NULL, NULL };
 
+/* XXX - Need some help here! */
+unsigned long read_persistent_clock(void)
+{
+	return 0;
+}
+
 void __init time_init(void)
 {
 	unsigned int epoch, year, mon, day, hour, min, sec;
diff --git a/arch/m68k/kernel/time.c b/arch/m68k/kernel/time.c
index 98e4b1a..56b51e2 100644
--- a/arch/m68k/kernel/time.c
+++ b/arch/m68k/kernel/time.c
@@ -72,7 +72,7 @@ static irqreturn_t timer_interrupt(int i
 	return IRQ_HANDLED;
 }
 
-void time_init(void)
+static unsigned long read_persistent_clock(void)
 {
 	struct rtc_time time;
 
@@ -81,12 +81,14 @@ void time_init(void)
 
 		if ((time.tm_year += 1900) < 1970)
 			time.tm_year += 100;
-		xtime.tv_sec = mktime(time.tm_year, time.tm_mon, time.tm_mday,
+		return mktime(time.tm_year, time.tm_mon, time.tm_mday,
 				      time.tm_hour, time.tm_min, time.tm_sec);
-		xtime.tv_nsec = 0;
 	}
-	wall_to_monotonic.tv_sec = -xtime.tv_sec;
+	return 0;
+}
 
+void time_init(void)
+{
 	mach_sched_init(timer_interrupt);
 }
 
diff --git a/arch/m68knommu/kernel/time.c b/arch/m68knommu/kernel/time.c
index 1db9872..2db1557 100644
--- a/arch/m68knommu/kernel/time.c
+++ b/arch/m68knommu/kernel/time.c
@@ -100,7 +100,7 @@ static irqreturn_t timer_interrupt(int i
 	return(IRQ_HANDLED);
 }
 
-void time_init(void)
+static unsigned long read_persistent_clock(void)
 {
 	unsigned int year, mon, day, hour, min, sec;
 
@@ -111,10 +111,11 @@ void time_init(void)
 
 	if ((year += 1900) < 1970)
 		year += 100;
-	xtime.tv_sec = mktime(year, mon, day, hour, min, sec);
-	xtime.tv_nsec = 0;
-	wall_to_monotonic.tv_sec = -xtime.tv_sec;
+	return mktime(year, mon, day, hour, min, sec);
+}
 
+void time_init(void)
+{
 	mach_sched_init(timer_interrupt);
 }
 
diff --git a/arch/mips/kernel/time.c b/arch/mips/kernel/time.c
index 170cb67..abac10c 100644
--- a/arch/mips/kernel/time.c
+++ b/arch/mips/kernel/time.c
@@ -625,6 +625,11 @@ static unsigned int __init calibrate_hpt
 	return frequency >> log_2_loops;
 }
 
+unsigned long read_persistent_clock(void)
+{
+	return rtc_mips_get_time();
+}
+
 void __init time_init(void)
 {
 	if (board_time_init)
@@ -633,12 +638,6 @@ void __init time_init(void)
 	if (!rtc_mips_set_mmss)
 		rtc_mips_set_mmss = rtc_mips_set_time;
 
-	xtime.tv_sec = rtc_mips_get_time();
-	xtime.tv_nsec = 0;
-
-	set_normalized_timespec(&wall_to_monotonic,
-	                        -xtime.tv_sec, -xtime.tv_nsec);
-
 	/* Choose appropriate high precision timer routines.  */
 	if (!cpu_has_counter && !mips_hpt_read) {
 		/* No high precision timer -- sorry.  */
diff --git a/arch/parisc/kernel/time.c b/arch/parisc/kernel/time.c
index 5facc9b..c837d29 100644
--- a/arch/parisc/kernel/time.c
+++ b/arch/parisc/kernel/time.c
@@ -225,6 +225,13 @@ unsigned long long sched_clock(void)
 	return (unsigned long long)jiffies * (1000000000 / HZ);
 }
 
+unsigned long read_persistent_clock(void)
+{
+	struct tod_data;
+	if(pdc_tod_read(&tod_data))
+		return 0;
+	return tod.tod_sec;
+}
 
 void __init time_init(void)
 {
@@ -243,17 +250,5 @@ void __init time_init(void)
 	/* kick off Itimer (CR16) */
 	mtctl(next_tick, 16);
 
-	if(pdc_tod_read(&tod_data) == 0) {
-		write_seqlock_irq(&xtime_lock);
-		xtime.tv_sec = tod_data.tod_sec;
-		xtime.tv_nsec = tod_data.tod_usec * 1000;
-		set_normalized_timespec(&wall_to_monotonic,
-		                        -xtime.tv_sec, -xtime.tv_nsec);
-		write_sequnlock_irq(&xtime_lock);
-	} else {
-		printk(KERN_ERR "Error reading tod clock\n");
-	        xtime.tv_sec = 0;
-		xtime.tv_nsec = 0;
-	}
 }
 
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 774c0a3..c5e87eb 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -913,7 +913,7 @@ void __init generic_calibrate_decr(void)
 #endif
 }
 
-unsigned long get_boot_time(void)
+unsigned long read_persistent_clock(void)
 {
 	struct rtc_time tm;
 
@@ -1011,7 +1011,7 @@ void __init time_init(void)
 	tb_to_ns_scale = scale;
 	tb_to_ns_shift = shift;
 
-	tm = get_boot_time();
+	tm = read_persistent_clock();
 
 	write_seqlock_irqsave(&xtime_lock, flags);
 
diff --git a/arch/ppc/kernel/time.c b/arch/ppc/kernel/time.c
index 6ab8cc7..59b58e9 100644
--- a/arch/ppc/kernel/time.c
+++ b/arch/ppc/kernel/time.c
@@ -281,6 +281,14 @@ int do_settimeofday(struct timespec *tv)
 
 EXPORT_SYMBOL(do_settimeofday);
 
+unsigned long read_persistent_clock(void)
+{
+	int sec = 0;
+	if (ppc_md.get_rtc_time)
+		sec = ppc_md.get_rtc_time();
+	return sec;
+}
+
 /* This function is only called on the boot processor */
 void __init time_init(void)
 {
diff --git a/arch/s390/kernel/time.c b/arch/s390/kernel/time.c
index 74e6178..b1e62b4 100644
--- a/arch/s390/kernel/time.c
+++ b/arch/s390/kernel/time.c
@@ -341,6 +341,12 @@ void init_cpu_timer(void)
 
 extern void vtime_init(void);
 
+unsigned long read_persistent_clock(void)
+{
+	/* XXX I have no clue here. s390 folks, help! */
+	return 0;
+}
+
 /*
  * Initialize the TOD clock and the CPU timer of
  * the boot cpu.
diff --git a/arch/sh/kernel/time.c b/arch/sh/kernel/time.c
index a1589f8..cf5c5ec 100644
--- a/arch/sh/kernel/time.c
+++ b/arch/sh/kernel/time.c
@@ -161,6 +161,16 @@ device_initcall(timer_init_sysfs);
 
 void (*board_time_init)(void);
 
+unsigned long read_persistent_clock(void)
+{
+	struct timespec ts;
+	if (rtc_get_time) {
+		rtc_get_time(&ts);
+		return ts.tv_sec;
+	}
+	return 0;
+}
+
 void __init time_init(void)
 {
 	if (board_time_init)
@@ -168,16 +178,6 @@ void __init time_init(void)
 
 	clk_init();
 
-	if (rtc_get_time) {
-		rtc_get_time(&xtime);
-	} else {
-		xtime.tv_sec = mktime(2000, 1, 1, 0, 0, 0);
-		xtime.tv_nsec = 0;
-	}
-
-        set_normalized_timespec(&wall_to_monotonic,
-                                -xtime.tv_sec, -xtime.tv_nsec);
-
 	/*
 	 * Find the timer to use as the system timer, it will be
 	 * initialized for us.
diff --git a/arch/sh64/kernel/time.c b/arch/sh64/kernel/time.c
index b8162e5..d6feac5 100644
--- a/arch/sh64/kernel/time.c
+++ b/arch/sh64/kernel/time.c
@@ -487,6 +487,11 @@ static irqreturn_t sh64_rtc_interrupt(in
 static struct irqaction irq0  = { timer_interrupt, IRQF_DISABLED, CPU_MASK_NONE, "timer", NULL, NULL};
 static struct irqaction irq1  = { sh64_rtc_interrupt, IRQF_DISABLED, CPU_MASK_NONE, "rtc", NULL, NULL};
 
+unsigned long read_persistent_clock(void)
+{
+	return get_rtc_time();
+}
+
 void __init time_init(void)
 {
 	unsigned int cpu_clock, master_clock, bus_clock, module_clock;
@@ -511,9 +516,6 @@ void __init time_init(void)
 		panic("Unable to remap CPRC\n");
 	}
 
-	xtime.tv_sec = get_rtc_time();
-	xtime.tv_nsec = 0;
-
 	setup_irq(TIMER_IRQ, &irq0);
 	setup_irq(RTC_IRQ, &irq1);
 
diff --git a/arch/sparc/kernel/time.c b/arch/sparc/kernel/time.c
index 845081b..21c25ea 100644
--- a/arch/sparc/kernel/time.c
+++ b/arch/sparc/kernel/time.c
@@ -366,6 +366,12 @@ static int __init clock_init(void)
 fs_initcall(clock_init);
 #endif /* !CONFIG_SUN4 */
 
+unsigned long read_persistent_clock(void)
+{
+	/* XXX - help! not quite sure what to do here. */
+	return 0;
+}
+
 void __init sbus_time_init(void)
 {
 
diff --git a/arch/sparc64/kernel/time.c b/arch/sparc64/kernel/time.c
index 094d3e3..df48592 100644
--- a/arch/sparc64/kernel/time.c
+++ b/arch/sparc64/kernel/time.c
@@ -606,23 +606,9 @@ static int __init has_low_battery(void)
 	return (data1 == data2);	/* Was the write blocked? */
 }
 
-/* Probe for the real time clock chip. */
-static void __init set_system_time(void)
+unsigned long read_persistent_clock(void)
 {
-	unsigned int year, mon, day, hour, min, sec;
 	void __iomem *mregs = mstk48t02_regs;
-#ifdef CONFIG_PCI
-	unsigned long dregs = ds1287_regs;
-#else
-	unsigned long dregs = 0UL;
-#endif
-	u8 tmp;
-
-	if (!mregs && !dregs) {
-		prom_printf("Something wrong, clock regs not mapped yet.\n");
-		prom_halt();
-	}		
-
 	if (mregs) {
 		spin_lock_irq(&mostek_lock);
 
@@ -637,6 +623,7 @@ static void __init set_system_time(void)
 		day = MSTK_REG_DOM(mregs);
 		mon = MSTK_REG_MONTH(mregs);
 		year = MSTK_CVT_YEAR( MSTK_REG_YEAR(mregs) );
+		spin_unlock_irq(&mostek_lock);
 	} else {
 		/* Dallas 12887 RTC chip. */
 
@@ -649,7 +636,8 @@ static void __init set_system_time(void)
 			year = CMOS_READ(RTC_YEAR);
 		} while (sec != CMOS_READ(RTC_SECONDS));
 
-		if (!(CMOS_READ(RTC_CONTROL) & RTC_DM_BINARY) || RTC_ALWAYS_BCD) {
+		if (!(CMOS_READ(RTC_CONTROL) & RTC_DM_BINARY)
+				|| RTC_ALWAYS_BCD) {
 			BCD_TO_BIN(sec);
 			BCD_TO_BIN(min);
 			BCD_TO_BIN(hour);
@@ -660,13 +648,29 @@ static void __init set_system_time(void)
 		if ((year += 1900) < 1970)
 			year += 100;
 	}
+	return mktime(year, mon, day, hour, min, sec);
+}
 
-	xtime.tv_sec = mktime(year, mon, day, hour, min, sec);
-	xtime.tv_nsec = (INITIAL_JIFFIES % HZ) * (NSEC_PER_SEC / HZ);
-	set_normalized_timespec(&wall_to_monotonic,
- 	                        -xtime.tv_sec, -xtime.tv_nsec);
+/* Probe for the real time clock chip. */
+static void __init set_system_time(void)
+{
+	unsigned int year, mon, day, hour, min, sec;
+	void __iomem *mregs = mstk48t02_regs;
+#ifdef CONFIG_PCI
+	unsigned long dregs = ds1287_regs;
+#else
+	unsigned long dregs = 0UL;
+#endif
+	u8 tmp;
+
+	if (!mregs && !dregs) {
+		prom_printf("Something wrong, clock regs not mapped yet.\n");
+		prom_halt();
+	}
 
+	/* XXX - I probably messed this bit up. HELP! */
 	if (mregs) {
+		spin_lock_irq(&mostek_lock);
 		tmp = mostek_read(mregs + MOSTEK_CREG);
 		tmp &= ~MSTK_CREG_READ;
 		mostek_write(mregs + MOSTEK_CREG, tmp);
diff --git a/arch/um/kernel/time.c b/arch/um/kernel/time.c
index 552ca1c..85c56ae 100644
--- a/arch/um/kernel/time.c
+++ b/arch/um/kernel/time.c
@@ -123,13 +123,14 @@ static void register_timer(void)
 
 extern void (*late_time_init)(void);
 
+unsigned long read_persistent_clock(void)
+{
+	return (long)os_nsecs()/BILLION;
+}
+
 void time_init(void)
 {
 	long long nsecs;
-
-	nsecs = os_nsecs();
-	set_normalized_timespec(&wall_to_monotonic, -nsecs / BILLION,
-				-nsecs % BILLION);
 	late_time_init = register_timer;
 }
 
diff --git a/arch/v850/kernel/time.c b/arch/v850/kernel/time.c
index a0b4669..dea2753 100644
--- a/arch/v850/kernel/time.c
+++ b/arch/v850/kernel/time.c
@@ -184,8 +184,14 @@ static struct irqaction timer_irqaction 
 	NULL
 };
 
+unsigned long read_persistent_clock(void)
+{
+	struct timespec ts;
+	mach_gettimeofday(&ts);
+	return ts.tv_sec;
+}
+
 void time_init (void)
 {
-	mach_gettimeofday (&xtime);
 	mach_sched_init (&timer_irqaction);
 }
diff --git a/arch/x86_64/kernel/time.c b/arch/x86_64/kernel/time.c
index 7a9b182..343461b 100644
--- a/arch/x86_64/kernel/time.c
+++ b/arch/x86_64/kernel/time.c
@@ -511,7 +511,7 @@ unsigned long long sched_clock(void)
 	return cycles_2_ns(a);
 }
 
-static unsigned long get_cmos_time(void)
+static unsigned long read_persistent_clock(void)
 {
 	unsigned int year, mon, day, hour, min, sec;
 	unsigned long flags;
@@ -901,12 +901,6 @@ void __init time_init(void)
 	if (nohpet)
 		vxtime.hpet_address = 0;
 
-	xtime.tv_sec = get_cmos_time();
-	xtime.tv_nsec = 0;
-
-	set_normalized_timespec(&wall_to_monotonic,
-	                        -xtime.tv_sec, -xtime.tv_nsec);
-
 	if (!hpet_init())
                 vxtime_hz = (FSEC_PER_SEC + hpet_period / 2) / hpet_period;
 	else
@@ -1018,7 +1012,7 @@ static int timer_suspend(struct sys_devi
 	/*
 	 * Estimate time zone so that set_time can update the clock
 	 */
-	long cmos_time =  get_cmos_time();
+	long cmos_time = read_persistent_clock();
 
 	clock_cmos_diff = -cmos_time;
 	clock_cmos_diff += get_seconds();
@@ -1030,7 +1024,7 @@ static int timer_resume(struct sys_devic
 {
 	unsigned long flags;
 	unsigned long sec;
-	unsigned long ctime = get_cmos_time();
+	unsigned long ctime = read_persistent_clock();
 	unsigned long sleep_length = (ctime - sleep_start) * HZ;
 
 	if (vxtime.hpet_address)
diff --git a/arch/xtensa/kernel/time.c b/arch/xtensa/kernel/time.c
index 412ab32..3588484 100644
--- a/arch/xtensa/kernel/time.c
+++ b/arch/xtensa/kernel/time.c
@@ -56,10 +56,19 @@ static struct irqaction timer_irqaction 
 	.name =		"timer",
 };
 
-void __init time_init(void)
+
+unsigned long read_persistent_clock(void)
 {
 	time_t sec_o, sec_n = 0;
+	if (platform_get_rtc_time(&sec_o) == 0)
+		while (platform_get_rtc_time(&sec_n))
+			if (sec_o != sec_n)
+				break;
+	return sec_n;
+}
 
+void __init time_init(void)
+{
 	/* The platform must provide a function to calibrate the processor
 	 * speed for the CALIBRATE.
 	 */
@@ -71,20 +80,9 @@ void __init time_init(void)
 			(int)(ccount_per_jiffy/(10000/HZ))%100);
 #endif
 
-	/* Set time from RTC (if provided) */
-
-	if (platform_get_rtc_time(&sec_o) == 0)
-		while (platform_get_rtc_time(&sec_n))
-			if (sec_o != sec_n)
-				break;
-
-	xtime.tv_nsec = 0;
-	last_rtc_update = xtime.tv_sec = sec_n;
+	last_rtc_update = xtime.tv_sec;
 	last_ccount_stamp = get_ccount();
 
-	set_normalized_timespec(&wall_to_monotonic,
-		-xtime.tv_sec, -xtime.tv_nsec);
-
 	/* Initialize the linux timer interrupt. */
 
 	setup_irq(LINUX_TIMER_INT, &timer_irqaction);
diff --git a/kernel/timer.c b/kernel/timer.c
index b650f04..6ce2fd9 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -961,10 +961,16 @@ void __init timekeeping_init(void)
 	unsigned long flags;
 
 	write_seqlock_irqsave(&xtime_lock, flags);
+	ntp_clear();
+
 	clock = clocksource_get_next();
 	clocksource_calculate_interval(clock, tick_nsec);
 	clock->cycle_last = clocksource_read(clock);
-	ntp_clear();
+
+	xtime.tv_sec = read_persistent_clock();
+	xtime.tv_nsec = (INITIAL_JIFFIES % HZ) * (NSEC_PER_SEC / HZ);
+	set_normalized_timespec(&wall_to_monotonic,
+		-xtime.tv_sec, -xtime.tv_nsec);
 	write_sequnlock_irqrestore(&xtime_lock, flags);
 }
 

Re: [RFC][PATCH] Unify interface to persistent CMOS/RTC/whatever clock

[john stultz]

On Wed, 2006-08-16 at 15:45 -0700, john stultz wrote:
> My first pass at this can be found below. There are a few arches
> (specifically: m32r, s390, sparc, sparc64) where I just didn't know what
> to do, or where I suspect I didn't get it right, so I've CC'ed those
> maintainers for suggestions.

Just realized I skipped the ARM arches. I'll get that fixed (or atleast
make an attempt at it) before the next release.

thanks
-john

Re: [RFC][PATCH] Unify interface to persistent CMOS/RTC/whatever clock

[Martin Schwidefsky]

On Wed, 2006-08-16 at 15:45 -0700, john stultz wrote:
> My first pass at this can be found below. There are a few arches
> (specifically: m32r, s390, sparc, sparc64) where I just didn't know
> what
> to do, or where I suspect I didn't get it right, so I've CC'ed those
> maintainers for suggestions.

But it is sooo easy ;-)
See patch below.

-- 
blue skies,
  Martin.

Martin Schwidefsky
Linux for zSeries Development & Services
IBM Deutschland Entwicklung GmbH

"Reality continues to ruin my life." - Calvin.

---

 arch/s390/kernel/time.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff -urpN linux-2.6/arch/s390/kernel/time.c linux-2.6-clock/arch/s390/kernel/time.c
--- linux-2.6/arch/s390/kernel/time.c	2006-08-17 10:34:58.000000000 +0200
+++ linux-2.6-clock/arch/s390/kernel/time.c	2006-08-17 11:05:28.000000000 +0200
@@ -40,6 +40,9 @@
 #define USECS_PER_JIFFY     ((unsigned long) 1000000/HZ)
 #define CLK_TICKS_PER_JIFFY ((unsigned long) USECS_PER_JIFFY << 12)
 
+/* The value of the TOD clock for 1.1.1970. */
+#define TOD_UNIX_EPOCH 0x7d91048bca000000ULL
+
 /*
  * Create a small time difference between the timer interrupts
  * on the different cpus to avoid lock contention.
@@ -343,8 +346,9 @@ extern void vtime_init(void);
 
 unsigned long read_persistent_clock(void)
 {
-	/* XXX I have no clue here. s390 folks, help! */
-	return 0;
+	__u64 tod = (get_clock() - TOD_UNIX_EPOCH) >> 12;
+	do_div(tod, 1000000);
+	return (unsigned long) tod;
 }
 
 /*
@@ -353,7 +357,6 @@ unsigned long read_persistent_clock(void
  */
 void __init time_init(void)
 {
-	__u64 set_time_cc;
 	int cc;
 
         /* kick the TOD clock */
@@ -378,9 +381,7 @@ void __init time_init(void)
 
 	/* set xtime */
 	xtime_cc = init_timer_cc + CLK_TICKS_PER_JIFFY;
-	set_time_cc = init_timer_cc - 0x8126d60e46000000LL +
-		(0x3c26700LL*1000000*4096);
-        tod_to_timeval(set_time_cc, &xtime);
+        tod_to_timeval(init_timer_cc - TOD_UNIX_EPOCH, &xtime);
         set_normalized_timespec(&wall_to_monotonic,
                                 -xtime.tv_sec, -xtime.tv_nsec);
 

Re: [RFC][PATCH] Unify interface to persistent CMOS/RTC/whatever clock

[David Brownell]

>         Almost every arch has some form of persistent clock (CMOS, RTC, etc)
> which is normally used at boot time to initialize xtime and
> wall_to_monotonic. As part of the timekeeping consolidation, I propose
> the following generic interface to the arch specific persistent clock:

Hmm, this seems to ignore the RTC framework rather entirely, which
seems to me like the wrong implementation approach.  You'd likely have
noticed that if you had supported a few ARM boards.  :)

Here's a fairly common scenario for an embedded system:  the battery
backed clock is accessed through I2C or SPI, rather than an ISA style
direct register access, so it's not accessible until after those driver
stacks have initialized.

Or similarly, the SOC family may have a powerful RTC ("arch specific")
with alarm and system wakeup capabilities, but it may not be the system's
battery backed clock ... so that RTC would be initialized from another one,
which is accessed using I2C/SPI/etc.  (RTCs integrated on SOCs evidently
have a hard time being as power-miserly as the discrete ones.)

The approach in this patch doesn't seem to play well with those scenarios,
because it expects to do timekeeping setup long before the system's RTC is
necessarily going to be available ... and doesn't have an answer for those
cases where the RTC is unavailable before e.g. late_initcall().

I'd be more interested in something that improves on CONFIG_RTC_HCTOSYS,
and for example addresses the need to update the system wall time from
such RTCs after resume, not just at boot time.

- Dave

Re: [RFC][PATCH] Unify interface to persistent CMOS/RTC/whatever clock

[john stultz]

On Wed, 2006-08-16 at 22:47 -0700, David Brownell wrote:
> >         Almost every arch has some form of persistent clock (CMOS, RTC, etc)
> > which is normally used at boot time to initialize xtime and
> > wall_to_monotonic. As part of the timekeeping consolidation, I propose
> > the following generic interface to the arch specific persistent clock:
> 
> Hmm, this seems to ignore the RTC framework rather entirely, which
> seems to me like the wrong implementation approach.  You'd likely have
> noticed that if you had supported a few ARM boards.  :)

Good point. I've not spent enough time looking at the RTC code, and it
looks like it has potential to resolve some of the issues I'm looking
at. Although, how common is it really? Could we currently replace all
arches xtime initialization code with it? I suspect that would require
it to be built at all times. Would that be acceptable?

> Here's a fairly common scenario for an embedded system:  the battery
> backed clock is accessed through I2C or SPI, rather than an ISA style
> direct register access, so it's not accessible until after those driver
> stacks have initialized.

Yea. Although late initialization isn't that much of an issue in my
mind.

> Or similarly, the SOC family may have a powerful RTC ("arch specific")
> with alarm and system wakeup capabilities, but it may not be the system's
> battery backed clock ... so that RTC would be initialized from another one,
> which is accessed using I2C/SPI/etc.  (RTCs integrated on SOCs evidently
> have a hard time being as power-miserly as the discrete ones.)

SOC?

> The approach in this patch doesn't seem to play well with those scenarios,
> because it expects to do timekeeping setup long before the system's RTC is
> necessarily going to be available ... and doesn't have an answer for those
> cases where the RTC is unavailable before e.g. late_initcall().

That's fair. While the patch I just sent didn't consider these cases, I
don't think these scenarios break the intent of what I'm wanting to do.


> I'd be more interested in something that improves on CONFIG_RTC_HCTOSYS,
> and for example addresses the need to update the system wall time from
> such RTCs after resume, not just at boot time.

Agreed.

Basically what I'm trying to provide w/ read_persistent_clock() is:
1) A method for generic timekeeping to access a battery backed clock to
avoid potential suspend/resume ordering problems.
2) A generic interface to a low-res hardware driven clock for the
hangcheck-timer and other watchdog timers.
3) To cleanup the arch time initialization code and encapsulate xtime

Do you think the RTC interface is sufficient for this?

Thanks, I appreciate the feedback!
-john

Re: [RFC][PATCH] Unify interface to persistent CMOS/RTC/whatever clock

[David Brownell]

On Thursday 17 August 2006 12:08 pm, john stultz wrote:
> On Wed, 2006-08-16 at 22:47 -0700, David Brownell wrote:
> > 
> > Hmm, this seems to ignore the RTC framework rather entirely, which
> > seems to me like the wrong implementation approach.  You'd likely have
> > noticed that if you had supported a few ARM boards.  :)
> 
> Good point. I've not spent enough time looking at the RTC code, and it
> looks like it has potential to resolve some of the issues I'm looking
> at. Although, how common is it really?

Increasingly so; it solves a lot of reinvent-the-wheel problems, much
like the recent generic time-of-day updates are doing.  And just like
with clocksource updates, not all architectures have converted yet!


> Could we currently replace all 
> arches xtime initialization code with it?

Not yet, I think, mostly because RTCs are still being converted to
that framework.

For ACPI PCs, I converted drivers/char/rtc.c to that framework (see:
http://article.gmane.org/gmane.linux.kernel/427493) covering most
hardware over the last few years.


> I suspect that would require 
> it to be built at all times. Would that be acceptable?

It's just a question of _which_ code to build all the time, right?
Having it be standardized core code would seem appropriate.


> > Here's a fairly common scenario for an embedded system:  the battery
> > backed clock is accessed through I2C or SPI, rather than an ISA style
> > direct register access, so it's not accessible until after those driver
> > stacks have initialized.
> 
> Yea. Although late initialization isn't that much of an issue in my
> mind.

... except that unless I traced the code wrong, your current patch is
initializing the (wall) clock extremely early ... ergo my comment about
assuming ISA style access.  I'm happy to call that a minor implementation
artifact though, since I haven't noticed any issues with "late" init
for the system wall clock either.


> > Or similarly, the SOC family may have a powerful RTC ("arch specific")
> > with alarm and system wakeup capabilities, but it may not be the system's
> > battery backed clock ... so that RTC would be initialized from another one,
> > which is accessed using I2C/SPI/etc.  (RTCs integrated on SOCs evidently
> > have a hard time being as power-miserly as the discrete ones.)
> 
> SOC?

System-On-Chip ... one chip has not just CPU+cache, but also memory controllers
and enough I/O that few external chips are needed.  The one in your cell phone
is probably Linux-capable; the one in your microwave probably isn't.  (For some
sophisticated examples, www.omap.com ... click the left menu then look at the
chips' block diagrams to see what cell phone CPUs look like.)


> > The approach in this patch doesn't seem to play well with those scenarios,
> > because it expects to do timekeeping setup long before the system's RTC is
> > necessarily going to be available ... and doesn't have an answer for those
> > cases where the RTC is unavailable before e.g. late_initcall().
> 
> That's fair. While the patch I just sent didn't consider these cases, I
> don't think these scenarios break the intent of what I'm wanting to do.

Not in the big picture, nope.  Which is why I called it an implementation
issue.  I've also noticed the mess of arch/platform specific clock setup
wierdness, and was glad to see you start fixing it.  :)


> > I'd be more interested in something that improves on CONFIG_RTC_HCTOSYS,
> > and for example addresses the need to update the system wall time from
> > such RTCs after resume, not just at boot time.
> 
> Agreed.

There's some ARM code to restore the jiffies-vs-rtc clock offset on resume,
which seems like a candidate for wider use.


> Basically what I'm trying to provide w/ read_persistent_clock() is:
> 1) A method for generic timekeeping to access a battery backed clock to
> avoid potential suspend/resume ordering problems.
> 2) A generic interface to a low-res hardware driven clock for the
> hangcheck-timer and other watchdog timers.
> 3) To cleanup the arch time initialization code and encapsulate xtime
> 
> Do you think the RTC interface is sufficient for this?

I think the resume side needs some tweaks, but basically yes.  (The
resume sequencing, like the suspend sequencing, is excessively dumb.
It'd probably be good to resume RTCs before drivers that care about
elapsed time, for example ... )

I'm not entirely sure I see RTC as a clocksource, but maybe that'd
make sense...

- Dave

Re: [RFC][PATCH] Unify interface to persistent CMOS/RTC/whatever clock

[john stultz]

On Thu, 2006-08-17 at 13:28 -0700, David Brownell wrote:
> On Thursday 17 August 2006 12:08 pm, john stultz wrote:
> > On Wed, 2006-08-16 at 22:47 -0700, David Brownell wrote:
> > > 
> > > Hmm, this seems to ignore the RTC framework rather entirely, which
> > > seems to me like the wrong implementation approach.  You'd likely have
> > > noticed that if you had supported a few ARM boards.  :)
> > 
> > Good point. I've not spent enough time looking at the RTC code, and it
> > looks like it has potential to resolve some of the issues I'm looking
> > at. Although, how common is it really?
> 
> Increasingly so; it solves a lot of reinvent-the-wheel problems, much
> like the recent generic time-of-day updates are doing.  And just like
> with clocksource updates, not all architectures have converted yet!

Is a breakdown somewhere of what is covered at this point? The two
letters followed by six number driver files make it difficult to see
which arches use what. :)

> > Could we currently replace all 
> > arches xtime initialization code with it?
> 
> Not yet, I think, mostly because RTCs are still being converted to
> that framework.

With a list of arches that are covered, I'll start working on patches to
do so.


> For ACPI PCs, I converted drivers/char/rtc.c to that framework (see:
> http://article.gmane.org/gmane.linux.kernel/427493) covering most
> hardware over the last few years.

Cool.

> > I suspect that would require 
> > it to be built at all times. Would that be acceptable?
> 
> It's just a question of _which_ code to build all the time, right?
> Having it be standardized core code would seem appropriate.

Yea. We also need to make sure every arch has something that will work
w/o special compile time config options.


> > > Here's a fairly common scenario for an embedded system:  the battery
> > > backed clock is accessed through I2C or SPI, rather than an ISA style
> > > direct register access, so it's not accessible until after those driver
> > > stacks have initialized.
> > 
> > Yea. Although late initialization isn't that much of an issue in my
> > mind.
> 
> ... except that unless I traced the code wrong, your current patch is
> initializing the (wall) clock extremely early ... ergo my comment about
> assuming ISA style access.  I'm happy to call that a minor implementation
> artifact though, since I haven't noticed any issues with "late" init
> for the system wall clock either.

Indeed. If we cannot initialize xtime early, that's not an issue.
settimeofday() will fix that up. However the suspend/resume ordering
issue is more difficult. On resume we need to have something usable
before we resume the timekeeping code in order to correctly increment
xtime and jiffies in one atomic action.

> > > Or similarly, the SOC family may have a powerful RTC ("arch specific")
> > > with alarm and system wakeup capabilities, but it may not be the system's
> > > battery backed clock ... so that RTC would be initialized from another one,
> > > which is accessed using I2C/SPI/etc.  (RTCs integrated on SOCs evidently
> > > have a hard time being as power-miserly as the discrete ones.)
> > 
> > SOC?
> 
> System-On-Chip ... one chip has not just CPU+cache, but also memory controllers
> and enough I/O that few external chips are needed.  The one in your cell phone
> is probably Linux-capable; the one in your microwave probably isn't.  (For some
> sophisticated examples, www.omap.com ... click the left menu then look at the
> chips' block diagrams to see what cell phone CPUs look like.)

Thanks for the clarification.

With regards to more powerful RTCs, those I don't really care too much
about. I just want access the battery backed clock for suspend/resume
and watchdog uses.


> > > I'd be more interested in something that improves on CONFIG_RTC_HCTOSYS,
> > > and for example addresses the need to update the system wall time from
> > > such RTCs after resume, not just at boot time.
> > 
> > Agreed.
> 
> There's some ARM code to restore the jiffies-vs-rtc clock offset on resume,
> which seems like a candidate for wider use.

Where is this code? I'm interested here, but I suspect for correctness
that bit should be under the control of the timekeeping core.


> > Basically what I'm trying to provide w/ read_persistent_clock() is:
> > 1) A method for generic timekeeping to access a battery backed clock to
> > avoid potential suspend/resume ordering problems.
> > 2) A generic interface to a low-res hardware driven clock for the
> > hangcheck-timer and other watchdog timers.
> > 3) To cleanup the arch time initialization code and encapsulate xtime
> > 
> > Do you think the RTC interface is sufficient for this?
> 
> I think the resume side needs some tweaks, but basically yes.  (The
> resume sequencing, like the suspend sequencing, is excessively dumb.
> It'd probably be good to resume RTCs before drivers that care about
> elapsed time, for example ... )

Ok. I'm not totally sold on it, but I'll start looking in more detail
here. Starting w/ i386 and possibly moving across the rest of the
arches.


> I'm not entirely sure I see RTC as a clocksource, but maybe that'd
> make sense...

Ehh. That's definitely not the direction I'm trying to go (although I
guess one could possibly do so). I just want the 3 items above. :)


thanks
-john

Re: [RFC][PATCH] Unify interface to persistent CMOS/RTC/whatever clock

[David Brownell]

On Thursday 17 August 2006 2:42 pm, john stultz wrote:
> On Thu, 2006-08-17 at 13:28 -0700, David Brownell wrote:
> > On Thursday 17 August 2006 12:08 pm, john stultz wrote:
> > > On Wed, 2006-08-16 at 22:47 -0700, David Brownell wrote:
> > > > 
> > > > Hmm, this seems to ignore the RTC framework rather entirely, which
> > > > seems to me like the wrong implementation approach.  You'd likely have
> > > > noticed that if you had supported a few ARM boards.  :)
> > > 
> > > Good point. I've not spent enough time looking at the RTC code, and it
> > > looks like it has potential to resolve some of the issues I'm looking
> > > at. Although, how common is it really?
> > 
> > Increasingly so; it solves a lot of reinvent-the-wheel problems, much
> > like the recent generic time-of-day updates are doing.  And just like
> > with clocksource updates, not all architectures have converted yet!
> 
> Is a breakdown somewhere of what is covered at this point? The two
> letters followed by six number driver files make it difficult to see
> which arches use what. :)

A lot of those are part numbers for discrete chips, and some names are
for various ARM SOC families.

Seems to me the better question is what non-framework RTC drivers are still
in the tree ... and actually a good first pass at answering that is that
every platform touched in your original patch was NOT using the RTC class
framework.  (Not the answer you wanted, right?)  And drivers/char/*rtc* users.
There may be a few other RTCs elsewhere in the tree.

It seems all in-kernel ARM platforms except ARM Ltd's own "Integrator" use
the new framework instead of the older arch/arm/common/rtctime.c code (from
which the RTC framework evolved).  And I know there are other RTC drivers
and patches in the queue.  ARMs that haven't yet merged to mainline will,
as usual, pay the price for their perfidy.


> 	 However the suspend/resume ordering
> issue is more difficult. On resume we need to have something usable
> before we resume the timekeeping code in order to correctly increment
> xtime and jiffies in one atomic action.

Right; not something addressed in your original patch, or by many current
drivers AFAIK.  For the longest time, swsusp resume got that wrong...


> > > > I'd be more interested in something that improves on CONFIG_RTC_HCTOSYS,
> > > > and for example addresses the need to update the system wall time from
> > > > such RTCs after resume, not just at boot time.
> > > 
> > > Agreed.
> > 
> > There's some ARM code to restore the jiffies-vs-rtc clock offset on resume,
> > which seems like a candidate for wider use.
> 
> Where is this code? I'm interested here, but I suspect for correctness
> that bit should be under the control of the timekeeping core.

{save,restore}_time_delta() in

	include/asm-arm/mach/time.h
	arch/arm/kernel/time.c

The rtc-at91.c code is layered classically ... RTC is the first platform
device registered, and thus resumed, but it's resumed after the jiffies
timer sysdev (thus with IRQs enabled).  So it will restore the time delta
very early in the resume sequence.  (PXA uses a different approach.)

Presumably this is stuff that should be done by the RTC class resume()
method, probably for the CONFIG_RTC_HCTOSYS_DEVICE clock (though there
could be a better RTC ... one that's being NTP-corrected).  That'd
be no sooner than 2.6.19, which adds new class-level suspend()/resume()
calls to help offload individual drivers.


> > I'm not entirely sure I see RTC as a clocksource, but maybe that'd
> > make sense...
> 
> Ehh. That's definitely not the direction I'm trying to go (although I
> guess one could possibly do so). I just want the 3 items above. :)

Glad to hear it!  Though strictly speaking it _ought_ to make sense to
declare RTCs as clock sources.  They are clocks, after all.  I can even
imagine systems where the RTC issues (non-dynamic) tick interupts; some
RTCs (like those on PCs) are happy giving e.g 256 or 1024 irq/sec.

- Dave

Re: [RFC][PATCH] Unify interface to persistent CMOS/RTC/whatever clock

[john stultz]

On Thu, 2006-08-17 at 16:41 -0700, David Brownell wrote:
> On Thursday 17 August 2006 2:42 pm, john stultz wrote:
> > On Thu, 2006-08-17 at 13:28 -0700, David Brownell wrote:
> > > On Thursday 17 August 2006 12:08 pm, john stultz wrote:
> > > > On Wed, 2006-08-16 at 22:47 -0700, David Brownell wrote:
> > > > > 
> > > > > Hmm, this seems to ignore the RTC framework rather entirely, which
> > > > > seems to me like the wrong implementation approach.  You'd likely have
> > > > > noticed that if you had supported a few ARM boards.  :)
> > > > 
> > > > Good point. I've not spent enough time looking at the RTC code, and it
> > > > looks like it has potential to resolve some of the issues I'm looking
> > > > at. Although, how common is it really?
> > > 
> > > Increasingly so; it solves a lot of reinvent-the-wheel problems, much
> > > like the recent generic time-of-day updates are doing.  And just like
> > > with clocksource updates, not all architectures have converted yet!
> > 
> > Is a breakdown somewhere of what is covered at this point? The two
> > letters followed by six number driver files make it difficult to see
> > which arches use what. :)
> 
> A lot of those are part numbers for discrete chips, and some names are
> for various ARM SOC families.
> 
> Seems to me the better question is what non-framework RTC drivers are still
> in the tree ... and actually a good first pass at answering that is that
> every platform touched in your original patch was NOT using the RTC class
> framework.  (Not the answer you wanted, right?)  And drivers/char/*rtc* users.
> There may be a few other RTCs elsewhere in the tree.

Uh, so you're saying the RTC framework is basically ARM only?
Eek. You're right, that's not the answer I wanted to hear!

> It seems all in-kernel ARM platforms except ARM Ltd's own "Integrator" use
> the new framework instead of the older arch/arm/common/rtctime.c code (from
> which the RTC framework evolved).  And I know there are other RTC drivers
> and patches in the queue.  ARMs that haven't yet merged to mainline will,
> as usual, pay the price for their perfidy.
> 
> 
> > 	 However the suspend/resume ordering
> > issue is more difficult. On resume we need to have something usable
> > before we resume the timekeeping code in order to correctly increment
> > xtime and jiffies in one atomic action.
> 
> Right; not something addressed in your original patch, or by many current
> drivers AFAIK.  For the longest time, swsusp resume got that wrong...

True, I left it out of my patch since the generic suspend/resume takes a
bit more arch code then I had. You can find my current arch generic
portion of the patch (including suspend/resume adjustments) below.
Following this one would be the i386 patch that provides
read_persistent_clock() and removes all i386 arch specific timekeeping
adjustments (I can send it on request if needed).


> > > > > I'd be more interested in something that improves on CONFIG_RTC_HCTOSYS,
> > > > > and for example addresses the need to update the system wall time from
> > > > > such RTCs after resume, not just at boot time.
> > > > 
> > > > Agreed.
> > > 
> > > There's some ARM code to restore the jiffies-vs-rtc clock offset on resume,
> > > which seems like a candidate for wider use.
> > 
> > Where is this code? I'm interested here, but I suspect for correctness
> > that bit should be under the control of the timekeeping core.
> 
> {save,restore}_time_delta() in
> 
> 	include/asm-arm/mach/time.h
> 	arch/arm/kernel/time.c
> 
> The rtc-at91.c code is layered classically ... RTC is the first platform
> device registered, and thus resumed, but it's resumed after the jiffies
> timer sysdev (thus with IRQs enabled).  So it will restore the time delta
> very early in the resume sequence.  (PXA uses a different approach.)

Hmmm. So looking at the code, ARM doesn't update jiffies on resume?

> Presumably this is stuff that should be done by the RTC class resume()
> method, probably for the CONFIG_RTC_HCTOSYS_DEVICE clock (though there
> could be a better RTC ... one that's being NTP-corrected).  That'd
> be no sooner than 2.6.19, which adds new class-level suspend()/resume()
> calls to help offload individual drivers.

Hrm. I'm a bit skeptical that the RTC resume code should update the
timekeeping code instead of the timekeeping code doing it. It seems that
it would cause additional complexity (what if there are two RTC
devices?) and would still have some of the suspend/resume ordering
issues I'm worried about.

thanks
-john



 include/linux/time.h |    1 +
 kernel/timer.c       |   36 ++++++++++++++++++++++++++++++------
 2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/include/linux/time.h b/include/linux/time.h
index a5b7399..db31d2a 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -92,6 +92,7 @@ extern struct timespec xtime;
 extern struct timespec wall_to_monotonic;
 extern seqlock_t xtime_lock;
 
+extern unsigned long read_persistent_clock(void);
 void timekeeping_init(void);
 
 static inline unsigned long get_seconds(void)
diff --git a/kernel/timer.c b/kernel/timer.c
index b650f04..bbf4d99 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -47,6 +47,9 @@ static void time_interpolator_update(lon
 #define time_interpolator_update(x)
 #endif
 
+/* jiffies at the most recent update of wall time */
+unsigned long wall_jiffies = INITIAL_JIFFIES;
+
 u64 jiffies_64 __cacheline_aligned_in_smp = INITIAL_JIFFIES;
 
 EXPORT_SYMBOL(jiffies_64);
@@ -953,6 +956,14 @@ int timekeeping_is_continuous(void)
 	return ret;
 }
 
+/* Weak dummy function for arches that do not yet support it.
+ * XXX - Do be sure to remove it once all arches implement it.
+ */
+unsigned long __attribute__((weak)) read_persistent_clock(void)
+{
+	return 0;
+}
+
 /*
  * timekeeping_init - Initializes the clocksource and common timekeeping values
  */
@@ -961,15 +972,22 @@ void __init timekeeping_init(void)
 	unsigned long flags;
 
 	write_seqlock_irqsave(&xtime_lock, flags);
+	ntp_clear();
+
 	clock = clocksource_get_next();
 	clocksource_calculate_interval(clock, tick_nsec);
 	clock->cycle_last = clocksource_read(clock);
-	ntp_clear();
+
+	xtime.tv_sec = read_persistent_clock();
+	xtime.tv_nsec = (INITIAL_JIFFIES % HZ) * (NSEC_PER_SEC / HZ);
+	set_normalized_timespec(&wall_to_monotonic,
+		-xtime.tv_sec, -xtime.tv_nsec);
 	write_sequnlock_irqrestore(&xtime_lock, flags);
 }
 
 
 static int timekeeping_suspended;
+static unsigned long timekeeping_suspend_time;
 /*
  * timekeeping_resume - Resumes the generic timekeeping subsystem.
  * @dev:	unused
@@ -980,10 +998,18 @@ static int timekeeping_suspended;
  */
 static int timekeeping_resume(struct sys_device *dev)
 {
-	unsigned long flags;
+	unsigned long flags, now;
 
 	write_seqlock_irqsave(&xtime_lock, flags);
-	/* restart the last cycle value */
+	/* calculate time suspended using the persistent clock */
+	now = read_persistent_clock();
+	if (now && (now > timekeeping_suspend_time)) {
+		unsigned long sleep_length = now - timekeeping_suspend_time;
+		xtime.tv_sec += sleep_length;
+		jiffies_64 += sleep_length * HZ;
+		wall_jiffies += sleep_length * HZ;
+	}
+	/* re-base the last cycle value */
 	clock->cycle_last = clocksource_read(clock);
 	clock->error = 0;
 	timekeeping_suspended = 0;
@@ -997,6 +1023,7 @@ static int timekeeping_suspend(struct sy
 
 	write_seqlock_irqsave(&xtime_lock, flags);
 	timekeeping_suspended = 1;
+	timekeeping_suspend_time = read_persistent_clock();
 	write_sequnlock_irqrestore(&xtime_lock, flags);
 	return 0;
 }
@@ -1227,9 +1254,6 @@ static inline void calc_load(unsigned lo
 	}
 }
 
-/* jiffies at the most recent update of wall time */
-unsigned long wall_jiffies = INITIAL_JIFFIES;
-
 /*
  * This read-write spinlock protects us from races in SMP while
  * playing with xtime and avenrun.

Re: [RFC][PATCH] Unify interface to persistent CMOS/RTC/whatever clock

[David Brownell]

On Thursday 17 August 2006 5:07 pm, john stultz wrote:

> > Seems to me the better question is what non-framework RTC drivers are still
> > in the tree ... and actually a good first pass at answering that is that
> > every platform touched in your original patch was NOT using the RTC class
> > framework.  (Not the answer you wanted, right?)  And drivers/char/*rtc* users.
> > There may be a few other RTCs elsewhere in the tree.
> 
> Uh, so you're saying the RTC framework is basically ARM only?
> Eek. You're right, that's not the answer I wanted to hear!

That's not what I said, no ... I hope you don't think that only ARM
chips can use I2C or SPI to connect to battery backed RTCs!!

Maybe you should think of it more like "so far mostly embedded systems
use that framework" ... since it's embedded systems that need to cope
with that variety in hardware.  RTCs are not architectural features, but
are just implementation choices made during board or SOC design.  Since
custom designs are endemic to embedded hardware, that's where the need
for this RTC framework is critical.  A huge chunk of those markets use
ARM (it's the most popular embedded 32bit CPU), but I know various MIPS
and PPC boards also rely on those Linux RTC framework drivers.

The RTC framework is no more ARM-only than the generic TOD framework
is x86-only.  But those changes did start from different corners of the
Linux market, which likely explains some of the surprise associated with
this little collision.  (If rtc-acpi got a bit more attention, that'd
surely help raise awareness outside the embedded space...)


> > {save,restore}_time_delta() in
> > 
> > 	include/asm-arm/mach/time.h
> > 	arch/arm/kernel/time.c
> > 
> > The rtc-at91.c code is layered classically ... RTC is the first platform
> > device registered, and thus resumed, but it's resumed after the jiffies
> > timer sysdev (thus with IRQs enabled).  So it will restore the time delta
> > very early in the resume sequence.  (PXA uses a different approach.)
> 
> Hmmm. So looking at the code, ARM doesn't update jiffies on resume?

Now that you mention it ... that's right.  Just the wall clocks.  If you
want to argue that's buglike, take it up with Russell King.  Presumably
some other architectures do advance jiffies, not just the wall clock?


> > Presumably this is stuff that should be done by the RTC class resume()
> > method, probably for the CONFIG_RTC_HCTOSYS_DEVICE clock (though there
> > could be a better RTC ... one that's being NTP-corrected).  That'd
> > be no sooner than 2.6.19, which adds new class-level suspend()/resume()
> > calls to help offload individual drivers.
> 
> Hrm. I'm a bit skeptical that the RTC resume code should update the
> timekeeping code instead of the timekeeping code doing it. It seems that
> it would cause additional complexity (what if there are two RTC
> devices?) and would still have some of the suspend/resume ordering
> issues I'm worried about.

Could be.  "Two RTCs" scenarios are real, as I've mentioned before.  I'll
leave it to you to sort this stuff out, you seem to have a good handle
on it all ... I was mostly concerned that you incorporate the RTC class
support into your updates.  Neither of us created the mess surrounding
persistent wall clocks, but if you want to unify things, then you should
build on the existing RTC framework or else come up with a better one ...

- Dave

Re: [RFC][PATCH] Unify interface to persistent CMOS/RTC/whatever clock

[john stultz]

On Thu, 2006-08-17 at 18:38 -0700, David Brownell wrote:
> On Thursday 17 August 2006 5:07 pm, john stultz wrote:
> 
> > > Seems to me the better question is what non-framework RTC drivers are still
> > > in the tree ... and actually a good first pass at answering that is that
> > > every platform touched in your original patch was NOT using the RTC class
> > > framework.  (Not the answer you wanted, right?)  And drivers/char/*rtc* users.
> > > There may be a few other RTCs elsewhere in the tree.
> > 
> > Uh, so you're saying the RTC framework is basically ARM only?
> > Eek. You're right, that's not the answer I wanted to hear!
> 
> That's not what I said, no ... I hope you don't think that only ARM
> chips can use I2C or SPI to connect to battery backed RTCs!!
> 
> Maybe you should think of it more like "so far mostly embedded systems
> use that framework" ... since it's embedded systems that need to cope
> with that variety in hardware.  RTCs are not architectural features, but
> are just implementation choices made during board or SOC design.  Since
> custom designs are endemic to embedded hardware, that's where the need
> for this RTC framework is critical.  A huge chunk of those markets use
> ARM (it's the most popular embedded 32bit CPU), but I know various MIPS
> and PPC boards also rely on those Linux RTC framework drivers.

No, I'm sorry, I realize the RTC interface has the potential to be more
widely used. I'm just a bit frustrated that in order to utilize the RTC
framework for what I'm trying to do, I have to first implement RTC
drivers for 90% of the arches. :(

Also the "RTC might not be available when you need it" issue makes it
uh.. difficult to use. :)

So if we go w/  the "it may not be available, so always assume it isn't"
way of thinking, it forces us to rely upon the RTC driver(s) to resume
time (which means every RTC, no matter how simple has to have
suspend/resume hooks and call settimeofday at least). 

I don't really like that uses-graph (I can imagine someone system not
resuming properly because they forgot to compile in the right RTC
driver). Also it doesn't resolve the timekeeing resume ordering issue
I'm trying to address.

But we might have to deal with it. Just to make sure we can balance this
properly, what is the percentage of RTC drivers where the might not be
available at timekeeping_resume()?  If it is small, it might be
reasonable to special case them (and by "special case", i *don't* mean
ignore :)


> The RTC framework is no more ARM-only than the generic TOD framework
> is x86-only.  But those changes did start from different corners of the
> Linux market, which likely explains some of the surprise associated with
> this little collision.  (If rtc-acpi got a bit more attention, that'd
> surely help raise awareness outside the embedded space...)

Looking at the rtc-acpi code, it describes itself as being AT compatible
(ie: The old CMOS clock), but its not clear if it requires ACPI or not
to work. Further, does it work on x86_64, or ia64 as well?

Another comment, drivers/rtc/ is a bit overwhelming. Same with the
Kconfig. Is there any way it could be broken up so arch-specific RTCs
aren't shown on arches that do not support them? Or maybe there should
be two classes, the arch-defined RTCs and then the generic ones?

Your thoughts?

> > > {save,restore}_time_delta() in
> > > 
> > > 	include/asm-arm/mach/time.h
> > > 	arch/arm/kernel/time.c
> > > 
> > > The rtc-at91.c code is layered classically ... RTC is the first platform
> > > device registered, and thus resumed, but it's resumed after the jiffies
> > > timer sysdev (thus with IRQs enabled).  So it will restore the time delta
> > > very early in the resume sequence.  (PXA uses a different approach.)
> > 
> > Hmmm. So looking at the code, ARM doesn't update jiffies on resume?
> 
> Now that you mention it ... that's right.  Just the wall clocks.  If you
> want to argue that's buglike, take it up with Russell King.  Presumably
> some other architectures do advance jiffies, not just the wall clock?

Well, its a "some arches do, some arches don't" discrepancy, rather then
a specific bug. It was my hope that by putting the logic in generic code
we would get consistent behavior across arches.

> > > Presumably this is stuff that should be done by the RTC class resume()
> > > method, probably for the CONFIG_RTC_HCTOSYS_DEVICE clock (though there
> > > could be a better RTC ... one that's being NTP-corrected).  That'd
> > > be no sooner than 2.6.19, which adds new class-level suspend()/resume()
> > > calls to help offload individual drivers.
> > 
> > Hrm. I'm a bit skeptical that the RTC resume code should update the
> > timekeeping code instead of the timekeeping code doing it. It seems that
> > it would cause additional complexity (what if there are two RTC
> > devices?) and would still have some of the suspend/resume ordering
> > issues I'm worried about.
> 
> Could be.  "Two RTCs" scenarios are real, as I've mentioned before.  I'll
> leave it to you to sort this stuff out, you seem to have a good handle
> on it all ... I was mostly concerned that you incorporate the RTC class
> support into your updates.  Neither of us created the mess surrounding
> persistent wall clocks, but if you want to unify things, then you should
> build on the existing RTC framework or else come up with a better one ...

I do appreciate you bringing these point up. I do think the RTC class
looks promising, but until its widely implemented, its difficult to
use/rely upon generically.

A thought: The approach taken w/ the generic timekeeping was that by
using the jiffies clocksource, we could stagger the transition. So the
clocksource code can be used generically, because all arches are
guaranteed to have atleast one clocksource. 

Is there some way we can take a similar approach, by combining the
read_persistent_clock() and the RTC framework? Basically creating a
dummy RTC that would be on every arch, implementing only the read method
which would call something like read_persistent_clock(). This way the
RTC infrastructure could be used on all arches, while the conversion was
still happening.

Alternatively it may be that the real solution is to either introduce
resume ordering levels or explicitly call timekeeping resume, instead of
leaving it to the sysclass logic.

thanks
-john

Re: [RFC][PATCH] Unify interface to persistent CMOS/RTC/whatever clock

[David Brownell]

On Friday 18 August 2006 4:36 pm, john stultz wrote:

> No, I'm sorry, I realize the RTC interface has the potential to be more
> widely used. I'm just a bit frustrated that in order to utilize the RTC
> framework for what I'm trying to do, I have to first implement RTC
> drivers for 90% of the arches. :(

Maybe not ... you could do a lot with just a hook function.  Your original
patch provided one hook (an arch-level function call) assuming the RTC is
always accessible with IRQs blocked.  The RTC framework could provide such
a similar hook too ... best done through a function pointer, though.  It
shouldn't be a kernel build error if there's no RTC; userspace can use an
external one via NTP, load a module later than you'd like, etc.

Then be sure to call that hook from some can-sleep context, and you're as
set for the boot/init issue as possible.  (That is, no luck if the RTC is
in a module loaded after init starts.)


> Also the "RTC might not be available when you need it" issue makes it
> uh.. difficult to use. :)

I can't see that "when you need it" thing.  Especially since you had
proposed that "late" wall clock init was not a problem ...


> So if we go w/  the "it may not be available, so always assume it isn't"
> way of thinking, it forces us to rely upon the RTC driver(s) to resume
> time (which means every RTC, no matter how simple has to have
> suspend/resume hooks and call settimeofday at least). 

No, that was the point of my comment about using the new class level
suspend/resume calls.  The RTC drivers wouldn't be responsible for
that; the RTC framework would be.  RTC drivers may still want the
suspend/resume hooks to make sure they issue system wakeup events,
and so on, but no longer for maintaining the wall clock.  I'll send
a patch (of the "it compiles" type) later.

 
> I don't really like that uses-graph (I can imagine someone system not
> resuming properly because they forgot to compile in the right RTC
> driver).

Without the right RTC support, they'd not be getting initial clock
setup right either ... same difference, same userspace fix (using
external RTC, via NTP etc).


> Also it doesn't resolve the timekeeping resume ordering issue 
> I'm trying to address.

Worth exploring that a bit more.  Exactly what is the issue?  I'm
not sure your first explanation came across in enough depth...


> But we might have to deal with it. Just to make sure we can balance this
> properly, what is the percentage of RTC drivers where the might not be
> available at timekeeping_resume()?  If it is small, it might be
> reasonable to special case them (and by "special case", i *don't* mean
> ignore :)

Basically 100%.  It's because timekeeping_resume() applies to a (fake)
sys_device, and the sys_device resume phase kicks in before "real"
drivers resume, and with IRQs blocked ... ergo before sleeping calls
can be issued (e.g. waiting for I2C or SPI access to complete).

The RTC drivers are all "real" device drivers, and the RTC framework
itself issues sleeping calls, like mutex_lock.


> > The RTC framework is no more ARM-only than the generic TOD framework
> > is x86-only.  But those changes did start from different corners of the
> > Linux market, which likely explains some of the surprise associated with
> > this little collision.  (If rtc-acpi got a bit more attention, that'd
> > surely help raise awareness outside the embedded space...)
> 
> Looking at the rtc-acpi code, it describes itself as being AT compatible
> (ie: The old CMOS clock), but its not clear if it requires ACPI or not
> to work. Further, does it work on x86_64, or ia64 as well?

It's called "rtc-acpi" since it requires ACPI ... in fact, PNPACPI is
what provides the driver model device it binds to.  Plus it hooks into
ACPI to get the wakeup function, and the extra register options.  I'm
running it on both i386 and x86_64.  So like I said, most modern PCs
should be able to run it just fine.  IA64 uses ACPI, so presumably it
should be able to use the driver too.

The core of the driver could be reused on some non-PC platforms, some
not-modern PCs, and on ACPI platforms without PNPACPI ... given the
addition of platform code to register a platform_device.  And for the
ACPI-but-not-PNPCACPI configs, wakeup and the extra registers could
still be used.

Someone would have to provide the relevant patches; I'd not mind if
the driver were renamed to e.g. "rtc-cmos" at that point.


> Another comment, drivers/rtc/ is a bit overwhelming. Same with the
> Kconfig. Is there any way it could be broken up so arch-specific RTCs
> aren't shown on arches that do not support them?

They aren't.  You won't see the various SOC's RTC unless configuring
a kernel for that SOC ...


> Or maybe there should 
> be two classes, the arch-defined RTCs and then the generic ones?
> 
> Your thoughts?

I CC'd Allesandro Zummo, maintaining the RTC framework ... I'm sure
he's noticed the explosion of drivers!  It's not so overwhelming as
drivers/net though... :)

Sectioning Kconfig seems to me like a reasonable and simple change,
first the integrated/arch RTCs (link them first too, so they'll be
"rtc0" when not modular) then the discrete ones ... maybe the I2C
ones first (in alphabetical order), then SPI ones, memory mapped
ones, etc.  Any janitors reading this thread?

But I don't see a need for multiple classes; they're still all just
RTCs, the type of implementation isn't supposed to matter at all.


> > > > {save,restore}_time_delta() in
> > > > 
> > > > 	include/asm-arm/mach/time.h
> > > > 	arch/arm/kernel/time.c
> > > > 
> > > > The rtc-at91.c code is layered classically ... RTC is the first platform
> > > > device registered, and thus resumed, but it's resumed after the jiffies
> > > > timer sysdev (thus with IRQs enabled).  So it will restore the time delta
> > > > very early in the resume sequence.  (PXA uses a different approach.)
> > > 
> > > Hmmm. So looking at the code, ARM doesn't update jiffies on resume?
> > 
> > Now that you mention it ... that's right.  Just the wall clocks.  If you
> > want to argue that's buglike, take it up with Russell King.  Presumably
> > some other architectures do advance jiffies, not just the wall clock?
> 
> Well, its a "some arches do, some arches don't" discrepancy, rather then
> a specific bug. It was my hope that by putting the logic in generic code
> we would get consistent behavior across arches.

I'm all for that!


> > > > Presumably this is stuff that should be done by the RTC class resume()
> > > > method, probably for the CONFIG_RTC_HCTOSYS_DEVICE clock (though there
> > > > could be a better RTC ... one that's being NTP-corrected).  That'd
> > > > be no sooner than 2.6.19, which adds new class-level suspend()/resume()
> > > > calls to help offload individual drivers.
> > > 
> > > Hrm. I'm a bit skeptical that the RTC resume code should update the
> > > timekeeping code instead of the timekeeping code doing it. It seems that
> > > it would cause additional complexity (what if there are two RTC
> > > devices?) and would still have some of the suspend/resume ordering
> > > issues I'm worried about.
> > 
> > Could be.  "Two RTCs" scenarios are real, as I've mentioned before.  I'll
> > leave it to you to sort this stuff out, you seem to have a good handle
> > on it all ... I was mostly concerned that you incorporate the RTC class
> > support into your updates.  Neither of us created the mess surrounding
> > persistent wall clocks, but if you want to unify things, then you should
> > build on the existing RTC framework or else come up with a better one ...
> 
> I do appreciate you bringing these point up. I do think the RTC class
> looks promising, but until its widely implemented, its difficult to
> use/rely upon generically.

Every Linux system I work on today -- x86, x86_64, multiple ARMs -- supports
the RTC framework though.  To me it's *already* "widely implemented" ... :)


> A thought: The approach taken w/ the generic timekeeping was that by
> using the jiffies clocksource, we could stagger the transition. So the
> clocksource code can be used generically, because all arches are
> guaranteed to have atleast one clocksource. 
> 
> Is there some way we can take a similar approach, by combining the
> read_persistent_clock() and the RTC framework? Basically creating a
> dummy RTC that would be on every arch, implementing only the read method
> which would call something like read_persistent_clock(). This way the
> RTC infrastructure could be used on all arches, while the conversion was
> still happening.

What I sketched above is somewhat similar ... there couldn't be an
RTC framework driver, since the platform would need to provide a
driver model device in order to use the RTC framework.  And as I've
already noted, "every arch" can't implement such a method...

But it would be easy for read_persistent_clock() to be a pointer to a
function that's filled in by whatever -- non-RTC arch code, or the RTC
framework -- and called some appropriate can-sleep place.


> Alternatively it may be that the real solution is to either introduce
> resume ordering levels or explicitly call timekeeping resume, instead of
> leaving it to the sysclass logic.

The fact that timekeeping_resume ignores its parameter is IMNSHO a good
hint that it shouldn't be shoehorned into the driver model in that way.
Now, the RTC framework _does_ fit well into the driver model ... :)

- Dave

Re: [RFC][PATCH] Unify interface to persistent CMOS/RTC/whatever clock

[David Brownell]

On Saturday 19 August 2006 9:39 am, David Brownell wrote:
> > So if we go w/  the "it may not be available, so always assume it isn't"
> > way of thinking, it forces us to rely upon the RTC driver(s) to resume
> > time (which means every RTC, no matter how simple has to have
> > suspend/resume hooks and call settimeofday at least). 
> 
> No, that was the point of my comment about using the new class level
> suspend/resume calls.  The RTC drivers wouldn't be responsible for
> that; the RTC framework would be.  RTC drivers may still want the
> suspend/resume hooks to make sure they issue system wakeup events,
> and so on, but no longer for maintaining the wall clock.  I'll send
> a patch (of the "it compiles" type) later.

Welcome to "later"!

------------------------

Preliminary RTC class suspend/resume support:

 - Inlining the same code used by ARM, save and restore the delta between
   a selected RTC and the current system time.

 - Removes calls to that ARM code from the AT91 and S3C RTCs; the AT91
   calls are left as stubs, because a pending patch still needs those
   (currently empty) routines to properly issue system wakeup events.

 - Selects rtc0 by default, else CONFIG_RTC_HCTOSYS_DEVICE.  We expect all RTCs
   to be powered during "real" sleep states, but swsusp/hibernation requires
   something that's also battery-backed.

Depends on new class suspend/resume methods, added by a patch in the MM tree.

(Preliminary because untested, and because of a FIXME ... nothing recovers
from unregistering the selected RTC; that could happen only with badly written
RTC drivers though.)


Index: g26/drivers/rtc/class.c
===================================================================
--- g26.orig/drivers/rtc/class.c	2006-08-19 09:20:36.000000000 -0700
+++ g26/drivers/rtc/class.c	2006-08-19 09:20:37.000000000 -0700
@@ -29,6 +29,86 @@ static void rtc_device_release(struct cl
 	kfree(rtc);
 }
 
+#ifdef	CONFIG_PM
+
+/*
+ * Re-initialize wall clock on resume, to match the delta (calculated
+ * during suspend) between that and an appropriate RTC.
+ *
+ * We assume any RTC is good enough, meaning it stays running during
+ * system sleep states; but prefer the CONFIG_RTC_HCTOSYS_DEVICE on
+ * the grounds that it's expected to be battery-backed and thus will
+ * stay running even during the "off" states used by swsusp.
+ *
+ * REVISIT ... we should probably have a way to tell if a given RTC
+ * will stay powered during the target system state, rather than just
+ * assume a "valid" configuration (where no RTC that powers off will
+ * be selected, and no selected RTC will be removed).  That could let
+ * us get rid of the need for CONFIG_RTC_HCTOSYS_DEVICE too...
+ */
+
+static struct class_device	*sleep_rtc;
+static struct timespec		delta;
+
+static int rtc_suspend(struct device *dev, pm_message_t mesg)
+{
+	struct rtc_time tm;
+	struct timespec time;
+
+	if (!sleep_rtc || dev != sleep_rtc->dev)
+		return 0;
+
+	time.tv_nsec = 0;
+
+	rtc_read_time(sleep_rtc, &tm);
+	rtc_tm_to_time(&tm, &time.tv_sec);
+
+	set_normalized_timespec(&delta,
+				xtime.tv_sec - time.tv_sec,
+				xtime.tv_nsec - time.tv_nsec);
+
+	pr_debug("%s:  %s %4d-%02d-%02d %02d:%02d:%02d\n",
+		sleep_rtc->class_id, "suspend",
+		1900 + tm.tm_year, tm.tm_mon, tm.tm_mday,
+		tm.tm_hour, tm.tm_min, tm.tm_sec);
+
+	return 0;
+}
+
+static int rtc_resume(struct device *dev)
+{
+	struct rtc_time tm;
+	struct timespec time;
+
+	/* REVISIT some swsusp configurations may feed us an RTC that
+	 * lost power ... we should probably try detecting that, and
+	 * refuse to update the time using a bogus clock.
+	 */
+
+	if (!sleep_rtc || dev != sleep_rtc->dev)
+		return 0;
+
+	time.tv_nsec = 0;
+
+	rtc_read_time(sleep_rtc, &tm);
+	rtc_tm_to_time(&tm, &time.tv_sec);
+
+	set_normalized_timespec(&time,
+				xtime.tv_sec - time.tv_sec,
+				xtime.tv_nsec - time.tv_nsec);
+	do_settimeofday(&time);
+
+	pr_debug("%s:  %s %4d-%02d-%02d %02d:%02d:%02d\n",
+		sleep_rtc->class_id, "resume",
+		1900 + tm.tm_year, tm.tm_mon, tm.tm_mday,
+		tm.tm_hour, tm.tm_min, tm.tm_sec);
+
+	return 0;
+}
+
+#endif
+
+
 /**
  * rtc_device_register - register w/ RTC class
  * @dev: the device to register
@@ -85,6 +165,21 @@ struct rtc_device *rtc_device_register(c
 	if (err)
 		goto exit_kfree;
 
+#ifdef	CONFIG_PM
+	mutex_lock(&idr_lock);
+#ifdef CONFIG_RTC_HCTOSYS_DEVICE
+	if (sleep_rtc && strncmp(rtc->class_dev.class_id,
+				CONFIG_RTC_HCTOSYS_DEVICE,
+				BUS_ID_SIZE) == 0) {
+		rtc_class_close(sleep_rtc);
+		sleep_rtc = NULL;
+	}
+#endif
+	if (!sleep_rtc)
+		sleep_rtc = rtc_class_open(rtc->class_dev.class_id);
+	mutex_unlock(&idr_lock);
+#endif
+
 	dev_info(dev, "rtc core: registered %s as %s\n",
 			rtc->name, rtc->class_dev.class_id);
 
@@ -113,6 +208,17 @@ EXPORT_SYMBOL_GPL(rtc_device_register);
  */
 void rtc_device_unregister(struct rtc_device *rtc)
 {
+#ifdef	CONFIG_PM
+	mutex_lock(&idr_lock);
+	if (&rtc->class_dev == sleep_rtc) {
+		rtc_class_close(sleep_rtc);
+		sleep_rtc = NULL;
+		/* FIXME */
+		printk(KERN_WARNING "rtc: now what to use during sleep??\n");
+	}
+	mutex_unlock(&idr_lock);
+#endif
+
 	mutex_lock(&rtc->ops_lock);
 	rtc->ops = NULL;
 	mutex_unlock(&rtc->ops_lock);
@@ -134,6 +240,10 @@ static int __init rtc_init(void)
 		printk(KERN_ERR "%s: couldn't create class\n", __FILE__);
 		return PTR_ERR(rtc_class);
 	}
+#ifdef	CONFIG_PM
+	rtc_class->suspend = rtc_suspend;
+	rtc_class->resume = rtc_resume;
+#endif
 	return 0;
 }
 
Index: g26/drivers/rtc/rtc-at91.c
===================================================================
--- g26.orig/drivers/rtc/rtc-at91.c	2006-08-19 09:20:36.000000000 -0700
+++ g26/drivers/rtc/rtc-at91.c	2006-08-19 09:20:37.000000000 -0700
@@ -339,38 +339,13 @@ static struct timespec at91_rtc_delta;
 
 static int at91_rtc_suspend(struct platform_device *pdev, pm_message_t state)
 {
-	struct rtc_time tm;
-	struct timespec time;
-
-	time.tv_nsec = 0;
-
-	/* calculate time delta for suspend */
-	at91_rtc_readtime(&pdev->dev, &tm);
-	rtc_tm_to_time(&tm, &time.tv_sec);
-	save_time_delta(&at91_rtc_delta, &time);
-
-	pr_debug("%s(): %4d-%02d-%02d %02d:%02d:%02d\n", __FUNCTION__,
-		1900 + tm.tm_year, tm.tm_mon, tm.tm_mday,
-		tm.tm_hour, tm.tm_min, tm.tm_sec);
-
+	/* STUB, pending merge of rtc wakeup patch */
 	return 0;
 }
 
 static int at91_rtc_resume(struct platform_device *pdev)
 {
-	struct rtc_time tm;
-	struct timespec time;
-
-	time.tv_nsec = 0;
-
-	at91_rtc_readtime(&pdev->dev, &tm);
-	rtc_tm_to_time(&tm, &time.tv_sec);
-	restore_time_delta(&at91_rtc_delta, &time);
-
-	pr_debug("%s(): %4d-%02d-%02d %02d:%02d:%02d\n", __FUNCTION__,
-		1900 + tm.tm_year, tm.tm_mon, tm.tm_mday,
-		tm.tm_hour, tm.tm_min, tm.tm_sec);
-
+	/* STUB, pending merge of rtc wakeup patch */
 	return 0;
 }
 #else
Index: g26/drivers/rtc/rtc-s3c.c
===================================================================
--- g26.orig/drivers/rtc/rtc-s3c.c	2006-08-19 09:20:36.000000000 -0700
+++ g26/drivers/rtc/rtc-s3c.c	2006-08-19 09:20:37.000000000 -0700
@@ -536,37 +536,15 @@ static int ticnt_save;
 
 static int s3c_rtc_suspend(struct platform_device *pdev, pm_message_t state)
 {
-	struct rtc_time tm;
-	struct timespec time;
-
-	time.tv_nsec = 0;
-
 	/* save TICNT for anyone using periodic interrupts */
-
 	ticnt_save = readb(S3C2410_TICNT);
-
-	/* calculate time delta for suspend */
-
-	s3c_rtc_gettime(&pdev->dev, &tm);
-	rtc_tm_to_time(&tm, &time.tv_sec);
-	save_time_delta(&s3c_rtc_delta, &time);
 	s3c_rtc_enable(pdev, 0);
-
 	return 0;
 }
 
 static int s3c_rtc_resume(struct platform_device *pdev)
 {
-	struct rtc_time tm;
-	struct timespec time;
-
-	time.tv_nsec = 0;
-
 	s3c_rtc_enable(pdev, 1);
-	s3c_rtc_gettime(&pdev->dev, &tm);
-	rtc_tm_to_time(&tm, &time.tv_sec);
-	restore_time_delta(&s3c_rtc_delta, &time);
-
 	writeb(ticnt_save, S3C2410_TICNT);
 	return 0;
 }

Re: [RFC][PATCH] Unify interface to persistent CMOS/RTC/whatever clock

[David Brownell]

By the way, one furher observation:

> > Presumably this is stuff that should be done by the RTC class resume()
> > method, probably for the CONFIG_RTC_HCTOSYS_DEVICE clock (though there
> > could be a better RTC ... one that's being NTP-corrected).  That'd
> > be no sooner than 2.6.19, which adds new class-level suspend()/resume()
> > calls to help offload individual drivers.
> 
> Hrm. I'm a bit skeptical that the RTC resume code should update the
> timekeeping code instead of the timekeeping code doing it.

But how could timekeeping code know when the RTC is accessible again?

It might be unusable until after e.g. an I2C controller and then the RTC
are resumed (and maybe a parent to the I2C controller) ... the class
resume mechanism piggybacks on the normal resume sequencing, so doing it
that way would automatically resolve that issue.


> It seems that 
> it would cause additional complexity (what if there are two RTC
> devices?) and would still have some of the suspend/resume ordering
> issues I'm worried about.

The RTC framework currently "knows" which RTC is used for the hctosys
update, so that approach can be addressed by saving a single pointer
and adding some class suspend()/resume() code to test it.  No complexity
necessary; if it's good enough to set the initial system time, it's
good enough to set it again after resume.  (And maybe all the RTCs
should be getting NTP updates, not just one of them ...)

I think that if you keep in mind that scenario -- I2C used to access
the RTC clock -- and make sure it works, you'll notice a few details
of your current approach that need to be changed.

In particular, it is impossible for any sys_device's driver resume()
to access I2C (since that resume is done with irqs disabled, while i2c
controllers normally require irqs to run); but that's what the current
version of timekeeping_resume() in your patch would be doing.

- Dave