All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] touch tablet support for the w8001 serial driver
@ 2010-08-20  4:55 Peter Hutterer
  2010-08-20  4:55   ` Peter Hutterer
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Peter Hutterer @ 2010-08-20  4:55 UTC (permalink / raw)
  To: linux-input, linux-kernel, pinglinux, rydberg


These patches add support for newer tablets to the wacom w8001 serial
driver. The first patch simply adds the support for the Pen/Eraser tool and
the stylus button.

The second patch detects touch-capable tablets and discard their packets.
Touch packets are interleaved with pen packets and their length depends on
the sensor model. Detect these packets and discard them silently.

The third patch adds MT slot support for two-finger touch devices.

Patches tested on a 2 finger touch device but i've had positive confirmation
that 1 finger devices don't break.

Cheers,
  Peter

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/3] input: send BTN_TOOL_PEN/RUBBER and BTN_STYLUS for the w8001.
  2010-08-20  4:55 [PATCH 0/3] touch tablet support for the w8001 serial driver Peter Hutterer
@ 2010-08-20  4:55   ` Peter Hutterer
  2010-08-20  4:55 ` [PATCH 2/3] input: support (and ignore) touch tablets in " Peter Hutterer
  2010-08-20  4:55 ` [PATCH 3/3] input: add multitouch slot support to w8001 Peter Hutterer
  2 siblings, 0 replies; 11+ messages in thread
From: Peter Hutterer @ 2010-08-20  4:55 UTC (permalink / raw)
  To: linux-input, linux-kernel, pinglinux, rydberg; +Cc: Peter Hutterer

The protocol used by the w8001 supports status fields for tip, side
switch and eraser as well as a RDY field for proximity.

The protocol has a double usage for the f2 bit in the packet. If set,
the data is either pen + side2 button or eraser. Assume eraser if the
device comes into proximity with the f2 bit set, otherwise trigger the
side2 button.
If the device comes into proximity with the f2 bit and that bit
disappears afterwards, fake proximity out for the eraser and proximity
in for the pen.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
---
 drivers/input/touchscreen/wacom_w8001.c |   51 +++++++++++++++++++++++++++----
 1 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
index 56dc35c..0e14ae1 100644
--- a/drivers/input/touchscreen/wacom_w8001.c
+++ b/drivers/input/touchscreen/wacom_w8001.c
@@ -62,6 +62,7 @@ struct w8001 {
 	unsigned char response[W8001_MAX_LENGTH];
 	unsigned char data[W8001_MAX_LENGTH];
 	char phys[32];
+	int type;
 };
 
 static void parse_data(u8 *data, struct w8001_coord *coord)
@@ -88,11 +89,49 @@ static void parse_data(u8 *data, struct w8001_coord *coord)
 	coord->tilt_y = data[8] & 0x7F;
 }
 
+static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
+{
+	struct input_dev *dev = w8001->dev;
+
+	/* We have 1 bit for proximity (rdy) and 3 bits for tip, side,
+	 * side2/eraser. If rdy && f2 are set, this can be either pen + side2,
+	 * or eraser. assume
+	 * - if dev is already in proximity and f2 is toggled → pen + side2
+	 * - if dev comes into proximity with f2 set → eraser
+	 * If f2 disappears after assuming eraser, fake proximity out for
+	 * eraser and in for pen.
+	 */
+
+	if (!w8001->type) {
+		w8001->type = coord->f2 ? BTN_TOOL_RUBBER : BTN_TOOL_PEN;
+	} else if (w8001->type == BTN_TOOL_RUBBER) {
+		if (!coord->f2) {
+			input_report_abs(dev, ABS_PRESSURE, 0);
+			input_report_key(dev, BTN_TOUCH, 0);
+			input_report_key(dev, BTN_STYLUS, 0);
+			input_report_key(dev, BTN_STYLUS2, 0);
+			input_report_key(dev, BTN_TOOL_RUBBER, 0);
+			input_sync(dev);
+			w8001->type = BTN_TOOL_PEN;
+		}
+	} else
+		input_report_key(dev, BTN_STYLUS2, coord->f2);
+
+	input_report_abs(dev, ABS_X, coord->x);
+	input_report_abs(dev, ABS_Y, coord->y);
+	input_report_abs(dev, ABS_PRESSURE, coord->pen_pressure);
+	input_report_key(dev, BTN_TOUCH, coord->tsw);
+	input_report_key(dev, BTN_STYLUS, coord->f1);
+	input_report_key(dev, w8001->type, coord->rdy);
+	input_sync(dev);
+	if (!coord->rdy)
+		w8001->type = 0;
+}
+
 static irqreturn_t w8001_interrupt(struct serio *serio,
 				   unsigned char data, unsigned int flags)
 {
 	struct w8001 *w8001 = serio_get_drvdata(serio);
-	struct input_dev *dev = w8001->dev;
 	struct w8001_coord coord;
 	unsigned char tmp;
 
@@ -112,11 +151,7 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
 
 		w8001->idx = 0;
 		parse_data(w8001->data, &coord);
-		input_report_abs(dev, ABS_X, coord.x);
-		input_report_abs(dev, ABS_Y, coord.y);
-		input_report_abs(dev, ABS_PRESSURE, coord.pen_pressure);
-		input_report_key(dev, BTN_TOUCH, coord.tsw);
-		input_sync(dev);
+		report_pen_events(w8001, &coord);
 		break;
 
 	case 10:
@@ -221,6 +256,10 @@ static int w8001_connect(struct serio *serio, struct serio_driver *drv)
 
 	input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
 	input_dev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
+	input_dev->keybit[BIT_WORD(BTN_TOOL_PEN)] |= BIT_MASK(BTN_TOOL_PEN);
+	input_dev->keybit[BIT_WORD(BTN_TOOL_RUBBER)] |= BIT_MASK(BTN_TOOL_RUBBER);
+	input_dev->keybit[BIT_WORD(BTN_STYLUS)] |= BIT_MASK(BTN_STYLUS);
+	input_dev->keybit[BIT_WORD(BTN_STYLUS2)] |= BIT_MASK(BTN_STYLUS2);
 
 	serio_set_drvdata(serio, w8001);
 	err = serio_open(serio, drv);
-- 
1.7.2.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 1/3] input: send BTN_TOOL_PEN/RUBBER and BTN_STYLUS for the w8001.
@ 2010-08-20  4:55   ` Peter Hutterer
  0 siblings, 0 replies; 11+ messages in thread
From: Peter Hutterer @ 2010-08-20  4:55 UTC (permalink / raw)
  To: linux-input, linux-kernel, pinglinux, rydberg; +Cc: Peter Hutterer

The protocol used by the w8001 supports status fields for tip, side
switch and eraser as well as a RDY field for proximity.

The protocol has a double usage for the f2 bit in the packet. If set,
the data is either pen + side2 button or eraser. Assume eraser if the
device comes into proximity with the f2 bit set, otherwise trigger the
side2 button.
If the device comes into proximity with the f2 bit and that bit
disappears afterwards, fake proximity out for the eraser and proximity
in for the pen.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
---
 drivers/input/touchscreen/wacom_w8001.c |   51 +++++++++++++++++++++++++++----
 1 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
index 56dc35c..0e14ae1 100644
--- a/drivers/input/touchscreen/wacom_w8001.c
+++ b/drivers/input/touchscreen/wacom_w8001.c
@@ -62,6 +62,7 @@ struct w8001 {
 	unsigned char response[W8001_MAX_LENGTH];
 	unsigned char data[W8001_MAX_LENGTH];
 	char phys[32];
+	int type;
 };
 
 static void parse_data(u8 *data, struct w8001_coord *coord)
@@ -88,11 +89,49 @@ static void parse_data(u8 *data, struct w8001_coord *coord)
 	coord->tilt_y = data[8] & 0x7F;
 }
 
+static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
+{
+	struct input_dev *dev = w8001->dev;
+
+	/* We have 1 bit for proximity (rdy) and 3 bits for tip, side,
+	 * side2/eraser. If rdy && f2 are set, this can be either pen + side2,
+	 * or eraser. assume
+	 * - if dev is already in proximity and f2 is toggled → pen + side2
+	 * - if dev comes into proximity with f2 set → eraser
+	 * If f2 disappears after assuming eraser, fake proximity out for
+	 * eraser and in for pen.
+	 */
+
+	if (!w8001->type) {
+		w8001->type = coord->f2 ? BTN_TOOL_RUBBER : BTN_TOOL_PEN;
+	} else if (w8001->type == BTN_TOOL_RUBBER) {
+		if (!coord->f2) {
+			input_report_abs(dev, ABS_PRESSURE, 0);
+			input_report_key(dev, BTN_TOUCH, 0);
+			input_report_key(dev, BTN_STYLUS, 0);
+			input_report_key(dev, BTN_STYLUS2, 0);
+			input_report_key(dev, BTN_TOOL_RUBBER, 0);
+			input_sync(dev);
+			w8001->type = BTN_TOOL_PEN;
+		}
+	} else
+		input_report_key(dev, BTN_STYLUS2, coord->f2);
+
+	input_report_abs(dev, ABS_X, coord->x);
+	input_report_abs(dev, ABS_Y, coord->y);
+	input_report_abs(dev, ABS_PRESSURE, coord->pen_pressure);
+	input_report_key(dev, BTN_TOUCH, coord->tsw);
+	input_report_key(dev, BTN_STYLUS, coord->f1);
+	input_report_key(dev, w8001->type, coord->rdy);
+	input_sync(dev);
+	if (!coord->rdy)
+		w8001->type = 0;
+}
+
 static irqreturn_t w8001_interrupt(struct serio *serio,
 				   unsigned char data, unsigned int flags)
 {
 	struct w8001 *w8001 = serio_get_drvdata(serio);
-	struct input_dev *dev = w8001->dev;
 	struct w8001_coord coord;
 	unsigned char tmp;
 
@@ -112,11 +151,7 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
 
 		w8001->idx = 0;
 		parse_data(w8001->data, &coord);
-		input_report_abs(dev, ABS_X, coord.x);
-		input_report_abs(dev, ABS_Y, coord.y);
-		input_report_abs(dev, ABS_PRESSURE, coord.pen_pressure);
-		input_report_key(dev, BTN_TOUCH, coord.tsw);
-		input_sync(dev);
+		report_pen_events(w8001, &coord);
 		break;
 
 	case 10:
@@ -221,6 +256,10 @@ static int w8001_connect(struct serio *serio, struct serio_driver *drv)
 
 	input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
 	input_dev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
+	input_dev->keybit[BIT_WORD(BTN_TOOL_PEN)] |= BIT_MASK(BTN_TOOL_PEN);
+	input_dev->keybit[BIT_WORD(BTN_TOOL_RUBBER)] |= BIT_MASK(BTN_TOOL_RUBBER);
+	input_dev->keybit[BIT_WORD(BTN_STYLUS)] |= BIT_MASK(BTN_STYLUS);
+	input_dev->keybit[BIT_WORD(BTN_STYLUS2)] |= BIT_MASK(BTN_STYLUS2);
 
 	serio_set_drvdata(serio, w8001);
 	err = serio_open(serio, drv);
-- 
1.7.2.1

--
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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/3] input: support (and ignore) touch tablets in the w8001.
  2010-08-20  4:55 [PATCH 0/3] touch tablet support for the w8001 serial driver Peter Hutterer
  2010-08-20  4:55   ` Peter Hutterer
