Re: [PATCH 6/9 v3] RFC: timekeeping: rtc: remove CONFIG_RTC_HCTOSYS and RTC_HCTOSYS_DEVICE

[John Stultz <john.stultz@linaro.org>]

On 06/14/2013 09:52 AM, Alexander Holler wrote:
> Those config options don't make sense anymore with the new hctosys
> mechanism introduced with the previous patch.
>
> That means two things:
>
> - If a (hardware) clock is available it will be used to set the time at
>    boot. This was already the case for system which have a "persistent"
>    clock, e.g. most x86 systems. The only way to specify the device used
>    for hctosys is now by using the kernel parameter hctosys= introduced
>    with a previous patch.
>
> - If a hardware clock was used for hctosys before suspend, this clock
>    will be used to adjust the clock at resume. Again, this doesn't change
>    anything on systems with a "persistent" clock.
>
> What's missing:
>
> I don't know much about those "persistent" clocks and I haven't had a
> deep look at them. That's especially true for the suspend/resume
> mechanism used by them. The mechanism I want to use is the following:
> The RTC subsystem now maintains the ID of the RTC device which was used
> for hctosys (in rtc_hctosys_dev_id) and therefor specifies the device
> which should be used to adjust the time after resume. Additionaly the
> (new) flag systime_was_set will be set to false at suspend and on resume
> this flag will be set to true if either the clock will be adjusted by
> the device used for hctosys or by userspace (through do_settimeofday()).
>
> That all should already work as expected for RTCs, what's missing for
> "persistent" clocks is that the flag systime_was_set is set to false on
> suspend and set to true on resume. Currently it just stays at true
> (which is set through hctosys if a "persistent" clock is found.
> But because "persistent" clocks don't go away (as it is possible with
> RTCs by removing the driver or the RTC itself), nor do "persistent"
> clocks might have two instances, this shouldn't be a problem at all.

This one concerns me a bit. Since you're removing quite a bit and it 
looks like it may break userland expectations.

I ran into this myself recently, when I found some distros look for 
/sys/class/rtc/rtcN/hctosys in order to determine which rtc device 
should be synced with from userland.

So I'd probably suggest instead to re-factor this so you leave all the 
hctosys bits alone, but just change it from being called by a 
late_initcall() and instead have it called when we register the RTC that 
matches CONFIG_RTC_HCTOSYS_DEVICE.

I suspect it will end up being a much smaller change that way.

Then the last bit is just a matter of adding the 
timekeeping_systimeset() check to the hctosys bits.


thanks
-john