MFC r293781:

  Restore uart PPS signal capture polarity to its historical norm, and add an
  option to invert the polarity in software. Also add an option to capture
  very narrow pulses by using the hardware's MSR delta-bit capability of
  latching line state changes.

Relnotes:	yes

5 files changed, 248 insertions, 73 deletions

diff --git a/share/man/man4/uart.4 b/share/man/man4/uart.4
index d520409..c0f49a1 100644
--- a/share/man/man4/uart.4
+++ b/share/man/man4/uart.4
@@ -25,7 +25,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd August 10, 2015
+.Dd December 9, 2015
 .Dt UART 4
 .Os
 .Sh NAME
@@ -160,7 +160,9 @@ The API, accessed via
 is available on the tty device.
 To use the PPS capture feature with
 .Xr ntpd 8 ,
-symlink the tty device to
+symlink the tty callout device
+.Va /dev/cuau?
+to
 .Va /dev/pps0.
 .Pp
 The
@@ -175,15 +177,54 @@ it can be set in
 .Xr loader.conf 5
 or
 .Xr sysctl.conf 5 .
+.Pp
 The following capture modes are available:
 .Bl -tag -compact -offset "mmmm" -width "mmmm"
-.It 0
+.It 0x00
 Capture disabled.
-.It 1
+.It 0x01
 Capture pulses on the CTS line.
-.It 2
-Capture pulses on the DCD line (default).
+.It 0x02
+Capture pulses on the DCD line.
+.El
+.Pp
+The following values may be ORed with the capture mode to configure 
+capture processing options:
+.Bl -tag -compact -offset "mmmm" -width "mmmm"
+.It 0x10
+Invert the pulse (RS-232 logic low = ASSERT, high = CLEAR).
+.It 0x20
+Attempt to capture narrow pulses.
 .El
+.Pp
+Add the narrow pulse option when the incoming PPS pulse width is small
+enough to prevent reliable capture in normal mode.
+In narrow mode the driver uses the hardware's ability to latch a line
+state change; not all hardware has this capability.
+The hardware latch provides a reliable indication that a pulse occurred,
+but prevents distinguishing between the CLEAR and ASSERT edges of the pulse.
+For each detected pulse, the driver synthesizes both an ASSERT and a CLEAR
+event, using the same timestamp for each.
+To prevent spurious events when the hardware is intermittently able to
+see both edges of a pulse, the driver will not generate a new pair of
+events within a half second of the prior pair.
+Both normal and narrow pulse modes work with
+.Xr ntpd 8 .
+.Pp
+Add the invert option when the connection to the uart device uses TTL
+level signals, or when the PPS source emits inverted pulses.
+RFC 2783 defines an ASSERT event as a higher-voltage line level, and a CLEAR
+event as a lower-voltage line level, in the context of the RS-232 protocol.
+The modem control signals on a TTL-level connection are typically
+inverted from the RS-232 levels.
+For example, carrier presence is indicated by a high signal on an RS-232
+DCD line, and by a low signal on a TTL DCD line.
+This is due to the use of inverting line driver buffers to convert between
+TTL and RS-232 line levels in most hardware designs.
+Generally speaking, a connection to a DB-9 style connector is an RS-232
+level signal at up to 12 volts.
+A connection to header pins or an edge-connector on an embedded board
+is typically a TTL signal at 3.3 or 5 volts.
 .Sh FILES
 .Bl -tag -width ".Pa /dev/ttyu?.init" -compact
 .It Pa /dev/ttyu?
