[patch] Generic time fixes

[patch] Generic time fixes

[Maciej W. Rozycki]

Hello,

 In preparation to merging DECstation's time support with the generic
version I did the following clean-ups to generic time support.  Most of
these are coding style fixes (which might have already been fixed by
someone else during past two days -- I haven't been able to study the
changes), but there are a few code changes (detailed below) as well.

 Before I proceed further I need to get an aswer to the following
question: why do we use rtc_set_time() for NTP RTC updates instead of
rtc_set_mmss() like most other architectures do?  Traditionally Linux only
updated minutes and seconds in this context and I don't think we need to
do anything more.  And setting minutes and seconds only is way, way
faster. Which might not matter that much every 11 minutes, except doing
things slowly here incurs a disruption in the latency of the timer
interrupt, which NTP might not like and the slow calculation of the RTC
time causes less precise time being stored in the RTC chip. 

 It's already questionable whether the update should be done at all (this
was discussed zillion of times at the NTP group) and it disrupts
timekeeping of the DECstation severely, but given the current choice, I'd
prefer to make it as lightweight as possible.

 Here are the changes done:

1. Explicit variable initializations that used zero or NULL are removed.

2. Inline assembly returns a result in "hi" instead of doing an explicit
"mfhi"; clobbers for "lo" and "accum" are added.

3. local_timer_interrupt() is called with arguments passed from the above
instead of fake ones (although "irq" and "dev_id" could be completely
removed here; in the next iteration, I suppose). 

4. ISO C initializers for timer_irqaction; improves readability.

5. Leap years are handled properly.

6. Missing "extern" qualifiers for function declarations are added.

 OK to apply?

  Maciej

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

patch-mips-2.4.21-20030711-mips-time-0
diff -up --recursive --new-file linux-mips-2.4.21-20030711.macro/arch/mips/kernel/time.c linux-mips-2.4.21-20030711/arch/mips/kernel/time.c
--- linux-mips-2.4.21-20030711.macro/arch/mips/kernel/time.c	2003-04-23 02:56:41.000000000 +0000
+++ linux-mips-2.4.21-20030711/arch/mips/kernel/time.c	2003-07-21 20:28:39.000000000 +0000
@@ -1,9 +1,10 @@
 /*
  * Copyright 2001 MontaVista Software Inc.
  * Author: Jun Sun, jsun@mvista.com or jsun@junsun.net
+ * Copyright (c) 2003  Maciej W. Rozycki
  *
  * Common time service routines for MIPS machines. See
- * Documents/MIPS/README.txt.
+ * Documentation/mips/time.README.
  *
  * This program is free software; you can redistribute  it and/or modify it
  * under  the terms of  the GNU General  Public License as published by the
@@ -70,7 +71,7 @@ void do_gettimeofday(struct timeval *tv)
 {
 	unsigned long flags;
 
-	read_lock_irqsave (&xtime_lock, flags);
+	read_lock_irqsave(&xtime_lock, flags);
 	*tv = xtime;
 	tv->tv_usec += do_gettimeoffset();
 
@@ -81,7 +82,7 @@ void do_gettimeofday(struct timeval *tv)
 	if (jiffies - wall_jiffies)
 		tv->tv_usec += USECS_PER_JIFFY;
 
-	read_unlock_irqrestore (&xtime_lock, flags);
+	read_unlock_irqrestore(&xtime_lock, flags);
 
 	if (tv->tv_usec >= 1000000) {
 		tv->tv_usec -= 1000000;
@@ -91,7 +92,7 @@ void do_gettimeofday(struct timeval *tv)
 
 void do_settimeofday(struct timeval *tv)
 {
-	write_lock_irq (&xtime_lock);
+	write_lock_irq(&xtime_lock);
 
 	/* This is revolting. We need to set the xtime.tv_usec
 	 * correctly. However, the value in this location is
@@ -111,7 +112,7 @@ void do_settimeofday(struct timeval *tv)
 	time_maxerror = NTP_PHASE_LIMIT;
 	time_esterror = NTP_PHASE_LIMIT;
 
-	write_unlock_irq (&xtime_lock);
+	write_unlock_irq(&xtime_lock);
 }
 
 
@@ -128,14 +129,11 @@ void do_settimeofday(struct timeval *tv)
  */
 
 
-/* This is for machines which generate the exact clock. */
-#define USECS_PER_JIFFY (1000000/HZ)
-
 /* usecs per counter cycle, shifted to left by 32 bits */
-static unsigned int sll32_usecs_per_cycle=0;
+static unsigned int sll32_usecs_per_cycle;
 
 /* how many counter cycles in a jiffy */
-static unsigned long cycles_per_jiffy=0;
+static unsigned long cycles_per_jiffy;
 
 /* Cycle counter value at the previous timer interrupt.. */
 static unsigned int timerhi, timerlo;
@@ -165,34 +163,33 @@ unsigned long fixed_rate_gettimeoffset(v
 	/* .. relative to previous jiffy (32 bits is enough) */
 	count -= timerlo;
 
-	__asm__("multu\t%1,%2\n\t"
-	        "mfhi\t%0"
-	        :"=r" (res)
-	        :"r" (count),
-	         "r" (sll32_usecs_per_cycle));
+	__asm__("multu	%1,%2"
+		: "=h" (res)
+		: "r" (count), "r" (sll32_usecs_per_cycle)
+		: "lo", "accum");
 
 	/*
 	 * Due to possible jiffies inconsistencies, we need to check
 	 * the result so that we'll get a timer that is monotonic.
 	 */
 	if (res >= USECS_PER_JIFFY)
-		res = USECS_PER_JIFFY-1;
+		res = USECS_PER_JIFFY - 1;
 
 	return res;
 }
 
 /*
- * Cached "1/(clocks per usec)*2^32" value.
+ * Cached "1/(clocks per usec) * 2^32" value.
  * It has to be recalculated once each jiffy.
  */
 static unsigned long cached_quotient;
 
 /* Last jiffy when calibrate_divXX_gettimeoffset() was called. */
-static unsigned long last_jiffies = 0;
+static unsigned long last_jiffies;
 
 
 /*
- * This is copied from dec/time.c:do_ioasic_gettimeoffset() by Mercij.
+ * This is copied from dec/time.c:do_ioasic_gettimeoffset() by Maciej.
  */
 unsigned long calibrate_div32_gettimeoffset(void)
 {
@@ -210,7 +207,7 @@ unsigned long calibrate_div32_gettimeoff
 			unsigned long r0;
 			do_div64_32(r0, timerhi, timerlo, tmp);
 			do_div64_32(quotient, USECS_PER_JIFFY,
-			            USECS_PER_JIFFY_FRAC, r0);
+				    USECS_PER_JIFFY_FRAC, r0);
 			cached_quotient = quotient;
 		}
 	}
@@ -221,9 +218,10 @@ unsigned long calibrate_div32_gettimeoff
 	/* .. relative to previous jiffy (32 bits is enough) */
 	count -= timerlo;
 
-	__asm__("multu  %2,%3"
-	        : "=l" (tmp), "=h" (res)
-	        : "r" (count), "r" (quotient));
+	__asm__("multu  %1,%2"
+		: "=h" (res)
+		: "r" (count), "r" (quotient)
+		: "lo", "accum");
 
 	/*
 	 * Due to possible jiffies inconsistencies, we need to check
@@ -247,27 +245,24 @@ unsigned long calibrate_div64_gettimeoff
 
 	if (tmp && last_jiffies != tmp) {
 		last_jiffies = tmp;
-		__asm__(".set\tnoreorder\n\t"
-	        ".set\tnoat\n\t"
-	        ".set\tmips3\n\t"
-	        "lwu\t%0,%2\n\t"
-	        "dsll32\t$1,%1,0\n\t"
-	        "or\t$1,$1,%0\n\t"
-	        "ddivu\t$0,$1,%3\n\t"
-	        "mflo\t$1\n\t"
-	        "dsll32\t%0,%4,0\n\t"
-	        "nop\n\t"
-	        "ddivu\t$0,%0,$1\n\t"
-	        "mflo\t%0\n\t"
-	        ".set\tmips0\n\t"
-	        ".set\tat\n\t"
-	        ".set\treorder"
-	        :"=&r" (quotient)
-	        :"r" (timerhi),
-	         "m" (timerlo),
-	         "r" (tmp),
-	         "r" (USECS_PER_JIFFY));
-	        cached_quotient = quotient;
+		__asm__(".set	push\n\t"
+			".set	noreorder\n\t"
+			".set	noat\n\t"
+			".set	mips3\n\t"
+			"lwu	%0,%2\n\t"
+			"dsll32	$1,%1,0\n\t"
+			"or	$1,$1,%0\n\t"
+			"ddivu	$0,$1,%3\n\t"
+			"mflo	$1\n\t"
+			"dsll32	%0,%4,0\n\t"
+			"nop\n\t"
+			"ddivu	$0,%0,$1\n\t"
+			"mflo	%0\n\t"
+			".set	pop"
+			: "=&r" (quotient)
+			: "r" (timerhi), "m" (timerlo),
+			  "r" (tmp), "r" (USECS_PER_JIFFY));
+		cached_quotient = quotient;
 	}
 
 	/* Get last timer tick in absolute kernel time */
@@ -276,18 +271,17 @@ unsigned long calibrate_div64_gettimeoff
 	/* .. relative to previous jiffy (32 bits is enough) */
 	count -= timerlo;
 
-	__asm__("multu\t%1,%2\n\t"
-	        "mfhi\t%0"
-	        :"=r" (res)
-	        :"r" (count),
-	         "r" (quotient));
+	__asm__("multu	%1,%2"
+		: "=h" (res)
+		: "r" (count), "r" (quotient)
+		: "lo", "accum");
 
 	/*
 	 * Due to possible jiffies inconsistencies, we need to check
 	 * the result so that we'll get a timer that is monotonic.
 	 */
 	if (res >= USECS_PER_JIFFY)
