linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 3/3 v2] input - wacom_w8001: Add one finger touch support
@ 2010-12-17 17:37 Ping Cheng
  2010-12-29  7:40 ` Dmitry Torokhov
  0 siblings, 1 reply; 11+ messages in thread
From: Ping Cheng @ 2010-12-17 17:37 UTC (permalink / raw)
  To: linux-input; +Cc: dmitry.torokhov, Ping Cheng, Ping Cheng

Signed-off-by: Ping Cheng <pingc@wacom.com>
---
 drivers/input/touchscreen/wacom_w8001.c |   89 ++++++++++++++++++++++++++++---
 1 files changed, 82 insertions(+), 7 deletions(-)

diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
index 59664a8..763eb8f 100644
--- a/drivers/input/touchscreen/wacom_w8001.c
+++ b/drivers/input/touchscreen/wacom_w8001.c
@@ -3,6 +3,7 @@
  *
  * Copyright (c) 2008 Jaya Kumar
  * Copyright (c) 2010 Red Hat, Inc.
+ * Copyright (c) 2010 Ping Cheng, Wacom. <pingc@wacom.com>
  *
  * 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
@@ -86,6 +87,12 @@ struct w8001 {
 	char phys[32];
 	int type;
 	unsigned int pktlen;
+	bool pen_in_prox;
+	bool has_touch;
+	int max_touch_x;
+	int max_touch_y;
+	int max_pen_x;
+	int max_pen_y;
 };
 
 static void parse_data(u8 *data, struct w8001_coord *coord)
@@ -112,6 +119,29 @@ static void parse_data(u8 *data, struct w8001_coord *coord)
 	coord->tilt_y = data[8] & 0x7F;
 }
 
+static void parse_single_touch(struct w8001 *w8001)
+{
+	struct input_dev *dev = w8001->dev;
+	unsigned char *data = w8001->data;
+
+	int x = (data[1] << 7) | data[2];
+	int y = (data[3] << 7) | data[4];
+	w8001->has_touch = data[0] & 0x1;
+
+	/* scale to pen maximum */
+	if (w8001->max_pen_x && w8001->max_pen_y && w8001->max_touch_x) {
+		x = x * w8001->max_pen_x / w8001->max_touch_x;
+		y = y * w8001->max_pen_y / w8001->max_touch_y;
+	}
+
+	input_report_abs(dev, ABS_X, x);
+	input_report_abs(dev, ABS_Y, y);
+	input_report_key(dev, BTN_TOUCH, w8001->has_touch);
+	input_report_key(dev, BTN_TOOL_FINGER, w8001->has_touch);
+
+	input_sync(dev);
+}
+
 static void parse_touch(struct w8001 *w8001)
 {
 	struct input_dev *dev = w8001->dev;
@@ -151,6 +181,15 @@ static void parse_touchquery(u8 *data, struct w8001_touch_query *query)
 	query->y = data[5] << 9;
 	query->y |= data[6] << 2;
 	query->y |= (data[2] >> 3) & 0x3;
+
+	/* Early days' one finger touch models need the following defaults */
+	if (!query->x && !query->y) {
+		query->x = 1024;
+		query->y = 1024;
+		if (query->panel_res)
+			query->x = query->y = (1 << query->panel_res);
+		query->panel_res = 10;
+	}
 }
 
 static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
@@ -199,6 +238,7 @@ 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;
 
@@ -213,9 +253,15 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
 
 	case W8001_PKTLEN_TOUCH93 - 1:
 	case W8001_PKTLEN_TOUCH9A - 1:
-		/* ignore one-finger touch packet. */
-		if (w8001->pktlen == w8001->idx)
+		tmp = (w8001->data[0] & W8001_TOUCH_BYTE);
+		if (tmp != W8001_TOUCH_BYTE)
+			break;
+
+		if (w8001->pktlen == w8001->idx) {
 			w8001->idx = 0;
+			if (!w8001->pen_in_prox)
+				parse_single_touch(w8001);
+		}
 		break;
 
 	/* Pen coordinates packet */
@@ -228,9 +274,18 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
 		if (tmp == W8001_TOUCH_BYTE)
 			break;
 
+		if (w8001->has_touch) {
+			/* send touch data out first */
+			w8001->has_touch = 0;
+			input_report_key(dev, BTN_TOUCH, 0);
+			input_report_key(dev, BTN_TOOL_FINGER, 0);
+			input_sync(dev);
+		}
+
 		w8001->idx = 0;
 		parse_data(w8001->data, &coord);
 		report_pen_events(w8001, &coord);
+		w8001->pen_in_prox = coord.rdy ? true : false;
 		break;
 
 	/* control packet */
@@ -274,6 +329,24 @@ static int w8001_command(struct w8001 *w8001, unsigned char command,
 	return rc;
 }
 
+static void w8001_setup_single_touch(struct w8001 *w8001)
+{
+	struct input_dev *dev = w8001->dev;
+	int px = w8001->max_touch_x, py = w8001->max_touch_y;
+
+	__set_bit(BTN_TOUCH, dev->keybit);
+	__set_bit(BTN_TOOL_FINGER, dev->keybit);
+
+	/* scale to pen maximum */
+	if (w8001->max_pen_x && w8001->max_pen_y) {
+		px = w8001->max_pen_x;
+		py = w8001->max_pen_y;
+	}
+
+	input_set_abs_params(dev, ABS_X, 0, px, 0, 0);
+	input_set_abs_params(dev, ABS_Y, 0, py, 0, 0);
+}
+
 static int w8001_setup(struct w8001 *w8001)
 {
 	struct input_dev *dev = w8001->dev;
@@ -289,6 +362,7 @@ static int w8001_setup(struct w8001 *w8001)
 	/* penabled? */
 	error = w8001_command(w8001, W8001_CMD_QUERY, true);
 	if (!error) {
+		__set_bit(BTN_TOUCH, dev->keybit);
 		__set_bit(BTN_TOOL_PEN, dev->keybit);
 		__set_bit(BTN_TOOL_RUBBER, dev->keybit);
 		__set_bit(BTN_STYLUS, dev->keybit);
@@ -302,6 +376,8 @@ 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);
 		}
+		w8001->max_pen_x = coord.x;
+		w8001->max_pen_y = coord.y;
 	}
 
 	/* touch enabled? */
@@ -314,20 +390,20 @@ static int w8001_setup(struct w8001 *w8001)
 		struct w8001_touch_query touch;
 
 		parse_touchquery(w8001->response, &touch);
-
-		input_set_abs_params(dev, ABS_X, 0, touch.x, 0, 0);
-		input_set_abs_params(dev, ABS_Y, 0, touch.y, 0, 0);
-		__set_bit(BTN_TOOL_FINGER, dev->keybit);
+		w8001->max_touch_x = touch.x;
+		w8001->max_touch_y = touch.y;
 
 		switch (touch.sensor_id) {
 		case 0:
 		case 2:
 			w8001->pktlen = W8001_PKTLEN_TOUCH93;
+			w8001_setup_single_touch(w8001);
 			break;
 		case 1:
 		case 3:
 		case 4:
 			w8001->pktlen = W8001_PKTLEN_TOUCH9A;
+			w8001_setup_single_touch(w8001);
 			break;
 		case 5:
 			w8001->pktlen = W8001_PKTLEN_TOUCH2FG;
@@ -396,7 +472,6 @@ static int w8001_connect(struct serio *serio, struct serio_driver *drv)
 	input_dev->dev.parent = &serio->dev;
 
 	input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
-	__set_bit(BTN_TOUCH, input_dev->keybit);
 
 	serio_set_drvdata(serio, w8001);
 	err = serio_open(serio, drv);
-- 
1.7.3.3


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

* Re: [PATCH 3/3 v2] input - wacom_w8001: Add one finger touch support
  2010-12-17 17:37 [PATCH 3/3 v2] input - wacom_w8001: Add one finger touch support Ping Cheng
@ 2010-12-29  7:40 ` Dmitry Torokhov
  2010-12-29  7:42   ` Dmitry Torokhov
  2010-12-29 22:30   ` Ping Cheng
  0 siblings, 2 replies; 11+ messages in thread
From: Dmitry Torokhov @ 2010-12-29  7:40 UTC (permalink / raw)
  To: Ping Cheng; +Cc: linux-input, Ping Cheng

Hi Ping,

