[PATCH 1/4] alps: v7: Document the v7 touchpad packet protocol

[PATCH 1/4] alps: v7: Document the v7 touchpad packet protocol

[Hans de Goede]

Add a table documenting where all the bits are in the v7 touchpad packets.

Cc: stable@vger.kernel.org # 3.17
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/input/mouse/alps.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index d125a01..aceaade 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -881,6 +881,34 @@ static void alps_get_finger_coordinate_v7(struct input_mt_pos *mt,
 					  unsigned char *pkt,
 					  unsigned char pkt_id)
 {
+	/*
+	 *       packet-fmt    b7   b6    b5   b4   b3   b2   b1   b0
+	 * Byte0 TWO & MULTI    L    1     R    M    1 Y0-2 Y0-1 Y0-0
+	 * Byte0 NEW            L    1  X1-5    1    1 Y0-2 Y0-1 Y0-0
+	 * Byte1            Y0-10 Y0-9  Y0-8 Y0-7 Y0-6 Y0-5 Y0-4 Y0-3
+	 * Byte2            X0-11    1 X0-10 X0-9 X0-8 X0-7 X0-6 X0-5
+	 * Byte3            X1-11    1  X0-4 X0-3    1 X0-2 X0-1 X0-0
+	 * Byte4 TWO        X1-10  TWO  X1-9 X1-8 X1-7 X1-6 X1-5 X1-4
+	 * Byte4 MULTI      X1-10  TWO  X1-9 X1-8 X1-7 X1-6 Y1-5    1
+	 * Byte4 NEW        X1-10  TWO  X1-9 X1-8 X1-7 X1-6    0    0
+	 * Byte5 TWO & NEW  Y1-10    0  Y1-9 Y1-8 Y1-7 Y1-6 Y1-5 Y1-4
+	 * Byte5 MULTI      Y1-10    0  Y1-9 Y1-8 Y1-7 Y1-6  F-1  F-0
+	 * L:         Left button
+	 * R / M:     Non-clickpads: Right / Middle button
+	 *            Clickpads: When > 2 fingers are down, and some fingers
+	 *            are in the button area, then the 2 coordinates reported
+	 *            are for fingers outside the button area and these report
+	 *            extra fingers being present in the right / left button
+	 *            area. Note these fingers are not added to the F field!
+	 *            so if a TWO packet is received and R = 1 then there are
+	 *            3 fingers down, etc.
+	 * TWO:       1: Two touches present, byte 0/4/5 are in TWO fmt
+	 *            0: If byte 4 bit 0 is 1, then byte 0/4/5 are in MULTI fmt
+	 *               otherwise byte 0 bit 4 must be set and byte 0/4/5 are
+	 *               in NEW fmt
+	 * F:         Number of fingers - 3, 0 means 3 fingers, 1 means 4 ...
+	 */
+
 	mt[0].x = ((pkt[2] & 0x80) << 4);
 	mt[0].x |= ((pkt[2] & 0x3F) << 5);
 	mt[0].x |= ((pkt[3] & 0x30) >> 1);
-- 
2.1.0

[PATCH 2/4] alps: v7: Ignore new packets

[Hans de Goede]

NEW packets are send to indicate a discontinuity in the finger coordinate
reporting. Specifically a finger may have moved from slot 0 to 1 or vice versa.
INPUT_MT_TRACK takes care of this for us.

NEW packets have 3 problems:
1) They do not contain middle / right button info (on non clickpads)
   this can be worked around by preserving the old button state
2) They do not contain an accurate fingercount, and they are
   typically send when the number of fingers changes. We cannot use
   the old finger count as that may mismatch with the amount of
   touch coordinates we've available in the NEW packet
3) Their x data for the second touch is inaccurate leading to
   a possible jump of the x coordinate by 16 units when the first
   non NEW packet comes in

Since problems 2 & 3 cannot be worked around, just ignore them.

BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=86338
Cc: stable@vger.kernel.org # 3.17
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/input/mouse/alps.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index aceaade..6b11629 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -966,18 +966,36 @@ static int alps_decode_packet_v7(struct alps_fields *f,
 		return 0;
 	if (pkt_id == V7_PACKET_ID_UNKNOWN)
 		return -1;
+	/*
+	 * NEW packets are send to indicate a discontinuity in the finger
+	 * coordinate reporting. Specifically a finger may have moved from
+	 * slot 0 to 1 or vice versa. INPUT_MT_TRACK takes care of this for
+	 * us.
+	 *
+	 * NEW packets have 3 problems:
+	 * 1) They do not contain middle / right button info (on non clickpads)
+	 *    this can be worked around by preserving the old button state
+	 * 2) They do not contain an accurate fingercount, and they are
+	 *    typically send when the number of fingers changes. We cannot use
+	 *    the old finger count as that may mismatch with the amount of
+	 *    touch coordinates we've available in the NEW packet
+	 * 3) Their x data for the second touch is inaccurate leading to
+	 *    a possible jump of the x coordinate by 16 units when the first
+	 *    non NEW packet comes in
+	 * Since problems 2 & 3 cannot be worked around, just ignore them.
+	 */
+	if (pkt_id == V7_PACKET_ID_NEW)
+		return 1;
 
 	alps_get_finger_coordinate_v7(f->mt, p, pkt_id);
 
-	if (pkt_id == V7_PACKET_ID_TWO || pkt_id == V7_PACKET_ID_MULTI) {
-		f->left = (p[0] & 0x80) >> 7;
-		f->right = (p[0] & 0x20) >> 5;
-		f->middle = (p[0] & 0x10) >> 4;
-	}
+	f->left = (p[0] & 0x80) >> 7;
+	f->right = (p[0] & 0x20) >> 5;
+	f->middle = (p[0] & 0x10) >> 4;
 
 	if (pkt_id == V7_PACKET_ID_TWO)
 		f->fingers = alps_get_mt_count(f->mt);
-	else if (pkt_id == V7_PACKET_ID_MULTI)
+	else /* pkt_id == V7_PACKET_ID_MULTI */
 		f->fingers = 3 + (p[5] & 0x03);
 
 	return 0;
-- 
2.1.0

[PATCH 3/4] alps: v7: Sometimes a single touch is reported in mt[1] rather then mt[0]

[Hans de Goede]

The v7 proto differentiates between a primary touch (with high precision)
and a secondary touch (with lower precision). Normally when 2 fingers are
down and one is lifted the still present touch becomes the primary touch,
but some traces have shown that this does not happen always.

This commit deals with this by making alps_get_mt_count() not stop at the
first empty mt slot, and if a touch is present in mt[1] and not mt[0] moving
the data to mt[0] (for input_mt_assign_slots).

BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=86338
Cc: stable@vger.kernel.org # 3.17
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/input/mouse/alps.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 6b11629..77c7afe 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -947,12 +947,14 @@ static void alps_get_finger_coordinate_v7(struct input_mt_pos *mt,
 
 static int alps_get_mt_count(struct input_mt_pos *mt)
 {
-	int i;
+	int i, fingers = 0;
 
-	for (i = 0; i < MAX_TOUCHES && mt[i].x != 0 && mt[i].y != 0; i++)
-		/* empty */;
+	for (i = 0; i < MAX_TOUCHES; i++) {
+		if (mt[i].x != 0 || mt[i].y != 0)
+			fingers++;
+	}
 
-	return i;
+	return fingers;
 }
 
 static int alps_decode_packet_v7(struct alps_fields *f,
@@ -998,6 +1000,14 @@ static int alps_decode_packet_v7(struct alps_fields *f,
 	else /* pkt_id == V7_PACKET_ID_MULTI */
 		f->fingers = 3 + (p[5] & 0x03);
 
+	/* Sometimes a single touch is reported in mt[1] rather then mt[0] */
+	if (f->fingers == 1 && f->mt[0].x == 0 && f->mt[0].y == 0) {
+		f->mt[0].x = f->mt[1].x;
+		f->mt[0].y = f->mt[1].y;
+		f->mt[1].x = 0;
+		f->mt[1].y = 0;
+	}
+
 	return 0;
 }
 
-- 
2.1.0

[PATCH 4/4] alps: v7: Fix finger counting for > 2 fingers on clickpads

[Hans de Goede]

Protocol v7 uses the middle / right button bits on clickpads to communicate
"location" information of a 3th touch (and possible 4th) touch on clickpads.

Specifically when 3 touches are down, if one of the 3 touches is in the
left / right button area, this will get reported in the middle / right button
bits and the touchpad will still send a TWO type packet rather then a MULTI
type packet, so when this happens we must add the finger reported in the
button area to the finger count.

Likewise we must also add fingers reported this way to the finger count
when we get MULTI packets.

BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=86338
Cc: stable@vger.kernel.org # 3.17
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/input/mouse/alps.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 77c7afe..d88d73d 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -961,6 +961,7 @@ static int alps_decode_packet_v7(struct alps_fields *f,
 				  unsigned char *p,
 				  struct psmouse *psmouse)
 {
+	struct alps_data *priv = psmouse->private;
 	unsigned char pkt_id;
 
 	pkt_id = alps_get_packet_id_v7(p);
@@ -991,15 +992,22 @@ static int alps_decode_packet_v7(struct alps_fields *f,
 
 	alps_get_finger_coordinate_v7(f->mt, p, pkt_id);
 
-	f->left = (p[0] & 0x80) >> 7;
-	f->right = (p[0] & 0x20) >> 5;
-	f->middle = (p[0] & 0x10) >> 4;
-
 	if (pkt_id == V7_PACKET_ID_TWO)
 		f->fingers = alps_get_mt_count(f->mt);
 	else /* pkt_id == V7_PACKET_ID_MULTI */
 		f->fingers = 3 + (p[5] & 0x03);
 
+	f->left = (p[0] & 0x80) >> 7;
+	if (priv->flags & ALPS_BUTTONPAD) {
+		if (p[0] & 0x20)
+			f->fingers++;
+		if (p[0] & 0x10)
+			f->fingers++;
+	} else {
+		f->right = (p[0] & 0x20) >> 5;
+		f->middle = (p[0] & 0x10) >> 4;
+	}
+
 	/* Sometimes a single touch is reported in mt[1] rather then mt[0] */
 	if (f->fingers == 1 && f->mt[0].x == 0 && f->mt[0].y == 0) {
 		f->mt[0].x = f->mt[1].x;
-- 
2.1.0

Re: [PATCH 1/4] alps: v7: Document the v7 touchpad packet protocol

[Benjamin Tissoires]

On Wed, Dec 3, 2014 at 8:27 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Add a table documenting where all the bits are in the v7 touchpad packets.
>
> Cc: stable@vger.kernel.org # 3.17
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---

Hi Hans,

I tested yesterday the series on a Toshiba W50-A and a Z30-A if my
memory is accurate. Internal QA tested these on a Dell M4700 (don't
know if it has a v7 touchpad though).
Both tests were conclusive, i.e. they fixes the spurious release when
right-clicking.

Tested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

I am not confident enough with the code to review 3/4 and 4/4, but
they look sane to me.

Cheers,
Benjamin

>  drivers/input/mouse/alps.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index d125a01..aceaade 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -881,6 +881,34 @@ static void alps_get_finger_coordinate_v7(struct input_mt_pos *mt,
>                                           unsigned char *pkt,
>                                           unsigned char pkt_id)
>  {
> +       /*
> +        *       packet-fmt    b7   b6    b5   b4   b3   b2   b1   b0
> +        * Byte0 TWO & MULTI    L    1     R    M    1 Y0-2 Y0-1 Y0-0
> +        * Byte0 NEW            L    1  X1-5    1    1 Y0-2 Y0-1 Y0-0
> +        * Byte1            Y0-10 Y0-9  Y0-8 Y0-7 Y0-6 Y0-5 Y0-4 Y0-3
> +        * Byte2            X0-11    1 X0-10 X0-9 X0-8 X0-7 X0-6 X0-5
> +        * Byte3            X1-11    1  X0-4 X0-3    1 X0-2 X0-1 X0-0
> +        * Byte4 TWO        X1-10  TWO  X1-9 X1-8 X1-7 X1-6 X1-5 X1-4
> +        * Byte4 MULTI      X1-10  TWO  X1-9 X1-8 X1-7 X1-6 Y1-5    1
> +        * Byte4 NEW        X1-10  TWO  X1-9 X1-8 X1-7 X1-6    0    0
> +        * Byte5 TWO & NEW  Y1-10    0  Y1-9 Y1-8 Y1-7 Y1-6 Y1-5 Y1-4
> +        * Byte5 MULTI      Y1-10    0  Y1-9 Y1-8 Y1-7 Y1-6  F-1  F-0
> +        * L:         Left button
> +        * R / M:     Non-clickpads: Right / Middle button
> +        *            Clickpads: When > 2 fingers are down, and some fingers
> +        *            are in the button area, then the 2 coordinates reported
> +        *            are for fingers outside the button area and these report
> +        *            extra fingers being present in the right / left button
> +        *            area. Note these fingers are not added to the F field!
> +        *            so if a TWO packet is received and R = 1 then there are
> +        *            3 fingers down, etc.
> +        * TWO:       1: Two touches present, byte 0/4/5 are in TWO fmt
> +        *            0: If byte 4 bit 0 is 1, then byte 0/4/5 are in MULTI fmt
> +        *               otherwise byte 0 bit 4 must be set and byte 0/4/5 are
> +        *               in NEW fmt
> +        * F:         Number of fingers - 3, 0 means 3 fingers, 1 means 4 ...
> +        */
> +
>         mt[0].x = ((pkt[2] & 0x80) << 4);
>         mt[0].x |= ((pkt[2] & 0x3F) << 5);
>         mt[0].x |= ((pkt[3] & 0x30) >> 1);
> --
> 2.1.0
>
> --
> 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 1/4] alps: v7: Document the v7 touchpad packet protocol

[Dmitry Torokhov]

On Fri, Dec 05, 2014 at 04:58:00PM -0500, Benjamin Tissoires wrote:
> On Wed, Dec 3, 2014 at 8:27 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> > Add a table documenting where all the bits are in the v7 touchpad packets.
> >
> > Cc: stable@vger.kernel.org # 3.17
> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > ---
> 
> Hi Hans,
> 
> I tested yesterday the series on a Toshiba W50-A and a Z30-A if my
> memory is accurate. Internal QA tested these on a Dell M4700 (don't
> know if it has a v7 touchpad though).
> Both tests were conclusive, i.e. they fixes the spurious release when
> right-clicking.
> 
> Tested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> 
> I am not confident enough with the code to review 3/4 and 4/4, but
> they look sane to me.

Applied all, thank you.

-- 
Dmitry