NTP: Add a CONFIG_RTC_SYSTOHC configuration

PATCH Superseded Jason Gunthorpe 2 participants 3 messages Dec 14, 2012 – Dec 17, 2012

Subsystems (3)

PowerPC Real time clock (RTC) Timekeeping, clocksource core, NTP, alarmtimer

lore

Share & tools

lore

Jason Gunthorpe

author

2012-12-14 23:19 UTC #0 lore raw

PATCH v2 1/1 NTP: Add a CONFIG_RTC_SYSTOHC configuration

The purpose of this option is to allow ARM/etc systems that rely on the class RTC subsystem to have the same kind of automatic NTP based synchronization that we have on PC platforms. Today ARM does not implement update_persistent_clock and makes extensive use of the class RTC system.

When enabled CONFIG_RTC_SYSTOHC will provide a generic rtc_update_persistent_clock that stores the current time in the RTC and is intended complement the existing CONFIG_RTC_HCTOSYS option that loads the RTC at boot.

Like with RTC_HCTOSYS the platform's update_persistent_clock is used first, if it works. Platforms with mixed class RTC and non-RTC drivers need to return ENODEV when class RTC should be used. Such an update for PPC is included in this patch.

Long term, implementations of update_persistent_clock should migrate to proper class RTC drivers and use CONFIG_RTC_SYSTOHC instead.

Tested on ARM kirkwood and PPC405

Signed-off-by Jason Gunthorpe <[email protected]>

---
 arch/powerpc/kernel/time.c |    2 +-
 drivers/rtc/Kconfig        |    9 +++++++++
 drivers/rtc/Makefile       |    1 +
 drivers/rtc/systohc.c      |   44 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/time.h       |    1 +
 kernel/time/ntp.c          |   15 +++++++++++----
 6 files changed, 67 insertions(+), 5 deletions(-)
 create mode 100644 drivers/rtc/systohc.c

v2 updates:
- Be very careful to return ENODEV from rtc_update_persistent_clock, we don't want to loop on the fast retry path of sync_cmos_clock if there is no RTC set support available
- Call the platform update_persistent_clock first. Only try the RTC version if it is compiled out, or explicitly returns ENODEV
- Added 'depends on RTC_SYSTOHC = y' to KConfig
- Don't fast rety in sync_cmos_clock if ENODEV is returned
- Update PPC to return ENODEV if there is no mach specific function available in ppc_md. This will give rtc_update_persistent_clock a chance.
- Use rtc_set_time not mms since rtc_hctosys assumes UTC.

arch/powerpc/kernel/time.c +1 -1

index ce4cb77..bc844a8 100644
@@ -667,7 +667,7 @@ int update_persistent_clock(struct timespec now)
 	struct rtc_time tm;
 
 	if (!ppc_md.set_rtc_time)
-		return 0;
+		return -ENODEV;
 
 	to_tm(now.tv_sec + 1 + timezone_offset, &tm);
 	tm.tm_year -= 1900;

drivers/rtc/Kconfig +9 -0

index 19c03ab..7b3702b 100644
@@ -28,6 +28,7 @@ config RTC_HCTOSYS
 config RTC_HCTOSYS_DEVICE
 	string "RTC used to set the system time"
 	depends on RTC_HCTOSYS = y
+	depends on RTC_SYSTOHC = y
 	default "rtc0"
 	help
 	  The RTC device that will be used to (re)initialize the system

@@ -48,6 +49,14 @@ 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 the RTC time based on NTP synchronization"
+	default y
+	help
+	  If you say yes here, the system time (wall clock) will be stored
+          in the RTC specified by RTC_HCTOSYS_DEVICE approximately every 11
+  	  minutes if userspace reports synchronized NTP status.
+
 config RTC_DEBUG
 	bool "RTC debug support"
 	help

drivers/rtc/Makefile +1 -0

index 56297f0..69d11f1 100644
@@ -6,6 +6,7 @@ ccflags-$(CONFIG_RTC_DEBUG)	:= -DDEBUG
 
 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

drivers/rtc/systohc.c new file +44 -0

44 lines not on this page · raw message

include/linux/time.h +1 -0

index 4d358e9..d668f9c 100644
@@ -118,6 +118,7 @@ static inline bool timespec_valid_strict(const struct timespec *ts)
 extern void read_persistent_clock(struct timespec *ts);
 extern void read_boot_clock(struct timespec *ts);
 extern int update_persistent_clock(struct timespec now);
+extern int rtc_update_persistent_clock(struct timespec now);
 void timekeeping_init(void);
 extern int timekeeping_suspended;

kernel/time/ntp.c +11 -4

index 24174b4..0c5d1a1 100644
@@ -483,8 +483,7 @@ out:
 	return leap;
 }
 
-#ifdef CONFIG_GENERIC_CMOS_UPDATE
-
+#if defined(CONFIG_GENERIC_CMOS_UPDATE) || defined(CONFIG_RTC_SYSTOHC)
 static void sync_cmos_clock(struct work_struct *work);
 
 static DECLARE_DELAYED_WORK(sync_cmos_work, sync_cmos_clock);

