linux-mips
[Top] [All Lists]

Re: [patch] Generic time fixes

To: "Maciej W. Rozycki" <macro@ds2.pg.gda.pl>
Subject: Re: [patch] Generic time fixes
From: Jun Sun <jsun@mvista.com>
Date: Tue, 22 Jul 2003 18:14:30 -0700
Cc: Ralf Baechle <ralf@linux-mips.org>, linux-mips@linux-mips.org, jsun@mvista.com
In-reply-to: <Pine.GSO.3.96.1030723014418.607S-100000@delta.ds2.pg.gda.pl>; from macro@ds2.pg.gda.pl on Wed, Jul 23, 2003 at 02:30:53AM +0200
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <20030722163701.G3135@mvista.com> <Pine.GSO.3.96.1030723014418.607S-100000@delta.ds2.pg.gda.pl>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Mutt/1.2.5i
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

<Prev in Thread] Current Thread [Next in Thread>