[PATCH 0/2][RFC] Try to handle hctosys w/ rtc modules

[PATCH 0/2][RFC] Try to handle hctosys w/ rtc modules

[John Stultz]

Its been pointed out that the RTC hctosys functionality doesn't
work well with RTC modules, which may not be loaded until after
late_init().

While there have been other attempts to sovle this, this patchset
is a very quick 10 minute effort to show how I'd try to resolve
this. There likely are still issues here, but I'd be happy to
make fixes and adjustments to ensure it works.

Feedback and comments always appreciated!

-john

Cc: John Whitmore <arigead@gmail.com>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexander Holler <holler@ahsoftware.de>

John Stultz (2):
  time: Introduce do_first_settimeofday()
  rtc: Rework hctosys so that it is called on RTC registration

 drivers/rtc/class.c       |  2 ++
 drivers/rtc/hctosys.c     | 19 +++++++----------
 include/linux/rtc.h       |  6 ++++++
 include/linux/time.h      |  2 ++
 kernel/time/timekeeping.c | 54 ++++++++++++++++++++++++++++++++++++++---------
 5 files changed, 62 insertions(+), 21 deletions(-)

-- 
1.9.1

[PATCH 1/2][RFC] time: Introduce do_first_settimeofday()

[John Stultz]

There's some cases in the kernel, particuarly with RTC drivers,
where we want to set the time, but only if no one else has
already set it.

This is useful for systems where the RTC driver is a module, and
is loaded late in initialization. However, we want to be sure we
don't override the time that may have been set by userspace.

Cc: John Whitmore <arigead@gmail.com>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexander Holler <holler@ahsoftware.de>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 include/linux/time.h      |  2 ++
 kernel/time/timekeeping.c | 54 ++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/include/linux/time.h b/include/linux/time.h
index d5d229b..7379291 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -157,6 +157,8 @@ extern void do_gettimeofday(struct timeval *tv);
 extern int do_settimeofday(const struct timespec *tv);
 extern int do_sys_settimeofday(const struct timespec *tv,
 			       const struct timezone *tz);
+extern int do_first_settimeofday(const struct timespec *ts);
+
 #define do_posix_clock_monotonic_gettime(ts) ktime_get_ts(ts)
 extern long do_utimes(int dfd, const char __user *filename, struct timespec *times, int flags);
 struct itimerval;
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 32d8d6a..45c2642 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -487,6 +487,25 @@ void do_gettimeofday(struct timeval *tv)
 }
 EXPORT_SYMBOL(do_gettimeofday);
 
+
+static void __do_settimeofday(struct timekeeper *tk, const struct timespec *tv)
+{
+	struct timespec ts_delta, xt;
+
+	timekeeping_forward_now(tk);
+
+	xt = tk_xtime(tk);
+	ts_delta.tv_sec = tv->tv_sec - xt.tv_sec;
+	ts_delta.tv_nsec = tv->tv_nsec - xt.tv_nsec;
+
+	tk_set_wall_to_mono(tk, timespec_sub(tk->wall_to_monotonic, ts_delta));
+
+	tk_set_xtime(tk, tv);
+
+	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET);
+}
+
+
 /**
  * do_settimeofday - Sets the time of day
  * @tv:		pointer to the timespec variable containing the new time
@@ -496,7 +515,6 @@ EXPORT_SYMBOL(do_gettimeofday);
 int do_settimeofday(const struct timespec *tv)
 {
 	struct timekeeper *tk = &timekeeper;
-	struct timespec ts_delta, xt;
 	unsigned long flags;
 
 	if (!timespec_valid_strict(tv))
@@ -505,27 +523,43 @@ int do_settimeofday(const struct timespec *tv)
 	raw_spin_lock_irqsave(&timekeeper_lock, flags);
 	write_seqcount_begin(&timekeeper_seq);
 
-	timekeeping_forward_now(tk);
+	__do_settimeofday(tk, tv);
 
-	xt = tk_xtime(tk);
-	ts_delta.tv_sec = tv->tv_sec - xt.tv_sec;
-	ts_delta.tv_nsec = tv->tv_nsec - xt.tv_nsec;
+	write_seqcount_end(&timekeeper_seq);
+	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
 
-	tk_set_wall_to_mono(tk, timespec_sub(tk->wall_to_monotonic, ts_delta));
+	/* signal hrtimers about time change */
+	clock_was_set();
 