-		res = USECS_PER_JIFFY-1;
+		res = USECS_PER_JIFFY - 1;
 
 	return res;
 }
@@ -317,8 +311,8 @@ void local_timer_interrupt(int irq, void
 			 * put them into the last histogram slot, so if
 			 * present, they will show up as a sharp peak.
 			 */
-			if (pc > prof_len-1)
-			pc = prof_len-1;
+			if (pc > prof_len - 1)
+				pc = prof_len - 1;
 			atomic_inc((atomic_t *)&prof_buffer[pc]);
 		}
 	}
@@ -345,7 +339,7 @@ void timer_interrupt(int irq, void *dev_
 
 		/* check to see if we have missed any timer interrupts */
 		if ((count - expirelo) < 0x7fffffff) {
-			/* missed_timer_count ++; */
+			/* missed_timer_count++; */
 			expirelo = count + cycles_per_jiffy;
 			write_c0_compare(expirelo);
 		}
@@ -365,7 +359,7 @@ void timer_interrupt(int irq, void *dev_
 	 * CMOS clock accordingly every ~11 minutes. rtc_set_time() has to be
 	 * called as close as possible to 500 ms before the new second starts.
 	 */
-	read_lock (&xtime_lock);
+	read_lock(&xtime_lock);
 	if ((time_status & STA_UNSYNC) == 0 &&
 	    xtime.tv_sec > last_rtc_update + 660 &&
 	    xtime.tv_usec >= 500000 - ((unsigned) tick) / 2 &&
@@ -373,11 +367,11 @@ void timer_interrupt(int irq, void *dev_
 		if (rtc_set_time(xtime.tv_sec) == 0) {
 			last_rtc_update = xtime.tv_sec;
 		} else {
-			last_rtc_update = xtime.tv_sec - 600;
 			/* do it again in 60 s */
+			last_rtc_update = xtime.tv_sec - 600;
 		}
 	}
-	read_unlock (&xtime_lock);
+	read_unlock(&xtime_lock);
 
 	/*
 	 * If jiffies has overflowed in this timer_interrupt we must
@@ -396,7 +390,7 @@ void timer_interrupt(int irq, void *dev_
 	 * In SMP mode, local_timer_interrupt() is invoked by appropriate
 	 * low-level local timer interrupt handler.
 	 */
-	local_timer_interrupt(0, NULL, regs);
+	local_timer_interrupt(irq, dev_id, regs);
 
 #else	/* CONFIG_SMP */
 
@@ -463,18 +457,15 @@ asmlinkage void ll_local_timer_interrupt
  *	c) enable the timer interrupt
  */
 
-void (*board_time_init)(void) = NULL;
-void (*board_timer_setup)(struct irqaction *irq) = NULL;
+void (*board_time_init)(void);
+void (*board_timer_setup)(struct irqaction *irq);
 
-unsigned int mips_counter_frequency = 0;
+unsigned int mips_counter_frequency;
 
 static struct irqaction timer_irqaction = {
-	timer_interrupt,
-	SA_INTERRUPT,
-	0,
-	"timer",
-	NULL,
-	NULL
+	.handler = timer_interrupt,
+	.flags = SA_INTERRUPT,
+	.name = "timer",
 };
 
 void __init time_init(void)
@@ -541,15 +532,15 @@ void __init time_init(void)
 #define STARTOFTIME		1970
 #define SECDAY			86400L
 #define SECYR			(SECDAY * 365)
-#define leapyear(year)		((year) % 4 == 0)
-#define days_in_year(a)		(leapyear(a) ? 366 : 365)
-#define days_in_month(a)	(month_days[(a) - 1])
+#define leapyear(y)		((!((y) % 4) && ((y) % 100)) || !((y) % 400))
+#define days_in_year(y)		(leapyear(y) ? 366 : 365)
+#define days_in_month(m)	(month_days[(m) - 1])
 
 static int month_days[12] = {
 	31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31
 };
 
-void to_tm(unsigned long tim, struct rtc_time * tm)
+void to_tm(unsigned long tim, struct rtc_time *tm)
 {
 	long hms, day, gday;
 	int i;
@@ -564,16 +555,16 @@ void to_tm(unsigned long tim, struct rtc
 
 	/* Number of years in days */
 	for (i = STARTOFTIME; day >= days_in_year(i); i++)
-	day -= days_in_year(i);
+		day -= days_in_year(i);
 	tm->tm_year = i;
 
 	/* Number of months in days left */
 	if (leapyear(tm->tm_year))
-	days_in_month(FEBRUARY) = 29;
+		days_in_month(FEBRUARY) = 29;
 	for (i = 1; day >= days_in_month(i); i++)
-	day -= days_in_month(i);
+		day -= days_in_month(i);
 	days_in_month(FEBRUARY) = 28;
-	tm->tm_mon = i-1;	/* tm_mon starts from 0 to 11 */
+	tm->tm_mon = i - 1;		/* tm_mon starts from 0 to 11 */
 
 	/* Days are what is left over (+1) from all that. */
 	tm->tm_mday = day + 1;
@@ -581,7 +572,7 @@ void to_tm(unsigned long tim, struct rtc
 	/*
 	 * Determine the day of week
 	 */
-	tm->tm_wday = (gday + 4) % 7; /* 1970/1/1 was Thursday */
+	tm->tm_wday = (gday + 4) % 7;	/* 1970/1/1 was Thursday */
 }
 
 EXPORT_SYMBOL(rtc_lock);
diff -up --recursive --new-file linux-mips-2.4.21-20030711.macro/arch/mips64/kernel/time.c linux-mips-2.4.21-20030711/arch/mips64/kernel/time.c
--- linux-mips-2.4.21-20030711.macro/arch/mips64/kernel/time.c	2003-04-23 02:56:48.000000000 +0000
+++ linux-mips-2.4.21-20030711/arch/mips64/kernel/time.c	2003-07-21 20:28:39.000000000 +0000
@@ -1,9 +1,10 @@
 /*
  * Copyright 2001 MontaVista Software Inc.
  * Author: Jun Sun, jsun@mvista.com or jsun@junsun.net
+ * Copyright (c) 2003  Maciej W. Rozycki
  *
  * Common time service routines for MIPS machines. See
- * Documents/MIPS/README.txt.
+ * Documentation/mips/time.README.
  *
  * This program is free software; you can redistribute  it and/or modify it
  * under  the terms of  the GNU General  Public License as published by the
@@ -70,7 +71,7 @@ void do_gettimeofday(struct timeval *tv)
 {
 	unsigned long flags;
 
-	read_lock_irqsave (&xtime_lock, flags);
+	read_lock_irqsave(&xtime_lock, flags);
 	*tv = xtime;
 	tv->tv_usec += do_gettimeoffset();
 
@@ -81,7 +82,7 @@ void do_gettimeofday(struct timeval *tv)
 	if (jiffies - wall_jiffies)
 		tv->tv_usec += USECS_PER_JIFFY;
 
-	read_unlock_irqrestore (&xtime_lock, flags);
+	read_unlock_irqrestore(&xtime_lock, flags);
 
 	if (tv->tv_usec >= 1000000) {
 		tv->tv_usec -= 1000000;
@@ -91,7 +92,7 @@ void do_gettimeofday(struct timeval *tv)
 
 void do_settimeofday(struct timeval *tv)
 {
-	write_lock_irq (&xtime_lock);
+	write_lock_irq(&xtime_lock);
 
 	/* This is revolting. We need to set the xtime.tv_usec
 	 * correctly. However, the value in this location is
@@ -111,7 +112,7 @@ void do_settimeofday(struct timeval *tv)
 	time_maxerror = NTP_PHASE_LIMIT;
 	time_esterror = NTP_PHASE_LIMIT;
 
-	write_unlock_irq (&xtime_lock);
+	write_unlock_irq(&xtime_lock);
 }
 
 
@@ -128,14 +129,11 @@ void do_settimeofday(struct timeval *tv)
  */
 
 
-/* This is for machines which generate the exact clock. */
-#define USECS_PER_JIFFY (1000000/HZ)
-
 /* usecs per counter cycle, shifted to left by 32 bits */
-static unsigned int sll32_usecs_per_cycle=0;
+static unsigned int sll32_usecs_per_cycle;
 
 /* how many counter cycles in a jiffy */
-static unsigned long cycles_per_jiffy=0;
+static unsigned long cycles_per_jiffy;
 
 /* Cycle counter value at the previous timer interrupt.. */
 static unsigned int timerhi, timerlo;
@@ -165,34 +163,33 @@ unsigned long fixed_rate_gettimeoffset(v
 	/* .. relative to previous jiffy (32 bits is enough) */
 	count -= timerlo;
 
-	__asm__("multu\t%1,%2\n\t"
-	        "mfhi\t%0"
-	        :"=r" (res)
-	        :"r" (count),
-	         "r" (sll32_usecs_per_cycle));
+	__asm__("multu	%1,%2"
+		: "=h" (res)
+		: "r" (count), "r" (sll32_usecs_per_cycle)
+		: "lo", "accum");
 
 	/*
 	 * Due to possible jiffies inconsistencies, we need to check
 	 * the result so that we'll get a timer that is monotonic.
 	 */
 	if (res >= USECS_PER_JIFFY)
-		res = USECS_PER_JIFFY-1;
+		res = USECS_PER_JIFFY - 1;
 
 	return res;
 }
 
 /*
- * Cached "1/(clocks per usec)*2^32" value.
+ * Cached "1/(clocks per usec) * 2^32" value.
  * It has to be recalculated once each jiffy.
  */
 static unsigned long cached_quotient;
 
 /* Last jiffy when calibrate_divXX_gettimeoffset() was called. */
-static unsigned long last_jiffies = 0;
+static unsigned long last_jiffies;
 
 
 /*
- * This is copied from dec/time.c:do_ioasic_gettimeoffset() by Mercij.
+ * This is copied from dec/time.c:do_ioasic_gettimeoffset() by Maciej.
  */
 unsigned long calibrate_div32_gettimeoffset(void)
 {
@@ -210,7 +207,7 @@ unsigned long calibrate_div32_gettimeoff
 			unsigned long r0;
 			do_div64_32(r0, timerhi, timerlo, tmp);
 			do_div64_32(quotient, USECS_PER_JIFFY,
-			            USECS_PER_JIFFY_FRAC, r0);
+				    USECS_PER_JIFFY_FRAC, r0);
 			cached_quotient = quotient;
 		}
 	}
@@ -221,9 +218,10 @@ unsigned long calibrate_div32_gettimeoff
 	/* .. relative to previous jiffy (32 bits is enough) */
 	count -= timerlo;
 
-	__asm__("multu  %2,%3"
-	        : "=l" (tmp), "=h" (res)
-	        : "r" (count), "r" (quotient));
+	__asm__("multu  %1,%2"
+		: "=h" (res)
+		: "r" (count), "r" (quotient)
+		: "lo", "accum");
 
 	/*
 	 * Due to possible jiffies inconsistencies, we need to check
@@ -247,27 +245,24 @@ unsigned long calibrate_div64_gettimeoff
 
 	if (tmp && last_jiffies != tmp) {
 		last_jiffies = tmp;
-		__asm__(".set\tnoreorder\n\t"
-	        ".set\tnoat\n\t"
-	        ".set\tmips3\n\t"
-	        "lwu\t%0,%2\n\t"
-	        "dsll32\t$1,%1,0\n\t"
-	        "or\t$1,$1,%0\n\t"
-	        "ddivu\t$0,$1,%3\n\t"
-	        "mflo\t$1\n\t"
-	        "dsll32\t%0,%4,0\n\t"
-	        "nop\n\t"
-	        "ddivu\t$0,%0,$1\n\t"
-	        "mflo\t%0\n\t"
-	        ".set\tmips0\n\t"
-	        ".set\tat\n\t"
-	        ".set\treorder"
-	        :"=&r" (quotient)
-	        :"r" (timerhi),
-	         "m" (timerlo),
-	         "r" (tmp),
-	         "r" (USECS_PER_JIFFY));
-	        cached_quotient = quotient;
+		__asm__(".set	push\n\t"
+			".set	noreorder\n\t"
+			".set	noat\n\t"
+			".set	mips3\n\t"
+			"lwu	%0,%2\n\t"
+			"dsll32	$1,%1,0\n\t"
+			"or	$1,$1,%0\n\t"
+			"ddivu	$0,$1,%3\n\t"
+			"mflo	$1\n\t"
+			"dsll32	%0,%4,0\n\t"
+			"nop\n\t"
+			"ddivu	$0,%0,$1\n\t"
+			"mflo	%0\n\t"
+			".set	pop"
+			: "=&r" (quotient)
+			: "r" (timerhi), "m" (timerlo),
+			  "r" (tmp), "r" (USECS_PER_JIFFY));
+		cached_quotient = quotient;
 	}
 
 	/* Get last timer tick in absolute kernel time */
@@ -276,18 +271,17 @@ unsigned long calibrate_div64_gettimeoff
 	/* .. relative to previous jiffy (32 bits is enough) */
 	count -= timerlo;
 
-	__asm__("multu\t%1,%2\n\t"
-	        "mfhi\t%0"
-	        :"=r" (res)
-	        :"r" (count),
-	         "r" (quotient));
+	__asm__("multu	%1,%2"
+		: "=h" (res)
+		: "r" (count), "r" (quotient)
+		: "lo", "accum");
 
 	/*
 	 * Due to possible jiffies inconsistencies, we need to check
 	 * the result so that we'll get a timer that is monotonic.
 	 */
 	if (res >= USECS_PER_JIFFY)
-		res = USECS_PER_JIFFY-1;
+		res = USECS_PER_JIFFY - 1;
 
 	return res;
 }
@@ -317,8 +311,8 @@ void local_timer_interrupt(int irq, void
 			 * put them into the last histogram slot, so if
 			 * present, they will show up as a sharp peak.
 			 */
-			if (pc > prof_len-1)
-			pc = prof_len-1;
+			if (pc > prof_len - 1)
+				pc = prof_len - 1;
 			atomic_inc((atomic_t *)&prof_buffer[pc]);
 		}
 	}