On Fri, Dec 17, 2010 at 09:37:54AM -0800, Ping Cheng wrote:
> Signed-off-by: Ping Cheng <pingc@wacom.com>
> ---
>  drivers/input/touchscreen/wacom_w8001.c |   89 ++++++++++++++++++++++++++++---
>  1 files changed, 82 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
> index 59664a8..763eb8f 100644
> --- a/drivers/input/touchscreen/wacom_w8001.c
> +++ b/drivers/input/touchscreen/wacom_w8001.c
> @@ -3,6 +3,7 @@
>   *
>   * Copyright (c) 2008 Jaya Kumar
>   * Copyright (c) 2010 Red Hat, Inc.
> + * Copyright (c) 2010 Ping Cheng, Wacom. <pingc@wacom.com>
>   *
>   * 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
> @@ -86,6 +87,12 @@ struct w8001 {
>  	char phys[32];
>  	int type;
>  	unsigned int pktlen;
> +	bool pen_in_prox;
> +	bool has_touch;

We already have type, why do we need these 2 fields a well?

Actually, I tried massaging the patch a bit, could you please tell me if
the patch below still works for you?

Thanks.

-- 
Dmitry


Input: wacom_w8001 - add one finger touch support

From: Ping Cheng <pinglinux@gmail.com>

Signed-off-by: Ping Cheng <pingc@wacom.com>
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/input/touchscreen/wacom_w8001.c |  123 +++++++++++++++++++++++++------
 1 files changed, 101 insertions(+), 22 deletions(-)


diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
index 4774c09..ce8c8e1 100644
--- a/drivers/input/touchscreen/wacom_w8001.c
+++ b/drivers/input/touchscreen/wacom_w8001.c
@@ -3,6 +3,7 @@
  *
  * Copyright (c) 2008 Jaya Kumar
  * Copyright (c) 2010 Red Hat, Inc.
+ * Copyright (c) 2010 Ping Cheng, Wacom. <pingc@wacom.com>
  *
  * 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
@@ -63,11 +64,11 @@ struct w8001_coord {
 
 /* touch query reply packet */
 struct w8001_touch_query {
+	u16 x;
+	u16 y;
 	u8 panel_res;
 	u8 capacity_res;
 	u8 sensor_id;
-	u16 x;
-	u16 y;
 };
 
 /*
@@ -86,9 +87,13 @@ struct w8001 {
 	char phys[32];
 	int type;
 	unsigned int pktlen;
+	u16 max_touch_x;
+	u16 max_touch_y;
+	u16 max_pen_x;
+	u16 max_pen_y;
 };
 
-static void parse_data(u8 *data, struct w8001_coord *coord)
+static void parse_pen_data(u8 *data, struct w8001_coord *coord)
 {
 	memset(coord, 0, sizeof(*coord));
 
@@ -112,7 +117,14 @@ static void parse_data(u8 *data, struct w8001_coord *coord)
 	coord->tilt_y = data[8] & 0x7F;
 }
 
-static void parse_touch(struct w8001 *w8001)
+static void parse_single_touch(u8 *data, struct w8001_coord *coord)
+{
+	coord->x = (data[1] << 7) | data[2];
+	coord->y = (data[3] << 7) | data[4];
+	coord->tsw = data[0] & 0x01;
+}
+
+static void parse_multi_touch(struct w8001 *w8001)
 {
 	struct input_dev *dev = w8001->dev;
 	unsigned char *data = w8001->data;
@@ -124,8 +136,8 @@ static void parse_touch(struct w8001 *w8001)
 		input_mt_slot(dev, i);
 		input_mt_report_slot_state(dev, MT_TOOL_FINGER, touch);
 		if (touch) {
-			int x = (data[6 * i + 1] << 7) | (data[6 * i + 2]);
-			int y = (data[6 * i + 3] << 7) | (data[6 * i + 4]);
+			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);
@@ -151,6 +163,15 @@ static void parse_touchquery(u8 *data, struct w8001_touch_query *query)
 	query->y = data[5] << 9;
 	query->y |= data[6] << 2;
 	query->y |= (data[2] >> 3) & 0x3;
+
+	/* Early days' single-finger touch models need the following defaults */
+	if (!query->x && !query->y) {
+		query->x = 1024;
+		query->y = 1024;
+		if (query->panel_res)
+			query->x = query->y = (1 << query->panel_res);
+		query->panel_res = 10;
+	}
 }
 
 static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
@@ -160,16 +181,15 @@ static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
 	/*
 	 * 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
+	 * 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) {
+	switch (w8001->type) {
+	case BTN_TOOL_RUBBER:
 		if (!coord->f2) {
 			input_report_abs(dev, ABS_PRESSURE, 0);
 			input_report_key(dev, BTN_TOUCH, 0);
@@ -179,8 +199,21 @@ static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
 			input_sync(dev);
 			w8001->type = BTN_TOOL_PEN;
 		}
-	} else {
+		break;
+
+	case BTN_TOOL_FINGER:
+		input_report_key(dev, BTN_TOUCH, 0);
+		input_report_key(dev, BTN_TOOL_FINGER, 0);
+		input_sync(dev);
+		/* fall through */
+
+	case KEY_RESERVED:
+		w8001->type = coord->f2 ? BTN_TOOL_RUBBER : BTN_TOOL_PEN;
+		break;
+
+	default:
 		input_report_key(dev, BTN_STYLUS2, coord->f2);
+		break;
 	}
 
 	input_report_abs(dev, ABS_X, coord->x);
@@ -192,7 +225,30 @@ static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
 	input_sync(dev);
 
 	if (!coord->rdy)
-		w8001->type = 0;
+		w8001->type = KEY_RESERVED;
+}
+
+static void report_single_touch(struct w8001 *w8001, struct w8001_coord *coord)
+{
+	struct input_dev *dev = w8001->dev;
+	unsigned int x = coord->x;
+	unsigned int y = coord->y;
+
+	/* scale to pen maximum */
+	if (w8001->max_pen_x && w8001->max_touch_x)
+		x = x * w8001->max_pen_x / w8001->max_touch_x;
+
+	if (w8001->max_pen_y && w8001->max_touch_y)
+		y = y * w8001->max_pen_y / w8001->max_touch_y;
+
+	input_report_abs(dev, ABS_X, x);
+	input_report_abs(dev, ABS_Y, y);
+	input_report_key(dev, BTN_TOUCH, coord->tsw);
+	input_report_key(dev, BTN_TOOL_FINGER, coord->tsw);
+
+	input_sync(dev);
+
+	w8001->type = coord->tsw ? BTN_TOOL_FINGER : KEY_RESERVED;
 }
 
 static irqreturn_t w8001_interrupt(struct serio *serio,
@@ -213,9 +269,18 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
 
 	case W8001_PKTLEN_TOUCH93 - 1:
 	case W8001_PKTLEN_TOUCH9A - 1:
-		/* ignore one-finger touch packet. */
-		if (w8001->pktlen == w8001->idx)
+		tmp = w8001->data[0] & W8001_TOUCH_BYTE;
+		if (tmp != W8001_TOUCH_BYTE)
+			break;
+
+		if (w8001->pktlen == w8001->idx) {
 			w8001->idx = 0;
+			if (w8001->type != BTN_TOOL_PEN &&
+			    w8001->type != BTN_TOOL_RUBBER) {
+				parse_single_touch(w8001->data, &coord);
+				report_single_touch(w8001, &coord);
+			}
+		}
 		break;
 
 	/* Pen coordinates packet */
@@ -224,18 +289,18 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
 		if (unlikely(tmp == W8001_TAB_BYTE))
 			break;
 
-		tmp = (w8001->data[0] & W8001_TOUCH_BYTE);
+		tmp = w8001->data[0] & W8001_TOUCH_BYTE;
 		if (tmp == W8001_TOUCH_BYTE)
 			break;
 
 		w8001->idx = 0;
-		parse_data(w8001->data, &coord);
+		parse_pen_data(w8001->data, &coord);
 		report_pen_events(w8001, &coord);
 		break;
 
 	/* control packet */
 	case W8001_PKTLEN_TPCCTL - 1:
-		tmp = (w8001->data[0] & W8001_TOUCH_MASK);
+		tmp = w8001->data[0] & W8001_TOUCH_MASK;
 		if (tmp == W8001_TOUCH_BYTE)
 			break;
 
@@ -248,7 +313,7 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
 	/* 2 finger touch packet */
 	case W8001_PKTLEN_TOUCH2FG - 1:
 		w8001->idx = 0;
-		parse_touch(w8001);
+		parse_multi_touch(w8001);
 		break;
 	}
 