@@ -510,14 +509,22 @@ static void sync_cmos_clock(struct work_struct *work)
 	}
 
 	getnstimeofday(&now);
-	if (abs(now.tv_nsec - (NSEC_PER_SEC / 2)) <= tick_nsec / 2)
+	if (abs(now.tv_nsec - (NSEC_PER_SEC / 2)) <= tick_nsec / 2) {
+		fail = -ENODEV;
+#ifdef CONFIG_GENERIC_CMOS_UPDATE
 		fail = update_persistent_clock(now);
+#endif
+#ifdef CONFIG_RTC_SYSTOHC
+		if (fail == -ENODEV)
+			fail = rtc_update_persistent_clock(now);
+#endif
+	}
 
 	next.tv_nsec = (NSEC_PER_SEC / 2) - now.tv_nsec - (TICK_NSEC / 2);
 	if (next.tv_nsec <= 0)
 		next.tv_nsec += NSEC_PER_SEC;
 
-	if (!fail)
+	if (!fail || fail == -ENODEV)
 		next.tv_sec = 659;
 	else
 		next.tv_sec = 0;

Signature · 1 line

1.7.5.4

John Stultz

2012-12-14 23:41 UTC #1 lore raw

Quoted 11 lines · Jason Gunthorpe

The purpose of this option is to allow ARM/etc systems that rely on the class RTC subsystem to have the same kind of automatic NTP based synchronization that we have on PC platforms. Today ARM does not implement update_persistent_clock and makes extensive use of the class RTC system.
When enabled CONFIG_RTC_SYSTOHC will provide a generic rtc_update_persistent_clock that stores the current time in the RTC and is intended complement the existing CONFIG_RTC_HCTOSYS option that loads the RTC at boot.

Quoted 7 lines

Like with RTC_HCTOSYS the platform's update_persistent_clock is used first, if it works. Platforms with mixed class RTC and non-RTC drivers need to return ENODEV when class RTC should be used. Such an update for PPC is included in this patch.
Long term, implementations of update_persistent_clock should migrate to proper class RTC drivers and use CONFIG_RTC_SYSTOHC instead.

Ok. This all sounds good.

Still one minor question below.

drivers/rtc/Kconfig config RTC_HCTOSYS · line 31

Quoted 9 lines

drivers/rtc/Kconfig +1 -0

index 19c03ab..7b3702b 100644
@@ -28,6 +28,7 @@ config RTC_HCTOSYS
  config RTC_HCTOSYS_DEVICE
  	string "RTC used to set the system time"
  	depends on RTC_HCTOSYS = y
+	depends on RTC_SYSTOHC = y

Is this right?

This should probably be a depends on RTC_HCTOSYS OR RTC_SYSTOCH.. Otherwise you have to select both in order to change the default device.

Quoted 19 lines

drivers/rtc/systohc.c new file +13 -0

new file mode 100644
index 0000000..a625740
@@ -0,0 +1,44 @@
+/*
+ * 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>
+#include <linux/time.h>
+
+/**
+ * rtc_update_persistent_clock - Save NTP synchronized time to the RTC
+ * @now: Current time of day
+ *

drivers/rtc/systohc.c line 23

Quoted 10 lines

drivers/rtc/systohc.c new file +10 -0

+ * Replacement for the NTP platform function update_persistent_clock
+ * that stores time for later retrieval by rtc_hctosys
+ *
+ * Returns 0 on successful RTC update, -ENODEV if a RTC update is not
+ * possible at all, and various other -errno for specific temporary failure
+ * cases.
+ *
+ * If temporary failure is indicated the caller should try again 'soon'
+ */
+int rtc_update_persistent_clock(struct timespec now)

If we're going to move away from update_persistent_clock across the board (and as the update path doesn't have the same constraints of the read_persistent_clock interface), might it be better just to name this:
rtc_update_clock() (or something similar)?

That way if/when we do finally remove the other users of update_persistent_clock() and move them to an RTC driver, we will avoid any confusion between read/update.

This is in the similar vein of your suggestion of changing update_persistent_clock to platform_save_ntp_time_to_rtc()..

Sorry for the last minute nit! Other then these two issues, I'm happy to queue this (if Alessandro doesn't object). Although with the merge window already open it may have to wait to 3.9, but I'll see what Thomas says.

thanks
-john

Jason Gunthorpe

author

2012-12-17 21:30 UTC #2 lore raw

PATCH v3 1/1 NTP: Add a CONFIG_RTC_SYSTOHC configuration

Long term, implementations of update_persistent_clock should migrate to proper class RTC drivers and use CONFIG_RTC_SYSTOHC instead.

Tested on ARM kirkwood and PPC405

Signed-off-by Jason Gunthorpe <[email protected]>

