[alps] Timing patch, revised again :-)

[alps] Timing patch, revised again :-)

[Sebastian Kapfer]

Dear Dmitry,

the timer patch so far works great.  You can't desync the touchpad
anymore by just pressing all buttons.  It still goes wrong when you
press all buttons and move both touchpad & trackpoint pointer tough :-)

I've changed two little things:

1. The reporting of buttons went to dev2 sometimes, as bare_ps2_packet
signalled the click on 'dev', not psmouse->dev.  This lead to sticking
buttons.

2. We needlessly break the distinction of PS/2 and touchpad on older
models, where it may have been cleanly possible.  (If you don't want to
keep that feature and rather simplify the code, that's fine with me,
too.  But as I said, bare_ps2_packet needs to be fixed up then.)

In the end, I think it's good enough to commit it.  I'm not really happy
with the fact that we need a timer now, but hey, it wasn't our fault.

Again, working great as long as you don't do really crazy stuff as
described above.  I just hope the 20msecs is not a machine parameter
which just happens to work for me.

Best wishes,
  Sebastian


Input: ALPS - add interleaved protocol support (Dell E6x00 series)

From: Sebastian Kapfer <sebastian_kapfer@gmx.net>

Properly handle version of the protocol where standard PS/2 packets
from trackpoint are stuffed into middle (byte 3-6) of the standard
ALPS packets when both the touchpad and trackpoint are used together.

The patch is based on work done by Matthew Chapman and additional
research done by David Kubicek and Erik Osterholm:

        https://bugs.launchpad.net/ubuntu/+source/linux/+bug/296610

Signed-off-by: Sebastian Kapfer <sebastian_kapfer@gmx.net>

---
 drivers/input/mouse/alps.c |  211 +++++++++++++++++++++++++++++++++++++++-----
 drivers/input/mouse/alps.h |    3 +-
 2 files changed, 189 insertions(+), 25 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index a3f492a..38aabb7 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -5,6 +5,7 @@
  * Copyright (c) 2003-2005 Peter Osterlund <petero2@telia.com>
  * Copyright (c) 2004 Dmitry Torokhov <dtor@mail.ru>
  * Copyright (c) 2005 Vojtech Pavlik <vojtech@suse.cz>
+ * Copyright (c) 2009 Sebastian Kapfer <sebastian_kapfer@gmx.net>
  *
  * ALPS detection, tap switching and status querying info is taken from
  * tpconfig utility (by C. Scott Ananian and Bruce Kall).
@@ -37,6 +38,9 @@
 #define ALPS_FW_BK_1		0x10	/* front & back buttons present */
 #define ALPS_FW_BK_2		0x20	/* front & back buttons present */
 #define ALPS_FOUR_BUTTONS	0x40	/* 4 direction button present */