diff --git a/sys/dev/uart/uart_bus.h b/sys/dev/uart/uart_bus.h
index 4a2ab91..3422269 100644
--- a/sys/dev/uart/uart_bus.h
+++ b/sys/dev/uart/uart_bus.h
@@ -112,6 +112,7 @@ struct uart_softc {
 	/* Pulse capturing support (PPS). */
 	struct pps_state sc_pps;
 	int		 sc_pps_mode;
+	sbintime_t	 sc_pps_captime;
 
 	/* Upper layer data. */
 	void		*sc_softih;
diff --git a/sys/dev/uart/uart_core.c b/sys/dev/uart/uart_core.c
index 3da37e0..b9cffe3 100644
--- a/sys/dev/uart/uart_core.c
+++ b/sys/dev/uart/uart_core.c
@@ -48,6 +48,7 @@ __FBSDID("$FreeBSD$");
 #include <dev/uart/uart.h>
 #include <dev/uart/uart_bus.h>
 #include <dev/uart/uart_cpu.h>
+#include <dev/uart/uart_ppstypes.h>
 
 #include "uart_if.h"
 
@@ -65,47 +66,47 @@ static MALLOC_DEFINE(M_UART, "UART", "UART driver");
 static int uart_poll_freq = UART_POLL_FREQ;
 TUNABLE_INT("debug.uart_poll_freq", &uart_poll_freq);
 
-#define	PPS_MODE_DISABLED	0
-#define	PPS_MODE_CTS		1
-#define	PPS_MODE_DCD		2
-
-static inline int
-uart_pps_signal(int pps_mode)
-{
-
-	switch(pps_mode) {
-	case PPS_MODE_CTS:
-		return (SER_CTS);
-	case PPS_MODE_DCD:
-		return (SER_DCD);
-	}
-	return (0);
-}
 static inline int
 uart_pps_mode_valid(int pps_mode)
 {
-
-	switch(pps_mode) {
-	case PPS_MODE_DISABLED:
-	case PPS_MODE_CTS:
-	case PPS_MODE_DCD:
-		return (true);
+	int opt;
+
+	switch(pps_mode & UART_PPS_SIGNAL_MASK) {
+	case UART_PPS_DISABLED:
+	case UART_PPS_CTS:
+	case UART_PPS_DCD:
+		break;
+	default:
+		return (false);
 	}
-	return (false);
+
+	opt = pps_mode & UART_PPS_OPTION_MASK;
+	if ((opt & ~(UART_PPS_INVERT_PULSE | UART_PPS_NARROW_PULSE)) != 0)
+		return (false);
+
+	return (true);
 }
 
-static const char *
-uart_pps_mode_name(int pps_mode)
+static void
+uart_pps_print_mode(struct uart_softc *sc)
 {
-	switch(pps_mode) {
-	case PPS_MODE_DISABLED:
-		return ("disabled");
-	case PPS_MODE_CTS:
-		return ("CTS");
-	case PPS_MODE_DCD:
-		return ("DCD");
+
+	device_printf(sc->sc_dev, "PPS capture mode: ");
+	switch(sc->sc_pps_mode) {
+	case UART_PPS_DISABLED:
+		printf("disabled");
+	case UART_PPS_CTS:
+		printf("CTS");
+	case UART_PPS_DCD:
+		printf("DCD");
+	default:
+		printf("invalid");
 	}
-	return ("invalid");
+	if (sc->sc_pps_mode & UART_PPS_INVERT_PULSE)
+		printf("-Inverted");
+	if (sc->sc_pps_mode & UART_PPS_NARROW_PULSE)
+		printf("-NarrowPulse");
+	printf("\n");
 }
 
 static int
@@ -126,6 +127,55 @@ uart_pps_mode_sysctl(SYSCTL_HANDLER_ARGS)
 }
 
 static void
+uart_pps_process(struct uart_softc *sc, int ser_sig)
+{
+	sbintime_t now;
+	int is_assert, pps_sig;
+
+	/* Which signal is configured as PPS?  Early out if none. */
+	switch(sc->sc_pps_mode & UART_PPS_SIGNAL_MASK) {
+	case UART_PPS_CTS:
+		pps_sig = SER_CTS;
+		break;
+	case UART_PPS_DCD:
+		pps_sig = SER_DCD;
+		break;
+	default:
+		return;
+	}
+
+	/* Early out if there is no change in the signal configured as PPS. */
+	if ((ser_sig & SER_DELTA(pps_sig)) == 0)
+		return;
+
+	/*
+	 * In narrow-pulse mode we need to synthesize both capture and clear
+	 * events from a single "delta occurred" indication from the uart
+	 * hardware because the pulse width is too narrow to reliably detect
+	 * both edges.  However, when the pulse width is close to our interrupt
+	 * processing latency we might intermittantly catch both edges.  To
+	 * guard against generating spurious events when that happens, we use a
+	 * separate timer to ensure at least half a second elapses before we
+	 * generate another event.
+	 */
+	pps_capture(&sc->sc_pps);
+	if (sc->sc_pps_mode & UART_PPS_NARROW_PULSE) {
+		now = getsbinuptime();
+		if (now > sc->sc_pps_captime + 500 * SBT_1MS) {
+			sc->sc_pps_captime = now;
+			pps_event(&sc->sc_pps, PPS_CAPTUREASSERT);
+			pps_event(&sc->sc_pps, PPS_CAPTURECLEAR);
+		}
+	} else  {
+		is_assert = ser_sig & pps_sig;
+		if (sc->sc_pps_mode & UART_PPS_INVERT_PULSE)
+			is_assert = !is_assert;
+		pps_event(&sc->sc_pps, is_assert ? PPS_CAPTUREASSERT :
+		    PPS_CAPTURECLEAR);
+	}
+}
+
+static void
 uart_pps_init(struct uart_softc *sc)
 {
 	struct sysctl_ctx_list *ctx;
@@ -142,23 +192,23 @@ uart_pps_init(struct uart_softc *sc)
 	 * for one specific device.
 	 */
 #ifdef UART_PPS_ON_CTS
-	sc->sc_pps_mode = PPS_MODE_CTS;
+	sc->sc_pps_mode = UART_PPS_CTS;
 #else
-	sc->sc_pps_mode = PPS_MODE_DCD;
+	sc->sc_pps_mode = UART_PPS_DCD;
 #endif
 	TUNABLE_INT_FETCH("hw.uart.pps_mode", &sc->sc_pps_mode);
 	SYSCTL_ADD_PROC(ctx, SYSCTL_CHILDREN(tree), OID_AUTO, "pps_mode",
 	    CTLTYPE_INT | CTLFLAG_RWTUN, sc, 0, uart_pps_mode_sysctl, "I",
-	    "pulse capturing mode - 0/1/2 - disabled/CTS/DCD");
+	    "pulse mode: 0/1/2=disabled/CTS/DCD; "
+	    "add 0x10 to invert, 0x20 for narrow pulse");
 
 	if (!uart_pps_mode_valid(sc->sc_pps_mode)) {
 		device_printf(sc->sc_dev, 
-		    "Invalid pps_mode %d configured; disabling PPS capture\n",
+		    "Invalid pps_mode 0x%02x configured; disabling PPS capture\n",
 		    sc->sc_pps_mode);
-		sc->sc_pps_mode = PPS_MODE_DISABLED;
+		sc->sc_pps_mode = UART_PPS_DISABLED;
 	} else if (bootverbose) {
-		device_printf(sc->sc_dev, "PPS capture mode %d (%s)\n",
-		    sc->sc_pps_mode, uart_pps_mode_name(sc->sc_pps_mode));
+		uart_pps_print_mode(sc);
 	}
 
 	sc->sc_pps.ppscap = PPS_CAPTUREBOTH;
@@ -302,23 +352,16 @@ static __inline int
 uart_intr_sigchg(void *arg)
 {
 	struct uart_softc *sc = arg;
-	int new, old, pps_sig, sig;
+	int new, old, sig;
 
 	sig = UART_GETSIG(sc);
 
 	/*
-	 * Time pulse counting support. Note that both CTS and DCD are
-	 * active-low signals. The status bit is high to indicate that
-	 * the signal on the line is low, which corresponds to a PPS
-	 * clear event.
+	 * Time pulse counting support, invoked whenever the PPS parameters are
+	 * currently set to capture either edge of the signal.
 	 */
 	if (sc->sc_pps.ppsparam.mode & PPS_CAPTUREBOTH) {
-		pps_sig = uart_pps_signal(sc->sc_pps_mode);
-		if (sig & SER_DELTA(pps_sig)) {
-			pps_capture(&sc->sc_pps);
-			pps_event(&sc->sc_pps, (sig & pps_sig) ?
-			    PPS_CAPTURECLEAR : PPS_CAPTUREASSERT);
-		}
+		uart_pps_process(sc, sig);
 	}
 
 	/*
diff --git a/sys/dev/uart/uart_dev_ns8250.c b/sys/dev/uart/uart_dev_ns8250.c
index 99692e6..67149d6 100644
--- a/sys/dev/uart/uart_dev_ns8250.c
+++ b/sys/dev/uart/uart_dev_ns8250.c
@@ -50,6 +50,7 @@ __FBSDID("$FreeBSD$");
 #endif
 #include <dev/uart/uart_bus.h>
 #include <dev/uart/uart_dev_ns8250.h>
+#include <dev/uart/uart_ppstypes.h>
 
 #include <dev/ic/ns16550.h>
 
@@ -390,11 +391,40 @@ static struct ofw_compat_data compat_data[] = {
 UART_FDT_CLASS_AND_DEVICE(compat_data);
 #endif
 
-#define	SIGCHG(c, i, s, d)				\
-	if (c) {					\
-		i |= (i & s) ? s : s | d;		\
-	} else {					\
-		i = (i & s) ? (i & ~s) | d : i;		\
+/* Use token-pasting to form SER_ and MSR_ named constants. */
+#define	SER(sig)	SER_##sig
+#define	SERD(sig)	SER_D##sig
+#define	MSR(sig)	MSR_##sig
+#define	MSRD(sig)	MSR_D##sig
+
+/*
+ * Detect signal changes using software delta detection.  The previous state of
+ * the signals is in 'var' the new hardware state is in 'msr', and 'sig' is the
+ * short name (DCD, CTS, etc) of the signal bit being processed; 'var' gets the
+ * new state of both the signal and the delta bits.
+ */
+#define SIGCHGSW(var, msr, sig)					\
+	if ((msr) & MSR(sig)) {					\
+		if ((var & SER(sig)) == 0)			\
+			var |= SERD(sig) | SER(sig);		\
+	} else {						\
+		if ((var & SER(sig)) != 0)			\
+			var = SERD(sig) | (var & ~SER(sig));	\
+	}
+
+/*
+ * Detect signal changes using the hardware msr delta bits.  This is currently
+ * used only when PPS timing information is being captured using the "narrow
+ * pulse" option.  With a narrow PPS pulse the signal may not still be asserted
+ * by time the interrupt handler is invoked.  The hardware will latch the fact
+ * that it changed in the delta bits.
+ */
+#define SIGCHGHW(var, msr, sig)					\
+	if ((msr) & MSRD(sig)) {				\
+		if (((msr) & MSR(sig)) != 0)			\
+			var |= SERD(sig) | SER(sig);		\
+		else						\
+			var = SERD(sig) | (var & ~SER(sig));	\
 	}
 
 int
@@ -521,21 +551,37 @@ ns8250_bus_flush(struct uart_softc *sc, int what)
 int
 ns8250_bus_getsig(struct uart_softc *sc)
 {
-	uint32_t new, old, sig;
+	uint32_t old, sig;
 	uint8_t msr;
 
+	/*
+	 * The delta bits are reputed to be broken on some hardware, so use
+	 * software delta detection by default.  Use the hardware delta bits
+	 * when capturing PPS pulses which are too narrow for software detection
+	 * to see the edges.  Hardware delta for RI doesn't work like the
+	 * others, so always use software for it.  Other threads may be changing
+	 * other (non-MSR) bits in sc_hwsig, so loop until it can succesfully
+	 * update without other changes happening.  Note that the SIGCHGxx()
+	 * macros carefully preserve the delta bits when we have to loop several
+	 * times and a signal transitions between iterations.
+	 */
 	do {
 		old = sc->sc_hwsig;
 		sig = old;
 		uart_lock(sc->sc_hwmtx);
 		msr = uart_getreg(&sc->sc_bas, REG_MSR);
 		uart_unlock(sc->sc_hwmtx);
-		SIGCHG(msr & MSR_DSR, sig, SER_DSR, SER_DDSR);
-		SIGCHG(msr & MSR_CTS, sig, SER_CTS, SER_DCTS);
-		SIGCHG(msr & MSR_DCD, sig, SER_DCD, SER_DDCD);
-		SIGCHG(msr & MSR_RI,  sig, SER_RI,  SER_DRI);
-		new = sig & ~SER_MASK_DELTA;
-	} while (!atomic_cmpset_32(&sc->sc_hwsig, old, new));
+		if (sc->sc_pps_mode & UART_PPS_NARROW_PULSE) {
+			SIGCHGHW(sig, msr, DSR);
+			SIGCHGHW(sig, msr, CTS);
+			SIGCHGHW(sig, msr, DCD);
+		} else {
+			SIGCHGSW(sig, msr, DSR);
+			SIGCHGSW(sig, msr, CTS);
+			SIGCHGSW(sig, msr, DCD);
+		}
+		SIGCHGSW(sig, msr, RI);
+	} while (!atomic_cmpset_32(&sc->sc_hwsig, old, sig & ~SER_MASK_DELTA));
 	return (sig);
 }
 
@@ -889,12 +935,10 @@ ns8250_bus_setsig(struct uart_softc *sc, int sig)
 		old = sc->sc_hwsig;
 		new = old;
 		if (sig & SER_DDTR) {
-			SIGCHG(sig & SER_DTR, new, SER_DTR,
-			    SER_DDTR);
+			new = (new & ~SER_DTR) | (sig & (SER_DTR | SER_DDTR));
 		}
 		if (sig & SER_DRTS) {
-			SIGCHG(sig & SER_RTS, new, SER_RTS,
-			    SER_DRTS);
+			new = (new & ~SER_RTS) | (sig & (SER_RTS | SER_DRTS));
 		}
 	} while (!atomic_cmpset_32(&sc->sc_hwsig, old, new));
 	uart_lock(sc->sc_hwmtx);
diff --git a/sys/dev/uart/uart_ppstypes.h b/sys/dev/uart/uart_ppstypes.h
new file mode 100644
index 0000000..142d0f1
--- /dev/null
+++ b/sys/dev/uart/uart_ppstypes.h
@@ -0,0 +1,46 @@
+/*-
+ * Copyright (c) 2015 	Ian Lepore
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+ * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * $FreeBSD$
+ */
+
+#ifndef _DEV_UART_PPSTYPES_H_
+#define _DEV_UART_PPSTYPES_H_
+
+/*
+ * These constants are shared by several drivers including uart and usb_serial.
+ */
+
+#define	UART_PPS_SIGNAL_MASK	0x0f
+#define	UART_PPS_OPTION_MASK	0xf0
+
+#define	UART_PPS_DISABLED	0x00
+#define	UART_PPS_CTS		0x01
+#define	UART_PPS_DCD		0x02
+
+#define	UART_PPS_INVERT_PULSE	0x10
+#define	UART_PPS_NARROW_PULSE	0x20
+
+#endif /* _DEV_UART_PPSTYPES_H_ */