[RFC][PATCH 0/14] Convert remaining arches to read/update_persistent_clock

[RFC][PATCH 0/14] Convert remaining arches to read/update_persistent_clock

[john stultz]

So as the timekeeping system has become more and more generic, folks
have been careful to allow a slow and steady evolution without breaking
all the arches at once, allowing each arch maintainer to convert over to
generic code when they're ready.

However, this slow conversion has forced us to keep multiple methods for
various functionality around, cluttering up the code and making
maintenance more difficult. Further, there's no central road-map or
notification to maintainers on when these new generic functions appear,
so its likely folks wouldn't notice until the old interfaces were
removed.

In this case the generic read_persistent_clock() and
update_persistent_clock() methods have been provided to allow the
generic timekeeping code to initialize xtime and set the persistent
clock when NTP is synced. However many arches haven't converted, so the
generic code has to handle the case where the arch is doing this
management itself.

This patch series tries to convert the following 14 architectures over
to use read_persistent_clock() and update_persistent_clock() as
applicable, killing off about 200 lines of arch specific code.

I'm posting this tonight in somewhat rough form (none of the code has
been compiled or tested) so I can get feedback tomorrow before I'm off
on vacation until the new year. I'd like to get these changes into
2.6.34 so further generic cleanups can be made.

Let me know what you think.

thanks
-john

[RFC][PATCH 1/14] Convert alpha to read/update_persistent_clock

[john stultz]

This patch converts the alpha architecture to use the generic
read_persistent_clock and update_persistent_clock interfaces, reducing
the amount of arch specific code we have to maintain, and allowing for
further cleanups in the future.

I have not built or tested this patch, so help from arch maintainers
would be appreciated.

Signed-off-by: John Stultz <johnstul@us.ibm.com>
---
 Kconfig       |    3 +
 kernel/time.c |  101 ++++++++++++++++++++++++++--------------------------------
 2 files changed, 50 insertions(+), 54 deletions(-)

Index: gettimeoffset/arch/alpha/Kconfig
===================================================================
--- gettimeoffset.orig/arch/alpha/Kconfig	2009-12-22 18:50:55.000000000 -0800
+++ gettimeoffset/arch/alpha/Kconfig	2009-12-22 18:51:01.000000000 -0800
@@ -54,6 +54,9 @@ config ARCH_USES_GETTIMEOFFSET
 	bool
 	default y
 
+config GENERIC_CMOS_UPDATE
+        def_bool y
+
 config ZONE_DMA
 	bool
 	default y
Index: gettimeoffset/arch/alpha/kernel/time.c
===================================================================
--- gettimeoffset.orig/arch/alpha/kernel/time.c	2009-12-22 18:50:55.000000000 -0800
+++ gettimeoffset/arch/alpha/kernel/time.c	2009-12-22 18:51:01.000000000 -0800
@@ -75,8 +75,6 @@ static struct {
 	__u32 last_time;
 	/* ticks/cycle * 2^48 */
 	unsigned long scaled_ticks_per_cycle;
-	/* last time the CMOS clock got updated */
-	time_t last_rtc_update;
 	/* partial unused tick */
 	unsigned long partial_tick;
 } state;
@@ -91,6 +89,52 @@ static inline __u32 rpcc(void)
     return result;
 }
 
+int update_persistent_clock(struct timespec now)
+{
+	return set_rtc_mmss(now.tv_sec);
+}
+
+void read_persistent_clock(struct timespec *ts)
+{
+	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) {
+		sec = bcd2bin(sec);
+		min = bcd2bin(min);
+		hour = bcd2bin(hour);
+		day = bcd2bin(day);
+		mon = bcd2bin(mon);
+		year = bcd2bin(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;
+
+	ts->tv_sec = mktime(year, mon, day, hour, min, sec);
+}
+
+
+
 /*
  * timer_interrupt() needs to keep up the real-time clock,
  * as well as call the "do_timer()" routine every clocktick
@@ -123,19 +167,6 @@ irqreturn_t timer_interrupt(int irq, voi
 	if (nticks)
 		do_timer(nticks);
 
-	/*
-	 * If we have an externally synchronized Linux clock, then update
-	 * CMOS clock accordingly every ~11 minutes. Set_rtc_mmss() has to be
-	 * called as close as possible to 500 ms before the new second starts.
-	 */
-	if (ntp_synced()
-	    && xtime.tv_sec > state.last_rtc_update + 660
-	    && xtime.tv_nsec >= 500000 - ((unsigned) TICK_SIZE) / 2
-	    && xtime.tv_nsec <= 500000 + ((unsigned) TICK_SIZE) / 2) {
-		int tmp = set_rtc_mmss(xtime.tv_sec);
-		state.last_rtc_update = xtime.tv_sec - (tmp ? 600 : 0);
-	}
-
 	write_sequnlock(&xtime_lock);
 
 #ifndef CONFIG_SMP
@@ -304,7 +335,7 @@ rpcc_after_update_in_progress(void)
 void __init
 time_init(void)
 {
-	unsigned int year, mon, day, hour, min, sec, cc1, cc2, epoch;
+	unsigned int cc1, cc2;
 	unsigned long cycle_freq, tolerance;
 	long diff;
 
@@ -348,43 +379,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) {
-		sec = bcd2bin(sec);
-		min = bcd2bin(min);
-		hour = bcd2bin(hour);
-		day = bcd2bin(day);
-		mon = bcd2bin(mon);
-		year = bcd2bin(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);
@@ -394,7 +388,6 @@ time_init(void)
 	state.last_time = cc1;
 	state.scaled_ticks_per_cycle
 		= ((unsigned long) HZ << FIX_SHIFT) / cycle_freq;
-	state.last_rtc_update = 0;
 	state.partial_tick = 0L;
 
 	/* Startup the timer source. */

Re: [RFC][PATCH 0/14] Convert remaining arches to read/update_persistent_clock

[Paul Mundt]

On Tue, Dec 22, 2009 at 07:59:22PM -0800, john stultz wrote:
> In this case the generic read_persistent_clock() and
> update_persistent_clock() methods have been provided to allow the
> generic timekeeping code to initialize xtime and set the persistent
> clock when NTP is synced. However many arches haven't converted, so the
> generic code has to handle the case where the arch is doing this
> management itself.
> 
> This patch series tries to convert the following 14 architectures over
> to use read_persistent_clock() and update_persistent_clock() as
> applicable, killing off about 200 lines of arch specific code.
> 
While I think that this is a good goal, many of the underlying
architectures have veered pretty far away from having meaningful
persistent clock interfaces after having moved entirely to generic
timekeeping and the RTC subsystem.

In the case of SH at least that interface along with the generic CMOS
stuff is largely a stop-gap for antiquated platforms that don't have
proper RTC drivers and likely never will, while the default for all of
the rest of the platforms effectively returns a fixed dummy value. I
copied this approach from MIPS originally, so there are at least a few
architectures that this will apply to.

In any event, I wonder if it might make more sense to take something like
the SPARC implementation that is simply a wrapper around the RTC, move
that out in to a more generic place, and permit architectures to select
an RTC class backed persistent clock instead (it seems to be only
platforms that haven't caught up yet in terms of generic time and RTC
migration that would want to define this interface on their own at all at
this point)?

Re: [RFC][PATCH 0/14] Convert remaining arches to read/update_persistent_clock

[Geert Uytterhoeven]

On Wed, Dec 23, 2009 at 06:08, Paul Mundt <lethal@linux-sh.org> wrote:
> On Tue, Dec 22, 2009 at 07:59:22PM -0800, john stultz wrote:
>> In this case the generic read_persistent_clock() and
>> update_persistent_clock() methods have been provided to allow the
>> generic timekeeping code to initialize xtime and set the persistent
>> clock when NTP is synced. However many arches haven't converted, so the
>> generic code has to handle the case where the arch is doing this
>> management itself.
>>
>> This patch series tries to convert the following 14 architectures over
>> to use read_persistent_clock() and update_persistent_clock() as
>> applicable, killing off about 200 lines of arch specific code.
>>
> While I think that this is a good goal, many of the underlying
> architectures have veered pretty far away from having meaningful
> persistent clock interfaces after having moved entirely to generic
> timekeeping and the RTC subsystem.

Indeed. When moving to the RTC subsystem, you loose the persistent
clock at boot;
i.e. on m68k, mach_hwclk() can no longer be set, as the RTC driver is
in a separate
(possible loadable) module.

> In the case of SH at least that interface along with the generic CMOS
> stuff is largely a stop-gap for antiquated platforms that don't have
> proper RTC drivers and likely never will, while the default for all of
> the rest of the platforms effectively returns a fixed dummy value. I
> copied this approach from MIPS originally, so there are at least a few
> architectures that this will apply to.
>
> In any event, I wonder if it might make more sense to take something like
> the SPARC implementation that is simply a wrapper around the RTC, move
> that out in to a more generic place, and permit architectures to select
> an RTC class backed persistent clock instead (it seems to be only
> platforms that haven't caught up yet in terms of generic time and RTC
> migration that would want to define this interface on their own at all at
> this point)?

Hmm, haven't looked into how SPARC handles this yet...
Yes, looks like a good idea to me. Any disadvantages with this approach?

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [RFC][PATCH 0/14] Convert remaining arches to read/update_persistent_clock

[john stultz]

On Wed, 2009-12-23 at 14:08 +0900, Paul Mundt wrote:
> On Tue, Dec 22, 2009 at 07:59:22PM -0800, john stultz wrote:
> > In this case the generic read_persistent_clock() and
> > update_persistent_clock() methods have been provided to allow the
> > generic timekeeping code to initialize xtime and set the persistent
> > clock when NTP is synced. However many arches haven't converted, so the
> > generic code has to handle the case where the arch is doing this
> > management itself.
> > 
> > This patch series tries to convert the following 14 architectures over
> > to use read_persistent_clock() and update_persistent_clock() as
> > applicable, killing off about 200 lines of arch specific code.
> > 
> While I think that this is a good goal, many of the underlying
> architectures have veered pretty far away from having meaningful
> persistent clock interfaces after having moved entirely to generic
> timekeeping and the RTC subsystem.
> 
> In the case of SH at least that interface along with the generic CMOS
> stuff is largely a stop-gap for antiquated platforms that don't have
> proper RTC drivers and likely never will, while the default for all of
> the rest of the platforms effectively returns a fixed dummy value. I
> copied this approach from MIPS originally, so there are at least a few
> architectures that this will apply to.
> 
> In any event, I wonder if it might make more sense to take something like
> the SPARC implementation that is simply a wrapper around the RTC, move
> that out in to a more generic place, and permit architectures to select
> an RTC class backed persistent clock instead (it seems to be only
> platforms that haven't caught up yet in terms of generic time and RTC
> migration that would want to define this interface on their own at all at
> this point)?

Yea, there's some additional complications with the RTC interface via
read_persistent_clock, as some RTC clocks require irqs enabled to be
able to read. This keeps them from being used via
read_persistent_clock() as it is used prior to irqs being enabled, and
then again with irqs disabled in the suspend and resume path.

This has a bit of a trade-off, as we can better handle timekeeping
around a suspend/resume with read_persistent_clock(), but for some
hardware we just can't use the RTC for that.

Anyway, if we can improve the timekeeping/RTC interface used for
initialization and suspend/resume, I'm all for it (maybe having the RTC
code to tell the timekeeping code if it can be accessed with irqs off?).
But for hardware that needs irqs, i'm not sure how we can handle resumes
correctly there. So suggestions would be welcome.

Anyway, the main point of this patch set is to remove the direct access
to timekeeping internals (xtime, wall_to_monotonic). Those need to go
soon, as they're limiting changes in the timekeeping code.
read_persistent_clock() is the current way to avoid it, but if systems
are fine doing a settimeofday() at init that's ok too (although some
oddities may be seen wrt boot time with the direct settimeofday, I need
to refresh my head on how the varying default boot times between arches
may be effected).

