>Number:         13702
>Category:       kern
>Synopsis:       kernel hardpps support is not working
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Aug 13 01:30:00 PDT 2001
>Closed-Date:
>Last-Modified:
>Originator:     Charles Carvalho
>Release:        NetBSD 1.5.1
>Organization:
	self
>Environment:
	
System: NetBSD carvalho-home-ss20 1.5.1 NetBSD 1.5.1 (SUN4M) #0: Sun Aug 12 23:17:22 PDT 2001 root@carvalho-home-ss20:/usr/src/sys/arch/sparc/compile/SUN4M sparc


>Description:
	Uncommenting "options PPS_SYNC" in the config file is not sufficient
	to enable the kernel PPS support.  Diffs are provided below that
	enable PPS support for the generic z8530 driver and correct an
	error that prevent applications from successfully binding a PPS
	source.
>How-To-Repeat:
	Configure ntpd to enable kernel PPS support; observe the error
	message ("refclock_ioctl: time_pps_kcbind failed: Bad address").
	After fixing the API error in timepps.h and rebuilding, observe
	that attempts to specify a terminal interface as the PPS source
	now fail with "Operation not supported on device".
>Fix:
The following changes are required:

in dev/ic/z8530tty.c:

a) opt_ntp.h must be included to enable the conditional compilation under
the PPS_SYNC conditional

b) existing references to PPS_HARDPPSONASSERT and PPS_HARDPPSONCLEAR
were removed: rather than adding extra bits to the per-device mode
word, one global mode word (pps_kc_hardpps_mode) is used. Its value
may be zero, PPS_CAPTUREASSERT, PPS_CAPTURECLEAR, or PPS_CAPTUREBOTH.
(This addresses the "XXX revoke any previous HARDPPS source" comments
in the code being removed).

c) support for the PPS_IOC_KCBIND ioctl was added to the z8530
driver.  If the new edge value is zero (unbind), the binding is
removed only if the current device is bound.  If the new edge value
is nonzero, any current binding is replaced; the address of the
"zstty_softc" structure for this terminal is saved as the unique
identifier in pps_kc_hardpps_source, as it is readily available to
the interrupt routine.

d) when the DCD line changes state, we compare the global PPS source
ID against the current device's zstty_softc pointer; if they match,
and the the current edge transition is flagged as interesting, the
hardpps routine is invoked.

Note that the current binding is not removed on the first open or
the last close of the device, although the per-device mask and mode
are cleared at these times, which will prevent the PPS signal from
being recognized until another PPS_IOC_SETPARAMS is issued for the
device; according to RFC 2873, section 3.4.4, the binding is supposed
to be unaffected by changes to device's mode bits by the user, so
this still isn't quite right, but does work.  (We're also ignoring
the kernel_consumer and tsformat arguments to time_pps_kcbind)

in kern/kern_clock.c:

global variables pps_kc_hardpps_source and pps_kc_hardpps_mode were
added.  (The PPSkit 0.9.2 changes for Linux made the source a pointer
to struct tty; I used pointer to void instead, since the value is
never dereferenced and this does not preclude implementing a non-tty
clock source, such as via a parallel port)

in sys/systm.h:

added externs for pps_kc_hardpps_source and pps_kc_hardpps_mode.  (I
don't know if this is really the best place for them, but I didn't
see any better places when checking for include files using PPS_SYNC)

in sys/timepps.h (and /usr/include/sys/timepps.h):

changed the ioctl definition to indicate that the parameter is not
modified, since it's defined by RFC 2783 as const, and passed its
address rather than its value to fix the "bad address" error.

These changes have been running successfully on my SS20 under NetBSD
1.4.2 with an Oncore VP GPS for many months; the updated patches
below are for NetBSD 1.5.1.

Actual diffs, in unidiff format:

--- dev/ic/z8530tty.c.orig	Fri Mar 16 11:46:52 2001
+++ dev/ic/z8530tty.c	Sun Aug 12 21:35:29 2001
@@ -98,6 +98,8 @@
  * fixing *many* bugs, and substantially improving performance.
  */
 
+#include "opt_ntp.h"	/* for "options PPS_SYNC" */
+
 #include <sys/param.h>
 #include <sys/systm.h>
 #include <sys/proc.h>
@@ -142,9 +144,6 @@
 	PPS_TSFMT_TSPEC |
 	PPS_CAPTUREASSERT |
 	PPS_CAPTURECLEAR |