@ 2010-08-20  4:55 ` Peter Hutterer
  2010-08-20  4:55 ` [PATCH 3/3] input: add multitouch slot support to w8001 Peter Hutterer
  2 siblings, 0 replies; 11+ messages in thread
From: Peter Hutterer @ 2010-08-20  4:55 UTC (permalink / raw)
  To: linux-input, linux-kernel, pinglinux, rydberg; +Cc: Peter Hutterer

Tablets that support touch input may report different sized packages,
depending on the touch sensor in the tablet. For now, discard the
packages until we report them as touch input proper.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
---
 drivers/input/touchscreen/wacom_w8001.c |   88 ++++++++++++++++++++++++++++++-
 1 files changed, 86 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
index 0e14ae1..c302cc3 100644
--- a/drivers/input/touchscreen/wacom_w8001.c
+++ b/drivers/input/touchscreen/wacom_w8001.c
@@ -2,6 +2,7 @@
  * Wacom W8001 penabled serial touchscreen driver
  *
  * Copyright (c) 2008 Jaya Kumar
+ * Copyright (c) 2010 Red Hat, Inc.
  *
  * This file is subject to the terms and conditions of the GNU General Public
  * License. See the file COPYING in the main directory of this archive for
@@ -30,11 +31,22 @@ MODULE_LICENSE("GPL");
 #define W8001_LEAD_BYTE		0x80
 #define W8001_TAB_MASK		0x40
 #define W8001_TAB_BYTE		0x40
+/* set in first byte of touch data packets */
+#define W8001_TOUCH_MASK	(0x10 | W8001_LEAD_MASK)
+#define W8001_TOUCH_BYTE	(0x10 | W8001_LEAD_BYTE)
 
 #define W8001_QUERY_PACKET	0x20
 
 #define W8001_CMD_START		'1'
 #define W8001_CMD_QUERY		'*'
+#define W8001_CMD_TOUCHQUERY	'%'
+
+/* length of data packets in bytes, depends on device. */
+#define W8001_PKTLEN_TOUCH93	5
+#define W8001_PKTLEN_TOUCH9A	7
+#define W8001_PKTLEN_TPCPEN	9
+#define W8001_PKTLEN_TPCCTL	11	/* control packet */
+#define W8001_PKTLEN_TOUCH2FG	13
 
 struct w8001_coord {
 	u8 rdy;
@@ -48,6 +60,15 @@ struct w8001_coord {
 	u8 tilt_y;
 };
 
+/* touch query reply packet */
+struct w8001_touch_query {
+	u8 panel_res;
+	u8 capacity_res;
+	u8 sensor_id;
+	u16 x;
+	u16 y;
+};
+
 /*
  * Per-touchscreen data.
  */
@@ -63,6 +84,7 @@ struct w8001 {
 	unsigned char data[W8001_MAX_LENGTH];
 	char phys[32];
 	int type;
+	unsigned int pktlen;
 };
 
 static void parse_data(u8 *data, struct w8001_coord *coord)
@@ -89,6 +111,23 @@ static void parse_data(u8 *data, struct w8001_coord *coord)
 	coord->tilt_y = data[8] & 0x7F;
 }
 
