linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv6 00/16] pps: several fixes and improvements
@ 2010-12-17 19:54 Alexander Gordeev
  2010-12-17 19:54 ` [PATCHv6 01/16] pps: trivial fixes Alexander Gordeev
                   ` (17 more replies)
  0 siblings, 18 replies; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-17 19:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Nikita V. Youshchenko, linuxpps, Rodolfo Giometti, Andrew Morton,
	Alexander Gordeev

This patchset contains several changes that improve an overall
design/performance of PPS subsystem. I'd like these patches to be
merged mainline if no one objects.

Patches 1-3 are bugfixes.
Patches 4-11 are other improvements to PPS subsystem.
Patches 12-14 add kernel consumer support.
Patch 15 adds parallel port PPS client.
Patch 16 adds parallel port PPS generator.

You can find description for my previous patchset (it describes patches
12-16 in more detail) here: http://lkml.org/lkml/2010/2/24/189

This patchset is tested against the vanilla 2.6.37-rc6 kernel. But we are
actually using it on 2.6.33.7-rt29 rt-preempt kernel most of the time.
Those who are interested in other versions of the patchset can find
them in my git repository:
git://github.com/ago/linux-2.6.git

There is one problem however: kernel consumer works bad (if enabled)
when CONFIG_NO_HZ is enabled. The reason for this is commit
a092ff0f90cae22b2ac8028ecd2c6f6c1a9e4601. Without it hardpps() is able
to sync to 1us precision in about 10 seconds. With CONFIG_NO_HZ it is
not syncing at all. This only affects patches 12-14, others are ok.

Changelog
v5 -> v6:
 * drop patch that fixes a race condition in tty code because it's already
   fixed in 2.6.37-rc2
 * fix getnstime_raw_and_real() comments (thanks Thomas Gleixner for the note)

Alexander Gordeev (16):
  pps: trivial fixes
  pps: declare variables where they are used in switch
  pps: fix race in PPS_FETCH handler
  pps: unify timestamp gathering
  pps: access pps device by direct pointer
  pps: convert printk/pr_* to dev_*
  pps: move idr stuff to pps.c
  pps: do not disable interrupts for idr operations
  pps: use BUG_ON for kernel API safety checks
  pps: simplify conditions a bit
  pps: timestamp is always passed to dcd_change()
  ntp: add hardpps implementation
  pps: capture MONOTONIC_RAW timestamps as well
  pps: add kernel consumer support
  pps: add parallel port PPS client
  pps: add parallel port PPS signal generator

 Documentation/ioctl/ioctl-number.txt     |    2 +-
 Documentation/pps/pps.txt                |   46 ++++
 Documentation/serial/tty.txt             |    2 +-
 drivers/pps/Kconfig                      |   10 +
 drivers/pps/Makefile                     |    2 +-
 drivers/pps/clients/Kconfig              |    7 +
 drivers/pps/clients/Makefile             |    1 +
 drivers/pps/clients/pps-ktimer.c         |   44 ++--
 drivers/pps/clients/pps-ldisc.c          |   59 ++--
 drivers/pps/clients/pps_parport.c        |  247 +++++++++++++++++
 drivers/pps/generators/Kconfig           |   17 ++
 drivers/pps/generators/Makefile          |    9 +
 drivers/pps/generators/pps_gen_parport.c |  275 +++++++++++++++++++
 drivers/pps/kapi.c                       |  221 +++++-----------
 drivers/pps/pps.c                        |  177 ++++++++++---
 include/linux/pps.h                      |    7 +
 include/linux/pps_kernel.h               |   54 +++-
 include/linux/serial_core.h              |    5 +-
 include/linux/time.h                     |    2 +
 include/linux/timex.h                    |    1 +
 include/linux/tty_ldisc.h                |    7 +-
 kernel/time/ntp.c                        |  425 ++++++++++++++++++++++++++++-
 kernel/time/timekeeping.c                |   39 +++
 23 files changed, 1374 insertions(+), 285 deletions(-)
 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

-- 
1.7.2.3


^ permalink raw reply	[flat|nested] 37+ messages in thread

* [PATCHv6 01/16] pps: trivial fixes
  2010-12-17 19:54 [PATCHv6 00/16] pps: several fixes and improvements Alexander Gordeev
@ 2010-12-17 19:54 ` Alexander Gordeev
  2010-12-17 19:54 ` [PATCHv6 02/16] pps: declare variables where they are used in switch Alexander Gordeev
                   ` (16 subsequent siblings)
  17 siblings, 0 replies; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-17 19:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Nikita V. Youshchenko, linuxpps, Rodolfo Giometti, Andrew Morton,
	Alexander Gordeev, Linus Torvalds

Here are some very trivial fixes combined:
 * add macro definitions to protect header file from including several times
 * remove declaration for an unexistent array
 * fix typos

Acked-by: Rodolfo Giometti <giometti@linux.it>
Signed-off-by: Alexander Gordeev <lasaine@lvk.cs.msu.su>
---
 drivers/pps/kapi.c         |    2 +-
 include/linux/pps_kernel.h |    7 ++++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/pps/kapi.c b/drivers/pps/kapi.c
index 1aa02db..55f3961 100644
--- a/drivers/pps/kapi.c
+++ b/drivers/pps/kapi.c
@@ -324,7 +324,7 @@ void pps_event(int source, struct pps_ktime *ts, int event, void *data)
 		captured = ~0;
 	}
 
-	/* Wake up iif captured somthing */
+	/* Wake up if captured something */
 	if (captured) {
 		pps->go = ~0;
 		wake_up_interruptible(&pps->queue);
diff --git a/include/linux/pps_kernel.h b/include/linux/pps_kernel.h
index e0a193f..c930d11 100644
--- a/include/linux/pps_kernel.h
+++ b/include/linux/pps_kernel.h
@@ -18,6 +18,9 @@
  *   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>
@@ -71,7 +74,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[];
 
@@ -87,3 +89,6 @@ 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);
+
+#endif /* LINUX_PPS_KERNEL_H */
+
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCHv6 02/16] pps: declare variables where they are used in switch
  2010-12-17 19:54 [PATCHv6 00/16] pps: several fixes and improvements Alexander Gordeev
  2010-12-17 19:54 ` [PATCHv6 01/16] pps: trivial fixes Alexander Gordeev
@ 2010-12-17 19:54 ` Alexander Gordeev
  2010-12-17 19:54 ` [PATCHv6 03/16] pps: fix race in PPS_FETCH handler Alexander Gordeev
                   ` (15 subsequent siblings)
  17 siblings, 0 replies; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-17 19:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Nikita V. Youshchenko, linuxpps, Rodolfo Giometti, Andrew Morton,
	Alexander Gordeev, Linus Torvalds

Move variable declarations where they are used in pps_cdev_ioctl.

Acked-by: Rodolfo Giometti <giometti@linux.it>
Signed-off-by: Alexander Gordeev <lasaine@lvk.cs.msu.su>
---
 drivers/pps/pps.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index ca5183b..c76afb9 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -61,8 +61,6 @@ static long pps_cdev_ioctl(struct file *file,
 {
 	struct pps_device *pps = file->private_data;
 	struct pps_kparams params;
-	struct pps_fdata fdata;
-	unsigned long ticks;
 	void __user *uarg = (void __user *) arg;
 	int __user *iuarg = (int __user *) arg;
 	int err;
@@ -136,7 +134,9 @@ static long pps_cdev_ioctl(struct file *file,
 
 		break;
 
-	case PPS_FETCH:
+	case PPS_FETCH: {
+		struct pps_fdata fdata;
+
 		pr_debug("PPS_FETCH: source %d\n", pps->id);
 
 		err = copy_from_user(&fdata, uarg, sizeof(struct pps_fdata));
@@ -149,6 +149,8 @@ static long pps_cdev_ioctl(struct file *file,
 		if (fdata.timeout.flags & PPS_TIME_INVALID)
 			err = wait_event_interruptible(pps->queue, pps->go);
 		else {
+			unsigned long ticks;
+
 			pr_debug("timeout %lld.%09d\n",
 					(long long) fdata.timeout.sec,
 					fdata.timeout.nsec);
@@ -185,7 +187,7 @@ static long pps_cdev_ioctl(struct file *file,
 			return -EFAULT;
 
 		break;
-
+	}
 	default:
 		return -ENOTTY;
 		break;
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCHv6 03/16] pps: fix race in PPS_FETCH handler
  2010-12-17 19:54 [PATCHv6 00/16] pps: several fixes and improvements Alexander Gordeev
  2010-12-17 19:54 ` [PATCHv6 01/16] pps: trivial fixes Alexander Gordeev
  2010-12-17 19:54 ` [PATCHv6 02/16] pps: declare variables where they are used in switch Alexander Gordeev
@ 2010-12-17 19:54 ` Alexander Gordeev
  2010-12-17 19:54 ` [PATCHv6 04/16] pps: unify timestamp gathering Alexander Gordeev
                   ` (14 subsequent siblings)
  17 siblings, 0 replies; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-17 19:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Nikita V. Youshchenko, linuxpps, Rodolfo Giometti, Andrew Morton,
	Alexander Gordeev, Linus Torvalds

There was a race in PPS_FETCH ioctl handler when several processes want
to obtain PPS data simultaneously using sleeping PPS_FETCH. They all
sleep most of the time in the system call.
With the old approach when the first process waiting on the pps queue
is waken up it makes new system call right away and zeroes pps->go. So
other processes continue to sleep. This is a clear race condition
because of the global 'go' variable.
With the new approach pps->last_ev holds some value increasing at each
PPS event. PPS_FETCH ioctl handler saves current value to the local
variable at the very beginning so it can safely check that there is a
new event by just comparing both variables.

Acked-by: Rodolfo Giometti <giometti@linux.it>
Signed-off-by: Alexander Gordeev <lasaine@lvk.cs.msu.su>
---
 drivers/pps/kapi.c         |    4 ++--
 drivers/pps/pps.c          |   10 +++++++---
 include/linux/pps_kernel.h |    2 +-
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/pps/kapi.c b/drivers/pps/kapi.c
index 55f3961..3f89f5eb 100644
--- a/drivers/pps/kapi.c
+++ b/drivers/pps/kapi.c
@@ -326,8 +326,8 @@ void pps_event(int source, struct pps_ktime *ts, int event, void *data)
 
 	/* Wake up if captured something */
 	if (captured) {
-		pps->go = ~0;
-		wake_up_interruptible(&pps->queue);
+		pps->last_ev++;
+		wake_up_interruptible_all(&pps->queue);
 
 		kill_fasync(&pps->async_queue, SIGIO, POLL_IN);
 	}
diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index c76afb9..dc7e66c 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -136,6 +136,7 @@ static long pps_cdev_ioctl(struct file *file,
 
 	case PPS_FETCH: {
 		struct pps_fdata fdata;
+		unsigned int ev;
 
 		pr_debug("PPS_FETCH: source %d\n", pps->id);
 
@@ -143,11 +144,12 @@ static long pps_cdev_ioctl(struct file *file,
 		if (err)
 			return -EFAULT;
 
-		pps->go = 0;
+		ev = pps->last_ev;
 
 		/* Manage the timeout */
 		if (fdata.timeout.flags & PPS_TIME_INVALID)
-			err = wait_event_interruptible(pps->queue, pps->go);
+			err = wait_event_interruptible(pps->queue,
+					ev != pps->last_ev);
 		else {
 			unsigned long ticks;
 
@@ -159,7 +161,9 @@ static long pps_cdev_ioctl(struct file *file,
 
 			if (ticks != 0) {
 				err = wait_event_interruptible_timeout(
-						pps->queue, pps->go, ticks);
+						pps->queue,
+						ev != pps->last_ev,
+						ticks);
 				if (err == 0)
 					return -ETIMEDOUT;
 			}
diff --git a/include/linux/pps_kernel.h b/include/linux/pps_kernel.h
index c930d11..c3aed4b 100644
--- a/include/linux/pps_kernel.h
+++ b/include/linux/pps_kernel.h
@@ -55,7 +55,7 @@ struct pps_device {
 	struct pps_ktime clear_tu;
 	int current_mode;			/* PPS mode at event time */
 
-	int go;					/* PPS event is arrived? */
+	unsigned int last_ev;			/* last PPS event id */
 	wait_queue_head_t queue;		/* PPS event queue */
 
 	unsigned int id;			/* PPS source unique ID */
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCHv6 04/16] pps: unify timestamp gathering
  2010-12-17 19:54 [PATCHv6 00/16] pps: several fixes and improvements Alexander Gordeev
                   ` (2 preceding siblings ...)
  2010-12-17 19:54 ` [PATCHv6 03/16] pps: fix race in PPS_FETCH handler Alexander Gordeev
@ 2010-12-17 19:54 ` Alexander Gordeev
  2010-12-17 19:54 ` [PATCHv6 05/16] pps: access pps device by direct pointer Alexander Gordeev
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-17 19:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Nikita V. Youshchenko, linuxpps, Rodolfo Giometti, Andrew Morton,
	Alexander Gordeev, Greg Kroah-Hartman

Add a helper function to gather timestamps. This way clients don't have
to duplicate it.

Acked-by: Rodolfo Giometti <giometti@linux.it>
Signed-off-by: Alexander Gordeev <lasaine@lvk.cs.msu.su>
---
 drivers/pps/clients/pps-ktimer.c |    9 ++-------
 drivers/pps/clients/pps-ldisc.c  |   18 ++++++------------
 drivers/pps/kapi.c               |   19 ++++++++++++-------
 include/linux/pps_kernel.h       |   19 ++++++++++++++++++-
 include/linux/serial_core.h      |    5 +++--
 include/linux/tty_ldisc.h        |    5 +++--
 6 files changed, 44 insertions(+), 31 deletions(-)

diff --git a/drivers/pps/clients/pps-ktimer.c b/drivers/pps/clients/pps-ktimer.c
index e7ef5b8..e1bdd8b 100644
--- a/drivers/pps/clients/pps-ktimer.c
+++ b/drivers/pps/clients/pps-ktimer.c
@@ -40,18 +40,13 @@ static struct timer_list ktimer;
 
 static void pps_ktimer_event(unsigned long ptr)
 {
-	struct timespec __ts;
-	struct pps_ktime ts;
+	struct pps_event_time ts;
 
 	/* First of all we get the time stamp... */
-	getnstimeofday(&__ts);
+	pps_get_ts(&ts);
 
 	pr_info("PPS event at %lu\n", jiffies);
 
-	/* ... and translate it to PPS time data struct */
-	ts.sec = __ts.tv_sec;
-	ts.nsec = __ts.tv_nsec;
-
 	pps_event(source, &ts, PPS_CAPTUREASSERT, NULL);
 
 	mod_timer(&ktimer, jiffies + HZ);
diff --git a/drivers/pps/clients/pps-ldisc.c b/drivers/pps/clients/pps-ldisc.c
index 8e1932d..20fc9f7 100644
--- a/drivers/pps/clients/pps-ldisc.c
+++ b/drivers/pps/clients/pps-ldisc.c
@@ -27,26 +27,20 @@
 #define PPS_TTY_MAGIC		0x0001
 
 static void pps_tty_dcd_change(struct tty_struct *tty, unsigned int status,
-				struct timespec *ts)
+				struct pps_event_time *ts)
 {
 	int id = (long)tty->disc_data;
-	struct timespec __ts;
-	struct pps_ktime pps_ts;
+	struct pps_event_time __ts;
 
 	/* First of all we get the time stamp... */
-	getnstimeofday(&__ts);
+	pps_get_ts(&__ts);
 
 	/* Does caller give us a timestamp? */
-	if (ts) {	/* Yes. Let's use it! */
-		pps_ts.sec = ts->tv_sec;
-		pps_ts.nsec = ts->tv_nsec;
-	} else {	/* No. Do it ourself! */
-		pps_ts.sec = __ts.tv_sec;
-		pps_ts.nsec = __ts.tv_nsec;
-	}
+	if (!ts)	/* No. Do it ourself! */
+		ts = &__ts;
 
 	/* Now do the PPS event report */
-	pps_event(id, &pps_ts, status ? PPS_CAPTUREASSERT : PPS_CAPTURECLEAR,
+	pps_event(id, ts, status ? PPS_CAPTUREASSERT : PPS_CAPTURECLEAR,
 			NULL);
 
 	pr_debug("PPS %s at %lu on source #%d\n",
diff --git a/drivers/pps/kapi.c b/drivers/pps/kapi.c
index 3f89f5eb..b431d83 100644
--- a/drivers/pps/kapi.c
+++ b/drivers/pps/kapi.c
@@ -268,11 +268,12 @@ 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;
 	int captured = 0;
+	struct pps_ktime ts_real;
 
 	if ((event & (PPS_CAPTUREASSERT | PPS_CAPTURECLEAR)) == 0) {
 		printk(KERN_ERR "pps: unknown event (%x) for source %d\n",
@@ -284,8 +285,10 @@ void pps_event(int source, struct pps_ktime *ts, int event, void *data)
 	if (!pps)
 		return;
 
-	pr_debug("PPS event on source %d at %llu.%06u\n",
-			pps->id, (unsigned long long) ts->sec, ts->nsec);
+	pr_debug("PPS event on source %d at %ld.%09ld\n",
+			pps->id, ts->ts_real.tv_sec, ts->ts_real.tv_nsec);
+
+	timespec_to_pps_ktime(&ts_real, ts->ts_real);
 
 	spin_lock_irqsave(&pps->lock, flags);
 
@@ -299,10 +302,11 @@ 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_real,
+					&pps->params.assert_off_tu);
 
 		/* Save the time stamp */
-		pps->assert_tu = *ts;
+		pps->assert_tu = ts_real;
 		pps->assert_sequence++;
 		pr_debug("capture assert seq #%u for source %d\n",
 			pps->assert_sequence, source);
@@ -313,10 +317,11 @@ 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_real,
+					&pps->params.clear_off_tu);
 
 		/* Save the time stamp */
-		pps->clear_tu = *ts;
+		pps->clear_tu = ts_real;
 		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 c3aed4b..32aa676 100644
--- a/include/linux/pps_kernel.h
+++ b/include/linux/pps_kernel.h
@@ -43,6 +43,10 @@ struct pps_source_info {
 	struct device *dev;
 };
 
+struct pps_event_time {
+	struct timespec ts_real;
+};
+
 /* The main struct */
 struct pps_device {
 	struct pps_source_info info;		/* PSS source info */
@@ -88,7 +92,20 @@ 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)
+{
+	getnstimeofday(&ts->ts_real);
+}
 
 #endif /* LINUX_PPS_KERNEL_H */
 
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 212eb4c..7a0e9ce 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -209,6 +209,7 @@
 #include <linux/tty.h>
 #include <linux/mutex.h>
 #include <linux/sysrq.h>
+#include <linux/pps_kernel.h>
 
 struct uart_port;
 struct serial_struct;
@@ -523,10 +524,10 @@ uart_handle_dcd_change(struct uart_port *uport, unsigned int status)
 	struct uart_state *state = uport->state;
 	struct tty_port *port = &state->port;
 	struct tty_ldisc *ld = tty_ldisc_ref(port->tty);
-	struct timespec ts;
+	struct pps_event_time ts;
 
 	if (ld && ld->ops->dcd_change)
-		getnstimeofday(&ts);
+		pps_get_ts(&ts);
 
 	uport->icount.dcd++;
 #ifdef CONFIG_HARD_PPS
diff --git a/include/linux/tty_ldisc.h b/include/linux/tty_ldisc.h
index 526d66f..763e061 100644
--- a/include/linux/tty_ldisc.h
+++ b/include/linux/tty_ldisc.h
@@ -101,7 +101,7 @@
  *	any pending driver I/O is completed.
  *
  * void (*dcd_change)(struct tty_struct *tty, unsigned int status,
- * 			struct timespec *ts)
+ *			struct pps_event_time *ts)
  *
  *	Tells the discipline that the DCD pin has changed its status and
  *	the relative timestamp. Pointer ts can be NULL.
@@ -109,6 +109,7 @@
 
 #include <linux/fs.h>
 #include <linux/wait.h>
+#include <linux/pps_kernel.h>
 
 struct tty_ldisc_ops {
 	int	magic;
@@ -143,7 +144,7 @@ struct tty_ldisc_ops {
 			       char *fp, int count);
 	void	(*write_wakeup)(struct tty_struct *);
 	void	(*dcd_change)(struct tty_struct *, unsigned int,
-				struct timespec *);
+				struct pps_event_time *);
 
 	struct  module *owner;
 	
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCHv6 05/16] pps: access pps device by direct pointer
  2010-12-17 19:54 [PATCHv6 00/16] pps: several fixes and improvements Alexander Gordeev
                   ` (3 preceding siblings ...)
  2010-12-17 19:54 ` [PATCHv6 04/16] pps: unify timestamp gathering Alexander Gordeev
@ 2010-12-17 19:54 ` Alexander Gordeev
  2010-12-17 19:54 ` [PATCHv6 06/16] pps: convert printk/pr_* to dev_* Alexander Gordeev
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-17 19:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Nikita V. Youshchenko, linuxpps, Rodolfo Giometti, Andrew Morton,
	Alexander Gordeev

Using device index as a pointer needs some unnecessary work to be done
every time the pointer is needed (in irq handler for example).
Using a direct pointer is much more easy (and safe as well).

Acked-by: Rodolfo Giometti <giometti@linux.it>
Signed-off-by: Alexander Gordeev <lasaine@lvk.cs.msu.su>
---
 drivers/pps/clients/pps-ktimer.c |   30 ++++-----
 drivers/pps/clients/pps-ldisc.c  |   41 ++++++++-----
 drivers/pps/kapi.c               |  124 +++++++++-----------------------------
 drivers/pps/pps.c                |   22 ++-----
 include/linux/pps_kernel.h       |   23 +++----
 5 files changed, 82 insertions(+), 158 deletions(-)

diff --git a/drivers/pps/clients/pps-ktimer.c b/drivers/pps/clients/pps-ktimer.c
index e1bdd8b..966ead1 100644
--- a/drivers/pps/clients/pps-ktimer.c
+++ b/drivers/pps/clients/pps-ktimer.c
@@ -31,7 +31,7 @@
  * Global variables
  */
 
-static int source;
+static struct pps_device *pps;
 static struct timer_list ktimer;
 
 /*
@@ -47,7 +47,7 @@ static void pps_ktimer_event(unsigned long ptr)
 
 	pr_info("PPS event at %lu\n", jiffies);
 
-	pps_event(source, &ts, PPS_CAPTUREASSERT, NULL);
+	pps_event(pps, &ts, PPS_CAPTUREASSERT, NULL);
 
 	mod_timer(&ktimer, jiffies + HZ);
 }
@@ -56,12 +56,11 @@ static void pps_ktimer_event(unsigned long ptr)
  * The echo function
  */
 
-static void pps_ktimer_echo(int source, int event, void *data)
+static void pps_ktimer_echo(struct pps_device *pps, int event, void *data)
 {
-	pr_info("echo %s %s for source %d\n",
+	dev_info(pps->dev, "echo %s %s\n",
 		event & PPS_CAPTUREASSERT ? "assert" : "",
-		event & PPS_CAPTURECLEAR ? "clear" : "",
-		source);
+		event & PPS_CAPTURECLEAR ? "clear" : "");
 }
 
 /*
@@ -84,30 +83,27 @@ static struct pps_source_info pps_ktimer_info = {
 
 static void __exit pps_ktimer_exit(void)
 {
-	del_timer_sync(&ktimer);
-	pps_unregister_source(source);
+	dev_info(pps->dev, "ktimer PPS source unregistered\n");
 
-	pr_info("ktimer PPS source unregistered\n");
+	del_timer_sync(&ktimer);
+	pps_unregister_source(pps);
 }
 
 static int __init pps_ktimer_init(void)
 {
-	int ret;
-
-	ret = pps_register_source(&pps_ktimer_info,
+	pps = pps_register_source(&pps_ktimer_info,
 				PPS_CAPTUREASSERT | PPS_OFFSETASSERT);
-	if (ret < 0) {
+	if (pps == NULL) {
 		printk(KERN_ERR "cannot register ktimer source\n");
-		return ret;
+		return -ENOMEM;
 	}
-	source = ret;
 
 	setup_timer(&ktimer, pps_ktimer_event, 0);
 	mod_timer(&ktimer, jiffies + HZ);
 
-	pr_info("ktimer PPS source registered at %d\n", source);
+	dev_info(pps->dev, "ktimer PPS source registered\n");
 
-	return  0;
+	return 0;
 }
 
 module_init(pps_ktimer_init);
diff --git a/drivers/pps/clients/pps-ldisc.c b/drivers/pps/clients/pps-ldisc.c
index 20fc9f7..1517f54 100644
--- a/drivers/pps/clients/pps-ldisc.c
+++ b/drivers/pps/clients/pps-ldisc.c
@@ -29,7 +29,7 @@
 static void pps_tty_dcd_change(struct tty_struct *tty, unsigned int status,
 				struct pps_event_time *ts)
 {
-	int id = (long)tty->disc_data;
+	struct pps_device *pps = (struct pps_device *)tty->disc_data;
 	struct pps_event_time __ts;
 
 	/* First of all we get the time stamp... */
@@ -39,12 +39,14 @@ static void pps_tty_dcd_change(struct tty_struct *tty, unsigned int status,
 	if (!ts)	/* No. Do it ourself! */
 		ts = &__ts;
 
+	BUG_ON(pps == NULL);
+
 	/* Now do the PPS event report */
-	pps_event(id, ts, status ? PPS_CAPTUREASSERT : PPS_CAPTURECLEAR,
-			NULL);
+	pps_event(pps, ts, status ? PPS_CAPTUREASSERT :
+			PPS_CAPTURECLEAR, NULL);
 
-	pr_debug("PPS %s at %lu on source #%d\n",
-			status ? "assert" : "clear", jiffies, id);
+	dev_dbg(pps->dev, "PPS %s at %lu\n",
+			status ? "assert" : "clear", jiffies);
 }
 
 static int (*alias_n_tty_open)(struct tty_struct *tty);
@@ -54,6 +56,7 @@ static int pps_tty_open(struct tty_struct *tty)
 	struct pps_source_info info;
 	struct tty_driver *drv = tty->driver;
 	int index = tty->index + drv->name_base;
+	struct pps_device *pps;
 	int ret;
 
 	info.owner = THIS_MODULE;
@@ -64,34 +67,42 @@ static int pps_tty_open(struct tty_struct *tty)
 			PPS_OFFSETASSERT | PPS_OFFSETCLEAR | \
 			PPS_CANWAIT | PPS_TSFMT_TSPEC;
 
-	ret = pps_register_source(&info, PPS_CAPTUREBOTH | \
+	pps = pps_register_source(&info, PPS_CAPTUREBOTH | \
 				PPS_OFFSETASSERT | PPS_OFFSETCLEAR);
-	if (ret < 0) {
+	if (pps == NULL) {
 		pr_err("cannot register PPS source \"%s\"\n", info.path);
-		return ret;
+		return -ENOMEM;
 	}
-	tty->disc_data = (void *)(long)ret;
+	tty->disc_data = pps;
 
 	/* Should open N_TTY ldisc too */
 	ret = alias_n_tty_open(tty);
-	if (ret < 0)
-		pps_unregister_source((long)tty->disc_data);
+	if (ret < 0) {
+		pr_err("cannot open tty ldisc \"%s\"\n", info.path);
+		goto err_unregister;
+	}
 
-	pr_info("PPS source #%d \"%s\" added\n", ret, info.path);
+	dev_info(pps->dev, "source \"%s\" added\n", info.path);
 
 	return 0;
+
+err_unregister:
+	tty->disc_data = NULL;
+	pps_unregister_source(pps);
+	return ret;
 }
 
 static void (*alias_n_tty_close)(struct tty_struct *tty);
 
 static void pps_tty_close(struct tty_struct *tty)
 {
-	int id = (long)tty->disc_data;
+	struct pps_device *pps = (struct pps_device *)tty->disc_data;
 
-	pps_unregister_source(id);
 	alias_n_tty_close(tty);
 
-	pr_info("PPS source #%d removed\n", id);
+	tty->disc_data = NULL;
+	dev_info(pps->dev, "removed\n");
+	pps_unregister_source(pps);
 }
 
 static struct tty_ldisc_ops pps_ldisc_ops;
diff --git a/drivers/pps/kapi.c b/drivers/pps/kapi.c
index b431d83..98d4012 100644
--- a/drivers/pps/kapi.c
+++ b/drivers/pps/kapi.c
@@ -32,11 +32,11 @@
 #include <linux/slab.h>
 
 /*
- * Global variables
+ * Local variables
  */
 
-DEFINE_SPINLOCK(pps_idr_lock);
-DEFINE_IDR(pps_idr);
+static DEFINE_SPINLOCK(pps_idr_lock);
+static DEFINE_IDR(pps_idr);
 
 /*
  * Local functions
@@ -60,60 +60,6 @@ static void pps_add_offset(struct pps_ktime *ts, struct pps_ktime *offset)
  * Exported functions
  */
 
-/* pps_get_source - find a PPS source
- * @source: the PPS source ID.
- *
- * This function is used to find an already registered PPS source into the
- * system.
- *
- * The function returns NULL if found nothing, otherwise it returns a pointer
- * to the PPS source data struct (the refcounter is incremented by 1).
- */
-
-struct pps_device *pps_get_source(int source)
-{
-	struct pps_device *pps;
-	unsigned long flags;
-
-	spin_lock_irqsave(&pps_idr_lock, flags);
-
-	pps = idr_find(&pps_idr, source);
-	if (pps != NULL)
-		atomic_inc(&pps->usage);
-
-	spin_unlock_irqrestore(&pps_idr_lock, flags);
-
-	return pps;
-}
-
-/* pps_put_source - free the PPS source data
- * @pps: a pointer to the PPS source.
- *
- * This function is used to free a PPS data struct if its refcount is 0.
- */
-
-void pps_put_source(struct pps_device *pps)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&pps_idr_lock, flags);
-	BUG_ON(atomic_read(&pps->usage) == 0);
-
-	if (!atomic_dec_and_test(&pps->usage)) {
-		pps = NULL;
-		goto exit;
-	}
-
-	/* No more reference to the PPS source. We can safely remove the
-	 * PPS data struct.
-	 */
-	idr_remove(&pps_idr, pps->id);
-
-exit:
-	spin_unlock_irqrestore(&pps_idr_lock, flags);
-	kfree(pps);
-}
-
 /* pps_register_source - add a PPS source in the system
  * @info: the PPS info struct
  * @default_params: the default PPS parameters of the new source
@@ -122,10 +68,11 @@ exit:
  * source is described by info's fields and it will have, as default PPS
  * parameters, the ones specified into default_params.
  *
- * The function returns, in case of success, the PPS source ID.
+ * The function returns, in case of success, the PPS device. Otherwise NULL.
  */
 
-int pps_register_source(struct pps_source_info *info, int default_params)
+struct pps_device *pps_register_source(struct pps_source_info *info,
+		int default_params)
 {
 	struct pps_device *pps;
 	int id;
@@ -168,7 +115,6 @@ int pps_register_source(struct pps_source_info *info, int default_params)
 
 	init_waitqueue_head(&pps->queue);
 	spin_lock_init(&pps->lock);
-	atomic_set(&pps->usage, 1);
 
 	/* Get new ID for the new PPS source */
 	if (idr_pre_get(&pps_idr, GFP_KERNEL) == 0) {
@@ -211,7 +157,7 @@ int pps_register_source(struct pps_source_info *info, int default_params)
 
 	pr_info("new PPS source %s at ID %d\n", info->name, id);
 
-	return id;
+	return pps;
 
 free_idr:
 	spin_lock_irq(&pps_idr_lock);
@@ -224,38 +170,33 @@ kfree_pps:
 pps_register_source_exit:
 	printk(KERN_ERR "pps: %s: unable to register source\n", info->name);
 
-	return err;
+	return NULL;
 }
 EXPORT_SYMBOL(pps_register_source);
 
 /* pps_unregister_source - remove a PPS source from the system
- * @source: the PPS source ID
+ * @pps: the PPS source
  *
  * This function is used to remove a previously registered PPS source from
  * the system.
  */
 
-void pps_unregister_source(int source)
+void pps_unregister_source(struct pps_device *pps)
 {
-	struct pps_device *pps;
+	unsigned int id = pps->id;
 
-	spin_lock_irq(&pps_idr_lock);
-	pps = idr_find(&pps_idr, source);
+	pps_unregister_cdev(pps);
 
-	if (!pps) {
-		BUG();
-		spin_unlock_irq(&pps_idr_lock);
-		return;
-	}
+	spin_lock_irq(&pps_idr_lock);
+	idr_remove(&pps_idr, pps->id);
 	spin_unlock_irq(&pps_idr_lock);
 
-	pps_unregister_cdev(pps);
-	pps_put_source(pps);
+	kfree(pps);
 }
 EXPORT_SYMBOL(pps_unregister_source);
 
 /* pps_event - register a PPS event into the system
- * @source: the PPS source ID
+ * @pps: the PPS device
  * @ts: the event timestamp
  * @event: the event type
  * @data: userdef pointer
@@ -263,30 +204,24 @@ EXPORT_SYMBOL(pps_unregister_source);
  * This function is used by each PPS client in order to register a new
  * PPS event into the system (it's usually called inside an IRQ handler).
  *
- * If an echo function is associated with the PPS source it will be called
+ * If an echo function is associated with the PPS device it will be called
  * as:
- *	pps->info.echo(source, event, data);
+ *	pps->info.echo(pps, event, data);
  */
-
-void pps_event(int source, struct pps_event_time *ts, int event, void *data)
+void pps_event(struct pps_device *pps, struct pps_event_time *ts, int event,
+		void *data)
 {
-	struct pps_device *pps;
 	unsigned long flags;
 	int captured = 0;
 	struct pps_ktime ts_real;
 
 	if ((event & (PPS_CAPTUREASSERT | PPS_CAPTURECLEAR)) == 0) {
-		printk(KERN_ERR "pps: unknown event (%x) for source %d\n",
-			event, source);
+		dev_err(pps->dev, "unknown event (%x)\n", event);
 		return;
 	}
 
-	pps = pps_get_source(source);
-	if (!pps)
-		return;
-
-	pr_debug("PPS event on source %d at %ld.%09ld\n",
-			pps->id, ts->ts_real.tv_sec, ts->ts_real.tv_nsec);
+	dev_dbg(pps->dev, "PPS event at %ld.%09ld\n",
+			ts->ts_real.tv_sec, ts->ts_real.tv_nsec);
 
 	timespec_to_pps_ktime(&ts_real, ts->ts_real);
 
@@ -294,7 +229,7 @@ void pps_event(int source, struct pps_event_time *ts, int event, void *data)
 
 	/* Must call the echo function? */
 	if ((pps->params.mode & (PPS_ECHOASSERT | PPS_ECHOCLEAR)))
-		pps->info.echo(source, event, data);
+		pps->info.echo(pps, event, data);
 
 	/* Check the event */
 	pps->current_mode = pps->params.mode;
@@ -308,8 +243,8 @@ void pps_event(int source, struct pps_event_time *ts, int event, void *data)
 		/* Save the time stamp */
 		pps->assert_tu = ts_real;
 		pps->assert_sequence++;
-		pr_debug("capture assert seq #%u for source %d\n",
-			pps->assert_sequence, source);
+		dev_dbg(pps->dev, "capture assert seq #%u\n",
+			pps->assert_sequence);
 
 		captured = ~0;
 	}
@@ -323,8 +258,8 @@ void pps_event(int source, struct pps_event_time *ts, int event, void *data)
 		/* Save the time stamp */
 		pps->clear_tu = ts_real;
 		pps->clear_sequence++;
-		pr_debug("capture clear seq #%u for source %d\n",
-			pps->clear_sequence, source);
+		dev_dbg(pps->dev, "capture clear seq #%u\n",
+			pps->clear_sequence);
 
 		captured = ~0;
 	}
@@ -338,8 +273,5 @@ void pps_event(int source, struct pps_event_time *ts, int event, void *data)
 	}
 
 	spin_unlock_irqrestore(&pps->lock, flags);
-
-	/* Now we can release the PPS source for (possible) deregistration */
-	pps_put_source(pps);
 }
 EXPORT_SYMBOL(pps_event);
diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index dc7e66c..1922f27 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -204,12 +204,6 @@ static int pps_cdev_open(struct inode *inode, struct file *file)
 {
 	struct pps_device *pps = container_of(inode->i_cdev,
 						struct pps_device, cdev);
-	int found;
-
-	found = pps_get_source(pps->id) != 0;
-	if (!found)
-		return -ENODEV;
-
 	file->private_data = pps;
 
 	return 0;
@@ -217,11 +211,6 @@ static int pps_cdev_open(struct inode *inode, struct file *file)
 
 static int pps_cdev_release(struct inode *inode, struct file *file)
 {
-	struct pps_device *pps = file->private_data;
-
-	/* Free the PPS source and wake up (possible) deregistration */
-	pps_put_source(pps);
-
 	return 0;
 }
 
@@ -242,22 +231,23 @@ static const struct file_operations pps_cdev_fops = {
 int pps_register_cdev(struct pps_device *pps)
 {
 	int err;
+	dev_t devt;
+
+	devt = MKDEV(MAJOR(pps_devt), pps->id);
 
-	pps->devno = MKDEV(MAJOR(pps_devt), pps->id);
 	cdev_init(&pps->cdev, &pps_cdev_fops);
 	pps->cdev.owner = pps->info.owner;
 
-	err = cdev_add(&pps->cdev, pps->devno, 1);
+	err = cdev_add(&pps->cdev, devt, 1);
 	if (err) {
 		printk(KERN_ERR "pps: %s: failed to add char device %d:%d\n",
 				pps->info.name, MAJOR(pps_devt), pps->id);
 		return err;
 	}
-	pps->dev = device_create(pps_class, pps->info.dev, pps->devno, NULL,
+	pps->dev = device_create(pps_class, pps->info.dev, devt, pps,
 							"pps%d", pps->id);
 	if (IS_ERR(pps->dev))
 		goto del_cdev;
-	dev_set_drvdata(pps->dev, pps);
 
 	pr_debug("source %s got cdev (%d:%d)\n", pps->info.name,
 			MAJOR(pps_devt), pps->id);
@@ -272,7 +262,7 @@ del_cdev:
 
 void pps_unregister_cdev(struct pps_device *pps)
 {
-	device_destroy(pps_class, pps->devno);
+	device_destroy(pps_class, pps->dev->devt);
 	cdev_del(&pps->cdev);
 }
 
diff --git a/include/linux/pps_kernel.h b/include/linux/pps_kernel.h
index 32aa676..1aedf50 100644
--- a/include/linux/pps_kernel.h
+++ b/include/linux/pps_kernel.h
@@ -31,13 +31,16 @@
  * Global defines
  */
 
+struct pps_device;
+
 /* The specific PPS source info */
 struct pps_source_info {
 	char name[PPS_MAX_NAME_LEN];		/* simbolic name */
 	char path[PPS_MAX_NAME_LEN];		/* path of connected device */
 	int mode;				/* PPS's allowed mode */
 
-	void (*echo)(int source, int event, void *data); /* PPS echo function */
+	void (*echo)(struct pps_device *pps,
+			int event, void *data);	/* PPS echo function */
 
 	struct module *owner;
 	struct device *dev;
@@ -65,35 +68,27 @@ struct pps_device {
 	unsigned int id;			/* PPS source unique ID */
 	struct cdev cdev;
 	struct device *dev;
-	int devno;
 	struct fasync_struct *async_queue;	/* fasync method */
 	spinlock_t lock;
-
-	atomic_t usage;				/* usage count */
 };
 
 /*
  * Global variables
  */
 
-extern spinlock_t pps_idr_lock;
-extern struct idr pps_idr;
-
 extern struct device_attribute pps_attrs[];
 
 /*
  * Exported functions
  */
 
-struct pps_device *pps_get_source(int source);
-extern void pps_put_source(struct pps_device *pps);
-extern int pps_register_source(struct pps_source_info *info,
-				int default_params);
-extern void pps_unregister_source(int source);
+extern struct pps_device *pps_register_source(
+		struct pps_source_info *info, int default_params);
+extern void pps_unregister_source(struct pps_device *pps);
 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_event_time *ts, int event,
-		void *data);
+extern void pps_event(struct pps_device *pps,
+		struct pps_event_time *ts, int event, void *data);
 
 static inline void timespec_to_pps_ktime(struct pps_ktime *kt,
 		struct timespec ts)
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCHv6 06/16] pps: convert printk/pr_* to dev_*
  2010-12-17 19:54 [PATCHv6 00/16] pps: several fixes and improvements Alexander Gordeev
                   ` (4 preceding siblings ...)
  2010-12-17 19:54 ` [PATCHv6 05/16] pps: access pps device by direct pointer Alexander Gordeev
@ 2010-12-17 19:54 ` Alexander Gordeev
  2010-12-17 19:54 ` [PATCHv6 07/16] pps: move idr stuff to pps.c Alexander Gordeev
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-17 19:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Nikita V. Youshchenko, linuxpps, Rodolfo Giometti, Andrew Morton,
	Alexander Gordeev

Since we now have direct pointers to struct pps_device everywhere it's
easy to use dev_* functions to print messages instead of plain printks.
Where dev_* cannot be used printks are converted to pr_*.

Acked-by: Rodolfo Giometti <giometti@linux.it>
Signed-off-by: Alexander Gordeev <lasaine@lvk.cs.msu.su>
---
 drivers/pps/clients/pps-ktimer.c |    5 +++--
 drivers/pps/clients/pps-ldisc.c  |    2 ++
 drivers/pps/kapi.c               |   15 ++++++++-------
 drivers/pps/pps.c                |   25 +++++++++++++------------
 4 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/drivers/pps/clients/pps-ktimer.c b/drivers/pps/clients/pps-ktimer.c
index 966ead1..2728469 100644
--- a/drivers/pps/clients/pps-ktimer.c
+++ b/drivers/pps/clients/pps-ktimer.c
@@ -19,6 +19,7 @@
  *   Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -45,7 +46,7 @@ static void pps_ktimer_event(unsigned long ptr)
 	/* First of all we get the time stamp... */
 	pps_get_ts(&ts);
 
-	pr_info("PPS event at %lu\n", jiffies);
+	dev_info(pps->dev, "PPS event at %lu\n", jiffies);
 
 	pps_event(pps, &ts, PPS_CAPTUREASSERT, NULL);
 
@@ -94,7 +95,7 @@ static int __init pps_ktimer_init(void)
 	pps = pps_register_source(&pps_ktimer_info,
 				PPS_CAPTUREASSERT | PPS_OFFSETASSERT);
 	if (pps == NULL) {
-		printk(KERN_ERR "cannot register ktimer source\n");
+		pr_err("cannot register PPS source\n");
 		return -ENOMEM;
 	}
 
diff --git a/drivers/pps/clients/pps-ldisc.c b/drivers/pps/clients/pps-ldisc.c
index 1517f54..1b3c6ed 100644
--- a/drivers/pps/clients/pps-ldisc.c
+++ b/drivers/pps/clients/pps-ldisc.c
@@ -19,6 +19,8 @@
  *   Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/module.h>
 #include <linux/serial_core.h>
 #include <linux/tty.h>
diff --git a/drivers/pps/kapi.c b/drivers/pps/kapi.c
index 98d4012..e8847c1 100644
--- a/drivers/pps/kapi.c
+++ b/drivers/pps/kapi.c
@@ -19,6 +19,7 @@
  *   Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -80,20 +81,20 @@ struct pps_device *pps_register_source(struct pps_source_info *info,
 
 	/* Sanity checks */
 	if ((info->mode & default_params) != default_params) {
-		printk(KERN_ERR "pps: %s: unsupported default parameters\n",
+		pr_err("%s: unsupported default parameters\n",
 					info->name);
 		err = -EINVAL;
 		goto pps_register_source_exit;
 	}
 	if ((info->mode & (PPS_ECHOASSERT | PPS_ECHOCLEAR)) != 0 &&
 			info->echo == NULL) {
-		printk(KERN_ERR "pps: %s: echo function is not defined\n",
+		pr_err("%s: echo function is not defined\n",
 					info->name);
 		err = -EINVAL;
 		goto pps_register_source_exit;
 	}
 	if ((info->mode & (PPS_TSFMT_TSPEC | PPS_TSFMT_NTPFP)) == 0) {
-		printk(KERN_ERR "pps: %s: unspecified time format\n",
+		pr_err("%s: unspecified time format\n",
 					info->name);
 		err = -EINVAL;
 		goto pps_register_source_exit;
@@ -138,7 +139,7 @@ struct pps_device *pps_register_source(struct pps_source_info *info,
 	if (id >= PPS_MAX_SOURCES) {
 		spin_unlock_irq(&pps_idr_lock);
 
-		printk(KERN_ERR "pps: %s: too many PPS sources in the system\n",
+		pr_err("%s: too many PPS sources in the system\n",
 					info->name);
 		err = -EBUSY;
 		goto free_idr;
@@ -150,12 +151,12 @@ struct pps_device *pps_register_source(struct pps_source_info *info,
 	/* Create the char device */
 	err = pps_register_cdev(pps);
 	if (err < 0) {
-		printk(KERN_ERR "pps: %s: unable to create char device\n",
+		pr_err("%s: unable to create char device\n",
 					info->name);
 		goto free_idr;
 	}
 
-	pr_info("new PPS source %s at ID %d\n", info->name, id);
+	dev_info(pps->dev, "new PPS source %s\n", info->name);
 
 	return pps;
 
@@ -168,7 +169,7 @@ kfree_pps:
 	kfree(pps);
 
 pps_register_source_exit:
-	printk(KERN_ERR "pps: %s: unable to register source\n", info->name);
+	pr_err("%s: unable to register source\n", info->name);
 
 	return NULL;
 }
diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index 1922f27..9f7c2e8 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -19,6 +19,7 @@
  *   Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -67,7 +68,7 @@ static long pps_cdev_ioctl(struct file *file,
 
 	switch (cmd) {
 	case PPS_GETPARAMS:
-		pr_debug("PPS_GETPARAMS: source %d\n", pps->id);
+		dev_dbg(pps->dev, "PPS_GETPARAMS\n");
 
 		spin_lock_irq(&pps->lock);
 
@@ -83,7 +84,7 @@ static long pps_cdev_ioctl(struct file *file,
 		break;
 
 	case PPS_SETPARAMS:
-		pr_debug("PPS_SETPARAMS: source %d\n", pps->id);
+		dev_dbg(pps->dev, "PPS_SETPARAMS\n");
 
 		/* Check the capabilities */
 		if (!capable(CAP_SYS_TIME))
@@ -93,14 +94,14 @@ static long pps_cdev_ioctl(struct file *file,
 		if (err)
 			return -EFAULT;
 		if (!(params.mode & (PPS_CAPTUREASSERT | PPS_CAPTURECLEAR))) {
-			pr_debug("capture mode unspecified (%x)\n",
+			dev_dbg(pps->dev, "capture mode unspecified (%x)\n",
 								params.mode);
 			return -EINVAL;
 		}
 
 		/* Check for supported capabilities */
 		if ((params.mode & ~pps->info.mode) != 0) {
-			pr_debug("unsupported capabilities (%x)\n",
+			dev_dbg(pps->dev, "unsupported capabilities (%x)\n",
 								params.mode);
 			return -EINVAL;
 		}
@@ -113,7 +114,7 @@ static long pps_cdev_ioctl(struct file *file,
 		/* Restore the read only parameters */
 		if ((params.mode & (PPS_TSFMT_TSPEC | PPS_TSFMT_NTPFP)) == 0) {
 			/* section 3.3 of RFC 2783 interpreted */
-			pr_debug("time format unspecified (%x)\n",
+			dev_dbg(pps->dev, "time format unspecified (%x)\n",
 								params.mode);
 			pps->params.mode |= PPS_TSFMT_TSPEC;
 		}
@@ -126,7 +127,7 @@ static long pps_cdev_ioctl(struct file *file,
 		break;
 
 	case PPS_GETCAP:
-		pr_debug("PPS_GETCAP: source %d\n", pps->id);
+		dev_dbg(pps->dev, "PPS_GETCAP\n");
 
 		err = put_user(pps->info.mode, iuarg);
 		if (err)
@@ -138,7 +139,7 @@ static long pps_cdev_ioctl(struct file *file,
 		struct pps_fdata fdata;
 		unsigned int ev;
 
-		pr_debug("PPS_FETCH: source %d\n", pps->id);
+		dev_dbg(pps->dev, "PPS_FETCH\n");
 
 		err = copy_from_user(&fdata, uarg, sizeof(struct pps_fdata));
 		if (err)
@@ -153,7 +154,7 @@ static long pps_cdev_ioctl(struct file *file,
 		else {
 			unsigned long ticks;
 
-			pr_debug("timeout %lld.%09d\n",
+			dev_dbg(pps->dev, "timeout %lld.%09d\n",
 					(long long) fdata.timeout.sec,
 					fdata.timeout.nsec);
 			ticks = fdata.timeout.sec * HZ;
@@ -171,7 +172,7 @@ static long pps_cdev_ioctl(struct file *file,
 
 		/* Check for pending signals */
 		if (err == -ERESTARTSYS) {
-			pr_debug("pending signal caught\n");
+			dev_dbg(pps->dev, "pending signal caught\n");
 			return -EINTR;
 		}
 
@@ -240,7 +241,7 @@ int pps_register_cdev(struct pps_device *pps)
 
 	err = cdev_add(&pps->cdev, devt, 1);
 	if (err) {
-		printk(KERN_ERR "pps: %s: failed to add char device %d:%d\n",
+		pr_err("%s: failed to add char device %d:%d\n",
 				pps->info.name, MAJOR(pps_devt), pps->id);
 		return err;
 	}
@@ -282,14 +283,14 @@ static int __init pps_init(void)
 
 	pps_class = class_create(THIS_MODULE, "pps");
 	if (!pps_class) {
-		printk(KERN_ERR "pps: failed to allocate class\n");
+		pr_err("failed to allocate class\n");
 		return -ENOMEM;
 	}
 	pps_class->dev_attrs = pps_attrs;
 
 	err = alloc_chrdev_region(&pps_devt, 0, PPS_MAX_SOURCES, "pps");
 	if (err < 0) {
-		printk(KERN_ERR "pps: failed to allocate char device region\n");
+		pr_err("failed to allocate char device region\n");
 		goto remove_class;
 	}
 
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCHv6 07/16] pps: move idr stuff to pps.c
  2010-12-17 19:54 [PATCHv6 00/16] pps: several fixes and improvements Alexander Gordeev
                   ` (5 preceding siblings ...)
  2010-12-17 19:54 ` [PATCHv6 06/16] pps: convert printk/pr_* to dev_* Alexander Gordeev
@ 2010-12-17 19:54 ` Alexander Gordeev
  2010-12-18  0:13   ` Andrew Morton
  2010-12-17 19:54 ` [PATCHv6 08/16] pps: do not disable interrupts for idr operations Alexander Gordeev
                   ` (10 subsequent siblings)
  17 siblings, 1 reply; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-17 19:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Nikita V. Youshchenko, linuxpps, Rodolfo Giometti, Andrew Morton,
	Alexander Gordeev

Since now idr is only used to manage char device id's and not used in
kernel API anymore it should be moved to pps.c. This also makes it
possible to release id only at actual device freeing so nobody can
register a pps device with the same id while our device is not freed
yet.

Acked-by: Rodolfo Giometti <giometti@linux.it>
Signed-off-by: Alexander Gordeev <lasaine@lvk.cs.msu.su>
---
 drivers/pps/kapi.c |   56 ++-------------------------------------------------
 drivers/pps/pps.c  |   50 +++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 52 insertions(+), 54 deletions(-)

diff --git a/drivers/pps/kapi.c b/drivers/pps/kapi.c
index e8847c1..c42d3cb 100644
--- a/drivers/pps/kapi.c
+++ b/drivers/pps/kapi.c
@@ -27,19 +27,11 @@
 #include <linux/sched.h>
 #include <linux/time.h>
 #include <linux/spinlock.h>
-#include <linux/idr.h>
 #include <linux/fs.h>
 #include <linux/pps_kernel.h>
 #include <linux/slab.h>
 
 /*
- * Local variables
- */
-
-static DEFINE_SPINLOCK(pps_idr_lock);
-static DEFINE_IDR(pps_idr);
-
-/*
  * Local functions
  */
 
@@ -76,7 +68,6 @@ struct pps_device *pps_register_source(struct pps_source_info *info,
 		int default_params)
 {
 	struct pps_device *pps;
-	int id;
 	int err;
 
 	/* Sanity checks */
@@ -117,54 +108,18 @@ struct pps_device *pps_register_source(struct pps_source_info *info,
 	init_waitqueue_head(&pps->queue);
 	spin_lock_init(&pps->lock);
 
-	/* Get new ID for the new PPS source */
-	if (idr_pre_get(&pps_idr, GFP_KERNEL) == 0) {
-		err = -ENOMEM;
-		goto kfree_pps;
-	}
-
-	spin_lock_irq(&pps_idr_lock);
-
-	/* Now really allocate the PPS source.
-	 * After idr_get_new() calling the new source will be freely available
-	 * into the kernel.
-	 */
-	err = idr_get_new(&pps_idr, pps, &id);
-	if (err < 0) {
-		spin_unlock_irq(&pps_idr_lock);
-		goto kfree_pps;
-	}
-
-	id = id & MAX_ID_MASK;
-	if (id >= PPS_MAX_SOURCES) {
-		spin_unlock_irq(&pps_idr_lock);
-
-		pr_err("%s: too many PPS sources in the system\n",
-					info->name);
-		err = -EBUSY;
-		goto free_idr;
-	}
-	pps->id = id;
-
-	spin_unlock_irq(&pps_idr_lock);
-
 	/* Create the char device */
 	err = pps_register_cdev(pps);
 	if (err < 0) {
 		pr_err("%s: unable to create char device\n",
 					info->name);
-		goto free_idr;
+		goto kfree_pps;
 	}
 
 	dev_info(pps->dev, "new PPS source %s\n", info->name);
 
 	return pps;
 
-free_idr:
-	spin_lock_irq(&pps_idr_lock);
-	idr_remove(&pps_idr, id);
-	spin_unlock_irq(&pps_idr_lock);
-
 kfree_pps:
 	kfree(pps);
 
@@ -184,15 +139,10 @@ EXPORT_SYMBOL(pps_register_source);
 
 void pps_unregister_source(struct pps_device *pps)
 {
-	unsigned int id = pps->id;
-
 	pps_unregister_cdev(pps);
 
-	spin_lock_irq(&pps_idr_lock);
-	idr_remove(&pps_idr, pps->id);
-	spin_unlock_irq(&pps_idr_lock);
-
-	kfree(pps);
+	/* don't have to kfree(pps) here because it will be done on
+	 * device destruction */
 }
 EXPORT_SYMBOL(pps_unregister_source);
 
diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index 9f7c2e8..79b4455 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -30,6 +30,7 @@
 #include <linux/cdev.h>
 #include <linux/poll.h>
 #include <linux/pps_kernel.h>
+#include <linux/slab.h>
 
 /*
  * Local variables
@@ -38,6 +39,9 @@
 static dev_t pps_devt;
 static struct class *pps_class;
 
+static DEFINE_SPINLOCK(pps_idr_lock);
+static DEFINE_IDR(pps_idr);
+
 /*
  * Char device methods
  */
@@ -229,11 +233,48 @@ static const struct file_operations pps_cdev_fops = {
 	.release	= pps_cdev_release,
 };
 
+static void pps_device_destruct(struct device *dev)
+{
+	struct pps_device *pps = dev_get_drvdata(dev);
+
+	/* release id here to protect others from using it while it's
+	 * still in use */
+	spin_lock_irq(&pps_idr_lock);
+	idr_remove(&pps_idr, pps->id);
+	spin_unlock_irq(&pps_idr_lock);
+
+	kfree(dev);
+	kfree(pps);
+}
+
 int pps_register_cdev(struct pps_device *pps)
 {
 	int err;
 	dev_t devt;
 
+	/* Get new ID for the new PPS source */
+	if (idr_pre_get(&pps_idr, GFP_KERNEL) == 0)
+		return -ENOMEM;
+
+	/* Now really allocate the PPS source.
+	 * After idr_get_new() calling the new source will be freely available
+	 * into the kernel.
+	 */
+	spin_lock_irq(&pps_idr_lock);
+	err = idr_get_new(&pps_idr, pps, &pps->id);
+	spin_unlock_irq(&pps_idr_lock);
+
+	if (err < 0)
+		return err;
+
+	pps->id &= MAX_ID_MASK;
+	if (pps->id >= PPS_MAX_SOURCES) {
+		pr_err("%s: too many PPS sources in the system\n",
+					pps->info.name);
+		err = -EBUSY;
+		goto free_idr;
+	}
+
 	devt = MKDEV(MAJOR(pps_devt), pps->id);
 
 	cdev_init(&pps->cdev, &pps_cdev_fops);
@@ -243,13 +284,15 @@ int pps_register_cdev(struct pps_device *pps)
 	if (err) {
 		pr_err("%s: failed to add char device %d:%d\n",
 				pps->info.name, MAJOR(pps_devt), pps->id);
-		return err;
+		goto free_idr;
 	}
 	pps->dev = device_create(pps_class, pps->info.dev, devt, pps,
 							"pps%d", pps->id);
 	if (IS_ERR(pps->dev))
 		goto del_cdev;
 
+	pps->dev->release = pps_device_destruct;
+
 	pr_debug("source %s got cdev (%d:%d)\n", pps->info.name,
 			MAJOR(pps_devt), pps->id);
 
@@ -258,6 +301,11 @@ int pps_register_cdev(struct pps_device *pps)
 del_cdev:
 	cdev_del(&pps->cdev);
 
+free_idr:
+	spin_lock_irq(&pps_idr_lock);
+	idr_remove(&pps_idr, pps->id);
+	spin_unlock_irq(&pps_idr_lock);
+
 	return err;
 }
 
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCHv6 08/16] pps: do not disable interrupts for idr operations
  2010-12-17 19:54 [PATCHv6 00/16] pps: several fixes and improvements Alexander Gordeev
                   ` (6 preceding siblings ...)
  2010-12-17 19:54 ` [PATCHv6 07/16] pps: move idr stuff to pps.c Alexander Gordeev
@ 2010-12-17 19:54 ` Alexander Gordeev
  2010-12-17 19:54 ` [PATCHv6 09/16] pps: use BUG_ON for kernel API safety checks Alexander Gordeev
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-17 19:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Nikita V. Youshchenko, linuxpps, Rodolfo Giometti, Andrew Morton,
	Alexander Gordeev

Now pps_idr_lock is never used in interrupt context so replace
spin_lock_irq/spin_unlock_irq with plain spin_lock/spin_unlock.

Acked-by: Rodolfo Giometti <giometti@linux.it>
Signed-off-by: Alexander Gordeev <lasaine@lvk.cs.msu.su>
---
 drivers/pps/pps.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index 79b4455..54964a5 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -239,9 +239,9 @@ static void pps_device_destruct(struct device *dev)
 
 	/* release id here to protect others from using it while it's
 	 * still in use */
-	spin_lock_irq(&pps_idr_lock);
+	spin_lock(&pps_idr_lock);
 	idr_remove(&pps_idr, pps->id);
-	spin_unlock_irq(&pps_idr_lock);
+	spin_unlock(&pps_idr_lock);
 
 	kfree(dev);
 	kfree(pps);
@@ -260,9 +260,9 @@ int pps_register_cdev(struct pps_device *pps)
 	 * After idr_get_new() calling the new source will be freely available
 	 * into the kernel.
 	 */
-	spin_lock_irq(&pps_idr_lock);
+	spin_lock(&pps_idr_lock);
 	err = idr_get_new(&pps_idr, pps, &pps->id);
-	spin_unlock_irq(&pps_idr_lock);
+	spin_unlock(&pps_idr_lock);
 
 	if (err < 0)
 		return err;
@@ -302,9 +302,9 @@ del_cdev:
 	cdev_del(&pps->cdev);
 
 free_idr:
-	spin_lock_irq(&pps_idr_lock);
+	spin_lock(&pps_idr_lock);
 	idr_remove(&pps_idr, pps->id);
-	spin_unlock_irq(&pps_idr_lock);
+	spin_unlock(&pps_idr_lock);
 
 	return err;
 }
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCHv6 09/16] pps: use BUG_ON for kernel API safety checks
  2010-12-17 19:54 [PATCHv6 00/16] pps: several fixes and improvements Alexander Gordeev
                   ` (7 preceding siblings ...)
  2010-12-17 19:54 ` [PATCHv6 08/16] pps: do not disable interrupts for idr operations Alexander Gordeev
@ 2010-12-17 19:54 ` Alexander Gordeev
  2010-12-17 19:54 ` [PATCHv6 10/16] pps: simplify conditions a bit Alexander Gordeev
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-17 19:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Nikita V. Youshchenko, linuxpps, Rodolfo Giometti, Andrew Morton,
	Alexander Gordeev

This way less overhead is involved when running production kernel.
If you want to debug a pps client module please define DEBUG to enable
the checks.

Acked-by: Rodolfo Giometti <giometti@linux.it>
Signed-off-by: Alexander Gordeev <lasaine@lvk.cs.msu.su>
---
 drivers/pps/kapi.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/pps/kapi.c b/drivers/pps/kapi.c
index c42d3cb..3e8eb3f 100644
--- a/drivers/pps/kapi.c
+++ b/drivers/pps/kapi.c
@@ -166,10 +166,8 @@ void pps_event(struct pps_device *pps, struct pps_event_time *ts, int event,
 	int captured = 0;
 	struct pps_ktime ts_real;
 
-	if ((event & (PPS_CAPTUREASSERT | PPS_CAPTURECLEAR)) == 0) {
-		dev_err(pps->dev, "unknown event (%x)\n", event);
-		return;
-	}
+	/* check event type */
+	BUG_ON((event & (PPS_CAPTUREASSERT | PPS_CAPTURECLEAR)) == 0);
 
 	dev_dbg(pps->dev, "PPS event at %ld.%09ld\n",
 			ts->ts_real.tv_sec, ts->ts_real.tv_nsec);
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCHv6 10/16] pps: simplify conditions a bit
  2010-12-17 19:54 [PATCHv6 00/16] pps: several fixes and improvements Alexander Gordeev
                   ` (8 preceding siblings ...)
  2010-12-17 19:54 ` [PATCHv6 09/16] pps: use BUG_ON for kernel API safety checks Alexander Gordeev
@ 2010-12-17 19:54 ` Alexander Gordeev
  2010-12-17 19:54 ` [PATCHv6 11/16] pps: timestamp is always passed to dcd_change() Alexander Gordeev
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-17 19:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Nikita V. Youshchenko, linuxpps, Rodolfo Giometti, Andrew Morton,
	Alexander Gordeev

Bitwise conjunction is distributive so we can simplify some conditions.

Acked-by: Rodolfo Giometti <giometti@linux.it>
Signed-off-by: Alexander Gordeev <lasaine@lvk.cs.msu.su>
---
 drivers/pps/kapi.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/pps/kapi.c b/drivers/pps/kapi.c
index 3e8eb3f..ebf3374 100644
--- a/drivers/pps/kapi.c
+++ b/drivers/pps/kapi.c
@@ -182,8 +182,7 @@ void pps_event(struct pps_device *pps, struct pps_event_time *ts, int event,
 
 	/* Check the event */
 	pps->current_mode = pps->params.mode;
-	if ((event & PPS_CAPTUREASSERT) &
-			(pps->params.mode & PPS_CAPTUREASSERT)) {
+	if (event & pps->params.mode & PPS_CAPTUREASSERT) {
 		/* We have to add an offset? */
 		if (pps->params.mode & PPS_OFFSETASSERT)
 			pps_add_offset(&ts_real,
@@ -197,8 +196,7 @@ void pps_event(struct pps_device *pps, struct pps_event_time *ts, int event,
 
 		captured = ~0;
 	}
-	if ((event & PPS_CAPTURECLEAR) &
-			(pps->params.mode & PPS_CAPTURECLEAR)) {
+	if (event & pps->params.mode & PPS_CAPTURECLEAR) {
 		/* We have to add an offset? */
 		if (pps->params.mode & PPS_OFFSETCLEAR)
 			pps_add_offset(&ts_real,
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCHv6 11/16] pps: timestamp is always passed to dcd_change()
  2010-12-17 19:54 [PATCHv6 00/16] pps: several fixes and improvements Alexander Gordeev
                   ` (9 preceding siblings ...)
  2010-12-17 19:54 ` [PATCHv6 10/16] pps: simplify conditions a bit Alexander Gordeev
@ 2010-12-17 19:54 ` Alexander Gordeev
  2010-12-17 19:54 ` [PATCHv6 12/16] ntp: add hardpps implementation Alexander Gordeev
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-17 19:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Nikita V. Youshchenko, linuxpps, Rodolfo Giometti, Andrew Morton,
	Alexander Gordeev, Randy Dunlap, linux-doc

Remove the code that gatheres timestamp in pps_tty_dcd_change() in case
passed ts parameter is NULL because it never happens in the current
code. Fix comments as well.

Acked-by: Rodolfo Giometti <giometti@linux.it>
Signed-off-by: Alexander Gordeev <lasaine@lvk.cs.msu.su>
---
 Documentation/serial/tty.txt    |    2 +-
 drivers/pps/clients/pps-ldisc.c |    8 --------
 include/linux/tty_ldisc.h       |    2 +-
 3 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/Documentation/serial/tty.txt b/Documentation/serial/tty.txt
index 7c90050..540db41 100644
--- a/Documentation/serial/tty.txt
+++ b/Documentation/serial/tty.txt
@@ -107,7 +107,7 @@ write_wakeup()	-	May be called at any point between open and close.
 
 dcd_change()	-	Report to the tty line the current DCD pin status
 			changes and the relative timestamp. The timestamp
-			can be NULL.
+			cannot be NULL.
 
 
 Driver Access
diff --git a/drivers/pps/clients/pps-ldisc.c b/drivers/pps/clients/pps-ldisc.c
index 1b3c6ed..79451f2 100644
--- a/drivers/pps/clients/pps-ldisc.c
+++ b/drivers/pps/clients/pps-ldisc.c
@@ -32,14 +32,6 @@ static void pps_tty_dcd_change(struct tty_struct *tty, unsigned int status,
 				struct pps_event_time *ts)
 {
 	struct pps_device *pps = (struct pps_device *)tty->disc_data;
-	struct pps_event_time __ts;
-
-	/* First of all we get the time stamp... */
-	pps_get_ts(&__ts);
-
-	/* Does caller give us a timestamp? */
-	if (!ts)	/* No. Do it ourself! */
-		ts = &__ts;
 
 	BUG_ON(pps == NULL);
 
diff --git a/include/linux/tty_ldisc.h b/include/linux/tty_ldisc.h
index 763e061..ff7dc08 100644
--- a/include/linux/tty_ldisc.h
+++ b/include/linux/tty_ldisc.h
@@ -104,7 +104,7 @@
  *			struct pps_event_time *ts)
  *
  *	Tells the discipline that the DCD pin has changed its status and
- *	the relative timestamp. Pointer ts can be NULL.
+ *	the relative timestamp. Pointer ts cannot be NULL.
  */
 
 #include <linux/fs.h>
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCHv6 12/16] ntp: add hardpps implementation
  2010-12-17 19:54 [PATCHv6 00/16] pps: several fixes and improvements Alexander Gordeev
                   ` (10 preceding siblings ...)
  2010-12-17 19:54 ` [PATCHv6 11/16] pps: timestamp is always passed to dcd_change() Alexander Gordeev
@ 2010-12-17 19:54 ` Alexander Gordeev
  2010-12-17 19:54 ` [PATCHv6 13/16] pps: capture MONOTONIC_RAW timestamps as well Alexander Gordeev
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-17 19:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Nikita V. Youshchenko, linuxpps, Rodolfo Giometti, Andrew Morton,
	Alexander Gordeev, John Stultz, Thomas Gleixner,
	Miroslav Lichvar

This commit adds hardpps() implementation based upon the original one
from the NTPv4 reference kernel code from David Mills. 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.

Acked-by: John Stultz <johnstul@us.ibm.com>
Signed-off-by: Alexander Gordeev <lasaine@lvk.cs.msu.su>
---
 drivers/pps/Kconfig   |    7 +
 include/linux/timex.h |    1 +
 kernel/time/ntp.c     |  425 +++++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 418 insertions(+), 15 deletions(-)

diff --git a/drivers/pps/Kconfig b/drivers/pps/Kconfig
index 1afe4e0..33c9126 100644
--- a/drivers/pps/Kconfig
+++ b/drivers/pps/Kconfig
@@ -30,6 +30,13 @@ 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.
 
+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.
+
 source drivers/pps/clients/Kconfig
 
 endmenu
diff --git a/include/linux/timex.h b/include/linux/timex.h
index 32d852f..d23999f 100644
--- a/include/linux/timex.h
+++ b/include/linux/timex.h
@@ -268,6 +268,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 *, const struct timespec *);
 
 int read_current_timer(unsigned long *timer_val);
 
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index d232189..5c00242 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,162 @@ static 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. They establish the engineering parameters of the clock
+ * discipline loop when controlled by the PPS signal.
+ */
+#define PPS_VALID	10	/* PPS signal watchdog max (s) */
+#define PPS_POPCORN	4	/* popcorn spike threshold (shift) */
+#define PPS_INTMIN	2	/* min freq interval (s) (shift) */
+#define PPS_INTMAX	8	/* max freq interval (s) (shift) */
+#define PPS_INTCOUNT	4	/* number of consecutive good intervals to
+				   increase pps_shift or consecutive bad
+				   intervals to decrease it */
+#define PPS_MAXWANDER	100000	/* max PPS freq wander (ns/s) */
+
+static int pps_valid;		/* signal watchdog counter */
+static long pps_tf[3];		/* phase median filter */
+static long pps_jitter;		/* current jitter (ns) */
+static struct timespec pps_fbase; /* beginning of the last freq interval */
+static int pps_shift;		/* current interval duration (s) (shift) */
+static int pps_intcnt;		/* interval counter */
+static s64 pps_freq;		/* frequency offset (scaled ns/s) */
+static long pps_stabil;		/* current stability (scaled ns/s) */
+
+/*
+ * PPS signal quality monitors
+ */
+static long pps_calcnt;		/* calibration intervals */
+static long pps_jitcnt;		/* jitter limit exceeded */
+static long pps_stbcnt;		/* stability limit exceeded */
+static long pps_errcnt;		/* calibration errors */
+
+
+/* PPS kernel consumer compensates the whole phase error immediately.
+ * Otherwise, reduce the offset by a fixed factor times the time constant.
+ */
+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);
+}
+
+static inline void pps_reset_freq_interval(void)
+{
+	/* the PPS calibration interval may end
+	   surprisingly early */
+	pps_shift = PPS_INTMIN;
+	pps_intcnt = 0;
+}
+
+/**
+ * pps_clear - Clears the PPS state variables
+ *
+ * Must be called while holding a write on the xtime_lock
+ */
+static inline void pps_clear(void)
+{
+	pps_reset_freq_interval();
+	pps_tf[0] = 0;
+	pps_tf[1] = 0;
+	pps_tf[2] = 0;
+	pps_fbase.tv_sec = pps_fbase.tv_nsec = 0;
+	pps_freq = 0;
+}
+
+/* Decrease pps_valid to indicate that another second has passed since
+ * the last PPS signal. When it reaches 0, indicate that PPS signal is
+ * missing.
+ *
+ * Must be called while holding a write on the xtime_lock
+ */
+static inline void pps_dec_valid(void)
+{
+	if (pps_valid > 0)
+		pps_valid--;
+	else {
+		time_status &= ~(STA_PPSSIGNAL | STA_PPSJITTER |
+				 STA_PPSWANDER | STA_PPSERROR);
+		pps_clear();
+	}
+}
+
+static inline void pps_set_freq(s64 freq)
+{
+	pps_freq = freq;
+}
+
+static inline int is_error_status(int status)
+{
+	return (time_status & (STA_UNSYNC|STA_CLOCKERR))
+		/* 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)));
+}
+
+static inline void pps_fill_timex(struct timex *txc)
+{
+	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 */
+
+static inline s64 ntp_offset_chunk(s64 offset)
+{
+	return shift_right(offset, SHIFT_PLL + time_constant);
+}
+
+static inline void pps_reset_freq_interval(void) {}
+static inline void pps_clear(void) {}
+static inline void pps_dec_valid(void) {}
+static inline void pps_set_freq(s64 freq) {}
+
+static inline int is_error_status(int status)
+{
+	return status & (STA_UNSYNC|STA_CLOCKERR);
+}
+
+static inline void pps_fill_timex(struct timex *txc)
+{
+	/* PPS is not implemented, so these are zero */
+	txc->ppsfreq	   = 0;
+	txc->jitter	   = 0;
+	txc->shift	   = 0;
+	txc->stabil	   = 0;
+	txc->jitcnt	   = 0;
+	txc->calcnt	   = 0;
+	txc->errcnt	   = 0;
+	txc->stbcnt	   = 0;
+}
+
+#endif /* CONFIG_NTP_PPS */
+
 /*
  * NTP methods:
  */
@@ -185,6 +342,9 @@ void ntp_clear(void)
 
 	tick_length	= tick_length_base;
 	time_offset	= 0;
+
+	/* Clear PPS state variables */
+	pps_clear();
 }
 
 /*
@@ -250,16 +410,16 @@ void second_overflow(void)
 		time_status |= STA_UNSYNC;
 	}
 
-	/*
-	 * Compute the phase adjustment for the next second. The offset is
-	 * reduced by a fixed factor times the time constant.
-	 */
+	/* Compute the phase adjustment for the next second */
 	tick_length	 = tick_length_base;
 
-	delta		 = shift_right(time_offset, SHIFT_PLL + time_constant);
+	delta		 = ntp_offset_chunk(time_offset);
 	time_offset	-= delta;
 	tick_length	+= delta;
 
+	/* Check PPS signal */
+	pps_dec_valid();
+
 	if (!time_adjust)
 		return;
 
@@ -369,6 +529,8 @@ 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;
+		/* restart PPS frequency calibration */
+		pps_reset_freq_interval();
 	}
 
 	/*
@@ -418,6 +580,8 @@ 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);
+		/* update pps_freq */
+		pps_set_freq(time_freq);
 	}
 
 	if (txc->modes & ADJ_MAXERROR)
@@ -508,7 +672,8 @@ int do_adjtimex(struct timex *txc)
 	}
 
 	result = time_state;	/* mostly `TIME_OK' */
-	if (time_status & (STA_UNSYNC|STA_CLOCKERR))
+	/* check for errors */
+	if (is_error_status(time_status))
 		result = TIME_ERROR;
 
 	txc->freq	   = shift_right((time_freq >> PPM_SCALE_INV_SHIFT) *
@@ -522,15 +687,8 @@ int do_adjtimex(struct timex *txc)
 	txc->tick	   = tick_usec;
 	txc->tai	   = time_tai;
 
-	/* PPS is not implemented, so these are zero */
-	txc->ppsfreq	   = 0;
-	txc->jitter	   = 0;
-	txc->shift	   = 0;
-	txc->stabil	   = 0;
-	txc->jitcnt	   = 0;
-	txc->calcnt	   = 0;
-	txc->errcnt	   = 0;
-	txc->stbcnt	   = 0;
+	/* fill PPS status fields */
+	pps_fill_timex(txc);
 
 	write_sequnlock_irq(&xtime_lock);
 
@@ -544,6 +702,243 @@ int do_adjtimex(struct timex *txc)
 	return result;
 }
 
+#ifdef	CONFIG_NTP_PPS
+
+/* actually struct pps_normtime is good old struct timespec, but it is
+ * semantically different (and it is the reason why it was invented):
+ * pps_normtime.nsec has a range of ( -NSEC_PER_SEC / 2, NSEC_PER_SEC / 2 ]
+ * while timespec.tv_nsec has a range of [0, NSEC_PER_SEC) */
+struct pps_normtime {
+	__kernel_time_t	sec;	/* seconds */
+	long		nsec;	/* nanoseconds */
+};
+
+/* normalize the timestamp so that nsec is in the
+   ( -NSEC_PER_SEC / 2, NSEC_PER_SEC / 2 ] interval */
+static inline struct pps_normtime pps_normalize_ts(struct timespec ts)
+{
+	struct pps_normtime norm = {
+		.sec = ts.tv_sec,
+		.nsec = ts.tv_nsec
+	};
+
+	if (norm.nsec > (NSEC_PER_SEC >> 1)) {
+		norm.nsec -= NSEC_PER_SEC;
+		norm.sec++;
+	}
+
+	return norm;
+}
+
+/* get current phase correction and jitter */
+static inline long pps_phase_filter_get(long *jitter)
+{
+	*jitter = pps_tf[0] - pps_tf[1];
+	if (*jitter < 0)
+		*jitter = -*jitter;
+
+	/* TODO: test various filters */
+	return pps_tf[0];
+}
+
+/* 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.
+ * It is halved after four consecutive unstable intervals.
+ */
+static inline void pps_dec_freq_interval(void)
+{
+	if (--pps_intcnt <= -PPS_INTCOUNT) {
+		pps_intcnt = -PPS_INTCOUNT;
+		if (pps_shift > PPS_INTMIN) {
+			pps_shift--;
+			pps_intcnt = 0;
+		}
+	}
+}
+
+/* increase frequency calibration interval length.
+ * It is doubled after four consecutive stable intervals.
+ */
+static inline void pps_inc_freq_interval(void)
+{
+	if (++pps_intcnt >= PPS_INTCOUNT) {
+		pps_intcnt = PPS_INTCOUNT;
+		if (pps_shift < PPS_INTMAX) {
+			pps_shift++;
+			pps_intcnt = 0;
+		}
+	}
+}
+
+/* update clock frequency based on MONOTONIC_RAW clock PPS signal
+ * timestamps
+ *
+ * At the end of the calibration interval the difference between the
+ * first and last MONOTONIC_RAW clock timestamps divided by the length
+ * of the interval becomes the frequency update. If the interval was
+ * too long, the data are discarded.
+ * Returns the difference between old and new frequency values.
+ */
+static long hardpps_update_freq(struct pps_normtime freq_norm)
+{
+	long delta, delta_mod;
+	s64 ftemp;
+
+	/* check if the frequency interval was too long */
+	if (freq_norm.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.sec);
+		return 0;
+	}
+
+	/* here the raw frequency offset and wander (stability) is
+	 * calculated. If the wander is less than the wander threshold
+	 * the interval is increased; otherwise it is decreased.
+	 */
+	ftemp = div_s64(((s64)(-freq_norm.nsec)) << NTP_SCALE_SHIFT,
+			freq_norm.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 */
+		pps_inc_freq_interval();
+	}
+
+	/* the stability metric is calculated as the average of recent
+	 * frequency changes, but is used only for performance
+	 * monitoring
+	 */
+	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_INTMIN;
+
+	/* 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 REALTIME clock 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);
+	correction = pps_phase_filter_get(&jitter);
+
+	/* Nominal jitter is due to PPS signal noise. If it exceeds the
+	 * threshold, the sample is discarded; otherwise, if so enabled,
+	 * the time offset is updated.
+	 */
+	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_INTMIN;
+}
+
+/*
+ * hardpps() - discipline CPU clock oscillator to external PPS signal
+ *
+ * This routine is called at each PPS signal arrival in order to
+ * discipline the CPU clock oscillator to the PPS signal. It takes two
+ * parameters: REALTIME and MONOTONIC_RAW clock timestamps. The former
+ * is used to correct clock phase error and the latter is used to
+ * correct the frequency.
+ *
+ * This code is based on David Mills's reference nanokernel
+ * implementation. It was mostly rewritten but keeps the same idea.
+ */
+void hardpps(const struct timespec *phase_ts, const struct timespec *raw_ts)
+{
+	struct pps_normtime pts_norm, freq_norm;
+	unsigned long flags;
+
+	pts_norm = pps_normalize_ts(*phase_ts);
+
+	write_seqlock_irqsave(&xtime_lock, flags);
+
+	/* 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;
+
+	/* when called for the first time,
+	 * just start the frequency interval */
+	if (unlikely(pps_fbase.tv_sec == 0)) {
+		pps_fbase = *raw_ts;
+		write_sequnlock_irqrestore(&xtime_lock, flags);
+		return;
+	}
+
+	/* ok, now we have a base for frequency calculation */
+	freq_norm = pps_normalize_ts(timespec_sub(*raw_ts, pps_fbase));
+
+	/* check that the signal is in the range
+	 * [1s - MAXFREQ us, 1s + MAXFREQ us], otherwise reject it */
+	if ((freq_norm.sec == 0) ||
+			(freq_norm.nsec > MAXFREQ * freq_norm.sec) ||
+			(freq_norm.nsec < -MAXFREQ * freq_norm.sec)) {
+		time_status |= STA_PPSJITTER;
+		/* restart the frequency calibration interval */
+		pps_fbase = *raw_ts;
+		write_sequnlock_irqrestore(&xtime_lock, flags);
+		pr_err("hardpps: PPSJITTER: bad pulse\n");
+		return;
+	}
+
+	/* signal is ok */
+
+	/* check if the current frequency interval is finished */
+	if (freq_norm.sec >= (1 << pps_shift)) {
+		pps_calcnt++;
+		/* restart the frequency calibration interval */
+		pps_fbase = *raw_ts;
+		hardpps_update_freq(freq_norm);
+	}
+
+	hardpps_update_phase(pts_norm.nsec);
+
+	write_sequnlock_irqrestore(&xtime_lock, flags);
+}
+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.7.2.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCHv6 13/16] pps: capture MONOTONIC_RAW timestamps as well
  2010-12-17 19:54 [PATCHv6 00/16] pps: several fixes and improvements Alexander Gordeev
                   ` (11 preceding siblings ...)
  2010-12-17 19:54 ` [PATCHv6 12/16] ntp: add hardpps implementation Alexander Gordeev
@ 2010-12-17 19:54 ` Alexander Gordeev
  2010-12-17 19:54 ` [PATCHv6 14/16] pps: add kernel consumer support Alexander Gordeev
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-17 19:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Nikita V. Youshchenko, linuxpps, Rodolfo Giometti, Andrew Morton,
	Alexander Gordeev, John Stultz, Thomas Gleixner, David Howells,
	H. Peter Anvin, Magnus Damm, Jason Wessel

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.

Acked-by: John Stultz <johnstul@us.ibm.com>
Signed-off-by: Alexander Gordeev <lasaine@lvk.cs.msu.su>
---
 include/linux/pps_kernel.h |    3 ++-
 include/linux/time.h       |    2 ++
 kernel/time/timekeeping.c  |   39 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 43 insertions(+), 1 deletions(-)

diff --git a/include/linux/pps_kernel.h b/include/linux/pps_kernel.h
index 1aedf50..6ae8898 100644
--- a/include/linux/pps_kernel.h
+++ b/include/linux/pps_kernel.h
@@ -47,6 +47,7 @@ struct pps_source_info {
 };
 
 struct pps_event_time {
+	struct timespec ts_raw;
 	struct timespec ts_real;
 };
 
@@ -99,7 +100,7 @@ static inline void timespec_to_pps_ktime(struct pps_ktime *kt,
 
 static inline void pps_get_ts(struct pps_event_time *ts)
 {
-	getnstimeofday(&ts->ts_real);
+	getnstime_raw_and_real(&ts->ts_raw, &ts->ts_real);
 }
 
 #endif /* LINUX_PPS_KERNEL_H */
diff --git a/include/linux/time.h b/include/linux/time.h
index 9f15ac7..1e6d3b5 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -158,6 +158,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 49010d8..f42af9d 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -285,6 +285,45 @@ void ktime_get_ts(struct timespec *ts)
 EXPORT_SYMBOL_GPL(ktime_get_ts);
 
 /**
+ * getnstime_raw_and_real - get day and raw monotonic time in timespec format
+ * @ts_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
+ *
+ * This function reads both the time of day and raw monotonic time at the
+ * same time atomically and stores the resulting timestamps in timespec
+ * format.
+ */
+void getnstime_raw_and_real(struct timespec *ts_raw, struct timespec *ts_real)
+{
+	unsigned long seq;
+	s64 nsecs_raw, nsecs_real;
+
+	WARN_ON_ONCE(timekeeping_suspended);
+
+	do {
+		u32 arch_offset;
+
+		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() */
+		arch_offset = arch_gettimeoffset();
+		nsecs_raw += arch_offset;
+		nsecs_real += arch_offset;
+
+	} 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.7.2.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCHv6 14/16] pps: add kernel consumer support
  2010-12-17 19:54 [PATCHv6 00/16] pps: several fixes and improvements Alexander Gordeev
                   ` (12 preceding siblings ...)
  2010-12-17 19:54 ` [PATCHv6 13/16] pps: capture MONOTONIC_RAW timestamps as well Alexander Gordeev
@ 2010-12-17 19:54 ` Alexander Gordeev
  2010-12-17 19:54 ` [PATCHv6 15/16] pps: add parallel port PPS client Alexander Gordeev
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-17 19:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Nikita V. Youshchenko, linuxpps, Rodolfo Giometti, Andrew Morton,
	Alexander Gordeev, Randy Dunlap, Greg Kroah-Hartman,
	Thomas Weber, Mike Frysinger, Stefan Richter, linux-doc

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

Acked-by: Rodolfo Giometti <giometti@linux.it>
Signed-off-by: Alexander Gordeev <lasaine@lvk.cs.msu.su>
---
 Documentation/ioctl/ioctl-number.txt |    2 +-
 drivers/pps/Kconfig                  |    1 +
 drivers/pps/kapi.c                   |   27 +++++++++++++++
 drivers/pps/pps.c                    |   62 +++++++++++++++++++++++++++++++++-
 include/linux/pps.h                  |    7 ++++
 include/linux/pps_kernel.h           |    6 +++
 6 files changed, 103 insertions(+), 2 deletions(-)

diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
index 63ffd78..7081847 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -249,7 +249,7 @@ Code  Seq#(hex)	Include File		Comments
 'p'	40-7F	linux/nvram.h
 'p'	80-9F	linux/ppdev.h		user-space parport
 					<mailto:tim@cyberelk.net>
-'p'	A1-A4	linux/pps.h		LinuxPPS
+'p'	A1-A5	linux/pps.h		LinuxPPS
 					<mailto:giometti@linux.it>
 'q'	00-1F	linux/serio.h
 'q'	80-FF	linux/telephony.h	Internet PhoneJACK, Internet LineJACK
diff --git a/drivers/pps/Kconfig b/drivers/pps/Kconfig
index 33c9126..4823e47 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/drivers/pps/kapi.c b/drivers/pps/kapi.c
index ebf3374..ed78f6d 100644
--- a/drivers/pps/kapi.c
+++ b/drivers/pps/kapi.c
@@ -26,12 +26,23 @@
 #include <linux/init.h>
 #include <linux/sched.h>
 #include <linux/time.h>
+#include <linux/timex.h>
 #include <linux/spinlock.h>
 #include <linux/fs.h>
 #include <linux/pps_kernel.h>
 #include <linux/slab.h>
 
 /*
+ * Global variables
+ */
+
+/* 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
  */
 
@@ -139,6 +150,16 @@ EXPORT_SYMBOL(pps_register_source);
 
 void pps_unregister_source(struct pps_device *pps)
 {
+	spin_lock_irq(&pps_kc_hardpps_lock);
+	if (pps == pps_kc_hardpps_dev) {
+		pps_kc_hardpps_mode = 0;
+		pps_kc_hardpps_dev = NULL;
+		spin_unlock_irq(&pps_kc_hardpps_lock);
+		dev_info(pps->dev, "unbound kernel consumer"
+				" on device removal\n");
+	} else
+		spin_unlock_irq(&pps_kc_hardpps_lock);
+
 	pps_unregister_cdev(pps);
 
 	/* don't have to kfree(pps) here because it will be done on
@@ -211,6 +232,12 @@ void pps_event(struct pps_device *pps, struct pps_event_time *ts, int event,
 		captured = ~0;
 	}
 
+	/* Pass some events to kernel consumer if activated */
+	spin_lock(&pps_kc_hardpps_lock);
+	if (pps == pps_kc_hardpps_dev && event & pps_kc_hardpps_mode)
+		hardpps(&ts->ts_real, &ts->ts_raw);
+	spin_unlock(&pps_kc_hardpps_lock);
+
 	/* Wake up if captured something */
 	if (captured) {
 		pps->last_ev++;
diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index 54964a5..76f4b5e 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -197,9 +197,69 @@ static long pps_cdev_ioctl(struct file *file,
 
 		break;
 	}
+	case PPS_KC_BIND: {
+		struct pps_bind_args bind_args;
+
+		dev_dbg(pps->dev, "PPS_KC_BIND\n");
+
+		/* 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) {
+			dev_err(pps->dev, "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) {
+			dev_err(pps->dev, "invalid kernel consumer bind"
+					" parameters (%x)\n", bind_args.edge);
+			return -EINVAL;
+		}
+
+		/* Check if another consumer is already bound */
+		spin_lock_irq(&pps_kc_hardpps_lock);
+
+		if (bind_args.edge == 0)
+			if (pps_kc_hardpps_dev == pps) {
+				pps_kc_hardpps_mode = 0;
+				pps_kc_hardpps_dev = NULL;
+				spin_unlock_irq(&pps_kc_hardpps_lock);
+				dev_info(pps->dev, "unbound kernel"
+						" consumer\n");
+			} else {
+				spin_unlock_irq(&pps_kc_hardpps_lock);
+				dev_err(pps->dev, "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_irq(&pps_kc_hardpps_lock);
+				dev_info(pps->dev, "bound kernel consumer: "
+					"edge=0x%x\n", bind_args.edge);
+			} else {
+				spin_unlock_irq(&pps_kc_hardpps_lock);
+				dev_err(pps->dev, "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 6ae8898..2302274 100644
--- a/include/linux/pps_kernel.h
+++ b/include/linux/pps_kernel.h
@@ -79,6 +79,12 @@ struct pps_device {
 
 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.7.2.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCHv6 15/16] pps: add parallel port PPS client
  2010-12-17 19:54 [PATCHv6 00/16] pps: several fixes and improvements Alexander Gordeev
                   ` (13 preceding siblings ...)
  2010-12-17 19:54 ` [PATCHv6 14/16] pps: add kernel consumer support Alexander Gordeev
@ 2010-12-17 19:54 ` Alexander Gordeev
  2010-12-18  0:17   ` Andrew Morton
  2010-12-17 19:54 ` [PATCHv6 16/16] pps: add parallel port PPS signal generator Alexander Gordeev
                   ` (2 subsequent siblings)
  17 siblings, 1 reply; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-17 19:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Nikita V. Youshchenko, linuxpps, Rodolfo Giometti, Andrew Morton,
	Alexander Gordeev

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.
Clear edge capture could be disabled using clear_wait parameter.

Acked-by: Rodolfo Giometti <giometti@linux.it>
Signed-off-by: Alexander Gordeev <lasaine@lvk.cs.msu.su>
---
 drivers/pps/clients/Kconfig       |    7 +
 drivers/pps/clients/Makefile      |    1 +
 drivers/pps/clients/pps_parport.c |  247 +++++++++++++++++++++++++++++++++++++
 3 files changed, 255 insertions(+), 0 deletions(-)
 create mode 100644 drivers/pps/clients/pps_parport.c

diff --git a/drivers/pps/clients/Kconfig b/drivers/pps/clients/Kconfig
index 4e801bd..8520a7f 100644
--- a/drivers/pps/clients/Kconfig
+++ b/drivers/pps/clients/Kconfig
@@ -22,4 +22,11 @@ config PPS_CLIENT_LDISC
 	  If you say yes here you get support for a PPS source connected
 	  with the CD (Carrier Detect) pin of your serial port.
 
+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
index 812c9b1..42517da 100644
--- a/drivers/pps/clients/Makefile
+++ b/drivers/pps/clients/Makefile
@@ -4,6 +4,7 @@
 
 obj-$(CONFIG_PPS_CLIENT_KTIMER)	+= pps-ktimer.o
 obj-$(CONFIG_PPS_CLIENT_LDISC)	+= pps-ldisc.o
+obj-$(CONFIG_PPS_CLIENT_PARPORT) += pps_parport.o
 
 ifeq ($(CONFIG_PPS_DEBUG),y)
 EXTRA_CFLAGS += -DDEBUG
diff --git a/drivers/pps/clients/pps_parport.c b/drivers/pps/clients/pps_parport.c
new file mode 100644
index 0000000..501d440
--- /dev/null
+++ b/drivers/pps/clients/pps_parport.c
@@ -0,0 +1,247 @@
+/*
+ * pps_parport.c -- kernel parallel port PPS client
+ *
+ *
+ * Copyright (C) 2009   Alexander Gordeev <lasaine@lvk.cs.msu.su>
+ *
+ * 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:
+ * implement echo over SEL pin
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#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 DRVDESC "parallel port PPS client"
+
+/* module parameters */
+
+#define CLEAR_WAIT_MAX		100
+#define CLEAR_WAIT_MAX_ERRORS	5
+
+static unsigned int clear_wait = 100;
+MODULE_PARM_DESC(clear_wait,
+	"Maximum number of port reads when polling for signal clear,"
+	" zero turns clear edge capture off entirely");
+module_param(clear_wait, uint, 0);
+
+
+/* internal per port structure */
+struct pps_client_pp {
+	struct pardevice *pardev;	/* parport device */
+	struct pps_device *pps;		/* PPS device */
+	unsigned int cw;		/* port clear timeout */
+	unsigned int cw_err;		/* number of timeouts */
+};
+
+static inline int signal_is_set(struct parport *port)
+{
+	return (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;
+	unsigned int i;
+	unsigned long flags;
+
+	/* first of all we get the time stamp... */
+	pps_get_ts(&ts_assert);
+
+	if (dev->cw == 0)
+		/* clear edge capture disabled */
+		goto out_assert;
+
+	/* try capture the clear edge */
+	local_irq_save(flags);
+	/* check the signal (no signal means the pulse is lost this time) */
+	if (!signal_is_set(port)) {
+		local_irq_restore(flags);
+		dev_err(dev->pps->dev, "lost the signal\n");
+		goto out_assert;
+	}
+
+	/* poll the port until the signal is unset */
+	for (i = dev->cw; i; i--)
+		if (!signal_is_set(port)) {
+			pps_get_ts(&ts_clear);
+			local_irq_restore(flags);
+			dev->cw_err = 0;
+			goto out_both;
+		}
+	local_irq_restore(flags);
+
+	/* timeout */
+	dev->cw_err++;
+	if (dev->cw_err >= CLEAR_WAIT_MAX_ERRORS) {
+		dev_err(dev->pps->dev, "disabled clear edge capture after %d"
+				" timeouts\n", dev->cw_err);
+		dev->cw = 0;
+		dev->cw_err = 0;
+	}
+
+out_assert:
+	/* fire assert event */
+	pps_event(dev->pps, &ts_assert,
+			PPS_CAPTUREASSERT, NULL);
+	return;
+
+out_both:
+	/* fire assert event */
+	pps_event(dev->pps, &ts_assert,
+			PPS_CAPTUREASSERT, NULL);
+	/* fire clear event */
+	pps_event(dev->pps, &ts_clear,
+			PPS_CAPTURECLEAR, NULL);
+	return;
+}
+
+/* the PPS echo function */
+static void pps_echo(struct pps_device *pps, int event, void *data)
+{
+	dev_info(pps->dev, "echo %s %s\n",
+		event & PPS_CAPTUREASSERT ? "assert" : "",
+		event & PPS_CAPTURECLEAR ? "clear" : "");
+}
+
+static void parport_attach(struct parport *port)
+{
+	struct pps_client_pp *device;
+	struct pps_source_info info = {
+		.name		= KBUILD_MODNAME,
+		.path		= "",
+		.mode		= PPS_CAPTUREBOTH | \
+				  PPS_OFFSETASSERT | PPS_OFFSETCLEAR | \
+				  PPS_ECHOASSERT | PPS_ECHOCLEAR | \
+				  PPS_CANWAIT | PPS_TSFMT_TSPEC,
+		.echo		= pps_echo,
+		.owner		= THIS_MODULE,
+		.dev		= NULL
+	};
+
+	device = kzalloc(sizeof(struct pps_client_pp), GFP_KERNEL);
+	if (!device) {
+		pr_err("memory allocation failed, not attaching\n");
+		return;
+	}
+
+	device->pardev = parport_register_device(port, KBUILD_MODNAME,
+			NULL, NULL, parport_irq, 0, device);
+	if (!device->pardev) {
+		pr_err("couldn't register with %s\n", port->name);
+		goto err_free;
+	}
+
+	if (parport_claim_or_block(device->pardev) < 0) {
+		pr_err("couldn't claim %s\n", port->name);
+		goto err_unregister_dev;
+	}
+
+	device->pps = pps_register_source(&info,
+			PPS_CAPTUREBOTH | PPS_OFFSETASSERT | PPS_OFFSETCLEAR);
+	if (device->pps == NULL) {
+		pr_err("couldn't register PPS source\n");
+		goto err_release_dev;
+	}
+
+	device->cw = clear_wait;
+
+	port->ops->enable_irq(port);
+
+	pr_info("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;
+
+	/* FIXME: oooh, this is ugly! */
+	if (strcmp(pardev->name, KBUILD_MODNAME))
+		/* not our port */
+		return;
+
+	device = pardev->private;
+
+	port->ops->disable_irq(port);
+	pps_unregister_source(device->pps);
+	parport_release(pardev);
+	parport_unregister_device(pardev);
+	kfree(device);
+}
+
+static struct parport_driver pps_parport_driver = {
+	.name = KBUILD_MODNAME,
+	.attach = parport_attach,
+	.detach = parport_detach,
+};
+
+/* module staff */
+
+static int __init pps_parport_init(void)
+{
+	int ret;
+
+	pr_info(DRVDESC "\n");
+
+	if (clear_wait > CLEAR_WAIT_MAX) {
+		pr_err("clear_wait value should be not greater"
+				" then %d\n", CLEAR_WAIT_MAX);
+		return -EINVAL;
+	}
+
+	ret = parport_register_driver(&pps_parport_driver);
+	if (ret) {
+		pr_err("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 <lasaine@lvk.cs.msu.su>");
+MODULE_DESCRIPTION(DRVDESC);
+MODULE_LICENSE("GPL");
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCHv6 16/16] pps: add parallel port PPS signal generator
  2010-12-17 19:54 [PATCHv6 00/16] pps: several fixes and improvements Alexander Gordeev
                   ` (14 preceding siblings ...)
  2010-12-17 19:54 ` [PATCHv6 15/16] pps: add parallel port PPS client Alexander Gordeev
@ 2010-12-17 19:54 ` Alexander Gordeev
  2010-12-18  0:18   ` Andrew Morton
  2010-12-18  0:19 ` [PATCHv6 00/16] pps: several fixes and improvements Andrew Morton
  2010-12-20 11:54 ` [PATCHv7 00/16] changed some patches Alexander Gordeev
  17 siblings, 1 reply; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-17 19:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Nikita V. Youshchenko, linuxpps, Rodolfo Giometti, Andrew Morton,
	Alexander Gordeev, Randy Dunlap, linux-doc

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.

Acked-by: Rodolfo Giometti <giometti@linux.it>
Signed-off-by: Alexander Gordeev <lasaine@lvk.cs.msu.su>
---
 Documentation/pps/pps.txt                |   46 +++++
 drivers/pps/Kconfig                      |    2 +
 drivers/pps/Makefile                     |    2 +-
 drivers/pps/generators/Kconfig           |   17 ++
 drivers/pps/generators/Makefile          |    9 +
 drivers/pps/generators/pps_gen_parport.c |  275 ++++++++++++++++++++++++++++++
 6 files changed, 350 insertions(+), 1 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/Documentation/pps/pps.txt b/Documentation/pps/pps.txt
index 125f4ab..d35dcdd 100644
--- a/Documentation/pps/pps.txt
+++ b/Documentation/pps/pps.txt
@@ -170,3 +170,49 @@ and the run ppstest as follow:
 
 Please, note that to compile userland programs you need the file timepps.h
 (see Documentation/pps/).
+
+
+Generators
+----------
+
+Sometimes one needs to be able not only to catch PPS signals but to produce
+them also. For example, running a distributed simulation, which requires
+computers' clock to be synchronized very tightly. One way to do this is to
+invent some complicated hardware solutions but it may be neither necessary
+nor affordable. The cheap way is to load a PPS generator on one of the
+computers (master) and PPS clients on others (slaves), and use very simple
+cables to deliver signals using parallel ports, for example.
+
+Parallel port cable pinout:
+pin	name	master      slave
+1	STROBE	  *------     *
+2	D0	  *     |     *
+3	D1	  *     |     *
+4	D2	  *     |     *
+5	D3	  *     |     *
+6	D4	  *     |     *
+7	D5	  *     |     *
+8	D6	  *     |     *
+9	D7	  *     |     *
+10	ACK	  *     ------*
+11	BUSY	  *           *
+12	PE	  *           *
+13	SEL	  *           *
+14	AUTOFD	  *           *
+15	ERROR	  *           *
+16	INIT	  *           *
+17	SELIN	  *           *
+18-25	GND	  *-----------*
+
+Please note that parallel port interrupt occurs only on high->low transition,
+so it is used for PPS assert edge. PPS clear edge can be determined only
+using polling in the interrupt handler which actually can be done way more
+precisely because interrupt handling delays can be quite big and random. So
+current parport PPS generator implementation (pps_gen_parport module) is
+geared towards using the clear edge for time synchronization.
+
+Clear edge polling is done with disabled interrupts so it's better to select
+delay between assert and clear edge as small as possible to reduce system
+latencies. But if it is too small slave won't be able to capture clear edge
+transition. The default of 30us should be good enough in most situations.
+The delay can be selected using 'delay' pps_gen_parport module parameter.
diff --git a/drivers/pps/Kconfig b/drivers/pps/Kconfig
index 4823e47..79bda60 100644
--- a/drivers/pps/Kconfig
+++ b/drivers/pps/Kconfig
@@ -40,4 +40,6 @@ config NTP_PPS
 
 source drivers/pps/clients/Kconfig
 
+source drivers/pps/generators/Kconfig
+
 endmenu
diff --git a/drivers/pps/Makefile b/drivers/pps/Makefile
index 98960dd..1906eb70 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				+= clients/
+obj-y				+= clients/ generators/
 
 ccflags-$(CONFIG_PPS_DEBUG) := -DDEBUG
diff --git a/drivers/pps/generators/Kconfig b/drivers/pps/generators/Kconfig
new file mode 100644
index 0000000..5fbd614
--- /dev/null
+++ b/drivers/pps/generators/Kconfig
@@ -0,0 +1,17 @@
+#
+# PPS generators 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..aa66803
--- /dev/null
+++ b/drivers/pps/generators/pps_gen_parport.c
@@ -0,0 +1,275 @@
+/*
+ * pps_gen_parport.c -- kernel parallel port PPS signal generator
+ *
+ *
+ * Copyright (C) 2009   Alexander Gordeev <lasaine@lvk.cs.msu.su>
+ *
+ * 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:
+ * fix issues when realtime clock is adjusted in a leap
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#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 DRVDESC "parallel port PPS signal generator"
+
+#define SIGNAL		0
+#define NO_SIGNAL	PARPORT_CONTROL_STROBE
+
+/* module parameters */
+
+#define SEND_DELAY_MAX		100000
+
+static unsigned int send_delay = 30000;
+MODULE_PARM_DESC(delay,
+	"Delay between setting and dropping the signal (ns)");
+module_param_named(delay, send_delay, uint, 0);
+
+
+#define SAFETY_INTERVAL	3000	/* 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 + 3 * 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("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 + SAFETY_INTERVAL +
+				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("port write takes %ldns\n", dev->port_write_time);
+}
+
+static inline 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 + 3 * SAFETY_INTERVAL));
+}
+
+static void parport_attach(struct parport *port)
+{
+	if (attached) {
+		/* we already have a port */
+		return;
+	}
+
+	device.pardev = parport_register_device(port, KBUILD_MODNAME,
+			NULL, NULL, NULL, 0, &device);
+	if (!device.pardev) {
+		pr_err("couldn't register with %s\n", port->name);
+		return;
+	}
+
+	if (parport_claim_or_block(device.pardev) < 0) {
+		pr_err("couldn't claim %s\n", port->name);
+		goto err_unregister_dev;
+	}
+
+	pr_info("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)
+		return;	/* not our port */
+
+	hrtimer_cancel(&device.timer);
+	parport_release(device.pardev);
+	parport_unregister_device(device.pardev);
+}
+
+static struct parport_driver pps_gen_parport_driver = {
+	.name = KBUILD_MODNAME,
+	.attach = parport_attach,
+	.detach = parport_detach,
+};
+
+/* module staff */
+
+static int __init pps_gen_parport_init(void)
+{
+	int ret;
+
+	pr_info(DRVDESC "\n");
+
+	if (send_delay > SEND_DELAY_MAX) {
+		pr_err("delay value should be not greater"
+				" then %d\n", SEND_DELAY_MAX);
+		return -EINVAL;
+	}
+
+	ret = parport_register_driver(&pps_gen_parport_driver);
+	if (ret) {
+		pr_err("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("hrtimer avg error is %ldns\n", hrtimer_error);
+}
+
+module_init(pps_gen_parport_init);
+module_exit(pps_gen_parport_exit);
+
+MODULE_AUTHOR("Alexander Gordeev <lasaine@lvk.cs.msu.su>");
+MODULE_DESCRIPTION(DRVDESC);
+MODULE_LICENSE("GPL");
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: [PATCHv6 07/16] pps: move idr stuff to pps.c
  2010-12-17 19:54 ` [PATCHv6 07/16] pps: move idr stuff to pps.c Alexander Gordeev
@ 2010-12-18  0:13   ` Andrew Morton
  2010-12-18  1:07     ` Alexander Gordeev
  0 siblings, 1 reply; 37+ messages in thread
From: Andrew Morton @ 2010-12-18  0:13 UTC (permalink / raw)
  To: Alexander Gordeev
  Cc: linux-kernel, Nikita V. Youshchenko, linuxpps, Rodolfo Giometti

On Fri, 17 Dec 2010 22:54:31 +0300
Alexander Gordeev <lasaine@lvk.cs.msu.su> wrote:

> Since now idr is only used to manage char device id's and not used in
> kernel API anymore it should be moved to pps.c. This also makes it
> possible to release id only at actual device freeing so nobody can
> register a pps device with the same id while our device is not freed
> yet.
> 
> ...
>
>  int pps_register_cdev(struct pps_device *pps)
>  {
>  	int err;
>  	dev_t devt;
>  
> +	/* Get new ID for the new PPS source */
> +	if (idr_pre_get(&pps_idr, GFP_KERNEL) == 0)
> +		return -ENOMEM;
> +
> +	/* Now really allocate the PPS source.
> +	 * After idr_get_new() calling the new source will be freely available
> +	 * into the kernel.
> +	 */
> +	spin_lock_irq(&pps_idr_lock);
> +	err = idr_get_new(&pps_idr, pps, &pps->id);
> +	spin_unlock_irq(&pps_idr_lock);
> +
> +	if (err < 0)
> +		return err;

The IDR interface really sucks :(

What this code should be doing is

retry:
	if (idr_pre_get(&pps_idr, GFP_KERNEL) == 0)
		return -ENOMEM;
	spin_lock_irq(&pps_idr_lock);
	err = idr_get_new(&pps_idr, pps, &pps->id);
	spin_unlock_irq(&pps_idr_lock);
	if (err < 0) {
		if (err == -EAGAIN)
			goto retry;
		return err;
	}

this way it correctly handles the case where the idr_pre_get()
succeeded in precharging the pool, but some other task cam in and stole
your reservation.



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCHv6 15/16] pps: add parallel port PPS client
  2010-12-17 19:54 ` [PATCHv6 15/16] pps: add parallel port PPS client Alexander Gordeev
@ 2010-12-18  0:17   ` Andrew Morton
  2010-12-18  0:50     ` Alexander Gordeev
  0 siblings, 1 reply; 37+ messages in thread
From: Andrew Morton @ 2010-12-18  0:17 UTC (permalink / raw)
  To: Alexander Gordeev
  Cc: linux-kernel, Nikita V. Youshchenko, linuxpps, Rodolfo Giometti

On Fri, 17 Dec 2010 22:54:39 +0300
Alexander Gordeev <lasaine@lvk.cs.msu.su> wrote:

> 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.
> Clear edge capture could be disabled using clear_wait parameter.
> 
> ...
>
> +/* 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;
> +	unsigned int i;
> +	unsigned long flags;
> +
> +	/* first of all we get the time stamp... */
> +	pps_get_ts(&ts_assert);
> +
> +	if (dev->cw == 0)
> +		/* clear edge capture disabled */
> +		goto out_assert;
> +
> +	/* try capture the clear edge */
> +	local_irq_save(flags);
> +	/* check the signal (no signal means the pulse is lost this time) */
> +	if (!signal_is_set(port)) {
> +		local_irq_restore(flags);
> +		dev_err(dev->pps->dev, "lost the signal\n");
> +		goto out_assert;
> +	}
> +
> +	/* poll the port until the signal is unset */
> +	for (i = dev->cw; i; i--)
> +		if (!signal_is_set(port)) {
> +			pps_get_ts(&ts_clear);
> +			local_irq_restore(flags);
> +			dev->cw_err = 0;
> +			goto out_both;
> +		}
> +	local_irq_restore(flags);

Why is this function paying around with local_irq_save()?  It's unusual
and looks buggy because local_irq_save() doesn't stop other CPUs from
taking an interrupt and coming in and playing with the "protected" data.

I have a feeling I asked this before, but you didn't add a comment
explaining it, so I went and asked it again!  Cause, effect.

> +static void parport_attach(struct parport *port)
> +{
> +	struct pps_client_pp *device;
> +	struct pps_source_info info = {
> +		.name		= KBUILD_MODNAME,
> +		.path		= "",
> +		.mode		= PPS_CAPTUREBOTH | \
> +				  PPS_OFFSETASSERT | PPS_OFFSETCLEAR | \
> +				  PPS_ECHOASSERT | PPS_ECHOCLEAR | \
> +				  PPS_CANWAIT | PPS_TSFMT_TSPEC,
> +		.echo		= pps_echo,
> +		.owner		= THIS_MODULE,
> +		.dev		= NULL
> +	};
> +
> +	device = kzalloc(sizeof(struct pps_client_pp), GFP_KERNEL);
> +	if (!device) {
> +		pr_err("memory allocation failed, not attaching\n");
> +		return;

And the void-returning parport_driver.attach() still sucks.

> +	}
> +
> +	device->pardev = parport_register_device(port, KBUILD_MODNAME,
> +			NULL, NULL, parport_irq, 0, device);
> +	if (!device->pardev) {
> +		pr_err("couldn't register with %s\n", port->name);
> +		goto err_free;
> +	}
> +
> +	if (parport_claim_or_block(device->pardev) < 0) {
> +		pr_err("couldn't claim %s\n", port->name);
> +		goto err_unregister_dev;
> +	}
> +
> +	device->pps = pps_register_source(&info,
> +			PPS_CAPTUREBOTH | PPS_OFFSETASSERT | PPS_OFFSETCLEAR);
> +	if (device->pps == NULL) {
> +		pr_err("couldn't register PPS source\n");
> +		goto err_release_dev;
> +	}
> +
> +	device->cw = clear_wait;
> +
> +	port->ops->enable_irq(port);
> +
> +	pr_info("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);
> +}
> +


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCHv6 16/16] pps: add parallel port PPS signal generator
  2010-12-17 19:54 ` [PATCHv6 16/16] pps: add parallel port PPS signal generator Alexander Gordeev
@ 2010-12-18  0:18   ` Andrew Morton
  2010-12-18  0:52     ` Alexander Gordeev
  0 siblings, 1 reply; 37+ messages in thread
From: Andrew Morton @ 2010-12-18  0:18 UTC (permalink / raw)
  To: Alexander Gordeev
  Cc: linux-kernel, Nikita V. Youshchenko, linuxpps, Rodolfo Giometti,
	Randy Dunlap, linux-doc

On Fri, 17 Dec 2010 22:54:40 +0300
Alexander Gordeev <lasaine@lvk.cs.msu.su> wrote:

> +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 + 3 * SAFETY_INTERVAL */
> +	local_irq_save(flags);

ditto.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCHv6 00/16] pps: several fixes and improvements
  2010-12-17 19:54 [PATCHv6 00/16] pps: several fixes and improvements Alexander Gordeev
                   ` (15 preceding siblings ...)
  2010-12-17 19:54 ` [PATCHv6 16/16] pps: add parallel port PPS signal generator Alexander Gordeev
@ 2010-12-18  0:19 ` Andrew Morton
  2010-12-18  1:00   ` Alexander Gordeev
  2010-12-20 11:54 ` [PATCHv7 00/16] changed some patches Alexander Gordeev
  17 siblings, 1 reply; 37+ messages in thread
From: Andrew Morton @ 2010-12-18  0:19 UTC (permalink / raw)
  To: Alexander Gordeev
  Cc: linux-kernel, Nikita V. Youshchenko, linuxpps, Rodolfo Giometti

On Fri, 17 Dec 2010 22:54:24 +0300
Alexander Gordeev <lasaine@lvk.cs.msu.su> wrote:

> There is one problem however: kernel consumer works bad (if enabled)
> when CONFIG_NO_HZ is enabled. The reason for this is commit
> a092ff0f90cae22b2ac8028ecd2c6f6c1a9e4601. Without it hardpps() is able
> to sync to 1us precision in about 10 seconds. With CONFIG_NO_HZ it is
> not syncing at all. This only affects patches 12-14, others are ok.

Is someone working on fixing all this?

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCHv6 15/16] pps: add parallel port PPS client
  2010-12-18  0:17   ` Andrew Morton
@ 2010-12-18  0:50     ` Alexander Gordeev
  2010-12-18  1:13       ` Andrew Morton
  0 siblings, 1 reply; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-18  0:50 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel, Nikita V. Youshchenko, linuxpps, Rodolfo Giometti

[-- Attachment #1: Type: text/plain, Size: 4851 bytes --]

В Fri, 17 Dec 2010 16:17:56 -0800
Andrew Morton <akpm@linux-foundation.org> пишет:

> On Fri, 17 Dec 2010 22:54:39 +0300
> Alexander Gordeev <lasaine@lvk.cs.msu.su> wrote:
> 
> > 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.
> > Clear edge capture could be disabled using clear_wait parameter.
> > 
> > ...
> >
> > +/* 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;
> > +	unsigned int i;
> > +	unsigned long flags;
> > +
> > +	/* first of all we get the time stamp... */
> > +	pps_get_ts(&ts_assert);
> > +
> > +	if (dev->cw == 0)
> > +		/* clear edge capture disabled */
> > +		goto out_assert;
> > +
> > +	/* try capture the clear edge */
> > +	local_irq_save(flags);
> > +	/* check the signal (no signal means the pulse is lost this time) */
> > +	if (!signal_is_set(port)) {
> > +		local_irq_restore(flags);
> > +		dev_err(dev->pps->dev, "lost the signal\n");
> > +		goto out_assert;
> > +	}
> > +
> > +	/* poll the port until the signal is unset */
> > +	for (i = dev->cw; i; i--)
> > +		if (!signal_is_set(port)) {
> > +			pps_get_ts(&ts_clear);
> > +			local_irq_restore(flags);
> > +			dev->cw_err = 0;
> > +			goto out_both;
> > +		}
> > +	local_irq_restore(flags);
> 
> Why is this function paying around with local_irq_save()?  It's unusual
> and looks buggy because local_irq_save() doesn't stop other CPUs from
> taking an interrupt and coming in and playing with the "protected" data.

The idea is to prevent other interrupts on the same processor to
introduce uncontrolled time lags here. Reading from IO port is known to
take approximately 1us while other interrupt handlers can probably take
much more. So I poll the port with locally disabled interrupts to
ensure that the maximum lag here is 1us. All my experiments show that
it is in fact very precise this way given that input signal is precise.

> I have a feeling I asked this before, but you didn't add a comment
> explaining it, so I went and asked it again!  Cause, effect.

No, you didn't ask this before, I think.

> > +static void parport_attach(struct parport *port)
> > +{
> > +	struct pps_client_pp *device;
> > +	struct pps_source_info info = {
> > +		.name		= KBUILD_MODNAME,
> > +		.path		= "",
> > +		.mode		= PPS_CAPTUREBOTH | \
> > +				  PPS_OFFSETASSERT | PPS_OFFSETCLEAR | \
> > +				  PPS_ECHOASSERT | PPS_ECHOCLEAR | \
> > +				  PPS_CANWAIT | PPS_TSFMT_TSPEC,
> > +		.echo		= pps_echo,
> > +		.owner		= THIS_MODULE,
> > +		.dev		= NULL
> > +	};
> > +
> > +	device = kzalloc(sizeof(struct pps_client_pp), GFP_KERNEL);
> > +	if (!device) {
> > +		pr_err("memory allocation failed, not attaching\n");
> > +		return;
> 
> And the void-returning parport_driver.attach() still sucks.

Hmm. Do you want me to rewrite the parport subsystem? There are lots of
problems in it now:
 * not part of LDM
 * controlled through /proc
 * detach is called for every port, not only the on being attached to
 * ...

Sure, I think it should be rewritten quite a bit but I cannot do that
right now, sorry.

> > +	}
> > +
> > +	device->pardev = parport_register_device(port, KBUILD_MODNAME,
> > +			NULL, NULL, parport_irq, 0, device);
> > +	if (!device->pardev) {
> > +		pr_err("couldn't register with %s\n", port->name);
> > +		goto err_free;
> > +	}
> > +
> > +	if (parport_claim_or_block(device->pardev) < 0) {
> > +		pr_err("couldn't claim %s\n", port->name);
> > +		goto err_unregister_dev;
> > +	}
> > +
> > +	device->pps = pps_register_source(&info,
> > +			PPS_CAPTUREBOTH | PPS_OFFSETASSERT | PPS_OFFSETCLEAR);
> > +	if (device->pps == NULL) {
> > +		pr_err("couldn't register PPS source\n");
> > +		goto err_release_dev;
> > +	}
> > +
> > +	device->cw = clear_wait;
> > +
> > +	port->ops->enable_irq(port);
> > +
> > +	pr_info("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);
> > +}
> > +
> 


-- 
  Alexander

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCHv6 16/16] pps: add parallel port PPS signal generator
  2010-12-18  0:18   ` Andrew Morton
@ 2010-12-18  0:52     ` Alexander Gordeev
  0 siblings, 0 replies; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-18  0:52 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel, Nikita V. Youshchenko, linuxpps, Rodolfo Giometti,
	Randy Dunlap, linux-doc

[-- Attachment #1: Type: text/plain, Size: 699 bytes --]

В Fri, 17 Dec 2010 16:18:37 -0800
Andrew Morton <akpm@linux-foundation.org> пишет:

> On Fri, 17 Dec 2010 22:54:40 +0300
> Alexander Gordeev <lasaine@lvk.cs.msu.su> wrote:
> 
> > +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 + 3 * SAFETY_INTERVAL */
> > +	local_irq_save(flags);
> 
> ditto.

The same thing here: we don't want random lags due to other local
interrupt handlers.

-- 
  Alexander

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCHv6 00/16] pps: several fixes and improvements
  2010-12-18  0:19 ` [PATCHv6 00/16] pps: several fixes and improvements Andrew Morton
@ 2010-12-18  1:00   ` Alexander Gordeev
  2010-12-18  1:14     ` Andrew Morton
  0 siblings, 1 reply; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-18  1:00 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel, Nikita V. Youshchenko, linuxpps, Rodolfo Giometti

[-- Attachment #1: Type: text/plain, Size: 759 bytes --]

В Fri, 17 Dec 2010 16:19:38 -0800
Andrew Morton <akpm@linux-foundation.org> пишет:

> On Fri, 17 Dec 2010 22:54:24 +0300
> Alexander Gordeev <lasaine@lvk.cs.msu.su> wrote:
> 
> > There is one problem however: kernel consumer works bad (if enabled)
> > when CONFIG_NO_HZ is enabled. The reason for this is commit
> > a092ff0f90cae22b2ac8028ecd2c6f6c1a9e4601. Without it hardpps() is able
> > to sync to 1us precision in about 10 seconds. With CONFIG_NO_HZ it is
> > not syncing at all. This only affects patches 12-14, others are ok.
> 
> Is someone working on fixing all this?

Not right now but it is planned. I can only ensure that users can never
select CONFIG_NO_HZ and CONFIG_NTP_PPS at the same time. Is it ok?

-- 
  Alexander

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCHv6 07/16] pps: move idr stuff to pps.c
  2010-12-18  0:13   ` Andrew Morton
@ 2010-12-18  1:07     ` Alexander Gordeev
  2010-12-18  1:18       ` Andrew Morton
  0 siblings, 1 reply; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-18  1:07 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel, Nikita V. Youshchenko, linuxpps, Rodolfo Giometti

[-- Attachment #1: Type: text/plain, Size: 1990 bytes --]

В Fri, 17 Dec 2010 16:13:28 -0800
Andrew Morton <akpm@linux-foundation.org> пишет:

> On Fri, 17 Dec 2010 22:54:31 +0300
> Alexander Gordeev <lasaine@lvk.cs.msu.su> wrote:
> 
> > Since now idr is only used to manage char device id's and not used in
> > kernel API anymore it should be moved to pps.c. This also makes it
> > possible to release id only at actual device freeing so nobody can
> > register a pps device with the same id while our device is not freed
> > yet.
> > 
> > ...
> >
> >  int pps_register_cdev(struct pps_device *pps)
> >  {
> >  	int err;
> >  	dev_t devt;
> >  
> > +	/* Get new ID for the new PPS source */
> > +	if (idr_pre_get(&pps_idr, GFP_KERNEL) == 0)
> > +		return -ENOMEM;
> > +
> > +	/* Now really allocate the PPS source.
> > +	 * After idr_get_new() calling the new source will be freely available
> > +	 * into the kernel.
> > +	 */
> > +	spin_lock_irq(&pps_idr_lock);
> > +	err = idr_get_new(&pps_idr, pps, &pps->id);
> > +	spin_unlock_irq(&pps_idr_lock);
> > +
> > +	if (err < 0)
> > +		return err;
> 
> The IDR interface really sucks :(
> 
> What this code should be doing is
> 
> retry:
> 	if (idr_pre_get(&pps_idr, GFP_KERNEL) == 0)
> 		return -ENOMEM;
> 	spin_lock_irq(&pps_idr_lock);
> 	err = idr_get_new(&pps_idr, pps, &pps->id);
> 	spin_unlock_irq(&pps_idr_lock);
> 	if (err < 0) {
> 		if (err == -EAGAIN)
> 			goto retry;
> 		return err;
> 	}
> 
> this way it correctly handles the case where the idr_pre_get()
> succeeded in precharging the pool, but some other task cam in and stole
> your reservation.

Yeah, I see. Maybe switching from spin lock to mutex and protecting the
whole thing with it can do? Like this:

...
mutex_lock(&pps_idr_lock);
if (idr_pre_get(&pps_idr, GFP_KERNEL) == 0) {
	mutex_unlock(&pps_idr_lock);
	return -ENOMEM;
}
err = idr_get_new(&pps_idr, pps, &pps->id);
mutex_unlock(&pps_idr_lock);

if (err < 0)
	return err;
...

-- 
  Alexander

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCHv6 15/16] pps: add parallel port PPS client
  2010-12-18  0:50     ` Alexander Gordeev
@ 2010-12-18  1:13       ` Andrew Morton
  0 siblings, 0 replies; 37+ messages in thread
From: Andrew Morton @ 2010-12-18  1:13 UTC (permalink / raw)
  To: Alexander Gordeev
  Cc: linux-kernel, Nikita V. Youshchenko, linuxpps, Rodolfo Giometti

On Sat, 18 Dec 2010 03:50:54 +0300
Alexander Gordeev <lasaine@lvk.cs.msu.su> wrote:

> __ Fri, 17 Dec 2010 16:17:56 -0800
> Andrew Morton <akpm@linux-foundation.org> __________:
> 
> > On Fri, 17 Dec 2010 22:54:39 +0300
> > Alexander Gordeev <lasaine@lvk.cs.msu.su> wrote:
> > 
> > > 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.
> > > Clear edge capture could be disabled using clear_wait parameter.
> > > 
> > > ...
> > >
> > > +/* 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;
> > > +	unsigned int i;
> > > +	unsigned long flags;
> > > +
> > > +	/* first of all we get the time stamp... */
> > > +	pps_get_ts(&ts_assert);
> > > +
> > > +	if (dev->cw == 0)
> > > +		/* clear edge capture disabled */
> > > +		goto out_assert;
> > > +
> > > +	/* try capture the clear edge */
> > > +	local_irq_save(flags);
> > > +	/* check the signal (no signal means the pulse is lost this time) */
> > > +	if (!signal_is_set(port)) {
> > > +		local_irq_restore(flags);
> > > +		dev_err(dev->pps->dev, "lost the signal\n");
> > > +		goto out_assert;
> > > +	}
> > > +
> > > +	/* poll the port until the signal is unset */
> > > +	for (i = dev->cw; i; i--)
> > > +		if (!signal_is_set(port)) {
> > > +			pps_get_ts(&ts_clear);
> > > +			local_irq_restore(flags);
> > > +			dev->cw_err = 0;
> > > +			goto out_both;
> > > +		}
> > > +	local_irq_restore(flags);
> > 
> > Why is this function paying around with local_irq_save()?  It's unusual
> > and looks buggy because local_irq_save() doesn't stop other CPUs from
> > taking an interrupt and coming in and playing with the "protected" data.
> 
> The idea is to prevent other interrupts on the same processor to
> introduce uncontrolled time lags here. Reading from IO port is known to
> take approximately 1us while other interrupt handlers can probably take
> much more. So I poll the port with locally disabled interrupts to
> ensure that the maximum lag here is 1us. All my experiments show that
> it is in fact very precise this way given that input signal is precise.

Please send along a patch which explains all this to future readers?

> Hmm. Do you want me to rewrite the parport subsystem?

yep!  And page reclaim, writeback and readahead, please.


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCHv6 00/16] pps: several fixes and improvements
  2010-12-18  1:00   ` Alexander Gordeev
@ 2010-12-18  1:14     ` Andrew Morton
  0 siblings, 0 replies; 37+ messages in thread
From: Andrew Morton @ 2010-12-18  1:14 UTC (permalink / raw)
  To: Alexander Gordeev
  Cc: linux-kernel, Nikita V. Youshchenko, linuxpps, Rodolfo Giometti

On Sat, 18 Dec 2010 04:00:01 +0300
Alexander Gordeev <lasaine@lvk.cs.msu.su> wrote:

> __ Fri, 17 Dec 2010 16:19:38 -0800
> Andrew Morton <akpm@linux-foundation.org> __________:
> 
> > On Fri, 17 Dec 2010 22:54:24 +0300
> > Alexander Gordeev <lasaine@lvk.cs.msu.su> wrote:
> > 
> > > There is one problem however: kernel consumer works bad (if enabled)
> > > when CONFIG_NO_HZ is enabled. The reason for this is commit
> > > a092ff0f90cae22b2ac8028ecd2c6f6c1a9e4601. Without it hardpps() is able
> > > to sync to 1us precision in about 10 seconds. With CONFIG_NO_HZ it is
> > > not syncing at all. This only affects patches 12-14, others are ok.
> > 
> > Is someone working on fixing all this?
> 
> Not right now but it is planned. I can only ensure that users can never
> select CONFIG_NO_HZ and CONFIG_NTP_PPS at the same time. Is it ok?

That's much better than permitting a subtly broken driver.

CONFIG_NO_HZ is pretty common nowadays, methinks.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCHv6 07/16] pps: move idr stuff to pps.c
  2010-12-18  1:07     ` Alexander Gordeev
@ 2010-12-18  1:18       ` Andrew Morton
  0 siblings, 0 replies; 37+ messages in thread
From: Andrew Morton @ 2010-12-18  1:18 UTC (permalink / raw)
  To: Alexander Gordeev
  Cc: linux-kernel, Nikita V. Youshchenko, linuxpps, Rodolfo Giometti

On Sat, 18 Dec 2010 04:07:38 +0300
Alexander Gordeev <lasaine@lvk.cs.msu.su> wrote:

> > > +	if (err < 0)
> > > +		return err;
> > 
> > The IDR interface really sucks :(
> > 
> > What this code should be doing is
> > 
> > retry:
> > 	if (idr_pre_get(&pps_idr, GFP_KERNEL) == 0)
> > 		return -ENOMEM;
> > 	spin_lock_irq(&pps_idr_lock);
> > 	err = idr_get_new(&pps_idr, pps, &pps->id);
> > 	spin_unlock_irq(&pps_idr_lock);
> > 	if (err < 0) {
> > 		if (err == -EAGAIN)
> > 			goto retry;
> > 		return err;
> > 	}
> > 
> > this way it correctly handles the case where the idr_pre_get()
> > succeeded in precharging the pool, but some other task cam in and stole
> > your reservation.
> 
> Yeah, I see. Maybe switching from spin lock to mutex and protecting the
> whole thing with it can do? Like this:
> 
> ...
> mutex_lock(&pps_idr_lock);
> if (idr_pre_get(&pps_idr, GFP_KERNEL) == 0) {
> 	mutex_unlock(&pps_idr_lock);
> 	return -ENOMEM;
> }
> err = idr_get_new(&pps_idr, pps, &pps->id);
> mutex_unlock(&pps_idr_lock);
> 
> if (err < 0)
> 	return err;
> ...

That works so, as long as no code path will take pps_idr_lock in the
page allocator direct-reclaim path.

That's unlikely to be happening in the PPS driver of course.  It's
conceivable that some filesystems might want to read the time when
playing with file timestamps in the direct-reclaim path, but I assume
pps_idr_lock wouldn't be taken on any read-system-time paths.


^ permalink raw reply	[flat|nested] 37+ messages in thread

* [PATCHv7 00/16] changed some patches
  2010-12-17 19:54 [PATCHv6 00/16] pps: several fixes and improvements Alexander Gordeev
                   ` (16 preceding siblings ...)
  2010-12-18  0:19 ` [PATCHv6 00/16] pps: several fixes and improvements Andrew Morton
@ 2010-12-20 11:54 ` Alexander Gordeev
  2010-12-20 11:54   ` [PATCHv7 08/16] pps: make idr lock a mutex and protect idr_pre_get Alexander Gordeev
                     ` (5 more replies)
  17 siblings, 6 replies; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-20 11:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Nikita V. Youshchenko, linuxpps, Rodolfo Giometti, Andrew Morton,
	Alexander Gordeev

I'm sending only the patches that were changed.

Changelog
v6 -> v7:
 * fixed issues pointaed out by Andrew Morton:
   * convert pps_idr_lock to mutex and protect idr_pre_get
   * comment usage of local_irq_save()/local_irq_restore()
     in pps_parport and pps_gen_parport
   * don't allow NTP_PPS and NO_HZ at the same time
 * allow to select generators if PPS is not selected
 * disable as much code related to kernel consumer as possible
   if NTP_PPS is not selected
   * extract kernel consumer code to drivers/pps/kc.{c,h}

Alexander Gordeev (16):
  pps: trivial fixes
  pps: declare variables where they are used in switch
  pps: fix race in PPS_FETCH handler
  pps: unify timestamp gathering
  pps: access pps device by direct pointer
  pps: convert printk/pr_* to dev_*
  pps: move idr stuff to pps.c
  pps: make idr lock a mutex and protect idr_pre_get
  pps: use BUG_ON for kernel API safety checks
  pps: simplify conditions a bit
  pps: timestamp is always passed to dcd_change()
  ntp: add hardpps implementation
  pps: capture MONOTONIC_RAW timestamps as well
  pps: add kernel consumer support
  pps: add parallel port PPS client
  pps: add parallel port PPS signal generator

 Documentation/ioctl/ioctl-number.txt     |    2 +-
 Documentation/pps/pps.txt                |   46 ++++
 Documentation/serial/tty.txt             |    2 +-
 drivers/pps/Kconfig                      |   11 +
 drivers/pps/Makefile                     |    3 +-
 drivers/pps/clients/Kconfig              |    7 +
 drivers/pps/clients/Makefile             |    1 +
 drivers/pps/clients/pps-ktimer.c         |   44 ++--
 drivers/pps/clients/pps-ldisc.c          |   59 ++--
 drivers/pps/clients/pps_parport.c        |  258 ++++++++++++++++++
 drivers/pps/generators/Kconfig           |   13 +
 drivers/pps/generators/Makefile          |    9 +
 drivers/pps/generators/pps_gen_parport.c |  282 ++++++++++++++++++++
 drivers/pps/kapi.c                       |  210 ++++-----------
 drivers/pps/kc.c                         |  123 +++++++++
 drivers/pps/kc.h                         |   47 ++++
 drivers/pps/pps.c                        |  156 +++++++++---
 include/linux/pps.h                      |    7 +
 include/linux/pps_kernel.h               |   61 ++++-
 include/linux/serial_core.h              |    5 +-
 include/linux/time.h                     |    2 +
 include/linux/timex.h                    |    1 +
 include/linux/tty_ldisc.h                |    7 +-
 kernel/time/ntp.c                        |  425 ++++++++++++++++++++++++++++-
 kernel/time/timekeeping.c                |   43 +++
 25 files changed, 1534 insertions(+), 290 deletions(-)
 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
 create mode 100644 drivers/pps/kc.c
 create mode 100644 drivers/pps/kc.h

-- 
1.7.2.3


^ permalink raw reply	[flat|nested] 37+ messages in thread

* [PATCHv7 08/16] pps: make idr lock a mutex and protect idr_pre_get
  2010-12-20 11:54 ` [PATCHv7 00/16] changed some patches Alexander Gordeev
@ 2010-12-20 11:54   ` Alexander Gordeev
  2010-12-20 11:54   ` [PATCHv7 12/16] ntp: add hardpps implementation Alexander Gordeev
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-20 11:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Nikita V. Youshchenko, linuxpps, Rodolfo Giometti, Andrew Morton,
	Alexander Gordeev

Now pps_idr_lock is never used in interrupt context so we can replace
spin_lock_irq/spin_unlock_irq with plain spin_lock/spin_unlock. But
there is also a potential race condition when someone can steal an id
which was allocated by idr_pre_get before it is used. So convert spin
lock to mutex and protect the whole id generation process.

Signed-off-by: Alexander Gordeev <lasaine@lvk.cs.msu.su>
---
 drivers/pps/pps.c |   19 +++++++++++--------
 1 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index 79b4455..9e15cf1 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -27,6 +27,7 @@
 #include <linux/sched.h>
 #include <linux/uaccess.h>
 #include <linux/idr.h>
+#include <linux/mutex.h>
 #include <linux/cdev.h>
 #include <linux/poll.h>
 #include <linux/pps_kernel.h>
@@ -39,7 +40,7 @@
 static dev_t pps_devt;
 static struct class *pps_class;
 
-static DEFINE_SPINLOCK(pps_idr_lock);
+static DEFINE_MUTEX(pps_idr_lock);
 static DEFINE_IDR(pps_idr);
 
 /*
@@ -239,9 +240,9 @@ static void pps_device_destruct(struct device *dev)
 
 	/* release id here to protect others from using it while it's
 	 * still in use */
-	spin_lock_irq(&pps_idr_lock);
+	mutex_lock(&pps_idr_lock);
 	idr_remove(&pps_idr, pps->id);
-	spin_unlock_irq(&pps_idr_lock);
+	mutex_unlock(&pps_idr_lock);
 
 	kfree(dev);
 	kfree(pps);
@@ -252,17 +253,19 @@ int pps_register_cdev(struct pps_device *pps)
 	int err;
 	dev_t devt;
 
+	mutex_lock(&pps_idr_lock);
 	/* Get new ID for the new PPS source */
-	if (idr_pre_get(&pps_idr, GFP_KERNEL) == 0)
+	if (idr_pre_get(&pps_idr, GFP_KERNEL) == 0) {
+		mutex_unlock(&pps_idr_lock);
 		return -ENOMEM;
+	}
 
 	/* Now really allocate the PPS source.
 	 * After idr_get_new() calling the new source will be freely available
 	 * into the kernel.
 	 */
-	spin_lock_irq(&pps_idr_lock);
 	err = idr_get_new(&pps_idr, pps, &pps->id);
-	spin_unlock_irq(&pps_idr_lock);
+	mutex_unlock(&pps_idr_lock);
 
 	if (err < 0)
 		return err;
@@ -302,9 +305,9 @@ del_cdev:
 	cdev_del(&pps->cdev);
 
 free_idr:
-	spin_lock_irq(&pps_idr_lock);
+	mutex_lock(&pps_idr_lock);
 	idr_remove(&pps_idr, pps->id);
-	spin_unlock_irq(&pps_idr_lock);
+	mutex_unlock(&pps_idr_lock);
 
 	return err;
 }
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCHv7 12/16] ntp: add hardpps implementation
  2010-12-20 11:54 ` [PATCHv7 00/16] changed some patches Alexander Gordeev
  2010-12-20 11:54   ` [PATCHv7 08/16] pps: make idr lock a mutex and protect idr_pre_get Alexander Gordeev
@ 2010-12-20 11:54   ` Alexander Gordeev
  2010-12-20 11:54   ` [PATCHv7 13/16] pps: capture MONOTONIC_RAW timestamps as well Alexander Gordeev
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-20 11:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Nikita V. Youshchenko, linuxpps, Rodolfo Giometti, Andrew Morton,
	Alexander Gordeev, John Stultz, Thomas Gleixner,
	Miroslav Lichvar

This commit adds hardpps() implementation based upon the original one
from the NTPv4 reference kernel code from David Mills. 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.

Acked-by: John Stultz <johnstul@us.ibm.com>
Signed-off-by: Alexander Gordeev <lasaine@lvk.cs.msu.su>
---
 drivers/pps/Kconfig   |    9 +
 include/linux/timex.h |    1 +
 kernel/time/ntp.c     |  425 +++++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 420 insertions(+), 15 deletions(-)

diff --git a/drivers/pps/Kconfig b/drivers/pps/Kconfig
index 1afe4e0..0ad5ff3 100644
--- a/drivers/pps/Kconfig
+++ b/drivers/pps/Kconfig
@@ -30,6 +30,15 @@ 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.
 
+config NTP_PPS
+	bool "PPS kernel consumer support"
+	depends on PPS && !NO_HZ
+	help
+	  This option adds support for direct in-kernel time
+	  syncronization using an external PPS signal.
+
+	  It doesn't work on tickless systems at the moment.
+
 source drivers/pps/clients/Kconfig
 
 endmenu
diff --git a/include/linux/timex.h b/include/linux/timex.h
index 32d852f..d23999f 100644
--- a/include/linux/timex.h
+++ b/include/linux/timex.h
@@ -268,6 +268,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 *, const struct timespec *);
 
 int read_current_timer(unsigned long *timer_val);
 
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index d232189..5c00242 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,162 @@ static 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. They establish the engineering parameters of the clock
+ * discipline loop when controlled by the PPS signal.
+ */
+#define PPS_VALID	10	/* PPS signal watchdog max (s) */
+#define PPS_POPCORN	4	/* popcorn spike threshold (shift) */
+#define PPS_INTMIN	2	/* min freq interval (s) (shift) */
+#define PPS_INTMAX	8	/* max freq interval (s) (shift) */
+#define PPS_INTCOUNT	4	/* number of consecutive good intervals to
+				   increase pps_shift or consecutive bad
+				   intervals to decrease it */
+#define PPS_MAXWANDER	100000	/* max PPS freq wander (ns/s) */
+
+static int pps_valid;		/* signal watchdog counter */
+static long pps_tf[3];		/* phase median filter */
+static long pps_jitter;		/* current jitter (ns) */
+static struct timespec pps_fbase; /* beginning of the last freq interval */
+static int pps_shift;		/* current interval duration (s) (shift) */
+static int pps_intcnt;		/* interval counter */
+static s64 pps_freq;		/* frequency offset (scaled ns/s) */
+static long pps_stabil;		/* current stability (scaled ns/s) */
+
+/*
+ * PPS signal quality monitors
+ */
+static long pps_calcnt;		/* calibration intervals */
+static long pps_jitcnt;		/* jitter limit exceeded */
+static long pps_stbcnt;		/* stability limit exceeded */
+static long pps_errcnt;		/* calibration errors */
+
+
+/* PPS kernel consumer compensates the whole phase error immediately.
+ * Otherwise, reduce the offset by a fixed factor times the time constant.
+ */
+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);
+}
+
+static inline void pps_reset_freq_interval(void)
+{
+	/* the PPS calibration interval may end
+	   surprisingly early */
+	pps_shift = PPS_INTMIN;
+	pps_intcnt = 0;
+}
+
+/**
+ * pps_clear - Clears the PPS state variables
+ *
+ * Must be called while holding a write on the xtime_lock
+ */
+static inline void pps_clear(void)
+{
+	pps_reset_freq_interval();
+	pps_tf[0] = 0;
+	pps_tf[1] = 0;
+	pps_tf[2] = 0;
+	pps_fbase.tv_sec = pps_fbase.tv_nsec = 0;
+	pps_freq = 0;
+}
+
+/* Decrease pps_valid to indicate that another second has passed since
+ * the last PPS signal. When it reaches 0, indicate that PPS signal is
+ * missing.
+ *
+ * Must be called while holding a write on the xtime_lock
+ */
+static inline void pps_dec_valid(void)
+{
+	if (pps_valid > 0)
+		pps_valid--;
+	else {
+		time_status &= ~(STA_PPSSIGNAL | STA_PPSJITTER |
+				 STA_PPSWANDER | STA_PPSERROR);
+		pps_clear();
+	}
+}
+
+static inline void pps_set_freq(s64 freq)
+{
+	pps_freq = freq;
+}
+
+static inline int is_error_status(int status)
+{
+	return (time_status & (STA_UNSYNC|STA_CLOCKERR))
+		/* 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)));
+}
+
+static inline void pps_fill_timex(struct timex *txc)
+{
+	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 */
+
+static inline s64 ntp_offset_chunk(s64 offset)
+{
+	return shift_right(offset, SHIFT_PLL + time_constant);
+}
+
+static inline void pps_reset_freq_interval(void) {}
+static inline void pps_clear(void) {}
+static inline void pps_dec_valid(void) {}
+static inline void pps_set_freq(s64 freq) {}
+
+static inline int is_error_status(int status)
+{
+	return status & (STA_UNSYNC|STA_CLOCKERR);
+}
+
+static inline void pps_fill_timex(struct timex *txc)
+{
+	/* PPS is not implemented, so these are zero */
+	txc->ppsfreq	   = 0;
+	txc->jitter	   = 0;
+	txc->shift	   = 0;
+	txc->stabil	   = 0;
+	txc->jitcnt	   = 0;
+	txc->calcnt	   = 0;
+	txc->errcnt	   = 0;
+	txc->stbcnt	   = 0;
+}
+
+#endif /* CONFIG_NTP_PPS */
+
 /*
  * NTP methods:
  */
@@ -185,6 +342,9 @@ void ntp_clear(void)
 
 	tick_length	= tick_length_base;
 	time_offset	= 0;
+
+	/* Clear PPS state variables */
+	pps_clear();
 }
 
 /*
@@ -250,16 +410,16 @@ void second_overflow(void)
 		time_status |= STA_UNSYNC;
 	}
 
-	/*
-	 * Compute the phase adjustment for the next second. The offset is
-	 * reduced by a fixed factor times the time constant.
-	 */
+	/* Compute the phase adjustment for the next second */
 	tick_length	 = tick_length_base;
 
-	delta		 = shift_right(time_offset, SHIFT_PLL + time_constant);
+	delta		 = ntp_offset_chunk(time_offset);
 	time_offset	-= delta;
 	tick_length	+= delta;
 
+	/* Check PPS signal */
+	pps_dec_valid();
+
 	if (!time_adjust)
 		return;
 
@@ -369,6 +529,8 @@ 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;
+		/* restart PPS frequency calibration */
+		pps_reset_freq_interval();
 	}
 
 	/*
@@ -418,6 +580,8 @@ 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);
+		/* update pps_freq */
+		pps_set_freq(time_freq);
 	}
 
 	if (txc->modes & ADJ_MAXERROR)
@@ -508,7 +672,8 @@ int do_adjtimex(struct timex *txc)
 	}
 
 	result = time_state;	/* mostly `TIME_OK' */
-	if (time_status & (STA_UNSYNC|STA_CLOCKERR))
+	/* check for errors */
+	if (is_error_status(time_status))
 		result = TIME_ERROR;
 
 	txc->freq	   = shift_right((time_freq >> PPM_SCALE_INV_SHIFT) *
@@ -522,15 +687,8 @@ int do_adjtimex(struct timex *txc)
 	txc->tick	   = tick_usec;
 	txc->tai	   = time_tai;
 
-	/* PPS is not implemented, so these are zero */
-	txc->ppsfreq	   = 0;
-	txc->jitter	   = 0;
-	txc->shift	   = 0;
-	txc->stabil	   = 0;
-	txc->jitcnt	   = 0;
-	txc->calcnt	   = 0;
-	txc->errcnt	   = 0;
-	txc->stbcnt	   = 0;
+	/* fill PPS status fields */
+	pps_fill_timex(txc);
 
 	write_sequnlock_irq(&xtime_lock);
 
@@ -544,6 +702,243 @@ int do_adjtimex(struct timex *txc)
 	return result;
 }
 
+#ifdef	CONFIG_NTP_PPS
+
+/* actually struct pps_normtime is good old struct timespec, but it is
+ * semantically different (and it is the reason why it was invented):
+ * pps_normtime.nsec has a range of ( -NSEC_PER_SEC / 2, NSEC_PER_SEC / 2 ]
+ * while timespec.tv_nsec has a range of [0, NSEC_PER_SEC) */
+struct pps_normtime {
+	__kernel_time_t	sec;	/* seconds */
+	long		nsec;	/* nanoseconds */
+};
+
+/* normalize the timestamp so that nsec is in the
+   ( -NSEC_PER_SEC / 2, NSEC_PER_SEC / 2 ] interval */
+static inline struct pps_normtime pps_normalize_ts(struct timespec ts)
+{
+	struct pps_normtime norm = {
+		.sec = ts.tv_sec,
+		.nsec = ts.tv_nsec
+	};
+
+	if (norm.nsec > (NSEC_PER_SEC >> 1)) {
+		norm.nsec -= NSEC_PER_SEC;
+		norm.sec++;
+	}
+
+	return norm;
+}
+
+/* get current phase correction and jitter */
+static inline long pps_phase_filter_get(long *jitter)
+{
+	*jitter = pps_tf[0] - pps_tf[1];
+	if (*jitter < 0)
+		*jitter = -*jitter;
+
+	/* TODO: test various filters */
+	return pps_tf[0];
+}
+
+/* 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.
+ * It is halved after four consecutive unstable intervals.
+ */
+static inline void pps_dec_freq_interval(void)
+{
+	if (--pps_intcnt <= -PPS_INTCOUNT) {
+		pps_intcnt = -PPS_INTCOUNT;
+		if (pps_shift > PPS_INTMIN) {
+			pps_shift--;
+			pps_intcnt = 0;
+		}
+	}
+}
+
+/* increase frequency calibration interval length.
+ * It is doubled after four consecutive stable intervals.
+ */
+static inline void pps_inc_freq_interval(void)
+{
+	if (++pps_intcnt >= PPS_INTCOUNT) {
+		pps_intcnt = PPS_INTCOUNT;
+		if (pps_shift < PPS_INTMAX) {
+			pps_shift++;
+			pps_intcnt = 0;
+		}
+	}
+}
+
+/* update clock frequency based on MONOTONIC_RAW clock PPS signal
+ * timestamps
+ *
+ * At the end of the calibration interval the difference between the
+ * first and last MONOTONIC_RAW clock timestamps divided by the length
+ * of the interval becomes the frequency update. If the interval was
+ * too long, the data are discarded.
+ * Returns the difference between old and new frequency values.
+ */
+static long hardpps_update_freq(struct pps_normtime freq_norm)
+{
+	long delta, delta_mod;
+	s64 ftemp;
+
+	/* check if the frequency interval was too long */
+	if (freq_norm.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.sec);
+		return 0;
+	}
+
+	/* here the raw frequency offset and wander (stability) is
+	 * calculated. If the wander is less than the wander threshold
+	 * the interval is increased; otherwise it is decreased.
+	 */
+	ftemp = div_s64(((s64)(-freq_norm.nsec)) << NTP_SCALE_SHIFT,
+			freq_norm.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 */
+		pps_inc_freq_interval();
+	}
+
+	/* the stability metric is calculated as the average of recent
+	 * frequency changes, but is used only for performance
+	 * monitoring
+	 */
+	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_INTMIN;
+
+	/* 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 REALTIME clock 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);
+	correction = pps_phase_filter_get(&jitter);
+
+	/* Nominal jitter is due to PPS signal noise. If it exceeds the
+	 * threshold, the sample is discarded; otherwise, if so enabled,
+	 * the time offset is updated.
+	 */
+	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_INTMIN;
+}
+
+/*
+ * hardpps() - discipline CPU clock oscillator to external PPS signal
+ *
+ * This routine is called at each PPS signal arrival in order to
+ * discipline the CPU clock oscillator to the PPS signal. It takes two
+ * parameters: REALTIME and MONOTONIC_RAW clock timestamps. The former
+ * is used to correct clock phase error and the latter is used to
+ * correct the frequency.
+ *
+ * This code is based on David Mills's reference nanokernel
+ * implementation. It was mostly rewritten but keeps the same idea.
+ */
+void hardpps(const struct timespec *phase_ts, const struct timespec *raw_ts)
+{
+	struct pps_normtime pts_norm, freq_norm;
+	unsigned long flags;
+
+	pts_norm = pps_normalize_ts(*phase_ts);
+
+	write_seqlock_irqsave(&xtime_lock, flags);
+
+	/* 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;
+
+	/* when called for the first time,
+	 * just start the frequency interval */
+	if (unlikely(pps_fbase.tv_sec == 0)) {
+		pps_fbase = *raw_ts;
+		write_sequnlock_irqrestore(&xtime_lock, flags);
+		return;
+	}
+
+	/* ok, now we have a base for frequency calculation */
+	freq_norm = pps_normalize_ts(timespec_sub(*raw_ts, pps_fbase));
+
+	/* check that the signal is in the range
+	 * [1s - MAXFREQ us, 1s + MAXFREQ us], otherwise reject it */
+	if ((freq_norm.sec == 0) ||
+			(freq_norm.nsec > MAXFREQ * freq_norm.sec) ||
+			(freq_norm.nsec < -MAXFREQ * freq_norm.sec)) {
+		time_status |= STA_PPSJITTER;
+		/* restart the frequency calibration interval */
+		pps_fbase = *raw_ts;
+		write_sequnlock_irqrestore(&xtime_lock, flags);
+		pr_err("hardpps: PPSJITTER: bad pulse\n");
+		return;
+	}
+
+	/* signal is ok */
+
+	/* check if the current frequency interval is finished */
+	if (freq_norm.sec >= (1 << pps_shift)) {
+		pps_calcnt++;
+		/* restart the frequency calibration interval */
+		pps_fbase = *raw_ts;
+		hardpps_update_freq(freq_norm);
+	}
+
+	hardpps_update_phase(pts_norm.nsec);
+
+	write_sequnlock_irqrestore(&xtime_lock, flags);
+}
+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.7.2.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCHv7 13/16] pps: capture MONOTONIC_RAW timestamps as well
  2010-12-20 11:54 ` [PATCHv7 00/16] changed some patches Alexander Gordeev
  2010-12-20 11:54   ` [PATCHv7 08/16] pps: make idr lock a mutex and protect idr_pre_get Alexander Gordeev
  2010-12-20 11:54   ` [PATCHv7 12/16] ntp: add hardpps implementation Alexander Gordeev
@ 2010-12-20 11:54   ` Alexander Gordeev
  2010-12-20 11:54   ` [PATCHv7 14/16] pps: add kernel consumer support Alexander Gordeev
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-20 11:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Nikita V. Youshchenko, linuxpps, Rodolfo Giometti, Andrew Morton,
	Alexander Gordeev, John Stultz, Thomas Gleixner, David Howells,
	H. Peter Anvin, Magnus Damm, Jason Wessel

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.

Acked-by: John Stultz <johnstul@us.ibm.com>
Signed-off-by: Alexander Gordeev <lasaine@lvk.cs.msu.su>
---
 include/linux/pps_kernel.h |   14 ++++++++++++++
 include/linux/time.h       |    2 ++
 kernel/time/timekeeping.c  |   43 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 59 insertions(+), 0 deletions(-)

diff --git a/include/linux/pps_kernel.h b/include/linux/pps_kernel.h
index 1aedf50..9404854 100644
--- a/include/linux/pps_kernel.h
+++ b/include/linux/pps_kernel.h
@@ -47,6 +47,9 @@ struct pps_source_info {
 };
 
 struct pps_event_time {
+#ifdef CONFIG_NTP_PPS
+	struct timespec ts_raw;
+#endif /* CONFIG_NTP_PPS */
 	struct timespec ts_real;
 };
 
@@ -97,10 +100,21 @@ static inline void timespec_to_pps_ktime(struct pps_ktime *kt,
 	kt->nsec = ts.tv_nsec;
 }
 
+#ifdef CONFIG_NTP_PPS
+
+static inline void pps_get_ts(struct pps_event_time *ts)
+{
+	getnstime_raw_and_real(&ts->ts_raw, &ts->ts_real);
+}
+
+#else /* CONFIG_NTP_PPS */
+
 static inline void pps_get_ts(struct pps_event_time *ts)
 {
 	getnstimeofday(&ts->ts_real);
 }
 
+#endif /* CONFIG_NTP_PPS */
+
 #endif /* LINUX_PPS_KERNEL_H */
 
diff --git a/include/linux/time.h b/include/linux/time.h
index 9f15ac7..1e6d3b5 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -158,6 +158,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 49010d8..5c4be30 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -284,6 +284,49 @@ void ktime_get_ts(struct timespec *ts)
 }
 EXPORT_SYMBOL_GPL(ktime_get_ts);
 
+#ifdef CONFIG_NTP_PPS
+
+/**
+ * getnstime_raw_and_real - get day and raw monotonic time in timespec format
+ * @ts_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
+ *
+ * This function reads both the time of day and raw monotonic time at the
+ * same time atomically and stores the resulting timestamps in timespec
+ * format.
+ */
+void getnstime_raw_and_real(struct timespec *ts_raw, struct timespec *ts_real)
+{
+	unsigned long seq;
+	s64 nsecs_raw, nsecs_real;
+
+	WARN_ON_ONCE(timekeeping_suspended);
+
+	do {
+		u32 arch_offset;
+
+		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() */
+		arch_offset = arch_gettimeoffset();
+		nsecs_raw += arch_offset;
+		nsecs_real += arch_offset;
+
+	} 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);
+
+#endif /* CONFIG_NTP_PPS */
+
 /**
  * do_gettimeofday - Returns the time of day in a timeval
  * @tv:		pointer to the timeval to be set
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCHv7 14/16] pps: add kernel consumer support
  2010-12-20 11:54 ` [PATCHv7 00/16] changed some patches Alexander Gordeev
                     ` (2 preceding siblings ...)
  2010-12-20 11:54   ` [PATCHv7 13/16] pps: capture MONOTONIC_RAW timestamps as well Alexander Gordeev
@ 2010-12-20 11:54   ` Alexander Gordeev
  2010-12-20 11:54   ` [PATCHv7 15/16] pps: add parallel port PPS client Alexander Gordeev
  2010-12-20 11:54   ` [PATCHv7 16/16] pps: add parallel port PPS signal generator Alexander Gordeev
  5 siblings, 0 replies; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-20 11:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Nikita V. Youshchenko, linuxpps, Rodolfo Giometti, Andrew Morton,
	Alexander Gordeev, Randy Dunlap, Greg Kroah-Hartman,
	Thomas Weber, Mike Frysinger, Stefan Richter, linux-doc

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

Signed-off-by: Alexander Gordeev <lasaine@lvk.cs.msu.su>
---
 Documentation/ioctl/ioctl-number.txt |    2 +-
 drivers/pps/Makefile                 |    1 +
 drivers/pps/kapi.c                   |    6 ++
 drivers/pps/kc.c                     |  123 ++++++++++++++++++++++++++++++++++
 drivers/pps/kc.h                     |   47 +++++++++++++
 drivers/pps/pps.c                    |   38 ++++++++++-
 include/linux/pps.h                  |    7 ++
 7 files changed, 222 insertions(+), 2 deletions(-)
 create mode 100644 drivers/pps/kc.c
 create mode 100644 drivers/pps/kc.h

diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
index 63ffd78..7081847 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -249,7 +249,7 @@ Code  Seq#(hex)	Include File		Comments
 'p'	40-7F	linux/nvram.h
 'p'	80-9F	linux/ppdev.h		user-space parport
 					<mailto:tim@cyberelk.net>
-'p'	A1-A4	linux/pps.h		LinuxPPS
+'p'	A1-A5	linux/pps.h		LinuxPPS
 					<mailto:giometti@linux.it>
 'q'	00-1F	linux/serio.h
 'q'	80-FF	linux/telephony.h	Internet PhoneJACK, Internet LineJACK
diff --git a/drivers/pps/Makefile b/drivers/pps/Makefile
index 98960dd..77c23457 100644
--- a/drivers/pps/Makefile
+++ b/drivers/pps/Makefile
@@ -3,6 +3,7 @@
 #
 
 pps_core-y			:= pps.o kapi.o sysfs.o
+pps_core-$(CONFIG_NTP_PPS)	+= kc.o
 obj-$(CONFIG_PPS)		:= pps_core.o
 obj-y				+= clients/
 
diff --git a/drivers/pps/kapi.c b/drivers/pps/kapi.c
index ebf3374..cba1b43 100644
--- a/drivers/pps/kapi.c
+++ b/drivers/pps/kapi.c
@@ -26,11 +26,14 @@
 #include <linux/init.h>
 #include <linux/sched.h>
 #include <linux/time.h>
+#include <linux/timex.h>
 #include <linux/spinlock.h>
 #include <linux/fs.h>
 #include <linux/pps_kernel.h>
 #include <linux/slab.h>
 
+#include "kc.h"
+
 /*
  * Local functions
  */
@@ -139,6 +142,7 @@ EXPORT_SYMBOL(pps_register_source);
 
 void pps_unregister_source(struct pps_device *pps)
 {
+	pps_kc_remove(pps);
 	pps_unregister_cdev(pps);
 
 	/* don't have to kfree(pps) here because it will be done on
@@ -211,6 +215,8 @@ void pps_event(struct pps_device *pps, struct pps_event_time *ts, int event,
 		captured = ~0;
 	}
 
+	pps_kc_event(pps, ts, event);
+
 	/* Wake up if captured something */
 	if (captured) {
 		pps->last_ev++;
diff --git a/drivers/pps/kc.c b/drivers/pps/kc.c
new file mode 100644
index 0000000..059a6c0
--- /dev/null
+++ b/drivers/pps/kc.c
@@ -0,0 +1,123 @@
+/*
+ * PPS kernel consumer API
+ *
+ * Copyright (C) 2009-2010   Alexander Gordeev <lasaine@lvk.cs.msu.su>
+ *
+ *   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.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/spinlock.h>
+#include <linux/pps_kernel.h>
+
+#include "kc.h"
+
+/*
+ * Global variables
+ */
+
+/* state variables to bind kernel consumer */
+DEFINE_SPINLOCK(pps_kc_hardpps_lock);
+/* PPS API (RFC 2783): current source and mode for kernel consumer */
+struct pps_device *pps_kc_hardpps_dev;	/* unique pointer to device */
+int pps_kc_hardpps_mode;		/* mode bits for kernel consumer */
+
+/* pps_kc_bind - control PPS kernel consumer binding
+ * @pps: the PPS source
+ * @bind_args: kernel consumer bind parameters
+ *
+ * This function is used to bind or unbind PPS kernel consumer according to
+ * supplied parameters. Should not be called in interrupt context.
+ */
+int pps_kc_bind(struct pps_device *pps, struct pps_bind_args *bind_args)
+{
+	/* Check if another consumer is already bound */
+	spin_lock_irq(&pps_kc_hardpps_lock);
+
+	if (bind_args->edge == 0)
+		if (pps_kc_hardpps_dev == pps) {
+			pps_kc_hardpps_mode = 0;
+			pps_kc_hardpps_dev = NULL;
+			spin_unlock_irq(&pps_kc_hardpps_lock);
+			dev_info(pps->dev, "unbound kernel"
+					" consumer\n");
+		} else {
+			spin_unlock_irq(&pps_kc_hardpps_lock);
+			dev_err(pps->dev, "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_irq(&pps_kc_hardpps_lock);
+			dev_info(pps->dev, "bound kernel consumer: "
+				"edge=0x%x\n", bind_args->edge);
+		} else {
+			spin_unlock_irq(&pps_kc_hardpps_lock);
+			dev_err(pps->dev, "another kernel consumer"
+					" is already bound\n");
+			return -EINVAL;
+		}
+
+	return 0;
+}
+
+/* pps_kc_remove - unbind kernel consumer on PPS source removal
+ * @pps: the PPS source
+ *
+ * This function is used to disable kernel consumer on PPS source removal
+ * if this source was bound to PPS kernel consumer. Can be called on any
+ * source safely. Should not be called in interrupt context.
+ */
+void pps_kc_remove(struct pps_device *pps)
+{
+	spin_lock_irq(&pps_kc_hardpps_lock);
+	if (pps == pps_kc_hardpps_dev) {
+		pps_kc_hardpps_mode = 0;
+		pps_kc_hardpps_dev = NULL;
+		spin_unlock_irq(&pps_kc_hardpps_lock);
+		dev_info(pps->dev, "unbound kernel consumer"
+				" on device removal\n");
+	} else
+		spin_unlock_irq(&pps_kc_hardpps_lock);
+}
+
+/* pps_kc_event - call hardpps() on PPS event
+ * @pps: the PPS source
+ * @ts: PPS event timestamp
+ * @event: PPS event edge
+ *
+ * This function calls hardpps() when an event from bound PPS source occurs.
+ */
+void pps_kc_event(struct pps_device *pps, struct pps_event_time *ts,
+		int event)
+{
+	unsigned long flags;
+
+	/* Pass some events to kernel consumer if activated */
+	spin_lock_irqsave(&pps_kc_hardpps_lock, flags);
+	if (pps == pps_kc_hardpps_dev && event & pps_kc_hardpps_mode)
+		hardpps(&ts->ts_real, &ts->ts_raw);
+	spin_unlock_irqrestore(&pps_kc_hardpps_lock, flags);
+}
+
diff --git a/drivers/pps/kc.h b/drivers/pps/kc.h
new file mode 100644
index 0000000..1cdcf75
--- /dev/null
+++ b/drivers/pps/kc.h
@@ -0,0 +1,47 @@
+/*
+ * PPS kernel consumer API header
+ *
+ * Copyright (C) 2009-2010   Alexander Gordeev <lasaine@lvk.cs.msu.su>
+ *
+ *   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.
+ */
+
+#ifndef LINUX_PPS_KC_H
+#define LINUX_PPS_KC_H
+
+#include <linux/errno.h>
+#include <linux/pps_kernel.h>
+
+#ifdef CONFIG_NTP_PPS
+
+extern int pps_kc_bind(struct pps_device *pps,
+		struct pps_bind_args *bind_args);
+extern void pps_kc_remove(struct pps_device *pps);
+extern void pps_kc_event(struct pps_device *pps,
+		struct pps_event_time *ts, int event);
+
+
+#else /* CONFIG_NTP_PPS */
+
+static inline int pps_kc_bind(struct pps_device *pps,
+		struct pps_bind_args *bind_args) { return -EOPNOTSUPP; }
+static inline void pps_kc_remove(struct pps_device *pps) {}
+static inline void pps_kc_event(struct pps_device *pps,
+		struct pps_event_time *ts, int event) {}
+
+#endif /* CONFIG_NTP_PPS */
+
+#endif /* LINUX_PPS_KC_H */
+
diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index 9e15cf1..2baadd2 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -33,6 +33,8 @@
 #include <linux/pps_kernel.h>
 #include <linux/slab.h>
 
+#include "kc.h"
+
 /*
  * Local variables
  */
@@ -198,9 +200,43 @@ static long pps_cdev_ioctl(struct file *file,
 
 		break;
 	}
+	case PPS_KC_BIND: {
+		struct pps_bind_args bind_args;
+
+		dev_dbg(pps->dev, "PPS_KC_BIND\n");
+
+		/* 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) {
+			dev_err(pps->dev, "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) {
+			dev_err(pps->dev, "invalid kernel consumer bind"
+					" parameters (%x)\n", bind_args.edge);
+			return -EINVAL;
+		}
+
+		err = pps_kc_bind(pps, &bind_args);
+		if (err < 0)
+			return err;
+
+		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_ */
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCHv7 15/16] pps: add parallel port PPS client
  2010-12-20 11:54 ` [PATCHv7 00/16] changed some patches Alexander Gordeev
                     ` (3 preceding siblings ...)
  2010-12-20 11:54   ` [PATCHv7 14/16] pps: add kernel consumer support Alexander Gordeev
@ 2010-12-20 11:54   ` Alexander Gordeev
  2010-12-20 11:54   ` [PATCHv7 16/16] pps: add parallel port PPS signal generator Alexander Gordeev
  5 siblings, 0 replies; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-20 11:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Nikita V. Youshchenko, linuxpps, Rodolfo Giometti, Andrew Morton,
	Alexander Gordeev

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.
Clear edge capture could be disabled using clear_wait parameter.

Acked-by: Rodolfo Giometti <giometti@linux.it>
Signed-off-by: Alexander Gordeev <lasaine@lvk.cs.msu.su>
---
 drivers/pps/clients/Kconfig       |    7 +
 drivers/pps/clients/Makefile      |    1 +
 drivers/pps/clients/pps_parport.c |  258 +++++++++++++++++++++++++++++++++++++
 3 files changed, 266 insertions(+), 0 deletions(-)
 create mode 100644 drivers/pps/clients/pps_parport.c

diff --git a/drivers/pps/clients/Kconfig b/drivers/pps/clients/Kconfig
index 4e801bd..8520a7f 100644
--- a/drivers/pps/clients/Kconfig
+++ b/drivers/pps/clients/Kconfig
@@ -22,4 +22,11 @@ config PPS_CLIENT_LDISC
 	  If you say yes here you get support for a PPS source connected
 	  with the CD (Carrier Detect) pin of your serial port.
 
+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
index 812c9b1..42517da 100644
--- a/drivers/pps/clients/Makefile
+++ b/drivers/pps/clients/Makefile
@@ -4,6 +4,7 @@
 
 obj-$(CONFIG_PPS_CLIENT_KTIMER)	+= pps-ktimer.o
 obj-$(CONFIG_PPS_CLIENT_LDISC)	+= pps-ldisc.o
+obj-$(CONFIG_PPS_CLIENT_PARPORT) += pps_parport.o
 
 ifeq ($(CONFIG_PPS_DEBUG),y)
 EXTRA_CFLAGS += -DDEBUG
diff --git a/drivers/pps/clients/pps_parport.c b/drivers/pps/clients/pps_parport.c
new file mode 100644
index 0000000..32221ef
--- /dev/null
+++ b/drivers/pps/clients/pps_parport.c
@@ -0,0 +1,258 @@
+/*
+ * pps_parport.c -- kernel parallel port PPS client
+ *
+ *
+ * Copyright (C) 2009   Alexander Gordeev <lasaine@lvk.cs.msu.su>
+ *
+ * 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:
+ * implement echo over SEL pin
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#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 DRVDESC "parallel port PPS client"
+
+/* module parameters */
+
+#define CLEAR_WAIT_MAX		100
+#define CLEAR_WAIT_MAX_ERRORS	5
+
+static unsigned int clear_wait = 100;
+MODULE_PARM_DESC(clear_wait,
+	"Maximum number of port reads when polling for signal clear,"
+	" zero turns clear edge capture off entirely");
+module_param(clear_wait, uint, 0);
+
+
+/* internal per port structure */
+struct pps_client_pp {
+	struct pardevice *pardev;	/* parport device */
+	struct pps_device *pps;		/* PPS device */
+	unsigned int cw;		/* port clear timeout */
+	unsigned int cw_err;		/* number of timeouts */
+};
+
+static inline int signal_is_set(struct parport *port)
+{
+	return (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;
+	unsigned int i;
+	unsigned long flags;
+
+	/* first of all we get the time stamp... */
+	pps_get_ts(&ts_assert);
+
+	if (dev->cw == 0)
+		/* clear edge capture disabled */
+		goto out_assert;
+
+	/* try capture the clear edge */
+
+	/* We have to disable interrupts here. The idea is to prevent
+	 * other interrupts on the same processor to introduce random
+	 * lags while polling the port. Reading from IO port is known
+	 * to take approximately 1us while other interrupt handlers can
+	 * take much more potentially.
+	 *
+	 * Interrupts won't be disabled for a long time because the
+	 * number of polls is limited by clear_wait parameter which is
+	 * kept rather low. So it should never be an issue.
+	 */
+	local_irq_save(flags);
+	/* check the signal (no signal means the pulse is lost this time) */
+	if (!signal_is_set(port)) {
+		local_irq_restore(flags);
+		dev_err(dev->pps->dev, "lost the signal\n");
+		goto out_assert;
+	}
+
+	/* poll the port until the signal is unset */
+	for (i = dev->cw; i; i--)
+		if (!signal_is_set(port)) {
+			pps_get_ts(&ts_clear);
+			local_irq_restore(flags);
+			dev->cw_err = 0;
+			goto out_both;
+		}
+	local_irq_restore(flags);
+
+	/* timeout */
+	dev->cw_err++;
+	if (dev->cw_err >= CLEAR_WAIT_MAX_ERRORS) {
+		dev_err(dev->pps->dev, "disabled clear edge capture after %d"
+				" timeouts\n", dev->cw_err);
+		dev->cw = 0;
+		dev->cw_err = 0;
+	}
+
+out_assert:
+	/* fire assert event */
+	pps_event(dev->pps, &ts_assert,
+			PPS_CAPTUREASSERT, NULL);
+	return;
+
+out_both:
+	/* fire assert event */
+	pps_event(dev->pps, &ts_assert,
+			PPS_CAPTUREASSERT, NULL);
+	/* fire clear event */
+	pps_event(dev->pps, &ts_clear,
+			PPS_CAPTURECLEAR, NULL);
+	return;
+}
+
+/* the PPS echo function */
+static void pps_echo(struct pps_device *pps, int event, void *data)
+{
+	dev_info(pps->dev, "echo %s %s\n",
+		event & PPS_CAPTUREASSERT ? "assert" : "",
+		event & PPS_CAPTURECLEAR ? "clear" : "");
+}
+
+static void parport_attach(struct parport *port)
+{
+	struct pps_client_pp *device;
+	struct pps_source_info info = {
+		.name		= KBUILD_MODNAME,
+		.path		= "",
+		.mode		= PPS_CAPTUREBOTH | \
+				  PPS_OFFSETASSERT | PPS_OFFSETCLEAR | \
+				  PPS_ECHOASSERT | PPS_ECHOCLEAR | \
+				  PPS_CANWAIT | PPS_TSFMT_TSPEC,
+		.echo		= pps_echo,
+		.owner		= THIS_MODULE,
+		.dev		= NULL
+	};
+
+	device = kzalloc(sizeof(struct pps_client_pp), GFP_KERNEL);
+	if (!device) {
+		pr_err("memory allocation failed, not attaching\n");
+		return;
+	}
+
+	device->pardev = parport_register_device(port, KBUILD_MODNAME,
+			NULL, NULL, parport_irq, 0, device);
+	if (!device->pardev) {
+		pr_err("couldn't register with %s\n", port->name);
+		goto err_free;
+	}
+
+	if (parport_claim_or_block(device->pardev) < 0) {
+		pr_err("couldn't claim %s\n", port->name);
+		goto err_unregister_dev;
+	}
+
+	device->pps = pps_register_source(&info,
+			PPS_CAPTUREBOTH | PPS_OFFSETASSERT | PPS_OFFSETCLEAR);
+	if (device->pps == NULL) {
+		pr_err("couldn't register PPS source\n");
+		goto err_release_dev;
+	}
+
+	device->cw = clear_wait;
+
+	port->ops->enable_irq(port);
+
+	pr_info("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;
+
+	/* FIXME: oooh, this is ugly! */
+	if (strcmp(pardev->name, KBUILD_MODNAME))
+		/* not our port */
+		return;
+
+	device = pardev->private;
+
+	port->ops->disable_irq(port);
+	pps_unregister_source(device->pps);
+	parport_release(pardev);
+	parport_unregister_device(pardev);
+	kfree(device);
+}
+
+static struct parport_driver pps_parport_driver = {
+	.name = KBUILD_MODNAME,
+	.attach = parport_attach,
+	.detach = parport_detach,
+};
+
+/* module staff */
+
+static int __init pps_parport_init(void)
+{
+	int ret;
+
+	pr_info(DRVDESC "\n");
+
+	if (clear_wait > CLEAR_WAIT_MAX) {
+		pr_err("clear_wait value should be not greater"
+				" then %d\n", CLEAR_WAIT_MAX);
+		return -EINVAL;
+	}
+
+	ret = parport_register_driver(&pps_parport_driver);
+	if (ret) {
+		pr_err("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 <lasaine@lvk.cs.msu.su>");
+MODULE_DESCRIPTION(DRVDESC);
+MODULE_LICENSE("GPL");
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCHv7 16/16] pps: add parallel port PPS signal generator
  2010-12-20 11:54 ` [PATCHv7 00/16] changed some patches Alexander Gordeev
                     ` (4 preceding siblings ...)
  2010-12-20 11:54   ` [PATCHv7 15/16] pps: add parallel port PPS client Alexander Gordeev
@ 2010-12-20 11:54   ` Alexander Gordeev
  2010-12-24  0:35     ` Andrew Morton
  5 siblings, 1 reply; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-20 11:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: Nikita V. Youshchenko, linuxpps, Rodolfo Giometti, Andrew Morton,
	Alexander Gordeev, Randy Dunlap, linux-doc

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.

Acked-by: Rodolfo Giometti <giometti@linux.it>
Signed-off-by: Alexander Gordeev <lasaine@lvk.cs.msu.su>
---
 Documentation/pps/pps.txt                |   46 +++++
 drivers/pps/Kconfig                      |    2 +
 drivers/pps/Makefile                     |    2 +-
 drivers/pps/generators/Kconfig           |   13 ++
 drivers/pps/generators/Makefile          |    9 +
 drivers/pps/generators/pps_gen_parport.c |  282 ++++++++++++++++++++++++++++++
 6 files changed, 353 insertions(+), 1 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/Documentation/pps/pps.txt b/Documentation/pps/pps.txt
index 125f4ab..d35dcdd 100644
--- a/Documentation/pps/pps.txt
+++ b/Documentation/pps/pps.txt
@@ -170,3 +170,49 @@ and the run ppstest as follow:
 
 Please, note that to compile userland programs you need the file timepps.h
 (see Documentation/pps/).
+
+
+Generators
+----------
+
+Sometimes one needs to be able not only to catch PPS signals but to produce
+them also. For example, running a distributed simulation, which requires
+computers' clock to be synchronized very tightly. One way to do this is to
+invent some complicated hardware solutions but it may be neither necessary
+nor affordable. The cheap way is to load a PPS generator on one of the
+computers (master) and PPS clients on others (slaves), and use very simple
+cables to deliver signals using parallel ports, for example.
+
+Parallel port cable pinout:
+pin	name	master      slave
+1	STROBE	  *------     *
+2	D0	  *     |     *
+3	D1	  *     |     *
+4	D2	  *     |     *
+5	D3	  *     |     *
+6	D4	  *     |     *
+7	D5	  *     |     *
+8	D6	  *     |     *
+9	D7	  *     |     *
+10	ACK	  *     ------*
+11	BUSY	  *           *
+12	PE	  *           *
+13	SEL	  *           *
+14	AUTOFD	  *           *
+15	ERROR	  *           *
+16	INIT	  *           *
+17	SELIN	  *           *
+18-25	GND	  *-----------*
+
+Please note that parallel port interrupt occurs only on high->low transition,
+so it is used for PPS assert edge. PPS clear edge can be determined only
+using polling in the interrupt handler which actually can be done way more
+precisely because interrupt handling delays can be quite big and random. So
+current parport PPS generator implementation (pps_gen_parport module) is
+geared towards using the clear edge for time synchronization.
+
+Clear edge polling is done with disabled interrupts so it's better to select
+delay between assert and clear edge as small as possible to reduce system
+latencies. But if it is too small slave won't be able to capture clear edge
+transition. The default of 30us should be good enough in most situations.
+The delay can be selected using 'delay' pps_gen_parport module parameter.
diff --git a/drivers/pps/Kconfig b/drivers/pps/Kconfig
index 0ad5ff3..f0d3376 100644
--- a/drivers/pps/Kconfig
+++ b/drivers/pps/Kconfig
@@ -41,4 +41,6 @@ config NTP_PPS
 
 source drivers/pps/clients/Kconfig
 
+source drivers/pps/generators/Kconfig
+
 endmenu
diff --git a/drivers/pps/Makefile b/drivers/pps/Makefile
index 77c23457..4483eaa 100644
--- a/drivers/pps/Makefile
+++ b/drivers/pps/Makefile
@@ -5,6 +5,6 @@
 pps_core-y			:= pps.o kapi.o sysfs.o
 pps_core-$(CONFIG_NTP_PPS)	+= kc.o
 obj-$(CONFIG_PPS)		:= pps_core.o
-obj-y				+= clients/
+obj-y				+= clients/ generators/
 
 ccflags-$(CONFIG_PPS_DEBUG) := -DDEBUG
diff --git a/drivers/pps/generators/Kconfig b/drivers/pps/generators/Kconfig
new file mode 100644
index 0000000..f3a73dd
--- /dev/null
+++ b/drivers/pps/generators/Kconfig
@@ -0,0 +1,13 @@
+#
+# PPS generators configuration
+#
+
+comment "PPS generators support"
+
+config PPS_GENERATOR_PARPORT
+	tristate "Parallel port PPS signal generator"
+	depends on PARPORT
+	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.
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..92bf7b5
--- /dev/null
+++ b/drivers/pps/generators/pps_gen_parport.c
@@ -0,0 +1,282 @@
+/*
+ * pps_gen_parport.c -- kernel parallel port PPS signal generator
+ *
+ *
+ * Copyright (C) 2009   Alexander Gordeev <lasaine@lvk.cs.msu.su>
+ *
+ * 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:
+ * fix issues when realtime clock is adjusted in a leap
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#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 DRVDESC "parallel port PPS signal generator"
+
+#define SIGNAL		0
+#define NO_SIGNAL	PARPORT_CONTROL_STROBE
+
+/* module parameters */
+
+#define SEND_DELAY_MAX		100000
+
+static unsigned int send_delay = 30000;
+MODULE_PARM_DESC(delay,
+	"Delay between setting and dropping the signal (ns)");
+module_param_named(delay, send_delay, uint, 0);
+
+
+#define SAFETY_INTERVAL	3000	/* 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;
+
+	/* We have to disable interrupts here. The idea is to prevent
+	 * other interrupts on the same processor to introduce random
+	 * lags while polling the clock. getnstimeofday() takes <1us on
+	 * most machines while other interrupt handlers can take much
+	 * more potentially.
+	 *
+	 * NB: approx time with blocked interrupts =
+	 * send_delay + 3 * 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("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 + SAFETY_INTERVAL +
+				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("port write takes %ldns\n", dev->port_write_time);
+}
+
+static inline 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 + 3 * SAFETY_INTERVAL));
+}
+
+static void parport_attach(struct parport *port)
+{
+	if (attached) {
+		/* we already have a port */
+		return;
+	}
+
+	device.pardev = parport_register_device(port, KBUILD_MODNAME,
+			NULL, NULL, NULL, 0, &device);
+	if (!device.pardev) {
+		pr_err("couldn't register with %s\n", port->name);
+		return;
+	}
+
+	if (parport_claim_or_block(device.pardev) < 0) {
+		pr_err("couldn't claim %s\n", port->name);
+		goto err_unregister_dev;
+	}
+
+	pr_info("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)
+		return;	/* not our port */
+
+	hrtimer_cancel(&device.timer);
+	parport_release(device.pardev);
+	parport_unregister_device(device.pardev);
+}
+
+static struct parport_driver pps_gen_parport_driver = {
+	.name = KBUILD_MODNAME,
+	.attach = parport_attach,
+	.detach = parport_detach,
+};
+
+/* module staff */
+
+static int __init pps_gen_parport_init(void)
+{
+	int ret;
+
+	pr_info(DRVDESC "\n");
+
+	if (send_delay > SEND_DELAY_MAX) {
+		pr_err("delay value should be not greater"
+				" then %d\n", SEND_DELAY_MAX);
+		return -EINVAL;
+	}
+
+	ret = parport_register_driver(&pps_gen_parport_driver);
+	if (ret) {
+		pr_err("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("hrtimer avg error is %ldns\n", hrtimer_error);
+}
+
+module_init(pps_gen_parport_init);
+module_exit(pps_gen_parport_exit);
+
+MODULE_AUTHOR("Alexander Gordeev <lasaine@lvk.cs.msu.su>");
+MODULE_DESCRIPTION(DRVDESC);
+MODULE_LICENSE("GPL");
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: [PATCHv7 16/16] pps: add parallel port PPS signal generator
  2010-12-20 11:54   ` [PATCHv7 16/16] pps: add parallel port PPS signal generator Alexander Gordeev
@ 2010-12-24  0:35     ` Andrew Morton
  2010-12-24  1:37       ` Alexander Gordeev
  0 siblings, 1 reply; 37+ messages in thread
From: Andrew Morton @ 2010-12-24  0:35 UTC (permalink / raw)
  To: Alexander Gordeev
  Cc: linux-kernel, Nikita V. Youshchenko, linuxpps, Rodolfo Giometti,
	Randy Dunlap, linux-doc, john stultz

On Mon, 20 Dec 2010 14:54:56 +0300
Alexander Gordeev <lasaine@lvk.cs.msu.su> wrote:

> +	expire_time = ktime_to_timespec(timer->_expires);

It's bad to play around with internal fields, especially when an
accessor API was provided to access those fields.

And lo, hrtimers got changed in linux-next:

drivers/pps/generators/pps_gen_parport.c: In function 'hrtimer_event':
drivers/pps/generators/pps_gen_parport.c:92: error: 'struct hrtimer' has no member named '_expires'

The documentation in hrtimer.h is of course complete crap and
Documentation/timers/hrtimers.txt appears to be bitrot, but with a little
sleuthing and guesswork, it seems that what you want here is



--- a/drivers/pps/generators/pps_gen_parport.c~pps-add-parallel-port-pps-signal-generator-fix
+++ a/drivers/pps/generators/pps_gen_parport.c
@@ -82,7 +82,7 @@ static enum hrtimer_restart hrtimer_even
 
 	/* first of all we get the time stamp... */
 	getnstimeofday(&ts1);
-	expire_time = ktime_to_timespec(timer->_expires);
+	expire_time = ktime_to_timespec(hrtimer_get_softexpires(timer));
 	dev = container_of(timer, struct pps_generator_pp, timer);
 	lim = NSEC_PER_SEC - send_delay - dev->port_write_time;
 
_


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCHv7 16/16] pps: add parallel port PPS signal generator
  2010-12-24  0:35     ` Andrew Morton
@ 2010-12-24  1:37       ` Alexander Gordeev
  0 siblings, 0 replies; 37+ messages in thread
From: Alexander Gordeev @ 2010-12-24  1:37 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel, Nikita V. Youshchenko, linuxpps, Rodolfo Giometti,
	Randy Dunlap, linux-doc, john stultz

[-- Attachment #1: Type: text/plain, Size: 1419 bytes --]

В Thu, 23 Dec 2010 16:35:49 -0800
Andrew Morton <akpm@linux-foundation.org> пишет:

> On Mon, 20 Dec 2010 14:54:56 +0300
> Alexander Gordeev <lasaine@lvk.cs.msu.su> wrote:
> 
> > +	expire_time = ktime_to_timespec(timer->_expires);
> 
> It's bad to play around with internal fields, especially when an
> accessor API was provided to access those fields.
> 
> And lo, hrtimers got changed in linux-next:
> 
> drivers/pps/generators/pps_gen_parport.c: In function 'hrtimer_event':
> drivers/pps/generators/pps_gen_parport.c:92: error: 'struct hrtimer'
> has no member named '_expires'
> 
> The documentation in hrtimer.h is of course complete crap and
> Documentation/timers/hrtimers.txt appears to be bitrot, but with a
> little sleuthing and guesswork, it seems that what you want here is

Ah, cool, thanks for the fix!

> ---
> a/drivers/pps/generators/pps_gen_parport.c~pps-add-parallel-port-pps-signal-generator-fix
> +++ a/drivers/pps/generators/pps_gen_parport.c @@ -82,7 +82,7 @@
> static enum hrtimer_restart hrtimer_even 
>  	/* first of all we get the time stamp... */
>  	getnstimeofday(&ts1);
> -	expire_time = ktime_to_timespec(timer->_expires);
> +	expire_time =
> ktime_to_timespec(hrtimer_get_softexpires(timer)); dev =
> container_of(timer, struct pps_generator_pp, timer); lim =
> NSEC_PER_SEC - send_delay - dev->port_write_time; 
> _
> 


-- 
  Alexander

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply	[flat|nested] 37+ messages in thread

end of thread, other threads:[~2010-12-24  1:37 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-12-17 19:54 [PATCHv6 00/16] pps: several fixes and improvements Alexander Gordeev
2010-12-17 19:54 ` [PATCHv6 01/16] pps: trivial fixes Alexander Gordeev
2010-12-17 19:54 ` [PATCHv6 02/16] pps: declare variables where they are used in switch Alexander Gordeev
2010-12-17 19:54 ` [PATCHv6 03/16] pps: fix race in PPS_FETCH handler Alexander Gordeev
2010-12-17 19:54 ` [PATCHv6 04/16] pps: unify timestamp gathering Alexander Gordeev
2010-12-17 19:54 ` [PATCHv6 05/16] pps: access pps device by direct pointer Alexander Gordeev
2010-12-17 19:54 ` [PATCHv6 06/16] pps: convert printk/pr_* to dev_* Alexander Gordeev
2010-12-17 19:54 ` [PATCHv6 07/16] pps: move idr stuff to pps.c Alexander Gordeev
2010-12-18  0:13   ` Andrew Morton
2010-12-18  1:07     ` Alexander Gordeev
2010-12-18  1:18       ` Andrew Morton
2010-12-17 19:54 ` [PATCHv6 08/16] pps: do not disable interrupts for idr operations Alexander Gordeev
2010-12-17 19:54 ` [PATCHv6 09/16] pps: use BUG_ON for kernel API safety checks Alexander Gordeev
2010-12-17 19:54 ` [PATCHv6 10/16] pps: simplify conditions a bit Alexander Gordeev
2010-12-17 19:54 ` [PATCHv6 11/16] pps: timestamp is always passed to dcd_change() Alexander Gordeev
2010-12-17 19:54 ` [PATCHv6 12/16] ntp: add hardpps implementation Alexander Gordeev
2010-12-17 19:54 ` [PATCHv6 13/16] pps: capture MONOTONIC_RAW timestamps as well Alexander Gordeev
2010-12-17 19:54 ` [PATCHv6 14/16] pps: add kernel consumer support Alexander Gordeev
2010-12-17 19:54 ` [PATCHv6 15/16] pps: add parallel port PPS client Alexander Gordeev
2010-12-18  0:17   ` Andrew Morton
2010-12-18  0:50     ` Alexander Gordeev
2010-12-18  1:13       ` Andrew Morton
2010-12-17 19:54 ` [PATCHv6 16/16] pps: add parallel port PPS signal generator Alexander Gordeev
2010-12-18  0:18   ` Andrew Morton
2010-12-18  0:52     ` Alexander Gordeev
2010-12-18  0:19 ` [PATCHv6 00/16] pps: several fixes and improvements Andrew Morton
2010-12-18  1:00   ` Alexander Gordeev
2010-12-18  1:14     ` Andrew Morton
2010-12-20 11:54 ` [PATCHv7 00/16] changed some patches Alexander Gordeev
2010-12-20 11:54   ` [PATCHv7 08/16] pps: make idr lock a mutex and protect idr_pre_get Alexander Gordeev
2010-12-20 11:54   ` [PATCHv7 12/16] ntp: add hardpps implementation Alexander Gordeev
2010-12-20 11:54   ` [PATCHv7 13/16] pps: capture MONOTONIC_RAW timestamps as well Alexander Gordeev
2010-12-20 11:54   ` [PATCHv7 14/16] pps: add kernel consumer support Alexander Gordeev
2010-12-20 11:54   ` [PATCHv7 15/16] pps: add parallel port PPS client Alexander Gordeev
2010-12-20 11:54   ` [PATCHv7 16/16] pps: add parallel port PPS signal generator Alexander Gordeev
2010-12-24  0:35     ` Andrew Morton
2010-12-24  1:37       ` Alexander Gordeev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).