@@ -345,7 +339,7 @@ void timer_interrupt(int irq, void *dev_
 
 		/* check to see if we have missed any timer interrupts */
 		if ((count - expirelo) < 0x7fffffff) {
-			/* missed_timer_count ++; */
+			/* missed_timer_count++; */
 			expirelo = count + cycles_per_jiffy;
 			write_c0_compare(expirelo);
 		}
@@ -365,7 +359,7 @@ void timer_interrupt(int irq, void *dev_
 	 * CMOS clock accordingly every ~11 minutes. rtc_set_time() has to be
 	 * called as close as possible to 500 ms before the new second starts.
 	 */
-	read_lock (&xtime_lock);
+	read_lock(&xtime_lock);
 	if ((time_status & STA_UNSYNC) == 0 &&
 	    xtime.tv_sec > last_rtc_update + 660 &&
 	    xtime.tv_usec >= 500000 - ((unsigned) tick) / 2 &&
@@ -373,11 +367,11 @@ void timer_interrupt(int irq, void *dev_
 		if (rtc_set_time(xtime.tv_sec) == 0) {
 			last_rtc_update = xtime.tv_sec;
 		} else {
-			last_rtc_update = xtime.tv_sec - 600;
 			/* do it again in 60 s */
+			last_rtc_update = xtime.tv_sec - 600;
 		}
 	}
-	read_unlock (&xtime_lock);
+	read_unlock(&xtime_lock);
 
 	/*
 	 * If jiffies has overflowed in this timer_interrupt we must
@@ -396,7 +390,7 @@ void timer_interrupt(int irq, void *dev_
 	 * In SMP mode, local_timer_interrupt() is invoked by appropriate
 	 * low-level local timer interrupt handler.
 	 */
-	local_timer_interrupt(0, NULL, regs);
+	local_timer_interrupt(irq, dev_id, regs);
 
 #else	/* CONFIG_SMP */
 
@@ -463,18 +457,15 @@ asmlinkage void ll_local_timer_interrupt
  *	c) enable the timer interrupt
  */
 
-void (*board_time_init)(void) = NULL;
-void (*board_timer_setup)(struct irqaction *irq) = NULL;
+void (*board_time_init)(void);
+void (*board_timer_setup)(struct irqaction *irq);
 
-unsigned int mips_counter_frequency = 0;
+unsigned int mips_counter_frequency;
 
 static struct irqaction timer_irqaction = {
-	timer_interrupt,
-	SA_INTERRUPT,
-	0,
-	"timer",
-	NULL,
-	NULL
+	.handler = timer_interrupt,
+	.flags = SA_INTERRUPT,
+	.name = "timer",
 };
 
 void __init time_init(void)
@@ -541,15 +532,15 @@ void __init time_init(void)
 #define STARTOFTIME		1970
 #define SECDAY			86400L
 #define SECYR			(SECDAY * 365)
-#define leapyear(year)		((year) % 4 == 0)
-#define days_in_year(a)		(leapyear(a) ? 366 : 365)
-#define days_in_month(a)	(month_days[(a) - 1])
+#define leapyear(y)		((!((y) % 4) && ((y) % 100)) || !((y) % 400))
+#define days_in_year(y)		(leapyear(y) ? 366 : 365)
+#define days_in_month(m)	(month_days[(m) - 1])
 
 static int month_days[12] = {
 	31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31
 };
 
-void to_tm(unsigned long tim, struct rtc_time * tm)
+void to_tm(unsigned long tim, struct rtc_time *tm)
 {
 	long hms, day, gday;
 	int i;
@@ -564,16 +555,16 @@ void to_tm(unsigned long tim, struct rtc
 
 	/* Number of years in days */
 	for (i = STARTOFTIME; day >= days_in_year(i); i++)
-	day -= days_in_year(i);
+		day -= days_in_year(i);
 	tm->tm_year = i;
 
 	/* Number of months in days left */
 	if (leapyear(tm->tm_year))
-	days_in_month(FEBRUARY) = 29;
+		days_in_month(FEBRUARY) = 29;
 	for (i = 1; day >= days_in_month(i); i++)
-	day -= days_in_month(i);
+		day -= days_in_month(i);
 	days_in_month(FEBRUARY) = 28;
-	tm->tm_mon = i-1;	/* tm_mon starts from 0 to 11 */
+	tm->tm_mon = i - 1;		/* tm_mon starts from 0 to 11 */
 
 	/* Days are what is left over (+1) from all that. */
 	tm->tm_mday = day + 1;
@@ -581,7 +572,7 @@ void to_tm(unsigned long tim, struct rtc
 	/*
 	 * Determine the day of week
 	 */
-	tm->tm_wday = (gday + 4) % 7; /* 1970/1/1 was Thursday */
+	tm->tm_wday = (gday + 4) % 7;	/* 1970/1/1 was Thursday */
 }
 
 EXPORT_SYMBOL(rtc_lock);
diff -up --recursive --new-file linux-mips-2.4.21-20030711.macro/include/asm-mips/time.h linux-mips-2.4.21-20030711/include/asm-mips/time.h
--- linux-mips-2.4.21-20030711.macro/include/asm-mips/time.h	2003-07-16 01:17:24.000000000 +0000
+++ linux-mips-2.4.21-20030711/include/asm-mips/time.h	2003-07-21 21:02:58.000000000 +0000
@@ -1,6 +1,7 @@
 /*
  * Copyright (C) 2001, 2002, MontaVista Software Inc.
  * Author: Jun Sun, jsun@mvista.com or jsun@junsun.net
+ * Copyright (c) 2003  Maciej W. Rozycki
  *
  * include/asm-mips/time.h
  *     header file for the new style time.c file and time services.
@@ -13,7 +14,7 @@
  */
 
 /*
- * Please refer to Documentation/MIPS/time.README.
+ * Please refer to Documentation/mips/time.README.
  */
 
 #ifndef _ASM_TIME_H
@@ -24,7 +25,7 @@
 #include <linux/rtc.h>                  /* for struct rtc_time */
 
 /*
- * RTC ops.  By default, they point a no-RTC functions.
+ * RTC ops.  By default, they point to no-RTC functions.
  *	rtc_get_time - mktime(year, mon, day, hour, min, sec) in seconds.
  *	rtc_set_time - reverse the above translation and set time to RTC.
  */
@@ -36,12 +37,12 @@ extern int (*rtc_set_time)(unsigned long
  * It is intended to help implement rtc_set_time() functions.
  * Copied from PPC implementation.
  */
-extern void to_tm(unsigned long tim, struct rtc_time * tm);
+extern void to_tm(unsigned long tim, struct rtc_time *tm);
 
 /*
  * do_gettimeoffset(). By default, this func pointer points to
  * do_null_gettimeoffset(), which leads to the same resolution as HZ.
- * Higher resolution versions are vailable, which gives ~1us resolution.
+ * Higher resolution versions are available, which give ~1us resolution.
  */
 extern unsigned long (*do_gettimeoffset)(void);
 
@@ -58,17 +59,17 @@ extern void timer_interrupt(int irq, voi
 /*
  * the corresponding low-level timer interrupt routine.
  */
-asmlinkage void ll_timer_interrupt(int irq, struct pt_regs *regs);
+extern asmlinkage void ll_timer_interrupt(int irq, struct pt_regs *regs);
 
 /*
  * profiling and process accouting is done separately in local_timer_interrupt
  */
-void local_timer_interrupt(int irq, void *dev_id, struct pt_regs *regs);
-asmlinkage void ll_local_timer_interrupt(int irq, struct pt_regs *regs);
+extern void local_timer_interrupt(int irq, void *dev_id, struct pt_regs *regs);
+extern asmlinkage void ll_local_timer_interrupt(int irq, struct pt_regs *regs);
 
 /*
  * board specific routines required by time_init().
- * board_time_init is defaulted to NULL and can remains so.
+ * board_time_init is defaulted to NULL and can remain so.
  * board_timer_setup must be setup properly in machine setup routine.
  */
 struct irqaction;
diff -up --recursive --new-file linux-mips-2.4.21-20030711.macro/include/asm-mips64/time.h linux-mips-2.4.21-20030711/include/asm-mips64/time.h
--- linux-mips-2.4.21-20030711.macro/include/asm-mips64/time.h	2003-01-11 19:23:38.000000000 +0000
+++ linux-mips-2.4.21-20030711/include/asm-mips64/time.h	2003-07-21 21:02:58.000000000 +0000
@@ -1,6 +1,7 @@
 /*
  * Copyright (C) 2001, 2002, MontaVista Software Inc.
  * Author: Jun Sun, jsun@mvista.com or jsun@junsun.net
+ * Copyright (c) 2003  Maciej W. Rozycki
  *
  * include/asm-mips/time.h
  *     header file for the new style time.c file and time services.
@@ -13,7 +14,7 @@
  */
 
 /*
- * Please refer to Documentation/MIPS/time.README.
+ * Please refer to Documentation/mips/time.README.
  */
 
 #ifndef _ASM_TIME_H
@@ -24,7 +25,7 @@
 #include <linux/rtc.h>                  /* for struct rtc_time */
 
 /*
- * RTC ops.  By default, they point a no-RTC functions.
+ * RTC ops.  By default, they point to no-RTC functions.
  *	rtc_get_time - mktime(year, mon, day, hour, min, sec) in seconds.
  *	rtc_set_time - reverse the above translation and set time to RTC.
  */
@@ -36,12 +37,12 @@ extern int (*rtc_set_time)(unsigned long
  * It is intended to help implement rtc_set_time() functions.
  * Copied from PPC implementation.
  */
-extern void to_tm(unsigned long tim, struct rtc_time * tm);
+extern void to_tm(unsigned long tim, struct rtc_time *tm);
 
 /*
  * do_gettimeoffset(). By default, this func pointer points to
  * do_null_gettimeoffset(), which leads to the same resolution as HZ.
- * Higher resolution versions are vailable, which gives ~1us resolution.
+ * Higher resolution versions are available, which give ~1us resolution.
  */
 extern unsigned long (*do_gettimeoffset)(void);
 
@@ -58,17 +59,17 @@ extern void timer_interrupt(int irq, voi
 /*
  * the corresponding low-level timer interrupt routine.
  */
-asmlinkage void ll_timer_interrupt(int irq, struct pt_regs *regs);
+extern asmlinkage void ll_timer_interrupt(int irq, struct pt_regs *regs);
 
 /*
  * profiling and process accouting is done separately in local_timer_interrupt
  */
-void local_timer_interrupt(int irq, void *dev_id, struct pt_regs *regs);
-asmlinkage void ll_local_timer_interrupt(int irq, struct pt_regs *regs);
+extern void local_timer_interrupt(int irq, void *dev_id, struct pt_regs *regs);
+extern asmlinkage void ll_local_timer_interrupt(int irq, struct pt_regs *regs);
 
 /*
  * board specific routines required by time_init().
- * board_time_init is defaulted to NULL and can remains so.
+ * board_time_init is defaulted to NULL and can remain so.
  * board_timer_setup must be setup properly in machine setup routine.
  */
 struct irqaction;

Re: [patch] Generic time fixes

[Jan-Benedict Glaw]

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

On Tue, 2003-07-22 09:58:46 +0200, Maciej W. Rozycki <macro@ds2.pg.gda.pl>
wrote in message <Pine.GSO.3.96.1030722093500.607A-100000@delta.ds2.pg.gda.pl>:
> Hello,
> 
>  In preparation to merging DECstation's time support with the generic
> version I did the following clean-ups to generic time support.  Most of
> these are coding style fixes (which might have already been fixed by
> someone else during past two days -- I haven't been able to study the
> changes), but there are a few code changes (detailed below) as well.

Nice to see you working on the DECstations again! Mine are not yet set
up in my cellar, but I'm slowly reaching there. Yesterday we had some
harsh thunderstorms (incl lightnings...) and I'm still recovering...
Maybe my assurance need to step in:(

MfG, JBG

-- 
   Jan-Benedict Glaw       jbglaw@lug-owl.de    . +49-172-7608481
   "Eine Freie Meinung in  einem Freien Kopf    | Gegen Zensur | Gegen Krieg
    fuer einen Freien Staat voll Freier Bürger" | im Internet! |   im Irak!
      ret = do_actions((curr | FREE_SPEECH) & ~(IRAQ_WAR_2 | DRM | TCPA));

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [patch] Generic time fixes

[Ralf Baechle]

On Tue, Jul 22, 2003 at 09:58:46AM +0200, Maciej W. Rozycki wrote:

>  Before I proceed further I need to get an aswer to the following
> question: why do we use rtc_set_time() for NTP RTC updates instead of
> rtc_set_mmss() like most other architectures do?  Traditionally Linux only
> updated minutes and seconds in this context and I don't think we need to
> do anything more.  And setting minutes and seconds only is way, way
> faster. Which might not matter that much every 11 minutes, except doing
> things slowly here incurs a disruption in the latency of the timer
> interrupt, which NTP might not like and the slow calculation of the RTC
> time causes less precise time being stored in the RTC chip. 
> 
>  It's already questionable whether the update should be done at all (this
> was discussed zillion of times at the NTP group) and it disrupts
> timekeeping of the DECstation severely, but given the current choice, I'd
> prefer to make it as lightweight as possible.

It's a common case to have a system boot up with the RTC date being
completly off, then syncing via ntpdate and xntp to the accurate time.
If in that situation we only update the time the RTC will stay way far
from reality.  Another case would be updates of the time near midnight
where the RTC and NTP date happen to just differ.

I share your dislike of updating the RTC for performance reasons; these
chips are impressive performance pigs.  So how about updating the RTC
date only when

 - write the time to the RTC for the first time after NTP synchronizes
 - write the time to the RTC if xtime.tv_sec <= last_rtc_update + 660

?

> 3. local_timer_interrupt() is called with arguments passed from the above
> instead of fake ones (although "irq" and "dev_id" could be completely
> removed here; in the next iteration, I suppose). 

Agreed.  I can't imagine any use for dev_id in the timer interrupt and
the interrupt number seems only marginally more useful.

> 5. Leap years are handled properly.

Good thing.  People at times do run their systems with clocks set wrongly,
even intensionally.  So I blame (year % 4) == 0 for being a too trivial
view of the world.

> 6. Missing "extern" qualifiers for function declarations are added.
> 
>  OK to apply?

Yes.

  Ralf

Re: [patch] Generic time fixes

[Wolfgang Denk]

In message <20030722100444.GA4148@linux-mips.org> Ralf Baechle wrote:
> 
> It's a common case to have a system boot up with the RTC date being
> completly off, then syncing via ntpdate and xntp to the accurate time.

Another common situation especially with embedded systems is that the
RTC holds the "correct"  time,  and  probably  runs  at  much  higher
precision  than  the systm clock. In such systems, NTP can be used to
keep the system time in sync with the RTC, but the system  time  must
never  be  written  back to the RTC. [Except when explicitely setting
the date&time.]

> I share your dislike of updating the RTC for performance reasons; these

There are more problems with the 11 minute mode.  We  ran  into  this
issue  for hard on some PowerPC systems. Assume a situation where the
RTC is connected through a I2C  bus.  Problem  1:  normally  the  i2c
drivers  will  be  loaded prety late, which means the system will run
initially with an undefined time. Problem 2: on some  actions  a  i2c
transfer  involves  programming an on-chip i2c controller, which will
perform the task and then signal it's done by an interrupt. On such a
system the 11 minute mode will crash the system because rtc_set  will
be called from interrupt contect itself.

There was a  somewhat  controverse  discussion  on  the  linuxppc_dev
mailing list, without a real solution.

> chips are impressive performance pigs.  So how about updating the RTC
> date only when
> 
>  - write the time to the RTC for the first time after NTP synchronizes
>  - write the time to the RTC if xtime.tv_sec <= last_rtc_update + 660

And never, ever update the RTC from interrupt context, please.


Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd@denx.de
"Free markets select for winning solutions."        - Eric S. Raymond

Re: [patch] Generic time fixes

[Ralf Baechle]

On Tue, Jul 22, 2003 at 12:21:04PM +0200, Wolfgang Denk wrote:

> Another common situation especially with embedded systems is that the
> RTC holds the "correct"  time,  and  probably  runs  at  much  higher
> precision  than  the systm clock. In such systems, NTP can be used to
> keep the system time in sync with the RTC, but the system  time  must
> never  be  written  back to the RTC. [Except when explicitely setting
> the date&time.]

True; supposedly the TXOs in RTC have higher long term frequency accuracy
than those commonly used to clock system though some RTC are really
badly off.

> > I share your dislike of updating the RTC for performance reasons; these
> 
> There are more problems with the 11 minute mode.  We  ran  into  this
> issue  for hard on some PowerPC systems. Assume a situation where the
> RTC is connected through a I2C  bus.  Problem  1:  normally  the  i2c
> drivers  will  be  loaded prety late, which means the system will run
> initially with an undefined time. Problem 2: on some  actions  a  i2c
> transfer  involves  programming an on-chip i2c controller, which will
> perform the task and then signal it's done by an interrupt. On such a
> system the 11 minute mode will crash the system because rtc_set  will
> be called from interrupt contect itself.
> 
> There was a  somewhat  controverse  discussion  on  the  linuxppc_dev
> mailing list, without a real solution.

[...]

> And never, ever update the RTC from interrupt context, please.

It's the right thing for > 99% of systems and doing it triggered by an
interrupt seems the right thing.  If you happen to have hardware that
has trouble with updates in interrupts you still could implement the
RTC update procedure to just trigger the update in softirq or process
context, as appropriate.

  Ralf

Re: [patch] Generic time fixes

[Jun Sun]

On Tue, Jul 22, 2003 at 09:58:46AM +0200, Maciej W. Rozycki wrote:
> Hello,
> 
>  In preparation to merging DECstation's time support with the generic
> version I did the following clean-ups to generic time support.  Most of
> these are coding style fixes (which might have already been fixed by
> someone else during past two days -- I haven't been able to study the
> changes), but there are a few code changes (detailed below) as well.
> 
>  Before I proceed further I need to get an aswer to the following
> question: why do we use rtc_set_time() for NTP RTC updates instead of
> rtc_set_mmss() like most other architectures do?  Traditionally Linux only
> updated minutes and seconds in this context and I don't think we need to
> do anything more.  And setting minutes and seconds only is way, way
> faster. Which might not matter that much every 11 minutes, except doing
> things slowly here incurs a disruption in the latency of the timer
> interrupt, which NTP might not like and the slow calculation of the RTC
> time causes less precise time being stored in the RTC chip. 
>

rtc_set_time() is more generic interface as it is also used in other 
places.  Boards which easily speed up (i.e., emulate rtc_set_mmss()) by
doing something like the following:

rtc_set_time(t)
{
	if (t-last_time_set < 660 + delta)
		rtc_set_mmss(t);
	else
		/* do a full rtc set */
	last_time_set = t;
}

A lot of boards don't do RTC update, and even when they do they
usually don't have performance issues (such as in vr41xx cases).
It is not fair to tax every board by requiring a new board interface
function.

BTW, at least one other arch (PPC) is not using rtc_set_mmss().

>  It's already questionable whether the update should be done at all (this
> was discussed zillion of times at the NTP group) and it disrupts
> timekeeping of the DECstation severely, but given the current choice, I'd
> prefer to make it as lightweight as possible.
> 

Whether to keep rtc in sync is an option which can be set by a board
Simply do a

time_status |= STA_UNSYNC 

in your <board>_setup routine will disable any RTC update.

>  Here are the changes done:
>

The changes look good to me.

Jun

Re: [patch] Generic time fixes

[Jun Sun]

On Tue, Jul 22, 2003 at 12:04:44PM +0200, Ralf Baechle wrote:
> On Tue, Jul 22, 2003 at 09:58:46AM +0200, Maciej W. Rozycki wrote:
> 
> >  Before I proceed further I need to get an aswer to the following
> > question: why do we use rtc_set_time() for NTP RTC updates instead of
> > rtc_set_mmss() like most other architectures do?  Traditionally Linux only
> > updated minutes and seconds in this context and I don't think we need to
> > do anything more.  And setting minutes and seconds only is way, way
> > faster. Which might not matter that much every 11 minutes, except doing
> > things slowly here incurs a disruption in the latency of the timer
> > interrupt, which NTP might not like and the slow calculation of the RTC
> > time causes less precise time being stored in the RTC chip. 
> > 
> >  It's already questionable whether the update should be done at all (this
> > was discussed zillion of times at the NTP group) and it disrupts
> > timekeeping of the DECstation severely, but given the current choice, I'd
> > prefer to make it as lightweight as possible.
> 
> It's a common case to have a system boot up with the RTC date being
> completly off, then syncing via ntpdate and xntp to the accurate time.
> If in that situation we only update the time the RTC will stay way far
> from reality.  Another case would be updates of the time near midnight
> where the RTC and NTP date happen to just differ.
> 
> I share your dislike of updating the RTC for performance reasons; these
> chips are impressive performance pigs.  So how about updating the RTC
> date only when
> 
>  - write the time to the RTC for the first time after NTP synchronizes

"First time after NTP synchronization" is not readily known to arch
time code.  This will involve kernel common code change.

>  - write the time to the RTC if xtime.tv_sec <= last_rtc_update + 660
>

Do you mean "xtime.tv_sec > last_rtc_update + 660", which is already
the case?

Again, board does have choice as to whether it wants RTC update or not.

Jun

Re: [patch] Generic time fixes

[Maciej W. Rozycki]

On Tue, 22 Jul 2003, Ralf Baechle wrote:

> >  It's already questionable whether the update should be done at all (this
> > was discussed zillion of times at the NTP group) and it disrupts
> > timekeeping of the DECstation severely, but given the current choice, I'd
> > prefer to make it as lightweight as possible.
> 
> It's a common case to have a system boot up with the RTC date being
> completly off, then syncing via ntpdate and xntp to the accurate time.
> If in that situation we only update the time the RTC will stay way far

 Hmm, any problems with `ntpdate <your_server>; hwclock -uw'?  Plus a lone
`hwclock -uw' before a graceful halt/reboot?  That's a typical userland
task and a trivial one, actually. 

> from reality.  Another case would be updates of the time near midnight
> where the RTC and NTP date happen to just differ.

 That's why rtc_set_mmss() never does an update that would require a
change to hours -- it defers it with a "set_rtc_mmss: can't update from x
to y" message.

> I share your dislike of updating the RTC for performance reasons; these
> chips are impressive performance pigs.  So how about updating the RTC
> date only when
> 
>  - write the time to the RTC for the first time after NTP synchronizes
>  - write the time to the RTC if xtime.tv_sec <= last_rtc_update + 660
> 
> ?

 I really see no point in doing it at all -- it's all easier and more
flexible to be done in the userland.

> > 5. Leap years are handled properly.
> 
> Good thing.  People at times do run their systems with clocks set wrongly,
> even intensionally.  So I blame (year % 4) == 0 for being a too trivial
> view of the world.

 Well, that's actually just the Julian view.

 There were a number of surprising discoveries in code here and there wrt
the y2k leap year exception -- apparently a number of people involved in
coding were not aware of the full Gregorian Calendar leap year rule, even
though doing that stuff should make them obliged to.  Somehow the rule was
already known to me many years ago, certainly before y2k and probably even
before I got involved with computers...

  Maciej

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

Re: [patch] Generic time fixes

[Maciej W. Rozycki]

On Tue, 22 Jul 2003, Wolfgang Denk wrote:

> > I share your dislike of updating the RTC for performance reasons; these
> 
> There are more problems with the 11 minute mode.  We  ran  into  this
> issue  for hard on some PowerPC systems. Assume a situation where the
> RTC is connected through a I2C  bus.  Problem  1:  normally  the  i2c
> drivers  will  be  loaded prety late, which means the system will run
> initially with an undefined time. Problem 2: on some  actions  a  i2c
> transfer  involves  programming an on-chip i2c controller, which will
> perform the task and then signal it's done by an interrupt. On such a
> system the 11 minute mode will crash the system because rtc_set  will
> be called from interrupt contect itself.
> 
> There was a  somewhat  controverse  discussion  on  the  linuxppc_dev
> mailing list, without a real solution.

 A possible solution is to make the update configurable with a sysctl.  It
really begs for it.  I think it may be worth discussing at the LKML. 
Additionally Ulrich Windl may wish to share a few thoughts on the matter. 

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

Re: [patch] Generic time fixes

[Maciej W. Rozycki]

On Tue, 22 Jul 2003, Jun Sun wrote:

> >  Before I proceed further I need to get an aswer to the following
> > question: why do we use rtc_set_time() for NTP RTC updates instead of
> > rtc_set_mmss() like most other architectures do?  Traditionally Linux only
> > updated minutes and seconds in this context and I don't think we need to
> > do anything more.  And setting minutes and seconds only is way, way
> > faster. Which might not matter that much every 11 minutes, except doing
> > things slowly here incurs a disruption in the latency of the timer
> > interrupt, which NTP might not like and the slow calculation of the RTC
> > time causes less precise time being stored in the RTC chip. 
> 
> rtc_set_time() is more generic interface as it is also used in other 
> places.  Boards which easily speed up (i.e., emulate rtc_set_mmss()) by
> doing something like the following:
> 
> rtc_set_time(t)
> {
> 	if (t-last_time_set < 660 + delta)
> 		rtc_set_mmss(t);
> 	else
> 		/* do a full rtc set */
> 	last_time_set = t;
> }
> 
> A lot of boards don't do RTC update, and even when they do they

 These should be fixed. 

> usually don't have performance issues (such as in vr41xx cases).
> It is not fair to tax every board by requiring a new board interface
> function.

 Well, rtc_set_time() is only used by the timekeeping code, so I see no
problem with renaming it.  And the interface remains the same -- it's a
number of seconds.  So if a full update is faster than changing minutes
and seconds only (e.g. the RTC is a monotonic counter -- I know a system
that just counts 10 ms intervals), an implementation is free to do so
(although that enforces UTC to be kept in the RTC; a good thing anyway),
but it shouldn't be required to.  And I think the name should be changed
to reflect that. 

 I you find such a cross-system update tedious -- don't worry.  I can do
that.  As a favor to platform maintainers I did such stuff in the past and
I can do it again.

> BTW, at least one other arch (PPC) is not using rtc_set_mmss().

 Their reasoning being?

> >  It's already questionable whether the update should be done at all (this
> > was discussed zillion of times at the NTP group) and it disrupts
> > timekeeping of the DECstation severely, but given the current choice, I'd
> > prefer to make it as lightweight as possible.
> > 
> 
> Whether to keep rtc in sync is an option which can be set by a board

 It can't.  But it should be configurable with a sysctl (but it isn't
now). 

> Simply do a
> 
> time_status |= STA_UNSYNC 
> 
> in your <board>_setup routine will disable any RTC update.

 Well, time_status = STA_UNSYNC initially, but ntpd will reset that.
Which is of course required to become a server.

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

Re: [patch] Generic time fixes

[Maciej W. Rozycki]

On Tue, 22 Jul 2003, Jun Sun wrote:

> Again, board does have choice as to whether it wants RTC update or not.

 Nope, it doesn't -- it has to update it.  There's something that is
called an interface and all systems are required to behave consistently. 
The only justifiable exception would be a system having no RTC at all.
Any other system ignoring the call is plain broken.

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

Re: [patch] Generic time fixes

[Ralf Baechle]

To add some performance numbers to the debate.  The DS1286 which is one of
the oldest chips we're supporting has a minimum write cycle time of 150ns.
The M48T02 which is used in the Origin needs 70ns, 150ns or 200ns depending
on the version.  Ok, those are slow numbers but they're not as bad as
postcards ...  I2C and it's evil brothers a is in a different universe
though.  There are busses in that cathegory that can transfer like 1kbyte/s.

  Ralf

Re: [patch] Generic time fixes

[Jun Sun]

On Tue, Jul 22, 2003 at 11:21:04PM +0200, Maciej W. Rozycki wrote:
> On Tue, 22 Jul 2003, Jun Sun wrote:
> 
> > >  Before I proceed further I need to get an aswer to the following
> > > question: why do we use rtc_set_time() for NTP RTC updates instead of
> > > rtc_set_mmss() like most other architectures do?  Traditionally Linux only
> > > updated minutes and seconds in this context and I don't think we need to
> > > do anything more.  And setting minutes and seconds only is way, way
> > > faster. Which might not matter that much every 11 minutes, except doing
> > > things slowly here incurs a disruption in the latency of the timer
> > > interrupt, which NTP might not like and the slow calculation of the RTC
> > > time causes less precise time being stored in the RTC chip. 
> > 
> > rtc_set_time() is more generic interface as it is also used in other 
> > places.  Boards which easily speed up (i.e., emulate rtc_set_mmss()) by
> > doing something like the following:
> > 
> > rtc_set_time(t)
> > {
> > 	if (t-last_time_set < 660 + delta)
> > 		rtc_set_mmss(t);
> > 	else
> > 		/* do a full rtc set */
> > 	last_time_set = t;
> > }
> > 
> > A lot of boards don't do RTC update, and even when they do they
> 
>  These should be fixed. 
> 
> > usually don't have performance issues (such as in vr41xx cases).
> > It is not fair to tax every board by requiring a new board interface
> > function.
> 
>  Well, rtc_set_time() is only used by the timekeeping code, so I see no
> problem with renaming it.  And the interface remains the same -- it's a
> number of seconds.  So if a full update is faster than changing minutes
> and seconds only (e.g. the RTC is a monotonic counter -- I know a system
> that just counts 10 ms intervals), an implementation is free to do so
> (although that enforces UTC to be kept in the RTC; a good thing anyway),
> but it shouldn't be required to.  And I think the name should be changed
> to reflect that. 
>

Actually the drivers/char/mips-rtc.c uses it too.  In that context
a full rtc set is needed.

The same interface can be used for the 2.6 gen-rtc.c as well, where
a full rtc update is needed also.

I like the current way it is because :

1) rtc_set_time() is a more generic interface so that it can be used
in more places (such as mips-rtc.c and gen-rtc.c).
2) rtc_set_mmss() can be reasonally emulated if it is desired on a particular
board
3) it is better to have just one rtc_set_xxx() instead of two.