+static void parse_touchquery(u8 *data, struct w8001_touch_query *query)
+{
+	memset(query, 0, sizeof(*query));
+
+	query->panel_res = data[1];
+	query->sensor_id = data[2] & 0x7;
+	query->capacity_res = data[7];
+
+	query->x = data[3] << 9;
+	query->x |= data[4] << 2;
+	query->x |= (data[2] >> 5) & 0x3;
+
+	query->y = data[5] << 9;
+	query->y |= data[6] << 2;
+	query->y |= (data[2] >> 3) & 0x3;
+}
+
 static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
 {
 	struct input_dev *dev = w8001->dev;
@@ -144,22 +183,45 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
 		}
 		break;
 
-	case 8:
+	case W8001_PKTLEN_TOUCH93 - 1:
+	case W8001_PKTLEN_TOUCH9A - 1:
+		/* ignore one-finger touch packet. */
+		if (w8001->pktlen == w8001->idx)
+			w8001->idx = 0;
+		break;
+
+	/* Pen coordinates packet */
+	case W8001_PKTLEN_TPCPEN - 1:
 		tmp = w8001->data[0] & W8001_TAB_MASK;
 		if (unlikely(tmp == W8001_TAB_BYTE))
 			break;
 
+		tmp = (w8001->data[0] & W8001_TOUCH_BYTE);
+		if (tmp == W8001_TOUCH_BYTE)
+			break;
+
 		w8001->idx = 0;
 		parse_data(w8001->data, &coord);
 		report_pen_events(w8001, &coord);
 		break;
 
-	case 10:
+	/* control packet */
+	case W8001_PKTLEN_TPCCTL - 1:
+		tmp = (w8001->data[0] & W8001_TOUCH_MASK);
+		if (tmp == W8001_TOUCH_BYTE)
+			break;
+
 		w8001->idx = 0;
 		memcpy(w8001->response, w8001->data, W8001_MAX_LENGTH);
 		w8001->response_type = W8001_QUERY_PACKET;
 		complete(&w8001->cmd_done);
 		break;
+
+	case W8001_PKTLEN_TOUCH2FG - 1:
+		/* ignore two-finger touch packet. */
+		if (w8001->pktlen == w8001->idx)
+			w8001->idx = 0;
+		break;
 	}
 
 	return IRQ_HANDLED;
@@ -202,6 +264,28 @@ static int w8001_setup(struct w8001 *w8001)
 	input_set_abs_params(dev, ABS_TILT_X, 0, coord.tilt_x, 0, 0);
 	input_set_abs_params(dev, ABS_TILT_Y, 0, coord.tilt_y, 0, 0);
 
+	error = w8001_command(w8001, W8001_CMD_TOUCHQUERY, true);
+	if (!error) {
+		struct w8001_touch_query touch;
+
+		parse_touchquery(w8001->response, &touch);
+
+		switch (touch.sensor_id) {
+		case 0:
+		case 2:
+			w8001->pktlen = W8001_PKTLEN_TOUCH93;
+			break;
+		case 1:
+		case 3:
+		case 4:
+			w8001->pktlen = W8001_PKTLEN_TOUCH9A;
+			break;
+		case 5:
+			w8001->pktlen = W8001_PKTLEN_TOUCH2FG;
+			break;
+		}
+	}
+
 	return w8001_command(w8001, W8001_CMD_START, false);
 }
 
-- 
1.7.2.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] input: add multitouch slot support to w8001.
  2010-08-20  4:55 [PATCH 0/3] touch tablet support for the w8001 serial driver Peter Hutterer
  2010-08-20  4:55   ` Peter Hutterer
  2010-08-20  4:55 ` [PATCH 2/3] input: support (and ignore) touch tablets in " Peter Hutterer
@ 2010-08-20  4:55 ` Peter Hutterer
  2010-08-23  8:18   ` Henrik Rydberg
  2010-08-26  2:01   ` [PATCH v2] " Peter Hutterer
  2 siblings, 2 replies; 11+ messages in thread
From: Peter Hutterer @ 2010-08-20  4:55 UTC (permalink / raw)
  To: linux-input, linux-kernel, pinglinux, rydberg; +Cc: Peter Hutterer

Some serial wacom devices support two-finger touch. Test for this during
init and parse the touch packets accordingly. Touch packets are
processed using Protocol B (MT Slots).

Note: there are several wacom versions that do touch but not two-finger
touch. These are not catered for here, touch events for these are simply
discarded.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
CC: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/input/touchscreen/wacom_w8001.c |   99 ++++++++++++++++++++++++++++++-
 1 files changed, 96 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
index c302cc3..a38a3aa 100644
--- a/drivers/input/touchscreen/wacom_w8001.c
+++ b/drivers/input/touchscreen/wacom_w8001.c
@@ -60,6 +60,17 @@ struct w8001_coord {
 	u8 tilt_y;
 };
 
+/* touch data packet */
+struct w8001_touch {
+	u8 f1_status;
+	u16 f1_x;
+	u16 f1_y;
+	/* only some tablets have 2FG info */
+	u8 f2_status;
+	u16 f2_x;
+	u16 f2_y;
+};
+
 /* touch query reply packet */
 struct w8001_touch_query {
 	u8 panel_res;
@@ -85,8 +96,18 @@ struct w8001 {
 	char phys[32];
 	int type;
 	unsigned int pktlen;
+	unsigned char tracking_id[2];
 };
 
+static int get_next_tracking_id(void)
+{
+	static unsigned char next_tracking_id;
+	next_tracking_id = (next_tracking_id + 1) % 256;
+	if (next_tracking_id == 0)
+		next_tracking_id = 1;
+	return next_tracking_id;
+}
+
 static void parse_data(u8 *data, struct w8001_coord *coord)
 {
 	memset(coord, 0, sizeof(*coord));
@@ -111,6 +132,26 @@ static void parse_data(u8 *data, struct w8001_coord *coord)
 	coord->tilt_y = data[8] & 0x7F;
 }
 
+static void parse_touch(u8 *data,
+			unsigned int pktlen, struct w8001_touch *touch)
+{
+	memset(touch, 0, sizeof(*touch));
+
+	touch->f1_status = data[0] & 0x1;
+	touch->f1_x = data[1] << 7;
+	touch->f1_x |= data[2];
+	touch->f1_y = data[3] << 7;
+	touch->f1_y |= data[4];
+
+	if (pktlen >= W8001_PKTLEN_TOUCH2FG) {
+		touch->f2_status = (data[0] & 0x2) >> 1;
+		touch->f2_x = data[7] << 7;
+		touch->f2_x |= data[8];
+		touch->f2_y = data[9] << 7;
+		touch->f2_y |= data[10];
+	}
+}
+
 static void parse_touchquery(u8 *data, struct w8001_touch_query *query)
 {
 	memset(query, 0, sizeof(*query));
@@ -128,6 +169,46 @@ static void parse_touchquery(u8 *data, struct w8001_touch_query *query)
 	query->y |= (data[2] >> 3) & 0x3;
 }
 
+static void w8001_mt_event(struct input_dev *dev,
+			   int slot, int tid, int x, int y)
+{
+	input_mt_slot(dev, slot);
+	if (tid != 0) {
+		input_report_abs(dev, ABS_MT_POSITION_X, x);
+		input_report_abs(dev, ABS_MT_POSITION_Y, y);
+	}
+	input_report_abs(dev, ABS_MT_TRACKING_ID, tid);
+	input_report_abs(dev, ABS_MT_TOOL_TYPE, MT_TOOL_FINGER);
+	input_mt_sync(dev);
+}
+
+static void w8001_track_fingers(struct w8001 *w8001, struct w8001_touch *touch)
+{
+	struct input_dev *dev = w8001->dev;
+
+	if (touch->f1_status) {
+		if (!w8001->tracking_id[0])
+			w8001->tracking_id[0] = get_next_tracking_id();
+		w8001_mt_event(dev, 0, w8001->tracking_id[0],
+			       touch->f1_x, touch->f1_y);
+	} else if (w8001->tracking_id[0]) {
+		w8001->tracking_id[0] = 0;
+		w8001_mt_event(dev, 0, 0, touch->f1_x, touch->f1_y);
+	}
+
+	if (touch->f2_status) {
+		if (!w8001->tracking_id[1])
+			w8001->tracking_id[1] = get_next_tracking_id();
+		w8001_mt_event(dev, 1, w8001->tracking_id[1],
+			       touch->f2_x, touch->f2_y);
+	} else if (w8001->tracking_id[1]) {
+		w8001->tracking_id[1] = 0;
+		w8001_mt_event(dev, 1, 0, touch->f2_x, touch->f2_y);
+	}
+
+	input_sync(dev);
+}
+
 static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
 {
 	struct input_dev *dev = w8001->dev;
@@ -172,6 +253,7 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
 {
 	struct w8001 *w8001 = serio_get_drvdata(serio);
 	struct w8001_coord coord;
+	struct w8001_touch touch;
 	unsigned char tmp;
 
 	w8001->data[w8001->idx] = data;
@@ -217,10 +299,11 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
 		complete(&w8001->cmd_done);
 		break;
 
+	/* 2 finger touch packet */
 	case W8001_PKTLEN_TOUCH2FG - 1:
-		/* ignore two-finger touch packet. */
-		if (w8001->pktlen == w8001->idx)
-			w8001->idx = 0;
+		w8001->idx = 0;
+		parse_touch(w8001->data, w8001->pktlen, &touch);
+		w8001_track_fingers(w8001, &touch);
 		break;
 	}
 
@@ -282,6 +365,16 @@ static int w8001_setup(struct w8001 *w8001)
 			break;
 		case 5:
 			w8001->pktlen = W8001_PKTLEN_TOUCH2FG;
+
+			input_mt_create_slots(dev, 2);
+			input_set_abs_params(dev, ABS_MT_TRACKING_ID,
+						0, 255, 0, 0);
+			input_set_abs_params(dev, ABS_MT_POSITION_X,
+						0, touch.x, 0, 0);
+			input_set_abs_params(dev, ABS_MT_POSITION_Y,
+						0, touch.y, 0, 0);
+			input_set_abs_params(dev, ABS_MT_TOOL_TYPE,
+						0, 0, 0, 0);
 			break;
 		}
 	}
-- 
1.7.2.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] input: add multitouch slot support to w8001.
  2010-08-20  4:55 ` [PATCH 3/3] input: add multitouch slot support to w8001 Peter Hutterer