-	tk_set_xtime(tk, tv);
+	return 0;
+}
+EXPORT_SYMBOL(do_settimeofday);
 
-	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET);
 
+int do_first_settimeofday(const struct timespec *tv)
+{
+	struct timekeeper *tk = &timekeeper;
+	unsigned long flags;
+	int ret = 0;
+
+	if (!timespec_valid_strict(tv))
+		return -EINVAL;
+
+	raw_spin_lock_irqsave(&timekeeper_lock, flags);
+	write_seqcount_begin(&timekeeper_seq);
+
+	if (!tk->wall_to_monotonic.tv_sec && !tk->wall_to_monotonic.tv_nsec)
+		__do_settimeofday(tk, tv);
+	else
+		ret = -EACCES;
 	write_seqcount_end(&timekeeper_seq);
 	raw_spin_unlock_irqrestore(&timekeeper_lock, flags);
 
 	/* signal hrtimers about time change */
 	clock_was_set();
 
-	return 0;
+	return ret;
 }
-EXPORT_SYMBOL(do_settimeofday);
 
 /**
  * timekeeping_inject_offset - Adds or subtracts from the current time.
-- 
1.9.1

[PATCH 2/2][RFC] rtc: Rework hctosys so that it is called on RTC registration

[John Stultz]

Rather then calling hctosys at late_init, when RTC modules may not
yet be loaded, call hctosys on each RTC device registration.

We also change hctosys to usilize do_first_settimeofday() in
order to make sure the time is only initialized once by an RTC
and doesn't overwrite any time changes already made by userspace.

Cc: John Whitmore <arigead@gmail.com>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexander Holler <holler@ahsoftware.de>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/rtc/class.c   |  2 ++
 drivers/rtc/hctosys.c | 19 ++++++++-----------
 include/linux/rtc.h   |  6 ++++++
 3 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
index 589351e..6986529 100644
--- a/drivers/rtc/class.c
+++ b/drivers/rtc/class.c
@@ -237,6 +237,8 @@ struct rtc_device *rtc_device_register(const char *name, struct device *dev,
 	dev_info(dev, "rtc core: registered %s as %s\n",
 			rtc->name, dev_name(&rtc->dev));
 
+	rtc_hctosys();
+
 	return rtc;
 
 exit_kfree:
diff --git a/drivers/rtc/hctosys.c b/drivers/rtc/hctosys.c
index 4aa60d7..72be031 100644
--- a/drivers/rtc/hctosys.c
+++ b/drivers/rtc/hctosys.c
@@ -22,7 +22,7 @@
  * the best guess is to add 0.5s.
  */
 
-static int __init rtc_hctosys(void)
+int rtc_hctosys(void)
 {
 	int err = -ENODEV;
 	struct rtc_time tm;
@@ -54,14 +54,13 @@ static int __init rtc_hctosys(void)
 
 	rtc_tm_to_time(&tm, &tv.tv_sec);
 
-	err = do_settimeofday(&tv);
-
-	dev_info(rtc->dev.parent,
-		"setting system clock to "
-		"%d-%02d-%02d %02d:%02d:%02d UTC (%u)\n",
-		tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
-		tm.tm_hour, tm.tm_min, tm.tm_sec,
-		(unsigned int) tv.tv_sec);
+	if (!do_first_settimeofday(&tv))
+		dev_info(rtc->dev.parent,
+			"setting system clock to "
+			"%d-%02d-%02d %02d:%02d:%02d UTC (%u)\n",
+			tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
+			tm.tm_hour, tm.tm_min, tm.tm_sec,
+			(unsigned int) tv.tv_sec);
 
 err_invalid:
 err_read:
@@ -72,5 +71,3 @@ err_open:
 
 	return err;
 }