> > BTW, at least one other arch (PPC) is not using rtc_set_mmss().
> 
>  Their reasoning being?
>

Probably the same reason as above?
 
> > >  It's already questionable whether the update should be done at all (this
> > > was discussed zillion of times at the NTP group) and it disrupts
> > > timekeeping of the DECstation severely, but given the current choice, I'd
> > > prefer to make it as lightweight as possible.
> > > 
> > 
> > Whether to keep rtc in sync is an option which can be set by a board
> 
>  It can't.  But it should be configurable with a sysctl (but it isn't
> now). 
> 
> > Simply do a
> > 
> > time_status |= STA_UNSYNC 
> > 
> > in your <board>_setup routine will disable any RTC update.
> 
>  Well, time_status = STA_UNSYNC initially, but ntpd will reset that.
> Which is of course required to become a server.
>

Actually searching through the kernel I can't find the place where
the flag is cleared.  Am I mistaken?

Jun

Re: [patch] Generic time fixes

[Maciej W. Rozycki]

On Wed, 23 Jul 2003, Ralf Baechle wrote:

> To add some performance numbers to the debate.  The DS1286 which is one of
> the oldest chips we're supporting has a minimum write cycle time of 150ns.
> The M48T02 which is used in the Origin needs 70ns, 150ns or 200ns depending
> on the version.  Ok, those are slow numbers but they're not as bad as

 Add a number of divisions to get the conversion done to that.

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

