2014-06-27 17:27:31

by John Stultz

[permalink] [raw]
Subject: [PATCH 0/2][RFC] Try to handle hctosys w/ rtc modules

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 <[email protected]>
Cc: Alessandro Zummo <[email protected]>
Cc: Alexander Holler <[email protected]>

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


2014-06-27 17:27:29

by John Stultz

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

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 <[email protected]>
Cc: Alessandro Zummo <[email protected]>
Cc: Alexander Holler <[email protected]>
Signed-off-by: John Stultz <[email protected]>
---
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

2014-06-27 17:27:27

by John Stultz

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

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 <[email protected]>
Cc: Alessandro Zummo <[email protected]>
Cc: Alexander Holler <[email protected]>
Signed-off-by: John Stultz <[email protected]>
---
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

2014-06-28 07:18:30

by Alexander Holler

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

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.

2014-06-28 07:33:00

by Alexander Holler

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

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. ;)

2014-06-28 18:54:51

by Marc Dietrich

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

On Sat, 28 Jun 2014 09:32:50 +0200
Alexander Holler <[email protected]> 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

2014-06-28 20:50:38

by Alexander Holler

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

Am 28.06.2014 20:54, schrieb Marc Dietrich:
> On Sat, 28 Jun 2014 09:32:50 +0200
> Alexander Holler <[email protected]> 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

2014-06-30 19:37:21

by Alessandro Zummo

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

On Sat, 28 Jun 2014 20:54:33 +0200
Marc Dietrich <[email protected]> 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

2014-07-01 18:42:51

by Alexander Holler

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

Am 30.06.2014 21:37, schrieb Alessandro Zummo:
> On Sat, 28 Jun 2014 20:54:33 +0200
> Marc Dietrich <[email protected]> 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