+#define ALPS_PS2_INTERLEAVED	0x80	/* 3-byte PS/2 packet interleaved with
+					   6-byte ALPS packet */
+#define ALPS_SHARED_BTNSTATE	0x100	/* PS/2 and touchpad share button st. */
 
 
 static const struct alps_model_info alps_model_data[] = {
@@ -58,7 +62,9 @@ static const struct alps_model_info alps_model_data[] = {
 	{ { 0x20, 0x02, 0x0e },	0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT }, /* XXX */
 	{ { 0x22, 0x02, 0x0a },	0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT },
 	{ { 0x22, 0x02, 0x14 }, 0xff, 0xff, ALPS_PASS | ALPS_DUALPOINT }, /* Dell Latitude D600 */
-	{ { 0x62, 0x02, 0x14 }, 0xcf, 0xcf, ALPS_PASS | ALPS_DUALPOINT }, /* Dell Latitude E6500 */
+	/* Dell Latitude E6400, E6500, E5500, Precision M4400 */
+	{ { 0x62, 0x02, 0x14 }, 0xcf, 0xcf,
+		ALPS_PASS | ALPS_DUALPOINT | ALPS_PS2_INTERLEAVED | ALPS_SHARED_BTNSTATE },
 	{ { 0x73, 0x02, 0x50 }, 0xcf, 0xcf, ALPS_FOUR_BUTTONS },	  /* Dell Vostro 1400 */
 };
 
@@ -69,7 +75,13 @@ static const struct alps_model_info alps_model_data[] = {
  */
 
 /*
- * ALPS abolute Mode - new format
+ * PS/2 packet format
+ *
+ * byte 0: YOFL XOFL YSGN XSGN  1    M    R    L
+ * byte 1: X7   X6   X5   X4   X3   X2   X1   X0
+ * byte 2: Y7   Y6   Y5   Y4   Y3   Y2   Y1   Y0
+ *
+ * ALPS absolute Mode - new format
  *
  * byte 0:  1    ?    ?    ?    1    ?    ?    ?
  * byte 1:  0   x6   x5   x4   x3   x2   x1   x0
@@ -78,11 +90,54 @@ static const struct alps_model_info alps_model_data[] = {
  * byte 4:  0   y6   y5   y4   y3   y2   y1   y0
  * byte 5:  0   z6   z5   z4   z3   z2   z1   z0
  *
+ * Dualpoint device -- interleaved packet format
+ *
+ * byte 0:    1    1    0    0    1    1    1    1
+ * byte 1:    0   x6   x5   x4   x3   x2   x1   x0
+ * byte 2:    0  x10   x9   x8   x7    0  fin  ges
+ * byte 3: YOFL XOFL YSGN XSGN    1    1    1    1
+ * byte 4:   X7   X6   X5   X4   X3   X2   X1   X0
+ * byte 5:   Y7   Y6   Y5   Y4   Y3   Y2   Y1   Y0
+ * byte 6:    0   y9   y8   y7    1    m    r    l
+ * byte 7:    0   y6   y5   y4   y3   y2   y1   y0
+ * byte 8:    0   z6   z5   z4   z3   z2   z1   z0
+ *
+ * CAPITALS = stick, miniscules = touchpad
+ *
  * ?'s can have different meanings on different models,
  * such as wheel rotation, extra buttons, stick buttons
  * on a dualpoint, etc.
  */
 
+/* at least some Alps units merge the mouse buttons of the touchpad and the
+ * PS/2 passthrough.  for the ALPS_PS2_INTERLEAVED units, we then can't
+ * distinguish which device a click came from, since the button bit in the
+ * interleaved packet could mean that either of the devices sent a click.
+ * (this has been tested in the field for all models except the M4400.)
+ * we route all the button clicks to the touchpad device.
+ * otherwise we easily end up with hung buttons.
+ */
+static void alps_report_buttons(struct psmouse *psmouse,
+                                int left, int right, int middle,
+                                struct input_dev *preferred_dev)
+{
+	struct alps_data *priv = psmouse->private;
+	const struct alps_model_info *model = priv->i;
+	struct input_dev *dev = preferred_dev;
+
+	if (model->flags & ALPS_SHARED_BTNSTATE
+	    && model->flags & ALPS_PS2_INTERLEAVED)
+		dev = psmouse->dev;
+
+	input_report_key(dev, BTN_LEFT, left);
+	input_report_key(dev, BTN_RIGHT, right);
+	input_report_key(dev, BTN_MIDDLE, middle);
+
+	/* calling code will input_sync the preferred_dev */
+	if (preferred_dev != dev)
+		input_sync(dev);
+}
+
 static void alps_process_packet(struct psmouse *psmouse)
 {
 	struct alps_data *priv = psmouse->private;
@@ -93,18 +148,6 @@ static void alps_process_packet(struct psmouse *psmouse)
 	int x, y, z, ges, fin, left, right, middle;
 	int back = 0, forward = 0;
 
-	if ((packet[0] & 0xc8) == 0x08) {   /* 3-byte PS/2 packet */
-		input_report_key(dev2, BTN_LEFT,   packet[0] & 1);
-		input_report_key(dev2, BTN_RIGHT,  packet[0] & 2);
-		input_report_key(dev2, BTN_MIDDLE, packet[0] & 4);
-		input_report_rel(dev2, REL_X,
-			packet[1] ? packet[1] - ((packet[0] << 4) & 0x100) : 0);
-		input_report_rel(dev2, REL_Y,
-			packet[2] ? ((packet[0] << 3) & 0x100) - packet[2] : 0);
-		input_sync(dev2);
-		return;
-	}
-
 	if (model->flags & ALPS_OLDPROTO) {
 		left = packet[2] & 0x10;
 		right = packet[2] & 0x08;
@@ -140,18 +183,13 @@ static void alps_process_packet(struct psmouse *psmouse)
 		input_report_rel(dev2, REL_X,  (x > 383 ? (x - 768) : x));
 		input_report_rel(dev2, REL_Y, -(y > 255 ? (y - 512) : y));
 
-		input_report_key(dev2, BTN_LEFT, left);
-		input_report_key(dev2, BTN_RIGHT, right);
-		input_report_key(dev2, BTN_MIDDLE, middle);
+		alps_report_buttons(psmouse, left, right, middle, dev2);
 
-		input_sync(dev);
 		input_sync(dev2);
 		return;
 	}
 
-	input_report_key(dev, BTN_LEFT, left);
-	input_report_key(dev, BTN_RIGHT, right);
-	input_report_key(dev, BTN_MIDDLE, middle);
+	alps_report_buttons(psmouse, left, right, middle, dev);
 
 	/* Convert hardware tap to a reasonable Z value */
 	if (ges && !fin)
@@ -202,25 +240,147 @@ static void alps_process_packet(struct psmouse *psmouse)
 	input_sync(dev);
 }
 
+static void alps_report_bare_ps2_packet(unsigned char packet[],
+					struct psmouse *psmouse)
+{
+	struct alps_data *priv = psmouse->private;
+	struct input_dev *dev2 = priv->dev2;
+
+	alps_report_buttons(psmouse, packet[0] & 1, packet[0] & 2,
+	                    packet[0] & 4, dev2);
+	input_report_rel(dev2, REL_X,
+		packet[1] ? packet[1] - ((packet[0] << 4) & 0x100) : 0);
+	input_report_rel(dev2, REL_Y,
+		packet[2] ? ((packet[0] << 3) & 0x100) - packet[2] : 0);
+	input_sync(dev2);
+}
+
+static psmouse_ret_t alps_handle_interleaved_ps2(struct psmouse *psmouse)
+{
+	struct alps_data *priv = psmouse->private;
+
+	if (psmouse->pktcnt < 7) {
+		if (psmouse->pktcnt == 6 && !(psmouse->packet[3] & 0x80)) {
+			/*
+			 * Need to time out before timeout in psmouse core
+			 * code. 20 ms should be enough to decide if we are
+			 * getting more data or not.
+			 */
+			mod_timer(&priv->timer, jiffies + msecs_to_jiffies(20));
+		}
+
+		return PSMOUSE_GOOD_DATA;
+	}
+
+	del_timer(&priv->timer);
+
+	if (psmouse->packet[6] & 0x80) {
+
+		/*
+		 * Highest bit is set - that means we either had
+		 * complete ALPS packet and this is start of the
+		 * next packet or we got garbage.
+		 */
+
+		if (((psmouse->packet[3] |
+		      psmouse->packet[4] |
+		      psmouse->packet[5]) & 0x80) ||
+		    (psmouse->packet[6] & priv->i->mask0) != priv->i->byte0) {
+			dbg("refusing packet %x %x %x %x "
+			    "(suspected interleaved ps/2)\n",
+			    psmouse->packet[3], psmouse->packet[4],
+			    psmouse->packet[5], psmouse->packet[6]);
+			return PSMOUSE_BAD_DATA;
+		}
+
+		alps_process_packet(psmouse);
+
+		/* Continue with the next packet */
+		psmouse->packet[0] = psmouse->packet[6];
+		psmouse->pktcnt = 1;
+
+	} else {
+
+		/*
+		 * High bit is 0 - that means that we indeed
+		 * got a PS/2 packet in the middle of ALPS packet
+		 */
+
+		psmouse->packet[3] &= 0xf0 | (psmouse->packet[6] & 0x0f);
+		alps_report_bare_ps2_packet(&psmouse->packet[3], psmouse);
+
+		/*
+		 * Continue with the standard ALPS protocol handling
+		 */
+		psmouse->packet[3] = psmouse->packet[6];
+		psmouse->pktcnt = 4;
+	}
+
+	return PSMOUSE_GOOD_DATA;
+}
+
+static void alps_flush_packet(unsigned long data)
+{
+	struct psmouse *psmouse = (struct psmouse *)data;
+
+	serio_pause_rx(psmouse->ps2dev.serio);
+
+	if (psmouse->pktcnt == 6) {
+
+		/*
+		 * We did not any more data in reasonable amount of time.
+		 * Validate the last 3 bytes and process as a standard
+		 * ALPS packet.
+		 */
+		if ((psmouse->packet[3] |
+		     psmouse->packet[4] |
+		     psmouse->packet[5]) & 0x80) {
+			dbg("refusing packet %x %x %x "
+			    "(suspected interleaved ps/2)\n",
+			    psmouse->packet[3], psmouse->packet[4],
+			    psmouse->packet[5]);
+		} else {
+			alps_process_packet(psmouse);
+		}
+		psmouse->pktcnt = 0;
+	}
+
+	serio_continue_rx(psmouse->ps2dev.serio);
+}
+
 static psmouse_ret_t alps_process_byte(struct psmouse *psmouse)
 {
 	struct alps_data *priv = psmouse->private;
+	const struct alps_model_info *model = priv->i;
 
 	if ((psmouse->packet[0] & 0xc8) == 0x08) { /* PS/2 packet */
 		if (psmouse->pktcnt == 3) {
-			alps_process_packet(psmouse);
+			alps_report_bare_ps2_packet(psmouse->packet, psmouse);
 			return PSMOUSE_FULL_PACKET;
 		}
 		return PSMOUSE_GOOD_DATA;
 	}
 
-	if ((psmouse->packet[0] & priv->i->mask0) != priv->i->byte0)
+	/* Check for PS/2 packet stuffed in the middle of ALPS packet. */
+
+	if ((model->flags & ALPS_PS2_INTERLEAVED) &&
+	    psmouse->pktcnt >= 4 && (psmouse->packet[3] & 0x0f) == 0x0f) {
+		return alps_handle_interleaved_ps2(psmouse);
+	}
+
+	if ((psmouse->packet[0] & model->mask0) != model->byte0) {
+		dbg("refusing packet[0] = %x (mask0 = %x, byte0 = %x)\n",
+		    psmouse->packet[0], model->mask0, model->byte0);
 		return PSMOUSE_BAD_DATA;
+	}
 
 	/* Bytes 2 - 6 should have 0 in the highest bit */
 	if (psmouse->pktcnt >= 2 && psmouse->pktcnt <= 6 &&
-	    (psmouse->packet[psmouse->pktcnt - 1] & 0x80))
+	    (psmouse->packet[psmouse->pktcnt - 1] & 0x80)) {
+		dbg("refusing packet[%i] = %x\n",
+		    psmouse->pktcnt - 1, psmouse->packet[psmouse->pktcnt - 1]);
 		return PSMOUSE_BAD_DATA;
+	}
 
 	if (psmouse->pktcnt == 6) {
 		alps_process_packet(psmouse);
@@ -459,6 +619,7 @@ static void alps_disconnect(struct psmouse *psmouse)
 	struct alps_data *priv = psmouse->private;
 
 	psmouse_reset(psmouse);
+	del_timer_sync(&priv->timer);
 	input_unregister_device(priv->dev2);
 	kfree(priv);
 }
@@ -476,6 +637,8 @@ int alps_init(struct psmouse *psmouse)
 		goto init_fail;
 
 	priv->dev2 = dev2;
+	setup_timer(&priv->timer, alps_flush_packet, (unsigned long)psmouse);
+
 	psmouse->private = priv;
 
 	model = alps_get_model(psmouse, &version);
diff --git a/drivers/input/mouse/alps.h b/drivers/input/mouse/alps.h
index bc87936..ba4a7f5 100644
--- a/drivers/input/mouse/alps.h
+++ b/drivers/input/mouse/alps.h
@@ -15,7 +15,7 @@
 struct alps_model_info {
         unsigned char signature[3];
         unsigned char byte0, mask0;
-        unsigned char flags;
+        unsigned int flags;
 };
 
 struct alps_data {
@@ -23,6 +23,7 @@ struct alps_data {
 	char phys[32];			/* Phys */
 	const struct alps_model_info *i;/* Info */
 	int prev_fin;			/* Finger bit from previous packet */
+	struct timer_list timer;
 };
 
 #ifdef CONFIG_MOUSE_PS2_ALPS
-- 
1.6.3.3

Re: [alps] Timing patch, revised again :-)

[Dmitry Torokhov]

Hi Sebastian,

Sorry for the delay, I finally had the time to look over the patch
again.

On Sun, Nov 29, 2009 at 05:54:55PM +0100, Sebastian Kapfer wrote:
> 
> Dear Dmitry,
> 
> the timer patch so far works great.  You can't desync the touchpad
> anymore by just pressing all buttons.  It still goes wrong when you
> press all buttons and move both touchpad & trackpoint pointer tough :-)
> 

Yeah, but due to protocol limitations we can't really do anything about
it. It should not desync though.

> I've changed two little things:
> 
> 1. The reporting of buttons went to dev2 sometimes, as bare_ps2_packet
> signalled the click on 'dev', not psmouse->dev.  This lead to sticking
> buttons.
> 
> 2. We needlessly break the distinction of PS/2 and touchpad on older
> models, where it may have been cleanly possible.  (If you don't want to
> keep that feature and rather simplify the code, that's fine with me,
> too.  But as I said, bare_ps2_packet needs to be fixed up then.)
> 

I tend to agree with Dave's preference of routing button data from 3-byte
packets to stick only and 6- and 9-byte packets to the pad and sending
release to both devices. I think it makes most sense.

> In the end, I think it's good enough to commit it.  I'm not really happy
> with the fact that we need a timer now, but hey, it wasn't our fault.
> 
> Again, working great as long as you don't do really crazy stuff as
> described above.  I just hope the 20msecs is not a machine parameter
> which just happens to work for me.

No it should not, unless the box is loaded very heavily so that
servicing of interrrupts and timers is delayed severely.

Anyway, I rearranged the patch again a bit, could you please try it out
once more and let me know if it still works and I will apply it so it is
included in 2.6.33.

Thanks!

-- 
Dmitry


Input: ALPS - add interleaved protocol support (Dell E6x00 series)

From: Sebastian Kapfer <sebastian_kapfer@gmx.net>

Properly handle version of the protocol where standard PS/2 packets
from trackpoint are stuffed into middle (byte 3-6) of the standard
ALPS packets when both the touchpad and trackpoint are used together.

The patch is based on work done by Matthew Chapman and additional
research done by David Kubicek and Erik Osterholm:

	https://bugs.launchpad.net/ubuntu/+source/linux/+bug/296610

Many thanks to David Kubicek for his efforts in researching fine points
of this new version of the protocol, especially interaction between pad
and stick in these models.

Signed-off-by: Sebastian Kapfer <sebastian_kapfer@gmx.net>
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/input/mouse/alps.c |  247 +++++++++++++++++++++++++++++++++++++++-----
 drivers/input/mouse/alps.h |    1 
 2 files changed, 221 insertions(+), 27 deletions(-)


diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index a3f492a..a8f6917 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -5,6 +5,7 @@
  * Copyright (c) 2003-2005 Peter Osterlund <petero2@telia.com>
  * Copyright (c) 2004 Dmitry Torokhov <dtor@mail.ru>
  * Copyright (c) 2005 Vojtech Pavlik <vojtech@suse.cz>
+ * Copyright (c) 2009 Sebastian Kapfer <sebastian_kapfer@gmx.net>
  *
  * ALPS detection, tap switching and status querying info is taken from
  * tpconfig utility (by C. Scott Ananian and Bruce Kall).
@@ -28,7 +29,6 @@
 #define dbg(format, arg...) do {} while (0)
 #endif
 
-
 #define ALPS_OLDPROTO		0x01	/* old style input */
 #define ALPS_DUALPOINT		0x02	/* touchpad has trackstick */
 #define ALPS_PASS		0x04	/* device has a pass-through port */
@@ -37,7 +37,8 @@
 #define ALPS_FW_BK_1		0x10	/* front & back buttons present */
 #define ALPS_FW_BK_2		0x20	/* front & back buttons present */
 #define ALPS_FOUR_BUTTONS	0x40	/* 4 direction button present */
-
+#define ALPS_PS2_INTERLEAVED	0x80	/* 3-byte PS/2 packet interleaved with
+					   6-byte ALPS packet */
 
 static const struct alps_model_info alps_model_data[] = {
 	{ { 0x32, 0x02, 0x14 },	0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT }, /* Toshiba Salellite Pro M10 */
@@ -58,7 +59,9 @@ static const struct alps_model_info alps_model_data[] = {
 	{ { 0x20, 0x02, 0x0e },	0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT }, /* XXX */
 	{ { 0x22, 0x02, 0x0a },	0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT },
 	{ { 0x22, 0x02, 0x14 }, 0xff, 0xff, ALPS_PASS | ALPS_DUALPOINT }, /* Dell Latitude D600 */
-	{ { 0x62, 0x02, 0x14 }, 0xcf, 0xcf, ALPS_PASS | ALPS_DUALPOINT }, /* Dell Latitude E6500 */
+	/* Dell Latitude E5500, E6400, E6500, Precision M4400 */
+	{ { 0x62, 0x02, 0x14 }, 0xcf, 0xcf,
+		ALPS_PASS | ALPS_DUALPOINT | ALPS_PS2_INTERLEAVED },
 	{ { 0x73, 0x02, 0x50 }, 0xcf, 0xcf, ALPS_FOUR_BUTTONS },	  /* Dell Vostro 1400 */
 };
 
@@ -69,20 +72,56 @@ static const struct alps_model_info alps_model_data[] = {
  */
 
 /*
- * ALPS abolute Mode - new format
+ * PS/2 packet format
+ *
+ * byte 0:  0    0 YSGN XSGN    1    M    R    L
+ * byte 1: X7   X6   X5   X4   X3   X2   X1   X0
+ * byte 2: Y7   Y6   Y5   Y4   Y3   Y2   Y1   Y0
+ *
+ * Note that the device never signals overflow condition.
+ *
+ * ALPS absolute Mode - new format
  *
  * byte 0:  1    ?    ?    ?    1    ?    ?    ?
  * byte 1:  0   x6   x5   x4   x3   x2   x1   x0
- * byte 2:  0   x10  x9   x8   x7    ?  fin  ges
+ * byte 2:  0  x10   x9   x8   x7    ?  fin  ges
  * byte 3:  0   y9   y8   y7    1    M    R    L
  * byte 4:  0   y6   y5   y4   y3   y2   y1   y0
  * byte 5:  0   z6   z5   z4   z3   z2   z1   z0
  *
+ * Dualpoint device -- interleaved packet format
+ *
+ * byte 0:    1    1    0    0    1    1    1    1
+ * byte 1:    0   x6   x5   x4   x3   x2   x1   x0
+ * byte 2:    0  x10   x9   x8   x7    0  fin  ges
+ * byte 3:    0    0 YSGN XSGN    1    1    1    1
+ * byte 4:   X7   X6   X5   X4   X3   X2   X1   X0
+ * byte 5:   Y7   Y6   Y5   Y4   Y3   Y2   Y1   Y0
+ * byte 6:    0   y9   y8   y7    1    m    r    l
+ * byte 7:    0   y6   y5   y4   y3   y2   y1   y0
+ * byte 8:    0   z6   z5   z4   z3   z2   z1   z0
+ *
+ * CAPITALS = stick, miniscules = touchpad
+ *
  * ?'s can have different meanings on different models,
  * such as wheel rotation, extra buttons, stick buttons
  * on a dualpoint, etc.
  */
 
+static void alps_report_buttons(struct input_dev *dev,
+				int left, int right, int middle,
+				bool release_only)
+{
+	if (!left || !release_only)
+		input_report_key(dev, BTN_LEFT, left);
+
+	if (!right || !release_only)
+		input_report_key(dev, BTN_RIGHT, right);
+
+	if (!middle || !release_only)
+		input_report_key(dev, BTN_MIDDLE, middle);
+}
+
 static void alps_process_packet(struct psmouse *psmouse)
 {
 	struct alps_data *priv = psmouse->private;
@@ -93,18 +132,6 @@ static void alps_process_packet(struct psmouse *psmouse)
 	int x, y, z, ges, fin, left, right, middle;
 	int back = 0, forward = 0;
 
-	if ((packet[0] & 0xc8) == 0x08) {   /* 3-byte PS/2 packet */
-		input_report_key(dev2, BTN_LEFT,   packet[0] & 1);
-		input_report_key(dev2, BTN_RIGHT,  packet[0] & 2);
-		input_report_key(dev2, BTN_MIDDLE, packet[0] & 4);
-		input_report_rel(dev2, REL_X,
-			packet[1] ? packet[1] - ((packet[0] << 4) & 0x100) : 0);
-		input_report_rel(dev2, REL_Y,
-			packet[2] ? ((packet[0] << 3) & 0x100) - packet[2] : 0);
-		input_sync(dev2);
-		return;
-	}
-
 	if (model->flags & ALPS_OLDPROTO) {
 		left = packet[2] & 0x10;
 		right = packet[2] & 0x08;
@@ -140,18 +167,25 @@ static void alps_process_packet(struct psmouse *psmouse)
 		input_report_rel(dev2, REL_X,  (x > 383 ? (x - 768) : x));
 		input_report_rel(dev2, REL_Y, -(y > 255 ? (y - 512) : y));
 
-		input_report_key(dev2, BTN_LEFT, left);
-		input_report_key(dev2, BTN_RIGHT, right);
-		input_report_key(dev2, BTN_MIDDLE, middle);
+		alps_report_buttons(dev2, left, right, middle, false);
 
-		input_sync(dev);
 		input_sync(dev2);
+		input_sync(dev);
 		return;
 	}
 
-	input_report_key(dev, BTN_LEFT, left);
-	input_report_key(dev, BTN_RIGHT, right);
-	input_report_key(dev, BTN_MIDDLE, middle);
+	alps_report_buttons(dev, left, right, middle, false);
+
+	if (model->flags & ALPS_PS2_INTERLEAVED) {
+		/*
+		 * On devices using interleaved packets, when user presses
+		 * the same button on both trackpoint and touchpad, the
+		 * release for the trackpoint is not reported so we have
+		 * send release events to both devices.
+		 */
+		alps_report_buttons(dev2, left, right, middle, true);
+		input_sync(dev2);
+	}
 
 	/* Convert hardware tap to a reasonable Z value */
 	if (ges && !fin)
@@ -202,25 +236,181 @@ static void alps_process_packet(struct psmouse *psmouse)
 	input_sync(dev);
 }
 
+static void alps_report_bare_ps2_packet(struct psmouse *psmouse,
+					unsigned char packet[],
+					bool report_buttons)
+{
+	struct alps_data *priv = psmouse->private;
+	struct input_dev *dev2 = priv->dev2;
+
+	if (report_buttons) {
+		int left = packet[0] & 1;
+		int right = packet[0] & 2;
+		int middle = packet[0] & 4;
+
+
+		if (priv->i->flags & ALPS_PS2_INTERLEAVED) {
+			alps_report_buttons(psmouse->dev,
+					    left, right, middle, true);
+			input_sync(psmouse->dev);
+		}
+
+		alps_report_buttons(dev2, left, right, middle, false);
+	}
+
+	input_report_rel(dev2, REL_X,
+		packet[1] ? packet[1] - ((packet[0] << 4) & 0x100) : 0);
+	input_report_rel(dev2, REL_Y,
+		packet[2] ? ((packet[0] << 3) & 0x100) - packet[2] : 0);
+
+	input_sync(dev2);
+}
+
+static bool alps_is_valid_first_byte(const struct alps_model_info *model,
+				     unsigned char data)
+{
+	return (data & model->mask0) == model->byte0;
+}
+
+static psmouse_ret_t alps_handle_interleaved_ps2(struct psmouse *psmouse)
+{
+	struct alps_data *priv = psmouse->private;
+
+	if (psmouse->pktcnt < 6)
+		return PSMOUSE_GOOD_DATA;
+
+	if (psmouse->pktcnt == 6) {
+		/*
+		 * Start a timer to flush the packet if it ends up last
+		 * 6-byte packet in the stream. Timer needs to fire
+		 * psmouse core times out itself. 20 ms should be enough
+		 * to decide if we are getting more data or not.
+		 */
+		mod_timer(&priv->timer, jiffies + msecs_to_jiffies(20));
+		return PSMOUSE_GOOD_DATA;
+	}
+
+	del_timer(&priv->timer);
+
+	if (psmouse->packet[6] & 0x80) {
+
+		/*
+		 * Highest bit is set - that means we either had
+		 * complete ALPS packet and this is start of the
+		 * next packet or we got garbage.
+		 */
+
+		if (((psmouse->packet[3] |
+		      psmouse->packet[4] |
+		      psmouse->packet[5]) & 0x80) ||
+		    (!alps_is_valid_first_byte(priv->i, psmouse->packet[6]))) {
+			dbg("refusing packet %x %x %x %x "
+			    "(suspected interleaved ps/2)\n",
+			    psmouse->packet[3], psmouse->packet[4],
+			    psmouse->packet[5], psmouse->packet[6]);
+			return PSMOUSE_BAD_DATA;
+		}
+
+		alps_process_packet(psmouse);
+
+		/* Continue with the next packet */
+		psmouse->packet[0] = psmouse->packet[6];
+		psmouse->pktcnt = 1;
+
+	} else {
+
+		/*
+		 * High bit is 0 - that means that we indeed got a PS/2
+		 * packet in the middle of ALPS packet.
+		 *
+		 * There is also possibility that we got 6-byte ALPS
+		 * packet followed  by 3-byte packet from trackpoint. We
+		 * can not distinguish between these 2 scenarios but
+		 * becase the latter is unlikely to happen in course of
+		 * normal operation (user would need to press all
+		 * buttons on the pad and start moving trackpoint
+		 * without touching the pad surface) we assume former.
+		 * Even if we are wrong the wost thing that would happen
+		 * the cursor would jump but we should not get protocol
+		 * desynchronization.
+		 */
+
+		alps_report_bare_ps2_packet(psmouse, &psmouse->packet[3],
+					    false);
+
+		/*
+		 * Continue with the standard ALPS protocol handling
+		 */
+		psmouse->packet[3] = psmouse->packet[6];
+		psmouse->pktcnt = 4;
+	}
+
+	return PSMOUSE_GOOD_DATA;
+}
+
+static void alps_flush_packet(unsigned long data)
+{
+	struct psmouse *psmouse = (struct psmouse *)data;
+
+	serio_pause_rx(psmouse->ps2dev.serio);
+
+	if (psmouse->pktcnt == 6) {
+
+		/*
+		 * We did not any more data in reasonable amount of time.
+		 * Validate the last 3 bytes and process as a standard
+		 * ALPS packet.
+		 */
+		if ((psmouse->packet[3] |
+		     psmouse->packet[4] |
+		     psmouse->packet[5]) & 0x80) {
+			dbg("refusing packet %x %x %x "
+			    "(suspected interleaved ps/2)\n",
+			    psmouse->packet[3], psmouse->packet[4],
+			    psmouse->packet[5]);
+		} else {
+			alps_process_packet(psmouse);
+		}
+		psmouse->pktcnt = 0;
+	}
+
+	serio_continue_rx(psmouse->ps2dev.serio);
+}
+
 static psmouse_ret_t alps_process_byte(struct psmouse *psmouse)
 {
 	struct alps_data *priv = psmouse->private;
+	const struct alps_model_info *model = priv->i;
 
 	if ((psmouse->packet[0] & 0xc8) == 0x08) { /* PS/2 packet */
 		if (psmouse->pktcnt == 3) {
-			alps_process_packet(psmouse);
+			alps_report_bare_ps2_packet(psmouse, psmouse->packet,
+						    true);
 			return PSMOUSE_FULL_PACKET;
 		}
 		return PSMOUSE_GOOD_DATA;
 	}
 
-	if ((psmouse->packet[0] & priv->i->mask0) != priv->i->byte0)
+	/* Check for PS/2 packet stuffed in the middle of ALPS packet. */
+
+	if ((model->flags & ALPS_PS2_INTERLEAVED) &&
+	    psmouse->pktcnt >= 4 && (psmouse->packet[3] & 0x0f) == 0x0f) {
+		return alps_handle_interleaved_ps2(psmouse);
+	}
+
+	if (!alps_is_valid_first_byte(model, psmouse->packet[0])) {
+		dbg("refusing packet[0] = %x (mask0 = %x, byte0 = %x)\n",
+		    psmouse->packet[0], model->mask0, model->byte0);
 		return PSMOUSE_BAD_DATA;
+	}
 
 	/* Bytes 2 - 6 should have 0 in the highest bit */
 	if (psmouse->pktcnt >= 2 && psmouse->pktcnt <= 6 &&
-	    (psmouse->packet[psmouse->pktcnt - 1] & 0x80))
+	    (psmouse->packet[psmouse->pktcnt - 1] & 0x80)) {
+		dbg("refusing packet[%i] = %x\n",
+		    psmouse->pktcnt - 1, psmouse->packet[psmouse->pktcnt - 1]);
 		return PSMOUSE_BAD_DATA;
+	}
 
 	if (psmouse->pktcnt == 6) {
 		alps_process_packet(psmouse);
@@ -459,6 +649,7 @@ static void alps_disconnect(struct psmouse *psmouse)
 	struct alps_data *priv = psmouse->private;
 
 	psmouse_reset(psmouse);
+	del_timer_sync(&priv->timer);
 	input_unregister_device(priv->dev2);
 	kfree(priv);
 }
@@ -476,6 +667,8 @@ int alps_init(struct psmouse *psmouse)
 		goto init_fail;
 
 	priv->dev2 = dev2;
+	setup_timer(&priv->timer, alps_flush_packet, (unsigned long)psmouse);
+
 	psmouse->private = priv;
 
 	model = alps_get_model(psmouse, &version);
diff --git a/drivers/input/mouse/alps.h b/drivers/input/mouse/alps.h
index bc87936..904ed8b 100644
--- a/drivers/input/mouse/alps.h
+++ b/drivers/input/mouse/alps.h
@@ -23,6 +23,7 @@ struct alps_data {
 	char phys[32];			/* Phys */
 	const struct alps_model_info *i;/* Info */
 	int prev_fin;			/* Finger bit from previous packet */
+	struct timer_list timer;
 };
 
 #ifdef CONFIG_MOUSE_PS2_ALPS

Re: [alps] Timing patch, revised again :-)

[Sebastian Kapfer]

Hi Dmitry,

thank you.  I've tested it and it basically works.  I still have two
more nits to pick though :-)

> Yeah, but due to protocol limitations we can't really do anything about
> it. It should not desync though.

It does still occasionally desync (but see below).

> > I've changed two little things:
> > 
> > 1. The reporting of buttons went to dev2 sometimes, as bare_ps2_packet
> > signalled the click on 'dev', not psmouse->dev.  This lead to sticking
> > buttons.
> > 
> > 2. We needlessly break the distinction of PS/2 and touchpad on older
> > models, where it may have been cleanly possible.  (If you don't want to
> > keep that feature and rather simplify the code, that's fine with me,
> > too.  But as I said, bare_ps2_packet needs to be fixed up then.)
> > 
> 
> I tend to agree with Dave's preference of routing button data from 3-byte
> packets to stick only and 6- and 9-byte packets to the pad and sending
> release to both devices. I think it makes most sense.

It does, in a way.  All I'm saying is that we're making a promise to
userspace clients that we can't completely fulfil (i.e. some buttons
will be reported incorrectly).  Anyway, I really don't want to argue
about this particular point anymore given Dave's previous attitude.