@ 2010-08-23  8:18   ` Henrik Rydberg
  2010-08-23 14:11       ` Ping Cheng
  2010-08-26  2:01   ` [PATCH v2] " Peter Hutterer
  1 sibling, 1 reply; 11+ messages in thread
From: Henrik Rydberg @ 2010-08-23  8:18 UTC (permalink / raw)
  To: Peter Hutterer; +Cc: linux-input, linux-kernel, pinglinux

Hi Peter,

thanks for the patches. Comments inline.

> Some serial wacom devices support two-finger touch. Test for this during

> init and parse the touch packets accordingly. Touch packets are
> processed using Protocol B (MT Slots).
> 
> Note: there are several wacom versions that do touch but not two-finger
> touch. These are not catered for here, touch events for these are simply
> discarded.
> 
> Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
> CC: Henrik Rydberg <rydberg@euromail.se>
> ---
>  drivers/input/touchscreen/wacom_w8001.c |   99 ++++++++++++++++++++++++++++++-
>  1 files changed, 96 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
> index c302cc3..a38a3aa 100644
> --- a/drivers/input/touchscreen/wacom_w8001.c
> +++ b/drivers/input/touchscreen/wacom_w8001.c
> @@ -60,6 +60,17 @@ struct w8001_coord {
>  	u8 tilt_y;
>  };
>  
> +/* touch data packet */
> +struct w8001_touch {
> +	u8 f1_status;
> +	u16 f1_x;
> +	u16 f1_y;
> +	/* only some tablets have 2FG info */
> +	u8 f2_status;
> +	u16 f2_x;
> +	u16 f2_y;
> +};


No pressure information from this device?

> +
>  /* touch query reply packet */
>  struct w8001_touch_query {
>  	u8 panel_res;
> @@ -85,8 +96,18 @@ struct w8001 {
>  	char phys[32];
>  	int type;
>  	unsigned int pktlen;
> +	unsigned char tracking_id[2];
>  };
>  
> +static int get_next_tracking_id(void)
> +{
> +	static unsigned char next_tracking_id;
> +	next_tracking_id = (next_tracking_id + 1) % 256;
> +	if (next_tracking_id == 0)
> +		next_tracking_id = 1;
> +	return next_tracking_id;
> +}


Zero is part of the valid range from 2.6.36. Can be implemented simply as
(tracking_id++ & 0xfff), see recent MT slots patches.

> +
>  static void parse_data(u8 *data, struct w8001_coord *coord)
>  {
>  	memset(coord, 0, sizeof(*coord));
> @@ -111,6 +132,26 @@ static void parse_data(u8 *data, struct w8001_coord *coord)
>  	coord->tilt_y = data[8] & 0x7F;
>  }
>  
> +static void parse_touch(u8 *data,
> +			unsigned int pktlen, struct w8001_touch *touch)
> +{
> +	memset(touch, 0, sizeof(*touch));
> +
> +	touch->f1_status = data[0] & 0x1;
> +	touch->f1_x = data[1] << 7;
> +	touch->f1_x |= data[2];
> +	touch->f1_y = data[3] << 7;
> +	touch->f1_y |= data[4];


What are data[5] and data[6]? Why is f1_x/y split into several lines?

> +
> +	if (pktlen >= W8001_PKTLEN_TOUCH2FG) {
> +		touch->f2_status = (data[0] & 0x2) >> 1;
> +		touch->f2_x = data[7] << 7;
> +		touch->f2_x |= data[8];
> +		touch->f2_y = data[9] << 7;
> +		touch->f2_y |= data[10];
> +	}


What are data[11] and data[12]?

Since all needed information is available here, there seems to be little reason
for the w8001_touch structure. Why not complete the report in this function?

> +}
> +
>  static void parse_touchquery(u8 *data, struct w8001_touch_query *query)
>  {
>  	memset(query, 0, sizeof(*query));
> @@ -128,6 +169,46 @@ static void parse_touchquery(u8 *data, struct w8001_touch_query *query)
>  	query->y |= (data[2] >> 3) & 0x3;
>  }
>  
> +static void w8001_mt_event(struct input_dev *dev,
> +			   int slot, int tid, int x, int y)
> +{
> +	input_mt_slot(dev, slot);
> +	if (tid != 0) {
> +		input_report_abs(dev, ABS_MT_POSITION_X, x);
> +		input_report_abs(dev, ABS_MT_POSITION_Y, y);
> +	}
> +	input_report_abs(dev, ABS_MT_TRACKING_ID, tid);
> +	input_report_abs(dev, ABS_MT_TOOL_TYPE, MT_TOOL_FINGER);
> +	input_mt_sync(dev);
> +}

> +
> +static void w8001_track_fingers(struct w8001 *w8001, struct w8001_touch *touch)
> +{
> +	struct input_dev *dev = w8001->dev;
> +
> +	if (touch->f1_status) {
> +		if (!w8001->tracking_id[0])
> +			w8001->tracking_id[0] = get_next_tracking_id();
> +		w8001_mt_event(dev, 0, w8001->tracking_id[0],
> +			       touch->f1_x, touch->f1_y);
> +	} else if (w8001->tracking_id[0]) {
> +		w8001->tracking_id[0] = 0;
> +		w8001_mt_event(dev, 0, 0, touch->f1_x, touch->f1_y);
> +	}

> +
> +	if (touch->f2_status) {
> +		if (!w8001->tracking_id[1])
> +			w8001->tracking_id[1] = get_next_tracking_id();
> +		w8001_mt_event(dev, 1, w8001->tracking_id[1],
> +			       touch->f2_x, touch->f2_y);
> +	} else if (w8001->tracking_id[1]) {
> +		w8001->tracking_id[1] = 0;
> +		w8001_mt_event(dev, 1, 0, touch->f2_x, touch->f2_y);
> +	}
> +
> +	input_sync(dev);
> +}


Apparently, f1 and f2 represent the slots themselves, i.e., the device uses
slots internally. Please simplify the logic accordingly. There is an example in
the recent patch for the Bamboo Touch.

> +
>  static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
>  {
>  	struct input_dev *dev = w8001->dev;
> @@ -172,6 +253,7 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
>  {
>  	struct w8001 *w8001 = serio_get_drvdata(serio);
>  	struct w8001_coord coord;
> +	struct w8001_touch touch;
>  	unsigned char tmp;
>  
>  	w8001->data[w8001->idx] = data;
> @@ -217,10 +299,11 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
>  		complete(&w8001->cmd_done);
>  		break;
>  
> +	/* 2 finger touch packet */
>  	case W8001_PKTLEN_TOUCH2FG - 1:
> -		/* ignore two-finger touch packet. */
> -		if (w8001->pktlen == w8001->idx)
> -			w8001->idx = 0;
> +		w8001->idx = 0;
> +		parse_touch(w8001->data, w8001->pktlen, &touch);
> +		w8001_track_fingers(w8001, &touch);
>  		break;
>  	}
>  
> @@ -282,6 +365,16 @@ static int w8001_setup(struct w8001 *w8001)
>  			break;
>  		case 5:
>  			w8001->pktlen = W8001_PKTLEN_TOUCH2FG;
> +
> +			input_mt_create_slots(dev, 2);
> +			input_set_abs_params(dev, ABS_MT_TRACKING_ID,
> +						0, 255, 0, 0);
> +			input_set_abs_params(dev, ABS_MT_POSITION_X,
> +						0, touch.x, 0, 0);
> +			input_set_abs_params(dev, ABS_MT_POSITION_Y,
> +						0, touch.y, 0, 0);
> +			input_set_abs_params(dev, ABS_MT_TOOL_TYPE,
> +						0, 0, 0, 0);
>  			break;
>  		}
>  	}


No information about signal-to-noise ratio (fuzz) for this device?

Thanks,
Henrik

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] input: add multitouch slot support to w8001.
  2010-08-23  8:18   ` Henrik Rydberg
