psmouse.c - synaptics touchpad driver sync problem

psmouse.c - synaptics touchpad driver sync problem

[Thorsten Hirsch]

Hi.

I'm having some sync trouble with kernel 2.6.6-rc3-mm1 (and older 2.6
kernels) in combination with Peter Osterlund's synaptics driver:

psmouse.c: TouchPad at isa0060/serio2/input0 lost sync at byte 1
psmouse.c: TouchPad at isa0060/serio2/input0 lost sync at byte 1
psmouse.c: TouchPad at isa0060/serio2/input0 lost sync at byte 1
psmouse.c: TouchPad at isa0060/serio2/input0 lost sync at byte 1
psmouse.c: TouchPad at isa0060/serio2/input0 lost sync at byte 1
psmouse.c: TouchPad at isa0060/serio2/input0 - driver resynched.
psmouse.c: TouchPad at isa0060/serio2/input0 lost sync at byte 4
psmouse.c: TouchPad at isa0060/serio2/input0 lost sync at byte 1
psmouse.c: TouchPad at isa0060/serio2/input0 - driver resynched.

...and so on. This is causing my mouse in X11 to hang, especially when
cpu load is high. The mouse pointer is doing an uncontrollable move and
some other things are hanging for a short moment, too: xmms for example.
(I guess the whole system is hanging, but xmms playing some music is
very easy to monitor)

I'm using the latest synaptics driver (0.13.0) and I've also tried
0.12.5. I wrote a mail to Peter, but he said, that I should write to the
lkml, so here I am. :-)
X11 version is X.org 6.7.0...but I think, that this doesn't matter. I
even had the same problem with XFree86 4.3.0.

Btw, there's no problem when I use standard ps/2 driver in X, but well,
I'm really missing the synaptics features then.

Regards,
Thorsten

P.S.: I'm not subscribed to the lkml, so please CC me in your answer!

Re: psmouse.c - synaptics touchpad driver sync problem

[Thorsten Hirsch]

Hi again.

I just wanted to confirm the problem for kernel 2.6.6-rc3-mm2.
Changelog says that there were patches concerning synaptics and psmouse,
but they didn't solve my problem.

Bye.
Thorsten

P.S.: again, please CC me in your answer!
-- 
PGP public key:
http://home.arcor.de/thorstenhirsch/thirschatwebde.asc

Re: psmouse.c - synaptics touchpad driver sync problem

[Dmitry Torokhov]

On Sunday 09 May 2004 02:30 pm, Thorsten Hirsch wrote:
> Hi again.
> 
> I just wanted to confirm the problem for kernel 2.6.6-rc3-mm2.
> Changelog says that there were patches concerning synaptics and psmouse,
> but they didn't solve my problem.
>
 
Hi,

Could you please try the patch below - it changes the way we process
interrupts from the keyboard controller. Also, in your dmesg, do you see
messages like "psmouse.c: bad data from KBC - timeout"?

--- linus-2.5/drivers/input/serio/i8042.c	2004-05-06 00:54:57.000000000 -0500
+++ dtor/drivers/input/serio/i8042.c	2004-05-06 00:35:32.000000000 -0500
@@ -74,6 +74,14 @@
 	unsigned char *phys;
 };
 
+#define I8042_QUEUE_LEN		64
+struct {
+	unsigned char str[I8042_QUEUE_LEN];
+	unsigned char data[I8042_QUEUE_LEN];
+	unsigned int read_pos;
+	unsigned int write_pos;
+} i8042_buf;
+
 static struct serio i8042_kbd_port;
 static struct serio i8042_aux_port;
 static unsigned char i8042_initial_ctr;
@@ -82,7 +90,7 @@
 static unsigned char i8042_mux_present;
 static unsigned char i8042_sysdev_initialized;
 static struct pm_dev *i8042_pm_dev;
-struct timer_list i8042_timer;
+static struct timer_list i8042_timer;
 
 /*
  * Shared IRQ's require a device pointer, but this driver doesn't support
@@ -374,77 +382,109 @@
 static char i8042_mux_phys[4][32];
 
 /*
- * i8042_interrupt() is the most important function in this driver -
- * it handles the interrupts from the i8042, and sends incoming bytes
- * to the upper layers.
+ * i8042_handle_data() is the most important function in this driver -
+ * it processes data received by i8042_interrupt and sends it to the
+ * upper layers.
  */
 