What is more worrysome to me is that we're now reporting the same
physical click on two input layer devices again.  We should not do this,
because this can lead to spurious double-click events.  Imagine for
example moving the touchpad while pressing the trackpoint button:

6byte -- no buttons set
3byte -- left set  (in response to user click)        dev2 down
6byte -- left set  (hw copies over from trackpoint)   dev down
3byte -- no buttons set (in response to user release) dev2, dev release

Given unlucky timing and the way X11 reads event data, this is an X11
double-click.

Reading the comments in the patch, I'm not sure if you're aware how the
hardware reports buttons.  When I press the trackpoint button, the
corresponding bit is set in _all_ packets (3s, 6s, 9s) sent
subsequently, until I release it.  This is not about pressing two
buttons at the same time, just one.  Sorry if I'm reading too much into
that comment and you already know this.

Given this behaviour of the hw, I'd favour not reporting button presses
on a device while the corresponding button on the other device is down.
(Dave called this behaviour 'masking'.)  Code implementing this was in
the patch I sent to linux-input dated Nov. 11 (see the parts involving
the btn_state variable).  I have not put it back in the patch below
because I'd like to await your opinion on this first.


There is still one failure mode left that causes de-sync.  It happens
when the else branch in  alps_handle_interleaved_ps2 gets called more
than once, i.e. we're accidentially reconstructing a 12-, 15- etc byte
packet.  This was easier to deal with in my first patch, I just
collected the whole 9 bytes in a buffer and implicitly knew when the
packet was over.  Example of this happening:

Dec  4 21:03:22 sardelle kernel: [410740.786121] alps.c: handle: cf
Dec  4 21:03:22 sardelle kernel: [410740.787499] alps.c: handle: 79
Dec  4 21:03:22 sardelle kernel: [410740.788688] alps.c: handle: 12
Dec  4 21:03:22 sardelle kernel: [410740.789979] alps.c: handle: 1f
Dec  4 21:03:22 sardelle kernel: [410740.791146] alps.c: handle: ff
Dec  4 21:03:22 sardelle kernel: [410740.792299] alps.c: handle: 1
<suspect 9byte (really is)>
Dec  4 21:03:22 sardelle kernel: [410740.796899] alps.c: handle: 4f
<yup, it is, fold back>
Dec  4 21:03:22 sardelle kernel: [410740.798069] alps.c: handle: 3c
Dec  4 21:03:22 sardelle kernel: [410740.799255] alps.c: handle: 5a
<packet should have ended here>
Dec  4 21:03:22 sardelle kernel: [410740.803803] alps.c: handle: f
<fold again>
Dec  4 21:03:22 sardelle kernel: [410740.804811] alps.c: handle: 0
Dec  4 21:03:22 sardelle kernel: [410740.806008] alps.c: handle: 1
Dec  4 21:03:22 sardelle kernel: [410740.815478] alps.c: handle: 1f
<fold again>
Dec  4 21:03:22 sardelle kernel: [410740.816656] alps.c: handle: ff
Dec  4 21:03:22 sardelle kernel: [410740.817817] alps.c: handle: 1
Dec  4 21:03:22 sardelle kernel: [410740.828146] alps.c: handle: f
<fold again>
Dec  4 21:03:22 sardelle kernel: [410740.829328] alps.c: handle: 0
Dec  4 21:03:22 sardelle kernel: [410740.830498] alps.c: handle: 1
Dec  4 21:03:22 sardelle kernel: [410740.840918] alps.c: handle: 1f
<fold again>
Dec  4 21:03:22 sardelle kernel: [410740.842035] alps.c: handle: ff
Dec  4 21:03:22 sardelle kernel: [410740.843201] alps.c: handle: 2
Dec  4 21:03:22 sardelle kernel: [410740.853508] alps.c: handle: 1f
<fold again>
Dec  4 21:03:22 sardelle kernel: [410740.854718] alps.c: handle: ff
Dec  4 21:03:22 sardelle kernel: [410740.855871] alps.c: handle: 1
Dec  4 21:03:22 sardelle kernel: [410740.866158] alps.c: handle: 1f
<fold again>
Dec  4 21:03:22 sardelle kernel: [410740.867329] alps.c: handle: ff
Dec  4 21:03:22 sardelle kernel: [410740.868491] alps.c: handle: 2
Dec  4 21:03:22 sardelle kernel: [410740.878770] alps.c: handle: f
<fold again>
Dec  4 21:03:22 sardelle kernel: [410740.879932] alps.c: handle: 0
Dec  4 21:03:22 sardelle kernel: [410740.881108] alps.c: handle: 2
Dec  4 21:03:22 sardelle kernel: [410740.891326] alps.c: handle: 1f
<fold again>
Dec  4 21:03:22 sardelle kernel: [410740.892492] alps.c: handle: ff
Dec  4 21:03:22 sardelle kernel: [410740.893663] alps.c: handle: 1
Dec  4 21:03:22 sardelle kernel: [410740.903987] alps.c: handle: 1f
<fold again>
Dec  4 21:03:22 sardelle kernel: [410740.905166] alps.c: handle: ff
Dec  4 21:03:22 sardelle kernel: [410740.906329] alps.c: handle: 2
Dec  4 21:03:22 sardelle kernel: [410740.910832] alps.c: handle: cf
<oops, something's wrong!>
Dec  4 21:03:22 sardelle kernel: [410740.910839] alps.c: refusing packet
1f ff 2 cf (suspected interleaved ps/2)


I've taken the liberty to fix this, see below.

Many greetings,
   Sebastian





Input: ALPS - add interleaved protocol support (Dell E6x00 series)

From: Sebastian Kapfer <sebastian_kapfer@gmx.net>

Properly handle version of the protocol where standard PS/2 packets
from trackpoint are stuffed into middle (byte 3-6) of the standard
ALPS packets when both the touchpad and trackpoint are used together.

The patch is based on work done by Matthew Chapman and additional
research done by David Kubicek and Erik Osterholm:

        https://bugs.launchpad.net/ubuntu/+source/linux/+bug/296610

Many thanks to David Kubicek for his efforts in researching fine points
of this new version of the protocol, especially interaction between pad
and stick in these models.

Signed-off-by: Sebastian Kapfer <sebastian_kapfer@gmx.net>

---
 drivers/input/mouse/alps.c    |  254 ++++++++++++++++++++++++++++++++++++-----
 drivers/input/mouse/alps.h    |    1 +
 drivers/input/mouse/psmouse.h |    2 +-
 3 files changed, 229 insertions(+), 28 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index a3f492a..71b40ae 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -5,6 +5,7 @@
  * Copyright (c) 2003-2005 Peter Osterlund <petero2@telia.com>
  * Copyright (c) 2004 Dmitry Torokhov <dtor@mail.ru>
  * Copyright (c) 2005 Vojtech Pavlik <vojtech@suse.cz>
+ * Copyright (c) 2009 Sebastian Kapfer <sebastian_kapfer@gmx.net>
  *
  * ALPS detection, tap switching and status querying info is taken from
  * tpconfig utility (by C. Scott Ananian and Bruce Kall).
@@ -28,7 +29,6 @@
 #define dbg(format, arg...) do {} while (0)
 #endif
 
-
 #define ALPS_OLDPROTO		0x01	/* old style input */
 #define ALPS_DUALPOINT		0x02	/* touchpad has trackstick */
 #define ALPS_PASS		0x04	/* device has a pass-through port */