@ 2010-08-23 14:11       ` Ping Cheng
  0 siblings, 0 replies; 11+ messages in thread
From: Ping Cheng @ 2010-08-23 14:11 UTC (permalink / raw)
  To: Henrik Rydberg; +Cc: Peter Hutterer, linux-input, linux-kernel

On Mon, Aug 23, 2010 at 10:18 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
> Hi Peter,
>
> thanks for the patches. Comments inline.
>
>> Some serial wacom devices support two-finger touch. Test for this during
>
>> init and parse the touch packets accordingly. Touch packets are
>> processed using Protocol B (MT Slots).
>>
>> Note: there are several wacom versions that do touch but not two-finger
>> touch. These are not catered for here, touch events for these are simply
>> discarded.
>>
>> Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
>> CC: Henrik Rydberg <rydberg@euromail.se>
>> ---
>>  drivers/input/touchscreen/wacom_w8001.c |   99 ++++++++++++++++++++++++++++++-
>>  1 files changed, 96 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
>> index c302cc3..a38a3aa 100644
>> --- a/drivers/input/touchscreen/wacom_w8001.c
>> +++ b/drivers/input/touchscreen/wacom_w8001.c
>> @@ -60,6 +60,17 @@ struct w8001_coord {
>>       u8 tilt_y;
>>  };
>>
>> +/* touch data packet */
>> +struct w8001_touch {
>> +     u8 f1_status;
>> +     u16 f1_x;
>> +     u16 f1_y;
>> +     /* only some tablets have 2FG info */
>> +     u8 f2_status;
>> +     u16 f2_x;
>> +     u16 f2_y;
>> +};
>
>
> No pressure information from this device?

No useful pressure/capacity available for this device.

>
>> +
>>  /* touch query reply packet */
>>  struct w8001_touch_query {
>>       u8 panel_res;
>> @@ -85,8 +96,18 @@ struct w8001 {
>>       char phys[32];
>>       int type;
>>       unsigned int pktlen;
>> +     unsigned char tracking_id[2];
>>  };
>>
>> +static int get_next_tracking_id(void)
>> +{
>> +     static unsigned char next_tracking_id;
>> +     next_tracking_id = (next_tracking_id + 1) % 256;
>> +     if (next_tracking_id == 0)
>> +             next_tracking_id = 1;
>> +     return next_tracking_id;
>> +}
>
>
> Zero is part of the valid range from 2.6.36. Can be implemented simply as
> (tracking_id++ & 0xfff), see recent MT slots patches.
>
>> +
>>  static void parse_data(u8 *data, struct w8001_coord *coord)
>>  {
>>       memset(coord, 0, sizeof(*coord));
>> @@ -111,6 +132,26 @@ static void parse_data(u8 *data, struct w8001_coord *coord)
>>       coord->tilt_y = data[8] & 0x7F;
>>  }
>>
>> +static void parse_touch(u8 *data,
>> +                     unsigned int pktlen, struct w8001_touch *touch)
>> +{
>> +     memset(touch, 0, sizeof(*touch));
>> +
>> +     touch->f1_status = data[0] & 0x1;
>> +     touch->f1_x = data[1] << 7;
>> +     touch->f1_x |= data[2];
>> +     touch->f1_y = data[3] << 7;
>> +     touch->f1_y |= data[4];
>
>
> What are data[5] and data[6]? Why is f1_x/y split into several lines?
>
>> +
>> +     if (pktlen >= W8001_PKTLEN_TOUCH2FG) {
>> +             touch->f2_status = (data[0] & 0x2) >> 1;
>> +             touch->f2_x = data[7] << 7;
>> +             touch->f2_x |= data[8];
>> +             touch->f2_y = data[9] << 7;
>> +             touch->f2_y |= data[10];
>> +     }
>
>
> What are data[11] and data[12]?
>
> Since all needed information is available here, there seems to be little reason
> for the w8001_touch structure. Why not complete the report in this function?
>
>> +}
>> +
>>  static void parse_touchquery(u8 *data, struct w8001_touch_query *query)
>>  {
>>       memset(query, 0, sizeof(*query));
>> @@ -128,6 +169,46 @@ static void parse_touchquery(u8 *data, struct w8001_touch_query *query)
>>       query->y |= (data[2] >> 3) & 0x3;
>>  }
>>
>> +static void w8001_mt_event(struct input_dev *dev,
>> +                        int slot, int tid, int x, int y)
>> +{
>> +     input_mt_slot(dev, slot);
>> +     if (tid != 0) {
>> +             input_report_abs(dev, ABS_MT_POSITION_X, x);
>> +             input_report_abs(dev, ABS_MT_POSITION_Y, y);
>> +     }
>> +     input_report_abs(dev, ABS_MT_TRACKING_ID, tid);
>> +     input_report_abs(dev, ABS_MT_TOOL_TYPE, MT_TOOL_FINGER);
>> +     input_mt_sync(dev);
>> +}
>
>> +
>> +static void w8001_track_fingers(struct w8001 *w8001, struct w8001_touch *touch)
>> +{
>> +     struct input_dev *dev = w8001->dev;
>> +
>> +     if (touch->f1_status) {
>> +             if (!w8001->tracking_id[0])
>> +                     w8001->tracking_id[0] = get_next_tracking_id();
>> +             w8001_mt_event(dev, 0, w8001->tracking_id[0],
>> +                            touch->f1_x, touch->f1_y);
>> +     } else if (w8001->tracking_id[0]) {
>> +             w8001->tracking_id[0] = 0;
>> +             w8001_mt_event(dev, 0, 0, touch->f1_x, touch->f1_y);
>> +     }
>
>> +
>> +     if (touch->f2_status) {
>> +             if (!w8001->tracking_id[1])
>> +                     w8001->tracking_id[1] = get_next_tracking_id();
>> +             w8001_mt_event(dev, 1, w8001->tracking_id[1],
>> +                            touch->f2_x, touch->f2_y);
>> +     } else if (w8001->tracking_id[1]) {
>> +             w8001->tracking_id[1] = 0;
>> +             w8001_mt_event(dev, 1, 0, touch->f2_x, touch->f2_y);
>> +     }
>> +
>> +     input_sync(dev);
>> +}
>
>
> Apparently, f1 and f2 represent the slots themselves, i.e., the device uses
> slots internally. Please simplify the logic accordingly. There is an example in
> the recent patch for the Bamboo Touch.
>
>> +
>>  static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
>>  {
>>       struct input_dev *dev = w8001->dev;
>> @@ -172,6 +253,7 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
>>  {
>>       struct w8001 *w8001 = serio_get_drvdata(serio);
>>       struct w8001_coord coord;
>> +     struct w8001_touch touch;
>>       unsigned char tmp;
>>
>>       w8001->data[w8001->idx] = data;
>> @@ -217,10 +299,11 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
>>               complete(&w8001->cmd_done);
>>               break;
>>
>> +     /* 2 finger touch packet */
>>       case W8001_PKTLEN_TOUCH2FG - 1:
>> -             /* ignore two-finger touch packet. */
>> -             if (w8001->pktlen == w8001->idx)
>> -                     w8001->idx = 0;
>> +             w8001->idx = 0;
>> +             parse_touch(w8001->data, w8001->pktlen, &touch);
>> +             w8001_track_fingers(w8001, &touch);
>>               break;
>>       }
>>
>> @@ -282,6 +365,16 @@ static int w8001_setup(struct w8001 *w8001)
>>                       break;
>>               case 5:
>>                       w8001->pktlen = W8001_PKTLEN_TOUCH2FG;
>> +
>> +                     input_mt_create_slots(dev, 2);
>> +                     input_set_abs_params(dev, ABS_MT_TRACKING_ID,
>> +                                             0, 255, 0, 0);
>> +                     input_set_abs_params(dev, ABS_MT_POSITION_X,
>> +                                             0, touch.x, 0, 0);
>> +                     input_set_abs_params(dev, ABS_MT_POSITION_Y,
>> +                                             0, touch.y, 0, 0);
>> +                     input_set_abs_params(dev, ABS_MT_TOOL_TYPE,
>> +                                             0, 0, 0, 0);
>>                       break;
>>               }
>>       }
>
>
> No information about signal-to-noise ratio (fuzz) for this device?

No.

Ping

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] input: add multitouch slot support to w8001.
@ 2010-08-23 14:11       ` Ping Cheng
  0 siblings, 0 replies; 11+ messages in thread
