[PATCH v2 0/3] Input: synaptics - add semi-mt support

[PATCH v2 0/3] Input: synaptics - add semi-mt support

[Henrik Rydberg]

Hi all,

So I ended up modifying the first patch, after all, implementing
Dmitry's comments. The patch did shrink a bit, and it it is still to
be signed off by the original authors, if you please.

Testing seems ok on my local machine, and I put a dkms out in the wild
to capture any oddities. Otherwise, the patches are doing exactly the
same thing as before.

Takashi, Chase, Chris?

Thanks,
Henrik

Henrik Rydberg (3):
  Input: synaptics - report clickpad property
  Input: synaptics - add multi-finger and semi-mt support
  Input: synaptics - ignore bogus mt packet

 drivers/input/mouse/synaptics.c |   95 ++++++++++++++++++++++++++++++++++++--
 drivers/input/mouse/synaptics.h |    3 +
 2 files changed, 93 insertions(+), 5 deletions(-)

-- 
1.7.2.3

[PATCH 1/3] Input: synaptics - report clickpad property

[Henrik Rydberg]

With the new input property interface, it is possible to report the
special quirks of a device using ioctl/sysfs. This patch sets up the
device as a pointer, and reports the clickpad functionality via the
INPUT_PROP_BUTTONPAD property.

Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/input/mouse/synaptics.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 2e300a4..8997cbc 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -622,6 +622,8 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
 {
 	int i;
 
+	__set_bit(INPUT_PROP_POINTER, dev->propbit);
+
 	__set_bit(EV_ABS, dev->evbit);
 	input_set_abs_params(dev, ABS_X,
 			     XMIN_NOMINAL, priv->x_max ?: XMAX_NOMINAL, 0, 0);
@@ -663,6 +665,7 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
 	input_abs_set_res(dev, ABS_Y, priv->y_res);
 
 	if (SYN_CAP_CLICKPAD(priv->ext_cap_0c)) {
+		__set_bit(INPUT_PROP_BUTTONPAD, dev->propbit);
 		/* Clickpads report only left button */
 		__clear_bit(BTN_RIGHT, dev->keybit);
 		__clear_bit(BTN_MIDDLE, dev->keybit);
-- 
1.7.2.3

[PATCH 2/3] Input: synaptics - add multi-finger and semi-mt support

[Henrik Rydberg]

The Synaptics 2.7 series of touchpads support a mode for reporting two
sets of X/Y/Pressure data (advanced gesture mode). By default, these
devices report only single finger data, depriving userspace of the
nowadays ubiquitous two-finger scroll gesture.

Enabling advanced gesture mode also enables the multi-finger report,
although the device does not claim that capability. Up to three
fingers can be reported this way.

While two or three fingers are touching, the normal packet is
prepended by a reduced finger packet of lower resolution. From the two
packets (which do not represent the actual fingers), the bounding
rectangle of the individual contacts can be extracted.  This
information is sufficient to perform scaling gestures and a limited
form of rotation gesture. The behavior has been coined semi-mt
capability, and is signaled to userspace via the INPUT_PROP_SEMI_MT
device property.

Work to decode the advanced gesture packet: Takashi Iwai.
Cleanup and testing of the original patch: Chase Douglas.
Minor cleanup and testing: Chris Bagwell.
Finalization and semi-mt support: Henrik Rydberg.

Reported-by: Tobyn Bertram
Not-yet-signed-off-by: Takashi Iwai <tiwai@suse.de>
Not-yet-signed-off-by: Chase Douglas <chase.douglas@canonical.com>
Not-yet-signed-off-by: Chris Bagwell <chris@cnpbagwell.com>
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/input/mouse/synaptics.c |   88 +++++++++++++++++++++++++++++++++++++-
 drivers/input/mouse/synaptics.h |    3 +
 2 files changed, 88 insertions(+), 3 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 8997cbc..2f8a97a 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -25,7 +25,7 @@
 
 #include <linux/module.h>
 #include <linux/dmi.h>
-#include <linux/input.h>
+#include <linux/input/mt.h>
 #include <linux/serio.h>
 #include <linux/libps2.h>
 #include <linux/slab.h>
@@ -279,6 +279,25 @@ static void synaptics_set_rate(struct psmouse *psmouse, unsigned int rate)
 	synaptics_mode_cmd(psmouse, priv->mode);
 }
 
+static int synaptics_set_advanced_gesture_mode(struct psmouse *psmouse)
+{
+	static unsigned char param = 0xc8;
+	struct synaptics_data *priv = psmouse->private;
+
+	if (!SYN_CAP_ADV_GESTURE(priv->ext_cap_0c))
+		return 0;
+
+	if (psmouse_sliced_command(psmouse, SYN_QUE_MODEL))
+		return -1;
+	if (ps2_command(&psmouse->ps2dev, &param, PSMOUSE_CMD_SETRATE))
+		return -1;
+
+	/* Advanced gesture mode also sends multi finger data */
+	priv->capabilities |= BIT(1);
+
+	return 0;
+}
+
 /*****************************************************************************
  *	Synaptics pass-through PS/2 port support
  ****************************************************************************/
@@ -380,7 +399,9 @@ static void synaptics_pt_create(struct psmouse *psmouse)
  *	Functions to interpret the absolute mode packets
  ****************************************************************************/
 
-static void synaptics_parse_hw_state(unsigned char buf[], struct synaptics_data *priv, struct synaptics_hw_state *hw)
+static int synaptics_parse_hw_state(const unsigned char buf[],
+				    struct synaptics_data *priv,
+				    struct synaptics_hw_state *hw)
 {
 	memset(hw, 0, sizeof(struct synaptics_hw_state));
 
@@ -397,6 +418,14 @@ static void synaptics_parse_hw_state(unsigned char buf[], struct synaptics_data
 			 ((buf[0] & 0x04) >> 1) |
 			 ((buf[3] & 0x04) >> 2));
 
+		if (SYN_CAP_ADV_GESTURE(priv->ext_cap_0c) && hw->w == 2) {
+			/* Gesture packet: (x, y, z) at half resolution */
+			priv->mt.x = (((buf[4] & 0x0f) << 8) | buf[1]) << 1;
+			priv->mt.y = (((buf[4] & 0xf0) << 4) | buf[2]) << 1;
+			priv->mt.z = ((buf[3] & 0x30) | (buf[5] & 0x0f)) << 1;
+			return 1;
+		}
+
 		hw->left  = (buf[0] & 0x01) ? 1 : 0;
 		hw->right = (buf[0] & 0x02) ? 1 : 0;
 
@@ -452,6 +481,36 @@ static void synaptics_parse_hw_state(unsigned char buf[], struct synaptics_data
 		hw->left  = (buf[0] & 0x01) ? 1 : 0;
 		hw->right = (buf[0] & 0x02) ? 1 : 0;
 	}
+
+	return 0;
+}
+
+static void set_slot(struct input_dev *dev, int slot, bool active, int x, int y)
+{
+	input_mt_slot(dev, slot);
+	input_mt_report_slot_state(dev, MT_TOOL_FINGER, active);
+	if (active) {
+		input_report_abs(dev, ABS_MT_POSITION_X, x);
+		input_report_abs(dev, ABS_MT_POSITION_Y,
+				 YMAX_NOMINAL + YMIN_NOMINAL - y);
+	}
+}
+
+static void synaptics_report_semi_mt_data(struct input_dev *dev,
+					  const struct synaptics_hw_state *a,
+					  const struct synaptics_hw_state *b,
+					  int num_fingers)
+{
+	if (num_fingers >= 2) {
+		set_slot(dev, 0, true, min(a->x, b->x), min(a->y, b->y));
+		set_slot(dev, 1, true, max(a->x, b->x), max(a->y, b->y));
+	} else if (num_fingers == 1) {
+		set_slot(dev, 0, true, a->x, a->y);
+		set_slot(dev, 1, false, 0, 0);
+	} else {
+		set_slot(dev, 0, false, 0, 0);
+		set_slot(dev, 1, false, 0, 0);
+	}
 }
 
 /*
@@ -466,7 +525,8 @@ static void synaptics_process_packet(struct psmouse *psmouse)
 	int finger_width;
 	int i;
 
-	synaptics_parse_hw_state(psmouse->packet, priv, &hw);
+	if (synaptics_parse_hw_state(psmouse->packet, priv, &hw))
+		return;
 
 	if (hw.scroll) {
 		priv->scroll += hw.scroll;
@@ -512,6 +572,9 @@ static void synaptics_process_packet(struct psmouse *psmouse)
 		finger_width = 0;
 	}
 
+	if (SYN_CAP_ADV_GESTURE(priv->ext_cap_0c))
+		synaptics_report_semi_mt_data(dev, &hw, &priv->mt, num_fingers);
+
 	/* Post events
 	 * BTN_TOUCH has to be first as mousedev relies on it when doing
 	 * absolute -> relative conversion
@@ -623,6 +686,7 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
 	int i;
 
 	__set_bit(INPUT_PROP_POINTER, dev->propbit);
+	__set_bit(INPUT_PROP_SEMI_MT, dev->propbit);
 
 	__set_bit(EV_ABS, dev->evbit);
 	input_set_abs_params(dev, ABS_X,
@@ -631,6 +695,14 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
 			     YMIN_NOMINAL, priv->y_max ?: YMAX_NOMINAL, 0, 0);
 	input_set_abs_params(dev, ABS_PRESSURE, 0, 255, 0, 0);
 
+	if (SYN_CAP_ADV_GESTURE(priv->ext_cap_0c)) {
+		input_mt_init_slots(dev, 2);
+		input_set_abs_params(dev, ABS_MT_POSITION_X, XMIN_NOMINAL,
+				     priv->x_max ?: XMAX_NOMINAL, 0, 0);
+		input_set_abs_params(dev, ABS_MT_POSITION_Y, YMIN_NOMINAL,
+				     priv->y_max ?: YMAX_NOMINAL, 0, 0);
+	}
+
 	if (SYN_CAP_PALMDETECT(priv->capabilities))
 		input_set_abs_params(dev, ABS_TOOL_WIDTH, 0, 15, 0, 0);
 
@@ -705,6 +777,11 @@ static int synaptics_reconnect(struct psmouse *psmouse)
 		return -1;
 	}
 
+	if (synaptics_set_advanced_gesture_mode(psmouse)) {
+		printk(KERN_ERR "Advanced gesture mode reconnect failed.\n");
+		return -1;
+	}
+
 	return 0;
 }
 
@@ -772,6 +849,11 @@ int synaptics_init(struct psmouse *psmouse)
 		goto init_fail;
 	}
 
+	if (synaptics_set_advanced_gesture_mode(psmouse)) {
+		printk(KERN_ERR "Advanced gesture mode init failed.\n");
+		goto init_fail;
+	}
+
 	priv->pkt_type = SYN_MODEL_NEWABS(priv->model_id) ? SYN_NEWABS : SYN_OLDABS;
 
 	printk(KERN_INFO "Synaptics Touchpad, model: %ld, fw: %ld.%ld, id: %#lx, caps: %#lx/%#lx/%#lx\n",
diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
index 613a365..50e20e9 100644
--- a/drivers/input/mouse/synaptics.h
+++ b/drivers/input/mouse/synaptics.h
@@ -53,6 +53,7 @@
 #define SYN_CAP_PRODUCT_ID(ec)		(((ec) & 0xff0000) >> 16)
 #define SYN_CAP_CLICKPAD(ex0c)		((ex0c) & 0x100100)
 #define SYN_CAP_MAX_DIMENSIONS(ex0c)	((ex0c) & 0x020000)
+#define SYN_CAP_ADV_GESTURE(ex0c)	((ex0c) & 0x080000)
 
 /* synaptics modes query bits */
 #define SYN_MODE_ABSOLUTE(m)		((m) & (1 << 7))
@@ -112,6 +113,8 @@ struct synaptics_data {
 	int scroll;
 
 	struct serio *pt_port;			/* Pass-through serio port */
+
+	struct synaptics_hw_state mt;		/* current gesture packet */
 };
 
 void synaptics_module_init(void);
-- 
1.7.2.3

[PATCH 3/3] Input: synaptics - ignore bogus mt packet

[Henrik Rydberg]

In multitouch mode, at least one device (fw: 7.4 id: 0x1c0b1) sometimes
sends a final main packet with x == 1. Since the normal values are above
1472, this is clearly bogus. At the same time, a two-finger touch is
signaled, even though only one finger was on the pad to begin with. This
patch ignores the packet altogether, removing the problem.

Acked-by: Chris Bagwell <chris@cnpbagwell.com>
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/input/mouse/synaptics.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 2f8a97a..ae85ab2 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -548,7 +548,7 @@ static void synaptics_process_packet(struct psmouse *psmouse)
 		return;
 	}
 