@@ -37,7 +37,8 @@
 #define ALPS_FW_BK_1		0x10	/* front & back buttons present */
 #define ALPS_FW_BK_2		0x20	/* front & back buttons present */
 #define ALPS_FOUR_BUTTONS	0x40	/* 4 direction button present */
-
+#define ALPS_PS2_INTERLEAVED	0x80	/* 3-byte PS/2 packet interleaved with
+					   6-byte ALPS packet */
 
 static const struct alps_model_info alps_model_data[] = {
 	{ { 0x32, 0x02, 0x14 },	0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT }, /* Toshiba Salellite Pro M10 */
@@ -58,7 +59,9 @@ static const struct alps_model_info alps_model_data[] = {
 	{ { 0x20, 0x02, 0x0e },	0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT }, /* XXX */
 	{ { 0x22, 0x02, 0x0a },	0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT },
 	{ { 0x22, 0x02, 0x14 }, 0xff, 0xff, ALPS_PASS | ALPS_DUALPOINT }, /* Dell Latitude D600 */
-	{ { 0x62, 0x02, 0x14 }, 0xcf, 0xcf, ALPS_PASS | ALPS_DUALPOINT }, /* Dell Latitude E6500 */
+	/* Dell Latitude E5500, E6400, E6500, Precision M4400 */
+	{ { 0x62, 0x02, 0x14 }, 0xcf, 0xcf,
+		ALPS_PASS | ALPS_DUALPOINT | ALPS_PS2_INTERLEAVED },
 	{ { 0x73, 0x02, 0x50 }, 0xcf, 0xcf, ALPS_FOUR_BUTTONS },	  /* Dell Vostro 1400 */
 };
 
@@ -69,20 +72,56 @@ static const struct alps_model_info alps_model_data[] = {
  */
 
 /*
- * ALPS abolute Mode - new format
+ * PS/2 packet format
+ *
+ * byte 0:  0    0 YSGN XSGN    1    M    R    L
+ * byte 1: X7   X6   X5   X4   X3   X2   X1   X0
+ * byte 2: Y7   Y6   Y5   Y4   Y3   Y2   Y1   Y0
+ *
+ * Note that the device never signals overflow condition.
+ *
+ * ALPS absolute Mode - new format
  *
  * byte 0:  1    ?    ?    ?    1    ?    ?    ?
  * byte 1:  0   x6   x5   x4   x3   x2   x1   x0
- * byte 2:  0   x10  x9   x8   x7    ?  fin  ges
+ * byte 2:  0  x10   x9   x8   x7    ?  fin  ges
  * byte 3:  0   y9   y8   y7    1    M    R    L
  * byte 4:  0   y6   y5   y4   y3   y2   y1   y0
  * byte 5:  0   z6   z5   z4   z3   z2   z1   z0
  *
+ * Dualpoint device -- interleaved packet format
+ *
+ * byte 0:    1    1    0    0    1    1    1    1
+ * byte 1:    0   x6   x5   x4   x3   x2   x1   x0
+ * byte 2:    0  x10   x9   x8   x7    0  fin  ges
+ * byte 3:    0    0 YSGN XSGN    1    1    1    1
+ * byte 4:   X7   X6   X5   X4   X3   X2   X1   X0
+ * byte 5:   Y7   Y6   Y5   Y4   Y3   Y2   Y1   Y0
+ * byte 6:    0   y9   y8   y7    1    m    r    l
+ * byte 7:    0   y6   y5   y4   y3   y2   y1   y0
+ * byte 8:    0   z6   z5   z4   z3   z2   z1   z0
+ *
+ * CAPITALS = stick, miniscules = touchpad
+ *
  * ?'s can have different meanings on different models,
  * such as wheel rotation, extra buttons, stick buttons
  * on a dualpoint, etc.
  */
 
+static void alps_report_buttons(struct input_dev *dev,
+				int left, int right, int middle,
+				bool release_only)
+{
+	if (!left || !release_only)
+		input_report_key(dev, BTN_LEFT, left);
+
+	if (!right || !release_only)
+		input_report_key(dev, BTN_RIGHT, right);
+
+	if (!middle || !release_only)
+		input_report_key(dev, BTN_MIDDLE, middle);
+}
+
 static void alps_process_packet(struct psmouse *psmouse)
 {
 	struct alps_data *priv = psmouse->private;
@@ -93,18 +132,6 @@ static void alps_process_packet(struct psmouse *psmouse)
 	int x, y, z, ges, fin, left, right, middle;
 	int back = 0, forward = 0;
 
-	if ((packet[0] & 0xc8) == 0x08) {   /* 3-byte PS/2 packet */
-		input_report_key(dev2, BTN_LEFT,   packet[0] & 1);
-		input_report_key(dev2, BTN_RIGHT,  packet[0] & 2);
-		input_report_key(dev2, BTN_MIDDLE, packet[0] & 4);
-		input_report_rel(dev2, REL_X,
-			packet[1] ? packet[1] - ((packet[0] << 4) & 0x100) : 0);
-		input_report_rel(dev2, REL_Y,
-			packet[2] ? ((packet[0] << 3) & 0x100) - packet[2] : 0);
-		input_sync(dev2);
-		return;
-	}
-
 	if (model->flags & ALPS_OLDPROTO) {
 		left = packet[2] & 0x10;
 		right = packet[2] & 0x08;
@@ -140,18 +167,25 @@ static void alps_process_packet(struct psmouse *psmouse)
 		input_report_rel(dev2, REL_X,  (x > 383 ? (x - 768) : x));
 		input_report_rel(dev2, REL_Y, -(y > 255 ? (y - 512) : y));
 
-		input_report_key(dev2, BTN_LEFT, left);
-		input_report_key(dev2, BTN_RIGHT, right);
-		input_report_key(dev2, BTN_MIDDLE, middle);
+		alps_report_buttons(dev2, left, right, middle, false);
 
-		input_sync(dev);
 		input_sync(dev2);
+		input_sync(dev);
 		return;
 	}
 
-	input_report_key(dev, BTN_LEFT, left);
-	input_report_key(dev, BTN_RIGHT, right);
-	input_report_key(dev, BTN_MIDDLE, middle);
+	alps_report_buttons(dev, left, right, middle, false);
+
+	if (model->flags & ALPS_PS2_INTERLEAVED) {
+		/*
+		 * On devices using interleaved packets, when user presses
+		 * the same button on both trackpoint and touchpad, the
+		 * release for the trackpoint is not reported so we have
+		 * send release events to both devices.
+		 */
+		alps_report_buttons(dev2, left, right, middle, true);
+		input_sync(dev2);
+	}
 
 	/* Convert hardware tap to a reasonable Z value */
 	if (ges && !fin)
@@ -202,25 +236,188 @@ static void alps_process_packet(struct psmouse *psmouse)
 	input_sync(dev);
 }
 
+static void alps_report_bare_ps2_packet(struct psmouse *psmouse,
+					unsigned char packet[],
+					bool report_buttons)
+{
+	struct alps_data *priv = psmouse->private;
+	struct input_dev *dev2 = priv->dev2;
+
+	if (report_buttons) {
+		int left = packet[0] & 1;
+		int right = packet[0] & 2;
+		int middle = packet[0] & 4;
+
+
+		if (priv->i->flags & ALPS_PS2_INTERLEAVED) {
+			alps_report_buttons(psmouse->dev,
+					    left, right, middle, true);
+			input_sync(psmouse->dev);
+		}
+
+		alps_report_buttons(dev2, left, right, middle, false);
+	}
+
+	input_report_rel(dev2, REL_X,
+		packet[1] ? packet[1] - ((packet[0] << 4) & 0x100) : 0);
+	input_report_rel(dev2, REL_Y,
+		packet[2] ? ((packet[0] << 3) & 0x100) - packet[2] : 0);
+
+	input_sync(dev2);
+}
+
+static bool alps_is_valid_first_byte(const struct alps_model_info *model,
+				     unsigned char data)
+{
+	return (data & model->mask0) == model->byte0;
+}
+
+static psmouse_ret_t alps_handle_interleaved_ps2(struct psmouse *psmouse)
+{
+	struct alps_data *priv = psmouse->private;
+
+	if (psmouse->pktcnt < 6)
+		return PSMOUSE_GOOD_DATA;
+
+	if (psmouse->pktcnt == 6) {
+		/*
+		 * Start a timer to flush the packet if it ends up last
+		 * 6-byte packet in the stream. Timer needs to fire
+		 * psmouse core times out itself. 20 ms should be enough
+		 * to decide if we are getting more data or not.
+		 */
+		mod_timer(&priv->timer, jiffies + msecs_to_jiffies(20));
+		return PSMOUSE_GOOD_DATA;
+	}
+
+	del_timer(&priv->timer);
+
+	if (psmouse->packet[6] & 0x80) {
+
+		/*
+		 * Highest bit is set - that means we either had
+		 * complete ALPS packet and this is start of the
+		 * next packet or we got garbage.
+		 */
+
+		if (((psmouse->packet[3] |
+		      psmouse->packet[4] |
+		      psmouse->packet[5]) & 0x80) ||
+		    (!alps_is_valid_first_byte(priv->i, psmouse->packet[6]))) {
+			dbg("refusing packet %x %x %x %x "
+			    "(suspected interleaved ps/2)\n",
+			    psmouse->packet[3], psmouse->packet[4],
+			    psmouse->packet[5], psmouse->packet[6]);
+			return PSMOUSE_BAD_DATA;
+		}
+
+		alps_process_packet(psmouse);
+
+		/* Continue with the next packet */
+		psmouse->packet[0] = psmouse->packet[6];
+		psmouse->pktcnt = 1;
+
+	} else {
+
+		/*
+		 * High bit is 0 - that means that we indeed got a PS/2
+		 * packet in the middle of ALPS packet.
+		 *
+		 * There is also possibility that we got 6-byte ALPS
+		 * packet followed  by 3-byte packet from trackpoint. We
+		 * can not distinguish between these 2 scenarios but
+		 * becase the latter is unlikely to happen in course of
+		 * normal operation (user would need to press all
+		 * buttons on the pad and start moving trackpoint
+		 * without touching the pad surface) we assume former.
+		 * Even if we are wrong the wost thing that would happen
+		 * the cursor would jump but we should not get protocol
+		 * desynchronization.
+		 */
+
+		if (psmouse->pktcnt == 9) {
+			/* Process interleaved PS/2 data */
+			alps_report_bare_ps2_packet(psmouse,
+				&psmouse->packet[3], false);
+
+			/* Continue with the standard ALPS protocol handling */
+			psmouse->packet[3] = psmouse->packet[6];
+			psmouse->packet[4] = psmouse->packet[7];
+			psmouse->packet[5] = psmouse->packet[8];
+			psmouse->pktcnt = 6;
+			alps_process_packet(psmouse);
+			return PSMOUSE_FULL_PACKET;
+		}
+	}
+
+	return PSMOUSE_GOOD_DATA;
+}
+
+static void alps_flush_packet(unsigned long data)
+{
+	struct psmouse *psmouse = (struct psmouse *)data;
+
+	serio_pause_rx(psmouse->ps2dev.serio);
+
+	if (psmouse->pktcnt == 6) {
+
+		/*
+		 * We did not any more data in reasonable amount of time.
+		 * Validate the last 3 bytes and process as a standard
+		 * ALPS packet.
+		 */
+		if ((psmouse->packet[3] |
+		     psmouse->packet[4] |
+		     psmouse->packet[5]) & 0x80) {
+			dbg("refusing packet %x %x %x "
+			    "(suspected interleaved ps/2)\n",
+			    psmouse->packet[3], psmouse->packet[4],
+			    psmouse->packet[5]);
+		} else {
+			alps_process_packet(psmouse);
+		}
+		psmouse->pktcnt = 0;
+	}
+
+	serio_continue_rx(psmouse->ps2dev.serio);
+}
+
 static psmouse_ret_t alps_process_byte(struct psmouse *psmouse)
 {
 	struct alps_data *priv = psmouse->private;
+	const struct alps_model_info *model = priv->i;
+
+        dbg ("handle: %x", psmouse->packet[psmouse->pktcnt-1]);
 
 	if ((psmouse->packet[0] & 0xc8) == 0x08) { /* PS/2 packet */
 		if (psmouse->pktcnt == 3) {
-			alps_process_packet(psmouse);
+			alps_report_bare_ps2_packet(psmouse, psmouse->packet,
+						    true);
 			return PSMOUSE_FULL_PACKET;
 		}
 		return PSMOUSE_GOOD_DATA;
 	}
 
-	if ((psmouse->packet[0] & priv->i->mask0) != priv->i->byte0)
+	/* Check for PS/2 packet stuffed in the middle of ALPS packet. */
+
+	if ((model->flags & ALPS_PS2_INTERLEAVED) &&
+	    psmouse->pktcnt >= 4 && (psmouse->packet[3] & 0x0f) == 0x0f) {
+		return alps_handle_interleaved_ps2(psmouse);
+	}
+
+	if (!alps_is_valid_first_byte(model, psmouse->packet[0])) {
+		dbg("refusing packet[0] = %x (mask0 = %x, byte0 = %x)\n",
+		    psmouse->packet[0], model->mask0, model->byte0);
 		return PSMOUSE_BAD_DATA;
+	}
 
 	/* Bytes 2 - 6 should have 0 in the highest bit */
 	if (psmouse->pktcnt >= 2 && psmouse->pktcnt <= 6 &&
-	    (psmouse->packet[psmouse->pktcnt - 1] & 0x80))
+	    (psmouse->packet[psmouse->pktcnt - 1] & 0x80)) {
+		dbg("refusing packet[%i] = %x\n",
+		    psmouse->pktcnt - 1, psmouse->packet[psmouse->pktcnt - 1]);
 		return PSMOUSE_BAD_DATA;
+	}
 
 	if (psmouse->pktcnt == 6) {
 		alps_process_packet(psmouse);
@@ -459,6 +656,7 @@ static void alps_disconnect(struct psmouse *psmouse)
 	struct alps_data *priv = psmouse->private;
 
 	psmouse_reset(psmouse);
+	del_timer_sync(&priv->timer);
 	input_unregister_device(priv->dev2);
 	kfree(priv);
 }
@@ -476,6 +674,8 @@ int alps_init(struct psmouse *psmouse)
 		goto init_fail;
 
 	priv->dev2 = dev2;
+	setup_timer(&priv->timer, alps_flush_packet, (unsigned long)psmouse);
+
 	psmouse->private = priv;
 
 	model = alps_get_model(psmouse, &version);
diff --git a/drivers/input/mouse/alps.h b/drivers/input/mouse/alps.h
index bc87936..904ed8b 100644
--- a/drivers/input/mouse/alps.h
+++ b/drivers/input/mouse/alps.h
@@ -23,6 +23,7 @@ struct alps_data {
 	char phys[32];			/* Phys */
 	const struct alps_model_info *i;/* Info */
 	int prev_fin;			/* Finger bit from previous packet */
+	struct timer_list timer;
 };
 
 #ifdef CONFIG_MOUSE_PS2_ALPS
diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h
index e053bdd..d4772fe 100644
--- a/drivers/input/mouse/psmouse.h
+++ b/drivers/input/mouse/psmouse.h
@@ -42,7 +42,7 @@ struct psmouse {
 	struct delayed_work resync_work;
 	char *vendor;
 	char *name;
-	unsigned char packet[8];
+	unsigned char packet[9];
 	unsigned char badbyte;
 	unsigned char pktcnt;
 	unsigned char pktsize;
-- 
1.6.3.3

Re: [alps] Timing patch, revised again :-)

[Dmitry Torokhov]

On Friday 04 December 2009 01:47:20 pm Sebastian Kapfer wrote:
> Hi Dmitry,
> 
> thank you.  I've tested it and it basically works.  I still have two
> more nits to pick though :-)
> 
> > Yeah, but due to protocol limitations we can't really do anything about
> > it. It should not desync though.
> 
> It does still occasionally desync (but see below).
> 
> > > I've changed two little things:
> > >
> > > 1. The reporting of buttons went to dev2 sometimes, as bare_ps2_packet
> > > signalled the click on 'dev', not psmouse->dev.  This lead to sticking
> > > buttons.
> > >
> > > 2. We needlessly break the distinction of PS/2 and touchpad on older
> > > models, where it may have been cleanly possible.  (If you don't want to
> > > keep that feature and rather simplify the code, that's fine with me,
> > > too.  But as I said, bare_ps2_packet needs to be fixed up then.)
> >
> > I tend to agree with Dave's preference of routing button data from 3-byte
> > packets to stick only and 6- and 9-byte packets to the pad and sending
> > release to both devices. I think it makes most sense.
> 
> It does, in a way.  All I'm saying is that we're making a promise to
> userspace clients that we can't completely fulfil (i.e. some buttons
> will be reported incorrectly).  Anyway, I really don't want to argue
> about this particular point anymore given Dave's previous attitude.
> 
> 
> What is more worrysome to me is that we're now reporting the same
> physical click on two input layer devices again.  We should not do this,
> because this can lead to spurious double-click events.  Imagine for
> example moving the touchpad while pressing the trackpoint button:
> 
> 6byte -- no buttons set
> 3byte -- left set  (in response to user click)        dev2 down
> 6byte -- left set  (hw copies over from trackpoint)   dev down
> 3byte -- no buttons set (in response to user release) dev2, dev release
> 
> Given unlucky timing and the way X11 reads event data, this is an X11
> double-click.

