2010-02-03 21:07:37

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH 0/5] pps: time synchronization over LPT

This patchset adds a complete solution (well, the kernel part) for fast
high-precision time synchronization between computers using their LPT
ports. My tests show that it is able to achieve synchronization in the
order of 1-2 microseconds and keep it for as long as needed. We are
using this facility in my company for distributed real-time simulation.

The idea is to have a single master computer whose local clock is used
as a standard. It then provides time information through both PPS
high-precision signal over LPT and NTP over network. Then any other
computer can combine these two time sources (this is what ntpd can do).
Client-side ntpd constantly checks that the local time divergence from
the master's time is less then 128ms. If this is true it uses PPS
signal to further decrease the difference. ntpd can try to do itself
but it's too slow and imprecise. This is really a job for the kernel.

All stuff related to the PPS signals in Linux kernel is handled by the
code from LinuxPPS project. It can be controlled from user-space
through PPSAPI (RFC 2783) and ntpd can use it to pass control over
synchronization to the kernel. Unfortunately this is an optional
feature of PPSAPI and it was not implemented. Also Linux code from
NTPv4 reference model was missing all the pieces that deal with PPS.
This patchset closes both gaps to achieve the highest precision and
speed. It:

* adds missing hardpps() function to the NTPv4 Linux kernel code

* adds getnstime_raw_and_real() to atomically get both MONOTONIC_RAW
and REALTIME clock timestamps for hardpps()

* adds missing optional PPSAPI method to bind the particular PPS
source to feed information to the time subsystem through hardpps()

Also the actual parallel port PPS generator and client were added. (I
know that not yet mainlined LinuxPPS patches provide an own parallel
port client but it is actually a hack on lp driver which I didn't like
at all.)

This patchset is tested against the vanilla 2.6.32.7 kernel. But we are
actually using it on 2.6.31.12-rt20 rt-preempt kernel most of the time.
Also there is a version which should be applied on top of LinuxPPS out
of tree patches (i.e. all clients and low-level irq timestamps stuff).
Those who are interested in other versions of the patchset can find
them in my git repository:
http://lvk.cs.msu.su/~lasaine/timesync/linux-2.6-timesync.git

Alexander Gordeev (5):
ntp: add hardpps implementation
pps: capture MONOTONIC_RAW timestamps as well
pps: add kernel consumer support
pps: add parallel port PPS signal generator
pps: add parallel port PPS client

Documentation/ioctl/ioctl-number.txt | 2 +-
drivers/pps/Kconfig | 5 +
drivers/pps/Makefile | 1 +
drivers/pps/clients/Kconfig | 16 ++
drivers/pps/clients/Makefile | 9 +
drivers/pps/clients/pps_parport.c | 211 ++++++++++++++++++
drivers/pps/generators/Kconfig | 17 ++
drivers/pps/generators/Makefile | 9 +
drivers/pps/generators/pps_gen_parport.c | 263 +++++++++++++++++++++++
drivers/pps/kapi.c | 45 ++++-
drivers/pps/pps.c | 67 ++++++-
include/linux/pps.h | 7 +
include/linux/pps_kernel.h | 39 ++++-
include/linux/time.h | 2 +
include/linux/timex.h | 1 +
kernel/time/Kconfig | 7 +
kernel/time/ntp.c | 346 +++++++++++++++++++++++++++++-
kernel/time/timekeeping.c | 34 +++
18 files changed, 1065 insertions(+), 16 deletions(-)
create mode 100644 drivers/pps/clients/Kconfig
create mode 100644 drivers/pps/clients/Makefile
create mode 100644 drivers/pps/clients/pps_parport.c
create mode 100644 drivers/pps/generators/Kconfig
create mode 100644 drivers/pps/generators/Makefile
create mode 100644 drivers/pps/generators/pps_gen_parport.c


2010-02-03 21:07:24

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH 5/5] pps: add parallel port PPS client

Add parallel port PPS client. It uses a standard method for capturing
timestamps for assert edge transitions: getting a timestamp soon after
an interrupt has happened. This is not a very precise source of time
information due to interrupt handling delays. However, timestamps for
clear edge transitions are much more precise because the interrupt
handler continuously polls hardware port until the transition is done.
Hardware port operations require only about 1us so the maximum error
should not exceed this value. This was my primary goal when developing
this client.

Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/pps/Kconfig | 2 +
drivers/pps/Makefile | 2 +-
drivers/pps/clients/Kconfig | 16 +++
drivers/pps/clients/Makefile | 9 ++
drivers/pps/clients/pps_parport.c | 211 +++++++++++++++++++++++++++++++++++++
5 files changed, 239 insertions(+), 1 deletions(-)
create mode 100644 drivers/pps/clients/Kconfig
create mode 100644 drivers/pps/clients/Makefile
create mode 100644 drivers/pps/clients/pps_parport.c

diff --git a/drivers/pps/Kconfig b/drivers/pps/Kconfig
index f3b14cd..3b70d9e 100644
--- a/drivers/pps/Kconfig
+++ b/drivers/pps/Kconfig
@@ -31,6 +31,8 @@ config PPS_DEBUG
messages to the system log. Select this if you are having a
problem with PPS support and want to see more of what is going on.

+source drivers/pps/clients/Kconfig
+
source drivers/pps/generators/Kconfig

endmenu
diff --git a/drivers/pps/Makefile b/drivers/pps/Makefile
index b299814..1906eb7 100644
--- a/drivers/pps/Makefile
+++ b/drivers/pps/Makefile
@@ -4,6 +4,6 @@

pps_core-y := pps.o kapi.o sysfs.o
obj-$(CONFIG_PPS) := pps_core.o
-obj-y += generators/
+obj-y += clients/ generators/