thanks
-john

Re: [RFC][PATCH 0/14] Convert remaining arches to read/update_persistent_clock

[Dialup Jon Norstog]

john,

IMO the biggest clock problem on Alpha is that dual booting Tru64 and Linux 
whacks the clock every time you go from one OS to the other.  Allowing Linux 
to decouple from SRM's TOY clock would be a signal advance.

jon norstog


---------- Original Message -----------
From: john stultz <johnstul@us.ibm.com>
To: Paul Mundt <lethal@linux-sh.org>
Cc: lkml <linux-kernel@vger.kernel.org>, Richard Henderson 
<rth@twiddle.net>, linux-alpha@vger.kernel.org, linux-sh@vger.kernel.org, 
Russell King <linux@arm.linux.org.uk>, Haavard Skinnemoen 
<hskinnemoen@atmel.com>, Mike Frysinger <vapier@gentoo.org>, Mikael Starvik 
<starvik@axis.com>, David Howells <dhowells@redhat.com>, Yoshinori Sato 
<ysato@users.sourceforge.jp>, Tony Luck <tony.luck@intel.com>, Hirokazu 
Takata <takata@linux-m32r.org>, Geert Uytterhoeven <geert@linux-m68k.org>, 
Koichi Yasutake <yasutake.koichi@jp.panasonic.com>, Kyle McMartin 
<kyle@mcmartin.ca>, "David S. Miller" <davem@davemloft.net>, David Brownell 
<dbrownell@users.sourceforge.net>
Sent: Wed, 23 Dec 2009 14:04:59 -0800
Subject: Re: [RFC][PATCH 0/14] Convert remaining arches to 
read/update_persistent_clock

