Subject: Re: ps/2 mouse timeout problems
To: David Laight <david@l8s.co.uk>
From: Martin Husemann <martin@duskware.de>
List: tech-kern
Date: 08/30/2002 17:47:28
--RnlQjJ0d97Da+TV1
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

So inspired by David's sugestions I made two patches.

The first restructures the timeout handling and uses callouts instead of
timestamps. It resets a callout every time a byte is received and stops it
when we reach inpustate 0 again. If the callout fires, something is wrong
and the reset thread is woken up.

This works, but the timing granularity is higher (on my i386 test notebook
with hz=100 the threshold moves from 25ms to 30ms). I don't think this is a
problem.

After this worked fine, I did a much simpler version that just replaces
the call to microtime with a read from mono_time and adjusts the timeout
threshold to the new granularity.

I prefer the mono_time version.

Any comments? Should I commit this?

Martin

--RnlQjJ0d97Da+TV1
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="pms_callout.patch"

Index: pms.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/pckbc/pms.c,v
retrieving revision 1.8
diff -c -u -r1.8 pms.c
--- pms.c	2002/07/10 02:05:25	1.8
+++ pms.c	2002/08/30 14:55:14
@@ -32,6 +32,7 @@
 #include <sys/ioctl.h>
 #include <sys/kernel.h>
 #include <sys/kthread.h>
+#include <sys/callout.h>
 
 #include <machine/bus.h>
 
@@ -81,7 +82,7 @@
 	u_int buttons;		/* mouse button status */
 	enum pms_type protocol;
 	unsigned char packet[4];
-	struct timeval last, current;
+	struct callout timeout;
 
 	struct device *sc_wsmousedev;
 	struct proc *sc_event_thread;
@@ -100,6 +101,7 @@
 static void	do_disable __P((struct pms_softc *));
 static void	pms_reset_thread __P((void*));
 static void	pms_spawn_reset_thread __P((void*));
+static void	pmstimeout __P((void *));
 int	pms_enable __P((void *));
 int	pms_ioctl __P((void *, u_long, caddr_t, int, struct proc *));
 void	pms_disable __P((void *));
@@ -203,6 +205,8 @@
 	sc->sc_kbctag = pa->pa_tag;
 	sc->sc_kbcslot = pa->pa_slot;
 
+	callout_init(&sc->timeout);
+
 	printf("\n");
 
 	/* Flush any garbage. */
@@ -509,35 +513,6 @@
 		return;
 	}
 
-	microtime(&sc->current);
-	if (sc->inputstate > 0) {
-		struct timeval diff;
-
-		timersub(&sc->current, &sc->last, &diff);
-		/*
-		 * Empirically, the delay should be about 1700us on a standard
-		 * PS/2 port.  I have seen delays as large as 4500us (rarely)
-		 * in regular use.  When using a confused mouse, I generally
-		 * see delays at least as large as 30,000us.  This serves as
-		 * a rough geometric compromise. -seebs
-		 *
-		 * The thinkpad trackball returns at 22-23ms. So we use
-		 * 25ms. In the future, I'll implement adaptable timeout
-		 * by increasing the timeout if the mouse reset happens
-		 * too frequently -christos
-		 */
-		if (diff.tv_sec > 0 || diff.tv_usec > 25000) {
-			DPRINTF(("pms_input: unusual delay (%ld.%06ld s), "
-			    "scheduling reset\n",
-			    (long)diff.tv_sec, (long)diff.tv_usec));
-			sc->inputstate = 0;
-			sc->sc_enabled = 0;
-			wakeup(&sc->sc_enabled);
-			return;
-		}
-	}
-	sc->last = sc->current;
-
 	if (sc->inputstate == 0) {
 		/*
 		 * Some devices (seen on trackballs anytime, and on some mice shortly after
@@ -569,6 +544,7 @@
 		if (!(sc->packet[0] & 0x8)) {
 			DPRINTF(("pmsinput: 0x8 not set in first byte "
 			    "[0x%02x], resetting\n", sc->packet[0]));
+			callout_stop(&sc->timeout);
 			sc->inputstate = 0;
 			sc->sc_enabled = 0;
 			wakeup(&sc->sc_enabled);
@@ -656,7 +632,49 @@
 		printf("pmsinput: very confused.  resetting.\n");
 		sc->inputstate = 0;
 		sc->sc_enabled = 0;
+		callout_stop(&sc->timeout);
 		wakeup(&sc->sc_enabled);
 		return;
 	}
+
+
+	/*
+	 * Empirically, the delay should be about 1700us on a standard
+	 * PS/2 port.  I have seen delays as large as 4500us (rarely)
+	 * in regular use.  When using a confused mouse, I generally
+	 * see delays at least as large as 30,000us.  This serves as
+	 * a rough geometric compromise. -seebs
+	 *
+	 * The thinkpad trackball returns at 22-23ms. So we use
+	 * 25ms. In the future, I'll implement adaptable timeout
+	 * by increasing the timeout if the mouse reset happens
+	 * too frequently -christos
+	 */
+
+	 /* number of ticks per 25ms, rounded up */
+#define CALLOUT_TICKS	(25*hz+999)/1000
+
+	if (sc->inputstate == 0)
+		callout_stop(&sc->timeout);
+	else
+		callout_reset(&sc->timeout, CALLOUT_TICKS, pmstimeout, sc);
+}
+
+void pmstimeout(void *token)
+{
+	struct pms_softc * sc = token;
+	int s;
+
+	if (sc->inputstate == 0) {
+		DPRINTF(("pms_timeout: inputstate == 0, ignored\n"));
+		return;
+	}
+
+	DPRINTF(("pms_timeout: unusual delay, scheduling reset\n"));
+
+	s = spltty();
+	sc->inputstate = 0;
+	sc->sc_enabled = 0;
+	splx(s);
+	wakeup(&sc->sc_enabled);
 }

--RnlQjJ0d97Da+TV1
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="pms_mono_time.patch"

Index: pms.c
===================================================================
RCS file: /cvsroot/syssrc/sys/dev/pckbc/pms.c,v
retrieving revision 1.8
diff -c -u -r1.8 pms.c
--- pms.c	2002/07/10 02:05:25	1.8
+++ pms.c	2002/08/30 15:33:37
@@ -501,6 +501,7 @@
 {
 	struct pms_softc *sc = vsc;
 	u_int changed;
+	unsigned long max_delay;
 	int dx, dy, dz = 0;
 	int newbuttons = 0;
 
@@ -509,7 +510,7 @@
 		return;
 	}
 
-	microtime(&sc->current);
+	sc->current = mono_time;
 	if (sc->inputstate > 0) {
 		struct timeval diff;
 
@@ -526,7 +527,12 @@
 		 * by increasing the timeout if the mouse reset happens
 		 * too frequently -christos
 		 */
-		if (diff.tv_sec > 0 || diff.tv_usec > 25000) {
+
+		/* round 25ms up to next value of hz granularity */
+		max_delay = (25*hz+999)/1000;	/* calc ticks */
+		max_delay *= 1000000/hz;	/* convert to usec */
+
+		if (diff.tv_sec > 0 || diff.tv_usec > max_delay) {
 			DPRINTF(("pms_input: unusual delay (%ld.%06ld s), "
 			    "scheduling reset\n",
 			    (long)diff.tv_sec, (long)diff.tv_usec));

--RnlQjJ0d97Da+TV1--