-
-late_initcall(rtc_hctosys);
diff --git a/include/linux/rtc.h b/include/linux/rtc.h
index c2c2897..a524cc6 100644
--- a/include/linux/rtc.h
+++ b/include/linux/rtc.h
@@ -191,6 +191,12 @@ static inline bool is_leap_year(unsigned int year)
 	return (!(year % 4) && (year % 100)) || !(year % 400);
 }
 
+#ifdef CONFIG_RTC_SYSTOHC
+int rtc_hctosys(void);
+#else
+#define rtc_hctosys() 0
+#endif
+
 #ifdef CONFIG_RTC_HCTOSYS_DEVICE
 extern int rtc_hctosys_ret;
 #else
-- 
1.9.1

Re: [PATCH 0/2][RFC] Try to handle hctosys w/ rtc modules

[Alexander Holler]

Am 27.06.2014 19:27, schrieb John Stultz:
> Its been pointed out that the RTC hctosys functionality doesn't
> work well with RTC modules, which may not be loaded until after
> late_init().
>
> While there have been other attempts to sovle this, this patchset
> is a very quick 10 minute effort to show how I'd try to resolve
> this. There likely are still issues here, but I'd be happy to
> make fixes and adjustments to ensure it works.
>
> Feedback and comments always appreciated!

And it still uses the non-deterministic and therefor almost unusable 
rtcN. Well done.

Re: [PATCH 0/2][RFC] Try to handle hctosys w/ rtc modules

[Alexander Holler]

Am 28.06.2014 09:18, schrieb Alexander Holler:
> Am 27.06.2014 19:27, schrieb John Stultz:
>> Its been pointed out that the RTC hctosys functionality doesn't
>> work well with RTC modules, which may not be loaded until after
>> late_init().
>>
>> While there have been other attempts to sovle this, this patchset
>> is a very quick 10 minute effort to show how I'd try to resolve
>> this. There likely are still issues here, but I'd be happy to
>> make fixes and adjustments to ensure it works.

How long you needed to getthe idea?

>>
>> Feedback and comments always appreciated!
>
> And it still uses the non-deterministic and therefor almost unusable
> rtcN. Well done.

Besides that the current hctosys-mechanism doesn't really work with 
hot-plugable devices at all. Guess what N will be when you unplug and 
plug in such a RTC again.

Looks good but doesn't work => high quality. ;)

Re: [PATCH 0/2][RFC] Try to handle hctosys w/ rtc modules

[Marc Dietrich]

On Sat, 28 Jun 2014 09:32:50 +0200
Alexander Holler <holler@ahsoftware.de> wrote:

> Am 28.06.2014 09:18, schrieb Alexander Holler:
> > Am 27.06.2014 19:27, schrieb John Stultz:
> >> Its been pointed out that the RTC hctosys functionality doesn't
> >> work well with RTC modules, which may not be loaded until after
> >> late_init().
> >>
> >> While there have been other attempts to sovle this, this patchset
> >> is a very quick 10 minute effort to show how I'd try to resolve
> >> this. There likely are still issues here, but I'd be happy to
> >> make fixes and adjustments to ensure it works.
> 
> How long you needed to getthe idea?
> 
> >>
> >> Feedback and comments always appreciated!
> >
> > And it still uses the non-deterministic and therefor almost unusable
> > rtcN. Well done.
> 
> Besides that the current hctosys-mechanism doesn't really work with 
> hot-plugable devices at all. Guess what N will be when you unplug and 
> plug in such a RTC again.

We have a patch in the kernel which binds the rtc number to the
hw device, so this even works for hotpluggable devices (at least on
systems supporting device-tree). Not sure what your needs are.

Marc

Re: [PATCH 0/2][RFC] Try to handle hctosys w/ rtc modules

[Alexander Holler]