-static irqreturn_t i8042_interrupt(int irq, void *dev_id, struct pt_regs *regs)
+static void i8042_handle_data(unsigned long notused)
 {
-	unsigned long flags;
 	unsigned char str, data = 0;
 	unsigned int dfl;
-	int ret;
 
-	mod_timer(&i8042_timer, jiffies + I8042_POLL_PERIOD);
+	/*
+	 * No locking it required on i8042_buf as the tasklet is guaranteed
+	 * to be serialized and if write_pos changes while comparing it with
+	 * read_pos another run will be scheduled by i8042_interrupt.
+	 */
+	while (i8042_buf.read_pos != i8042_buf.write_pos) {
 
-	spin_lock_irqsave(&i8042_lock, flags);
-	str = i8042_read_status();
-	if (str & I8042_STR_OBF)
-		data = i8042_read_data();
-	spin_unlock_irqrestore(&i8042_lock, flags);
+		str = i8042_buf.str[i8042_buf.read_pos];
+		data = i8042_buf.data[i8042_buf.read_pos];
 
-	if (~str & I8042_STR_OBF) {
-		if (irq) dbg("Interrupt %d, without any data", irq);
-		ret = 0;
-		goto out;
+		i8042_buf.read_pos++;
+		i8042_buf.read_pos %= I8042_QUEUE_LEN;
+
+		dfl = ((str & I8042_STR_PARITY)  ? SERIO_PARITY  : 0) |
+	      	      ((str & I8042_STR_TIMEOUT) ? SERIO_TIMEOUT : 0);
+
+		if (i8042_mux_values[0].exists && (str & I8042_STR_AUXDATA)) {
+
+			if (str & I8042_STR_MUXERR) {
+				switch (data) {
+					case 0xfd:
+					case 0xfe: dfl = SERIO_TIMEOUT; break;
+					case 0xff: dfl = SERIO_PARITY; break;
+				}
+				data = 0xfe;
+			} else dfl = 0;
+
+			dbg("%02x <- i8042 (interrupt, aux%d, %d%s%s)",
+				data, (str >> 6), irq,
+				dfl & SERIO_PARITY ? ", bad parity" : "",
+				dfl & SERIO_TIMEOUT ? ", timeout" : "");
+
+			serio_interrupt(i8042_mux_port + ((str >> 6) & 3), data, dfl, NULL);
+		} else {
+
+			dbg("%02x <- i8042 (interrupt, %s, %d%s%s)",
+				data, (str & I8042_STR_AUXDATA) ? "aux" : "kbd", irq,
+				dfl & SERIO_PARITY ? ", bad parity" : "",
+				dfl & SERIO_TIMEOUT ? ", timeout" : "");
+
+			if (i8042_aux_values.exists && (str & I8042_STR_AUXDATA))
+				serio_interrupt(&i8042_aux_port, data, dfl, NULL);
+			else if (i8042_kbd_values.exists)
+				serio_interrupt(&i8042_kbd_port, data, dfl, NULL);
+		}
 	}
+}
 
-	dfl = ((str & I8042_STR_PARITY) ? SERIO_PARITY : 0) |
-	      ((str & I8042_STR_TIMEOUT) ? SERIO_TIMEOUT : 0);
+DECLARE_TASKLET(i8042_tasklet, i8042_handle_data, 0);
 
-	if (i8042_mux_values[0].exists && (str & I8042_STR_AUXDATA)) {
+/*
+ * i8042_interrupt() handles the interrupts from i8042 and schedules
+ * i8042_handle_data to process and pass received bytes to the upper
+ * layers.
+ */
 
-		if (str & I8042_STR_MUXERR) {
-			switch (data) {
-				case 0xfd:
-				case 0xfe: dfl = SERIO_TIMEOUT; break;
-				case 0xff: dfl = SERIO_PARITY; break;
-			}
-			data = 0xfe;
-		} else dfl = 0;
+static irqreturn_t i8042_interrupt(int irq, void *dev_id, struct pt_regs *regs)
+{
+	unsigned long flags;
+	unsigned char str;
+	unsigned int n_bytes = 0;
 
-		dbg("%02x <- i8042 (interrupt, aux%d, %d%s%s)",
-			data, (str >> 6), irq, 
-			dfl & SERIO_PARITY ? ", bad parity" : "",
-			dfl & SERIO_TIMEOUT ? ", timeout" : "");
+	mod_timer(&i8042_timer, jiffies + I8042_POLL_PERIOD);
 
-		serio_interrupt(i8042_mux_port + ((str >> 6) & 3), data, dfl, regs);
-		
-		goto irq_ret;
-	}
+	spin_lock_irqsave(&i8042_lock, flags);
+
+	while ((str = i8042_read_status()) & I8042_STR_OBF) {
 
-	dbg("%02x <- i8042 (interrupt, %s, %d%s%s)",
-		data, (str & I8042_STR_AUXDATA) ? "aux" : "kbd", irq, 
-		dfl & SERIO_PARITY ? ", bad parity" : "",
-		dfl & SERIO_TIMEOUT ? ", timeout" : "");
+		n_bytes++;
 
-	if (i8042_aux_values.exists && (str & I8042_STR_AUXDATA)) {
-		serio_interrupt(&i8042_aux_port, data, dfl, regs);
-		goto irq_ret;
+		i8042_buf.str[i8042_buf.write_pos] = str;
+		i8042_buf.data[i8042_buf.write_pos] = i8042_read_data();
+
+		i8042_buf.write_pos++;
+		i8042_buf.write_pos %= I8042_QUEUE_LEN;
+
+		if (unlikely(i8042_buf.write_pos == i8042_buf.read_pos))
+			printk(KERN_WARNING "i8042.c: ring buffer full\n");
 	}
 
-	if (!i8042_kbd_values.exists)
-		goto irq_ret;
+	spin_unlock_irqrestore(&i8042_lock, flags);
+
+	if (unlikely(n_bytes == 0)) {
+		if (irq) dbg("Interrupt %d, without any data", irq);
+		return IRQ_NONE;
+	}
 
-	serio_interrupt(&i8042_kbd_port, data, dfl, regs);
+	tasklet_schedule(&i8042_tasklet);
 
-irq_ret:
-	ret = 1;
-out:
-	return IRQ_RETVAL(ret);
+	return IRQ_HANDLED;
 }
 
+
 /*
  * i8042_enable_mux_mode checks whether the controller has an active
  * multiplexor and puts the chip into Multiplexed (as opposed to
@@ -1011,6 +1051,8 @@
 		if (i8042_mux_values[i].exists)
 			serio_unregister_port(i8042_mux_port + i);
 
+	tasklet_kill(&i8042_tasklet);
+
 	i8042_platform_exit();
 }