From: Ping Cheng @ 2010-08-23 14:11 UTC (permalink / raw)
  To: Henrik Rydberg; +Cc: Peter Hutterer, linux-input, linux-kernel

On Mon, Aug 23, 2010 at 10:18 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
> Hi Peter,
>
> thanks for the patches. Comments inline.
>
>> Some serial wacom devices support two-finger touch. Test for this during
>
>> init and parse the touch packets accordingly. Touch packets are
>> processed using Protocol B (MT Slots).
>>
>> Note: there are several wacom versions that do touch but not two-finger
>> touch. These are not catered for here, touch events for these are simply
>> discarded.
>>
>> Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
>> CC: Henrik Rydberg <rydberg@euromail.se>
>> ---
>>  drivers/input/touchscreen/wacom_w8001.c |   99 ++++++++++++++++++++++++++++++-
>>  1 files changed, 96 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
>> index c302cc3..a38a3aa 100644
>> --- a/drivers/input/touchscreen/wacom_w8001.c
>> +++ b/drivers/input/touchscreen/wacom_w8001.c
>> @@ -60,6 +60,17 @@ struct w8001_coord {
>>       u8 tilt_y;
>>  };
>>
>> +/* touch data packet */
>> +struct w8001_touch {
>> +     u8 f1_status;
>> +     u16 f1_x;
>> +     u16 f1_y;
>> +     /* only some tablets have 2FG info */
>> +     u8 f2_status;
>> +     u16 f2_x;
>> +     u16 f2_y;
>> +};
>
>
> No pressure information from this device?

No useful pressure/capacity available for this device.

>
>> +
>>  /* touch query reply packet */
>>  struct w8001_touch_query {
>>       u8 panel_res;
>> @@ -85,8 +96,18 @@ struct w8001 {
>>       char phys[32];
>>       int type;
>>       unsigned int pktlen;
>> +     unsigned char tracking_id[2];
>>  };
>>
>> +static int get_next_tracking_id(void)
>> +{
>> +     static unsigned char next_tracking_id;
>> +     next_tracking_id = (next_tracking_id + 1) % 256;
>> +     if (next_tracking_id == 0)
>> +             next_tracking_id = 1;
>> +     return next_tracking_id;
>> +}
>
>
> Zero is part of the valid range from 2.6.36. Can be implemented simply as
> (tracking_id++ & 0xfff), see recent MT slots patches.
>
>> +
>>  static void parse_data(u8 *data, struct w8001_coord *coord)
>>  {
>>       memset(coord, 0, sizeof(*coord));
>> @@ -111,6 +132,26 @@ static void parse_data(u8 *data, struct w8001_coord *coord)
>>       coord->tilt_y = data[8] & 0x7F;
>>  }
>>
>> +static void parse_touch(u8 *data,
>> +                     unsigned int pktlen, struct w8001_touch *touch)
>> +{
>> +     memset(touch, 0, sizeof(*touch));
>> +
>> +     touch->f1_status = data[0] & 0x1;
>> +     touch->f1_x = data[1] << 7;
>> +     touch->f1_x |= data[2];
>> +     touch->f1_y = data[3] << 7;
>> +     touch->f1_y |= data[4];
>
>
> What are data[5] and data[6]? Why is f1_x/y split into several lines?
>
>> +
>> +     if (pktlen >= W8001_PKTLEN_TOUCH2FG) {
>> +             touch->f2_status = (data[0] & 0x2) >> 1;
>> +             touch->f2_x = data[7] << 7;
>> +             touch->f2_x |= data[8];
>> +             touch->f2_y = data[9] << 7;
>> +             touch->f2_y |= data[10];
>> +     }
>
>
> What are data[11] and data[12]?
>
> Since all needed information is available here, there seems to be little reason
> for the w8001_touch structure. Why not complete the report in this function?
>
>> +}
>> +
>>  static void parse_touchquery(u8 *data, struct w8001_touch_query *query)
>>  {
>>       memset(query, 0, sizeof(*query));
>> @@ -128,6 +169,46 @@ static void parse_touchquery(u8 *data, struct w8001_touch_query *query)
>>       query->y |= (data[2] >> 3) & 0x3;
>>  }
>>
>> +static void w8001_mt_event(struct input_dev *dev,
>> +                        int slot, int tid, int x, int y)
>> +{
>> +     input_mt_slot(dev, slot);
>> +     if (tid != 0) {
>> +             input_report_abs(dev, ABS_MT_POSITION_X, x);
>> +             input_report_abs(dev, ABS_MT_POSITION_Y, y);
>> +     }
>> +     input_report_abs(dev, ABS_MT_TRACKING_ID, tid);
>> +     input_report_abs(dev, ABS_MT_TOOL_TYPE, MT_TOOL_FINGER);
>> +     input_mt_sync(dev);
>> +}
>
>> +
>> +static void w8001_track_fingers(struct w8001 *w8001, struct w8001_touch *touch)
>> +{
>> +     struct input_dev *dev = w8001->dev;
>> +
>> +     if (touch->f1_status) {
>> +             if (!w8001->tracking_id[0])
>> +                     w8001->tracking_id[0] = get_next_tracking_id();
>> +             w8001_mt_event(dev, 0, w8001->tracking_id[0],
>> +                            touch->f1_x, touch->f1_y);
>> +     } else if (w8001->tracking_id[0]) {
>> +             w8001->tracking_id[0] = 0;
>> +             w8001_mt_event(dev, 0, 0, touch->f1_x, touch->f1_y);
>> +     }
>
>> +
>> +     if (touch->f2_status) {
>> +             if (!w8001->tracking_id[1])
>> +                     w8001->tracking_id[1] = get_next_tracking_id();
>> +             w8001_mt_event(dev, 1, w8001->tracking_id[1],
>> +                            touch->f2_x, touch->f2_y);
>> +     } else if (w8001->tracking_id[1]) {
>> +             w8001->tracking_id[1] = 0;
>> +             w8001_mt_event(dev, 1, 0, touch->f2_x, touch->f2_y);
>> +     }
>> +
>> +     input_sync(dev);
>> +}
>
>
> Apparently, f1 and f2 represent the slots themselves, i.e., the device uses
> slots internally. Please simplify the logic accordingly. There is an example in
> the recent patch for the Bamboo Touch.
>
>> +
>>  static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
>>  {
>>       struct input_dev *dev = w8001->dev;
>> @@ -172,6 +253,7 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
>>  {
>>       struct w8001 *w8001 = serio_get_drvdata(serio);
>>       struct w8001_coord coord;
>> +     struct w8001_touch touch;
>>       unsigned char tmp;
>>
>>       w8001->data[w8001->idx] = data;
>> @@ -217,10 +299,11 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
>>               complete(&w8001->cmd_done);
>>               break;
>>
>> +     /* 2 finger touch packet */
>>       case W8001_PKTLEN_TOUCH2FG - 1:
>> -             /* ignore two-finger touch packet. */
>> -             if (w8001->pktlen == w8001->idx)
>> -                     w8001->idx = 0;
>> +             w8001->idx = 0;
>> +             parse_touch(w8001->data, w8001->pktlen, &touch);
>> +             w8001_track_fingers(w8001, &touch);
>>               break;
>>       }
>>
>> @@ -282,6 +365,16 @@ static int w8001_setup(struct w8001 *w8001)
>>                       break;
>>               case 5:
>>                       w8001->pktlen = W8001_PKTLEN_TOUCH2FG;
>> +
>> +                     input_mt_create_slots(dev, 2);
>> +                     input_set_abs_params(dev, ABS_MT_TRACKING_ID,
>> +                                             0, 255, 0, 0);
>> +                     input_set_abs_params(dev, ABS_MT_POSITION_X,
>> +                                             0, touch.x, 0, 0);
>> +                     input_set_abs_params(dev, ABS_MT_POSITION_Y,
>> +                                             0, touch.y, 0, 0);
>> +                     input_set_abs_params(dev, ABS_MT_TOOL_TYPE,
>> +                                             0, 0, 0, 0);
>>                       break;
>>               }
>>       }
>
>
> No information about signal-to-noise ratio (fuzz) for this device?

No.

Ping
--
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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2] input: add multitouch slot support to w8001.
  2010-08-20  4:55 ` [PATCH 3/3] input: add multitouch slot support to w8001 Peter Hutterer
  2010-08-23  8:18   ` Henrik Rydberg
@ 2010-08-26  2:01   ` Peter Hutterer
  2010-08-26  6:42     ` Henrik Rydberg
  1 sibling, 1 reply; 11+ messages in thread
From: Peter Hutterer @ 2010-08-26  2:01 UTC (permalink / raw)
  To: linux-input, linux-kernel, pinglinux, rydberg

Some serial wacom devices support two-finger touch. Test for this during
init and parse the touch packets accordingly. Touch packets are
processed using Protocol B (MT Slots).

Note: there are several wacom versions that do touch but not two-finger
touch. These are not catered for here, touch events for these are simply
discarded.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
CC: Henrik Rydberg <rydberg@euromail.se>
---
Changes to v1:
- model MT slot approach after Henrik's bamboo patches
- add a comment about what the missing bytes are in the data packets

Henrik, I think this addresses all your comments? The patch is certainly
a bit simpler now.

 drivers/input/touchscreen/wacom_w8001.c |   49 +++++++++++++++++++++++++++++--
 1 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
index 7feac22..297c79d 100644
--- a/drivers/input/touchscreen/wacom_w8001.c
+++ b/drivers/input/touchscreen/wacom_w8001.c
@@ -49,6 +49,8 @@ MODULE_LICENSE("GPL");
 #define W8001_PKTLEN_TPCCTL	11	/* control packet */
 #define W8001_PKTLEN_TOUCH2FG	13
 
+#define MAX_TRACKING_ID		0xFF	/* arbitrarily chosen */
+
 struct w8001_coord {
 	u8 rdy;
 	u8 tsw;
@@ -86,6 +88,7 @@ struct w8001 {
 	char phys[32];
 	int type;
 	unsigned int pktlen;
+	int trkid[2];
 };
 
 static void parse_data(u8 *data, struct w8001_coord *coord)
@@ -112,6 +115,35 @@ static void parse_data(u8 *data, struct w8001_coord *coord)
 	coord->tilt_y = data[8] & 0x7F;
 }
 
+static void parse_touch(struct w8001 *w8001)
+{
+	static int trkid;
+	struct input_dev *dev = w8001->dev;
+	unsigned char *data = w8001->data;
+	int i;
+
+	for (i = 0; i < 2; i++) {
+		input_mt_slot(dev, i);
+
+		if (data[0] & (1 << i)) {
+			int x = (data[6 * i + 1] << 7) | (data[6 * i + 2]);
+			int y = (data[6 * i + 3] << 7) | (data[6 * i + 4]);
+			/* data[5,6] and [11,12] is finger capacity */
+
+			input_report_abs(dev, ABS_MT_POSITION_X, x);
+			input_report_abs(dev, ABS_MT_POSITION_Y, y);
+			input_report_abs(dev, ABS_MT_TOOL_TYPE, MT_TOOL_FINGER);
+			if (w8001->trkid[i] < 0)
+				w8001->trkid[i] = trkid++ & MAX_TRACKING_ID;
+		} else {
+			w8001->trkid[i] = -1;
+		}
+		input_report_abs(dev, ABS_MT_TRACKING_ID, w8001->trkid[i]);
+	}
+
+	input_sync(dev);
+}
+
 static void parse_touchquery(u8 *data, struct w8001_touch_query *query)
 {
 	memset(query, 0, sizeof(*query));
@@ -218,10 +250,10 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
 		complete(&w8001->cmd_done);
 		break;
 
+	/* 2 finger touch packet */
 	case W8001_PKTLEN_TOUCH2FG - 1:
-		/* ignore two-finger touch packet. */
-		if (w8001->pktlen == w8001->idx)
-			w8001->idx = 0;
+		w8001->idx = 0;
+		parse_touch(w8001);
 		break;
 	}
 
@@ -283,6 +315,16 @@ static int w8001_setup(struct w8001 *w8001)
 			break;
 		case 5:
 			w8001->pktlen = W8001_PKTLEN_TOUCH2FG;
+
+			input_mt_create_slots(dev, 2);
+			input_set_abs_params(dev, ABS_MT_TRACKING_ID,
+						0, MAX_TRACKING_ID, 0, 0);
+			input_set_abs_params(dev, ABS_MT_POSITION_X,
+						0, touch.x, 0, 0);
+			input_set_abs_params(dev, ABS_MT_POSITION_Y,
+						0, touch.y, 0, 0);
+			input_set_abs_params(dev, ABS_MT_TOOL_TYPE,
+						0, 0, 0, 0);
 			break;
 		}
 	}
@@ -328,6 +370,7 @@ static int w8001_connect(struct serio *serio, struct serio_driver *drv)
 	w8001->serio = serio;
 	w8001->id = serio->id.id;
 	w8001->dev = input_dev;
+	w8001->trkid[0] = w8001->trkid[1] = -1;
 	init_completion(&w8001->cmd_done);
 	snprintf(w8001->phys, sizeof(w8001->phys), "%s/input0", serio->phys);
 
-- 
1.7.2.2

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] input: add multitouch slot support to w8001.
  2010-08-26  2:01   ` [PATCH v2] " Peter Hutterer
@ 2010-08-26  6:42     ` Henrik Rydberg
  2010-08-29  5:48       ` Dmitry Torokhov
  0 siblings, 1 reply; 11+ messages in thread
From: Henrik Rydberg @ 2010-08-26  6:42 UTC (permalink / raw)
  To: Peter Hutterer; +Cc: linux-input, linux-kernel, pinglinux

On 08/26/2010 04:01 AM, Peter Hutterer wrote:

> Some serial wacom devices support two-finger touch. Test for this during
> init and parse the touch packets accordingly. Touch packets are
> processed using Protocol B (MT Slots).
>
> Note: there are several wacom versions that do touch but not two-finger
> touch. These are not catered for here, touch events for these are simply
> discarded.
>
> Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
> CC: Henrik Rydberg <rydberg@euromail.se>
> ---
> Changes to v1:
> - model MT slot approach after Henrik's bamboo patches
> - add a comment about what the missing bytes are in the data packets
>
> Henrik, I think this addresses all your comments? The patch is certainly
> a bit simpler now.


Acked-by: Henrik Rydberg <rydberg@euromail.se>

Yes, thanks, it looks fine now. And thanks for finding the bug in my own
version. :-)

Cheers,
Henrik

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] input: add multitouch slot support to w8001.
  2010-08-26  6:42     ` Henrik Rydberg
@ 2010-08-29  5:48       ` Dmitry Torokhov
  0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Torokhov @ 2010-08-29  5:48 UTC (permalink / raw)
  To: Henrik Rydberg; +Cc: Peter Hutterer, linux-input, linux-kernel, pinglinux

On Thu, Aug 26, 2010 at 08:42:18AM +0200, Henrik Rydberg wrote:
> On 08/26/2010 04:01 AM, Peter Hutterer wrote:
> 
> > Some serial wacom devices support two-finger touch. Test for this during
> > init and parse the touch packets accordingly. Touch packets are
> > processed using Protocol B (MT Slots).
> >
> > Note: there are several wacom versions that do touch but not two-finger
> > touch. These are not catered for here, touch events for these are simply
> > discarded.
> >
> > Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
> > CC: Henrik Rydberg <rydberg@euromail.se>
> > ---
> > Changes to v1:
> > - model MT slot approach after Henrik's bamboo patches
> > - add a comment about what the missing bytes are in the data packets
> >
> > Henrik, I think this addresses all your comments? The patch is certainly
> > a bit simpler now.
> 
> 
> Acked-by: Henrik Rydberg <rydberg@euromail.se>
> 
> Yes, thanks, it looks fine now. And thanks for finding the bug in my own
> version. :-)
> 

Applied all 3 to 'next', thanks Peter and Henrik.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2010-08-29  5:48 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-20  4:55 [PATCH 0/3] touch tablet support for the w8001 serial driver Peter Hutterer
2010-08-20  4:55 ` [PATCH 1/3] input: send BTN_TOOL_PEN/RUBBER and BTN_STYLUS for the w8001 Peter Hutterer
2010-08-20  4:55   ` Peter Hutterer
2010-08-20  4:55 ` [PATCH 2/3] input: support (and ignore) touch tablets in " Peter Hutterer
2010-08-20  4:55 ` [PATCH 3/3] input: add multitouch slot support to w8001 Peter Hutterer
2010-08-23  8:18   ` Henrik Rydberg
2010-08-23 14:11     ` Ping Cheng
2010-08-23 14:11       ` Ping Cheng
2010-08-26  2:01   ` [PATCH v2] " Peter Hutterer
2010-08-26  6:42     ` Henrik Rydberg
2010-08-29  5:48       ` Dmitry Torokhov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.