Re: [patch] Generic time fixes

[Maciej W. Rozycki]

On Tue, 22 Jul 2003, Jun Sun wrote:

> >  Well, rtc_set_time() is only used by the timekeeping code, so I see no
> > problem with renaming it.  And the interface remains the same -- it's a
> > number of seconds.  So if a full update is faster than changing minutes
> > and seconds only (e.g. the RTC is a monotonic counter -- I know a system
> > that just counts 10 ms intervals), an implementation is free to do so
> > (although that enforces UTC to be kept in the RTC; a good thing anyway),
> > but it shouldn't be required to.  And I think the name should be changed
> > to reflect that. 
> 
> Actually the drivers/char/mips-rtc.c uses it too.  In that context
> a full rtc set is needed.
> 
> The same interface can be used for the 2.6 gen-rtc.c as well, where
> a full rtc update is needed also.

 But that function should be provided by the driver (it may use
system-specific backends at will -- drivers/char/rtc.c does so as well),
with all that fancy stuff about epoch and century handling. 

> I like the current way it is because :
> 
> 1) rtc_set_time() is a more generic interface so that it can be used
> in more places (such as mips-rtc.c and gen-rtc.c).

 I see no problem with that interface being used there.

> 2) rtc_set_mmss() can be reasonally emulated if it is desired on a particular
> board

 I don't think so -- it would incur a race and a severe performance hit.
It makes no sense anyway.

> 3) it is better to have just one rtc_set_xxx() instead of two.

 Please define "better".  I think it's better to have a fast variation for
timekeeping as it's been used for years now for MC146818 and clones.

> >  Well, time_status = STA_UNSYNC initially, but ntpd will reset that.
> > Which is of course required to become a server.
> 
> Actually searching through the kernel I can't find the place where
> the flag is cleared.  Am I mistaken?

 See do_adjtimex().

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

Re: [patch] Generic time fixes

[Jun Sun]

On Wed, Jul 23, 2003 at 01:10:54AM +0200, Maciej W. Rozycki wrote:
> On Tue, 22 Jul 2003, Jun Sun wrote:
> 
> > >  Well, rtc_set_time() is only used by the timekeeping code, so I see no
> > > problem with renaming it.  And the interface remains the same -- it's a
> > > number of seconds.  So if a full update is faster than changing minutes
> > > and seconds only (e.g. the RTC is a monotonic counter -- I know a system
> > > that just counts 10 ms intervals), an implementation is free to do so
> > > (although that enforces UTC to be kept in the RTC; a good thing anyway),
> > > but it shouldn't be required to.  And I think the name should be changed
> > > to reflect that. 
> > 
> > Actually the drivers/char/mips-rtc.c uses it too.  In that context
> > a full rtc set is needed.
> > 
> > The same interface can be used for the 2.6 gen-rtc.c as well, where
> > a full rtc update is needed also.
> 
>  But that function should be provided by the driver (it may use
> system-specific backends at will -- drivers/char/rtc.c does so as well),