@@ -278,6 +343,7 @@ static int w8001_setup(struct w8001 *w8001)
 {
 	struct input_dev *dev = w8001->dev;
 	struct w8001_coord coord;
+	struct w8001_touch_query touch;
 	int error;
 
 	error = w8001_command(w8001, W8001_CMD_STOP, false);
@@ -289,11 +355,15 @@ static int w8001_setup(struct w8001 *w8001)
 	/* penabled? */
 	error = w8001_command(w8001, W8001_CMD_QUERY, true);
 	if (!error) {
+		__set_bit(BTN_TOUCH, dev->keybit);
 		__set_bit(BTN_TOOL_PEN, dev->keybit);
 		__set_bit(BTN_TOOL_RUBBER, dev->keybit);
 		__set_bit(BTN_STYLUS, dev->keybit);
 		__set_bit(BTN_STYLUS2, dev->keybit);
-		parse_data(w8001->response, &coord);
+
+		parse_pen_data(w8001->response, &coord);
+		w8001->max_pen_x = coord.x;
+		w8001->max_pen_y = coord.y;
 
 		input_set_abs_params(dev, ABS_X, 0, coord.x, 0, 0);
 		input_set_abs_params(dev, ABS_Y, 0, coord.y, 0, 0);
@@ -312,24 +382,34 @@ static int w8001_setup(struct w8001 *w8001)
 	 * second byte is empty, which indicates touch is not supported.
 	 */
 	if (!error && w8001->response[1]) {
-		struct w8001_touch_query touch;
+		__set_bit(BTN_TOUCH, dev->keybit);
+		__set_bit(BTN_TOOL_FINGER, dev->keybit);
 
 		parse_touchquery(w8001->response, &touch);
+		w8001->max_touch_x = touch.x;
+		w8001->max_touch_y = touch.y;
+
+		/* scale to pen maximum */
+		if (w8001->max_pen_x && w8001->max_pen_y) {
+			touch.x = w8001->max_pen_x;
+			touch.y = w8001->max_pen_y;
+		}
 
 		input_set_abs_params(dev, ABS_X, 0, touch.x, 0, 0);
 		input_set_abs_params(dev, ABS_Y, 0, touch.y, 0, 0);
-		__set_bit(BTN_TOOL_FINGER, dev->keybit);
 
 		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;
 
@@ -397,7 +477,6 @@ static int w8001_connect(struct serio *serio, struct serio_driver *drv)
 	input_dev->dev.parent = &serio->dev;
 
 	input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
-	__set_bit(BTN_TOUCH, input_dev->keybit);
 
 	serio_set_drvdata(serio, w8001);
 	err = serio_open(serio, drv);
--
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

* Re: [PATCH 3/3 v2] input - wacom_w8001: Add one finger touch support
  2010-12-29  7:40 ` Dmitry Torokhov
@ 2010-12-29  7:42   ` Dmitry Torokhov
  2010-12-29 22:54     ` Ping Cheng
  2010-12-29 22:30   ` Ping Cheng
  1 sibling, 1 reply; 11+ messages in thread
From: Dmitry Torokhov @ 2010-12-29  7:42 UTC (permalink / raw)
  To: Ping Cheng; +Cc: linux-input, Ping Cheng

On Tue, Dec 28, 2010 at 11:40:58PM -0800, Dmitry Torokhov wrote:
> Hi Ping,
> 
> On Fri, Dec 17, 2010 at 09:37:54AM -0800, Ping Cheng wrote:
> > Signed-off-by: Ping Cheng <pingc@wacom.com>
> > ---
> >  drivers/input/touchscreen/wacom_w8001.c |   89 ++++++++++++++++++++++++++++---
> >  1 files changed, 82 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
> > index 59664a8..763eb8f 100644
> > --- a/drivers/input/touchscreen/wacom_w8001.c
> > +++ b/drivers/input/touchscreen/wacom_w8001.c
> > @@ -3,6 +3,7 @@
> >   *
> >   * Copyright (c) 2008 Jaya Kumar
> >   * Copyright (c) 2010 Red Hat, Inc.
> > + * Copyright (c) 2010 Ping Cheng, Wacom. <pingc@wacom.com>
> >   *
> >   * 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
> > @@ -86,6 +87,12 @@ struct w8001 {
> >  	char phys[32];
> >  	int type;
> >  	unsigned int pktlen;
> > +	bool pen_in_prox;
> > +	bool has_touch;
> 
> We already have type, why do we need these 2 fields a well?
> 
> Actually, I tried massaging the patch a bit, could you please tell me if
> the patch below still works for you?
> 
> Thanks.
> 

And while we are at it could you please try this patch as well...

Thank you!

-- 
Dmitry

Input: wacom_w8001 - add single-touch pointer emulation

Let's emit single-touch compatible events for the 2-finger panels so that
they can be used with legacy clients.

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

 drivers/input/touchscreen/wacom_w8001.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)


diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
index ce8c8e1..01489f7 100644
--- a/drivers/input/touchscreen/wacom_w8001.c
+++ b/drivers/input/touchscreen/wacom_w8001.c
@@ -145,6 +145,7 @@ static void parse_multi_touch(struct w8001 *w8001)
 		}
 	}
 
+	input_mt_report_pointer_emulation(dev, true);
 	input_sync(dev);
 }
 
@@ -420,6 +421,9 @@ static int w8001_setup(struct w8001 *w8001)
 						0, touch.y, 0, 0);
 			input_set_abs_params(dev, ABS_MT_TOOL_TYPE,
 						0, MT_TOOL_MAX, 0, 0);
+
+			__set_bit(BTN_TOOL_DOUBLETAP, dev->keybit);
+
 			break;
 		}
 	}

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

* Re: [PATCH 3/3 v2] input - wacom_w8001: Add one finger touch support
  2010-12-29  7:40 ` Dmitry Torokhov
  2010-12-29  7:42   ` Dmitry Torokhov