-#ifdef  PPS_SYNC
-	PPS_HARDPPSONASSERT | PPS_HARDPPSONCLEAR |
-#endif	/* PPS_SYNC */
 	PPS_OFFSETASSERT | PPS_OFFSETCLEAR;
 
 struct zstty_softc {
@@ -788,16 +787,6 @@
 		 * compute masks from user-specified timestamp state.
 		 */
 		mode = zst->ppsparam.mode;
-#ifdef	PPS_SYNC
-		if (mode & PPS_HARDPPSONASSERT) {
-			mode |= PPS_CAPTUREASSERT;
-			/* XXX revoke any previous HARDPPS source */
-		}
-		if (mode & PPS_HARDPPSONCLEAR) {
-			mode |= PPS_CAPTURECLEAR;
-			/* XXX revoke any previous HARDPPS source */
-		}
-#endif	/* PPS_SYNC */
 		switch (mode & PPS_CAPTUREBOTH) {
 		case 0:
 			zst->zst_ppsmask = 0;
@@ -857,6 +846,31 @@
 		break;
 	}
 
+#ifdef PPS_SYNC
+	case PPS_IOC_KCBIND: {
+		int edge = (*(int *)data) & PPS_CAPTUREBOTH;
+		if (edge == 0) {
+			/*
+			 * remove binding for this source; ignore
+			 * the request if this is not the current
+			 * hardpps source
+			 */
+			if (pps_kc_hardpps_source == zst) {
+				pps_kc_hardpps_source = NULL;
+				pps_kc_hardpps_mode = 0;
+			}
+		} else {
+			/*
+			 * bind hardpps to this source, replacing any
+			 * previously specified source or edges
+			 */
+			pps_kc_hardpps_source = zst;
+			pps_kc_hardpps_mode = edge;
+		}
+		break;
+	}
+#endif /* PPS_SYNC */
+
 	case TIOCDCDTIMESTAMP:	/* XXX old, overloaded  API used by xntpd v3 */
 		if (cs->cs_rr0_pps == 0) {
 			error = EINVAL;
@@ -1534,8 +1548,10 @@
 				}
 
 #ifdef PPS_SYNC
-				if (zst->ppsparam.mode & PPS_HARDPPSONASSERT)
+				if (pps_kc_hardpps_source == zst &&
+				    pps_kc_hardpps_mode & PPS_CAPTUREASSERT) {
 					hardpps(&tv, tv.tv_usec);
+				}
 #endif
 				zst->ppsinfo.assert_sequence++;
 				zst->ppsinfo.current_mode = zst->ppsparam.mode;
@@ -1552,8 +1568,10 @@
 				}
 
 #ifdef PPS_SYNC
-				if (zst->ppsparam.mode & PPS_HARDPPSONCLEAR)
+				if (pps_kc_hardpps_source == zst &&
+				    pps_kc_hardpps_mode & PPS_CAPTURECLEAR) {
 					hardpps(&tv, tv.tv_usec);
+				}
 #endif
 				zst->ppsinfo.clear_sequence++;
 				zst->ppsinfo.current_mode = zst->ppsparam.mode;
--- kern/kern_clock.c.orig	Thu Jul 13 13:12:18 2000
+++ kern/kern_clock.c	Sun Aug 12 21:28:51 2001
@@ -227,6 +227,13 @@
  *
  * pps_intcnt counts the calibration intervals for use in the interval-
  * adaptation algorithm. It's just too complicated for words.
+ *
+ * pps_kc_hardpps_source contains an arbitrary value that uniquely
+ * identifies the currently bound source of the PPS signal, or NULL
+ * if no source is bound.
+ *
+ * pps_kc_hardpps_mode indicates which transitions, if any, of the PPS
+ * signal should be reported.
  */
 struct timeval pps_time;	/* kernel time at last interval */
 long pps_tf[] = {0, 0, 0};	/* pps time offset median filter (us) */
@@ -241,6 +248,8 @@
 int pps_count = 0;		/* calibration interval counter (s) */
 int pps_shift = PPS_SHIFT;	/* interval duration (s) (shift) */
 int pps_intcnt = 0;		/* intervals at current duration */
+void *pps_kc_hardpps_source = NULL; /* current PPS supplier's identifier */
+int pps_kc_hardpps_mode = 0;	/* interesting edges of PPS signal */
 
 /*
  * PPS signal quality monitors
--- sys/systm.h.orig	Sun May  6 08:08:02 2001
+++ sys/systm.h	Sun Aug 12 21:28:51 2001
@@ -239,6 +239,8 @@
 void	hardupdate __P((long offset));
 #ifdef PPS_SYNC
 void	hardpps __P((struct timeval *, long));
+extern	void *pps_kc_hardpps_source;
+extern	int pps_kc_hardpps_mode;
 #endif
 #endif
 
--- sys/timepps.h.orig	Wed Jul 26 16:09:33 2000
+++ sys/timepps.h	Sun Aug 12 21:28:51 2001
@@ -127,7 +127,7 @@
 #define PPS_IOC_GETPARAMS	_IOR('1', 4, pps_params_t)
 #define PPS_IOC_GETCAP		_IOR('1', 5, int)
 #define PPS_IOC_FETCH		_IOWR('1', 6, pps_info_t)
-#define PPS_IOC_KCBIND		_IOWR('1', 7, int)
+#define PPS_IOC_KCBIND		_IOW('1', 7, int)
 
 #ifndef _KERNEL
 
@@ -209,7 +209,7 @@
 	const int edge;
 	const int tsformat;
 {
-	return (ioctl(handle, PPS_IOC_KCBIND, edge));
+	return (ioctl(handle, PPS_IOC_KCBIND, &edge));
 }
 #endif /* !_KERNEL*/
 
>Release-Note:
>Audit-Trail:
>Unformatted: