Re: [patch] Generic time fixes

[Jun Sun <jsun@mvista.com>]

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