@ 2010-12-29 22:30   ` Ping Cheng
  2010-12-30 18:25     ` Ping Cheng
  2011-01-02  8:56     ` Dmitry Torokhov
  1 sibling, 2 replies; 11+ messages in thread
From: Ping Cheng @ 2010-12-29 22:30 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input

Dmitry,

Thank you for your effort in updating the patch. Most changes work for
me. I do have comments related to raw touch data scaling for MT and
single touch legacy client support. Details are inline.

Ping

On Tue, Dec 28, 2010 at 11:40 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi Ping,
>
> On Fri, Dec 17, 2010 at 09:37:54AM -0800, Ping Cheng wrote:
>> Signed-off-by: Ping Cheng <pingc@wacom.com>
>> ---
>>  drivers/input/touchscreen/wacom_w8001.c |   89 ++++++++++++++++++++++++++++---
>>  1 files changed, 82 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
>> index 59664a8..763eb8f 100644
>> --- a/drivers/input/touchscreen/wacom_w8001.c
>> +++ b/drivers/input/touchscreen/wacom_w8001.c
>> @@ -3,6 +3,7 @@
>>   *
>>   * Copyright (c) 2008 Jaya Kumar
>>   * Copyright (c) 2010 Red Hat, Inc.
>> + * Copyright (c) 2010 Ping Cheng, Wacom. <pingc@wacom.com>
>>   *
>>   * 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
>> @@ -86,6 +87,12 @@ struct w8001 {
>>       char phys[32];
>>       int type;
>>       unsigned int pktlen;
>> +     bool pen_in_prox;
>> +     bool has_touch;
>
> We already have type, why do we need these 2 fields a well?
>
> Actually, I tried massaging the patch a bit, could you please tell me if
> the patch below still works for you?
>
> Thanks.
>
> --
> Dmitry
>
>
> Input: wacom_w8001 - add one finger touch support
>
> From: Ping Cheng <pinglinux@gmail.com>
>
> Signed-off-by: Ping Cheng <pingc@wacom.com>
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---
>
>  drivers/input/touchscreen/wacom_w8001.c |  123 +++++++++++++++++++++++++------
>  1 files changed, 101 insertions(+), 22 deletions(-)
>
>
> diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
> index 4774c09..ce8c8e1 100644
> --- a/drivers/input/touchscreen/wacom_w8001.c
> +++ b/drivers/input/touchscreen/wacom_w8001.c
> @@ -3,6 +3,7 @@
>  *
>  * Copyright (c) 2008 Jaya Kumar
>  * Copyright (c) 2010 Red Hat, Inc.
> + * Copyright (c) 2010 Ping Cheng, Wacom. <pingc@wacom.com>
>  *
>  * 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
> @@ -63,11 +64,11 @@ struct w8001_coord {
>
>  /* touch query reply packet */
>  struct w8001_touch_query {
> +       u16 x;
> +       u16 y;
>        u8 panel_res;
>        u8 capacity_res;
>        u8 sensor_id;
> -       u16 x;
> -       u16 y;
>  };
>
>  /*
> @@ -86,9 +87,13 @@ struct w8001 {
>        char phys[32];
>        int type;
>        unsigned int pktlen;
> +       u16 max_touch_x;
> +       u16 max_touch_y;
> +       u16 max_pen_x;
> +       u16 max_pen_y;
>  };
>
> -static void parse_data(u8 *data, struct w8001_coord *coord)
> +static void parse_pen_data(u8 *data, struct w8001_coord *coord)
>  {
>        memset(coord, 0, sizeof(*coord));
>
> @@ -112,7 +117,14 @@ static void parse_data(u8 *data, struct w8001_coord *coord)
>        coord->tilt_y = data[8] & 0x7F;
>  }
>
> -static void parse_touch(struct w8001 *w8001)
> +static void parse_single_touch(u8 *data, struct w8001_coord *coord)
> +{
> +       coord->x = (data[1] << 7) | data[2];
> +       coord->y = (data[3] << 7) | data[4];
> +       coord->tsw = data[0] & 0x01;
> +}
> +
> +static void parse_multi_touch(struct w8001 *w8001)
>  {
>        struct input_dev *dev = w8001->dev;
>        unsigned char *data = w8001->data;
> @@ -124,8 +136,8 @@ static void parse_touch(struct w8001 *w8001)
>                input_mt_slot(dev, i);
>                input_mt_report_slot_state(dev, MT_TOOL_FINGER, touch);
>                if (touch) {
> -                       int x = (data[6 * i + 1] << 7) | (data[6 * i + 2]);
> -                       int y = (data[6 * i + 3] << 7) | (data[6 * i + 4]);
> +                       int x = (data[6 * i + 1] << 7) | data[6 * i + 2];
> +                       int y = (data[6 * i + 3] << 7) | data[6 * i + 4];

Since you scaled MT max_touch_x/y to max_pen_x/y, you would need to
scale the x and y here as well. I didn't scale MT touch data since MT
protocol can report touch resolution on its own. See more comments
below.

>                        /* data[5,6] and [11,12] is finger capacity */
>
>                        input_report_abs(dev, ABS_MT_POSITION_X, x);
> @@ -151,6 +163,15 @@ static void parse_touchquery(u8 *data, struct w8001_touch_query *query)
>        query->y = data[5] << 9;
>        query->y |= data[6] << 2;
>        query->y |= (data[2] >> 3) & 0x3;
> +
> +       /* Early days' single-finger touch models need the following defaults */
> +       if (!query->x && !query->y) {
> +               query->x = 1024;
> +               query->y = 1024;
> +               if (query->panel_res)
> +                       query->x = query->y = (1 << query->panel_res);
> +               query->panel_res = 10;
> +       }
>  }
>
>  static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
> @@ -160,16 +181,15 @@ static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
>        /*
>         * 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
> +        * 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) {
> +       switch (w8001->type) {
> +       case BTN_TOOL_RUBBER:
>                if (!coord->f2) {
>                        input_report_abs(dev, ABS_PRESSURE, 0);
>                        input_report_key(dev, BTN_TOUCH, 0);
> @@ -179,8 +199,21 @@ static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
>                        input_sync(dev);
>                        w8001->type = BTN_TOOL_PEN;
>                }
> -       } else {
> +               break;
> +
> +       case BTN_TOOL_FINGER:
> +               input_report_key(dev, BTN_TOUCH, 0);
> +               input_report_key(dev, BTN_TOOL_FINGER, 0);
> +               input_sync(dev);
> +               /* fall through */
> +
> +       case KEY_RESERVED:
> +               w8001->type = coord->f2 ? BTN_TOOL_RUBBER : BTN_TOOL_PEN;
> +               break;
> +
> +       default:
>                input_report_key(dev, BTN_STYLUS2, coord->f2);
> +               break;
>        }
>
>        input_report_abs(dev, ABS_X, coord->x);
> @@ -192,7 +225,30 @@ static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
>        input_sync(dev);
>
>        if (!coord->rdy)
> -               w8001->type = 0;
> +               w8001->type = KEY_RESERVED;
> +}
> +
> +static void report_single_touch(struct w8001 *w8001, struct w8001_coord *coord)
> +{
> +       struct input_dev *dev = w8001->dev;
> +       unsigned int x = coord->x;
> +       unsigned int y = coord->y;
> +
> +       /* scale to pen maximum */
> +       if (w8001->max_pen_x && w8001->max_touch_x)
> +               x = x * w8001->max_pen_x / w8001->max_touch_x;
> +
> +       if (w8001->max_pen_y && w8001->max_touch_y)
> +               y = y * w8001->max_pen_y / w8001->max_touch_y;
> +
> +       input_report_abs(dev, ABS_X, x);
> +       input_report_abs(dev, ABS_Y, y);
> +       input_report_key(dev, BTN_TOUCH, coord->tsw);
> +       input_report_key(dev, BTN_TOOL_FINGER, coord->tsw);
> +
> +       input_sync(dev);
> +
> +       w8001->type = coord->tsw ? BTN_TOOL_FINGER : KEY_RESERVED;
>  }
>
>  static irqreturn_t w8001_interrupt(struct serio *serio,
> @@ -213,9 +269,18 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
>
>        case W8001_PKTLEN_TOUCH93 - 1:
>        case W8001_PKTLEN_TOUCH9A - 1:
> -               /* ignore one-finger touch packet. */
> -               if (w8001->pktlen == w8001->idx)
> +               tmp = w8001->data[0] & W8001_TOUCH_BYTE;
> +               if (tmp != W8001_TOUCH_BYTE)
> +                       break;
> +
> +               if (w8001->pktlen == w8001->idx) {
>                        w8001->idx = 0;
> +                       if (w8001->type != BTN_TOOL_PEN &&
> +                           w8001->type != BTN_TOOL_RUBBER) {
> +                               parse_single_touch(w8001->data, &coord);
> +                               report_single_touch(w8001, &coord);
> +                       }
> +               }
>                break;
>
>        /* Pen coordinates packet */
> @@ -224,18 +289,18 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
>                if (unlikely(tmp == W8001_TAB_BYTE))
>                        break;
>
> -               tmp = (w8001->data[0] & W8001_TOUCH_BYTE);
> +               tmp = w8001->data[0] & W8001_TOUCH_BYTE;
>                if (tmp == W8001_TOUCH_BYTE)
>                        break;
>
>                w8001->idx = 0;
> -               parse_data(w8001->data, &coord);
> +               parse_pen_data(w8001->data, &coord);
>                report_pen_events(w8001, &coord);
>                break;
>
>        /* control packet */
>        case W8001_PKTLEN_TPCCTL - 1:
> -               tmp = (w8001->data[0] & W8001_TOUCH_MASK);
> +               tmp = w8001->data[0] & W8001_TOUCH_MASK;
>                if (tmp == W8001_TOUCH_BYTE)
>                        break;
>
> @@ -248,7 +313,7 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
>        /* 2 finger touch packet */
>        case W8001_PKTLEN_TOUCH2FG - 1:
>                w8001->idx = 0;
> -               parse_touch(w8001);
> +               parse_multi_touch(w8001);
>                break;
>        }
>
> @@ -278,6 +343,7 @@ static int w8001_setup(struct w8001 *w8001)
>  {
>        struct input_dev *dev = w8001->dev;
>        struct w8001_coord coord;
> +       struct w8001_touch_query touch;
>        int error;
>
>        error = w8001_command(w8001, W8001_CMD_STOP, false);
> @@ -289,11 +355,15 @@ static int w8001_setup(struct w8001 *w8001)
>        /* penabled? */
>        error = w8001_command(w8001, W8001_CMD_QUERY, true);
>        if (!error) {
> +               __set_bit(BTN_TOUCH, dev->keybit);
>                __set_bit(BTN_TOOL_PEN, dev->keybit);
>                __set_bit(BTN_TOOL_RUBBER, dev->keybit);
>                __set_bit(BTN_STYLUS, dev->keybit);
>                __set_bit(BTN_STYLUS2, dev->keybit);
> -               parse_data(w8001->response, &coord);
> +
> +               parse_pen_data(w8001->response, &coord);
> +               w8001->max_pen_x = coord.x;
> +               w8001->max_pen_y = coord.y;
>
>                input_set_abs_params(dev, ABS_X, 0, coord.x, 0, 0);
>                input_set_abs_params(dev, ABS_Y, 0, coord.y, 0, 0);
> @@ -312,24 +382,34 @@ static int w8001_setup(struct w8001 *w8001)
>         * second byte is empty, which indicates touch is not supported.
>         */
>        if (!error && w8001->response[1]) {
> -               struct w8001_touch_query touch;
> +               __set_bit(BTN_TOUCH, dev->keybit);
> +               __set_bit(BTN_TOOL_FINGER, dev->keybit);

I do not think we want to set BTN_TOUCH and BTN_TOOL_FINGER for MT
device since we do not need to support single touch for this MT
device. Please refer to my comments for your other patch in the next
email.

>                parse_touchquery(w8001->response, &touch);
> +               w8001->max_touch_x = touch.x;
> +               w8001->max_touch_y = touch.y;
> +
> +               /* scale to pen maximum */
> +               if (w8001->max_pen_x && w8001->max_pen_y) {
> +                       touch.x = w8001->max_pen_x;
> +                       touch.y = w8001->max_pen_y;
> +               }

If we scale touch for both single and MT, we need to scale the raw
(x,y) for both too. I purposely did not want to scale MT touch since
they can be supported in different resolution from pen by MT protocol
now.

>
>                input_set_abs_params(dev, ABS_X, 0, touch.x, 0, 0);
>                input_set_abs_params(dev, ABS_Y, 0, touch.y, 0, 0);
> -               __set_bit(BTN_TOOL_FINGER, dev->keybit);
>
>                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;
>
> @@ -397,7 +477,6 @@ static int w8001_connect(struct serio *serio, struct serio_driver *drv)
>        input_dev->dev.parent = &serio->dev;
>
>        input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> -       __set_bit(BTN_TOUCH, input_dev->keybit);
>
>        serio_set_drvdata(serio, w8001);
>        err = serio_open(serio, drv);
>
--
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

* Re: [PATCH 3/3 v2] input - wacom_w8001: Add one finger touch support
  2010-12-29  7:42   ` Dmitry Torokhov
@ 2010-12-29 22:54     ` Ping Cheng
  2011-01-02  8:59       ` Dmitry Torokhov
  0 siblings, 1 reply; 11+ messages in thread
From: Ping Cheng @ 2010-12-29 22:54 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input

On Tue, Dec 28, 2010 at 11:42 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Tue, Dec 28, 2010 at 11:40:58PM -0800, Dmitry Torokhov wrote:
>> Hi Ping,
>>
>> On Fri, Dec 17, 2010 at 09:37:54AM -0800, Ping Cheng wrote:
>> > Signed-off-by: Ping Cheng <pingc@wacom.com>
>> > ---
>> >  drivers/input/touchscreen/wacom_w8001.c |   89 ++++++++++++++++++++++++++++---
>> >  1 files changed, 82 insertions(+), 7 deletions(-)
>> >
>> > diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
>> > index 59664a8..763eb8f 100644
>> > --- a/drivers/input/touchscreen/wacom_w8001.c
>> > +++ b/drivers/input/touchscreen/wacom_w8001.c
>> > @@ -3,6 +3,7 @@
>> >   *
>> >   * Copyright (c) 2008 Jaya Kumar
>> >   * Copyright (c) 2010 Red Hat, Inc.
>> > + * Copyright (c) 2010 Ping Cheng, Wacom. <pingc@wacom.com>
>> >   *
>> >   * 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
>> > @@ -86,6 +87,12 @@ struct w8001 {
>> >     char phys[32];
>> >     int type;
>> >     unsigned int pktlen;
>> > +   bool pen_in_prox;
>> > +   bool has_touch;
>>
>> We already have type, why do we need these 2 fields a well?
>>
>> Actually, I tried massaging the patch a bit, could you please tell me if
>> the patch below still works for you?
>>
>> Thanks.
>>
>
> And while we are at it could you please try this patch as well...
>
> Thank you!
>
> --
> Dmitry
>
> Input: wacom_w8001 - add single-touch pointer emulation
>
> Let's emit single-touch compatible events for the 2-finger panels so that
> they can be used with legacy clients.

Depending on which legacy clients we are talking about, this patch may
introduce issue instead of support. We added
mt_report_pointer_emulation for Bamboo since it is a touchpad, that
can be driven by xf86-input-synaptics in relative mode. This device is
a touchscreen, that should be run in absolute mode. If we emulate ST
events, which X driver do we expect it to use? xf86-input-wacom does
not process emulated single touch data once we know MT is supported.
That's why I didn't want to add ST emulation for this device.

We expect clients that support this MT device will be in MT format
since this driver is new and the clients have to run inputattach to
enable the port before they can talk to the device. The client must
have a reason to do so. Do you see any legacy clients would like to
walk the extra mile to use this driver?

Ping

> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---
>
>  drivers/input/touchscreen/wacom_w8001.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
>
>
> diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
> index ce8c8e1..01489f7 100644
> --- a/drivers/input/touchscreen/wacom_w8001.c
> +++ b/drivers/input/touchscreen/wacom_w8001.c
> @@ -145,6 +145,7 @@ static void parse_multi_touch(struct w8001 *w8001)
>                }
>        }
>
> +       input_mt_report_pointer_emulation(dev, true);
>        input_sync(dev);
>  }
>
> @@ -420,6 +421,9 @@ static int w8001_setup(struct w8001 *w8001)
>                                                0, touch.y, 0, 0);
>                        input_set_abs_params(dev, ABS_MT_TOOL_TYPE,
>                                                0, MT_TOOL_MAX, 0, 0);
> +
> +                       __set_bit(BTN_TOOL_DOUBLETAP, dev->keybit);
> +
>                        break;
>                }
>        }
>
--
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