Isn't it cool to take care of the board-specific with the same interface
kernel time system uses?  Every MIPS board gets a basic RTC driver for free!

> > I like the current way it is because :
> > 
> > 1) rtc_set_time() is a more generic interface so that it can be used
> > in more places (such as mips-rtc.c and gen-rtc.c).
> 
>  I see no problem with that interface being used there.

Eh?  I assume you mean "no problem with rtc_set_mmss() being used there", true?

How come no problem?  rtc_set_mmss() does not even allow you set the time
if the new time is too off.  

> 
> > 2) rtc_set_mmss() can be reasonally emulated if it is desired on a particular
> > board
> 
>  I don't think so -- it would incur a race and a severe performance hit.
> It makes no sense anyway.

What is the race condition?  And what is the performance hit?

> > 3) it is better to have just one rtc_set_xxx() instead of two.
> 
>  Please define "better".  I think it's better to have a fast variation for
> timekeeping as it's been used for years now for MC146818 and clones.
>

Oh, yeah?  Look at those ugly #ifdefs include/asm-mips/mc146818rtc.h.
It is a poor abstraction of RTC.

If you convert DEC to the new RTC interface we could get rid of that
file completely.

And make no mistake, you _can_ have fast implementation that you are
looking for.

BTW, are you proposing to rename rtc_set_time() to rtc_set_mmss() and change
its semantic accordingly?  Or are you suggesting to add rtc_set_mmss()?

If you are suggesting the later, clearly fewer interface functions 
between MIPS common and board layer is better.

> > Actually searching through the kernel I can't find the place where
> > the flag is cleared.  Am I mistaken?
> 
>  See do_adjtimex().
>

I see.  Thanks.

Jun 

Re: [patch] Generic time fixes

[Maciej W. Rozycki]

On Tue, 22 Jul 2003, Jun Sun wrote:

> > > >  Well, rtc_set_time() is only used by the timekeeping code, so I see no
> > > > problem with renaming it.  And the interface remains the same -- it's a
> > > > number of seconds.  So if a full update is faster than changing minutes
> > > > and seconds only (e.g. the RTC is a monotonic counter -- I know a system
> > > > that just counts 10 ms intervals), an implementation is free to do so
> > > > (although that enforces UTC to be kept in the RTC; a good thing anyway),
> > > > but it shouldn't be required to.  And I think the name should be changed
> > > > to reflect that. 
> > > 
> > > Actually the drivers/char/mips-rtc.c uses it too.  In that context
> > > a full rtc set is needed.
> > > 
> > > The same interface can be used for the 2.6 gen-rtc.c as well, where
> > > a full rtc update is needed also.
> > 
> >  But that function should be provided by the driver (it may use
> > system-specific backends at will -- drivers/char/rtc.c does so as well),
> 
> Isn't it cool to take care of the board-specific with the same interface
> kernel time system uses?  Every MIPS board gets a basic RTC driver for free!

 Well, I'm not that convinced.  What's wrong with making real support for
the RTC chip instead?

> > > I like the current way it is because :
> > > 
> > > 1) rtc_set_time() is a more generic interface so that it can be used
> > > in more places (such as mips-rtc.c and gen-rtc.c).
> > 
> >  I see no problem with that interface being used there.
> 
> Eh?  I assume you mean "no problem with rtc_set_mmss() being used
> there", true? 

 Nope, I mean rtc_set_time() is just fine here -- do I sound crazy? 

> > > 2) rtc_set_mmss() can be reasonally emulated if it is desired on a particular
> > > board
> > 
> >  I don't think so -- it would incur a race and a severe performance hit.
> > It makes no sense anyway.
> 
> What is the race condition?  And what is the performance hit?

 You need to read from the RTC, modify minutes and seconds as appropriate
and write the RTC back.  Meanwhile the time as stored in the RTC may
change.  With the 500 ms offset approximation as used by time.c it may be
unlikely, but that assumption is for the MC146818 and it may not be true
for incompatible RTC chips.  That is the race.  The performance hit is
obvious -- now a read is added to the write.

> > > 3) it is better to have just one rtc_set_xxx() instead of two.
> > 
> >  Please define "better".  I think it's better to have a fast variation for
> > timekeeping as it's been used for years now for MC146818 and clones.
> 
> Oh, yeah?  Look at those ugly #ifdefs include/asm-mips/mc146818rtc.h.
> It is a poor abstraction of RTC.

 These should probably be removed and a few variables used instead.  I'll 
get it fixed one day.

> If you convert DEC to the new RTC interface we could get rid of that
> file completely.

 We won't be able -- at least drivers/char/rtc.c needs it.  And also the
driver is used for more systems than the lone DECstation.

> And make no mistake, you _can_ have fast implementation that you are
> looking for.

 I fail to see how a single division plus two iomem writes can be slower
than complicated arithmetics, involving a couple of loops and divisions,
then seven iomem writes, sorry.

> BTW, are you proposing to rename rtc_set_time() to rtc_set_mmss() and change
> its semantic accordingly?  Or are you suggesting to add rtc_set_mmss()?
> 
> If you are suggesting the later, clearly fewer interface functions 
> between MIPS common and board layer is better.

 It's mostly alike to me, except that I won't need rtc_set_time() as I
don't use mips_rtc.c or genrtc.c, so I won't implement it.

 And having no rtc_set_mmss() interface I *will* implement rtc_set_time()
as a lone minutes and seconds update with an appropriate comment why it is
done so.

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

Re: [patch] Generic time fixes

[Jun Sun]

On Wed, Jul 23, 2003 at 02:30:53AM +0200, Maciej W. Rozycki wrote:
> On Tue, 22 Jul 2003, Jun Sun wrote:
> 
> > Isn't it cool to take care of the board-specific with the same interface
> > kernel time system uses?  Every MIPS board gets a basic RTC driver for free!
> 
>  Well, I'm not that convinced.  What's wrong with making real support for
> the RTC chip instead?
>

Nothing wrong with full RTC driver support - it is just that when
30+ MIPS boards don't have to add #ifdef's to rtc.c and mc146818rtc.h
and hwclock still works people start appreciate more about the
existence of rtc_set_time().

> > 
> > What is the race condition?  And what is the performance hit?
> 
>  You need to read from the RTC, modify minutes and seconds as appropriate
> and write the RTC back.  Meanwhile the time as stored in the RTC may
> change.  With the 500 ms offset approximation as used by time.c it may be
> unlikely, but that assumption is for the MC146818 and it may not be true
> for incompatible RTC chips.  That is the race.  The performance hit is
> obvious -- now a read is added to the write.
>

OK, I see the performance hit now.

If you really want, how about the following change:

1) add set_rtc_mmss() function pointer in asm/time.h.
2) set it equal to set_rtc_time in time_init().  Board can override
   this decision in board_timer_setup() for better performance.
3) RTC update is changed to call set_rtc_mmss()

How does this sound?  It leaves all existing code unchanged, while
gives a way for optimization.  The default setting of set_rtc_mmss
to set_rtc_time makes logical sense too, because set_rtc_mmss is really
a "back door" version of set_rtc_time().

Jun

Re: [patch] Generic time fixes

[Maciej W. Rozycki]

On Tue, 22 Jul 2003, Jun Sun wrote:

> > > Isn't it cool to take care of the board-specific with the same interface
> > > kernel time system uses?  Every MIPS board gets a basic RTC driver for free!
> > 
> >  Well, I'm not that convinced.  What's wrong with making real support for
> > the RTC chip instead?
> >
> 
> Nothing wrong with full RTC driver support - it is just that when
> 30+ MIPS boards don't have to add #ifdef's to rtc.c and mc146818rtc.h
> and hwclock still works people start appreciate more about the
> existence of rtc_set_time().

 Hmm, but how many different RTC chips are out there?  I agree the current
rtc.c/mc146818rtc.h implementation sucks, but it should be fixed and not
worked around.

> If you really want, how about the following change:
> 
> 1) add set_rtc_mmss() function pointer in asm/time.h.
> 2) set it equal to set_rtc_time in time_init().  Board can override
>    this decision in board_timer_setup() for better performance.
> 3) RTC update is changed to call set_rtc_mmss()
> 
> How does this sound?  It leaves all existing code unchanged, while
> gives a way for optimization.  The default setting of set_rtc_mmss
> to set_rtc_time makes logical sense too, because set_rtc_mmss is really
> a "back door" version of set_rtc_time().

 That's just fine for me.

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

Re: [patch] Generic time fixes

[Jun Sun]

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

On Wed, Jul 23, 2003 at 04:52:31PM +0200, Maciej W. Rozycki wrote:
> On Tue, 22 Jul 2003, Jun Sun wrote:
> 
> > > > Isn't it cool to take care of the board-specific with the same interface
> > > > kernel time system uses?  Every MIPS board gets a basic RTC driver for free!
> > > 
> > >  Well, I'm not that convinced.  What's wrong with making real support for
> > > the RTC chip instead?
> > >
> > 
> > Nothing wrong with full RTC driver support - it is just that when
> > 30+ MIPS boards don't have to add #ifdef's to rtc.c and mc146818rtc.h
> > and hwclock still works people start appreciate more about the
> > existence of rtc_set_time().
> 
>  Hmm, but how many different RTC chips are out there?  I agree the current
> rtc.c/mc146818rtc.h implementation sucks, but it should be fixed and not
> worked around.
>

Most people seem to be happy with getting hwclock working.  rtc_set_time()
does allow a low-oevrhead way to implement a generic rtc driver which
makes hwclock happy.

I like see mc146818rtc related RTC go away eventually, but we don't have to 
agree on that right now.