> On Wed, 2009-12-23 at 14:08 +0900, Paul Mundt wrote:
> > On Tue, Dec 22, 2009 at 07:59:22PM -0800, john stultz wrote:
> > > In this case the generic read_persistent_clock() and
> > > update_persistent_clock() methods have been provided to allow the
> > > generic timekeeping code to initialize xtime and set the persistent
> > > clock when NTP is synced. However many arches haven't converted, so the
> > > generic code has to handle the case where the arch is doing this
> > > management itself.
> > > 
> > > This patch series tries to convert the following 14 architectures over
> > > to use read_persistent_clock() and update_persistent_clock() as
> > > applicable, killing off about 200 lines of arch specific code.
> > > 
> > While I think that this is a good goal, many of the underlying
> > architectures have veered pretty far away from having meaningful
> > persistent clock interfaces after having moved entirely to generic
> > timekeeping and the RTC subsystem.
> > 
> > In the case of SH at least that interface along with the generic CMOS
> > stuff is largely a stop-gap for antiquated platforms that don't have
> > proper RTC drivers and likely never will, while the default for all of
> > the rest of the platforms effectively returns a fixed dummy value. I
> > copied this approach from MIPS originally, so there are at least a few
> > architectures that this will apply to.
> > 
> > In any event, I wonder if it might make more sense to take something like
> > the SPARC implementation that is simply a wrapper around the RTC, move
> > that out in to a more generic place, and permit architectures to select
> > an RTC class backed persistent clock instead (it seems to be only
> > platforms that haven't caught up yet in terms of generic time and RTC
> > migration that would want to define this interface on their own at all at
> > this point)?
> 
> Yea, there's some additional complications with the RTC interface via
> read_persistent_clock, as some RTC clocks require irqs enabled to be
> able to read. This keeps them from being used via
> read_persistent_clock() as it is used prior to irqs being enabled, 
> and then again with irqs disabled in the suspend and resume path.
> 
> This has a bit of a trade-off, as we can better handle timekeeping
> around a suspend/resume with read_persistent_clock(), but for some
> hardware we just can't use the RTC for that.
> 
> Anyway, if we can improve the timekeeping/RTC interface used for
> initialization and suspend/resume, I'm all for it (maybe having the RTC
> code to tell the timekeeping code if it can be accessed with irqs 
> off?). But for hardware that needs irqs, i'm not sure how we can 
> handle resumes correctly there. So suggestions would be welcome.
> 
> Anyway, the main point of this patch set is to remove the direct access
> to timekeeping internals (xtime, wall_to_monotonic). Those need to go
> soon, as they're limiting changes in the timekeeping code.
> read_persistent_clock() is the current way to avoid it, but if 
> systems are fine doing a settimeofday() at init that's ok too 
> (although some oddities may be seen wrt boot time with the direct 
> settimeofday, I need to refresh my head on how the varying default 
> boot times between arches may be effected).
> 
> thanks
> -john
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-
> alpha" in the body of a message to majordomo@vger.kernel.org More 
> majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> -- 
> This message has been scanned for viruses and
> dangerous content by MailScanner, and is
> believed to be clean.
------- End of Original Message -------

Re: [RFC][PATCH 0/14] Convert remaining arches to read/update_persistent_clock

[David Miller]

From: Paul Mundt <lethal@linux-sh.org>
Date: Wed, 23 Dec 2009 14:08:10 +0900

> In any event, I wonder if it might make more sense to take something like
> the SPARC implementation that is simply a wrapper around the RTC, move
> that out in to a more generic place, and permit architectures to select
> an RTC class backed persistent clock instead (it seems to be only
> platforms that haven't caught up yet in terms of generic time and RTC
> migration that would want to define this interface on their own at all at
> this point)?

This sounds nice but don't we have a slew of RTC types that need
to be accessed over I2C and thus you can't touch them without
sleeping?

Re: [RFC][PATCH 0/14] Convert remaining arches to read/update_persistent_clock

[Paul Mundt]

On Wed, Dec 23, 2009 at 08:54:15PM -0800, David Miller wrote:
> From: Paul Mundt <lethal@linux-sh.org>
> Date: Wed, 23 Dec 2009 14:08:10 +0900
> 
> > In any event, I wonder if it might make more sense to take something like
> > the SPARC implementation that is simply a wrapper around the RTC, move
> > that out in to a more generic place, and permit architectures to select
> > an RTC class backed persistent clock instead (it seems to be only
> > platforms that haven't caught up yet in terms of generic time and RTC
> > migration that would want to define this interface on their own at all at
> > this point)?
> 
> This sounds nice but don't we have a slew of RTC types that need
> to be accessed over I2C and thus you can't touch them without
> sleeping?

Yes, and SPI and so on. We do however have plenty of available room for
adding a valid-for-persistent-clock flag to permit drivers to opt-in, so
we can certainly still do better than the status quo. I'll hack something
up and see how it goes.