* Re: [PATCH 3/3 v2] input - wacom_w8001: Add one finger touch support
  2010-12-29 22:30   ` Ping Cheng
@ 2010-12-30 18:25     ` Ping Cheng
  2011-01-02  9:01       ` Dmitry Torokhov
  2011-01-02  8:56     ` Dmitry Torokhov
  1 sibling, 1 reply; 11+ messages in thread
From: Ping Cheng @ 2010-12-30 18:25 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input

On Wed, Dec 29, 2010 at 2:30 PM, Ping Cheng <pinglinux@gmail.com> wrote:
> Dmitry,
>
> Thank you for your effort in updating the patch. Most changes work for
> me. I do have comments related to raw touch data scaling for MT and
> single touch legacy client support. Details are inline.

After revisited the change you made, I think I can accept both
suggestions. The legacy client support may introduce issue. But it
won't hurt too much, I hope ;).  The MT scaling offers a consistent
support between one and two finger touch, which is good.

So, the only change left is to scale the raw x,y of 2FGT to max_pen. I
can make a version 3 of this patch for you or you can change those two
lines if you don't mind. Just let me know.

Thank you.

Ping

> On Tue, Dec 28, 2010 at 11:40 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
>> Hi Ping,
>>
>> On Fri, Dec 17, 2010 at 09:37:54AM -0800, Ping Cheng wrote:
>>> Signed-off-by: Ping Cheng <pingc@wacom.com>
>>> ---
>>>  drivers/input/touchscreen/wacom_w8001.c |   89 ++++++++++++++++++++++++++++---
>>>  1 files changed, 82 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
>>> index 59664a8..763eb8f 100644
>>> --- a/drivers/input/touchscreen/wacom_w8001.c
>>> +++ b/drivers/input/touchscreen/wacom_w8001.c
>>> @@ -3,6 +3,7 @@
>>>   *
>>>   * Copyright (c) 2008 Jaya Kumar
>>>   * Copyright (c) 2010 Red Hat, Inc.
>>> + * Copyright (c) 2010 Ping Cheng, Wacom. <pingc@wacom.com>
>>>   *
>>>   * 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
>>> @@ -86,6 +87,12 @@ struct w8001 {
>>>       char phys[32];
>>>       int type;
>>>       unsigned int pktlen;
>>> +     bool pen_in_prox;
>>> +     bool has_touch;
>>
>> We already have type, why do we need these 2 fields a well?
>>
>> Actually, I tried massaging the patch a bit, could you please tell me if
>> the patch below still works for you?
>>
>> Thanks.
>>
>> --
>> Dmitry
>>
>>
>> Input: wacom_w8001 - add one finger touch support
>>
>> From: Ping Cheng <pinglinux@gmail.com>
>>
>> Signed-off-by: Ping Cheng <pingc@wacom.com>
>> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
>> ---
>>
>>  drivers/input/touchscreen/wacom_w8001.c |  123 +++++++++++++++++++++++++------
>>  1 files changed, 101 insertions(+), 22 deletions(-)
>>
>>
>> diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
>> index 4774c09..ce8c8e1 100644
>> --- a/drivers/input/touchscreen/wacom_w8001.c
>> +++ b/drivers/input/touchscreen/wacom_w8001.c
>> @@ -3,6 +3,7 @@
>>  *
>>  * Copyright (c) 2008 Jaya Kumar
>>  * Copyright (c) 2010 Red Hat, Inc.
>> + * Copyright (c) 2010 Ping Cheng, Wacom. <pingc@wacom.com>
>>  *
>>  * 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
>> @@ -63,11 +64,11 @@ struct w8001_coord {
>>
>>  /* touch query reply packet */
>>  struct w8001_touch_query {
>> +       u16 x;
>> +       u16 y;
>>        u8 panel_res;
>>        u8 capacity_res;
>>        u8 sensor_id;
>> -       u16 x;
>> -       u16 y;
>>  };
>>
>>  /*
>> @@ -86,9 +87,13 @@ struct w8001 {
>>        char phys[32];
>>        int type;
>>        unsigned int pktlen;
>> +       u16 max_touch_x;
>> +       u16 max_touch_y;
>> +       u16 max_pen_x;
>> +       u16 max_pen_y;
>>  };
>>
>> -static void parse_data(u8 *data, struct w8001_coord *coord)
>> +static void parse_pen_data(u8 *data, struct w8001_coord *coord)
>>  {
>>        memset(coord, 0, sizeof(*coord));
>>
>> @@ -112,7 +117,14 @@ static void parse_data(u8 *data, struct w8001_coord *coord)
>>        coord->tilt_y = data[8] & 0x7F;
>>  }
>>
>> -static void parse_touch(struct w8001 *w8001)
>> +static void parse_single_touch(u8 *data, struct w8001_coord *coord)
>> +{
>> +       coord->x = (data[1] << 7) | data[2];
>> +       coord->y = (data[3] << 7) | data[4];
>> +       coord->tsw = data[0] & 0x01;
>> +}
>> +
>> +static void parse_multi_touch(struct w8001 *w8001)
>>  {
>>        struct input_dev *dev = w8001->dev;
>>        unsigned char *data = w8001->data;
>> @@ -124,8 +136,8 @@ static void parse_touch(struct w8001 *w8001)
>>                input_mt_slot(dev, i);
>>                input_mt_report_slot_state(dev, MT_TOOL_FINGER, touch);
>>                if (touch) {
>> -                       int x = (data[6 * i + 1] << 7) | (data[6 * i + 2]);
>> -                       int y = (data[6 * i + 3] << 7) | (data[6 * i + 4]);
>> +                       int x = (data[6 * i + 1] << 7) | data[6 * i + 2];
>> +                       int y = (data[6 * i + 3] << 7) | data[6 * i + 4];
>
> Since you scaled MT max_touch_x/y to max_pen_x/y, you would need to
> scale the x and y here as well. I didn't scale MT touch data since MT
> protocol can report touch resolution on its own. See more comments
> below.
>
>>                        /* data[5,6] and [11,12] is finger capacity */
>>
>>                        input_report_abs(dev, ABS_MT_POSITION_X, x);
>> @@ -151,6 +163,15 @@ static void parse_touchquery(u8 *data, struct w8001_touch_query *query)
>>        query->y = data[5] << 9;
>>        query->y |= data[6] << 2;
>>        query->y |= (data[2] >> 3) & 0x3;
>> +
>> +       /* Early days' single-finger touch models need the following defaults */
>> +       if (!query->x && !query->y) {
>> +               query->x = 1024;
>> +               query->y = 1024;
>> +               if (query->panel_res)
>> +                       query->x = query->y = (1 << query->panel_res);
>> +               query->panel_res = 10;
>> +       }
>>  }
>>
>>  static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
>> @@ -160,16 +181,15 @@ static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
>>        /*
>>         * 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
>> +        * 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) {
>> +       switch (w8001->type) {
>> +       case BTN_TOOL_RUBBER:
>>                if (!coord->f2) {
>>                        input_report_abs(dev, ABS_PRESSURE, 0);
>>                        input_report_key(dev, BTN_TOUCH, 0);
>> @@ -179,8 +199,21 @@ static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
>>                        input_sync(dev);
>>                        w8001->type = BTN_TOOL_PEN;
>>                }
>> -       } else {
>> +               break;
>> +
>> +       case BTN_TOOL_FINGER:
>> +               input_report_key(dev, BTN_TOUCH, 0);
>> +               input_report_key(dev, BTN_TOOL_FINGER, 0);
>> +               input_sync(dev);
>> +               /* fall through */
>> +
>> +       case KEY_RESERVED:
>> +               w8001->type = coord->f2 ? BTN_TOOL_RUBBER : BTN_TOOL_PEN;
>> +               break;
>> +
>> +       default:
>>                input_report_key(dev, BTN_STYLUS2, coord->f2);
>> +               break;
>>        }
>>
>>        input_report_abs(dev, ABS_X, coord->x);
>> @@ -192,7 +225,30 @@ static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
>>        input_sync(dev);
>>
>>        if (!coord->rdy)
>> -               w8001->type = 0;
>> +               w8001->type = KEY_RESERVED;
>> +}
>> +
>> +static void report_single_touch(struct w8001 *w8001, struct w8001_coord *coord)
>> +{
>> +       struct input_dev *dev = w8001->dev;
>> +       unsigned int x = coord->x;
>> +       unsigned int y = coord->y;
>> +
>> +       /* scale to pen maximum */
>> +       if (w8001->max_pen_x && w8001->max_touch_x)
>> +               x = x * w8001->max_pen_x / w8001->max_touch_x;
>> +
>> +       if (w8001->max_pen_y && w8001->max_touch_y)
>> +               y = y * w8001->max_pen_y / w8001->max_touch_y;
>> +
>> +       input_report_abs(dev, ABS_X, x);
>> +       input_report_abs(dev, ABS_Y, y);
>> +       input_report_key(dev, BTN_TOUCH, coord->tsw);
>> +       input_report_key(dev, BTN_TOOL_FINGER, coord->tsw);
>> +
>> +       input_sync(dev);
>> +
>> +       w8001->type = coord->tsw ? BTN_TOOL_FINGER : KEY_RESERVED;
>>  }
>>
>>  static irqreturn_t w8001_interrupt(struct serio *serio,
>> @@ -213,9 +269,18 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
>>
>>        case W8001_PKTLEN_TOUCH93 - 1:
>>        case W8001_PKTLEN_TOUCH9A - 1:
>> -               /* ignore one-finger touch packet. */
>> -               if (w8001->pktlen == w8001->idx)
>> +               tmp = w8001->data[0] & W8001_TOUCH_BYTE;
>> +               if (tmp != W8001_TOUCH_BYTE)
>> +                       break;
>> +
>> +               if (w8001->pktlen == w8001->idx) {
>>                        w8001->idx = 0;
>> +                       if (w8001->type != BTN_TOOL_PEN &&
>> +                           w8001->type != BTN_TOOL_RUBBER) {
>> +                               parse_single_touch(w8001->data, &coord);
>> +                               report_single_touch(w8001, &coord);
>> +                       }
>> +               }
>>                break;
>>
>>        /* Pen coordinates packet */
>> @@ -224,18 +289,18 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
>>                if (unlikely(tmp == W8001_TAB_BYTE))
>>                        break;
>>
>> -               tmp = (w8001->data[0] & W8001_TOUCH_BYTE);
>> +               tmp = w8001->data[0] & W8001_TOUCH_BYTE;
>>                if (tmp == W8001_TOUCH_BYTE)
>>                        break;
>>
>>                w8001->idx = 0;
>> -               parse_data(w8001->data, &coord);
>> +               parse_pen_data(w8001->data, &coord);
>>                report_pen_events(w8001, &coord);
>>                break;
>>
>>        /* control packet */
>>        case W8001_PKTLEN_TPCCTL - 1:
>> -               tmp = (w8001->data[0] & W8001_TOUCH_MASK);
>> +               tmp = w8001->data[0] & W8001_TOUCH_MASK;
>>                if (tmp == W8001_TOUCH_BYTE)
>>                        break;
>>
>> @@ -248,7 +313,7 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
>>        /* 2 finger touch packet */
>>        case W8001_PKTLEN_TOUCH2FG - 1:
>>                w8001->idx = 0;
>> -               parse_touch(w8001);
>> +               parse_multi_touch(w8001);
>>                break;
>>        }
>>
>> @@ -278,6 +343,7 @@ static int w8001_setup(struct w8001 *w8001)
>>  {
>>        struct input_dev *dev = w8001->dev;
>>        struct w8001_coord coord;
>> +       struct w8001_touch_query touch;
>>        int error;
>>
>>        error = w8001_command(w8001, W8001_CMD_STOP, false);
>> @@ -289,11 +355,15 @@ static int w8001_setup(struct w8001 *w8001)
>>        /* penabled? */
>>        error = w8001_command(w8001, W8001_CMD_QUERY, true);
>>        if (!error) {
>> +               __set_bit(BTN_TOUCH, dev->keybit);
>>                __set_bit(BTN_TOOL_PEN, dev->keybit);
>>                __set_bit(BTN_TOOL_RUBBER, dev->keybit);
>>                __set_bit(BTN_STYLUS, dev->keybit);
>>                __set_bit(BTN_STYLUS2, dev->keybit);
>> -               parse_data(w8001->response, &coord);
>> +
>> +               parse_pen_data(w8001->response, &coord);
>> +               w8001->max_pen_x = coord.x;
>> +               w8001->max_pen_y = coord.y;
>>
>>                input_set_abs_params(dev, ABS_X, 0, coord.x, 0, 0);
>>                input_set_abs_params(dev, ABS_Y, 0, coord.y, 0, 0);
>> @@ -312,24 +382,34 @@ static int w8001_setup(struct w8001 *w8001)
>>         * second byte is empty, which indicates touch is not supported.
>>         */
>>        if (!error && w8001->response[1]) {
>> -               struct w8001_touch_query touch;
>> +               __set_bit(BTN_TOUCH, dev->keybit);
>> +               __set_bit(BTN_TOOL_FINGER, dev->keybit);
>
> I do not think we want to set BTN_TOUCH and BTN_TOOL_FINGER for MT
> device since we do not need to support single touch for this MT
> device. Please refer to my comments for your other patch in the next
> email.
>
>>                parse_touchquery(w8001->response, &touch);
>> +               w8001->max_touch_x = touch.x;
>> +               w8001->max_touch_y = touch.y;
>> +
>> +               /* scale to pen maximum */
>> +               if (w8001->max_pen_x && w8001->max_pen_y) {
>> +                       touch.x = w8001->max_pen_x;
>> +                       touch.y = w8001->max_pen_y;
>> +               }
>
> If we scale touch for both single and MT, we need to scale the raw
> (x,y) for both too. I purposely did not want to scale MT touch since
> they can be supported in different resolution from pen by MT protocol
> now.
>
>>
>>                input_set_abs_params(dev, ABS_X, 0, touch.x, 0, 0);
>>                input_set_abs_params(dev, ABS_Y, 0, touch.y, 0, 0);
>> -               __set_bit(BTN_TOOL_FINGER, dev->keybit);
>>
>>                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;
>>
>> @@ -397,7 +477,6 @@ static int w8001_connect(struct serio *serio, struct serio_driver *drv)
>>        input_dev->dev.parent = &serio->dev;
>>
>>        input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
>> -       __set_bit(BTN_TOUCH, input_dev->keybit);
>>
>>        serio_set_drvdata(serio, w8001);
>>        err = serio_open(serio, drv);
>>
>
--
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

* Re: [PATCH 3/3 v2] input - wacom_w8001: Add one finger touch support
  2010-12-29 22:30   ` Ping Cheng
  2010-12-30 18:25     ` Ping Cheng
@ 2011-01-02  8:56     ` Dmitry Torokhov
  1 sibling, 0 replies; 11+ messages in thread
From: Dmitry Torokhov @ 2011-01-02  8:56 UTC (permalink / raw)
  To: Ping Cheng; +Cc: linux-input

On Wed, Dec 29, 2010 at 02:30:24PM -0800, Ping Cheng wrote:
> Dmitry,
> 
> Thank you for your effort in updating the patch. Most changes work for
> me. I do have comments related to raw touch data scaling for MT and
> single touch legacy client support. Details are inline.
> 
> Ping
> 
> On Tue, Dec 28, 2010 at 11:40 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > Hi Ping,
> >
> > On Fri, Dec 17, 2010 at 09:37:54AM -0800, Ping Cheng wrote:
> >> Signed-off-by: Ping Cheng <pingc@wacom.com>
> >> ---
> >>  drivers/input/touchscreen/wacom_w8001.c |   89 ++++++++++++++++++++++++++++---
> >>  1 files changed, 82 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
> >> index 59664a8..763eb8f 100644
> >> --- a/drivers/input/touchscreen/wacom_w8001.c
> >> +++ b/drivers/input/touchscreen/wacom_w8001.c
> >> @@ -3,6 +3,7 @@
> >>   *
> >>   * Copyright (c) 2008 Jaya Kumar
> >>   * Copyright (c) 2010 Red Hat, Inc.
> >> + * Copyright (c) 2010 Ping Cheng, Wacom. <pingc@wacom.com>
> >>   *
> >>   * 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
> >> @@ -86,6 +87,12 @@ struct w8001 {
> >>       char phys[32];
> >>       int type;
> >>       unsigned int pktlen;
> >> +     bool pen_in_prox;
> >> +     bool has_touch;
> >
> > We already have type, why do we need these 2 fields a well?
> >
> > Actually, I tried massaging the patch a bit, could you please tell me if
> > the patch below still works for you?
> >
> > Thanks.
> >
> > --
> > Dmitry
> >
> >
> > Input: wacom_w8001 - add one finger touch support
> >
> > From: Ping Cheng <pinglinux@gmail.com>
> >
> > Signed-off-by: Ping Cheng <pingc@wacom.com>
> > Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> > ---
> >
> >  drivers/input/touchscreen/wacom_w8001.c |  123 +++++++++++++++++++++++++------
> >  1 files changed, 101 insertions(+), 22 deletions(-)
> >
> >
> > diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
> > index 4774c09..ce8c8e1 100644
> > --- a/drivers/input/touchscreen/wacom_w8001.c
> > +++ b/drivers/input/touchscreen/wacom_w8001.c
> > @@ -3,6 +3,7 @@
> >  *
> >  * Copyright (c) 2008 Jaya Kumar
> >  * Copyright (c) 2010 Red Hat, Inc.
> > + * Copyright (c) 2010 Ping Cheng, Wacom. <pingc@wacom.com>
> >  *
> >  * 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
> > @@ -63,11 +64,11 @@ struct w8001_coord {
> >
> >  /* touch query reply packet */
> >  struct w8001_touch_query {
> > +       u16 x;
> > +       u16 y;
> >        u8 panel_res;
> >        u8 capacity_res;
> >        u8 sensor_id;
> > -       u16 x;
> > -       u16 y;
> >  };
> >
> >  /*
> > @@ -86,9 +87,13 @@ struct w8001 {
> >        char phys[32];
> >        int type;
> >        unsigned int pktlen;
> > +       u16 max_touch_x;
> > +       u16 max_touch_y;
> > +       u16 max_pen_x;
> > +       u16 max_pen_y;
> >  };
> >
> > -static void parse_data(u8 *data, struct w8001_coord *coord)
> > +static void parse_pen_data(u8 *data, struct w8001_coord *coord)
> >  {
> >        memset(coord, 0, sizeof(*coord));
> >
> > @@ -112,7 +117,14 @@ static void parse_data(u8 *data, struct w8001_coord *coord)
> >        coord->tilt_y = data[8] & 0x7F;
> >  }
> >
> > -static void parse_touch(struct w8001 *w8001)
> > +static void parse_single_touch(u8 *data, struct w8001_coord *coord)
> > +{
> > +       coord->x = (data[1] << 7) | data[2];
> > +       coord->y = (data[3] << 7) | data[4];
> > +       coord->tsw = data[0] & 0x01;
> > +}
> > +
> > +static void parse_multi_touch(struct w8001 *w8001)
> >  {
> >        struct input_dev *dev = w8001->dev;
> >        unsigned char *data = w8001->data;
> > @@ -124,8 +136,8 @@ static void parse_touch(struct w8001 *w8001)
> >                input_mt_slot(dev, i);
> >                input_mt_report_slot_state(dev, MT_TOOL_FINGER, touch);
> >                if (touch) {
> > -                       int x = (data[6 * i + 1] << 7) | (data[6 * i + 2]);
> > -                       int y = (data[6 * i + 3] << 7) | (data[6 * i + 4]);
> > +                       int x = (data[6 * i + 1] << 7) | data[6 * i + 2];
> > +                       int y = (data[6 * i + 3] << 7) | data[6 * i + 4];
> 
> Since you scaled MT max_touch_x/y to max_pen_x/y, you would need to
> scale the x and y here as well. I didn't scale MT touch data since MT
> protocol can report touch resolution on its own. See more comments
> below.

Hmm, it depends on whether there are multitouch devices that are also
penabled. Are there such devices? If not then we won't actually scale
anyting.

> 
> >                        /* data[5,6] and [11,12] is finger capacity */
> >
> >                        input_report_abs(dev, ABS_MT_POSITION_X, x);
> > @@ -151,6 +163,15 @@ static void parse_touchquery(u8 *data, struct w8001_touch_query *query)
> >        query->y = data[5] << 9;
> >        query->y |= data[6] << 2;
> >        query->y |= (data[2] >> 3) & 0x3;
> > +
> > +       /* Early days' single-finger touch models need the following defaults */
> > +       if (!query->x && !query->y) {
> > +               query->x = 1024;
> > +               query->y = 1024;
> > +               if (query->panel_res)
> > +                       query->x = query->y = (1 << query->panel_res);
> > +               query->panel_res = 10;
> > +       }
> >  }
> >
> >  static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
> > @@ -160,16 +181,15 @@ static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
> >        /*
> >         * 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
> > +        * 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) {
> > +       switch (w8001->type) {
> > +       case BTN_TOOL_RUBBER:
> >                if (!coord->f2) {
> >                        input_report_abs(dev, ABS_PRESSURE, 0);
> >                        input_report_key(dev, BTN_TOUCH, 0);
> > @@ -179,8 +199,21 @@ static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
> >                        input_sync(dev);
> >                        w8001->type = BTN_TOOL_PEN;
> >                }
> > -       } else {
> > +               break;
> > +
> > +       case BTN_TOOL_FINGER:
> > +               input_report_key(dev, BTN_TOUCH, 0);
> > +               input_report_key(dev, BTN_TOOL_FINGER, 0);
> > +               input_sync(dev);
> > +               /* fall through */
> > +
> > +       case KEY_RESERVED:
> > +               w8001->type = coord->f2 ? BTN_TOOL_RUBBER : BTN_TOOL_PEN;
> > +               break;
> > +
> > +       default:
> >                input_report_key(dev, BTN_STYLUS2, coord->f2);
> > +               break;
> >        }
> >
> >        input_report_abs(dev, ABS_X, coord->x);
> > @@ -192,7 +225,30 @@ static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
> >        input_sync(dev);
> >
> >        if (!coord->rdy)
> > -               w8001->type = 0;
> > +               w8001->type = KEY_RESERVED;
> > +}
> > +
> > +static void report_single_touch(struct w8001 *w8001, struct w8001_coord *coord)
> > +{
> > +       struct input_dev *dev = w8001->dev;
> > +       unsigned int x = coord->x;
> > +       unsigned int y = coord->y;
> > +
> > +       /* scale to pen maximum */
> > +       if (w8001->max_pen_x && w8001->max_touch_x)
> > +               x = x * w8001->max_pen_x / w8001->max_touch_x;
> > +
> > +       if (w8001->max_pen_y && w8001->max_touch_y)
> > +               y = y * w8001->max_pen_y / w8001->max_touch_y;
> > +
> > +       input_report_abs(dev, ABS_X, x);
> > +       input_report_abs(dev, ABS_Y, y);
> > +       input_report_key(dev, BTN_TOUCH, coord->tsw);
> > +       input_report_key(dev, BTN_TOOL_FINGER, coord->tsw);
> > +
> > +       input_sync(dev);
> > +
> > +       w8001->type = coord->tsw ? BTN_TOOL_FINGER : KEY_RESERVED;
> >  }
> >
> >  static irqreturn_t w8001_interrupt(struct serio *serio,
> > @@ -213,9 +269,18 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
> >
> >        case W8001_PKTLEN_TOUCH93 - 1:
> >        case W8001_PKTLEN_TOUCH9A - 1:
> > -               /* ignore one-finger touch packet. */
> > -               if (w8001->pktlen == w8001->idx)
> > +               tmp = w8001->data[0] & W8001_TOUCH_BYTE;
> > +               if (tmp != W8001_TOUCH_BYTE)
> > +                       break;
> > +
> > +               if (w8001->pktlen == w8001->idx) {
> >                        w8001->idx = 0;
> > +                       if (w8001->type != BTN_TOOL_PEN &&
> > +                           w8001->type != BTN_TOOL_RUBBER) {
> > +                               parse_single_touch(w8001->data, &coord);
> > +                               report_single_touch(w8001, &coord);
> > +                       }
> > +               }
> >                break;
> >
> >        /* Pen coordinates packet */
> > @@ -224,18 +289,18 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
> >                if (unlikely(tmp == W8001_TAB_BYTE))
> >                        break;
> >
> > -               tmp = (w8001->data[0] & W8001_TOUCH_BYTE);
> > +               tmp = w8001->data[0] & W8001_TOUCH_BYTE;
> >                if (tmp == W8001_TOUCH_BYTE)
> >                        break;
> >
> >                w8001->idx = 0;
> > -               parse_data(w8001->data, &coord);
> > +               parse_pen_data(w8001->data, &coord);
> >                report_pen_events(w8001, &coord);
> >                break;
> >
> >        /* control packet */
> >        case W8001_PKTLEN_TPCCTL - 1:
> > -               tmp = (w8001->data[0] & W8001_TOUCH_MASK);
> > +               tmp = w8001->data[0] & W8001_TOUCH_MASK;
> >                if (tmp == W8001_TOUCH_BYTE)
> >                        break;
> >
> > @@ -248,7 +313,7 @@ static irqreturn_t w8001_interrupt(struct serio *serio,
> >        /* 2 finger touch packet */
> >        case W8001_PKTLEN_TOUCH2FG - 1:
> >                w8001->idx = 0;
> > -               parse_touch(w8001);
> > +               parse_multi_touch(w8001);
> >                break;
> >        }
> >
> > @@ -278,6 +343,7 @@ static int w8001_setup(struct w8001 *w8001)
> >  {
> >        struct input_dev *dev = w8001->dev;
> >        struct w8001_coord coord;
> > +       struct w8001_touch_query touch;
> >        int error;
> >
> >        error = w8001_command(w8001, W8001_CMD_STOP, false);
> > @@ -289,11 +355,15 @@ static int w8001_setup(struct w8001 *w8001)
> >        /* penabled? */
> >        error = w8001_command(w8001, W8001_CMD_QUERY, true);
> >        if (!error) {
> > +               __set_bit(BTN_TOUCH, dev->keybit);
> >                __set_bit(BTN_TOOL_PEN, dev->keybit);
> >                __set_bit(BTN_TOOL_RUBBER, dev->keybit);
> >                __set_bit(BTN_STYLUS, dev->keybit);
> >                __set_bit(BTN_STYLUS2, dev->keybit);
> > -               parse_data(w8001->response, &coord);
> > +
> > +               parse_pen_data(w8001->response, &coord);
> > +               w8001->max_pen_x = coord.x;
> > +               w8001->max_pen_y = coord.y;
> >
> >                input_set_abs_params(dev, ABS_X, 0, coord.x, 0, 0);
> >                input_set_abs_params(dev, ABS_Y, 0, coord.y, 0, 0);
> > @@ -312,24 +382,34 @@ static int w8001_setup(struct w8001 *w8001)
> >         * second byte is empty, which indicates touch is not supported.
> >         */
> >        if (!error && w8001->response[1]) {
> > -               struct w8001_touch_query touch;
> > +               __set_bit(BTN_TOUCH, dev->keybit);
> > +               __set_bit(BTN_TOOL_FINGER, dev->keybit);
> 
> I do not think we want to set BTN_TOUCH and BTN_TOOL_FINGER for MT
> device since we do not need to support single touch for this MT
> device. Please refer to my comments for your other patch in the next
> email.

I guess for touchscreens we may say that we may omit BTN_TOOL_FINGER if
finger is the only "tool" that is supported, to keep in line with
current touchscreen drivers. BTN_TOUCH is needed for legacy.

> 
> >                parse_touchquery(w8001->response, &touch);
> > +               w8001->max_touch_x = touch.x;
> > +               w8001->max_touch_y = touch.y;
> > +
> > +               /* scale to pen maximum */
> > +               if (w8001->max_pen_x && w8001->max_pen_y) {
> > +                       touch.x = w8001->max_pen_x;
> > +                       touch.y = w8001->max_pen_y;
> > +               }
> 
> If we scale touch for both single and MT, we need to scale the raw
> (x,y) for both too. I purposely did not want to scale MT touch since
> they can be supported in different resolution from pen by MT protocol
> now.

I am really not sure if this is good thing that MT has it's own
min/max/resolution. It is more implementation atrifact that it has...
My gut feelig is that we shoudl keep it in sync with ST values,
especially if we want to perform pointer emulation.

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

* Re: [PATCH 3/3 v2] input - wacom_w8001: Add one finger touch support
  2010-12-29 22:54     ` Ping Cheng
@ 2011-01-02  8:59       ` Dmitry Torokhov
  0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Torokhov @ 2011-01-02  8:59 UTC (permalink / raw)
  To: Ping Cheng; +Cc: linux-input

On Wed, Dec 29, 2010 at 02:54:14PM -0800, Ping Cheng wrote:
> On Tue, Dec 28, 2010 at 11:42 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Tue, Dec 28, 2010 at 11:40:58PM -0800, Dmitry Torokhov wrote:
> >> Hi Ping,
> >>
> >> On Fri, Dec 17, 2010 at 09:37:54AM -0800, Ping Cheng wrote:
> >> > Signed-off-by: Ping Cheng <pingc@wacom.com>
> >> > ---
> >> >  drivers/input/touchscreen/wacom_w8001.c |   89 ++++++++++++++++++++++++++++---
> >> >  1 files changed, 82 insertions(+), 7 deletions(-)
> >> >
> >> > diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
> >> > index 59664a8..763eb8f 100644
> >> > --- a/drivers/input/touchscreen/wacom_w8001.c
> >> > +++ b/drivers/input/touchscreen/wacom_w8001.c
> >> > @@ -3,6 +3,7 @@
> >> >   *
> >> >   * Copyright (c) 2008 Jaya Kumar
> >> >   * Copyright (c) 2010 Red Hat, Inc.
> >> > + * Copyright (c) 2010 Ping Cheng, Wacom. <pingc@wacom.com>
> >> >   *
> >> >   * 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
> >> > @@ -86,6 +87,12 @@ struct w8001 {
> >> >     char phys[32];
> >> >     int type;
> >> >     unsigned int pktlen;
> >> > +   bool pen_in_prox;
> >> > +   bool has_touch;
> >>
> >> We already have type, why do we need these 2 fields a well?
> >>
> >> Actually, I tried massaging the patch a bit, could you please tell me if
> >> the patch below still works for you?
> >>
> >> Thanks.
> >>
> >
> > And while we are at it could you please try this patch as well...
> >
> > Thank you!
> >
> > --
> > Dmitry
> >
> > Input: wacom_w8001 - add single-touch pointer emulation
> >
> > Let's emit single-touch compatible events for the 2-finger panels so that
> > they can be used with legacy clients.
> 
> Depending on which legacy clients we are talking about, this patch may
> introduce issue instead of support. We added
> mt_report_pointer_emulation for Bamboo since it is a touchpad, that
> can be driven by xf86-input-synaptics in relative mode. This device is
> a touchscreen, that should be run in absolute mode. If we emulate ST
> events, which X driver do we expect it to use? xf86-input-wacom does
> not process emulated single touch data once we know MT is supported.
> That's why I didn't want to add ST emulation for this device.
> 
> We expect clients that support this MT device will be in MT format
> since this driver is new and the clients have to run inputattach to
> enable the port before they can talk to the device. The client must
> have a reason to do so. Do you see any legacy clients would like to
> walk the extra mile to use this driver?
> 

Anything that is using mousedev / PS/2 protocol can use this device if
we provide ST emulation. It won't be pretty but some level of
fucntionality will be available which is handy for bootsrapping.

Thanks.

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

* Re: [PATCH 3/3 v2] input - wacom_w8001: Add one finger touch support
  2010-12-30 18:25     ` Ping Cheng
@ 2011-01-02  9:01       ` Dmitry Torokhov
  2011-01-03  4:59         ` Ping Cheng
  0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Torokhov @ 2011-01-02  9:01 UTC (permalink / raw)
  To: Ping Cheng; +Cc: linux-input

On Thu, Dec 30, 2010 at 10:25:20AM -0800, Ping Cheng wrote:
> On Wed, Dec 29, 2010 at 2:30 PM, Ping Cheng <pinglinux@gmail.com> wrote:
> > Dmitry,
> >
> > Thank you for your effort in updating the patch. Most changes work for
> > me. I do have comments related to raw touch data scaling for MT and
> > single touch legacy client support. Details are inline.
> 
> After revisited the change you made, I think I can accept both
> suggestions. The legacy client support may introduce issue. But it
> won't hurt too much, I hope ;).  The MT scaling offers a consistent
> support between one and two finger touch, which is good.
> 
> So, the only change left is to scale the raw x,y of 2FGT to max_pen. I
> can make a version 3 of this patch for you or you can change those two
> lines if you don't mind. Just let me know.

If there are such devices that have pen + 2FGT then please adjust the
patch as you should be able to test it ;)

Also, if you [re]send me inputattach part for 8001 I shoudl try adding
it to inputattach (it's not there yet, is it?).

-- 
Dmitry

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

* Re: [PATCH 3/3 v2] input - wacom_w8001: Add one finger touch support
  2011-01-02  9:01       ` Dmitry Torokhov
@ 2011-01-03  4:59         ` Ping Cheng
  2011-01-03  6:33           ` Dmitry Torokhov
  0 siblings, 1 reply; 11+ messages in thread
From: Ping Cheng @ 2011-01-03  4:59 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input

On Sun, Jan 2, 2011 at 1:01 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Thu, Dec 30, 2010 at 10:25:20AM -0800, Ping Cheng wrote:
>> On Wed, Dec 29, 2010 at 2:30 PM, Ping Cheng <pinglinux@gmail.com> wrote:
>> > Dmitry,
>> >
>> > Thank you for your effort in updating the patch. Most changes work for
>> > me. I do have comments related to raw touch data scaling for MT and
>> > single touch legacy client support. Details are inline.
>>
>> After revisited the change you made, I think I can accept both
>> suggestions. The legacy client support may introduce issue. But it
>> won't hurt too much, I hope ;).  The MT scaling offers a consistent
>> support between one and two finger touch, which is good.
>>
>> So, the only change left is to scale the raw x,y of 2FGT to max_pen. I
>> can make a version 3 of this patch for you or you can change those two
>> lines if you don't mind. Just let me know.
>
> If there are such devices that have pen + 2FGT then please adjust the
> patch as you should be able to test it ;)

Yes, there are pen + 2FGT devices in the market now. Will update the patch soon.

> Also, if you [re]send me inputattach part for 8001 I shoudl try adding
> it to inputattach (it's not there yet, is it?).

No, inputattach is not part of this patchset. I am not sure which
inputattach.c source my patch should be based on. The one under your
repo looks a bit old....

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

* Re: [PATCH 3/3 v2] input - wacom_w8001: Add one finger touch support
  2011-01-03  4:59         ` Ping Cheng
@ 2011-01-03  6:33           ` Dmitry Torokhov
  0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Torokhov @ 2011-01-03  6:33 UTC (permalink / raw)
  To: Ping Cheng; +Cc: linux-input

On Sun, Jan 02, 2011 at 08:59:39PM -0800, Ping Cheng wrote:
> On Sun, Jan 2, 2011 at 1:01 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Thu, Dec 30, 2010 at 10:25:20AM -0800, Ping Cheng wrote:
> >> On Wed, Dec 29, 2010 at 2:30 PM, Ping Cheng <pinglinux@gmail.com> wrote:
> >> > Dmitry,
> >> >
> >> > Thank you for your effort in updating the patch. Most changes work for
> >> > me. I do have comments related to raw touch data scaling for MT and
> >> > single touch legacy client support. Details are inline.
> >>
> >> After revisited the change you made, I think I can accept both
> >> suggestions. The legacy client support may introduce issue. But it
> >> won't hurt too much, I hope ;).  The MT scaling offers a consistent
> >> support between one and two finger touch, which is good.
> >>
> >> So, the only change left is to scale the raw x,y of 2FGT to max_pen. I
> >> can make a version 3 of this patch for you or you can change those two
> >> lines if you don't mind. Just let me know.
> >
> > If there are such devices that have pen + 2FGT then please adjust the
> > patch as you should be able to test it ;)
> 
> Yes, there are pen + 2FGT devices in the market now. Will update the patch soon.
> 
> > Also, if you [re]send me inputattach part for 8001 I shoudl try adding
> > it to inputattach (it's not there yet, is it?).
> 
> No, inputattach is not part of this patchset. I am not sure which
> inputattach.c source my patch should be based on. The one under your
> repo looks a bit old....
> 

The one on the sourceforge should be recent.

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

end of thread, other threads:[~2011-01-03  6:33 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-17 17:37 [PATCH 3/3 v2] input - wacom_w8001: Add one finger touch support Ping Cheng
2010-12-29  7:40 ` Dmitry Torokhov
2010-12-29  7:42   ` Dmitry Torokhov
2010-12-29 22:54     ` Ping Cheng
2011-01-02  8:59       ` Dmitry Torokhov
2010-12-29 22:30   ` Ping Cheng
2010-12-30 18:25     ` Ping Cheng
2011-01-02  9:01       ` Dmitry Torokhov
2011-01-03  4:59         ` Ping Cheng
2011-01-03  6:33           ` Dmitry Torokhov
2011-01-02  8:56     ` Dmitry Torokhov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).