ccflags-$(CONFIG_PPS_DEBUG) := -DDEBUG
diff --git a/drivers/pps/clients/Kconfig b/drivers/pps/clients/Kconfig
new file mode 100644
index 0000000..969a486
--- /dev/null
+++ b/drivers/pps/clients/Kconfig
@@ -0,0 +1,16 @@
+#
+# PPS clients configuration
+#
+
+if PPS
+
+comment "PPS clients support"
+
+config PPS_CLIENT_PARPORT
+ tristate "Parallel port PPS client"
+ depends on PPS && PARPORT
+ help
+ If you say yes here you get support for a PPS source connected
+ with the interrupt pin of your parallel port.
+
+endif
diff --git a/drivers/pps/clients/Makefile b/drivers/pps/clients/Makefile
new file mode 100644
index 0000000..95f6fc3
--- /dev/null
+++ b/drivers/pps/clients/Makefile
@@ -0,0 +1,9 @@
+#
+# Makefile for PPS clients.
+#
+
+obj-$(CONFIG_PPS_CLIENT_PARPORT) += pps_parport.o
+
+ifeq ($(CONFIG_PPS_DEBUG),y)
+EXTRA_CFLAGS += -DDEBUG
+endif
diff --git a/drivers/pps/clients/pps_parport.c b/drivers/pps/clients/pps_parport.c
new file mode 100644
index 0000000..390f8e4
--- /dev/null
+++ b/drivers/pps/clients/pps_parport.c
@@ -0,0 +1,211 @@
+/*
+ * pps_parport.c -- kernel parallel port PPS client
+ *
+ *
+ * Copyright (C) 2009 Alexander Gordeev <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+
+/*
+ * TODO:
+ * 1. try using SA_NODELAY for parport irq handler
+ * 2. test under heavy load
+ * 3. implement echo over SEL pin
+ * 4. module parameters
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/irqnr.h>
+#include <linux/time.h>
+#include <linux/parport.h>
+#include <linux/pps_kernel.h>
+
+#define DRVNAME "pps_parport"
+#define DRVDESC "parallel port PPS client"
+
+/* maximum number of port reads when polling for signal clear */
+#define RECEIVE_TIMEOUT 100
+
+/* internal per port structure */
+struct pps_client_pp {
+ struct pardevice *pardev; /* parport device */
+ int source; /* PPS source number */
+};
+
+#define SIGNAL_IS_SET(port) \
+ ((port->ops->read_status(port) & PARPORT_STATUS_ACK) != 0)
+
+/* parport interrupt handler */
+static void parport_irq(void *handle)
+{
+ struct pps_event_time ts_assert, ts_clear;
+ struct pps_client_pp *dev = handle;
+ struct parport *port = dev->pardev->port;
+ int i;
+ unsigned long flags;
+
+ /* first of all we get the time stamp... */
+ pps_get_ts(&ts_assert);
+
+ /* check the signal (no signal means the pulse is lost this time) */
+ if (!SIGNAL_IS_SET(port)) {
+ pr_err(DRVNAME ": lost the signal\n");
+ return;
+ }
+
+ /* FIXME: this is here until we have a fast interrupt */
+ local_irq_save(flags);
+ /* poll the port until the signal is unset */
+ for (i = RECEIVE_TIMEOUT; i; i--)
+ if (!SIGNAL_IS_SET(port)) {
+ pps_get_ts(&ts_clear);
+ local_irq_restore(flags);
+
+ /* FIXME: move these two calls to workqueue? */
+ /* fire assert event */
+ pps_event(dev->source, &ts_assert,
+ PPS_CAPTUREASSERT, NULL);
+ /* fire clear event */
+ pps_event(dev->source, &ts_clear,
+ PPS_CAPTURECLEAR, NULL);
+
+ return;
+ }
+ local_irq_restore(flags);
+
+ /* timeout */
+ pr_err(DRVNAME ": timeout in interrupt handler while waiting"
+ " for signal clear\n");
+}
+
+/* the PPS echo function */
+static void pps_echo(int source, int event, void *data)
+{
+ pr_info("echo %s %s for source %d\n",
+ event & PPS_CAPTUREASSERT ? "assert" : "",
+ event & PPS_CAPTURECLEAR ? "clear" : "",
+ source);
+}
+
+static void parport_attach(struct parport *port)
+{
+ struct pps_client_pp *device;
+ struct pps_source_info info = {
+ .name = DRVNAME,
+ .path = "",
+ .mode = PPS_CAPTUREBOTH | \
+ PPS_OFFSETASSERT | PPS_OFFSETCLEAR | \
+ PPS_ECHOASSERT | PPS_ECHOCLEAR | \
+ PPS_CANWAIT | PPS_TSFMT_TSPEC,
+ .echo = pps_echo,
+ .owner = THIS_MODULE,
+ };
+
+ device = kzalloc(sizeof(struct pps_client_pp), GFP_KERNEL);
+ if (!device) {
+ pr_err(DRVNAME ": memory allocation failed, not attaching\n");
+ return;
+ }
+
+ device->pardev = parport_register_device(port, DRVNAME,
+ NULL, NULL, parport_irq, 0, device);
+ if (!device->pardev) {
+ pr_err(DRVNAME ": couldn't register with %s\n", port->name);
+ goto err_free;
+ }
+
+ if (parport_claim_or_block(device->pardev) < 0) {
+ pr_err(DRVNAME ": couldn't claim %s\n", port->name);
+ goto err_unregister_dev;
+ }
+
+ device->source = pps_register_source(&info,
+ PPS_CAPTUREBOTH | PPS_OFFSETASSERT | PPS_OFFSETCLEAR);
+ if (device->source < 0) {
+ pr_err(DRVNAME ": couldn't register PPS source\n");
+ goto err_release_dev;
+ }
+
+ port->ops->enable_irq(port);
+
+ pr_info(DRVNAME ": attached to %s\n", port->name);
+
+ return;
+
+err_release_dev:
+ parport_release(device->pardev);
+err_unregister_dev:
+ parport_unregister_device(device->pardev);
+err_free:
+ kfree(device);
+}
+
+static void parport_detach(struct parport *port)
+{
+ struct pardevice *pardev = port->cad;
+ struct pps_client_pp *device;
+
+ /* oooh, this is ugly! */
+ if (strcmp(pardev->name, DRVNAME))
+ /* not our port */
+ return;
+
+ device = pardev->private;
+
+ port->ops->disable_irq(port);
+ pps_unregister_source(device->source);
+ parport_release(pardev);
+ parport_unregister_device(pardev);
+ kfree(device);
+}
+
+static struct parport_driver pps_parport_driver = {
+ .name = DRVNAME,
+ .attach = parport_attach,
+ .detach = parport_detach,
+};
+
+/* module staff */
+
+static int __init pps_parport_init(void)
+{
+ int ret;
+
+ pr_info(DRVNAME ": " DRVDESC "\n");
+
+ ret = parport_register_driver(&pps_parport_driver);
+ if (ret) {
+ pr_err(DRVNAME ": unable to register with parport\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static void __exit pps_parport_exit(void)
+{
+ parport_unregister_driver(&pps_parport_driver);
+}
+
+module_init(pps_parport_init);
+module_exit(pps_parport_exit);
+
+MODULE_AUTHOR("Alexander Gordeev <[email protected]>");
+MODULE_DESCRIPTION(DRVDESC);
+MODULE_LICENSE("GPL");
--
1.6.5

2010-02-03 21:07:29

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH 2/5] pps: capture MONOTONIC_RAW timestamps as well

MONOTONIC_RAW clock timestamps are ideally suited for frequency
calculation and also fit well into the original NTP hardpps design. Now
phase and frequency can be adjusted separately: the former based on
REALTIME clock and the latter based on MONOTONIC_RAW clock.
A new function getnstime_raw_and_real is added to timekeeping subsystem
to capture both timestamps at the same time and atomically.

Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/pps/kapi.c | 17 +++++++++++------
include/linux/pps_kernel.h | 33 +++++++++++++++++++++++++++++++--
include/linux/time.h | 2 ++
kernel/time/timekeeping.c | 34 ++++++++++++++++++++++++++++++++++
4 files changed, 78 insertions(+), 8 deletions(-)

diff --git a/drivers/pps/kapi.c b/drivers/pps/kapi.c
index 2d414e2..8a77102 100644
--- a/drivers/pps/kapi.c
+++ b/drivers/pps/kapi.c
@@ -267,7 +267,7 @@ EXPORT_SYMBOL(pps_unregister_source);
* pps->info.echo(source, event, data);
*/

-void pps_event(int source, struct pps_ktime *ts, int event, void *data)
+void pps_event(int source, struct pps_event_time *ts, int event, void *data)
{
struct pps_device *pps;
unsigned long flags;
@@ -284,7 +284,8 @@ void pps_event(int source, struct pps_ktime *ts, int event, void *data)
return;

pr_debug("PPS event on source %d at %llu.%06u\n",
- pps->id, (unsigned long long) ts->sec, ts->nsec);
+ pps->id, (unsigned long long) ts->ts_real.sec,
+ ts->ts_real.nsec);

spin_lock_irqsave(&pps->lock, flags);

@@ -298,10 +299,12 @@ void pps_event(int source, struct pps_ktime *ts, int event, void *data)
(pps->params.mode & PPS_CAPTUREASSERT)) {
/* We have to add an offset? */
if (pps->params.mode & PPS_OFFSETASSERT)
- pps_add_offset(ts, &pps->params.assert_off_tu);
+ pps_add_offset(&ts->ts_real,
+ &pps->params.assert_off_tu);

/* Save the time stamp */
- pps->assert_tu = *ts;
+ pps->assert_tu = ts->ts_real;
+ pps->assert_tu_raw = ts->ts_raw;
pps->assert_sequence++;
pr_debug("capture assert seq #%u for source %d\n",
pps->assert_sequence, source);
@@ -312,10 +315,12 @@ void pps_event(int source, struct pps_ktime *ts, int event, void *data)
(pps->params.mode & PPS_CAPTURECLEAR)) {
/* We have to add an offset? */
if (pps->params.mode & PPS_OFFSETCLEAR)
- pps_add_offset(ts, &pps->params.clear_off_tu);
+ pps_add_offset(&ts->ts_real,
+ &pps->params.clear_off_tu);

/* Save the time stamp */
- pps->clear_tu = *ts;
+ pps->clear_tu = ts->ts_real;
+ pps->clear_tu_raw = ts->ts_raw;
pps->clear_sequence++;
pr_debug("capture clear seq #%u for source %d\n",
pps->clear_sequence, source);
diff --git a/include/linux/pps_kernel.h b/include/linux/pps_kernel.h
index e0a193f..0d36a6f 100644
--- a/include/linux/pps_kernel.h
+++ b/include/linux/pps_kernel.h
@@ -18,6 +18,10 @@
* Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*/

+#ifndef LINUX_PPS_KERNEL_H
+#define LINUX_PPS_KERNEL_H
+
+
#include <linux/pps.h>

#include <linux/cdev.h>
@@ -40,6 +44,11 @@ struct pps_source_info {
struct device *dev;
};

+struct pps_event_time {
+ struct timespec ts_raw;
+ struct pps_ktime ts_real;
+};
+
/* The main struct */
struct pps_device {
struct pps_source_info info; /* PSS source info */
@@ -48,6 +57,8 @@ struct pps_device {

__u32 assert_sequence; /* PPS' assert event seq # */
__u32 clear_sequence; /* PPS' clear event seq # */
+ struct timespec assert_tu_raw;
+ struct timespec clear_tu_raw;
struct pps_ktime assert_tu;
struct pps_ktime clear_tu;
int current_mode; /* PPS mode at event time */
@@ -71,7 +82,6 @@ struct pps_device {

extern spinlock_t pps_idr_lock;
extern struct idr pps_idr;
-extern struct timespec pps_irq_ts[];

extern struct device_attribute pps_attrs[];

@@ -86,4 +96,23 @@ extern int pps_register_source(struct pps_source_info *info,
extern void pps_unregister_source(int source);
extern int pps_register_cdev(struct pps_device *pps);
extern void pps_unregister_cdev(struct pps_device *pps);
-extern void pps_event(int source, struct pps_ktime *ts, int event, void *data);
+extern void pps_event(int source, struct pps_event_time *ts, int event,
+ void *data);
+
+static inline void timespec_to_pps_ktime(struct pps_ktime *kt,
+ struct timespec ts)
+{
+ kt->sec = ts.tv_sec;
+ kt->nsec = ts.tv_nsec;
+}
+
+static inline void pps_get_ts(struct pps_event_time *ts)
+{
+ struct timespec ts_real;
+
+ getnstime_raw_and_real(&ts->ts_raw, &ts_real);
+ timespec_to_pps_ktime(&ts->ts_real, ts_real);
+}
+
+#endif /* LINUX_PPS_KERNEL_H */
+
diff --git a/include/linux/time.h b/include/linux/time.h
index 6e026e4..edf7eb7 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -143,6 +143,8 @@ extern unsigned int alarm_setitimer(unsigned int seconds);
extern int do_getitimer(int which, struct itimerval *value);
extern void getnstimeofday(struct timespec *tv);
extern void getrawmonotonic(struct timespec *ts);
+extern void getnstime_raw_and_real(struct timespec *ts_raw,
+ struct timespec *ts_real);
extern void getboottime(struct timespec *ts);
extern void monotonic_to_bootbased(struct timespec *ts);

diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index 66d090e..647bf35 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -293,6 +293,40 @@ void ktime_get_ts(struct timespec *ts)
EXPORT_SYMBOL_GPL(ktime_get_ts);

/**
+ * getnstime_raw_and_real - Returns both the time of day an raw
+ * monotonic time in a timespec format
+ * @ts_mono_raw: pointer to the timespec to be set to raw
+ * monotonic time
+ * @ts_real: pointer to the timespec to be set to the time
+ * of day
+ */
+void getnstime_raw_and_real(struct timespec *ts_raw, struct timespec *ts_real)
+{
+ unsigned long seq;
+ s64 nsecs_raw, nsecs_real;
+
+ WARN_ON(timekeeping_suspended);
+
+ do {
+ seq = read_seqbegin(&xtime_lock);
+
+ *ts_raw = raw_time;
+ *ts_real = xtime;
+
+ nsecs_raw = timekeeping_get_ns_raw();
+ nsecs_real = timekeeping_get_ns();
+
+ /* If arch requires, add in gettimeoffset() */
+ nsecs_real += arch_gettimeoffset();
+
+ } while (read_seqretry(&xtime_lock, seq));
+
+ timespec_add_ns(ts_raw, nsecs_raw);
+ timespec_add_ns(ts_real, nsecs_real);
+}
+EXPORT_SYMBOL(getnstime_raw_and_real);
+
+/**
* do_gettimeofday - Returns the time of day in a timeval
* @tv: pointer to the timeval to be set
*
--
1.6.5

2010-02-03 21:07:46

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH 1/5] ntp: add hardpps implementation

This commit adds hardpps() implementation based upon the original one
from the NTPv4 reference kernel code. However, it is highly optimized
towards very fast syncronization and maximum stickness to PPS signal.
The typical error is less then a microsecond.
To make it sync faster I had to throw away exponential phase filter so
that the full phase offset is corrected immediately. Then I also had to
throw away median phase filter because it gives a bigger error itself
if used without exponential filter.
Maybe we will find an appropriate filtering scheme in the future but
it's not necessary if the signal quality is ok.

Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/pps/Kconfig | 1 +
include/linux/timex.h | 1 +
kernel/time/Kconfig | 7 +
kernel/time/ntp.c | 346 ++++++++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 349 insertions(+), 6 deletions(-)

diff --git a/drivers/pps/Kconfig b/drivers/pps/Kconfig
index cc2eb8e..2bd4f65 100644
--- a/drivers/pps/Kconfig
+++ b/drivers/pps/Kconfig
@@ -7,6 +7,7 @@ menu "PPS support"
config PPS
tristate "PPS support"
depends on EXPERIMENTAL
+ select NTP_PPS
---help---
PPS (Pulse Per Second) is a special pulse provided by some GPS
antennae. Userland can use it to get a high-precision time
diff --git a/include/linux/timex.h b/include/linux/timex.h
index e6967d1..9efa3a8 100644
--- a/include/linux/timex.h
+++ b/include/linux/timex.h
@@ -274,6 +274,7 @@ extern u64 tick_length;
extern void second_overflow(void);
extern void update_ntp_one_tick(void);
extern int do_adjtimex(struct timex *);
+extern void hardpps(const struct timespec *, s64);

/* Don't use! Compatibility define for existing users. */
#define tickadj (500/HZ ? : 1)
diff --git a/kernel/time/Kconfig b/kernel/time/Kconfig
index 95ed429..2da4900 100644
--- a/kernel/time/Kconfig
+++ b/kernel/time/Kconfig
@@ -27,3 +27,10 @@ config GENERIC_CLOCKEVENTS_BUILD
default y
depends on GENERIC_CLOCKEVENTS || GENERIC_CLOCKEVENTS_MIGR

+config NTP_PPS
+ bool "PPS kernel consumer support"
+ depends on PPS
+ help
+ This option adds support for direct in-kernel time
+ syncronization using an external PPS signal.
+
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 4800f93..8cd20ad 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -14,6 +14,7 @@
#include <linux/timex.h>
#include <linux/time.h>
#include <linux/mm.h>
+#include <linux/module.h>

/*
* NTP timekeeping variables:
@@ -74,6 +75,40 @@ long time_adjust;
/* constant (boot-param configurable) NTP tick adjustment (upscaled) */
static s64 ntp_tick_adj;

+#ifdef CONFIG_NTP_PPS
+/*
+ * The following variables are used when a pulse-per-second (PPS) signal
+ * is available and connected via a modem control lead. They establish
+ * the engineering parameters of the clock discipline loop when
+ * controlled by the PPS signal.
+ */
+#define PPS_FAVG 2 /* min freq avg interval (s) (shift) */
+#define PPS_FAVGDEF 8 /* default freq avg interval (s) (shift) */
+#define PPS_FAVGMAX 15 /* max freq avg interval (s) (shift) */
+#define PPS_VALID 120 /* PPS signal watchdog max (s) */
+#define PPS_MAXWANDER 100000 /* max PPS wander (ns/s) */
+#define PPS_POPCORN 2 /* popcorn spike threshold (shift) */
+
+long pps_tf[3]; /* phase median filter */
+s64 pps_freq; /* scaled frequency offset (ns/s) */
+s64 pps_fcount; /* frequency accumulator */
+long pps_jitter; /* nominal jitter (ns) */
+long pps_stabil; /* nominal stability (scaled ns/s) */
+int pps_valid; /* signal watchdog counter */
+int pps_shift; /* nominal interval duration (s) (shift) */
+int pps_shiftmax; /* max interval duration (s) (shift) */
+int pps_intcnt; /* interval counter */
+
+/*
+ * PPS signal quality monitors
+ */
+long pps_calcnt; /* calibration intervals */
+long pps_jitcnt; /* jitter limit exceeded */
+long pps_stbcnt; /* stability limit exceeded */
+long pps_errcnt; /* calibration errors */
+
+#endif /* CONFIG_NTP_PPS */
+
/*
* NTP methods:
*/
@@ -177,6 +212,15 @@ void ntp_clear(void)

tick_length = tick_length_base;
time_offset = 0;
+#ifdef CONFIG_NTP_PPS
+ pps_shift = PPS_FAVG;
+ pps_shiftmax = PPS_FAVGDEF;
+ pps_tf[0] = 0;
+ pps_tf[1] = 0;
+ pps_tf[2] = 0;
+ pps_fcount = 0;
+ pps_freq = 0;
+#endif /* CONFIG_NTP_PPS */
}

/*
@@ -233,8 +277,6 @@ static enum hrtimer_restart ntp_leap_second(struct hrtimer *timer)
*/
void second_overflow(void)
{
- s64 delta;
-
/* Bump the maxerror field */
time_maxerror += MAXFREQ / NSEC_PER_USEC;
if (time_maxerror > NTP_PHASE_LIMIT) {
@@ -248,9 +290,27 @@ void second_overflow(void)
*/
tick_length = tick_length_base;

- delta = shift_right(time_offset, SHIFT_PLL + time_constant);
- time_offset -= delta;
- tick_length += delta;
+#ifdef CONFIG_NTP_PPS
+ if (time_status & STA_PPSTIME && time_status & STA_PPSSIGNAL) {
+ tick_length += time_offset;
+ time_offset = 0;
+ } else
+#endif /* CONFIG_NTP_PPS */
+ {
+ s64 delta;
+ delta =
+ shift_right(time_offset, SHIFT_PLL + time_constant);
+ time_offset -= delta;
+ tick_length += delta;
+ }
+
+#ifdef CONFIG_NTP_PPS
+ if (pps_valid > 0)
+ pps_valid--;
+ else
+ time_status &= ~(STA_PPSSIGNAL | STA_PPSJITTER |
+ STA_PPSWANDER | STA_PPSERROR);
+#endif /* CONFIG_NTP_PPS */

if (!time_adjust)
return;
@@ -361,6 +421,12 @@ static inline void process_adj_status(struct timex *txc, struct timespec *ts)
if ((time_status & STA_PLL) && !(txc->status & STA_PLL)) {
time_state = TIME_OK;
time_status = STA_UNSYNC;
+#ifdef CONFIG_NTP_PPS
+ /* the PPS calibration interval may end
+ surprisingly early */
+ pps_shift = PPS_FAVG;
+ pps_intcnt = 0;
+#endif /* CONFIG_NTP_PPS */
}

/*
@@ -410,6 +476,9 @@ static inline void process_adjtimex_modes(struct timex *txc, struct timespec *ts
time_freq = txc->freq * PPM_SCALE;
time_freq = min(time_freq, MAXFREQ_SCALED);
time_freq = max(time_freq, -MAXFREQ_SCALED);
+#ifdef CONFIG_NTP_PPS
+ pps_freq = time_freq;
+#endif /* CONFIG_NTP_PPS */
}

if (txc->modes & ADJ_MAXERROR)
@@ -500,7 +569,23 @@ int do_adjtimex(struct timex *txc)
}

result = time_state; /* mostly `TIME_OK' */
- if (time_status & (STA_UNSYNC|STA_CLOCKERR))
+ if ((time_status & (STA_UNSYNC|STA_CLOCKERR))
+#ifdef CONFIG_NTP_PPS
+ /* PPS signal lost when either PPS time or PPS frequency
+ * synchronization requested
+ */
+ || ((time_status & (STA_PPSFREQ|STA_PPSTIME))
+ && !(time_status & STA_PPSSIGNAL))
+ /* PPS jitter exceeded when PPS time synchronization requested */
+ || ((time_status & (STA_PPSTIME|STA_PPSJITTER))
+ == (STA_PPSTIME|STA_PPSJITTER))
+ /* PPS wander exceeded or calibration error when PPS frequency
+ * synchronization requested
+ */
+ || ((time_status & STA_PPSFREQ)
+ && (time_status & (STA_PPSWANDER|STA_PPSERROR)))
+#endif /* CONFIG_NTP_PPS */
+ )
result = TIME_ERROR;

txc->freq = shift_right((time_freq >> PPM_SCALE_INV_SHIFT) *
@@ -514,6 +599,19 @@ int do_adjtimex(struct timex *txc)
txc->tick = tick_usec;
txc->tai = time_tai;

+#ifdef CONFIG_NTP_PPS
+ txc->ppsfreq = shift_right((pps_freq >> PPM_SCALE_INV_SHIFT) *
+ PPM_SCALE_INV, NTP_SCALE_SHIFT);
+ txc->jitter = pps_jitter;
+ if (!(time_status & STA_NANO))
+ txc->jitter /= NSEC_PER_USEC;
+ txc->shift = pps_shift;
+ txc->stabil = pps_stabil;
+ txc->jitcnt = pps_jitcnt;
+ txc->calcnt = pps_calcnt;
+ txc->errcnt = pps_errcnt;
+ txc->stbcnt = pps_stbcnt;
+#else /* !CONFIG_NTP_PPS */
/* PPS is not implemented, so these are zero */
txc->ppsfreq = 0;
txc->jitter = 0;
@@ -523,6 +621,7 @@ int do_adjtimex(struct timex *txc)
txc->calcnt = 0;
txc->errcnt = 0;
txc->stbcnt = 0;
+#endif /* CONFIG_NTP_PPS */

write_sequnlock_irq(&xtime_lock);

@@ -536,6 +635,241 @@ int do_adjtimex(struct timex *txc)
return result;
}

+#ifdef CONFIG_NTP_PPS
+
+/* normalize the timestamp so that nsec is in the
+ ( -NSEC_PER_SEC / 2, NSEC_PER_SEC / 2 ] interval */
+static inline struct timespec pps_normalize_ts(struct timespec ts)
+{
+ if (ts.tv_nsec > (NSEC_PER_SEC >> 1)) {
+ ts.tv_nsec -= NSEC_PER_SEC;
+ ts.tv_sec++;
+ }
+
+ return ts;
+}
+
+/* use exponential average as an estimate for the phase correction */
+static inline long pps_phase_filter_get(long *jitter)
+{
+ *jitter = pps_tf[0] - pps_tf[1];
+ if (*jitter < 0)
+ *jitter = -*jitter;
+
+ return (3 * pps_tf[0] + pps_tf[1]) >> 2;
+}
+
+/* add the sample to the phase filter */
+static inline void pps_phase_filter_add(long err)
+{
+ pps_tf[2] = pps_tf[1];
+ pps_tf[1] = pps_tf[0];
+ pps_tf[0] = err;
+}
+
+/* decrease frequency calibration interval length */
+static inline void pps_dec_freq_interval(void)
+{
+ if (--pps_intcnt <= -4) {
+ pps_intcnt = -4;
+ if (pps_shift > PPS_FAVG) {
+ pps_shift--;
+ pps_intcnt = 0;
+ }
+ }
+}
+
+/* update frequency based on PPS signal
+ * returns the difference between old and new frequency values
+ */
+static long hardpps_update_freq(s64 nsec)
+{
+ struct timespec freq_norm;
+ long delta, delta_mod;
+ s64 ftemp;
+
+ pps_fcount += nsec;
+ freq_norm = pps_normalize_ts(ns_to_timespec(pps_fcount));
+
+ /* check if the current frequency interval is not finished */
+ if (freq_norm.tv_sec < (1 << pps_shift))
+ return 0;
+
+ /*
+ * At the end of the calibration interval the difference between
+ * the first and last counter values becomes the scaled
+ * frequency. It will later be divided by the length of the
+ * interval to determine the frequency update. If the frequency
+ * exceeds a sanity threshold, or if the actual calibration
+ * interval is not equal to the expected length, the data are
+ * discarded. We can tolerate a modest loss of data here without
+ * degrading frequency accuracy.
+ */
+ pps_calcnt++;
+ pps_fcount = 0;
+ /* check if the frequency interval was too long */
+ if (freq_norm.tv_sec > (2 << pps_shift)) {
+ time_status |= STA_PPSERROR;
+ pps_errcnt++;
+ pps_dec_freq_interval();
+ pr_err("hardpps: PPSERROR: interval too long - %ld s\n",
+ freq_norm.tv_sec);
+ return 0;
+ }
+
+ /*
+ * Here the raw frequency offset and wander (stability) is
+ * calculated. If the wander is less than the wander threshold
+ * for four consecutive averaging intervals, the interval is
+ * doubled; if it is greater than the threshold for four
+ * consecutive intervals, the interval is halved. The scaled
+ * frequency offset is converted to frequency offset. The
+ * stability metric is calculated as the average of recent
+ * frequency changes, but is used only for performance
+ * monitoring.
+ */
+ ftemp = div_s64(((s64)(-freq_norm.tv_nsec)) << NTP_SCALE_SHIFT,
+ freq_norm.tv_sec);
+ delta = shift_right(ftemp - pps_freq, NTP_SCALE_SHIFT);
+ pps_freq = ftemp;
+ if (delta > PPS_MAXWANDER || delta < -PPS_MAXWANDER) {
+ pr_warning("hardpps: PPSWANDER: change=%ld\n", delta);
+ time_status |= STA_PPSWANDER;
+ pps_stbcnt++;
+ pps_dec_freq_interval();
+ } else { /* good sample */
+ if (++pps_intcnt >= 4) {
+ pps_intcnt = 4;
+ if (pps_shift < pps_shiftmax) {
+ pps_shift++;
+ pps_intcnt = 0;
+ }
+ }
+ }
+
+ delta_mod = delta;
+ if (delta_mod < 0)
+ delta_mod = -delta_mod;
+ pps_stabil += (div_s64(((s64)delta_mod) <<
+ (NTP_SCALE_SHIFT - SHIFT_USEC),
+ NSEC_PER_USEC) - pps_stabil) >> PPS_FAVG;
+
+ /*
+ * If enabled, the system clock frequency is updated.
+ */
+ if ((time_status & STA_PPSFREQ) != 0 &&
+ (time_status & STA_FREQHOLD) == 0) {
+ time_freq = pps_freq;
+ ntp_update_frequency();
+ }
+
+ return delta;
+}
+
+/* correct phase error against PPS signal */
+static void hardpps_update_phase(long error)
+{
+ long correction = -error;
+ long jitter;
+
+ /* add the sample to the median filter */
+ pps_phase_filter_add(correction);
+ /* TODO: test various filters then use the return value in the
+ * next line */
+ pps_phase_filter_get(&jitter);
+
+ /*
+ * Nominal jitter is due to PPS signal noise and interrupt
+ * latency. If it exceeds the popcorn threshold, the sample is
+ * discarded. otherwise, if so enabled, the time offset is
+ * updated. We can tolerate a modest loss of data here without
+ * much degrading time accuracy.
+ */
+ if (jitter > (pps_jitter << PPS_POPCORN)) {
+ pr_warning("hardpps: PPSJITTER: jitter=%ld, limit=%ld\n",
+ jitter, (pps_jitter << PPS_POPCORN));
+ time_status |= STA_PPSJITTER;
+ pps_jitcnt++;
+ } else if (time_status & STA_PPSTIME) {
+ /* correct the time using the phase offset */
+ time_offset = div_s64(((s64)correction) << NTP_SCALE_SHIFT,
+ NTP_INTERVAL_FREQ);
+ /* cancel running adjtime() */
+ time_adjust = 0;
+ }
+ /* update jitter */
+ pps_jitter += (jitter - pps_jitter) >> PPS_FAVG;
+}
+
+/*
+ * hardpps() - discipline CPU clock oscillator to external PPS signal
+ *
+ * This routine is called at each PPS interrupt in order to discipline
+ * the CPU clock oscillator to the PPS signal. There are two independent
+ * first-order feedback loops, one for the phase, the other for the
+ * frequency. The phase loop measures and grooms the PPS phase offset
+ * and leaves it in a handy spot for the seconds overflow routine. The
+ * frequency loop averages successive PPS phase differences and
+ * calculates the PPS frequency offset, which is also processed by the
+ * seconds overflow routine. The code requires the caller to capture the
+ * time and architecture-dependent hardware counter values in
+ * nanoseconds at the on-time PPS signal transition.
+ *
+ * Note that, on some Unix systems this routine runs at an interrupt
+ * priority level higher than the timer interrupt routine second_overflow().
+ * Therefore, the variables used are distinct from the second_overflow()
+ * variables, except for the actual time and frequency variables, which
+ * are determined by this routine and updated atomically.
+ */
+void hardpps(const struct timespec *p_ts, s64 nsec)
+{
+ struct timespec ts_norm, freq_norm;
+
+ ts_norm = pps_normalize_ts(*p_ts);
+ freq_norm = pps_normalize_ts(ns_to_timespec(nsec));
+
+ /*
+ * The signal is first processed by a range gate and frequency
+ * discriminator. The range gate rejects noise spikes outside
+ * the range +-500 us. The frequency discriminator rejects input
+ * signals with apparent frequency outside the range 1 +-500
+ * PPM. If two hits occur in the same second, we ignore the
+ * later hit; if not and a hit occurs outside the range gate,
+ * keep the later hit for later comparison, but do not process
+ * it.
+ */
+
+ write_seqlock_irq(&xtime_lock);
+
+ /* clear the error bits, they will be set again if needed */
+ time_status &= ~(STA_PPSJITTER | STA_PPSWANDER | STA_PPSERROR);
+
+ /* indicate signal presence */
+ time_status |= STA_PPSSIGNAL;
+ pps_valid = PPS_VALID;
+
+ /* check the signal */
+ if ((nsec < NSEC_PER_SEC - MAXFREQ) ||
+ (freq_norm.tv_nsec > MAXFREQ * freq_norm.tv_sec) ||
+ (freq_norm.tv_nsec < -MAXFREQ * freq_norm.tv_sec)) {
+ time_status |= STA_PPSJITTER;
+ pps_fcount = 0;
+ write_sequnlock_irq(&xtime_lock);
+ pr_err("hardpps: PPSJITTER: bad pulse\n");
+ return;
+ }
+
+ /* signal is ok */
+
+ hardpps_update_freq(nsec);
+ hardpps_update_phase(ts_norm.tv_nsec);
+
+ write_sequnlock_irq(&xtime_lock);
+}
+EXPORT_SYMBOL(hardpps);
+
+#endif /* CONFIG_NTP_PPS */
+
static int __init ntp_tick_adj_setup(char *str)
{
ntp_tick_adj = simple_strtol(str, NULL, 0);
--
1.6.5

2010-02-03 21:07:41

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH 3/5] pps: add kernel consumer support

Add an optional feature of PPSAPI, kernel consumer support, which uses
the added hardpps() function.

Signed-off-by: Alexander Gordeev <[email protected]>
---
Documentation/ioctl/ioctl-number.txt | 2 +-
drivers/pps/kapi.c | 28 ++++++++++++++
drivers/pps/pps.c | 67 +++++++++++++++++++++++++++++++++-
include/linux/pps.h | 7 ++++
include/linux/pps_kernel.h | 6 +++
5 files changed, 108 insertions(+), 2 deletions(-)

diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
index 9473749..d68718b 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -152,7 +152,7 @@ Code Seq# Include File Comments
'p' 40-7F linux/nvram.h
'p' 80-9F user-space parport
<mailto:[email protected]>
-'p' a1-a4 linux/pps.h LinuxPPS
+'p' a1-a5 linux/pps.h LinuxPPS
<mailto:[email protected]>
'q' 00-1F linux/serio.h
'q' 80-FF Internet PhoneJACK, Internet LineJACK
diff --git a/drivers/pps/kapi.c b/drivers/pps/kapi.c
index 8a77102..59ac4f5 100644
--- a/drivers/pps/kapi.c
+++ b/drivers/pps/kapi.c
@@ -25,6 +25,7 @@
#include <linux/init.h>
#include <linux/sched.h>
#include <linux/time.h>
+#include <linux/timex.h>
#include <linux/spinlock.h>
#include <linux/idr.h>
#include <linux/fs.h>
@@ -37,6 +38,12 @@
DEFINE_SPINLOCK(pps_idr_lock);
DEFINE_IDR(pps_idr);

+/* state variables to bind kernel consumer */
+/* PPS API (RFC 2783): current source and mode for ``kernel consumer'' */
+DEFINE_SPINLOCK(pps_kc_hardpps_lock);
+void *pps_kc_hardpps_dev; /* some unique pointer to device */
+int pps_kc_hardpps_mode; /* mode bits for kernel consumer */
+
/*
* Local functions
*/
@@ -272,6 +279,7 @@ void pps_event(int source, struct pps_event_time *ts, int event, void *data)
struct pps_device *pps;
unsigned long flags;
int captured = 0;
+ s64 nsec = 0;

if ((event & (PPS_CAPTUREASSERT | PPS_CAPTURECLEAR)) == 0) {
printk(KERN_ERR "pps: unknown event (%x) for source %d\n",
@@ -304,6 +312,11 @@ void pps_event(int source, struct pps_event_time *ts, int event, void *data)

/* Save the time stamp */
pps->assert_tu = ts->ts_real;
+ if (likely(pps->assert_tu_raw.tv_sec != 0)) {
+ struct timespec d =
+ timespec_sub(ts->ts_raw, pps->assert_tu_raw);
+ nsec = timespec_to_ns(&d);
+ }
pps->assert_tu_raw = ts->ts_raw;
pps->assert_sequence++;
pr_debug("capture assert seq #%u for source %d\n",
@@ -320,6 +333,11 @@ void pps_event(int source, struct pps_event_time *ts, int event, void *data)

/* Save the time stamp */
pps->clear_tu = ts->ts_real;
+ if (likely(pps->clear_tu_raw.tv_sec != 0)) {
+ struct timespec d =
+ timespec_sub(ts->ts_raw, pps->clear_tu_raw);
+ nsec = timespec_to_ns(&d);
+ }
pps->clear_tu_raw = ts->ts_raw;
pps->clear_sequence++;
pr_debug("capture clear seq #%u for source %d\n",
@@ -327,6 +345,16 @@ void pps_event(int source, struct pps_event_time *ts, int event, void *data)

captured = ~0;
}
+ spin_lock(&pps_kc_hardpps_lock);
+ if (pps == pps_kc_hardpps_dev && event & pps_kc_hardpps_mode) {
+ struct timespec p_ts = {
+ .tv_sec = ts->ts_real.sec,
+ .tv_nsec = ts->ts_real.nsec
+ };
+
+ hardpps(&p_ts, nsec);
+ }
+ spin_unlock(&pps_kc_hardpps_lock);

/* Wake up iif captured somthing */
if (captured) {
diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index ca5183b..22fdcfe 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -62,6 +62,8 @@ static long pps_cdev_ioctl(struct file *file,
struct pps_device *pps = file->private_data;
struct pps_kparams params;
struct pps_fdata fdata;
+ struct pps_bind_args bind_args;
+ unsigned long irq_flags;
unsigned long ticks;
void __user *uarg = (void __user *) arg;
int __user *iuarg = (int __user *) arg;
@@ -186,9 +188,72 @@ static long pps_cdev_ioctl(struct file *file,

break;

+ case PPS_KC_BIND:
+ pr_info("PPS_KC_BIND: source %d\n", pps->id);
+
+ /* Check the capabilities */
+ if (!capable(CAP_SYS_TIME))
+ return -EPERM;
+
+ if (copy_from_user(&bind_args, uarg,
+ sizeof(struct pps_bind_args)))
+ return -EFAULT;
+
+ /* Check for supported capabilities */
+ if ((bind_args.edge & ~pps->info.mode) != 0) {
+ pr_err("PPS_KC_BIND: "
+ "unsupported capabilities (%x)\n",
+ bind_args.edge);
+ return -EINVAL;
+ }
+
+ /* Validate parameters roughly */
+ if (bind_args.tsformat != PPS_TSFMT_TSPEC ||
+ (bind_args.edge & ~PPS_CAPTUREBOTH) != 0 ||
+ bind_args.consumer != PPS_KC_HARDPPS) {
+ pr_err("PPS_KC_BIND: "
+ "invalid kcbind parameters (%x)\n",
+ bind_args.edge);
+ return -EINVAL;
+ }
+
+ /* Check if another consumer is already bound */
+ spin_lock_irqsave(&pps_kc_hardpps_lock, irq_flags);
+
+ if (bind_args.edge == 0)
+ if (pps_kc_hardpps_dev == pps) {
+ pps_kc_hardpps_mode = 0;
+ pps_kc_hardpps_dev = NULL;
+ spin_unlock_irqrestore(&pps_kc_hardpps_lock,
+ irq_flags);
+ pr_info("unbound kernel consumer\n");
+ } else {
+ spin_unlock_irqrestore(&pps_kc_hardpps_lock,
+ irq_flags);
+ pr_err("selected kernel consumer"
+ " is not bound\n");
+ return -EINVAL;
+ }
+ else
+ if (pps_kc_hardpps_dev == NULL ||
+ pps_kc_hardpps_dev == pps) {
+ pps_kc_hardpps_mode = bind_args.edge;
+ pps_kc_hardpps_dev = pps;
+ spin_unlock_irqrestore(&pps_kc_hardpps_lock,
+ irq_flags);
+ pr_info("bound kernel consumer: dev=%p, "
+ "edge=0x%x\n", pps, bind_args.edge);
+ } else {
+ spin_unlock_irqrestore(&pps_kc_hardpps_lock,
+ irq_flags);
+ pr_err("another kernel consumer"
+ " is already bound\n");
+ return -EINVAL;
+ }
+ break;
+
default:
return -ENOTTY;
- break;
}

return 0;
diff --git a/include/linux/pps.h b/include/linux/pps.h
index 0194ab0..a9bb1d9 100644
--- a/include/linux/pps.h
+++ b/include/linux/pps.h
@@ -114,11 +114,18 @@ struct pps_fdata {
struct pps_ktime timeout;
};

+struct pps_bind_args {
+ int tsformat; /* format of time stamps */
+ int edge; /* selected event type */
+ int consumer; /* selected kernel consumer */
+};
+
#include <linux/ioctl.h>

#define PPS_GETPARAMS _IOR('p', 0xa1, struct pps_kparams *)
#define PPS_SETPARAMS _IOW('p', 0xa2, struct pps_kparams *)
#define PPS_GETCAP _IOR('p', 0xa3, int *)
#define PPS_FETCH _IOWR('p', 0xa4, struct pps_fdata *)
+#define PPS_KC_BIND _IOW('p', 0xa5, struct pps_bind_args *)

#endif /* _PPS_H_ */
diff --git a/include/linux/pps_kernel.h b/include/linux/pps_kernel.h
index 0d36a6f..1251138 100644
--- a/include/linux/pps_kernel.h
+++ b/include/linux/pps_kernel.h
@@ -85,6 +85,12 @@ extern struct idr pps_idr;

extern struct device_attribute pps_attrs[];

+/* state variables to bind kernel consumer */
+/* PPS API (RFC 2783): current source and mode for ``kernel consumer'' */
+extern spinlock_t pps_kc_hardpps_lock;
+extern void *pps_kc_hardpps_dev; /* some unique pointer to device */
+extern int pps_kc_hardpps_mode; /* mode bits for kernel consumer */
+
/*
* Exported functions
*/
--
1.6.5

2010-02-03 21:07:20

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH 4/5] pps: add parallel port PPS signal generator

Add PPS signal generator which utilizes STROBE pin of a parallel port to
send PPS signals. It uses parport abstraction layer and hrtimers to
precisely control the signal.

Signed-off-by: Alexander Gordeev <[email protected]>
---
drivers/pps/Kconfig | 2 +
drivers/pps/Makefile | 1 +
drivers/pps/generators/Kconfig | 17 ++
drivers/pps/generators/Makefile | 9 +
drivers/pps/generators/pps_gen_parport.c | 263 ++++++++++++++++++++++++++++++
5 files changed, 292 insertions(+), 0 deletions(-)
create mode 100644 drivers/pps/generators/Kconfig
create mode 100644 drivers/pps/generators/Makefile
create mode 100644 drivers/pps/generators/pps_gen_parport.c

diff --git a/drivers/pps/Kconfig b/drivers/pps/Kconfig
index 2bd4f65..f3b14cd 100644
--- a/drivers/pps/Kconfig
+++ b/drivers/pps/Kconfig
@@ -31,4 +31,6 @@ config PPS_DEBUG
messages to the system log. Select this if you are having a
problem with PPS support and want to see more of what is going on.

+source drivers/pps/generators/Kconfig
+
endmenu
diff --git a/drivers/pps/Makefile b/drivers/pps/Makefile
index 19ea582..b299814 100644
--- a/drivers/pps/Makefile
+++ b/drivers/pps/Makefile
@@ -4,5 +4,6 @@

pps_core-y := pps.o kapi.o sysfs.o
obj-$(CONFIG_PPS) := pps_core.o
+obj-y += generators/

ccflags-$(CONFIG_PPS_DEBUG) := -DDEBUG
diff --git a/drivers/pps/generators/Kconfig b/drivers/pps/generators/Kconfig
new file mode 100644
index 0000000..9cc1118
--- /dev/null
+++ b/drivers/pps/generators/Kconfig
@@ -0,0 +1,17 @@
+#
+# PPS clients configuration
+#
+
+if PPS
+
+comment "PPS generators support"
+
+config PPS_GENERATOR_PARPORT
+ tristate "Parallel port PPS signal generator"
+ depends on PARPORT != n && GENERIC_TIME
+ help
+ If you say yes here you get support for a PPS signal generator which
+ utilizes STROBE pin of a parallel port to send PPS signals. It uses
+ parport abstraction layer and hrtimers to precisely control the signal.
+
+endif
diff --git a/drivers/pps/generators/Makefile b/drivers/pps/generators/Makefile
new file mode 100644
index 0000000..303304a
--- /dev/null
+++ b/drivers/pps/generators/Makefile
@@ -0,0 +1,9 @@
+#
+# Makefile for PPS generators.
+#
+
+obj-$(CONFIG_PPS_GENERATOR_PARPORT) += pps_gen_parport.o
+
+ifeq ($(CONFIG_PPS_DEBUG),y)
+EXTRA_CFLAGS += -DDEBUG
+endif
diff --git a/drivers/pps/generators/pps_gen_parport.c b/drivers/pps/generators/pps_gen_parport.c
new file mode 100644
index 0000000..5df7277
--- /dev/null
+++ b/drivers/pps/generators/pps_gen_parport.c
@@ -0,0 +1,263 @@
+/*
+ * pps_gen_parport.c -- kernel parallel port PPS signal generator
+ *
+ *
+ * Copyright (C) 2009 Alexander Gordeev <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+
+/*
+ * TODO:
+ * 1. module parameters
+ * 2. check that local interrupts are blocked in hrtimer handler
+ * 3. fix issues when realtime clock is adjusted in a leap
+ * 4. test under heavy load
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/time.h>
+#include <linux/hrtimer.h>
+#include <linux/parport.h>
+
+#define DRVNAME "pps_gen_parport"
+#define DRVDESC "parallel port PPS signal generator"
+
+#define SIGNAL 0
+#define NO_SIGNAL PARPORT_CONTROL_STROBE
+
+#define SEND_DELAY 30000 /* delay between setting and
+ dropping the signal (ns) */
+#define SAFETY_INTERVAL 5000 /* set the hrtimer earlier for safety (ns) */
+
+/* internal per port structure */
+struct pps_generator_pp {
+ struct pardevice *pardev; /* parport device */
+ struct hrtimer timer;
+ long port_write_time; /* calibrated port write time (ns) */
+};
+
+static struct pps_generator_pp device = {
+ .pardev = NULL,
+};
+
+static int attached;
+
+/* calibrated time between a hrtimer event and the reaction */
+static long hrtimer_error = SAFETY_INTERVAL;
+
+/* the kernel hrtimer event */
+static enum hrtimer_restart hrtimer_event(struct hrtimer *timer)
+{
+ struct timespec expire_time, ts1, ts2, ts3, dts;
+ struct pps_generator_pp *dev;
+ struct parport *port;
+ long lim, delta;
+ unsigned long flags;
+
+ /* NB: approx time with blocked interrupts =
+ SEND_DELAY + 2 * SAFETY_INTERVAL */
+ local_irq_save(flags);
+
+ /* first of all we get the time stamp... */
+ getnstimeofday(&ts1);
+ expire_time = ktime_to_timespec(timer->_expires);
+ dev = container_of(timer, struct pps_generator_pp, timer);
+ lim = NSEC_PER_SEC - SEND_DELAY - dev->port_write_time;
+
+ /* check if we are late */
+ if (expire_time.tv_sec != ts1.tv_sec || ts1.tv_nsec > lim) {
+ local_irq_restore(flags);
+ pr_err(DRVNAME ": we are late this time %ld.%09ld\n",
+ ts1.tv_sec, ts1.tv_nsec);
+ goto done;
+ }
+
+ /* busy loop until the time is right for an assert edge */
+ do {
+ getnstimeofday(&ts2);
+ } while (expire_time.tv_sec == ts2.tv_sec && ts2.tv_nsec < lim);
+
+ /* set the signal */
+ port = dev->pardev->port;
+ port->ops->write_control(port, SIGNAL);
+
+ /* busy loop until the time is right for a clear edge */
+ lim = NSEC_PER_SEC - dev->port_write_time;
+ do {
+ getnstimeofday(&ts2);
+ } while (expire_time.tv_sec == ts2.tv_sec && ts2.tv_nsec < lim);
+
+ /* unset the signal */
+ port->ops->write_control(port, NO_SIGNAL);
+
+ getnstimeofday(&ts3);
+
+ local_irq_restore(flags);
+
+ /* update calibrated port write time */
+ dts = timespec_sub(ts3, ts2);
+ dev->port_write_time =
+ (dev->port_write_time + timespec_to_ns(&dts)) >> 1;
+
+done:
+ /* update calibrated hrtimer error */
+ dts = timespec_sub(ts1, expire_time);
+ delta = timespec_to_ns(&dts);
+ /* If the new error value is bigger then the old, use the new
+ * value, if not then slowly move towards the new value. This
+ * way it should be safe in bad conditions and efficient in
+ * good conditions.
+ */
+ if (delta >= hrtimer_error)
+ hrtimer_error = delta;
+ else
+ hrtimer_error = (3 * hrtimer_error + delta) >> 2;
+
+ /* update the hrtimer expire time */
+ hrtimer_set_expires(timer,
+ ktime_set(expire_time.tv_sec + 1,
+ NSEC_PER_SEC - SEND_DELAY -
+ dev->port_write_time - 2 * hrtimer_error));
+
+ return HRTIMER_RESTART;
+}
+
+/* calibrate port write time */
+#define PORT_NTESTS_SHIFT 5
+static void calibrate_port(struct pps_generator_pp *dev)
+{
+ struct parport *port = dev->pardev->port;
+ int i;
+ long acc = 0;
+
+ for (i = 0; i < (1 << PORT_NTESTS_SHIFT); i++) {
+ struct timespec a, b;
+ unsigned long irq_flags;
+
+ local_irq_save(irq_flags);
+ getnstimeofday(&a);
+ port->ops->write_control(port, NO_SIGNAL);
+ getnstimeofday(&b);
+ local_irq_restore(irq_flags);
+
+ b = timespec_sub(b, a);
+ acc += timespec_to_ns(&b);
+ }
+
+ dev->port_write_time = acc >> PORT_NTESTS_SHIFT;
+ pr_info(DRVNAME ": port write takes %ldns\n", dev->port_write_time);
+}
+
+static ktime_t next_intr_time(struct pps_generator_pp *dev)
+{
+ struct timespec ts;
+
+ getnstimeofday(&ts);
+
+ return ktime_set(ts.tv_sec +
+ ((ts.tv_nsec > 990 * NSEC_PER_MSEC) ? 1 : 0),
+ NSEC_PER_SEC - SEND_DELAY -
+ dev->port_write_time - 2 * SAFETY_INTERVAL);
+}
+
+static void parport_attach(struct parport *port)
+{
+ if (attached) {
+ /* we already have a port */
+ return;
+ }
+
+ device.pardev = parport_register_device(port, DRVNAME,
+ NULL, NULL, NULL, 0, &device);
+ if (!device.pardev) {
+ pr_err(DRVNAME ": couldn't register with %s\n", port->name);
+ return;
+ }
+
+ if (parport_claim_or_block(device.pardev) < 0) {
+ pr_err(DRVNAME ": couldn't claim %s\n", port->name);
+ goto err_unregister_dev;
+ }
+
+ pr_info(DRVNAME ": attached to %s\n", port->name);
+ attached = 1;
+
+ calibrate_port(&device);
+
+ hrtimer_init(&device.timer, CLOCK_REALTIME, HRTIMER_MODE_ABS);
+ device.timer.function = hrtimer_event;
+#ifdef CONFIG_PREEMPT_RT
+ /* hrtimer interrupt will run in the interrupt context with this */
+ device.timer.irqsafe = 1;
+#endif
+
+ hrtimer_start(&device.timer, next_intr_time(&device), HRTIMER_MODE_ABS);
+
+ return;
+
+err_unregister_dev:
+ parport_unregister_device(device.pardev);
+}
+
+static void parport_detach(struct parport *port)
+{
+ if (port->cad != device.pardev)
+ /* not our port */
+ return;
+
+ hrtimer_cancel(&device.timer);
+ parport_release(device.pardev);
+ parport_unregister_device(device.pardev);
+}
+
+static struct parport_driver pps_gen_parport_driver = {
+ .name = DRVNAME,
+ .attach = parport_attach,
+ .detach = parport_detach,
+};
+
+/* module staff */
+
+static int __init pps_gen_parport_init(void)
+{
+ int ret;
+
+ pr_info(DRVNAME ": " DRVDESC "\n");
+
+ ret = parport_register_driver(&pps_gen_parport_driver);
+ if (ret) {
+ pr_err(DRVNAME ": unable to register with parport\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static void __exit pps_gen_parport_exit(void)
+{
+ parport_unregister_driver(&pps_gen_parport_driver);
+ pr_info(DRVNAME ": hrtimer avg error is %ldns\n", hrtimer_error);
+}
+
+module_init(pps_gen_parport_init);
+module_exit(pps_gen_parport_exit);
+
+MODULE_AUTHOR("Alexander Gordeev <[email protected]>");
+MODULE_DESCRIPTION(DRVDESC);
+MODULE_LICENSE("GPL");
--
1.6.5

2010-02-03 22:09:00

by john stultz

[permalink] [raw]
Subject: Re: [PATCH 1/5] ntp: add hardpps implementation

On Wed, 2010-02-03 at 23:56 +0300, Alexander Gordeev wrote:
> This commit adds hardpps() implementation based upon the original one
> from the NTPv4 reference kernel code. However, it is highly optimized
> towards very fast syncronization and maximum stickness to PPS signal.
> The typical error is less then a microsecond.
> To make it sync faster I had to throw away exponential phase filter so
> that the full phase offset is corrected immediately. Then I also had to
> throw away median phase filter because it gives a bigger error itself
> if used without exponential filter.
> Maybe we will find an appropriate filtering scheme in the future but
> it's not necessary if the signal quality is ok.
>
> Signed-off-by: Alexander Gordeev <[email protected]>

Very cool! Bunch of style comments below.

One other thing: From the comments in the code, it looks like this may
have come straight from the reference implementation. You might want to
provide some extra documentation or comment providing proper credit, and
clarifying that the code is in fact GPLv2 compatible.

Also please review Section 12 of Documentation/SubmittingPatches

thanks
-john




[snip]
> +long pps_tf[3]; /* phase median filter */
> +s64 pps_freq; /* scaled frequency offset (ns/s) */
> +s64 pps_fcount; /* frequency accumulator */
> +long pps_jitter; /* nominal jitter (ns) */
> +long pps_stabil; /* nominal stability (scaled ns/s) */
> +int pps_valid; /* signal watchdog counter */
> +int pps_shift; /* nominal interval duration (s) (shift) */
> +int pps_shiftmax; /* max interval duration (s) (shift) */
> +int pps_intcnt; /* interval counter */
> +
> +/*
> + * PPS signal quality monitors
> + */
> +long pps_calcnt; /* calibration intervals */
> +long pps_jitcnt; /* jitter limit exceeded */
> +long pps_stbcnt; /* stability limit exceeded */
> +long pps_errcnt; /* calibration errors */
> +

Shouldn't all of the above values be made static?

[snip]

> /*
> @@ -233,8 +277,6 @@ static enum hrtimer_restart ntp_leap_second(struct hrtimer *timer)
> */
> void second_overflow(void)
> {
> - s64 delta;
> -
> /* Bump the maxerror field */
> time_maxerror += MAXFREQ / NSEC_PER_USEC;
> if (time_maxerror > NTP_PHASE_LIMIT) {
> @@ -248,9 +290,27 @@ void second_overflow(void)
> */
> tick_length = tick_length_base;
>
> - delta = shift_right(time_offset, SHIFT_PLL + time_constant);
> - time_offset -= delta;
> - tick_length += delta;
> +#ifdef CONFIG_NTP_PPS
> + if (time_status & STA_PPSTIME && time_status & STA_PPSSIGNAL) {
> + tick_length += time_offset;
> + time_offset = 0;
> + } else
> +#endif /* CONFIG_NTP_PPS */
> + {
> + s64 delta;
> + delta =
> + shift_right(time_offset, SHIFT_PLL + time_constant);
> + time_offset -= delta;
> + tick_length += delta;
> + }


Ugh. Surely there's a better way to do this then using the ifdefs in
code?

Maybe something like:

#ifdef CONFIG_NTP_PPS
/* Comment about how pps consumes the whole offset */
static inline s64 ntp_offset_chunk(s64 offset)
{
if (time_status & STA_PPSTIME && time_status & STA_PPSSIGNAL)
return offset
else
return shift_right(offset, SHIFT_PLL + time_constant);
}
#else
static inline s64 ntp_offset_chunk(s64 offset)
{
return shift_right(offset, SHIFT_PLL + time_constant);
}
#endif

Then in second_overflow():

delta = ntp_offset_chunk(time_offset);
time_offset -= delta;
tick_length += delta;

Feel free to use a better name then ntp_offset_chunk(), but this keeps
the logic from being obfuscated by all the #ifdefs


> +#ifdef CONFIG_NTP_PPS
> + if (pps_valid > 0)
> + pps_valid--;
> + else
> + time_status &= ~(STA_PPSSIGNAL | STA_PPSJITTER |
> + STA_PPSWANDER | STA_PPSERROR);
> +#endif /* CONFIG_NTP_PPS */

Similarly, can some sort of pps_dec_valid() inline function be used
here?

[snip]

> +#ifdef CONFIG_NTP_PPS
> +
> +/* normalize the timestamp so that nsec is in the
> + ( -NSEC_PER_SEC / 2, NSEC_PER_SEC / 2 ] interval */
> +static inline struct timespec pps_normalize_ts(struct timespec ts)
> +{
> + if (ts.tv_nsec > (NSEC_PER_SEC >> 1)) {
> + ts.tv_nsec -= NSEC_PER_SEC;
> + ts.tv_sec++;
> + }
> +
> + return ts;
> +}

Hrmm.. So by converting a timespec into a second + [-NSEC_PER_SEC/2,
NSEC_PER_SEC/2] value, you aren't really using a timespec anymore,
right? I mean, the timepsec_valid() would fail in many cases, for
instance.

I realize its pretty close, and you *can* use a timespec this way, but
to me it seems reasonable to introduce a new structure (pps_normtime?)
so that its clear you shouldn't exchange timespec values and
pps_normtime values directly?

This is sort of a nit, and maybe others disagree, so not the most
critical issue. But it seems like you're abusing the structure a bit.

[snip]
> +/* decrease frequency calibration interval length */
> +static inline void pps_dec_freq_interval(void)
> +{
> + if (--pps_intcnt <= -4) {
> + pps_intcnt = -4;

Is -4 a magic value?

[snip]
> + } else { /* good sample */
> + if (++pps_intcnt >= 4) {
> + pps_intcnt = 4;

Again, the magic 4?

[snip]
> + pps_stabil += (div_s64(((s64)delta_mod) <<
> + (NTP_SCALE_SHIFT - SHIFT_USEC),
> + NSEC_PER_USEC) - pps_stabil) >> PPS_FAVG;

Hmm. Two 64bit divides in this path? This code would run each second,
right? It would probably be good to see if this could be optimized a
little bit more, but I guess its similar to ntp_update_frequency() which
is called on adjtimex() calls (every 16 seconds at most with networked
ntp).

> +void hardpps(const struct timespec *p_ts, s64 nsec)
> +{
> + struct timespec ts_norm, freq_norm;
> +
> + ts_norm = pps_normalize_ts(*p_ts);
> + freq_norm = pps_normalize_ts(ns_to_timespec(nsec));
> +
[snip]
> +
> + write_seqlock_irq(&xtime_lock);

This is called possibly in irq context, right? So you probably want to
use write_seqlock_irqsave(), no?


thanks
-john

2010-02-03 22:26:43

by john stultz

[permalink] [raw]
Subject: Re: [PATCH 2/5] pps: capture MONOTONIC_RAW timestamps as well

On Wed, 2010-02-03 at 23:56 +0300, Alexander Gordeev wrote:
> MONOTONIC_RAW clock timestamps are ideally suited for frequency
> calculation and also fit well into the original NTP hardpps design. Now
> phase and frequency can be adjusted separately: the former based on
> REALTIME clock and the latter based on MONOTONIC_RAW clock.
> A new function getnstime_raw_and_real is added to timekeeping subsystem
> to capture both timestamps at the same time and atomically.

Hrmm. So while I understand the need for it, the
getnstime_raw_and_real() makes me cringe a little. Part of the issue is
that there are multiple CLOCK_IDs and the current interface allows for
accesses to only one at a time. There's a similar hack in the hrtimer
code to get the CLOCK_REALTIME and CLOCK_MONOTONIC values at the same
time. Next I worry that folks will want a getnstime_mono_and_raw() or a
getnstime_real_mono_and_raw(), then a getnstime_real_and_realcoarse(),
etc..

I'm almost thinking the way to handle this would be a better
abstraction, like a get_two_times(CLOCKID, timepsec*, CLOCKID,
timespec*). But that might need some further discussion. Anyone else
have thoughts here?

So yea not opposed to this patch, but maybe try to avoid exporting the
symbol, so modules don't end up using it and we can change it fairly
easily later.

thanks
-john

2010-02-03 23:52:10

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [PATCH 1/5] ntp: add hardpps implementation

Thanks for the review!

В Wed, 03 Feb 2010 14:08:50 -0800
john stultz <[email protected]> пишет:

> On Wed, 2010-02-03 at 23:56 +0300, Alexander Gordeev wrote:
> > This commit adds hardpps() implementation based upon the original
> > one from the NTPv4 reference kernel code. However, it is highly
> > optimized towards very fast syncronization and maximum stickness to
> > PPS signal. The typical error is less then a microsecond.
> > To make it sync faster I had to throw away exponential phase filter
> > so that the full phase offset is corrected immediately. Then I also
> > had to throw away median phase filter because it gives a bigger
> > error itself if used without exponential filter.
> > Maybe we will find an appropriate filtering scheme in the future but
> > it's not necessary if the signal quality is ok.
> >
> > Signed-off-by: Alexander Gordeev <[email protected]>
>
> Very cool! Bunch of style comments below.
>
> One other thing: From the comments in the code, it looks like this may
> have come straight from the reference implementation. You might want
> to provide some extra documentation or comment providing proper
> credit, and clarifying that the code is in fact GPLv2 compatible.
>
> Also please review Section 12 of Documentation/SubmittingPatches

Indeed, this code is a direct descendant of the reference
implementation. However it is mostly rewritten because the original
code was too hard to read. But I thought it is not an issue because the
whole ntp.c seems to have a lot of code from it... Well, I'm not quite
good in license compatibility. :)

Here is the copyright notice from the original code for reference:
/***********************************************************************
* *
* Copyright (c) David L. Mills 1993-2001 *
* *
* Permission to use, copy, modify, and distribute this software and *
* its documentation for any purpose and without fee is hereby *
* granted, provided that the above copyright notice appears in all *
* copies and that both the copyright notice and this permission *
* notice appear in supporting documentation, and that the name *
* University of Delaware not be used in advertising or publicity *
* pertaining to distribution of the software without specific, *
* written prior permission. The University of Delaware makes no *
* representations about the suitability this software for any *
* purpose. It is provided "as is" without express or implied *
* warranty. *
* *
**********************************************************************/

Maybe adding a comment like the one above second_overflow() is enough?


> [snip]
> > +long pps_tf[3]; /* phase median filter */
> > +s64 pps_freq; /* scaled frequency offset
> > (ns/s) */ +s64 pps_fcount; /* frequency
> > accumulator */ +long pps_jitter; /* nominal jitter
> > (ns) */ +long pps_stabil; /* nominal stability
> > (scaled ns/s) */ +int pps_valid; /* signal
> > watchdog counter */ +int pps_shift; /*
> > nominal interval duration (s) (shift) */ +int
> > pps_shiftmax; /* max interval duration (s) (shift)
> > */ +int pps_intcnt; /* interval counter */ +
> > +/*
> > + * PPS signal quality monitors
> > + */
> > +long pps_calcnt; /* calibration intervals */
> > +long pps_jitcnt; /* jitter limit exceeded */
> > +long pps_stbcnt; /* stability limit exceeded */
> > +long pps_errcnt; /* calibration errors */
> > +
>
> Shouldn't all of the above values be made static?

Sure, thanks.

> [snip]
>
> > /*
> > @@ -233,8 +277,6 @@ static enum hrtimer_restart
> > ntp_leap_second(struct hrtimer *timer) */
> > void second_overflow(void)
> > {
> > - s64 delta;
> > -
> > /* Bump the maxerror field */
> > time_maxerror += MAXFREQ / NSEC_PER_USEC;
> > if (time_maxerror > NTP_PHASE_LIMIT) {
> > @@ -248,9 +290,27 @@ void second_overflow(void)
> > */
> > tick_length = tick_length_base;
> >
> > - delta = shift_right(time_offset, SHIFT_PLL
> > + time_constant);
> > - time_offset -= delta;
> > - tick_length += delta;
> > +#ifdef CONFIG_NTP_PPS
> > + if (time_status & STA_PPSTIME && time_status &
> > STA_PPSSIGNAL) {
> > + tick_length += time_offset;
> > + time_offset = 0;
> > + } else
> > +#endif /* CONFIG_NTP_PPS */
> > + {
> > + s64 delta;
> > + delta =
> > + shift_right(time_offset, SHIFT_PLL +
> > time_constant);
> > + time_offset -= delta;
> > + tick_length += delta;
> > + }
>
>
> Ugh. Surely there's a better way to do this then using the ifdefs in
> code?
>
> Maybe something like:
>
> #ifdef CONFIG_NTP_PPS
> /* Comment about how pps consumes the whole offset */
> static inline s64 ntp_offset_chunk(s64 offset)
> {
> if (time_status & STA_PPSTIME && time_status & STA_PPSSIGNAL)
> return offset
> else
> return shift_right(offset, SHIFT_PLL + time_constant);
> }
> #else
> static inline s64 ntp_offset_chunk(s64 offset)
> {
> return shift_right(offset, SHIFT_PLL + time_constant);
> }
> #endif
>
> Then in second_overflow():
>
> delta = ntp_offset_chunk(time_offset);
> time_offset -= delta;
> tick_length += delta;
>
> Feel free to use a better name then ntp_offset_chunk(), but this keeps
> the logic from being obfuscated by all the #ifdefs
>
> > +#ifdef CONFIG_NTP_PPS
> > + if (pps_valid > 0)
> > + pps_valid--;
> > + else
> > + time_status &= ~(STA_PPSSIGNAL | STA_PPSJITTER |
> > + STA_PPSWANDER | STA_PPSERROR);
> > +#endif /* CONFIG_NTP_PPS */
>
> Similarly, can some sort of pps_dec_valid() inline function be used
> here?

Ok, it is better indeed.

> [snip]
>
> > +#ifdef CONFIG_NTP_PPS
> > +
> > +/* normalize the timestamp so that nsec is in the
> > + ( -NSEC_PER_SEC / 2, NSEC_PER_SEC / 2 ] interval */
> > +static inline struct timespec pps_normalize_ts(struct timespec ts)
> > +{
> > + if (ts.tv_nsec > (NSEC_PER_SEC >> 1)) {
> > + ts.tv_nsec -= NSEC_PER_SEC;
> > + ts.tv_sec++;
> > + }
> > +
> > + return ts;
> > +}
>
> Hrmm.. So by converting a timespec into a second + [-NSEC_PER_SEC/2,
> NSEC_PER_SEC/2] value, you aren't really using a timespec anymore,
> right? I mean, the timepsec_valid() would fail in many cases, for
> instance.
>
> I realize its pretty close, and you *can* use a timespec this way, but
> to me it seems reasonable to introduce a new structure (pps_normtime?)
> so that its clear you shouldn't exchange timespec values and
> pps_normtime values directly?
>
> This is sort of a nit, and maybe others disagree, so not the most
> critical issue. But it seems like you're abusing the structure a bit.

Well, I don't mind adding a structure for this. Seems like timespec
is not expected to be used this way.

> [snip]
> > +/* decrease frequency calibration interval length */
> > +static inline void pps_dec_freq_interval(void)
> > +{
> > + if (--pps_intcnt <= -4) {
> > + pps_intcnt = -4;
>
> Is -4 a magic value?
>
> [snip]
> > + } else { /* good sample */
> > + if (++pps_intcnt >= 4) {
> > + pps_intcnt = 4;
>
> Again, the magic 4?

Yep :)
The values are from the reference implementation. Frequency calculation
is mostly the same as in the original code because it works good.
I'll add a define for these values.

> [snip]
> > + pps_stabil += (div_s64(((s64)delta_mod) <<
> > + (NTP_SCALE_SHIFT - SHIFT_USEC),
> > + NSEC_PER_USEC) - pps_stabil) >>
> > PPS_FAVG;
>
> Hmm. Two 64bit divides in this path? This code would run each second,
> right? It would probably be good to see if this could be optimized a
> little bit more, but I guess its similar to ntp_update_frequency()
> which is called on adjtimex() calls (every 16 seconds at most with
> networked ntp).

Well, this is not quite right. The interval is at least 4 seconds (1
<< PPS_FAVG) and increases if the signal is good. Maximum interval
length is 256 seconds (1 << PPS_FAVGDEF).
I don't see how it can be optimized right now.

> > +void hardpps(const struct timespec *p_ts, s64 nsec)
> > +{
> > + struct timespec ts_norm, freq_norm;
> > +
> > + ts_norm = pps_normalize_ts(*p_ts);
> > + freq_norm = pps_normalize_ts(ns_to_timespec(nsec));
> > +
> [snip]
> > +
> > + write_seqlock_irq(&xtime_lock);
>
> This is called possibly in irq context, right? So you probably want to
> use write_seqlock_irqsave(), no?

Right, thanks, it's bug. However, I think of moving it to a worqueue.

--
Alexander


Attachments:
signature.asc (490.00 B)

2010-02-04 11:05:32

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [PATCH 2/5] pps: capture MONOTONIC_RAW timestamps as well

В Wed, 03 Feb 2010 14:26:16 -0800
john stultz <[email protected]> пишет:

> On Wed, 2010-02-03 at 23:56 +0300, Alexander Gordeev wrote:
> > MONOTONIC_RAW clock timestamps are ideally suited for frequency
> > calculation and also fit well into the original NTP hardpps design.
> > Now phase and frequency can be adjusted separately: the former
> > based on REALTIME clock and the latter based on MONOTONIC_RAW clock.
> > A new function getnstime_raw_and_real is added to timekeeping
> > subsystem to capture both timestamps at the same time and
> > atomically.
>
> Hrmm. So while I understand the need for it, the
> getnstime_raw_and_real() makes me cringe a little. Part of the issue
> is that there are multiple CLOCK_IDs and the current interface allows
> for accesses to only one at a time. There's a similar hack in the
> hrtimer code to get the CLOCK_REALTIME and CLOCK_MONOTONIC values at
> the same time. Next I worry that folks will want a
> getnstime_mono_and_raw() or a getnstime_real_mono_and_raw(), then a
> getnstime_real_and_realcoarse(), etc..
>
> I'm almost thinking the way to handle this would be a better
> abstraction, like a get_two_times(CLOCKID, timepsec*, CLOCKID,
> timespec*). But that might need some further discussion. Anyone else
> have thoughts here?

Agreed with you completely. I don't like the approach but failed to
invent anything better.

> So yea not opposed to this patch, but maybe try to avoid exporting the
> symbol, so modules don't end up using it and we can change it fairly
> easily later.

Well, I'd like to, but how? getnstime_raw_and_real() is used in
module added in one of the next commits.

--
Alexander


Attachments:
signature.asc (489.00 B)

2010-02-05 11:41:11

by Rodolfo Giometti

[permalink] [raw]
Subject: Re: [PATCH 4/5] pps: add parallel port PPS signal generator

On Wed, Feb 03, 2010 at 11:56:50PM +0300, Alexander Gordeev wrote:
> Add PPS signal generator which utilizes STROBE pin of a parallel port to
> send PPS signals. It uses parport abstraction layer and hrtimers to
> precisely control the signal.

What this generators are used for?

Ciao,

Rodolfo

--

GNU/Linux Solutions e-mail: [email protected]
Linux Device Driver [email protected]
Embedded Systems phone: +39 349 2432127
UNIX programming skype: rodolfo.giometti
Freelance ICT Italia - Consulente ICT Italia - http://www.consulenti-ict.it

2010-02-06 08:57:58

by Alexander Gordeev

[permalink] [raw]
Subject: Re: [LinuxPPS] [PATCH 4/5] pps: add parallel port PPS signal generator

Hi Rodolfo,

I've accidentally found your email in the archives of linuxpps list.
Very strange that it didn't come to me directly because I was
subscribed...

В Fri, 5 Feb 2010 11:39:14 +0100
giometti at enneenne.com (Rodolfo Giometti) пишет:

> On Wed, Feb 03, 2010 at 11:56:50PM +0300, Alexander Gordeev wrote:
> > Add PPS signal generator which utilizes STROBE pin of a parallel
> > port to send PPS signals. It uses parport abstraction layer and
> > hrtimers to precisely control the signal.
>
> What this generators are used for?

This generator sends PPS signals over a parallel port from one computer
to another. So it's just doing what every PPS-capable GPS device does.
I use it to precisely synchronize time on several computers for
distributed real-time simulation. It has very strict timing conditions.



--
Alexander


Attachments:
signature.asc (490.00 B)