This is a generic implementation of rtc_read_persistent_clock() and
rtc_update_persistent_clock() suitable for platforms to be used for
read_persistent_clock() and update_persistent_clock() calls.  An RTC
device selected by the user with the RTC_HCTOSYS_DEVICE option is used.  

 As rtc_read_persistent_clock() is not available at the time
timekeeping_init() is called, it will now be disabled if the class device
is to be used as a reference.  In this case rtc_hctosys(), already
present, will be used to set up the system time at the late initcall time.  
This call has now been rewritten to make use of
rtc_read_persistent_clock().

 As rtc_set_mmss() used by rtc_update_persistent_clock() may sleep for 
some hardware, the call is now made from a work queue scheduled by the 
timer originally used for the entire function.

Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
---
patch-2.6.26-rc1-20080505-rtc-persistent-clock-11
diff -up --recursive --new-file linux-2.6.26-rc1-20080505.macro/drivers/rtc/Kconfig linux-2.6.26-rc1-20080505/drivers/rtc/Kconfig
--- linux-2.6.26-rc1-20080505.macro/drivers/rtc/Kconfig	2008-05-05 02:55:40.000000000 +0000
+++ linux-2.6.26-rc1-20080505/drivers/rtc/Kconfig	2008-05-05 21:41:34.000000000 +0000
@@ -21,24 +21,30 @@ menuconfig RTC_CLASS
 if RTC_CLASS
 
 config RTC_HCTOSYS
-	bool "Set system time from RTC on startup and resume"
+	bool "Use time from RTC as a reference for the system time"
 	depends on RTC_CLASS = y
 	default y
 	help
-	  If you say yes here, the system time (wall clock) will be set using
-	  the value read from a specified RTC device. This is useful to avoid
-	  unnecessary fsck runs at boot time, and to network better.
+	  If you say yes here, the specified RTC device will be used
+	  as a reference to the system time (wall clock).  The device
+	  will be used to set the system time as required during
+	  startup, suspend and, for platforms that support such usage,
+	  for NTP timekeeping.
+
+	  This is useful to avoid ...
[ message continues ]

Hrmm. So how is this going to work with suspend and resume?

Ideally, on resume we want to update the clock before interrupts are
reenabled so we don't get stale time values post-resume.  For systems
that sleep on reading the persistent clock, I'm open to having them
fix it up as best they can later (partly why the code can handle
read_persistent_clock() not returning anything), but unless I'm
misreading this, it seems you're proposing to make systems that do
have a safe persistent clock have to have the window where code may
see the pre-suspend time after resume.

Am I missing something here?

thanks
-john

Maciej: Sorry for the dup, I forgot to reply to all on my first mail.
--

Hi John,

 Sorry about the delay -- I have missed your comment in the flood.


 Hmm, I have never used suspend/resume, so I cannot really comment.  Here
is what I gathered by glancing over the code and some bits of

 Right now it looks the time is restored in two places, 
timekeeping_resume() and rtc_resume().  Of course once the transition to 
the new RTC infrastructure has been done, one is going to be redundant.  
For the time being I think it is harmless to have them both.

 That written, both are called from the relevant driver's ->resume()  
method.  My set of patches does not change it and as far as I can tell if
it worked before, it will work afterwards.  As I understand ->resume()  

 No idea -- anyone?

  Maciej
--

Ooh, shiny -- you saved me the trouble of doing this (and hopefully also
the trouble of looking through it to check whether all the callers of
read_persistent_clock() can sleep, etc.?)

One thing I was going to do in rtc_update_persistent_clock() was make it
use mutex_trylock() for grabbing rtc->lock. We go to great lengths to
make sure we're updating the clock at the correct time -- we don't want
to be doing things which delay the update. So we should probably just
use mutex_trylock() and abort the update (this time) if it fails.

I was also thinking of holding the RTC_HCTOSYS device open all the time,
too. If it's a problem that you then couldn't unload the module, perhaps
a sysfs interface to set/change/clear which device is used for this?

When we discussed it last week, Alessandro was concerned that the
'update at precisely 500ms past the second' rule was not universal to
all RTC devices, although I'm not entirely sure. It might be worth
moving that logic into a 'default' NTP-sync routine provided by the RTC
class, so that if any strange devices exist which require different
treatment, they can override that.

I wouldn't worry too much about leaving the old
update_persistent_clock() and read_persistent_clock() -- I hope we can
plan to remove those entirely in favour of the RTC class methods.

-- 
dwmw2

--

:-)

 Yes, the generic callers of read_persistent_clock() seem to be fine.  I
have not dug into various pieces of platform code to check there though.  
I do hope platform maintainers enable these debugging checks from time to
time -- I mean especially those taking care of all these more or less
exotic systems and boards residing one level below generic architecture


 I have thought of this and recognised the concern about modules.  I think
another possibility that could work with modules might be opening and
closing the device on demand.  But still it is just an optimisation, which
can certainly be done gradually on top of these changes without even
changing the interface.

 From the code structure's point of view it is certainly cleaner to open
and close the device each time it is used and I think as such it is a good

 Even better than that -- some devices may have better precision and 
require no strange handling.  For example this very M41T81 chip I am 
working with supports resolution up to .01s -- which means there is little 
point in trying to work out delays, etc. to get the clock written back at 
the right point, where we can easily obtain two levels of magnitude better 
a resolution by simply not discarding the sub-second part of a timestamp.  

 Of course for sub-second resolution of the RTC the interface of
read_persistent_clock() would have to be modified to return a struct
timespec; contrarily update_persistent_clock() is ready for that, but then
struct rtc_time plus rtc_read_time(), rtc_set_mmss() and rtc_set_time()  
will still have to be updated accordingly.

 BTW, this chip is even better than that, because it can be disciplined --
there is a five-bit calibration register that lets one add or remove
oscillator ticks to/from the input at a certain stage of the divider chain
-- that could be used by the NTP daemon or tools like `hwclock --adjust'

 Yes, that I would consider a reasonable long-term plan, but it will take
at least a short while during which ...
[ message continues ]

On Wed, 07 May 2008 09:24:15 +0100



 mm.. let's keep it easy. the chances the rtc is in use
 are usually real low.
 

-- 

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Torino, Italy

  http://www.towertech.it

--