Hmm, I see... Right, we should not leave it like this.

> 
> Reading the comments in the patch, I'm not sure if you're aware how the
> hardware reports buttons.  When I press the trackpoint button, the
> corresponding bit is set in _all_ packets (3s, 6s, 9s) sent
> subsequently, until I release it.  This is not about pressing two
> buttons at the same time, just one.  Sorry if I'm reading too much into
> that comment and you already know this.
> 
> Given this behaviour of the hw, I'd favour not reporting button presses
> on a device while the corresponding button on the other device is down.
> (Dave called this behaviour 'masking'.)  Code implementing this was in
> the patch I sent to linux-input dated Nov. 11 (see the parts involving
> the btn_state variable).  I have not put it back in the patch below
> because I'd like to await your opinion on this first.

OK, Let me take a look at that patch again.
 
> 
> There is still one failure mode left that causes de-sync.  It happens
> when the else branch in  alps_handle_interleaved_ps2 gets called more
> than once, i.e. we're accidentially reconstructing a 12-, 15- etc byte
> packet.  This was easier to deal with in my first patch, I just
> collected the whole 9 bytes in a buffer and implicitly knew when the
> packet was over.  Example of this happening:
> 
> Dec  4 21:03:22 sardelle kernel: [410740.786121] alps.c: handle: cf
> Dec  4 21:03:22 sardelle kernel: [410740.787499] alps.c: handle: 79
> Dec  4 21:03:22 sardelle kernel: [410740.788688] alps.c: handle: 12
> Dec  4 21:03:22 sardelle kernel: [410740.789979] alps.c: handle: 1f
> Dec  4 21:03:22 sardelle kernel: [410740.791146] alps.c: handle: ff
> Dec  4 21:03:22 sardelle kernel: [410740.792299] alps.c: handle: 1
> <suspect 9byte (really is)>
> Dec  4 21:03:22 sardelle kernel: [410740.796899] alps.c: handle: 4f
> <yup, it is, fold back>

Ah, ok, so when we report all 3 buttons pressed we mistaken it as interleaved 
packet again... Insteado f waiting till 9th byte can't we just forcefully
exit inetrleaved mode (once we processed the bare packet) by doing:

     psmouse->packet[3] &= 0xf7;

?

-- 
Dmitry

Re: [alps] Timing patch, revised again :-)

[Dmitry Torokhov]

On Fri, Dec 04, 2009 at 03:49:48PM -0800, Dmitry Torokhov wrote:
> On Friday 04 December 2009 01:47:20 pm Sebastian Kapfer wrote:
> > Hi Dmitry,
> > 
> > thank you.  I've tested it and it basically works.  I still have two
> > more nits to pick though :-)
> > 

BTW, could you please "reply all" on kernel-related lists - this way I
(and other kernel people) will reply much faster - we tend to go through
our inboxes first and mailing lists - when we happen to have more time
;)

Thanks.

-- 
Dmitry

Re: [alps] Timing patch, revised again :-)

[Sebastian Kapfer]

Hi Dmitry!


On Fr, 2009-12-04 at 15:49 -0800, Dmitry Torokhov wrote:
[...]
> > Given this behaviour of the hw, I'd favour not reporting button presses
> > on a device while the corresponding button on the other device is down.
> > (Dave called this behaviour 'masking'.)  Code implementing this was in
> > the patch I sent to linux-input dated Nov. 11 (see the parts involving
> > the btn_state variable).  I have not put it back in the patch below
> > because I'd like to await your opinion on this first.
> 
> OK, Let me take a look at that patch again.

I think it's pretty straightforward.  Let me know what you think.
 
> > There is still one failure mode left that causes de-sync.  It happens
> > when the else branch in  alps_handle_interleaved_ps2 gets called more
> > than once, i.e. we're accidentially reconstructing a 12-, 15- etc byte
> > packet.  This was easier to deal with in my first patch, I just
> > collected the whole 9 bytes in a buffer and implicitly knew when the
> > packet was over.  Example of this happening:
> > 
> > Dec  4 21:03:22 sardelle kernel: [410740.786121] alps.c: handle: cf
> > Dec  4 21:03:22 sardelle kernel: [410740.787499] alps.c: handle: 79
> > Dec  4 21:03:22 sardelle kernel: [410740.788688] alps.c: handle: 12
> > Dec  4 21:03:22 sardelle kernel: [410740.789979] alps.c: handle: 1f
> > Dec  4 21:03:22 sardelle kernel: [410740.791146] alps.c: handle: ff
> > Dec  4 21:03:22 sardelle kernel: [410740.792299] alps.c: handle: 1
> > <suspect 9byte (really is)>
> > Dec  4 21:03:22 sardelle kernel: [410740.796899] alps.c: handle: 4f
> > <yup, it is, fold back>
> 
> Ah, ok, so when we report all 3 buttons pressed we mistaken it as interleaved 
> packet again... Insteado f waiting till 9th byte can't we just forcefully
> exit inetrleaved mode (once we processed the bare packet) by doing:
> 
>      psmouse->packet[3] &= 0xf7;

Hmm I guess we could do something like this, but IMHO it makes the code
needlessly arcane.  What would be the point?  Save 3 bytes of buffer?
Deliver the mouse movement a microsecond earlier?  Given that the
interleaved packet is somehow a coherent piece of data sent by the
touchpad, I'd store it exactly like that.

Greetings!

-- 
Sebastian Kapfer
 Inst. für Theoretische Physik I        Office +49-9131-85-2-8450
 Universität Erlangen                   Mobile +49-160-9577-6436
 Staudtstr. 7, Raum 02.583.             sebastian.kapfer@physik.uni-erlangen.de

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [alps] Timing patch, revised again :-)

[Dmitry Torokhov]

On Sun, Dec 06, 2009 at 08:53:57PM +0100, Sebastian Kapfer wrote:
> > 
> > Ah, ok, so when we report all 3 buttons pressed we mistaken it as interleaved 
> > packet again... Insteado f waiting till 9th byte can't we just forcefully
> > exit inetrleaved mode (once we processed the bare packet) by doing:
> > 
> >      psmouse->packet[3] &= 0xf7;
> 
> Hmm I guess we could do something like this, but IMHO it makes the code
> needlessly arcane.  What would be the point?  Save 3 bytes of buffer?
> Deliver the mouse movement a microsecond earlier?

No, this allows us validate the data bytes as tehy come in and probably
detect desync right away and resync earlier.

-- 
Dmitry

Re: [alps] Timing patch, revised again :-)

[Sebastian Kapfer]

On So, 2009-12-06 at 12:09 -0800, Dmitry Torokhov wrote:
> On Sun, Dec 06, 2009 at 08:53:57PM +0100, Sebastian Kapfer wrote:
> > > 
> >[...] 
> > Hmm I guess we could do something like this, but IMHO it makes the code
> > needlessly arcane.  What would be the point?  Save 3 bytes of buffer?
> > Deliver the mouse movement a microsecond earlier?
> 
> No, this allows us validate the data bytes as tehy come in and probably
> detect desync right away and resync earlier.

Hi Dmitry,

note that the resync code is disabled anyway, and the hardware gets
reset in that case, which means losing tens of milliseconds.  (I don't
think we have desync problems left, at least for this particular hw.)

-- 
Best Regards,  | Hi! I'm a .signature virus. Copy me into
 Sebastian     | your ~/.signature to help me spread!

[PATCH 1/2]

[Sebastian Kapfer]

Hi Dmitry,

for the record, I have put together one last time what I consider the
best solution.  Sorry if I seem pushy, but I think we have ironed out
all the issues, and waiting longer isn't going to make the code prettier
(or more correct).

This is your last patch, with the desync fixed:



Date: Fri, 4 Dec 2009 21:50:37 +0100
Subject: [PATCH 1/2] reworked again, 9bytes


Input: ALPS - add interleaved protocol support (Dell E6x00 series)

From: Sebastian Kapfer <sebastian_kapfer@gmx.net>

Properly handle version of the protocol where standard PS/2 packets
from trackpoint are stuffed into middle (byte 3-6) of the standard
ALPS packets when both the touchpad and trackpoint are used together.

The patch is based on work done by Matthew Chapman and additional
research done by David Kubicek and Erik Osterholm:

        https://bugs.launchpad.net/ubuntu/+source/linux/+bug/296610

Many thanks to David Kubicek for his efforts in researching fine points
of this new version of the protocol, especially interaction between pad
and stick in these models.

Signed-off-by: Sebastian Kapfer <sebastian_kapfer@gmx.net>

---
 drivers/input/mouse/alps.c    |  254 ++++++++++++++++++++++++++++++++++++-----
 drivers/input/mouse/alps.h    |    1 +
 drivers/input/mouse/psmouse.h |    2 +-
 3 files changed, 229 insertions(+), 28 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index a3f492a..71b40ae 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -5,6 +5,7 @@
  * Copyright (c) 2003-2005 Peter Osterlund <petero2@telia.com>
  * Copyright (c) 2004 Dmitry Torokhov <dtor@mail.ru>
  * Copyright (c) 2005 Vojtech Pavlik <vojtech@suse.cz>
+ * Copyright (c) 2009 Sebastian Kapfer <sebastian_kapfer@gmx.net>
  *
  * ALPS detection, tap switching and status querying info is taken from
  * tpconfig utility (by C. Scott Ananian and Bruce Kall).
@@ -28,7 +29,6 @@
 #define dbg(format, arg...) do {} while (0)
 #endif
 
-
 #define ALPS_OLDPROTO		0x01	/* old style input */
 #define ALPS_DUALPOINT		0x02	/* touchpad has trackstick */
 #define ALPS_PASS		0x04	/* device has a pass-through port */
@@ -37,7 +37,8 @@
 #define ALPS_FW_BK_1		0x10	/* front & back buttons present */
 #define ALPS_FW_BK_2		0x20	/* front & back buttons present */
 #define ALPS_FOUR_BUTTONS	0x40	/* 4 direction button present */