> > If you really want, how about the following change:
> > 
> > 1) add set_rtc_mmss() function pointer in asm/time.h.
> > 2) set it equal to set_rtc_time in time_init().  Board can override
> >    this decision in board_timer_setup() for better performance.
> > 3) RTC update is changed to call set_rtc_mmss()
> > 
> > How does this sound?  It leaves all existing code unchanged, while
> > gives a way for optimization.  The default setting of set_rtc_mmss
> > to set_rtc_time makes logical sense too, because set_rtc_mmss is really
> > a "back door" version of set_rtc_time().
> 
>  That's just fine for me.
>

Something like the attached patch looks good to me.

Board now has the choice to implement either rtc_set_time, or
rtc_set_mmss, or both, or none.  Of course there are different
implications for each choice.  I probably should update the readme
a little too.

You can either include it in your next patch (if one is coming).  Or
just let me know and I will flesh it out and check it in.

Jun

[-- Attachment #2: junk --]
[-- Type: text/plain, Size: 1682 bytes --]

diff -Nru link/arch/mips/kernel/time.c.orig link/arch/mips/kernel/time.c
--- link/arch/mips/kernel/time.c.orig	Wed Jul 16 10:52:57 2003
+++ link/arch/mips/kernel/time.c	Wed Jul 23 09:52:53 2003
@@ -61,6 +61,7 @@
 
 unsigned long (*rtc_get_time)(void) = null_rtc_get_time;
 int (*rtc_set_time)(unsigned long) = null_rtc_set_time;
+int (*rtc_set_mmss)(unsigned long);
 
 
 /*
@@ -370,12 +371,16 @@
 	    xtime.tv_sec > last_rtc_update + 660 &&
 	    xtime.tv_usec >= 500000 - ((unsigned) tick) / 2 &&
 	    xtime.tv_usec <= 500000 + ((unsigned) tick) / 2) {
-		if (rtc_set_time(xtime.tv_sec) == 0) {
+		int ret;
+		if (rtc_set_mmss)
+			ret = rtc_set_mmss(xtime.tv_sec);
+		else
+			ret = rtc_set_time(xtime.tv_sec);
+		if (ret == 0) 
 			last_rtc_update = xtime.tv_sec;
-		} else {
-			last_rtc_update = xtime.tv_sec - 600;
+		else 
 			/* do it again in 60 s */
-		}
+			last_rtc_update = xtime.tv_sec - 600;
 	}
 	read_unlock (&xtime_lock);
 
diff -Nru link/include/asm-mips/time.h.orig link/include/asm-mips/time.h
--- link/include/asm-mips/time.h.orig	Wed Jul 16 11:49:53 2003
+++ link/include/asm-mips/time.h	Wed Jul 23 09:51:58 2003
@@ -27,9 +27,12 @@
  * RTC ops.  By default, they point a no-RTC functions.
  *	rtc_get_time - mktime(year, mon, day, hour, min, sec) in seconds.
  *	rtc_set_time - reverse the above translation and set time to RTC.
+ *	rtc_set_mmss - similar to rtc_set_time, but only mim and sec need
+ *			to be set.  Used by RTC sync-up.
  */
 extern unsigned long (*rtc_get_time)(void);
 extern int (*rtc_set_time)(unsigned long);
+extern int (*rtc_set_mmss)(unsigned long);
 
 /*
  * to_tm() converts system time back to (year, mon, day, hour, min, sec).

Re: [patch] Generic time fixes

[Maciej W. Rozycki]

On Wed, 23 Jul 2003, Jun Sun wrote:

> Most people seem to be happy with getting hwclock working.  rtc_set_time()
> does allow a low-oevrhead way to implement a generic rtc driver which
> makes hwclock happy.

 Well, some people are not most people and they want a full-featured
implementation as described in Documentation/rtc.txt.

> I like see mc146818rtc related RTC go away eventually, but we don't have to 
> agree on that right now.

 You'll need to convince guys at the LKML first (me inclusive ;-) ).  If
you write a clean and full-featured replacement, you'll most likely be
welcome.

> You can either include it in your next patch (if one is coming).  Or
> just let me know and I will flesh it out and check it in.

 Here is an optimized replacement I'm going to check in -- OK?  Ralf?

 I'm working on further changes, but there is no point in coalescing
self-contained changes.

  Maciej

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

patch-mips-2.4.21-20030711-mips-mmss-0
diff -up --recursive --new-file linux-mips-2.4.21-20030711.macro/arch/mips/kernel/time.c linux-mips-2.4.21-20030711/arch/mips/kernel/time.c
--- linux-mips-2.4.21-20030711.macro/arch/mips/kernel/time.c	2003-07-21 20:28:39.000000000 +0000
+++ linux-mips-2.4.21-20030711/arch/mips/kernel/time.c	2003-07-24 08:22:52.000000000 +0000
@@ -62,6 +62,7 @@ static int null_rtc_set_time(unsigned lo
 
 unsigned long (*rtc_get_time)(void) = null_rtc_get_time;
 int (*rtc_set_time)(unsigned long) = null_rtc_set_time;
+int (*rtc_set_mmss)(unsigned long);
 
 
 /*
@@ -364,7 +365,7 @@ void timer_interrupt(int irq, void *dev_
 	    xtime.tv_sec > last_rtc_update + 660 &&
 	    xtime.tv_usec >= 500000 - ((unsigned) tick) / 2 &&
 	    xtime.tv_usec <= 500000 + ((unsigned) tick) / 2) {
-		if (rtc_set_time(xtime.tv_sec) == 0) {
+		if (rtc_set_mmss(xtime.tv_sec) == 0) {
 			last_rtc_update = xtime.tv_sec;
 		} else {
 			/* do it again in 60 s */
@@ -473,6 +474,9 @@ void __init time_init(void)
 	if (board_time_init)
 		board_time_init();
 
+	if (!rtc_set_mmss)
+		rtc_set_mmss = rtc_set_time;
+
 	xtime.tv_sec = rtc_get_time();
 	xtime.tv_usec = 0;
 
diff -up --recursive --new-file linux-mips-2.4.21-20030711.macro/arch/mips64/kernel/time.c linux-mips-2.4.21-20030711/arch/mips64/kernel/time.c
--- linux-mips-2.4.21-20030711.macro/arch/mips64/kernel/time.c	2003-07-21 20:28:39.000000000 +0000
+++ linux-mips-2.4.21-20030711/arch/mips64/kernel/time.c	2003-07-24 08:22:52.000000000 +0000
@@ -62,6 +62,7 @@ static int null_rtc_set_time(unsigned lo
 
 unsigned long (*rtc_get_time)(void) = null_rtc_get_time;
 int (*rtc_set_time)(unsigned long) = null_rtc_set_time;
+int (*rtc_set_mmss)(unsigned long);
 
 
 /*
@@ -364,7 +365,7 @@ void timer_interrupt(int irq, void *dev_
 	    xtime.tv_sec > last_rtc_update + 660 &&
 	    xtime.tv_usec >= 500000 - ((unsigned) tick) / 2 &&
 	    xtime.tv_usec <= 500000 + ((unsigned) tick) / 2) {
-		if (rtc_set_time(xtime.tv_sec) == 0) {
+		if (rtc_set_mmss(xtime.tv_sec) == 0) {
 			last_rtc_update = xtime.tv_sec;
 		} else {
 			/* do it again in 60 s */
@@ -473,6 +474,9 @@ void __init time_init(void)
 	if (board_time_init)
 		board_time_init();
 
+	if (!rtc_set_mmss)
+		rtc_set_mmss = rtc_set_time;
+
 	xtime.tv_sec = rtc_get_time();
 	xtime.tv_usec = 0;
 
diff -up --recursive --new-file linux-mips-2.4.21-20030711.macro/include/asm-mips/time.h linux-mips-2.4.21-20030711/include/asm-mips/time.h
--- linux-mips-2.4.21-20030711.macro/include/asm-mips/time.h	2003-07-21 21:02:58.000000000 +0000
+++ linux-mips-2.4.21-20030711/include/asm-mips/time.h	2003-07-24 08:24:08.000000000 +0000
@@ -28,9 +28,12 @@
  * RTC ops.  By default, they point to no-RTC functions.
  *	rtc_get_time - mktime(year, mon, day, hour, min, sec) in seconds.
  *	rtc_set_time - reverse the above translation and set time to RTC.
+ *	rtc_set_mmss - similar to rtc_set_time, but only min and sec need
+ *			to be set.  Used by RTC sync-up.
  */
 extern unsigned long (*rtc_get_time)(void);
 extern int (*rtc_set_time)(unsigned long);
+extern int (*rtc_set_mmss)(unsigned long);
 
 /*
  * to_tm() converts system time back to (year, mon, day, hour, min, sec).
diff -up --recursive --new-file linux-mips-2.4.21-20030711.macro/include/asm-mips64/time.h linux-mips-2.4.21-20030711/include/asm-mips64/time.h
--- linux-mips-2.4.21-20030711.macro/include/asm-mips64/time.h	2003-07-21 21:02:58.000000000 +0000
+++ linux-mips-2.4.21-20030711/include/asm-mips64/time.h	2003-07-24 08:24:08.000000000 +0000
@@ -28,9 +28,12 @@
  * RTC ops.  By default, they point to no-RTC functions.
  *	rtc_get_time - mktime(year, mon, day, hour, min, sec) in seconds.
  *	rtc_set_time - reverse the above translation and set time to RTC.
+ *	rtc_set_mmss - similar to rtc_set_time, but only min and sec need
+ *			to be set.  Used by RTC sync-up.
  */
 extern unsigned long (*rtc_get_time)(void);
 extern int (*rtc_set_time)(unsigned long);
+extern int (*rtc_set_mmss)(unsigned long);
 
 /*
  * to_tm() converts system time back to (year, mon, day, hour, min, sec).