Am 28.06.2014 20:54, schrieb Marc Dietrich:
> On Sat, 28 Jun 2014 09:32:50 +0200
> Alexander Holler <holler@ahsoftware.de> wrote:
>
>> Am 28.06.2014 09:18, schrieb Alexander Holler:
>>> Am 27.06.2014 19:27, schrieb John Stultz:
>>>> Its been pointed out that the RTC hctosys functionality doesn't
>>>> work well with RTC modules, which may not be loaded until after
>>>> late_init().
>>>>
>>>> While there have been other attempts to sovle this, this patchset
>>>> is a very quick 10 minute effort to show how I'd try to resolve
>>>> this. There likely are still issues here, but I'd be happy to
>>>> make fixes and adjustments to ensure it works.
>>
>> How long you needed to getthe idea?
>>
>>>>
>>>> Feedback and comments always appreciated!
>>>
>>> And it still uses the non-deterministic and therefor almost unusable
>>> rtcN. Well done.
>>
>> Besides that the current hctosys-mechanism doesn't really work with
>> hot-plugable devices at all. Guess what N will be when you unplug and
>> plug in such a RTC again.
>
> We have a patch in the kernel which binds the rtc number to the
> hw device, so this even works for hotpluggable devices (at least on
> systems supporting device-tree). Not sure what your needs are.

First, the number depends on the kernel-configuration and the order how 
RTCs are detected and might even change between minor kernel versions. 
Thus it's totally non-deterministic across different kernel builds.

Second, I wonder about which hotpluggable devices you are talking. Last 
time I've dived into that part of the kernel was when I've written my 
fully working patch set and at that time a usb-rtc (the only really 
hotpluggable RTC I currently know about) did get a new N whenever it 
appeared again (e.g. on suspend/resume). So with the proposed 
non-working clone of my patch, the RTC would be gone after a resume.

Anyway, sorry, I won't spend more time on discussing with kernel 
maintainers. I don't receive any compensations to do so and therefor I 
absolutely have no reason to accept all the pain and insults which are 
happening when doing so. I've come to the conclusion that I should not 
have posted patches at all (not just this one) and really regret that.

Alexander Holler

Re: [PATCH 0/2][RFC] Try to handle hctosys w/ rtc modules

[Alessandro Zummo]

On Sat, 28 Jun 2014 20:54:33 +0200
Marc Dietrich <marvin24@gmx.de> wrote:

> > Besides that the current hctosys-mechanism doesn't really work with 
> > hot-plugable devices at all. Guess what N will be when you unplug and 
> > plug in such a RTC again.  
> 
> We have a patch in the kernel which binds the rtc number to the
> hw device, so this even works for hotpluggable devices (at least on
> systems supporting device-tree). Not sure what your needs are.

 I'm still not sure about how to solve this, maybe it would be better to
 identify the hctosys device that should be used at boot with 
 a different identifier than rtcX (and move the option to the kernel command line)

-- 

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Torino, Italy

  http://www.towertech.it

Re: [PATCH 0/2][RFC] Try to handle hctosys w/ rtc modules

[Alexander Holler]

Am 30.06.2014 21:37, schrieb Alessandro Zummo:
> On Sat, 28 Jun 2014 20:54:33 +0200
> Marc Dietrich <marvin24@gmx.de> wrote:
>
>>> Besides that the current hctosys-mechanism doesn't really work with
>>> hot-plugable devices at all. Guess what N will be when you unplug and
>>> plug in such a RTC again.
>>
>> We have a patch in the kernel which binds the rtc number to the
>> hw device, so this even works for hotpluggable devices (at least on
>> systems supporting device-tree). Not sure what your needs are.
>
>   I'm still not sure about how to solve this, maybe it would be better to
>   identify the hctosys device that should be used at boot with
>   a different identifier than rtcX (and move the option to the kernel command line)

Just in case some missunderstood me. I didn't talk about the userland 
but the in-kernel stuff. Currently, it uses the N which was defined by 
in the config (or zero) on resume to update the time. If you look at my 
3. patch, you will see I have changed this there to use the driver (by 
name) which was used at boot to set the time.

Fixing userspace would be easy too by just changing /dev/rtcN to 
/dev/rtc_drivername. But I didn't that in my 3 patches in order to not 
confuse maintainers (and userland people) with too much changes or new 
stuff at once (unsucessfully, but ...).

There still would be a problem about how to handle two RTCs which do use 
the same driver, but I think this is fictitious scenario (and broken if 
that should be for backup purposes) and therefor if someone would really 
design such a system, he should solve this problem himself.

Alexander Holler