---
 arch/powerpc/kernel/time.c |    2 +-
 drivers/rtc/Kconfig        |   10 +++++++++-
 drivers/rtc/Makefile       |    1 +
 drivers/rtc/systohc.c      |   44 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/rtc.h        |    1 +
 kernel/time/ntp.c          |   16 ++++++++++++----
 6 files changed, 68 insertions(+), 6 deletions(-)
 create mode 100644 drivers/rtc/systohc.c

v3 changes:
- Test menuconfig, reorder options to look better, fix kconfig RTC_HCTOSYS_DEVICE to be 'or' not 'and'
- Change the function name to rtc_set_ntp_time to avoid confusing this api with the distinct *_persistant_clock stuff
- Fix typo in the ENODEV test to be || not &&
- Fully retest on ARM+PPC

arch/powerpc/kernel/time.c +1 -1

index ce4cb77..bc844a8 100644
@@ -667,7 +667,7 @@ int update_persistent_clock(struct timespec now)
 	struct rtc_time tm;
 
 	if (!ppc_md.set_rtc_time)
-		return 0;
+		return -ENODEV;
 
 	to_tm(now.tv_sec + 1 + timezone_offset, &tm);
 	tm.tm_year -= 1900;

drivers/rtc/Kconfig +9 -1

index 19c03ab..b377e96 100644
@@ -25,9 +25,17 @@ config RTC_HCTOSYS
 	  the value read from a specified RTC device. This is useful to avoid
 	  unnecessary fsck runs at boot time, and to network better.
 
+config RTC_SYSTOHC
+	bool "Set the RTC time based on NTP synchronization"
+	default y
+	help
+	  If you say yes here, the system time (wall clock) will be stored
+	  in the RTC specified by RTC_HCTOSYS_DEVICE approximately every 11
+	  minutes if userspace reports synchronized NTP status.
+
 config RTC_HCTOSYS_DEVICE
 	string "RTC used to set the system time"
-	depends on RTC_HCTOSYS = y
+	depends on RTC_HCTOSYS = y || RTC_SYSTOHC = y
 	default "rtc0"
 	help
 	  The RTC device that will be used to (re)initialize the system

drivers/rtc/Makefile +1 -0

index 56297f0..69d11f1 100644
@@ -6,6 +6,7 @@ ccflags-$(CONFIG_RTC_DEBUG)	:= -DDEBUG
 
 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

drivers/rtc/systohc.c new file +44 -0

44 lines not on this page · raw message

include/linux/rtc.h +1 -0

index 9531845..11d05f9 100644
@@ -138,6 +138,7 @@ extern void rtc_device_unregister(struct rtc_device *rtc);
 extern int rtc_read_time(struct rtc_device *rtc, struct rtc_time *tm);
 extern int rtc_set_time(struct rtc_device *rtc, struct rtc_time *tm);
 extern int rtc_set_mmss(struct rtc_device *rtc, unsigned long secs);
+extern int rtc_set_ntp_time(struct timespec now);
 int __rtc_read_alarm(struct rtc_device *rtc, struct rtc_wkalrm *alarm);
 extern int rtc_read_alarm(struct rtc_device *rtc,
 			struct rtc_wkalrm *alrm);

kernel/time/ntp.c +12 -4

index 24174b4..313b161 100644
@@ -15,6 +15,7 @@
 #include <linux/time.h>
 #include <linux/mm.h>
 #include <linux/module.h>
+#include <linux/rtc.h>
 
 #include "tick-internal.h"

@@ -483,8 +484,7 @@ out:
 	return leap;
 }
 
-#ifdef CONFIG_GENERIC_CMOS_UPDATE
-
+#if defined(CONFIG_GENERIC_CMOS_UPDATE) || defined(CONFIG_RTC_SYSTOHC)
 static void sync_cmos_clock(struct work_struct *work);
 
 static DECLARE_DELAYED_WORK(sync_cmos_work, sync_cmos_clock);

@@ -510,14 +510,22 @@ static void sync_cmos_clock(struct work_struct *work)
 	}
 
 	getnstimeofday(&now);
-	if (abs(now.tv_nsec - (NSEC_PER_SEC / 2)) <= tick_nsec / 2)
+	if (abs(now.tv_nsec - (NSEC_PER_SEC / 2)) <= tick_nsec / 2) {
+		fail = -ENODEV;
+#ifdef CONFIG_GENERIC_CMOS_UPDATE
 		fail = update_persistent_clock(now);
+#endif
+#ifdef CONFIG_RTC_SYSTOHC
+		if (fail == -ENODEV)
+			fail = rtc_set_ntp_time(now);
+#endif
+	}
 
 	next.tv_nsec = (NSEC_PER_SEC / 2) - now.tv_nsec - (TICK_NSEC / 2);
 	if (next.tv_nsec <= 0)
 		next.tv_nsec += NSEC_PER_SEC;
 
-	if (!fail)
+	if (!fail || fail == -ENODEV)
 		next.tv_sec = 659;
 	else
 		next.tv_sec = 0;

Signature · 1 line

1.7.5.4