Message ID | 20081110154026.21405.47457.stgit@i1501.lan.towertech.it (mailing list archive) |
---|---|
State | Not Applicable, archived |
Headers | show |
On Mon, 2008-11-10 at 16:40 +0100, Alessandro Zummo wrote: > Adds in-kernel hctosys functionality that can > be used by ntp sync code. > > This is an RFC and has not been tested, I just want > to check if something similar could solve the problems > of those who want the NTP sync mode. You might do better to open the device once and keep it open, rather than taking the mutex and opening it again _during_ each call? You're going to be perturbing the timing by doing that. I believe you were also concerned that some device wouldn't want the behaviour given by the existing sync_cmos_clock() function and workqueue stuff in kernel/ntp.c, where we update the clock precisely half-way through the second? We should probably rip that code out of ntp.c (or just disable it ifdef CONFIG_RTC_CLASS), and provide our own version of notify_cmos_timer(). The workqueue stuff to trigger at half-past the second could be kept as a library function which most RTC devices would use, but they'd have the option to use their own instead.
On Tue, Nov 11, 2008 at 02:10:39PM +0000, David Woodhouse wrote: > On Mon, 2008-11-10 at 16:40 +0100, Alessandro Zummo wrote: > > Adds in-kernel hctosys functionality that can > > be used by ntp sync code. > > > > This is an RFC and has not been tested, I just want > > to check if something similar could solve the problems > > of those who want the NTP sync mode. > > You might do better to open the device once and keep it open, rather > than taking the mutex and opening it again _during_ each call? You're > going to be perturbing the timing by doing that. > > I believe you were also concerned that some device wouldn't want the > behaviour given by the existing sync_cmos_clock() function and workqueue > stuff in kernel/ntp.c, where we update the clock precisely half-way > through the second? FYI, the RTC that are in my PPC machines definitely want an update on the whole second, within a few hundred microseconds that is, since it seems that not all LSB bits are reset by a write of the time: the jitter is much higher than the 31µs peak-peak you'd expect from a 32768Hz crystal but I can't remember how much I found. Regards, Gabriel
On Tue, 11 Nov 2008 14:10:39 +0000 David Woodhouse <dwmw2@infradead.org> wrote: > You might do better to open the device once and keep it open, rather > than taking the mutex and opening it again _during_ each call? You're > going to be perturbing the timing by doing that. If you keep it open no other in-kernel user would be able to use it. > I believe you were also concerned that some device wouldn't want the > behaviour given by the existing sync_cmos_clock() function and workqueue > stuff in kernel/ntp.c, where we update the clock precisely half-way > through the second? > > We should probably rip that code out of ntp.c (or just disable it ifdef > CONFIG_RTC_CLASS), and provide our own version of notify_cmos_timer(). > > The workqueue stuff to trigger at half-past the second could be kept as > a library function which most RTC devices would use, but they'd have the > option to use their own instead. I'll give it a look.
On Tuesday 11 November 2008, David Woodhouse wrote: > I believe you were also concerned that some device wouldn't want the > behaviour given by the existing sync_cmos_clock() function and workqueue > stuff in kernel/ntp.c, where we update the clock precisely half-way > through the second? That's a problem, yes. I've never heard of any RTC that wants such delays ... except for the PC's "CMOS" RTC. There was some discussion about how to expose that knowledge to userspace too. The "hwclock" utility always imposes that delay, and it shouldn't (except when talking to a PC RTC). > We should probably rip that code out of ntp.c (or just disable it ifdef > CONFIG_RTC_CLASS), and provide our own version of notify_cmos_timer(). My suggestion for in-kernel code is that "struct rtc_device" just grow a field like "unsigned settime_delay_msec" which would never be initialized except by rtc-cmos (which uses 500) ... and the NTP sync code would use that value. For out-of-kernel use, that value could be extracted with an ioctl(), and used similarly. > The workqueue stuff to trigger at half-past the second could be kept as > a library function which most RTC devices would use, but they'd have the > option to use their own instead. I thought the workqueue stuff was primarily there to make sure that RTC was always updated in task context -- so it can grab the relevant mutex -- and the half-second delay was legacy PC code ... - Dave
On Tue, Nov 11, 2008 at 12:58:59PM -0800, David Brownell wrote: > On Tuesday 11 November 2008, David Woodhouse wrote: > > I believe you were also concerned that some device wouldn't want the > > behaviour given by the existing sync_cmos_clock() function and workqueue > > stuff in kernel/ntp.c, where we update the clock precisely half-way > > through the second? > > That's a problem, yes. I've never heard of any RTC that wants > such delays ... except for the PC's "CMOS" RTC. Indeed, they are the oddball, although very frequent. Could it be made a constant (500msec on x86, 0 on all other architectures) ? > > There was some discussion about how to expose that knowledge to > userspace too. The "hwclock" utility always imposes that delay, > and it shouldn't (except when talking to a PC RTC). > > > We should probably rip that code out of ntp.c (or just disable it ifdef > > CONFIG_RTC_CLASS), and provide our own version of notify_cmos_timer(). > > My suggestion for in-kernel code is that "struct rtc_device" just > grow a field like "unsigned settime_delay_msec" which would never > be initialized except by rtc-cmos (which uses 500) ... and the NTP > sync code would use that value. Hmm, I believed that most internal interfaces are now using nanoseconds for subsecond fields and values; 500000000 also fits in 32 bits and may avoid some unnecessary arithmetic. It is obviously overkill with 32768Hz crystals but consistency is nice. > > For out-of-kernel use, that value could be extracted with an ioctl(), > and used similarly. > > > > The workqueue stuff to trigger at half-past the second could be kept as > > a library function which most RTC devices would use, but they'd have the > > option to use their own instead. > > I thought the workqueue stuff was primarily there to make sure > that RTC was always updated in task context -- so it can grab the > relevant mutex -- and the half-second delay was legacy PC code ... Please allow configs that only use RTC internally without exposing the driver to userspace. At least that's how I use the RTC, I have some messages on shutdown telling me that the clock could not be accessed but I don't care since all these machines have ntp and the RTC is completely useless for anything else than setting the time quite precisely on reboot (on some of the boards I have, the RTC interrupt is not even wired, so no alarm, no periodic interrupt). Gabriel
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig index 8abbb20..1ff9427 100644 --- a/drivers/rtc/Kconfig +++ b/drivers/rtc/Kconfig @@ -30,7 +30,7 @@ config RTC_HCTOSYS unnecessary fsck runs at boot time, and to network better. config RTC_HCTOSYS_DEVICE - string "RTC used to set the system time" + string "RTC used to set the system time on startup and resume" depends on RTC_HCTOSYS = y default "rtc0" help @@ -52,6 +52,23 @@ config RTC_HCTOSYS_DEVICE sleep states. Do not specify an RTC here unless it stays powered during all this system's supported sleep states. +config RTC_SYSTOHC + bool "Set RTC from system time in NTP sync mode" + depends on RTC_CLASS = y + default y + help + If you say yes here, the system time (wall clock) will be written + to the hardware clock every 11 minutes, if the kernel is in NTP + mode and your platforms supports it. + +config RTC_SYSTOHC_DEVICE + string "RTC used to save the system time in NTP sync mode" + depends on RTC_SYSTOHC = y + default "rtc0" + help + The RTC device that will get written with the system time + in NTP mode. + config RTC_DEBUG bool "RTC debug support" depends on RTC_CLASS = y diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile index e9e8474..6d1c519 100644 --- a/drivers/rtc/Makefile +++ b/drivers/rtc/Makefile @@ -8,6 +8,7 @@ endif obj-$(CONFIG_RTC_LIB) += rtc-lib.o obj-$(CONFIG_RTC_HCTOSYS) += hctosys.o +obj-$(CONFIG_RTC_SYSTOHC) += systohc.o obj-$(CONFIG_RTC_CLASS) += rtc-core.o rtc-core-y := class.o interface.o diff --git a/drivers/rtc/systohc.c b/drivers/rtc/systohc.c new file mode 100644 index 0000000..41ec77b --- /dev/null +++ b/drivers/rtc/systohc.c @@ -0,0 +1,35 @@ +/* + * RTC subsystem, systohc for ntp use + * + * Copyright (C) 2008 Tower Technologies + * Author: Alessandro Zummo <a.zummo@towertech.it> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. +*/ + +#include <linux/rtc.h> + +static int rtc_systohc(struct rtc_time *tm) +{ + int err; + struct rtc_time tm; + struct rtc_device *rtc = rtc_class_open(CONFIG_RTC_SYSTOHC_DEVICE); + + if (rtc == NULL) { + printk("%s: unable to open rtc device (%s)\n", + __FILE__, CONFIG_RTC_SYSTOHC_DEVICE); + return -ENODEV; + } + + err = rtc_set_time(rtc, tm); + if (err != 0) + dev_err(rtc->dev.parent, + "systohc: unable to set the hardware clock\n"); + + rtc_class_close(rtc); + + return err; +} +EXPORT_SYMBOL(rtc_systohc);
Adds in-kernel hctosys functionality that can be used by ntp sync code. This is an RFC and has not been tested, I just want to check if something similar could solve the problems of those who want the NTP sync mode. Signed-off-by: Alessandro Zummo <a.zummo@towertech.it> Cc: Paul Mundt <lethal@linux-sh.org> Cc: David Brownell <david-b@pacbell.net> Cc: David Woodhouse <dwmw2@infradead.org> --- drivers/rtc/Kconfig | 19 ++++++++++++++++++- drivers/rtc/Makefile | 1 + drivers/rtc/systohc.c | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 1 deletions(-) create mode 100644 drivers/rtc/systohc.c