Hi Dmitry, thank you for your reply. It is much cleaner now! I had to make a few changes though: 1. As posted, the rearranged patch doesn't work properly. One has to retain the sign bits of the PS/2 subpacket. 2. I've also pulled the checking of byte0 before the demuxing of the PS/2 subpacket. I think it's safer to desync early if the data is bad. 3. The hardware is very broken: Touchpad and trackpoint share button state. This means that you can trigger this sequence of events: <user presses button on trackpoint> 3byte: left down --> reported to "dev2" <user moves pointer with touchpad> 6byte: left down --> reported to "dev" <user releases button on trackpoint> 3byte: left up --> reported to "dev2" The result is that dev has a stuck mouse button, and in X11 the mouse button stays down. That's why I explicitly cloned button events to both devices in my first patch. However, this created a different problem: If the user hammered at the mouse button very quickly, the events would be processed out of order, e.g. touch_press touch_release stick_press stick_release instead of touch_press stick_press touch_release stick_release As a result of this, a double click was detected in X11. I have added logic that assigns each button down event to only one of the devices, and also avoids hanging buttons. This is activated by a new flag. (I'm pretty sure the shared button state is true for most if not all Alps dualpoints, but I made a separate flag out of it for now). 4. I've noticed the applied patch for the 4-button Alps device. Is it really intended that one of the buttons fires both a BTN_x event and a BTN_MIDDLE event? I don't think so, even tough they share the same bit in the packet. BTN_MIDDLE should never be emitted from a device with the ALPS_FOUR_BUTTONS flag IMHO. I haven't changed this, never having seen such a unit. 5. There remains a slight conceptual problem. The distinction between 6-byte and 9-byte packets is not possible only looking at the first 6 bytes. (This is a protocoll issue. We have scrutinized every bit now, the protocol just seems to be broken in this regard.) This means that if the user triggers a 6-byte message while holding all three buttons down (e.g. hold buttons and move pointer via touchpad), we run into de-sync. We can't solve this without knowing the message size in the driver. I have no idea if we can somehow get this info out of the PS/2 layer. Dmitry, do you have any insight into this? I still vote strongly for applying the patch, since this is a mostly cosmetic problem that never occurs in practical work whereas in the current state my touchpad is unusable. Best wishes, Sebastian. Patch following: Implement protocol for certain Alps dualpoint touchpads sending 9-byte packets, common in Dell laptops, e.g. E6x00. Contains pieces of work from Sebastian Kapfer, David Kubicek, Erik Osterholm, and Dmitry Torokhov. Signed-off-by: Sebastian Kapfer <sebastian_kapfer@xxxxxxx> --- drivers/input/mouse/alps.c | 140 +++++++++++++++++++++++++++++++++++-------- drivers/input/mouse/alps.h | 3 +- 2 files changed, 116 insertions(+), 27 deletions(-) diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c index a3f492a..aa2498e 100644 --- a/drivers/input/mouse/alps.c +++ b/drivers/input/mouse/alps.c @@ -5,10 +5,14 @@ * Copyright (c) 2003-2005 Peter Osterlund <petero2@xxxxxxxxx> * Copyright (c) 2004 Dmitry Torokhov <dtor@xxxxxxx> * Copyright (c) 2005 Vojtech Pavlik <vojtech@xxxxxxx> + * Copyright (c) 2009 Sebastian Kapfer <sebastian_kapfer@xxxxxxx> * * ALPS detection, tap switching and status querying info is taken from * tpconfig utility (by C. Scott Ananian and Bruce Kall). * + * Additional research by David Kubicek and Erik Osterholm + * (https://bugs.launchpad.net/ubuntu/+source/linux/+bug/296610 ) + * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU General Public License version 2 as published by * the Free Software Foundation. @@ -28,7 +32,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 +40,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[] = { { { 0x32, 0x02, 0x14 }, 0xf8, 0xf8, ALPS_PASS | ALPS_DUALPOINT }, /* Toshiba Salellite Pro M10 */ @@ -58,7 +63,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, 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 +76,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 +91,59 @@ 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. */ + +/* for Alps units where PS/2 and touchpad share a common button state, try our + * best to disentangle them. If we don't do this, we either get hanging + * buttons (when the button-up occurs on the 'wrong' of the two input_dev's) or + * duplicated events (if we just broadcast button state to both input_dev's). + * the latter can arrive in X11 as doubleclicks if the user clicked quickly. + * This also has the advantage that users can assign different actions to + * the buttons. + */ +static void alps_report_button(struct psmouse *psmouse, + int whichbtn, int state, int mask, + struct input_dev *preferred_dev) +{ + struct alps_data *priv = psmouse->private; + if (priv->i->flags & ALPS_SHARED_BTNSTATE) { + if (state) { + if (priv->btn_state & mask) + /* already communicated, drop */ + return; + priv->btn_state |= mask; + } else { + /* release the button on the other device */ + struct input_dev *other = psmouse->dev; + if (preferred_dev == other) + other = priv->dev2; + priv->btn_state &= 7 ^ mask; + input_report_key(other, whichbtn, 0); + input_sync(other); + } + } + + input_report_key(preferred_dev, whichbtn, state); +} + static void alps_process_packet(struct psmouse *psmouse) { struct alps_data *priv = psmouse->private; @@ -93,18 +154,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 +189,17 @@ 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_button(psmouse, BTN_LEFT, left, 1, dev2); + alps_report_button(psmouse, BTN_RIGHT, right, 2, dev2); + alps_report_button(psmouse, BTN_MIDDLE, middle, 4, 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_button(psmouse, BTN_LEFT, left, 1, dev); + alps_report_button(psmouse, BTN_RIGHT, right, 2, dev); + alps_report_button(psmouse, BTN_MIDDLE, middle, 4, dev); /* Convert hardware tap to a reasonable Z value */ if (ges && !fin) @@ -202,25 +250,65 @@ 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_button(psmouse, BTN_LEFT, packet[0] & 1, 1, dev2); + alps_report_button(psmouse, BTN_RIGHT, packet[0] & 2, 2, dev2); + alps_report_button(psmouse, BTN_MIDDLE, packet[0] & 4, 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_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) + 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; + } + + /* 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) { + if (psmouse->pktcnt < 7) + return PSMOUSE_GOOD_DATA; + + /* Get the real button data */ + 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; + } /* 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); diff --git a/drivers/input/mouse/alps.h b/drivers/input/mouse/alps.h index bc87936..af0f370 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; /* Shared state of the buttons */ }; #ifdef CONFIG_MOUSE_PS2_ALPS -- 1.6.3.3 -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