-	if (hw.z > 0) {
+	if (hw.z > 0 && hw.x > 1) {
 		num_fingers = 1;
 		finger_width = 5;
 		if (SYN_CAP_EXTENDED(priv->capabilities)) {
@@ -582,7 +582,7 @@ static void synaptics_process_packet(struct psmouse *psmouse)
 	if (hw.z > 30) input_report_key(dev, BTN_TOUCH, 1);
 	if (hw.z < 25) input_report_key(dev, BTN_TOUCH, 0);
 
-	if (hw.z > 0) {
+	if (num_fingers > 0) {
 		input_report_abs(dev, ABS_X, hw.x);
 		input_report_abs(dev, ABS_Y, YMAX_NOMINAL + YMIN_NOMINAL - hw.y);
 	}
-- 
1.7.2.3

Re: [PATCH 2/3] Input: synaptics - add multi-finger and semi-mt support

[Chris Bagwell]

On Mon, Dec 20, 2010 at 7:39 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
> The Synaptics 2.7 series of touchpads support a mode for reporting two
> sets of X/Y/Pressure data (advanced gesture mode). By default, these
> devices report only single finger data, depriving userspace of the
> nowadays ubiquitous two-finger scroll gesture.
>
> Enabling advanced gesture mode also enables the multi-finger report,
> although the device does not claim that capability. Up to three
> fingers can be reported this way.
>
> While two or three fingers are touching, the normal packet is
> prepended by a reduced finger packet of lower resolution. From the two
> packets (which do not represent the actual fingers), the bounding
> rectangle of the individual contacts can be extracted.  This
> information is sufficient to perform scaling gestures and a limited
> form of rotation gesture. The behavior has been coined semi-mt
> capability, and is signaled to userspace via the INPUT_PROP_SEMI_MT
> device property.
>
> Work to decode the advanced gesture packet: Takashi Iwai.
> Cleanup and testing of the original patch: Chase Douglas.
> Minor cleanup and testing: Chris Bagwell.
> Finalization and semi-mt support: Henrik Rydberg.
>
> Reported-by: Tobyn Bertram
> Not-yet-signed-off-by: Takashi Iwai <tiwai@suse.de>
> Not-yet-signed-off-by: Chase Douglas <chase.douglas@canonical.com>
> Not-yet-signed-off-by: Chris Bagwell <chris@cnpbagwell.com>
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> ---


That turned out better.  You can keep my sign off on there.

Chris

Re: [PATCH 2/3] Input: synaptics - add multi-finger and semi-mt support

[Chase Douglas]

On 12/20/2010 08:39 AM, Henrik Rydberg wrote:
> The Synaptics 2.7 series of touchpads support a mode for reporting two
> sets of X/Y/Pressure data (advanced gesture mode). By default, these
> devices report only single finger data, depriving userspace of the
> nowadays ubiquitous two-finger scroll gesture.
> 
> Enabling advanced gesture mode also enables the multi-finger report,
> although the device does not claim that capability. Up to three
> fingers can be reported this way.
> 
> While two or three fingers are touching, the normal packet is
> prepended by a reduced finger packet of lower resolution. From the two
> packets (which do not represent the actual fingers), the bounding
> rectangle of the individual contacts can be extracted.  This
> information is sufficient to perform scaling gestures and a limited
> form of rotation gesture. The behavior has been coined semi-mt
> capability, and is signaled to userspace via the INPUT_PROP_SEMI_MT
> device property.
> 
> Work to decode the advanced gesture packet: Takashi Iwai.
> Cleanup and testing of the original patch: Chase Douglas.
> Minor cleanup and testing: Chris Bagwell.
> Finalization and semi-mt support: Henrik Rydberg.
> 
> Reported-by: Tobyn Bertram
> Not-yet-signed-off-by: Takashi Iwai <tiwai@suse.de>
> Not-yet-signed-off-by: Chase Douglas <chase.douglas@canonical.com>
> Not-yet-signed-off-by: Chris Bagwell <chris@cnpbagwell.com>
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>

You can keep my SOB.

> +static void set_slot(struct input_dev *dev, int slot, bool active, int x, int y)
> +{
> +	input_mt_slot(dev, slot);
> +	input_mt_report_slot_state(dev, MT_TOOL_FINGER, active);
> +	if (active) {
> +		input_report_abs(dev, ABS_MT_POSITION_X, x);
> +		input_report_abs(dev, ABS_MT_POSITION_Y,
> +				 YMAX_NOMINAL + YMIN_NOMINAL - y);
> +	}
> +}

I take it that you feel MT_TOOL_FINGER should always be set, even if
it's always the same as BTN_TOOL_*? I just want to be sure this is
intended so we document it appropriately.

> @@ -623,6 +686,7 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
>  	int i;
>  
>  	__set_bit(INPUT_PROP_POINTER, dev->propbit);
> +	__set_bit(INPUT_PROP_SEMI_MT, dev->propbit);

Shouldn't this only be set when SYN_CAP_ADV_GESTURE(priv->ext_cap_0c) is
true?

Thanks,

-- Chase

Re: [PATCH 1/3] Input: synaptics - report clickpad property

[Chase Douglas]

On 12/20/2010 08:39 AM, Henrik Rydberg wrote:
> With the new input property interface, it is possible to report the
> special quirks of a device using ioctl/sysfs. This patch sets up the
> device as a pointer, and reports the clickpad functionality via the
> INPUT_PROP_BUTTONPAD property.
> 
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>

Acked-by: Chase Douglas <chase.douglas@canonical.com>

Re: [PATCH 3/3] Input: synaptics - ignore bogus mt packet

[Chase Douglas]

On 12/20/2010 08:39 AM, Henrik Rydberg wrote:
> In multitouch mode, at least one device (fw: 7.4 id: 0x1c0b1) sometimes
> sends a final main packet with x == 1. Since the normal values are above
> 1472, this is clearly bogus. At the same time, a two-finger touch is
> signaled, even though only one finger was on the pad to begin with. This
> patch ignores the packet altogether, removing the problem.
> 
> Acked-by: Chris Bagwell <chris@cnpbagwell.com>
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>

Acked-by: Chase Douglas <chase.douglas@canonical.com>

Re: [PATCH 2/3] Input: synaptics - add multi-finger and semi-mt support

[Henrik Rydberg]

> > Reported-by: Tobyn Bertram
> > Not-yet-signed-off-by: Takashi Iwai <tiwai@suse.de>
> > Not-yet-signed-off-by: Chase Douglas <chase.douglas@canonical.com>
> > Not-yet-signed-off-by: Chris Bagwell <chris@cnpbagwell.com>
> > Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> 
> You can keep my SOB.

Great, thanks.
 
> > +static void set_slot(struct input_dev *dev, int slot, bool active, int x, int y)
> > +{
> > +	input_mt_slot(dev, slot);
> > +	input_mt_report_slot_state(dev, MT_TOOL_FINGER, active);
> > +	if (active) {
> > +		input_report_abs(dev, ABS_MT_POSITION_X, x);
> > +		input_report_abs(dev, ABS_MT_POSITION_Y,
> > +				 YMAX_NOMINAL + YMIN_NOMINAL - y);
> > +	}
> > +}
> 
> I take it that you feel MT_TOOL_FINGER should always be set, even if
> it's always the same as BTN_TOOL_*? I just want to be sure this is
> intended so we document it appropriately.

Yes - the MT_TOOLs are only emitted when explicitly set in absbit, but
the internal interface always handles it. This is actually documented
in the code (and DocBook).

> 
> > @@ -623,6 +686,7 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
> >  	int i;
> >  
> >  	__set_bit(INPUT_PROP_POINTER, dev->propbit);
> > +	__set_bit(INPUT_PROP_SEMI_MT, dev->propbit);
> 
> Shouldn't this only be set when SYN_CAP_ADV_GESTURE(priv->ext_cap_0c) is
> true?

Indeed - thanks.

Henrik

Re: [PATCH 2/3] Input: synaptics - add multi-finger and semi-mt support

[Chase Douglas]

On 12/21/2010 11:59 AM, Henrik Rydberg wrote:
>>> Reported-by: Tobyn Bertram
>>> Not-yet-signed-off-by: Takashi Iwai <tiwai@suse.de>
>>> Not-yet-signed-off-by: Chase Douglas <chase.douglas@canonical.com>
>>> Not-yet-signed-off-by: Chris Bagwell <chris@cnpbagwell.com>
>>> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
>>
>> You can keep my SOB.
> 
> Great, thanks.
>  
>>> +static void set_slot(struct input_dev *dev, int slot, bool active, int x, int y)
>>> +{
>>> +	input_mt_slot(dev, slot);
>>> +	input_mt_report_slot_state(dev, MT_TOOL_FINGER, active);
>>> +	if (active) {
>>> +		input_report_abs(dev, ABS_MT_POSITION_X, x);
>>> +		input_report_abs(dev, ABS_MT_POSITION_Y,
>>> +				 YMAX_NOMINAL + YMIN_NOMINAL - y);
>>> +	}
>>> +}
>>
>> I take it that you feel MT_TOOL_FINGER should always be set, even if
>> it's always the same as BTN_TOOL_*? I just want to be sure this is
>> intended so we document it appropriately.
> 
> Yes - the MT_TOOLs are only emitted when explicitly set in absbit, but
> the internal interface always handles it. This is actually documented
> in the code (and DocBook).

Ahh, I had forgotten about this.

>>> @@ -623,6 +686,7 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
>>>  	int i;
>>>  
>>>  	__set_bit(INPUT_PROP_POINTER, dev->propbit);
>>> +	__set_bit(INPUT_PROP_SEMI_MT, dev->propbit);
>>
>> Shouldn't this only be set when SYN_CAP_ADV_GESTURE(priv->ext_cap_0c) is
>> true?
> 
> Indeed - thanks.

All looks good to me with this change :). Thanks again!

-- Chase

Re: [PATCH 2/3] Input: synaptics - add multi-finger and semi-mt support

[Chris Bagwell]

On Tue, Dec 21, 2010 at 10:59 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
>> > Reported-by: Tobyn Bertram
>> > Not-yet-signed-off-by: Takashi Iwai <tiwai@suse.de>
>> > Not-yet-signed-off-by: Chase Douglas <chase.douglas@canonical.com>
>> > Not-yet-signed-off-by: Chris Bagwell <chris@cnpbagwell.com>
>> > Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
>>
>> You can keep my SOB.
>
> Great, thanks.
>
>> > +static void set_slot(struct input_dev *dev, int slot, bool active, int x, int y)
>> > +{
>> > +   input_mt_slot(dev, slot);
>> > +   input_mt_report_slot_state(dev, MT_TOOL_FINGER, active);
>> > +   if (active) {
>> > +           input_report_abs(dev, ABS_MT_POSITION_X, x);
>> > +           input_report_abs(dev, ABS_MT_POSITION_Y,
>> > +                            YMAX_NOMINAL + YMIN_NOMINAL - y);
>> > +   }
>> > +}
>>
>> I take it that you feel MT_TOOL_FINGER should always be set, even if
>> it's always the same as BTN_TOOL_*? I just want to be sure this is
>> intended so we document it appropriately.
>
> Yes - the MT_TOOLs are only emitted when explicitly set in absbit, but
> the internal interface always handles it. This is actually documented
> in the code (and DocBook).

That note is good guidance for developer side.  Its also worth noting
on app side that MT_TOOL_FINGER is a little special since its value is
0.  In most common case, I think it will get filtered out where as
BTN_TOOL_FINGER will always be sent.

Since we can't yet query per slot ABS_MT_TOOL_TYPE, I guess apps have
to just assume its a finger unless told otherwise?

Chris

Re: [PATCH 2/3] Input: synaptics - add multi-finger and semi-mt support

[Henrik Rydberg]

> > Yes - the MT_TOOLs are only emitted when explicitly set in absbit, but
> > the internal interface always handles it. This is actually documented
> > in the code (and DocBook).
> 
> That note is good guidance for developer side.  Its also worth noting
> on app side that MT_TOOL_FINGER is a little special since its value is
> 0.  In most common case, I think it will get filtered out where as
> BTN_TOOL_FINGER will always be sent.
> 
> Since we can't yet query per slot ABS_MT_TOOL_TYPE, I guess apps have
> to just assume its a finger unless told otherwise?

Yes - but this is actually done for every ABS value already, so it
just follows the standard transfer method.

Thanks,
Henrik

Re: [PATCH 3/3] Input: synaptics - ignore bogus mt packet

[Dmitry Torokhov]

On Tue, Dec 21, 2010 at 11:40:21AM -0500, Chase Douglas wrote:
> On 12/20/2010 08:39 AM, Henrik Rydberg wrote:
> > In multitouch mode, at least one device (fw: 7.4 id: 0x1c0b1) sometimes
> > sends a final main packet with x == 1. Since the normal values are above
> > 1472, this is clearly bogus. At the same time, a two-finger touch is
> > signaled, even though only one finger was on the pad to begin with. This
> > patch ignores the packet altogether, removing the problem.
> > 
> > Acked-by: Chris Bagwell <chris@cnpbagwell.com>
> > Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> 
> Acked-by: Chase Douglas <chase.douglas@canonical.com>

Looks good to me as well.

Acked-by: Dmitry Torokhov <dtor@mail.ru>

-- 
Dmitry