-
+#define ALPS_PS2_INTERLEAVED	0x80	/* 3-byte PS/2 packet interleaved with
+					   6-byte ALPS packet */
 
 static const struct alps_model_info alps_model_data[] = {
 	{ { 0x32, 0x02, 0x14 },	0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT }, /* Toshiba Salellite Pro M10 */
@@ -58,7 +59,9 @@ static const struct alps_model_info alps_model_data[] = {
 	{ { 0x20, 0x02, 0x0e },	0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT }, /* XXX */
 	{ { 0x22, 0x02, 0x0a },	0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT },
 	{ { 0x22, 0x02, 0x14 }, 0xff, 0xff, ALPS_PASS | ALPS_DUALPOINT }, /* Dell Latitude D600 */
-	{ { 0x62, 0x02, 0x14 }, 0xcf, 0xcf, ALPS_PASS | ALPS_DUALPOINT }, /* Dell Latitude E6500 */
+	/* Dell Latitude E5500, E6400, E6500, Precision M4400 */
+	{ { 0x62, 0x02, 0x14 }, 0xcf, 0xcf,
+		ALPS_PASS | ALPS_DUALPOINT | ALPS_PS2_INTERLEAVED },
 	{ { 0x73, 0x02, 0x50 }, 0xcf, 0xcf, ALPS_FOUR_BUTTONS },	  /* Dell Vostro 1400 */
 };
 
@@ -69,20 +72,56 @@ static const struct alps_model_info alps_model_data[] = {
  */
 
 /*
- * ALPS abolute Mode - new format
+ * PS/2 packet format
+ *
+ * byte 0:  0    0 YSGN XSGN    1    M    R    L
+ * byte 1: X7   X6   X5   X4   X3   X2   X1   X0
+ * byte 2: Y7   Y6   Y5   Y4   Y3   Y2   Y1   Y0
+ *
+ * Note that the device never signals overflow condition.
+ *
+ * ALPS absolute Mode - new format
  *
  * byte 0:  1    ?    ?    ?    1    ?    ?    ?
  * byte 1:  0   x6   x5   x4   x3   x2   x1   x0
- * byte 2:  0   x10  x9   x8   x7    ?  fin  ges
+ * byte 2:  0  x10   x9   x8   x7    ?  fin  ges
  * byte 3:  0   y9   y8   y7    1    M    R    L
  * byte 4:  0   y6   y5   y4   y3   y2   y1   y0
  * byte 5:  0   z6   z5   z4   z3   z2   z1   z0
  *
+ * Dualpoint device -- interleaved packet format
+ *
+ * byte 0:    1    1    0    0    1    1    1    1
+ * byte 1:    0   x6   x5   x4   x3   x2   x1   x0
+ * byte 2:    0  x10   x9   x8   x7    0  fin  ges
+ * byte 3:    0    0 YSGN XSGN    1    1    1    1
+ * byte 4:   X7   X6   X5   X4   X3   X2   X1   X0
+ * byte 5:   Y7   Y6   Y5   Y4   Y3   Y2   Y1   Y0
+ * byte 6:    0   y9   y8   y7    1    m    r    l
+ * byte 7:    0   y6   y5   y4   y3   y2   y1   y0
+ * byte 8:    0   z6   z5   z4   z3   z2   z1   z0
+ *
+ * CAPITALS = stick, miniscules = touchpad
+ *
  * ?'s can have different meanings on different models,
  * such as wheel rotation, extra buttons, stick buttons
  * on a dualpoint, etc.
  */
 
+static void alps_report_buttons(struct input_dev *dev,
+				int left, int right, int middle,
+				bool release_only)
+{
+	if (!left || !release_only)
+		input_report_key(dev, BTN_LEFT, left);
+
+	if (!right || !release_only)
+		input_report_key(dev, BTN_RIGHT, right);
+
+	if (!middle || !release_only)
+		input_report_key(dev, BTN_MIDDLE, middle);
+}
+
 static void alps_process_packet(struct psmouse *psmouse)
 {
 	struct alps_data *priv = psmouse->private;
@@ -93,18 +132,6 @@ static void alps_process_packet(struct psmouse *psmouse)
 	int x, y, z, ges, fin, left, right, middle;
 	int back = 0, forward = 0;
 
-	if ((packet[0] & 0xc8) == 0x08) {   /* 3-byte PS/2 packet */
-		input_report_key(dev2, BTN_LEFT,   packet[0] & 1);
-		input_report_key(dev2, BTN_RIGHT,  packet[0] & 2);
-		input_report_key(dev2, BTN_MIDDLE, packet[0] & 4);
-		input_report_rel(dev2, REL_X,
-			packet[1] ? packet[1] - ((packet[0] << 4) & 0x100) : 0);
-		input_report_rel(dev2, REL_Y,
-			packet[2] ? ((packet[0] << 3) & 0x100) - packet[2] : 0);
-		input_sync(dev2);
-		return;
-	}
-
 	if (model->flags & ALPS_OLDPROTO) {
 		left = packet[2] & 0x10;
 		right = packet[2] & 0x08;
@@ -140,18 +167,25 @@ static void alps_process_packet(struct psmouse *psmouse)
 		input_report_rel(dev2, REL_X,  (x > 383 ? (x - 768) : x));
 		input_report_rel(dev2, REL_Y, -(y > 255 ? (y - 512) : y));
 
-		input_report_key(dev2, BTN_LEFT, left);
-		input_report_key(dev2, BTN_RIGHT, right);
-		input_report_key(dev2, BTN_MIDDLE, middle);
+		alps_report_buttons(dev2, left, right, middle, false);
 
-		input_sync(dev);
 		input_sync(dev2);
+		input_sync(dev);
 		return;
 	}
 
-	input_report_key(dev, BTN_LEFT, left);
-	input_report_key(dev, BTN_RIGHT, right);
-	input_report_key(dev, BTN_MIDDLE, middle);
+	alps_report_buttons(dev, left, right, middle, false);
+
+	if (model->flags & ALPS_PS2_INTERLEAVED) {
+		/*
+		 * On devices using interleaved packets, when user presses
+		 * the same button on both trackpoint and touchpad, the
+		 * release for the trackpoint is not reported so we have
+		 * send release events to both devices.
+		 */
+		alps_report_buttons(dev2, left, right, middle, true);
+		input_sync(dev2);
+	}
 
 	/* Convert hardware tap to a reasonable Z value */
 	if (ges && !fin)
@@ -202,25 +236,188 @@ static void alps_process_packet(struct psmouse *psmouse)
 	input_sync(dev);
 }
 
+static void alps_report_bare_ps2_packet(struct psmouse *psmouse,
+					unsigned char packet[],
+					bool report_buttons)
+{
+	struct alps_data *priv = psmouse->private;
+	struct input_dev *dev2 = priv->dev2;
+
+	if (report_buttons) {
+		int left = packet[0] & 1;
+		int right = packet[0] & 2;
+		int middle = packet[0] & 4;
+
+
+		if (priv->i->flags & ALPS_PS2_INTERLEAVED) {
+			alps_report_buttons(psmouse->dev,
+					    left, right, middle, true);
+			input_sync(psmouse->dev);
+		}
+
+		alps_report_buttons(dev2, left, right, middle, false);
+	}
+
+	input_report_rel(dev2, REL_X,
+		packet[1] ? packet[1] - ((packet[0] << 4) & 0x100) : 0);
+	input_report_rel(dev2, REL_Y,
+		packet[2] ? ((packet[0] << 3) & 0x100) - packet[2] : 0);
+
+	input_sync(dev2);
+}
+
+static bool alps_is_valid_first_byte(const struct alps_model_info *model,
+				     unsigned char data)
+{
+	return (data & model->mask0) == model->byte0;
+}
+
+static psmouse_ret_t alps_handle_interleaved_ps2(struct psmouse *psmouse)
+{
+	struct alps_data *priv = psmouse->private;
+
+	if (psmouse->pktcnt < 6)
+		return PSMOUSE_GOOD_DATA;
+
+	if (psmouse->pktcnt == 6) {
+		/*
+		 * Start a timer to flush the packet if it ends up last
+		 * 6-byte packet in the stream. Timer needs to fire
+		 * psmouse core times out itself. 20 ms should be enough
+		 * to decide if we are getting more data or not.
+		 */
+		mod_timer(&priv->timer, jiffies + msecs_to_jiffies(20));
+		return PSMOUSE_GOOD_DATA;
+	}
+
+	del_timer(&priv->timer);
+
+	if (psmouse->packet[6] & 0x80) {
+
+		/*
+		 * Highest bit is set - that means we either had
+		 * complete ALPS packet and this is start of the
+		 * next packet or we got garbage.
+		 */
+
+		if (((psmouse->packet[3] |
+		      psmouse->packet[4] |
+		      psmouse->packet[5]) & 0x80) ||
+		    (!alps_is_valid_first_byte(priv->i, psmouse->packet[6]))) {
+			dbg("refusing packet %x %x %x %x "
+			    "(suspected interleaved ps/2)\n",
+			    psmouse->packet[3], psmouse->packet[4],
+			    psmouse->packet[5], psmouse->packet[6]);
+			return PSMOUSE_BAD_DATA;
+		}
+
+		alps_process_packet(psmouse);
+
+		/* Continue with the next packet */
+		psmouse->packet[0] = psmouse->packet[6];
+		psmouse->pktcnt = 1;
+
+	} else {
+
+		/*
+		 * High bit is 0 - that means that we indeed got a PS/2
+		 * packet in the middle of ALPS packet.
+		 *
+		 * There is also possibility that we got 6-byte ALPS
+		 * packet followed  by 3-byte packet from trackpoint. We
+		 * can not distinguish between these 2 scenarios but
+		 * becase the latter is unlikely to happen in course of
+		 * normal operation (user would need to press all
+		 * buttons on the pad and start moving trackpoint
+		 * without touching the pad surface) we assume former.
+		 * Even if we are wrong the wost thing that would happen
+		 * the cursor would jump but we should not get protocol
+		 * desynchronization.
+		 */
+
+		if (psmouse->pktcnt == 9) {
+			/* Process interleaved PS/2 data */
+			alps_report_bare_ps2_packet(psmouse,
+				&psmouse->packet[3], false);
+
+			/* Continue with the standard ALPS protocol handling */
+			psmouse->packet[3] = psmouse->packet[6];
+			psmouse->packet[4] = psmouse->packet[7];
+			psmouse->packet[5] = psmouse->packet[8];
+			psmouse->pktcnt = 6;
+			alps_process_packet(psmouse);
+			return PSMOUSE_FULL_PACKET;
+		}
+	}
+
+	return PSMOUSE_GOOD_DATA;
+}
+
+static void alps_flush_packet(unsigned long data)
+{
+	struct psmouse *psmouse = (struct psmouse *)data;
+
+	serio_pause_rx(psmouse->ps2dev.serio);
+
+	if (psmouse->pktcnt == 6) {
+
+		/*
+		 * We did not any more data in reasonable amount of time.
+		 * Validate the last 3 bytes and process as a standard
+		 * ALPS packet.
+		 */
+		if ((psmouse->packet[3] |
+		     psmouse->packet[4] |
+		     psmouse->packet[5]) & 0x80) {
+			dbg("refusing packet %x %x %x "
+			    "(suspected interleaved ps/2)\n",
+			    psmouse->packet[3], psmouse->packet[4],
+			    psmouse->packet[5]);
+		} else {
+			alps_process_packet(psmouse);
+		}
+		psmouse->pktcnt = 0;
+	}
+
+	serio_continue_rx(psmouse->ps2dev.serio);
+}
+
 static psmouse_ret_t alps_process_byte(struct psmouse *psmouse)
 {
 	struct alps_data *priv = psmouse->private;
+	const struct alps_model_info *model = priv->i;
+
+        dbg ("handle: %x", psmouse->packet[psmouse->pktcnt-1]);
 
 	if ((psmouse->packet[0] & 0xc8) == 0x08) { /* PS/2 packet */
 		if (psmouse->pktcnt == 3) {
-			alps_process_packet(psmouse);
+			alps_report_bare_ps2_packet(psmouse, psmouse->packet,
+						    true);
 			return PSMOUSE_FULL_PACKET;
 		}
 		return PSMOUSE_GOOD_DATA;
 	}
 
-	if ((psmouse->packet[0] & priv->i->mask0) != priv->i->byte0)
+	/* Check for PS/2 packet stuffed in the middle of ALPS packet. */
+
+	if ((model->flags & ALPS_PS2_INTERLEAVED) &&
+	    psmouse->pktcnt >= 4 && (psmouse->packet[3] & 0x0f) == 0x0f) {
+		return alps_handle_interleaved_ps2(psmouse);
+	}
+
+	if (!alps_is_valid_first_byte(model, psmouse->packet[0])) {
+		dbg("refusing packet[0] = %x (mask0 = %x, byte0 = %x)\n",
+		    psmouse->packet[0], model->mask0, model->byte0);
 		return PSMOUSE_BAD_DATA;
+	}
 
 	/* Bytes 2 - 6 should have 0 in the highest bit */
 	if (psmouse->pktcnt >= 2 && psmouse->pktcnt <= 6 &&
-	    (psmouse->packet[psmouse->pktcnt - 1] & 0x80))
+	    (psmouse->packet[psmouse->pktcnt - 1] & 0x80)) {
+		dbg("refusing packet[%i] = %x\n",
+		    psmouse->pktcnt - 1, psmouse->packet[psmouse->pktcnt - 1]);
 		return PSMOUSE_BAD_DATA;
+	}
 
 	if (psmouse->pktcnt == 6) {
 		alps_process_packet(psmouse);
@@ -459,6 +656,7 @@ static void alps_disconnect(struct psmouse *psmouse)
 	struct alps_data *priv = psmouse->private;
 
 	psmouse_reset(psmouse);
+	del_timer_sync(&priv->timer);
 	input_unregister_device(priv->dev2);
 	kfree(priv);
 }
@@ -476,6 +674,8 @@ int alps_init(struct psmouse *psmouse)
 		goto init_fail;
 
 	priv->dev2 = dev2;
+	setup_timer(&priv->timer, alps_flush_packet, (unsigned long)psmouse);
+
 	psmouse->private = priv;
 
 	model = alps_get_model(psmouse, &version);
diff --git a/drivers/input/mouse/alps.h b/drivers/input/mouse/alps.h
index bc87936..904ed8b 100644
--- a/drivers/input/mouse/alps.h
+++ b/drivers/input/mouse/alps.h
@@ -23,6 +23,7 @@ struct alps_data {
 	char phys[32];			/* Phys */
 	const struct alps_model_info *i;/* Info */
 	int prev_fin;			/* Finger bit from previous packet */
+	struct timer_list timer;
 };
 
 #ifdef CONFIG_MOUSE_PS2_ALPS
diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h
index e053bdd..d4772fe 100644
--- a/drivers/input/mouse/psmouse.h
+++ b/drivers/input/mouse/psmouse.h
@@ -42,7 +42,7 @@ struct psmouse {
 	struct delayed_work resync_work;
 	char *vendor;
 	char *name;
-	unsigned char packet[8];
+	unsigned char packet[9];
 	unsigned char badbyte;
 	unsigned char pktcnt;
 	unsigned char pktsize;
-- 
1.6.3.3

[PATCH 2/2] Alps Dualpoint, Interleaved packets

[Sebastian Kapfer]

... and this fixes the double-click issue when flooded with packets.  It
makes sure we never set the same mouse button on both dev and dev2.
When the button is released, it releases on both devices, since we don't
record which of them got the initial button-down.

Greetings,
 Sebastian



Date: Sun, 13 Dec 2009 22:44:10 +0100
Subject: [PATCH 2/2] fix double-click issue


Input: ALPS - add interleaved protocol support (Dell E6x00 series)

From: Sebastian Kapfer <sebastian_kapfer@gmx.net>

Fix a race between the button-down and the button-up events which could
lead to a single physical click on an Alps dualpoint unit being reported
as a double-click in X11.

This patch ensures that any click is assigned to only of the two
input_devs associated with the dualpoint unit, and never reported on the
other input_dev.

Signed-off-by: Sebastian Kapfer <sebastian_kapfer@gmx.net>


---
 drivers/input/mouse/alps.c |   78 ++++++++++++++++++++++++++------------------
 drivers/input/mouse/alps.h |    3 +-
 2 files changed, 48 insertions(+), 33 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 71b40ae..10b591c 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -39,6 +39,7 @@
 #define ALPS_FOUR_BUTTONS	0x40	/* 4 direction button present */
 #define ALPS_PS2_INTERLEAVED	0x80	/* 3-byte PS/2 packet interleaved with
 					   6-byte ALPS packet */
+#define ALPS_SHARED_BTNSTATE	0x100	/* PS/2 and touchpad share button st. */
 
 static const struct alps_model_info alps_model_data[] = {
 	{ { 0x32, 0x02, 0x14 },	0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT }, /* Toshiba Salellite Pro M10 */
@@ -61,7 +62,7 @@ static const struct alps_model_info alps_model_data[] = {
 	{ { 0x22, 0x02, 0x14 }, 0xff, 0xff, ALPS_PASS | ALPS_DUALPOINT }, /* Dell Latitude D600 */
 	/* Dell Latitude E5500, E6400, E6500, Precision M4400 */
 	{ { 0x62, 0x02, 0x14 }, 0xcf, 0xcf,
-		ALPS_PASS | ALPS_DUALPOINT | ALPS_PS2_INTERLEAVED },
+		ALPS_PASS | ALPS_DUALPOINT | ALPS_PS2_INTERLEAVED | ALPS_SHARED_BTNSTATE },
 	{ { 0x73, 0x02, 0x50 }, 0xcf, 0xcf, ALPS_FOUR_BUTTONS },	  /* Dell Vostro 1400 */
 };
 
@@ -108,18 +109,49 @@ static const struct alps_model_info alps_model_data[] = {
  * on a dualpoint, etc.
  */
 
-static void alps_report_buttons(struct input_dev *dev,
-				int left, int right, int middle,
-				bool release_only)
+static void alps_report_single_button(struct psmouse *psmouse,
+				      int keycode, int mask, int state,
+				      struct input_dev *dev)
 {
-	if (!left || !release_only)
-		input_report_key(dev, BTN_LEFT, left);
+	struct alps_data *priv = psmouse->private;
+	const struct alps_model_info *model = priv->i;
 
-	if (!right || !release_only)
-		input_report_key(dev, BTN_RIGHT, right);
+	/* some Alps units do not report touchpad and trackpoint
+	   buttons separately (e.g. even the trackpoint movement
+	   packets have buttons set while touchpad buttons are down) */
+	if (model->flags & ALPS_SHARED_BTNSTATE) {
+		if (state) {
+			/* need to avoid sending a button-down to both
+			   devices (can cause spurious double-click */
+			if (priv->btn_state & mask)
+				return;
+
+			priv->btn_state |= mask;
+		} else {
+			/* we don't know which device got the button
+			   down event.  just release both to be sure. */
+			struct input_dev *other = psmouse->dev;
+			if (dev == other)
+				other = priv->dev2;
+			input_report_key(other, keycode, state);
+			input_sync(other);
+
+			priv->btn_state &= 7 ^ mask;
+		}
+	}
 
-	if (!middle || !release_only)
-		input_report_key(dev, BTN_MIDDLE, middle);
+	/* input_sync called by our caller */
+	input_report_key(dev, keycode, state);
+}
+
+
+static void alps_report_buttons(struct psmouse *psmouse,
+				struct input_dev *dev,
+				int left, int right, int middle)
+{
+	alps_report_single_button(psmouse, BTN_LEFT,   1, left,   dev);
+	alps_report_single_button(psmouse, BTN_RIGHT,  2, right,  dev);
+	alps_report_single_button(psmouse, BTN_MIDDLE, 4, middle, dev);
 }
 
 static void alps_process_packet(struct psmouse *psmouse)
@@ -167,25 +199,13 @@ static void alps_process_packet(struct psmouse *psmouse)
 		input_report_rel(dev2, REL_X,  (x > 383 ? (x - 768) : x));
 		input_report_rel(dev2, REL_Y, -(y > 255 ? (y - 512) : y));
 
-		alps_report_buttons(dev2, left, right, middle, false);
+		alps_report_buttons(psmouse, dev2, left, right, middle);
 
 		input_sync(dev2);
-		input_sync(dev);
 		return;
 	}
 
-	alps_report_buttons(dev, left, right, middle, false);
-
-	if (model->flags & ALPS_PS2_INTERLEAVED) {
-		/*
-		 * On devices using interleaved packets, when user presses
-		 * the same button on both trackpoint and touchpad, the
-		 * release for the trackpoint is not reported so we have
-		 * send release events to both devices.
-		 */
-		alps_report_buttons(dev2, left, right, middle, true);
-		input_sync(dev2);
-	}
+	alps_report_buttons(psmouse, dev, left, right, middle);
 
 	/* Convert hardware tap to a reasonable Z value */
 	if (ges && !fin)
@@ -248,14 +268,7 @@ static void alps_report_bare_ps2_packet(struct psmouse *psmouse,
 		int right = packet[0] & 2;
 		int middle = packet[0] & 4;
 
-
-		if (priv->i->flags & ALPS_PS2_INTERLEAVED) {
-			alps_report_buttons(psmouse->dev,
-					    left, right, middle, true);
-			input_sync(psmouse->dev);
-		}
-
-		alps_report_buttons(dev2, left, right, middle, false);
+		alps_report_buttons(psmouse, dev2, left, right, middle);
 	}
 
 	input_report_rel(dev2, REL_X,
@@ -675,6 +688,7 @@ int alps_init(struct psmouse *psmouse)
 
 	priv->dev2 = dev2;
 	setup_timer(&priv->timer, alps_flush_packet, (unsigned long)psmouse);
+	priv->btn_state = 0;
 
 	psmouse->private = priv;
 
diff --git a/drivers/input/mouse/alps.h b/drivers/input/mouse/alps.h
index 904ed8b..b477063 100644
--- a/drivers/input/mouse/alps.h
+++ b/drivers/input/mouse/alps.h
@@ -15,7 +15,7 @@
 struct alps_model_info {
         unsigned char signature[3];
         unsigned char byte0, mask0;
-        unsigned char flags;
+        unsigned int flags;
 };
 
 struct alps_data {
@@ -23,6 +23,7 @@ struct alps_data {
 	char phys[32];			/* Phys */
 	const struct alps_model_info *i;/* Info */
 	int prev_fin;			/* Finger bit from previous packet */
+	int btn_state;			/* buttons reported to input_dev */
 	struct timer_list timer;
 };
 
-- 
1.6.3.3

Re: [PATCH 2/2] Alps Dualpoint, Interleaved packets

[Dmitry Torokhov]

Hi Sebastian,

On Sun, Dec 13, 2009 at 11:09:05PM +0100, Sebastian Kapfer wrote:
> 
> .... and this fixes the double-click issue when flooded with packets.  It
> makes sure we never set the same mouse button on both dev and dev2.
> When the button is released, it releases on both devices, since we don't
> record which of them got the initial button-down.
> 

Thank you for pinging me and updating the patch. I think we don't need a
separate shared buttons flag, at least for now, but branch the shared
buttons handling off the interleaved packet flag.

Also, instead of dealing with a separate state field I think we could
use the following rule: if when reporintg a button for a device first
check if the same button was reported for the other device. If it was
then this event should also go to the other device, otherwise we can
report to ours:

	if (test_bit(BTN_XXX, other->key)
		input_reprt_key(other, BTN_XXX, state);
	else
		input_reprt_key(this, BTN_XXX, state);

BTW, I still think that we should not wait for the all 9 bytes but
just make sure that we won't restart interleaved processing from
interleaved packet. The reason is that if device loses a byte in-between
we have bigger chnace of resynching on the data stream without needing
to do full-blown resync.

Could you please try the patch below and if it works I will send it off
to mainline.

Thanks for all your efforts.

-- 
Dmitry

Input: ALPS - add interleaved protocol support (Dell E6x00 series)

From: Sebastian Kapfer <sebastian_kapfer@gmx.net>

Properly handle version of the protocol where standard PS/2 packets
from trackpoint are stuffed into middle (byte 3-6) of the standard
ALPS packets when both the touchpad and trackpoint are used together.

The patch is based on work done by Matthew Chapman and additional
research done by David Kubicek and Erik Osterholm:

	https://bugs.launchpad.net/ubuntu/+source/linux/+bug/296610

Many thanks to David Kubicek for his efforts in researching fine points
of this new version of the protocol, especially interaction between pad
and stick in these models.

Signed-off-by: Sebastian Kapfer <sebastian_kapfer@gmx.net>
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/input/mouse/alps.c |  255 +++++++++++++++++++++++++++++++++++++++-----
 drivers/input/mouse/alps.h |    1 
 2 files changed, 229 insertions(+), 27 deletions(-)


diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index b03e7e0..da2a211 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -5,6 +5,7 @@
  * Copyright (c) 2003-2005 Peter Osterlund <petero2@telia.com>
  * Copyright (c) 2004 Dmitry Torokhov <dtor@mail.ru>
  * Copyright (c) 2005 Vojtech Pavlik <vojtech@suse.cz>
+ * Copyright (c) 2009 Sebastian Kapfer <sebastian_kapfer@gmx.net>
  *
  * ALPS detection, tap switching and status querying info is taken from
  * tpconfig utility (by C. Scott Ananian and Bruce Kall).
@@ -28,7 +29,6 @@
 #define dbg(format, arg...) do {} while (0)
 #endif
 
-
 #define ALPS_OLDPROTO		0x01	/* old style input */
 #define ALPS_DUALPOINT		0x02	/* touchpad has trackstick */
 #define ALPS_PASS		0x04	/* device has a pass-through port */
@@ -37,7 +37,8 @@
 #define ALPS_FW_BK_1		0x10	/* front & back buttons present */
 #define ALPS_FW_BK_2		0x20	/* front & back buttons present */
 #define ALPS_FOUR_BUTTONS	0x40	/* 4 direction button present */
-
+#define ALPS_PS2_INTERLEAVED	0x80	/* 3-byte PS/2 packet interleaved with
+					   6-byte ALPS packet */
 
 static const struct alps_model_info alps_model_data[] = {
 	{ { 0x32, 0x02, 0x14 },	0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT }, /* Toshiba Salellite Pro M10 */
@@ -58,7 +59,9 @@ static const struct alps_model_info alps_model_data[] = {
 	{ { 0x20, 0x02, 0x0e },	0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT }, /* XXX */
 	{ { 0x22, 0x02, 0x0a },	0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT },
 	{ { 0x22, 0x02, 0x14 }, 0xff, 0xff, ALPS_PASS | ALPS_DUALPOINT }, /* Dell Latitude D600 */
-	{ { 0x62, 0x02, 0x14 }, 0xcf, 0xcf, ALPS_PASS | ALPS_DUALPOINT }, /* Dell Latitude E6500 */
+	/* Dell Latitude E5500, E6400, E6500, Precision M4400 */
+	{ { 0x62, 0x02, 0x14 }, 0xcf, 0xcf,
+		ALPS_PASS | ALPS_DUALPOINT | ALPS_PS2_INTERLEAVED },
 	{ { 0x73, 0x02, 0x50 }, 0xcf, 0xcf, ALPS_FOUR_BUTTONS },	  /* Dell Vostro 1400 */
 };
 
@@ -69,20 +72,88 @@ static const struct alps_model_info alps_model_data[] = {
  */
 
 /*
- * ALPS abolute Mode - new format
+ * PS/2 packet format
+ *
+ * byte 0:  0    0 YSGN XSGN    1    M    R    L
+ * byte 1: X7   X6   X5   X4   X3   X2   X1   X0
+ * byte 2: Y7   Y6   Y5   Y4   Y3   Y2   Y1   Y0
+ *
+ * Note that the device never signals overflow condition.
+ *
+ * ALPS absolute Mode - new format
  *
  * byte 0:  1    ?    ?    ?    1    ?    ?    ?
  * byte 1:  0   x6   x5   x4   x3   x2   x1   x0
- * byte 2:  0   x10  x9   x8   x7    ?  fin  ges
+ * byte 2:  0  x10   x9   x8   x7    ?  fin  ges
  * byte 3:  0   y9   y8   y7    1    M    R    L
  * byte 4:  0   y6   y5   y4   y3   y2   y1   y0
  * byte 5:  0   z6   z5   z4   z3   z2   z1   z0
  *
+ * Dualpoint device -- interleaved packet format
+ *
+ * byte 0:    1    1    0    0    1    1    1    1
+ * byte 1:    0   x6   x5   x4   x3   x2   x1   x0
+ * byte 2:    0  x10   x9   x8   x7    0  fin  ges
+ * byte 3:    0    0 YSGN XSGN    1    1    1    1
+ * byte 4:   X7   X6   X5   X4   X3   X2   X1   X0
+ * byte 5:   Y7   Y6   Y5   Y4   Y3   Y2   Y1   Y0
+ * byte 6:    0   y9   y8   y7    1    m    r    l
+ * byte 7:    0   y6   y5   y4   y3   y2   y1   y0
+ * byte 8:    0   z6   z5   z4   z3   z2   z1   z0
+ *
+ * CAPITALS = stick, miniscules = touchpad
+ *
  * ?'s can have different meanings on different models,
  * such as wheel rotation, extra buttons, stick buttons
  * on a dualpoint, etc.
  */
 
+static bool alps_is_valid_first_byte(const struct alps_model_info *model,
+				     unsigned char data)
+{
+	return (data & model->mask0) == model->byte0;
+}
+
+static void alps_report_buttons(struct psmouse *psmouse,
+				struct input_dev *dev1, struct input_dev *dev2,
+				int left, int right, int middle)
+{
+	struct alps_data *priv = psmouse->private;
+	const struct alps_model_info *model = priv->i;
+
+	if (model->flags & ALPS_PS2_INTERLEAVED) {
+		struct input_dev *dev;
+
+		/*
+		 * If shared button has already been reported on the
+		 * other device (dev2) then this event should be also
+		 * sent through that device.
+		 */
+		dev = test_bit(BTN_LEFT, dev2->key) ? dev2 : dev1;
+		input_report_key(dev, BTN_LEFT, left);
+
+		dev = test_bit(BTN_RIGHT, dev2->key) ? dev2 : dev1;
+		input_report_key(dev, BTN_RIGHT, right);
+
+		dev = test_bit(BTN_MIDDLE, dev2->key) ? dev2 : dev1;
+		input_report_key(dev, BTN_MIDDLE, middle);
+
+		/*
+		 * Sync the _other_ device now, we'll do the first
+		 * device later once we report the rest of the events.
+		 */
+		input_sync(dev2);
+	} else {
+		/*
+		 * For devices with non-interleaved packets we know what
+		 * device buttons belong to so we can simply report them.
+		 */
+		input_report_key(dev1, BTN_LEFT, left);
+		input_report_key(dev1, BTN_RIGHT, right);
+		input_report_key(dev1, BTN_MIDDLE, middle);
+	}
+}
+
 static void alps_process_packet(struct psmouse *psmouse)
 {
 	struct alps_data *priv = psmouse->private;
@@ -93,18 +164,6 @@ static void alps_process_packet(struct psmouse *psmouse)
 	int x, y, z, ges, fin, left, right, middle;
 	int back = 0, forward = 0;
 
-	if ((packet[0] & 0xc8) == 0x08) {   /* 3-byte PS/2 packet */
-		input_report_key(dev2, BTN_LEFT,   packet[0] & 1);
-		input_report_key(dev2, BTN_RIGHT,  packet[0] & 2);
-		input_report_key(dev2, BTN_MIDDLE, packet[0] & 4);
-		input_report_rel(dev2, REL_X,
-			packet[1] ? packet[1] - ((packet[0] << 4) & 0x100) : 0);
-		input_report_rel(dev2, REL_Y,
-			packet[2] ? ((packet[0] << 3) & 0x100) - packet[2] : 0);
-		input_sync(dev2);
-		return;
-	}
-
 	if (model->flags & ALPS_OLDPROTO) {
 		left = packet[2] & 0x10;
 		right = packet[2] & 0x08;
@@ -140,18 +199,14 @@ static void alps_process_packet(struct psmouse *psmouse)
 		input_report_rel(dev2, REL_X,  (x > 383 ? (x - 768) : x));
 		input_report_rel(dev2, REL_Y, -(y > 255 ? (y - 512) : y));
 
-		input_report_key(dev2, BTN_LEFT, left);
-		input_report_key(dev2, BTN_RIGHT, right);
-		input_report_key(dev2, BTN_MIDDLE, middle);
+		alps_report_buttons(psmouse, dev2, dev, left, right, middle);
 
-		input_sync(dev);
 		input_sync(dev2);
+		input_sync(dev);
 		return;
 	}
 
-	input_report_key(dev, BTN_LEFT, left);
-	input_report_key(dev, BTN_RIGHT, right);
-	input_report_key(dev, BTN_MIDDLE, middle);
+	alps_report_buttons(psmouse, dev, dev2, left, right, middle);
 
 	/* Convert hardware tap to a reasonable Z value */
 	if (ges && !fin)
@@ -202,25 +257,168 @@ static void alps_process_packet(struct psmouse *psmouse)
 	input_sync(dev);
 }
 
+static void alps_report_bare_ps2_packet(struct psmouse *psmouse,
+					unsigned char packet[],
+					bool report_buttons)
+{
+	struct alps_data *priv = psmouse->private;
+	struct input_dev *dev2 = priv->dev2;
+
+	if (report_buttons)
+		alps_report_buttons(psmouse, dev2, psmouse->dev,
+				packet[0] & 1, packet[0] & 2, packet[0] & 4);
+
+	input_report_rel(dev2, REL_X,
+		packet[1] ? packet[1] - ((packet[0] << 4) & 0x100) : 0);
+	input_report_rel(dev2, REL_Y,
+		packet[2] ? ((packet[0] << 3) & 0x100) - packet[2] : 0);
+
+	input_sync(dev2);
+}
+
+static psmouse_ret_t alps_handle_interleaved_ps2(struct psmouse *psmouse)
+{
+	struct alps_data *priv = psmouse->private;
+
+	if (psmouse->pktcnt < 6)
+		return PSMOUSE_GOOD_DATA;
+
+	if (psmouse->pktcnt == 6) {
+		/*
+		 * Start a timer to flush the packet if it ends up last
+		 * 6-byte packet in the stream. Timer needs to fire
+		 * psmouse core times out itself. 20 ms should be enough
+		 * to decide if we are getting more data or not.
+		 */
+		mod_timer(&priv->timer, jiffies + msecs_to_jiffies(20));
+		return PSMOUSE_GOOD_DATA;
+	}
+
+	del_timer(&priv->timer);
+
+	if (psmouse->packet[6] & 0x80) {
+
+		/*
+		 * Highest bit is set - that means we either had
+		 * complete ALPS packet and this is start of the
+		 * next packet or we got garbage.
+		 */
+
+		if (((psmouse->packet[3] |
+		      psmouse->packet[4] |
+		      psmouse->packet[5]) & 0x80) ||
+		    (!alps_is_valid_first_byte(priv->i, psmouse->packet[6]))) {
+			dbg("refusing packet %x %x %x %x "
+			    "(suspected interleaved ps/2)\n",
+			    psmouse->packet[3], psmouse->packet[4],
+			    psmouse->packet[5], psmouse->packet[6]);
+			return PSMOUSE_BAD_DATA;
+		}
+
+		alps_process_packet(psmouse);
+
+		/* Continue with the next packet */
+		psmouse->packet[0] = psmouse->packet[6];
+		psmouse->pktcnt = 1;
+
+	} else {
+
+		/*
+		 * High bit is 0 - that means that we indeed got a PS/2
+		 * packet in the middle of ALPS packet.
+		 *
+		 * There is also possibility that we got 6-byte ALPS
+		 * packet followed  by 3-byte packet from trackpoint. We
+		 * can not distinguish between these 2 scenarios but
+		 * becase the latter is unlikely to happen in course of
+		 * normal operation (user would need to press all
+		 * buttons on the pad and start moving trackpoint
+		 * without touching the pad surface) we assume former.
+		 * Even if we are wrong the wost thing that would happen
+		 * the cursor would jump but we should not get protocol
+		 * desynchronization.
+		 */
+
+		alps_report_bare_ps2_packet(psmouse, &psmouse->packet[3],
+					    false);
+
+		/*
+		 * Continue with the standard ALPS protocol handling,
+		 * but make sure we won't process it as an interleaved
+		 * packet again, which may happen if all buttons are
+		 * pressed. To avoid this let's reset the 4th bit which
+		 * is normally 1.
+		 */
+		psmouse->packet[3] = psmouse->packet[6] & 0xf7;
+		psmouse->pktcnt = 4;
+	}
+
+	return PSMOUSE_GOOD_DATA;
+}
+
+static void alps_flush_packet(unsigned long data)
+{
+	struct psmouse *psmouse = (struct psmouse *)data;
+
+	serio_pause_rx(psmouse->ps2dev.serio);
+
+	if (psmouse->pktcnt == 6) {
+
+		/*
+		 * We did not any more data in reasonable amount of time.
+		 * Validate the last 3 bytes and process as a standard
+		 * ALPS packet.
+		 */
+		if ((psmouse->packet[3] |
+		     psmouse->packet[4] |
+		     psmouse->packet[5]) & 0x80) {
+			dbg("refusing packet %x %x %x "
+			    "(suspected interleaved ps/2)\n",
+			    psmouse->packet[3], psmouse->packet[4],
+			    psmouse->packet[5]);
+		} else {
+			alps_process_packet(psmouse);
+		}
+		psmouse->pktcnt = 0;
+	}
+
+	serio_continue_rx(psmouse->ps2dev.serio);
+}
+
 static psmouse_ret_t alps_process_byte(struct psmouse *psmouse)
 {
 	struct alps_data *priv = psmouse->private;
+	const struct alps_model_info *model = priv->i;
 
 	if ((psmouse->packet[0] & 0xc8) == 0x08) { /* PS/2 packet */
 		if (psmouse->pktcnt == 3) {
-			alps_process_packet(psmouse);
+			alps_report_bare_ps2_packet(psmouse, psmouse->packet,
+						    true);
 			return PSMOUSE_FULL_PACKET;
 		}
 		return PSMOUSE_GOOD_DATA;
 	}
 
-	if ((psmouse->packet[0] & priv->i->mask0) != priv->i->byte0)
+	/* Check for PS/2 packet stuffed in the middle of ALPS packet. */
+
+	if ((model->flags & ALPS_PS2_INTERLEAVED) &&
+	    psmouse->pktcnt >= 4 && (psmouse->packet[3] & 0x0f) == 0x0f) {
+		return alps_handle_interleaved_ps2(psmouse);
+	}
+
+	if (!alps_is_valid_first_byte(model, psmouse->packet[0])) {
+		dbg("refusing packet[0] = %x (mask0 = %x, byte0 = %x)\n",
+		    psmouse->packet[0], model->mask0, model->byte0);
 		return PSMOUSE_BAD_DATA;
+	}
 
 	/* Bytes 2 - 6 should have 0 in the highest bit */
 	if (psmouse->pktcnt >= 2 && psmouse->pktcnt <= 6 &&
-	    (psmouse->packet[psmouse->pktcnt - 1] & 0x80))
+	    (psmouse->packet[psmouse->pktcnt - 1] & 0x80)) {
+		dbg("refusing packet[%i] = %x\n",
+		    psmouse->pktcnt - 1, psmouse->packet[psmouse->pktcnt - 1]);
 		return PSMOUSE_BAD_DATA;
+	}
 
 	if (psmouse->pktcnt == 6) {
 		alps_process_packet(psmouse);
@@ -459,6 +657,7 @@ static void alps_disconnect(struct psmouse *psmouse)
 	struct alps_data *priv = psmouse->private;
 
 	psmouse_reset(psmouse);
+	del_timer_sync(&priv->timer);
 	input_unregister_device(priv->dev2);
 	kfree(priv);
 }
@@ -476,6 +675,8 @@ int alps_init(struct psmouse *psmouse)
 		goto init_fail;
 
 	priv->dev2 = dev2;
+	setup_timer(&priv->timer, alps_flush_packet, (unsigned long)psmouse);
+
 	psmouse->private = priv;
 
 	model = alps_get_model(psmouse, &version);
diff --git a/drivers/input/mouse/alps.h b/drivers/input/mouse/alps.h
index bc87936..904ed8b 100644
--- a/drivers/input/mouse/alps.h
+++ b/drivers/input/mouse/alps.h
@@ -23,6 +23,7 @@ struct alps_data {
 	char phys[32];			/* Phys */
 	const struct alps_model_info *i;/* Info */
 	int prev_fin;			/* Finger bit from previous packet */
+	struct timer_list timer;
 };
 
 #ifdef CONFIG_MOUSE_PS2_ALPS

Re: [PATCH 2/2] Alps Dualpoint, Interleaved packets

[Sebastian Kapfer]

Hi Dmitry,

I really hate to say, but I can still desync it.

[1240043.147596] alps.c: handle cf
[1240043.148778] alps.c: handle 14
[1240043.150043] alps.c: handle 1a
[1240043.157696] alps.c: handle 5f
[1240043.158851] alps.c: handle c
[1240043.160044] alps.c: handle 29
[1240043.163885] alps.c: handle 2f
[1240043.165051] alps.c: handle 0
[1240043.166213] alps.c: handle ff
[1240043.166218] alps.c: refusing packet[5] = ff
[1240043.166221] 
[1240043.166227] psmouse.c: DualPoint TouchPad at isa0060/serio1/input0 lost sync at byte 6

In short, it's a 6-byte followed by a 3-byte; we mistakenly process it
as a interleaved PS/2 (oh well), and, while the final bytes come in,
happen upon high bits set in the PS/2 packet.  In a way, this is correct
behaviour, because we _are_ processing the data incorrectly...  In my
last patch, this went unnoticed because I didn't check for the high bit
in bytes 7,8,9.

For all practical purposes (i.e. not pressing three buttons and fumbling
with both pointers at the same time), the patch works very well.

In particular, no double clicks.  :-)

-- 
Sebastian Kapfer
 Inst. für Theoretische Physik I        Office +49-9131-85-2-8450
 Universität Erlangen                   Mobile +49-160-9577-6436
 Staudtstr. 7, Raum 02.583.             sebastian.kapfer@physik.uni-erlangen.de

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] Alps Dualpoint, Interleaved packets

[Dmitry Torokhov]

Hi Sebastian,

On Tue, Dec 15, 2009 at 10:01:33PM +0100, Sebastian Kapfer wrote:
> 
> Hi Dmitry,
> 
> I really hate to say, but I can still desync it.
> 
> [1240043.147596] alps.c: handle cf
> [1240043.148778] alps.c: handle 14
> [1240043.150043] alps.c: handle 1a
> [1240043.157696] alps.c: handle 5f
> [1240043.158851] alps.c: handle c
> [1240043.160044] alps.c: handle 29
> [1240043.163885] alps.c: handle 2f
> [1240043.165051] alps.c: handle 0
> [1240043.166213] alps.c: handle ff
> [1240043.166218] alps.c: refusing packet[5] = ff
> [1240043.166221] 
> [1240043.166227] psmouse.c: DualPoint TouchPad at isa0060/serio1/input0 lost sync at byte 6
> 
> In short, it's a 6-byte followed by a 3-byte; we mistakenly process it
> as a interleaved PS/2 (oh well), and, while the final bytes come in,
> happen upon high bits set in the PS/2 packet.  In a way, this is correct
> behaviour, because we _are_ processing the data incorrectly...  In my
> last patch, this went unnoticed because I didn't check for the high bit
> in bytes 7,8,9.
>

*sigh* Yes, unfortunately in both cases we report incorrect data to
userspace... Unfortunately I don't see any way to fix that; not unless
somebody (ALPS?) tells us a bit more about protocol... But it is likely
that this as good as it gets even for them.

The good thing is that it shoudl not trigger during nrmal usage (I doubt
many people work the device with both hands).

> For all practical purposes (i.e. not pressing three buttons and fumbling
> with both pointers at the same time), the patch works very well.
> 
> In particular, no double clicks.  :-)
> 

OK, then I will apply it and send it on. Thank you for all your efforts.

-- 
Dmitry

Re: [PATCH 2/2] Alps Dualpoint, Interleaved packets

[Sebastian Kapfer]

Hi,

> OK, then I will apply it and send it on. Thank you for all your efforts.

No worries, it fixes my hardware :-)  Thanks for your help with the
timer thing!  I owe you a beer.

Greetings,
  Sebastian