Input: wacom_w8001 - use __set_bit for evbits

PATCH In review Peter Hutterer 2 participants 12 messages Nov 30, 2015 – Dec 3, 2015

Subsystems (2)

HID Input drivers

lore

Share & tools

lore

Peter Hutterer

author

2015-11-30 03:50 UTC #0 lore raw

PATCH 1/5 Input: wacom_w8001 - use __set_bit for evbits

Signed-off-by Peter Hutterer <[email protected]>

Acked-by Benjamin Tissoires <[email protected]>

---
 drivers/input/touchscreen/wacom_w8001.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/input/touchscreen/wacom_w8001.c +2 -1

index 2792ca3..d194d57 100644
@@ -393,7 +393,8 @@ static int w8001_setup(struct w8001 *w8001)
 
 	msleep(250);	/* wait 250ms before querying the device */
 
-	dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
+	__set_bit(EV_KEY, dev->evbit);
+	__set_bit(EV_ABS, dev->evbit);
 	strlcat(w8001->name, "Wacom Serial", sizeof(w8001->name));
 
 	__set_bit(INPUT_PROP_DIRECT, dev->propbit);

Signature · 1 line

2.5.0

Peter Hutterer

author

2015-11-30 03:50 UTC #1 lore raw

PATCH 2/5 Input: wacom_w8001 - set BTN_TOOL_DOUBLETAP if we have 2fg support

Signed-off-by Peter Hutterer <[email protected]>

Acked-by Benjamin Tissoires <[email protected]>

---
 drivers/input/touchscreen/wacom_w8001.c | 1 +
 1 file changed, 1 insertion(+)

drivers/input/touchscreen/wacom_w8001.c +1 -0

index d194d57..222006e 100644
@@ -471,6 +471,7 @@ static int w8001_setup(struct w8001 *w8001)
 		case 5:
 			w8001->pktlen = W8001_PKTLEN_TOUCH2FG;
 
+			__set_bit(BTN_TOOL_DOUBLETAP, dev->keybit);
 			input_mt_init_slots(dev, 2, 0);
 			input_set_abs_params(dev, ABS_MT_POSITION_X,
 						0, touch.x, 0, 0);

Signature · 1 line

2.5.0

Peter Hutterer

author

2015-11-30 03:50 UTC #2 lore raw

PATCH 3/5 Input: wacom_w8001 - handle touch error case correctly

In reply to Peter Hutterer

If a device failed at the pen setup and gets a zero reply from the touch device, we need to return an error. Otherwise we have a device with nothing but a name and the EV_KEY and EV_ABS bits.

Signed-off-by Peter Hutterer <[email protected]>

Acked-by Benjamin Tissoires <[email protected]>

---
 drivers/input/touchscreen/wacom_w8001.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

drivers/input/touchscreen/wacom_w8001.c +8 -6

index 222006e..cdebf8e 100644
@@ -385,7 +385,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;
+	int error, err_pen, err_touch;
 
 	error = w8001_command(w8001, W8001_CMD_STOP, false);
 	if (error)

@@ -400,8 +400,8 @@ static int w8001_setup(struct w8001 *w8001)
 	__set_bit(INPUT_PROP_DIRECT, dev->propbit);
 
 	/* penabled? */
-	error = w8001_command(w8001, W8001_CMD_QUERY, true);
-	if (!error) {
+	err_pen = w8001_command(w8001, W8001_CMD_QUERY, true);
+	if (!err_pen) {
 		__set_bit(BTN_TOUCH, dev->keybit);
 		__set_bit(BTN_TOOL_PEN, dev->keybit);
 		__set_bit(BTN_TOOL_RUBBER, dev->keybit);

@@ -426,13 +426,15 @@ static int w8001_setup(struct w8001 *w8001)
 	}
 
 	/* Touch enabled? */
-	error = w8001_command(w8001, W8001_CMD_TOUCHQUERY, true);
+	err_touch = w8001_command(w8001, W8001_CMD_TOUCHQUERY, true);
 
 	/*
 	 * Some non-touch devices may reply to the touch query. But their
 	 * second byte is empty, which indicates touch is not supported.
 	 */
-	if (!error && w8001->response[1]) {
+	if (!err_touch && w8001->response[1]) {
+		err_touch = 1;
+
 		__set_bit(BTN_TOUCH, dev->keybit);
 		__set_bit(BTN_TOOL_FINGER, dev->keybit);

@@ -491,7 +493,7 @@ static int w8001_setup(struct w8001 *w8001)
 
 	strlcat(w8001->name, " Touchscreen", sizeof(w8001->name));
 
-	return 0;
+	return !err_pen || !err_touch;
 }
 
 /*

Signature · 1 line

2.5.0

Dmitry Torokhov

maintainer

2015-12-01 22:02 UTC #3 lore raw

Re: [PATCH 3/5] Input: wacom_w8001 - handle touch error case correctly

Quoted 3 lines · Peter Hutterer

If a device failed at the pen setup and gets a zero reply from the touch device, we need to return an error. Otherwise we have a device with nothing but a name and the EV_KEY and EV_ABS bits.

But if device does not fail pen setup (err_pen == 0) we should not return error, which you will !err_pen || !err_touch will be 1.

Additionally we better return proper error codes to the upper layer instead of a boolean value.

Quoted 35 lines

Signed-off-by Peter Hutterer <[email protected]>

Acked-by Benjamin Tissoires <[email protected]>

---
 drivers/input/touchscreen/wacom_w8001.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

drivers/input/touchscreen/wacom_w8001.c +3 -3

index 222006e..cdebf8e 100644
@@ -385,7 +385,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;
+	int error, err_pen, err_touch;
 
 	error = w8001_command(w8001, W8001_CMD_STOP, false);
 	if (error)

@@ -400,8 +400,8 @@ static int w8001_setup(struct w8001 *w8001)
 	__set_bit(INPUT_PROP_DIRECT, dev->propbit);
 
 	/* penabled? */
-	error = w8001_command(w8001, W8001_CMD_QUERY, true);
-	if (!error) {
+	err_pen = w8001_command(w8001, W8001_CMD_QUERY, true);
+	if (!err_pen) {
 		__set_bit(BTN_TOUCH, dev->keybit);
 		__set_bit(BTN_TOOL_PEN, dev->keybit);
 		__set_bit(BTN_TOOL_RUBBER, dev->keybit);

@@ -426,13 +426,15 @@ static int w8001_setup(struct w8001 *w8001)
 	}
 
 	/* Touch enabled? */

drivers/input/touchscreen/wacom_w8001.c static int w8001_setup(struct w8001 *w8001) · line 436

Quoted 10 lines

drivers/input/touchscreen/wacom_w8001.c +3 -2

-	error = w8001_command(w8001, W8001_CMD_TOUCHQUERY, true);
+	err_touch = w8001_command(w8001, W8001_CMD_TOUCHQUERY, true);
 
 	/*
 	 * Some non-touch devices may reply to the touch query. But their
 	 * second byte is empty, which indicates touch is not supported.
 	 */
-	if (!error && w8001->response[1]) {
+	if (!err_touch && w8001->response[1]) {
+		err_touch = 1;

Huh? Let's do:

	if (!err_touch && !w8001->response[1])
		err_touch = -ENXIO;

	if (!err_touch) {
		...
	}

drivers/input/touchscreen/wacom_w8001.c static int w8001_setup(struct w8001 *w8001) · line 496

Quoted 10 lines

drivers/input/touchscreen/wacom_w8001.c +2 -1

+
 		__set_bit(BTN_TOUCH, dev->keybit);
 		__set_bit(BTN_TOOL_FINGER, dev->keybit);

@@ -491,7 +493,7 @@ static int w8001_setup(struct w8001 *w8001)
 
 	strlcat(w8001->name, " Touchscreen", sizeof(w8001->name));
 
-	return 0;
+	return !err_pen || !err_touch;

	return !err_pen || !err_touch ? 0 : -ENXIO;

Quoted 6 lines

drivers/input/touchscreen/wacom_w8001.c +0 -0
 }
 
 /*
Signature · 1 line
2.5.0

Thanks.

Signature · 1 line

Dmitry

Peter Hutterer

author

2015-12-03 04:21 UTC #4 lore raw

PATCH v2 3/5 Input: wacom_w8001 - handle touch error case correctly

If a device failed at the pen setup and gets a zero reply from the touch device, we need to return an error. Otherwise we have a device with nothing but a name and the EV_KEY and EV_ABS bits.

Signed-off-by Peter Hutterer <[email protected]>

Acked-by Benjamin Tissoires <[email protected]>

---
Change to v1:
- return -ENXIO if the first byte is empty
- fix return statement to only return an error if both fail

 drivers/input/touchscreen/wacom_w8001.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

drivers/input/touchscreen/wacom_w8001.c +9 ~~-10~~

index 222006e..9ea5f82 100644
@@ -385,7 +385,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;
+	int error, err_pen, err_touch;
 
 	error = w8001_command(w8001, W8001_CMD_STOP, false);
 	if (error)

@@ -400,8 +400,8 @@ static int w8001_setup(struct w8001 *w8001)
 	__set_bit(INPUT_PROP_DIRECT, dev->propbit);
 
 	/* penabled? */
-	error = w8001_command(w8001, W8001_CMD_QUERY, true);
-	if (!error) {
+	err_pen = w8001_command(w8001, W8001_CMD_QUERY, true);
+	if (!err_pen) {
 		__set_bit(BTN_TOUCH, dev->keybit);
 		__set_bit(BTN_TOOL_PEN, dev->keybit);
 		__set_bit(BTN_TOOL_RUBBER, dev->keybit);

@@ -426,13 +426,12 @@ static int w8001_setup(struct w8001 *w8001)
 	}
 
 	/* Touch enabled? */
-	error = w8001_command(w8001, W8001_CMD_TOUCHQUERY, true);
+	err_touch = w8001_command(w8001, W8001_CMD_TOUCHQUERY, true);
 
-	/*
-	 * Some non-touch devices may reply to the touch query. But their
-	 * second byte is empty, which indicates touch is not supported.
-	 */
-	if (!error && w8001->response[1]) {
+	if (!err_touch && !w8001->response[1])
+		err_touch = -ENXIO;
+
+	if (!err_touch) {
 		__set_bit(BTN_TOUCH, dev->keybit);
 		__set_bit(BTN_TOOL_FINGER, dev->keybit);

@@ -491,7 +490,7 @@ static int w8001_setup(struct w8001 *w8001)
 
 	strlcat(w8001->name, " Touchscreen", sizeof(w8001->name));
 
-	return 0;
+	return !err_pen || !err_touch ? 0 : -ENXIO;
 }
 
 /*

Signature · 1 line

2.5.0

Peter Hutterer

author

2015-12-03 04:22 UTC #5 lore raw

PATCH v2 4/5 Input: wacom_w8001 - split pen and touch initialization up

In reply to Dmitry Torokhov

This is preparation work for splitting it up for two event nodes.

The error handling for the touch init failure must be special-cased, a device may respond to a touch query with a zero reply. In this case we don't have a touch device, but we don't have a true failure either. Returning 1 as special value here so we can then restore the true error code on failure.

Signed-off-by Peter Hutterer <[email protected]>

Acked-by Benjamin Tissoires <[email protected]>

---
Change to v1:
- return -ENXIO if both pen and touch failed to initialize
- rebased for v2 of 3/5

 drivers/input/touchscreen/wacom_w8001.c | 204 ++++++++++++++++++--------------
 1 file changed, 116 insertions(+), 88 deletions(-)

drivers/input/touchscreen/wacom_w8001.c +116 ~~-88~~

index 9ea5f82..de8f0e4 100644
@@ -380,12 +380,10 @@ static void w8001_close(struct input_dev *dev)
 	w8001_command(w8001, W8001_CMD_STOP, false);
 }
 
-static int w8001_setup(struct w8001 *w8001)
+static int w8001_detect(struct w8001 *w8001)
 {
 	struct input_dev *dev = w8001->dev;
-	struct w8001_coord coord;
-	struct w8001_touch_query touch;
-	int error, err_pen, err_touch;
+	int error;
 
 	error = w8001_command(w8001, W8001_CMD_STOP, false);
 	if (error)

@@ -399,98 +397,121 @@ static int w8001_setup(struct w8001 *w8001)
 
 	__set_bit(INPUT_PROP_DIRECT, dev->propbit);
 
+	return 0;
+}
+
+static int w8001_setup_pen(struct w8001 *w8001)
+{
+	struct input_dev *dev = w8001->dev;
+	struct w8001_coord coord;
+	int error;
+
 	/* penabled? */
-	err_pen = w8001_command(w8001, W8001_CMD_QUERY, true);
-	if (!err_pen) {
-		__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);
+	error = w8001_command(w8001, W8001_CMD_QUERY, true);
+	if (error)
+		return error;
 
-		parse_pen_data(w8001->response, &coord);
-		w8001->max_pen_x = coord.x;
-		w8001->max_pen_y = coord.y;
+	__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);
 
-		input_set_abs_params(dev, ABS_X, 0, coord.x, 0, 0);
-		input_set_abs_params(dev, ABS_Y, 0, coord.y, 0, 0);
-		input_abs_set_res(dev, ABS_X, W8001_PEN_RESOLUTION);
-		input_abs_set_res(dev, ABS_Y, W8001_PEN_RESOLUTION);
-		input_set_abs_params(dev, ABS_PRESSURE, 0, coord.pen_pressure, 0, 0);
-		if (coord.tilt_x && coord.tilt_y) {
-			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->id = 0x90;
-		strlcat(w8001->name, " Penabled", sizeof(w8001->name));
+	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);
+	input_abs_set_res(dev, ABS_X, W8001_PEN_RESOLUTION);
+	input_abs_set_res(dev, ABS_Y, W8001_PEN_RESOLUTION);
+	input_set_abs_params(dev, ABS_PRESSURE, 0, coord.pen_pressure, 0, 0);
+	if (coord.tilt_x && coord.tilt_y) {
+		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->id = 0x90;
+	strlcat(w8001->name, " Penabled", sizeof(w8001->name));
+
+	return 0;
+}
+
+static int w8001_setup_touch(struct w8001 *w8001)
+{
+	struct input_dev *dev = w8001->dev;
+	struct w8001_touch_query touch;
+	int error;
+
 	/* Touch enabled? */
-	err_touch = w8001_command(w8001, W8001_CMD_TOUCHQUERY, true);
-
-	if (!err_touch && !w8001->response[1])
-		err_touch = -ENXIO;
-
-	if (!err_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;
-
-		if (w8001->max_pen_x && w8001->max_pen_y) {
-			/* if pen is supported scale to pen maximum */
-			touch.x = w8001->max_pen_x;
-			touch.y = w8001->max_pen_y;
-			touch.panel_res = W8001_PEN_RESOLUTION;
-		}
-
-		input_set_abs_params(dev, ABS_X, 0, touch.x, 0, 0);
-		input_set_abs_params(dev, ABS_Y, 0, touch.y, 0, 0);
-		input_abs_set_res(dev, ABS_X, touch.panel_res);
-		input_abs_set_res(dev, ABS_Y, touch.panel_res);
-
-		switch (touch.sensor_id) {
-		case 0:
-		case 2:
-			w8001->pktlen = W8001_PKTLEN_TOUCH93;
-			w8001->id = 0x93;
-			strlcat(w8001->name, " 1FG", sizeof(w8001->name));
-			break;
-
-		case 1:
-		case 3:
-		case 4:
-			w8001->pktlen = W8001_PKTLEN_TOUCH9A;
-			strlcat(w8001->name, " 1FG", sizeof(w8001->name));
-			w8001->id = 0x9a;
-			break;
-
-		case 5:
-			w8001->pktlen = W8001_PKTLEN_TOUCH2FG;
-
-			__set_bit(BTN_TOOL_DOUBLETAP, dev->keybit);
-			input_mt_init_slots(dev, 2, 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, MT_TOOL_MAX, 0, 0);
-
-			strlcat(w8001->name, " 2FG", sizeof(w8001->name));
-			if (w8001->max_pen_x && w8001->max_pen_y)
-				w8001->id = 0xE3;
-			else
-				w8001->id = 0xE2;
-			break;
-		}
+	error = w8001_command(w8001, W8001_CMD_TOUCHQUERY, true);
+	if (error)
+		return error;
+	/*
+	 * Some non-touch devices may reply to the touch query. But their
+	 * second byte is empty, which indicates touch is not supported.
+	 */
+	if (!w8001->response[1])
+		return -ENXIO;
+
+	__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;
+
+	if (w8001->max_pen_x && w8001->max_pen_y) {
+		/* if pen is supported scale to pen maximum */
+		touch.x = w8001->max_pen_x;
+		touch.y = w8001->max_pen_y;
+		touch.panel_res = W8001_PEN_RESOLUTION;
+	}
+
+	input_set_abs_params(dev, ABS_X, 0, touch.x, 0, 0);
+	input_set_abs_params(dev, ABS_Y, 0, touch.y, 0, 0);
+	input_abs_set_res(dev, ABS_X, touch.panel_res);
+	input_abs_set_res(dev, ABS_Y, touch.panel_res);
+
+	switch (touch.sensor_id) {
+	case 0:
+	case 2:
+		w8001->pktlen = W8001_PKTLEN_TOUCH93;
+		w8001->id = 0x93;
+		strlcat(w8001->name, " 1FG", sizeof(w8001->name));
+		break;
+
+	case 1:
+	case 3:
+	case 4:
+		w8001->pktlen = W8001_PKTLEN_TOUCH9A;
+		strlcat(w8001->name, " 1FG", sizeof(w8001->name));
+		w8001->id = 0x9a;
+		break;
+
+	case 5:
+		w8001->pktlen = W8001_PKTLEN_TOUCH2FG;
+
+		__set_bit(BTN_TOOL_DOUBLETAP, dev->keybit);
+		input_mt_init_slots(dev, 2, 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, MT_TOOL_MAX, 0, 0);
+
+		strlcat(w8001->name, " 2FG", sizeof(w8001->name));
+		if (w8001->max_pen_x && w8001->max_pen_y)
+			w8001->id = 0xE3;
+		else
+			w8001->id = 0xE2;
+		break;
 	}
 
 	strlcat(w8001->name, " Touchscreen", sizeof(w8001->name));
 
-	return !err_pen || !err_touch ? 0 : -ENXIO;
+	return 0;
 }
 
 /*

@@ -519,7 +540,7 @@ static int w8001_connect(struct serio *serio, struct serio_driver *drv)
 {
 	struct w8001 *w8001;
 	struct input_dev *input_dev;
-	int err;
+	int err, err_pen, err_touch;
 
 	w8001 = kzalloc(sizeof(struct w8001), GFP_KERNEL);
 	input_dev = input_allocate_device();

@@ -538,10 +559,17 @@ static int w8001_connect(struct serio *serio, struct serio_driver *drv)
 	if (err)
 		goto fail2;
 
-	err = w8001_setup(w8001);
+	err = w8001_detect(w8001);
 	if (err)
 		goto fail3;
 
+	err_pen = w8001_setup_pen(w8001);
+	err_touch = w8001_setup_touch(w8001);
+	if (err_pen && err_touch) {
+		err = -ENXIO;
+		goto fail3;
+	}
+
 	input_dev->name = w8001->name;
 	input_dev->phys = w8001->phys;
 	input_dev->id.product = w8001->id;

Signature · 1 line

2.5.0

Peter Hutterer

author

2015-11-30 03:50 UTC #6 lore raw

PATCH 4/5 Input: wacom_w8001 - split pen and touch initialization up

In reply to Peter Hutterer

This is preparation work for splitting it up for two event nodes.

Signed-off-by Peter Hutterer <[email protected]>

Acked-by Benjamin Tissoires <[email protected]>

---
 drivers/input/touchscreen/wacom_w8001.c | 198 ++++++++++++++++++--------------
 1 file changed, 112 insertions(+), 86 deletions(-)

drivers/input/touchscreen/wacom_w8001.c +112 ~~-86~~

index cdebf8e..363e510 100644
@@ -380,12 +380,10 @@ static void w8001_close(struct input_dev *dev)
 	w8001_command(w8001, W8001_CMD_STOP, false);
 }
 
-static int w8001_setup(struct w8001 *w8001)
+static int w8001_detect(struct w8001 *w8001)
 {
 	struct input_dev *dev = w8001->dev;
-	struct w8001_coord coord;
-	struct w8001_touch_query touch;
-	int error, err_pen, err_touch;
+	int error;
 
 	error = w8001_command(w8001, W8001_CMD_STOP, false);
 	if (error)

@@ -399,101 +397,122 @@ static int w8001_setup(struct w8001 *w8001)
 
 	__set_bit(INPUT_PROP_DIRECT, dev->propbit);
 
+	return 0;
+}
+
+static int w8001_setup_pen(struct w8001 *w8001)
+{
+	struct input_dev *dev = w8001->dev;
+	struct w8001_coord coord;
+	int error;
+
 	/* penabled? */
-	err_pen = w8001_command(w8001, W8001_CMD_QUERY, true);
-	if (!err_pen) {
-		__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);
+	error = w8001_command(w8001, W8001_CMD_QUERY, true);
+	if (error)
+		return error;
 
-		parse_pen_data(w8001->response, &coord);
-		w8001->max_pen_x = coord.x;
-		w8001->max_pen_y = coord.y;
+	__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);
 
-		input_set_abs_params(dev, ABS_X, 0, coord.x, 0, 0);
-		input_set_abs_params(dev, ABS_Y, 0, coord.y, 0, 0);
-		input_abs_set_res(dev, ABS_X, W8001_PEN_RESOLUTION);
-		input_abs_set_res(dev, ABS_Y, W8001_PEN_RESOLUTION);
-		input_set_abs_params(dev, ABS_PRESSURE, 0, coord.pen_pressure, 0, 0);
-		if (coord.tilt_x && coord.tilt_y) {
-			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->id = 0x90;
-		strlcat(w8001->name, " Penabled", sizeof(w8001->name));
+	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);
+	input_abs_set_res(dev, ABS_X, W8001_PEN_RESOLUTION);
+	input_abs_set_res(dev, ABS_Y, W8001_PEN_RESOLUTION);
+	input_set_abs_params(dev, ABS_PRESSURE, 0, coord.pen_pressure, 0, 0);
+	if (coord.tilt_x && coord.tilt_y) {
+		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->id = 0x90;
+	strlcat(w8001->name, " Penabled", sizeof(w8001->name));
+
+	return 0;
+}
+
+static int w8001_setup_touch(struct w8001 *w8001)
+{
+	struct input_dev *dev = w8001->dev;
+	struct w8001_touch_query touch;
+	int error;
+
 	/* Touch enabled? */
-	err_touch = w8001_command(w8001, W8001_CMD_TOUCHQUERY, true);
+	error = w8001_command(w8001, W8001_CMD_TOUCHQUERY, true);
+	if (error)
+		return error;
 
 	/*
 	 * Some non-touch devices may reply to the touch query. But their
 	 * second byte is empty, which indicates touch is not supported.
 	 */
-	if (!err_touch && w8001->response[1]) {
-		err_touch = 1;
-
-		__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;
-
-		if (w8001->max_pen_x && w8001->max_pen_y) {
-			/* if pen is supported scale to pen maximum */
-			touch.x = w8001->max_pen_x;
-			touch.y = w8001->max_pen_y;
-			touch.panel_res = W8001_PEN_RESOLUTION;
-		}
-
-		input_set_abs_params(dev, ABS_X, 0, touch.x, 0, 0);
-		input_set_abs_params(dev, ABS_Y, 0, touch.y, 0, 0);
-		input_abs_set_res(dev, ABS_X, touch.panel_res);
-		input_abs_set_res(dev, ABS_Y, touch.panel_res);
-
-		switch (touch.sensor_id) {
-		case 0:
-		case 2:
-			w8001->pktlen = W8001_PKTLEN_TOUCH93;
-			w8001->id = 0x93;
-			strlcat(w8001->name, " 1FG", sizeof(w8001->name));
-			break;
-
-		case 1:
-		case 3:
-		case 4:
-			w8001->pktlen = W8001_PKTLEN_TOUCH9A;
-			strlcat(w8001->name, " 1FG", sizeof(w8001->name));
-			w8001->id = 0x9a;
-			break;
-
-		case 5:
-			w8001->pktlen = W8001_PKTLEN_TOUCH2FG;
-
-			__set_bit(BTN_TOOL_DOUBLETAP, dev->keybit);
-			input_mt_init_slots(dev, 2, 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, MT_TOOL_MAX, 0, 0);
-
-			strlcat(w8001->name, " 2FG", sizeof(w8001->name));
-			if (w8001->max_pen_x && w8001->max_pen_y)
-				w8001->id = 0xE3;
-			else
-				w8001->id = 0xE2;
-			break;
-		}
+	if (!w8001->response[1])
+		return 1;
+
+	__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;
+
+	if (w8001->max_pen_x && w8001->max_pen_y) {
+		/* if pen is supported scale to pen maximum */
+		touch.x = w8001->max_pen_x;
+		touch.y = w8001->max_pen_y;
+		touch.panel_res = W8001_PEN_RESOLUTION;
+	}
+
+	input_set_abs_params(dev, ABS_X, 0, touch.x, 0, 0);
+	input_set_abs_params(dev, ABS_Y, 0, touch.y, 0, 0);
+	input_abs_set_res(dev, ABS_X, touch.panel_res);
+	input_abs_set_res(dev, ABS_Y, touch.panel_res);
+
+	switch (touch.sensor_id) {
+	case 0:
+	case 2:
+		w8001->pktlen = W8001_PKTLEN_TOUCH93;
+		w8001->id = 0x93;
+		strlcat(w8001->name, " 1FG", sizeof(w8001->name));
+		break;
+
+	case 1:
+	case 3:
+	case 4:
+		w8001->pktlen = W8001_PKTLEN_TOUCH9A;
+		strlcat(w8001->name, " 1FG", sizeof(w8001->name));
+		w8001->id = 0x9a;
+		break;
+
+	case 5:
+		w8001->pktlen = W8001_PKTLEN_TOUCH2FG;
+
+		__set_bit(BTN_TOOL_DOUBLETAP, dev->keybit);
+		input_mt_init_slots(dev, 2, 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, MT_TOOL_MAX, 0, 0);
+
+		strlcat(w8001->name, " 2FG", sizeof(w8001->name));
+		if (w8001->max_pen_x && w8001->max_pen_y)
+			w8001->id = 0xE3;
+		else
+			w8001->id = 0xE2;
+		break;
 	}
 
 	strlcat(w8001->name, " Touchscreen", sizeof(w8001->name));
 
-	return !err_pen || !err_touch;
+	return 0;
 }
 
 /*

@@ -522,7 +541,7 @@ static int w8001_connect(struct serio *serio, struct serio_driver *drv)
 {
 	struct w8001 *w8001;
 	struct input_dev *input_dev;
-	int err;
+	int err, err_pen, err_touch;
 
 	w8001 = kzalloc(sizeof(struct w8001), GFP_KERNEL);
 	input_dev = input_allocate_device();

@@ -541,10 +560,17 @@ static int w8001_connect(struct serio *serio, struct serio_driver *drv)
 	if (err)
 		goto fail2;
 
-	err = w8001_setup(w8001);
+	err = w8001_detect(w8001);
 	if (err)
 		goto fail3;
 
+	err_pen = w8001_setup_pen(w8001);
+	err_touch = w8001_setup_touch(w8001);
+	if (err_pen && err_touch) {
+		err = (err_touch == 1) ? err_pen : err_touch;
+		goto fail3;
+	}
+
 	input_dev->name = w8001->name;
 	input_dev->phys = w8001->phys;
 	input_dev->id.product = w8001->id;

Signature · 1 line

2.5.0

Dmitry Torokhov

maintainer

2015-12-01 22:04 UTC #7 lore raw

Re: [PATCH 4/5] Input: wacom_w8001 - split pen and touch initialization up

Quoted 248 lines · Peter Hutterer

This is preparation work for splitting it up for two event nodes.

Signed-off-by Peter Hutterer <[email protected]>

Acked-by Benjamin Tissoires <[email protected]>

---
 drivers/input/touchscreen/wacom_w8001.c | 198 ++++++++++++++++++--------------
 1 file changed, 112 insertions(+), 86 deletions(-)

drivers/input/touchscreen/wacom_w8001.c +104 ~~-85~~

index cdebf8e..363e510 100644
@@ -380,12 +380,10 @@ static void w8001_close(struct input_dev *dev)
 	w8001_command(w8001, W8001_CMD_STOP, false);
 }
 
-static int w8001_setup(struct w8001 *w8001)
+static int w8001_detect(struct w8001 *w8001)
 {
 	struct input_dev *dev = w8001->dev;
-	struct w8001_coord coord;
-	struct w8001_touch_query touch;
-	int error, err_pen, err_touch;
+	int error;
 
 	error = w8001_command(w8001, W8001_CMD_STOP, false);
 	if (error)

@@ -399,101 +397,122 @@ static int w8001_setup(struct w8001 *w8001)
 
 	__set_bit(INPUT_PROP_DIRECT, dev->propbit);
 
+	return 0;
+}
+
+static int w8001_setup_pen(struct w8001 *w8001)
+{
+	struct input_dev *dev = w8001->dev;
+	struct w8001_coord coord;
+	int error;
+
 	/* penabled? */
-	err_pen = w8001_command(w8001, W8001_CMD_QUERY, true);
-	if (!err_pen) {
-		__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);
+	error = w8001_command(w8001, W8001_CMD_QUERY, true);
+	if (error)
+		return error;
 
-		parse_pen_data(w8001->response, &coord);
-		w8001->max_pen_x = coord.x;
-		w8001->max_pen_y = coord.y;
+	__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);
 
-		input_set_abs_params(dev, ABS_X, 0, coord.x, 0, 0);
-		input_set_abs_params(dev, ABS_Y, 0, coord.y, 0, 0);
-		input_abs_set_res(dev, ABS_X, W8001_PEN_RESOLUTION);
-		input_abs_set_res(dev, ABS_Y, W8001_PEN_RESOLUTION);
-		input_set_abs_params(dev, ABS_PRESSURE, 0, coord.pen_pressure, 0, 0);
-		if (coord.tilt_x && coord.tilt_y) {
-			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->id = 0x90;
-		strlcat(w8001->name, " Penabled", sizeof(w8001->name));
+	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);
+	input_abs_set_res(dev, ABS_X, W8001_PEN_RESOLUTION);
+	input_abs_set_res(dev, ABS_Y, W8001_PEN_RESOLUTION);
+	input_set_abs_params(dev, ABS_PRESSURE, 0, coord.pen_pressure, 0, 0);
+	if (coord.tilt_x && coord.tilt_y) {
+		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->id = 0x90;
+	strlcat(w8001->name, " Penabled", sizeof(w8001->name));
+
+	return 0;
+}
+
+static int w8001_setup_touch(struct w8001 *w8001)
+{
+	struct input_dev *dev = w8001->dev;
+	struct w8001_touch_query touch;
+	int error;
+
 	/* Touch enabled? */
-	err_touch = w8001_command(w8001, W8001_CMD_TOUCHQUERY, true);
+	error = w8001_command(w8001, W8001_CMD_TOUCHQUERY, true);
+	if (error)
+		return error;
 
 	/*
 	 * Some non-touch devices may reply to the touch query. But their
 	 * second byte is empty, which indicates touch is not supported.
 	 */
-	if (!err_touch && w8001->response[1]) {
-		err_touch = 1;
-
-		__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;
-
-		if (w8001->max_pen_x && w8001->max_pen_y) {
-			/* if pen is supported scale to pen maximum */
-			touch.x = w8001->max_pen_x;
-			touch.y = w8001->max_pen_y;
-			touch.panel_res = W8001_PEN_RESOLUTION;
-		}
-
-		input_set_abs_params(dev, ABS_X, 0, touch.x, 0, 0);
-		input_set_abs_params(dev, ABS_Y, 0, touch.y, 0, 0);
-		input_abs_set_res(dev, ABS_X, touch.panel_res);
-		input_abs_set_res(dev, ABS_Y, touch.panel_res);
-
-		switch (touch.sensor_id) {
-		case 0:
-		case 2:
-			w8001->pktlen = W8001_PKTLEN_TOUCH93;
-			w8001->id = 0x93;
-			strlcat(w8001->name, " 1FG", sizeof(w8001->name));
-			break;
-
-		case 1:
-		case 3:
-		case 4:
-			w8001->pktlen = W8001_PKTLEN_TOUCH9A;
-			strlcat(w8001->name, " 1FG", sizeof(w8001->name));
-			w8001->id = 0x9a;
-			break;
-
-		case 5:
-			w8001->pktlen = W8001_PKTLEN_TOUCH2FG;
-
-			__set_bit(BTN_TOOL_DOUBLETAP, dev->keybit);
-			input_mt_init_slots(dev, 2, 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, MT_TOOL_MAX, 0, 0);
-
-			strlcat(w8001->name, " 2FG", sizeof(w8001->name));
-			if (w8001->max_pen_x && w8001->max_pen_y)
-				w8001->id = 0xE3;
-			else
-				w8001->id = 0xE2;
-			break;
-		}
+	if (!w8001->response[1])
+		return 1;
+
+	__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;
+
+	if (w8001->max_pen_x && w8001->max_pen_y) {
+		/* if pen is supported scale to pen maximum */
+		touch.x = w8001->max_pen_x;
+		touch.y = w8001->max_pen_y;
+		touch.panel_res = W8001_PEN_RESOLUTION;
+	}
+
+	input_set_abs_params(dev, ABS_X, 0, touch.x, 0, 0);
+	input_set_abs_params(dev, ABS_Y, 0, touch.y, 0, 0);
+	input_abs_set_res(dev, ABS_X, touch.panel_res);
+	input_abs_set_res(dev, ABS_Y, touch.panel_res);
+
+	switch (touch.sensor_id) {
+	case 0:
+	case 2:
+		w8001->pktlen = W8001_PKTLEN_TOUCH93;
+		w8001->id = 0x93;
+		strlcat(w8001->name, " 1FG", sizeof(w8001->name));
+		break;
+
+	case 1:
+	case 3:
+	case 4:
+		w8001->pktlen = W8001_PKTLEN_TOUCH9A;
+		strlcat(w8001->name, " 1FG", sizeof(w8001->name));
+		w8001->id = 0x9a;
+		break;
+
+	case 5:
+		w8001->pktlen = W8001_PKTLEN_TOUCH2FG;
+
+		__set_bit(BTN_TOOL_DOUBLETAP, dev->keybit);
+		input_mt_init_slots(dev, 2, 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, MT_TOOL_MAX, 0, 0);
+
+		strlcat(w8001->name, " 2FG", sizeof(w8001->name));
+		if (w8001->max_pen_x && w8001->max_pen_y)
+			w8001->id = 0xE3;
+		else
+			w8001->id = 0xE2;
+		break;
 	}
 
 	strlcat(w8001->name, " Touchscreen", sizeof(w8001->name));
 
-	return !err_pen || !err_touch;
+	return 0;
 }
 
 /*

@@ -522,7 +541,7 @@ static int w8001_connect(struct serio *serio, struct serio_driver *drv)
 {
 	struct w8001 *w8001;
 	struct input_dev *input_dev;
-	int err;
+	int err, err_pen, err_touch;
 
 	w8001 = kzalloc(sizeof(struct w8001), GFP_KERNEL);
 	input_dev = input_allocate_device();

@@ -541,10 +560,17 @@ static int w8001_connect(struct serio *serio, struct serio_driver *drv)
 	if (err)
 		goto fail2;

drivers/input/touchscreen/wacom_w8001.c static int w8001_connect(struct serio *serio, struct serio_driver *drv) · line 570

Quoted 10 lines

drivers/input/touchscreen/wacom_w8001.c +5 -1

 
-	err = w8001_setup(w8001);
+	err = w8001_detect(w8001);
 	if (err)
 		goto fail3;
 
+	err_pen = w8001_setup_pen(w8001);
+	err_touch = w8001_setup_touch(w8001);
+	if (err_pen && err_touch) {
+		err = (err_touch == 1) ? err_pen : err_touch;

Let's just do

		err = -ENXIO;

instead of trying to figure out error from which interface to report.

Quoted 9 lines

drivers/input/touchscreen/wacom_w8001.c +3 -0

+		goto fail3;
+	}
+
 	input_dev->name = w8001->name;
 	input_dev->phys = w8001->phys;
 	input_dev->id.product = w8001->id;

Signature · 1 line

2.5.0

Thanks.

Signature · 1 line

Dmitry

Peter Hutterer

author

2015-11-30 03:50 UTC #8 lore raw

PATCH 5/5 Input: wacom_w8001 - split the touch and pen devices into two devices

In reply to Peter Hutterer

These devices have a pen device and a touch device through the same serial protocol, split it up into two separate devices like we do for USB Wacom tablets too.

Userspace already matches on the device name so we can't drop it completely. Compose the same basename based on capabilities and append the tool type, leading to a name like "Wacom Serial Penabled 2FG Touchscreen Pen".

Note that this drops BTN_TOOL_FINGER, it is not needed once the tools are split out (and a touch device with BTN_TOOL_FINGER is interpreted as touchpad by most of userspace).

Signed-off-by Peter Hutterer <[email protected]>

Acked-by Benjamin Tissoires <[email protected]>

---
 drivers/input/touchscreen/wacom_w8001.c | 163 ++++++++++++++++++++++----------
 1 file changed, 112 insertions(+), 51 deletions(-)

drivers/input/touchscreen/wacom_w8001.c +112 ~~-51~~

index 363e510..9efc7ec 100644
@@ -80,7 +80,8 @@ struct w8001_touch_query {
  */
 
 struct w8001 {
-	struct input_dev *dev;
+	struct input_dev *pen_dev;
+	struct input_dev *touch_dev;
 	struct serio *serio;
 	struct completion cmd_done;
 	int id;

@@ -95,7 +96,10 @@ struct w8001 {
 	u16 max_touch_y;
 	u16 max_pen_x;
 	u16 max_pen_y;
-	char name[64];
+	char pen_name[64];
+	char touch_name[64];
+	int open_count;
+	struct mutex mutex;
 };
 
 static void parse_pen_data(u8 *data, struct w8001_coord *coord)

@@ -141,7 +145,7 @@ static void scale_touch_coordinates(struct w8001 *w8001,
 
 static void parse_multi_touch(struct w8001 *w8001)
 {
-	struct input_dev *dev = w8001->dev;
+	struct input_dev *dev = w8001->touch_dev;
 	unsigned char *data = w8001->data;
 	unsigned int x, y;
 	int i;

@@ -207,7 +211,7 @@ static void parse_touchquery(u8 *data, struct w8001_touch_query *query)
 
 static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
 {
-	struct input_dev *dev = w8001->dev;
+	struct input_dev *dev = w8001->pen_dev;
 
 	/*
 	 * We have 1 bit for proximity (rdy) and 3 bits for tip, side,

@@ -233,11 +237,6 @@ static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
 		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;

@@ -261,7 +260,7 @@ static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
 
 static void report_single_touch(struct w8001 *w8001, struct w8001_coord *coord)
 {
-	struct input_dev *dev = w8001->dev;
+	struct input_dev *dev = w8001->touch_dev;
 	unsigned int x = coord->x;
 	unsigned int y = coord->y;

@@ -271,7 +270,6 @@ static void report_single_touch(struct w8001 *w8001, struct w8001_coord *coord)
 	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);

@@ -369,20 +367,33 @@ static int w8001_command(struct w8001 *w8001, unsigned char command,
 static int w8001_open(struct input_dev *dev)
 {
 	struct w8001 *w8001 = input_get_drvdata(dev);
+	int err;
 
-	return w8001_command(w8001, W8001_CMD_START, false);
+	err = mutex_lock_interruptible(&w8001->mutex);
+	if (err)
+		return err;
+
+	if (w8001->open_count++ == 0)
+		err = w8001_command(w8001, W8001_CMD_START, false);
+
+	mutex_unlock(&w8001->mutex);
+	return err;
 }
 
 static void w8001_close(struct input_dev *dev)
 {
 	struct w8001 *w8001 = input_get_drvdata(dev);
 
-	w8001_command(w8001, W8001_CMD_STOP, false);
+	mutex_lock(&w8001->mutex);
+
+	if (--w8001->open_count == 0)
+		w8001_command(w8001, W8001_CMD_STOP, false);
+
+	mutex_unlock(&w8001->mutex);
 }
 
 static int w8001_detect(struct w8001 *w8001)
 {
-	struct input_dev *dev = w8001->dev;
 	int error;
 
 	error = w8001_command(w8001, W8001_CMD_STOP, false);

@@ -391,18 +402,13 @@ static int w8001_detect(struct w8001 *w8001)
 
 	msleep(250);	/* wait 250ms before querying the device */
 
-	__set_bit(EV_KEY, dev->evbit);
-	__set_bit(EV_ABS, dev->evbit);
-	strlcat(w8001->name, "Wacom Serial", sizeof(w8001->name));
-
-	__set_bit(INPUT_PROP_DIRECT, dev->propbit);
-
 	return 0;
 }
 
-static int w8001_setup_pen(struct w8001 *w8001)
+static int w8001_setup_pen(struct w8001 *w8001, char *basename,
+			   size_t basename_sz)
 {
-	struct input_dev *dev = w8001->dev;
+	struct input_dev *dev = w8001->pen_dev;
 	struct w8001_coord coord;
 	int error;

@@ -411,11 +417,14 @@ static int w8001_setup_pen(struct w8001 *w8001)
 	if (error)
 		return error;
 
+	__set_bit(EV_KEY, dev->evbit);
+	__set_bit(EV_ABS, dev->evbit);
 	__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);
+	__set_bit(INPUT_PROP_DIRECT, dev->propbit);
 
 	parse_pen_data(w8001->response, &coord);
 	w8001->max_pen_x = coord.x;

@@ -432,14 +441,15 @@ static int w8001_setup_pen(struct w8001 *w8001)
 	}
 
 	w8001->id = 0x90;
-	strlcat(w8001->name, " Penabled", sizeof(w8001->name));
+	strlcat(basename, " Penabled", basename_sz);
 
 	return 0;
 }
 
-static int w8001_setup_touch(struct w8001 *w8001)
+static int w8001_setup_touch(struct w8001 *w8001, char *basename,
+			     size_t basename_sz)
 {
-	struct input_dev *dev = w8001->dev;
+	struct input_dev *dev = w8001->touch_dev;
 	struct w8001_touch_query touch;
 	int error;

@@ -455,8 +465,10 @@ static int w8001_setup_touch(struct w8001 *w8001)
 	if (!w8001->response[1])
 		return 1;
 
+	__set_bit(EV_KEY, dev->evbit);
+	__set_bit(EV_ABS, dev->evbit);
 	__set_bit(BTN_TOUCH, dev->keybit);
-	__set_bit(BTN_TOOL_FINGER, dev->keybit);
+	__set_bit(INPUT_PROP_DIRECT, dev->propbit);
 
 	parse_touchquery(w8001->response, &touch);
 	w8001->max_touch_x = touch.x;

@@ -479,14 +491,14 @@ static int w8001_setup_touch(struct w8001 *w8001)
 	case 2:
 		w8001->pktlen = W8001_PKTLEN_TOUCH93;
 		w8001->id = 0x93;
-		strlcat(w8001->name, " 1FG", sizeof(w8001->name));
+		strlcat(basename, " 1FG", basename_sz);
 		break;
 
 	case 1:
 	case 3:
 	case 4:
 		w8001->pktlen = W8001_PKTLEN_TOUCH9A;
-		strlcat(w8001->name, " 1FG", sizeof(w8001->name));
+		strlcat(basename, " 1FG", basename_sz);
 		w8001->id = 0x9a;
 		break;

@@ -502,7 +514,7 @@ static int w8001_setup_touch(struct w8001 *w8001)
 		input_set_abs_params(dev, ABS_MT_TOOL_TYPE,
 					0, MT_TOOL_MAX, 0, 0);
 
-		strlcat(w8001->name, " 2FG", sizeof(w8001->name));
+		strlcat(basename, " 2FG", basename_sz);
 		if (w8001->max_pen_x && w8001->max_pen_y)
 			w8001->id = 0xE3;
 		else

@@ -510,11 +522,27 @@ static int w8001_setup_touch(struct w8001 *w8001)
 		break;
 	}
 
-	strlcat(w8001->name, " Touchscreen", sizeof(w8001->name));
+	strlcat(basename, " Touchscreen", basename_sz);
 
 	return 0;
 }
 
+static void w8001_set_devdata(struct input_dev *dev, struct w8001 *w8001,
+			      struct serio *serio)
+{
+	dev->phys = w8001->phys;
+	dev->id.bustype = BUS_RS232;
+	dev->id.product = w8001->id;
+	dev->id.vendor = 0x056a;
+	dev->id.version = 0x0100;
+	dev->open = w8001_open;
+	dev->close = w8001_close;
+
+	dev->dev.parent = &serio->dev;
+
+	input_set_drvdata(dev, w8001);
+}
+
 /*
  * w8001_disconnect() is the opposite of w8001_connect()
  */

@@ -525,7 +553,10 @@ static void w8001_disconnect(struct serio *serio)
 
 	serio_close(serio);
 
-	input_unregister_device(w8001->dev);
+	if (w8001->pen_dev)
+		input_unregister_device(w8001->pen_dev);
+	if (w8001->touch_dev)
+		input_unregister_device(w8001->touch_dev);
 	kfree(w8001);
 
 	serio_set_drvdata(serio, NULL);

@@ -540,18 +571,23 @@ static void w8001_disconnect(struct serio *serio)
 static int w8001_connect(struct serio *serio, struct serio_driver *drv)
 {
 	struct w8001 *w8001;
-	struct input_dev *input_dev;
+	struct input_dev *input_dev_pen;
+	struct input_dev *input_dev_touch;
+	char basename[64];
 	int err, err_pen, err_touch;
 
 	w8001 = kzalloc(sizeof(struct w8001), GFP_KERNEL);
-	input_dev = input_allocate_device();
-	if (!w8001 || !input_dev) {
+	input_dev_pen = input_allocate_device();
+	input_dev_touch = input_allocate_device();
+	if (!w8001 || !input_dev_pen || !input_dev_touch) {
 		err = -ENOMEM;
 		goto fail1;
 	}
 
 	w8001->serio = serio;
-	w8001->dev = input_dev;
+	w8001->pen_dev = input_dev_pen;
+	w8001->touch_dev = input_dev_touch;
+	mutex_init(&w8001->mutex);
 	init_completion(&w8001->cmd_done);
 	snprintf(w8001->phys, sizeof(w8001->phys), "%s/input0", serio->phys);

@@ -564,38 +600,63 @@ static int w8001_connect(struct serio *serio, struct serio_driver *drv)
 	if (err)
 		goto fail3;
 
-	err_pen = w8001_setup_pen(w8001);
-	err_touch = w8001_setup_touch(w8001);
+	/* For backwards-compatibility we compose the basename based on
+	 * capabilities and then just append the tool type
+	 */
+	strlcpy(basename, "Wacom Serial", sizeof(basename));
+
+	err_pen = w8001_setup_pen(w8001, basename, sizeof(basename));
+	err_touch = w8001_setup_touch(w8001, basename, sizeof(basename));
 	if (err_pen && err_touch) {
 		err = (err_touch == 1) ? err_pen : err_touch;
 		goto fail3;
 	}
 
-	input_dev->name = w8001->name;
-	input_dev->phys = w8001->phys;
-	input_dev->id.product = w8001->id;
-	input_dev->id.bustype = BUS_RS232;
-	input_dev->id.vendor = 0x056a;
-	input_dev->id.version = 0x0100;
-	input_dev->dev.parent = &serio->dev;
+	if (!err_pen) {
+		strlcpy(w8001->pen_name, basename, sizeof(w8001->pen_name));
+		strlcat(w8001->pen_name, " Pen", sizeof(w8001->pen_name));
+		input_dev_pen->name = w8001->pen_name;
 
-	input_dev->open = w8001_open;
-	input_dev->close = w8001_close;
+		w8001_set_devdata(input_dev_pen, w8001, serio);
 
-	input_set_drvdata(input_dev, w8001);
+		err = input_register_device(w8001->pen_dev);
+		if (err)
+			goto fail3;
+	} else {
+		input_free_device(input_dev_pen);
+		input_dev_pen = NULL;
+		w8001->pen_dev = NULL;
+	}
 
-	err = input_register_device(w8001->dev);
-	if (err)
-		goto fail3;
+	if (!err_touch) {
+		strlcpy(w8001->touch_name, basename, sizeof(w8001->touch_name));
+		strlcat(w8001->touch_name, " Finger",
+			sizeof(w8001->touch_name));
+		input_dev_touch->name = w8001->touch_name;
+
+		w8001_set_devdata(input_dev_touch, w8001, serio);
+
+		err = input_register_device(w8001->touch_dev);
+		if (err)
+			goto fail4;
+	} else {
+		input_free_device(input_dev_touch);
+		input_dev_touch = NULL;
+		w8001->touch_dev = NULL;
+	}
 
 	return 0;
 
+fail4:
+	if (w8001->pen_dev)
+		input_unregister_device(w8001->pen_dev);
 fail3:
 	serio_close(serio);
 fail2:
 	serio_set_drvdata(serio, NULL);
 fail1:
-	input_free_device(input_dev);
+	input_free_device(input_dev_pen);
+	input_free_device(input_dev_touch);
 	kfree(w8001);
 	return err;
 }

Signature · 1 line

2.5.0

Dmitry Torokhov

maintainer

2015-12-01 22:06 UTC #9 lore raw

Re: [PATCH 5/5] Input: wacom_w8001 - split the touch and pen devices into two devices

Quoted 95 lines · Peter Hutterer

These devices have a pen device and a touch device through the same serial protocol, split it up into two separate devices like we do for USB Wacom tablets too.

Note that this drops BTN_TOOL_FINGER, it is not needed once the tools are split out (and a touch device with BTN_TOOL_FINGER is interpreted as touchpad by most of userspace).

Signed-off-by Peter Hutterer <[email protected]>

Acked-by Benjamin Tissoires <[email protected]>

---
 drivers/input/touchscreen/wacom_w8001.c | 163 ++++++++++++++++++++++----------
 1 file changed, 112 insertions(+), 51 deletions(-)

drivers/input/touchscreen/wacom_w8001.c +9 ~~-11~~

index 363e510..9efc7ec 100644
@@ -80,7 +80,8 @@ struct w8001_touch_query {
  */
 
 struct w8001 {
-	struct input_dev *dev;
+	struct input_dev *pen_dev;
+	struct input_dev *touch_dev;
 	struct serio *serio;
 	struct completion cmd_done;
 	int id;

@@ -95,7 +96,10 @@ struct w8001 {
 	u16 max_touch_y;
 	u16 max_pen_x;
 	u16 max_pen_y;
-	char name[64];
+	char pen_name[64];
+	char touch_name[64];
+	int open_count;
+	struct mutex mutex;
 };
 
 static void parse_pen_data(u8 *data, struct w8001_coord *coord)

@@ -141,7 +145,7 @@ static void scale_touch_coordinates(struct w8001 *w8001,
 
 static void parse_multi_touch(struct w8001 *w8001)
 {
-	struct input_dev *dev = w8001->dev;
+	struct input_dev *dev = w8001->touch_dev;
 	unsigned char *data = w8001->data;
 	unsigned int x, y;
 	int i;

@@ -207,7 +211,7 @@ static void parse_touchquery(u8 *data, struct w8001_touch_query *query)
 
 static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
 {
-	struct input_dev *dev = w8001->dev;
+	struct input_dev *dev = w8001->pen_dev;
 
 	/*
 	 * We have 1 bit for proximity (rdy) and 3 bits for tip, side,

@@ -233,11 +237,6 @@ static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
 		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;

@@ -261,7 +260,7 @@ static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
 
 static void report_single_touch(struct w8001 *w8001, struct w8001_coord *coord)
 {
-	struct input_dev *dev = w8001->dev;
+	struct input_dev *dev = w8001->touch_dev;
 	unsigned int x = coord->x;
 	unsigned int y = coord->y;

@@ -271,7 +270,6 @@ static void report_single_touch(struct w8001 *w8001, struct w8001_coord *coord)
 	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);

@@ -369,20 +367,33 @@ static int w8001_command(struct w8001 *w8001, unsigned char command,
 static int w8001_open(struct input_dev *dev)
 {

drivers/input/touchscreen/wacom_w8001.c static int w8001_command(struct w8001 *w8001, unsigned char command, · line 377

Quoted 10 lines

drivers/input/touchscreen/wacom_w8001.c +7 -1

 	struct w8001 *w8001 = input_get_drvdata(dev);
+	int err;
 
-	return w8001_command(w8001, W8001_CMD_START, false);
+	err = mutex_lock_interruptible(&w8001->mutex);
+	if (err)
+		return err;
+
+	if (w8001->open_count++ == 0)
+		err = w8001_command(w8001, W8001_CMD_START, false);

We leak the count in case of error here. You need something like:

static int w8001_open(struct input_dev *dev)

{
	struct w8001 *w8001 = input_get_drvdata(dev);
	int retval;

	retval = mutex_lock_interruptible(&w8001->mutex);
	if (retval)
		return retval;

	if (w8001->open_count++ == 0) {
		retval = w8001_command(w8001, W8001_CMD_START, false);
		if (retval)
			w8001->open_count--;
	}

	mutex_unlock(&w8001->mutex);
	return retval;
}

Thanks.

Signature · 1 line

Dmitry

Peter Hutterer

author

2015-12-03 04:22 UTC #10 lore raw

PATCH v2 5/5 Input: wacom_w8001 - split the touch and pen devices into two devices

These devices have a pen device and a touch device through the same serial protocol, split it up into two separate devices like we do for USB Wacom tablets too.

Note that this drops BTN_TOOL_FINGER, it is not needed once the tools are split out (and a touch device with BTN_TOOL_FINGER is interpreted as touchpad by most of userspace).

Signed-off-by Peter Hutterer <[email protected]>

Acked-by Benjamin Tissoires <[email protected]>

---
Changes to v1:
- don't leak open_count on error

 drivers/input/touchscreen/wacom_w8001.c | 167 ++++++++++++++++++++++----------
 1 file changed, 116 insertions(+), 51 deletions(-)

drivers/input/touchscreen/wacom_w8001.c +116 ~~-51~~

index de8f0e4..fe983e7 100644
@@ -80,7 +80,8 @@ struct w8001_touch_query {
  */
 
 struct w8001 {
-	struct input_dev *dev;
+	struct input_dev *pen_dev;
+	struct input_dev *touch_dev;
 	struct serio *serio;
 	struct completion cmd_done;
 	int id;

@@ -95,7 +96,10 @@ struct w8001 {
 	u16 max_touch_y;
 	u16 max_pen_x;
 	u16 max_pen_y;
-	char name[64];
+	char pen_name[64];
+	char touch_name[64];
+	int open_count;
+	struct mutex mutex;
 };
 
 static void parse_pen_data(u8 *data, struct w8001_coord *coord)

@@ -141,7 +145,7 @@ static void scale_touch_coordinates(struct w8001 *w8001,
 
 static void parse_multi_touch(struct w8001 *w8001)
 {
-	struct input_dev *dev = w8001->dev;
+	struct input_dev *dev = w8001->touch_dev;
 	unsigned char *data = w8001->data;
 	unsigned int x, y;
 	int i;

@@ -207,7 +211,7 @@ static void parse_touchquery(u8 *data, struct w8001_touch_query *query)
 
 static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
 {
-	struct input_dev *dev = w8001->dev;
+	struct input_dev *dev = w8001->pen_dev;
 
 	/*
 	 * We have 1 bit for proximity (rdy) and 3 bits for tip, side,

@@ -233,11 +237,6 @@ static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
 		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;

@@ -261,7 +260,7 @@ static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
 
 static void report_single_touch(struct w8001 *w8001, struct w8001_coord *coord)
 {
-	struct input_dev *dev = w8001->dev;
+	struct input_dev *dev = w8001->touch_dev;
 	unsigned int x = coord->x;
 	unsigned int y = coord->y;

@@ -271,7 +270,6 @@ static void report_single_touch(struct w8001 *w8001, struct w8001_coord *coord)
 	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);

@@ -369,20 +367,36 @@ static int w8001_command(struct w8001 *w8001, unsigned char command,
 static int w8001_open(struct input_dev *dev)
 {
 	struct w8001 *w8001 = input_get_drvdata(dev);
+	int err;
 
-	return w8001_command(w8001, W8001_CMD_START, false);
+	err = mutex_lock_interruptible(&w8001->mutex);
+	if (err)
+		return err;
+
+	if (w8001->open_count++ == 0) {
+		err = w8001_command(w8001, W8001_CMD_START, false);
+		if (err)
+			w8001->open_count--;
+	}
+
+	mutex_unlock(&w8001->mutex);
+	return err;
 }
 
 static void w8001_close(struct input_dev *dev)
 {
 	struct w8001 *w8001 = input_get_drvdata(dev);
 
-	w8001_command(w8001, W8001_CMD_STOP, false);
+	mutex_lock(&w8001->mutex);
+
+	if (--w8001->open_count == 0)
+		w8001_command(w8001, W8001_CMD_STOP, false);
+
+	mutex_unlock(&w8001->mutex);
 }
 
 static int w8001_detect(struct w8001 *w8001)
 {
-	struct input_dev *dev = w8001->dev;
 	int error;
 
 	error = w8001_command(w8001, W8001_CMD_STOP, false);

@@ -391,18 +405,13 @@ static int w8001_detect(struct w8001 *w8001)
 
 	msleep(250);	/* wait 250ms before querying the device */
 
-	__set_bit(EV_KEY, dev->evbit);
-	__set_bit(EV_ABS, dev->evbit);
-	strlcat(w8001->name, "Wacom Serial", sizeof(w8001->name));
-
-	__set_bit(INPUT_PROP_DIRECT, dev->propbit);
-
 	return 0;
 }
 
-static int w8001_setup_pen(struct w8001 *w8001)
+static int w8001_setup_pen(struct w8001 *w8001, char *basename,
+			   size_t basename_sz)
 {
-	struct input_dev *dev = w8001->dev;
+	struct input_dev *dev = w8001->pen_dev;
 	struct w8001_coord coord;
 	int error;

@@ -411,11 +420,14 @@ static int w8001_setup_pen(struct w8001 *w8001)
 	if (error)
 		return error;
 
+	__set_bit(EV_KEY, dev->evbit);
+	__set_bit(EV_ABS, dev->evbit);
 	__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);
+	__set_bit(INPUT_PROP_DIRECT, dev->propbit);
 
 	parse_pen_data(w8001->response, &coord);
 	w8001->max_pen_x = coord.x;

@@ -432,17 +444,19 @@ static int w8001_setup_pen(struct w8001 *w8001)
 	}
 
 	w8001->id = 0x90;
-	strlcat(w8001->name, " Penabled", sizeof(w8001->name));
+	strlcat(basename, " Penabled", basename_sz);
 
 	return 0;
 }
 
-static int w8001_setup_touch(struct w8001 *w8001)
+static int w8001_setup_touch(struct w8001 *w8001, char *basename,
+			     size_t basename_sz)
 {
-	struct input_dev *dev = w8001->dev;
+	struct input_dev *dev = w8001->touch_dev;
 	struct w8001_touch_query touch;
 	int error;
 
+
 	/* Touch enabled? */
 	error = w8001_command(w8001, W8001_CMD_TOUCHQUERY, true);
 	if (error)

@@ -454,8 +468,10 @@ static int w8001_setup_touch(struct w8001 *w8001)
 	if (!w8001->response[1])
 		return -ENXIO;
 
+	__set_bit(EV_KEY, dev->evbit);
+	__set_bit(EV_ABS, dev->evbit);
 	__set_bit(BTN_TOUCH, dev->keybit);
-	__set_bit(BTN_TOOL_FINGER, dev->keybit);
+	__set_bit(INPUT_PROP_DIRECT, dev->propbit);
 
 	parse_touchquery(w8001->response, &touch);
 	w8001->max_touch_x = touch.x;

@@ -478,14 +494,14 @@ static int w8001_setup_touch(struct w8001 *w8001)
 	case 2:
 		w8001->pktlen = W8001_PKTLEN_TOUCH93;
 		w8001->id = 0x93;
-		strlcat(w8001->name, " 1FG", sizeof(w8001->name));
+		strlcat(basename, " 1FG", basename_sz);
 		break;
 
 	case 1:
 	case 3:
 	case 4:
 		w8001->pktlen = W8001_PKTLEN_TOUCH9A;
-		strlcat(w8001->name, " 1FG", sizeof(w8001->name));
+		strlcat(basename, " 1FG", basename_sz);
 		w8001->id = 0x9a;
 		break;

@@ -501,7 +517,7 @@ static int w8001_setup_touch(struct w8001 *w8001)
 		input_set_abs_params(dev, ABS_MT_TOOL_TYPE,
 					0, MT_TOOL_MAX, 0, 0);
 
-		strlcat(w8001->name, " 2FG", sizeof(w8001->name));
+		strlcat(basename, " 2FG", basename_sz);
 		if (w8001->max_pen_x && w8001->max_pen_y)
 			w8001->id = 0xE3;
 		else

@@ -509,11 +525,27 @@ static int w8001_setup_touch(struct w8001 *w8001)
 		break;
 	}
 
-	strlcat(w8001->name, " Touchscreen", sizeof(w8001->name));
+	strlcat(basename, " Touchscreen", basename_sz);
 
 	return 0;
 }
 
+static void w8001_set_devdata(struct input_dev *dev, struct w8001 *w8001,
+			      struct serio *serio)
+{
+	dev->phys = w8001->phys;
+	dev->id.bustype = BUS_RS232;
+	dev->id.product = w8001->id;
+	dev->id.vendor = 0x056a;
+	dev->id.version = 0x0100;
+	dev->open = w8001_open;
+	dev->close = w8001_close;
+
+	dev->dev.parent = &serio->dev;
+
+	input_set_drvdata(dev, w8001);
+}
+
 /*
  * w8001_disconnect() is the opposite of w8001_connect()
  */

@@ -524,7 +556,10 @@ static void w8001_disconnect(struct serio *serio)
 
 	serio_close(serio);
 
-	input_unregister_device(w8001->dev);
+	if (w8001->pen_dev)
+		input_unregister_device(w8001->pen_dev);
+	if (w8001->touch_dev)
+		input_unregister_device(w8001->touch_dev);
 	kfree(w8001);
 
 	serio_set_drvdata(serio, NULL);

@@ -539,18 +574,23 @@ static void w8001_disconnect(struct serio *serio)
 static int w8001_connect(struct serio *serio, struct serio_driver *drv)
 {
 	struct w8001 *w8001;
-	struct input_dev *input_dev;
+	struct input_dev *input_dev_pen;
+	struct input_dev *input_dev_touch;
+	char basename[64];
 	int err, err_pen, err_touch;
 
 	w8001 = kzalloc(sizeof(struct w8001), GFP_KERNEL);
-	input_dev = input_allocate_device();
-	if (!w8001 || !input_dev) {
+	input_dev_pen = input_allocate_device();
+	input_dev_touch = input_allocate_device();
+	if (!w8001 || !input_dev_pen || !input_dev_touch) {
 		err = -ENOMEM;
 		goto fail1;
 	}
 
 	w8001->serio = serio;
-	w8001->dev = input_dev;
+	w8001->pen_dev = input_dev_pen;
+	w8001->touch_dev = input_dev_touch;
+	mutex_init(&w8001->mutex);
 	init_completion(&w8001->cmd_done);
 	snprintf(w8001->phys, sizeof(w8001->phys), "%s/input0", serio->phys);

@@ -563,38 +603,63 @@ static int w8001_connect(struct serio *serio, struct serio_driver *drv)
 	if (err)
 		goto fail3;
 
-	err_pen = w8001_setup_pen(w8001);
-	err_touch = w8001_setup_touch(w8001);
+	/* For backwards-compatibility we compose the basename based on
+	 * capabilities and then just append the tool type
+	 */
+	strlcpy(basename, "Wacom Serial", sizeof(basename));
+
+	err_pen = w8001_setup_pen(w8001, basename, sizeof(basename));
+	err_touch = w8001_setup_touch(w8001, basename, sizeof(basename));
 	if (err_pen && err_touch) {
 		err = -ENXIO;
 		goto fail3;
 	}
 
-	input_dev->name = w8001->name;
-	input_dev->phys = w8001->phys;
-	input_dev->id.product = w8001->id;
-	input_dev->id.bustype = BUS_RS232;
-	input_dev->id.vendor = 0x056a;
-	input_dev->id.version = 0x0100;
-	input_dev->dev.parent = &serio->dev;
+	if (!err_pen) {
+		strlcpy(w8001->pen_name, basename, sizeof(w8001->pen_name));
+		strlcat(w8001->pen_name, " Pen", sizeof(w8001->pen_name));
+		input_dev_pen->name = w8001->pen_name;
 
-	input_dev->open = w8001_open;
-	input_dev->close = w8001_close;
+		w8001_set_devdata(input_dev_pen, w8001, serio);
 
-	input_set_drvdata(input_dev, w8001);
+		err = input_register_device(w8001->pen_dev);
+		if (err)
+			goto fail3;
+	} else {
+		input_free_device(input_dev_pen);
+		input_dev_pen = NULL;
+		w8001->pen_dev = NULL;
+	}
 
-	err = input_register_device(w8001->dev);
-	if (err)
-		goto fail3;
+	if (!err_touch) {
+		strlcpy(w8001->touch_name, basename, sizeof(w8001->touch_name));
+		strlcat(w8001->touch_name, " Finger",
+			sizeof(w8001->touch_name));
+		input_dev_touch->name = w8001->touch_name;
+
+		w8001_set_devdata(input_dev_touch, w8001, serio);
+
+		err = input_register_device(w8001->touch_dev);
+		if (err)
+			goto fail4;
+	} else {
+		input_free_device(input_dev_touch);
+		input_dev_touch = NULL;
+		w8001->touch_dev = NULL;
+	}
 
 	return 0;
 
+fail4:
+	if (w8001->pen_dev)
+		input_unregister_device(w8001->pen_dev);
 fail3:
 	serio_close(serio);
 fail2:
 	serio_set_drvdata(serio, NULL);
 fail1:
-	input_free_device(input_dev);
+	input_free_device(input_dev_pen);
+	input_free_device(input_dev_touch);
 	kfree(w8001);
 	return err;
 }

Signature · 1 line

2.5.0

Dmitry Torokhov

maintainer Applied

2015-12-03 23:59 UTC #11 lore raw

Re: [PATCH v2 5/5] Input: wacom_w8001 - split the touch and pen devices into two devices

Quoted 8 lines · Peter Hutterer

These devices have a pen device and a touch device through the same serial protocol, split it up into two separate devices like we do for USB Wacom tablets too.
Userspace already matches on the device name so we can't drop it completely. Compose the same basename based on capabilities and append the tool type, leading to a name like "Wacom Serial Penabled 2FG Touchscreen Pen".

Quoted 6 lines

Note that this drops BTN_TOOL_FINGER, it is not needed once the tools are split out (and a touch device with BTN_TOOL_FINGER is interpreted as touchpad by most of userspace).
Signed-off-by Peter Hutterer <[email protected]>
Acked-by Benjamin Tissoires <[email protected]>

Applied all 5, thank you.

Quoted 370 lines

---
Changes to v1:
- don't leak open_count on error

 drivers/input/touchscreen/wacom_w8001.c | 167 ++++++++++++++++++++++----------
 1 file changed, 116 insertions(+), 51 deletions(-)

drivers/input/touchscreen/wacom_w8001.c +116 ~~-52~~

index de8f0e4..fe983e7 100644
@@ -80,7 +80,8 @@ struct w8001_touch_query {
  */
 
 struct w8001 {
-       struct input_dev *dev;
+       struct input_dev *pen_dev;
+       struct input_dev *touch_dev;
        struct serio *serio;
        struct completion cmd_done;
        int id;

@@ -95,7 +96,10 @@ struct w8001 {
        u16 max_touch_y;
        u16 max_pen_x;
        u16 max_pen_y;
-       char name[64];
+       char pen_name[64];
+       char touch_name[64];
+       int open_count;
+       struct mutex mutex;
 };
 
 static void parse_pen_data(u8 *data, struct w8001_coord *coord)

@@ -141,7 +145,7 @@ static void scale_touch_coordinates(struct w8001 *w8001,
 
 static void parse_multi_touch(struct w8001 *w8001)
 {
-       struct input_dev *dev = w8001->dev;
+       struct input_dev *dev = w8001->touch_dev;
        unsigned char *data = w8001->data;
        unsigned int x, y;
        int i;

@@ -207,7 +211,7 @@ static void parse_touchquery(u8 *data, struct w8001_touch_query *query)
 
 static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
 {
-       struct input_dev *dev = w8001->dev;
+       struct input_dev *dev = w8001->pen_dev;
 
        /*
         * We have 1 bit for proximity (rdy) and 3 bits for tip, side,

@@ -233,11 +237,6 @@ static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
                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;

@@ -261,7 +260,7 @@ static void report_pen_events(struct w8001 *w8001, struct w8001_coord *coord)
 
 static void report_single_touch(struct w8001 *w8001, struct w8001_coord *coord)
 {
-       struct input_dev *dev = w8001->dev;
+       struct input_dev *dev = w8001->touch_dev;
        unsigned int x = coord->x;
        unsigned int y = coord->y;

@@ -271,7 +270,6 @@ static void report_single_touch(struct w8001 *w8001, struct w8001_coord *coord)
        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);

@@ -369,20 +367,36 @@ static int w8001_command(struct w8001 *w8001, unsigned char command,
 static int w8001_open(struct input_dev *dev)
 {
        struct w8001 *w8001 = input_get_drvdata(dev);
+       int err;
 
-       return w8001_command(w8001, W8001_CMD_START, false);
+       err = mutex_lock_interruptible(&w8001->mutex);
+       if (err)
+               return err;
+
+       if (w8001->open_count++ == 0) {
+               err = w8001_command(w8001, W8001_CMD_START, false);
+               if (err)
+                       w8001->open_count--;
+       }
+
+       mutex_unlock(&w8001->mutex);
+       return err;
 }
 
 static void w8001_close(struct input_dev *dev)
 {
        struct w8001 *w8001 = input_get_drvdata(dev);
 
-       w8001_command(w8001, W8001_CMD_STOP, false);
+       mutex_lock(&w8001->mutex);
+
+       if (--w8001->open_count == 0)
+               w8001_command(w8001, W8001_CMD_STOP, false);
+
+       mutex_unlock(&w8001->mutex);
 }
 
 static int w8001_detect(struct w8001 *w8001)
 {
-       struct input_dev *dev = w8001->dev;
        int error;
 
        error = w8001_command(w8001, W8001_CMD_STOP, false);

@@ -391,18 +405,13 @@ static int w8001_detect(struct w8001 *w8001)
 
        msleep(250);    /* wait 250ms before querying the device */
 
-       __set_bit(EV_KEY, dev->evbit);
-       __set_bit(EV_ABS, dev->evbit);
-       strlcat(w8001->name, "Wacom Serial", sizeof(w8001->name));
-
-       __set_bit(INPUT_PROP_DIRECT, dev->propbit);
-
        return 0;
 }
 
-static int w8001_setup_pen(struct w8001 *w8001)
+static int w8001_setup_pen(struct w8001 *w8001, char *basename,
+                          size_t basename_sz)
 {
-       struct input_dev *dev = w8001->dev;
+       struct input_dev *dev = w8001->pen_dev;
        struct w8001_coord coord;
        int error;

@@ -411,11 +420,14 @@ static int w8001_setup_pen(struct w8001 *w8001)
        if (error)
                return error;
 
+       __set_bit(EV_KEY, dev->evbit);
+       __set_bit(EV_ABS, dev->evbit);
        __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);
+       __set_bit(INPUT_PROP_DIRECT, dev->propbit);
 
        parse_pen_data(w8001->response, &coord);
        w8001->max_pen_x = coord.x;

@@ -432,17 +444,19 @@ static int w8001_setup_pen(struct w8001 *w8001)
        }
 
        w8001->id = 0x90;
-       strlcat(w8001->name, " Penabled", sizeof(w8001->name));
+       strlcat(basename, " Penabled", basename_sz);
 
        return 0;
 }
 
-static int w8001_setup_touch(struct w8001 *w8001)
+static int w8001_setup_touch(struct w8001 *w8001, char *basename,
+                            size_t basename_sz)
 {
-       struct input_dev *dev = w8001->dev;
+       struct input_dev *dev = w8001->touch_dev;
        struct w8001_touch_query touch;
        int error;
 
+
        /* Touch enabled? */
        error = w8001_command(w8001, W8001_CMD_TOUCHQUERY, true);
        if (error)

@@ -454,8 +468,10 @@ static int w8001_setup_touch(struct w8001 *w8001)
        if (!w8001->response[1])
                return -ENXIO;
 
+       __set_bit(EV_KEY, dev->evbit);
+       __set_bit(EV_ABS, dev->evbit);
        __set_bit(BTN_TOUCH, dev->keybit);
-       __set_bit(BTN_TOOL_FINGER, dev->keybit);
+       __set_bit(INPUT_PROP_DIRECT, dev->propbit);
 
        parse_touchquery(w8001->response, &touch);
        w8001->max_touch_x = touch.x;

@@ -478,14 +494,14 @@ static int w8001_setup_touch(struct w8001 *w8001)
        case 2:
                w8001->pktlen = W8001_PKTLEN_TOUCH93;
                w8001->id = 0x93;
-               strlcat(w8001->name, " 1FG", sizeof(w8001->name));
+               strlcat(basename, " 1FG", basename_sz);
                break;
 
        case 1:
        case 3:
        case 4:
                w8001->pktlen = W8001_PKTLEN_TOUCH9A;
-               strlcat(w8001->name, " 1FG", sizeof(w8001->name));
+               strlcat(basename, " 1FG", basename_sz);
                w8001->id = 0x9a;
                break;

@@ -501,7 +517,7 @@ static int w8001_setup_touch(struct w8001 *w8001)
                input_set_abs_params(dev, ABS_MT_TOOL_TYPE,
                                        0, MT_TOOL_MAX, 0, 0);
 
-               strlcat(w8001->name, " 2FG", sizeof(w8001->name));
+               strlcat(basename, " 2FG", basename_sz);
                if (w8001->max_pen_x && w8001->max_pen_y)
                        w8001->id = 0xE3;
                else

@@ -509,11 +525,27 @@ static int w8001_setup_touch(struct w8001 *w8001)
                break;
        }
 
-       strlcat(w8001->name, " Touchscreen", sizeof(w8001->name));
+       strlcat(basename, " Touchscreen", basename_sz);
 
        return 0;
 }
 
+static void w8001_set_devdata(struct input_dev *dev, struct w8001 *w8001,
+                             struct serio *serio)
+{
+       dev->phys = w8001->phys;
+       dev->id.bustype = BUS_RS232;
+       dev->id.product = w8001->id;
+       dev->id.vendor = 0x056a;
+       dev->id.version = 0x0100;
+       dev->open = w8001_open;
+       dev->close = w8001_close;
+
+       dev->dev.parent = &serio->dev;
+
+       input_set_drvdata(dev, w8001);
+}
+
 /*
  * w8001_disconnect() is the opposite of w8001_connect()
  */

@@ -524,7 +556,10 @@ static void w8001_disconnect(struct serio *serio)
 
        serio_close(serio);
 
-       input_unregister_device(w8001->dev);
+       if (w8001->pen_dev)
+               input_unregister_device(w8001->pen_dev);
+       if (w8001->touch_dev)
+               input_unregister_device(w8001->touch_dev);
        kfree(w8001);
 
        serio_set_drvdata(serio, NULL);

@@ -539,18 +574,23 @@ static void w8001_disconnect(struct serio *serio)
 static int w8001_connect(struct serio *serio, struct serio_driver *drv)
 {
        struct w8001 *w8001;
-       struct input_dev *input_dev;
+       struct input_dev *input_dev_pen;
+       struct input_dev *input_dev_touch;
+       char basename[64];
        int err, err_pen, err_touch;
 
        w8001 = kzalloc(sizeof(struct w8001), GFP_KERNEL);
-       input_dev = input_allocate_device();
-       if (!w8001 || !input_dev) {
+       input_dev_pen = input_allocate_device();
+       input_dev_touch = input_allocate_device();
+       if (!w8001 || !input_dev_pen || !input_dev_touch) {
                err = -ENOMEM;
                goto fail1;
        }
 
        w8001->serio = serio;
-       w8001->dev = input_dev;
+       w8001->pen_dev = input_dev_pen;
+       w8001->touch_dev = input_dev_touch;
+       mutex_init(&w8001->mutex);
        init_completion(&w8001->cmd_done);
        snprintf(w8001->phys, sizeof(w8001->phys), "%s/input0", serio->phys);

@@ -563,38 +603,63 @@ static int w8001_connect(struct serio *serio, struct serio_driver *drv)
        if (err)
                goto fail3;
 
-       err_pen = w8001_setup_pen(w8001);
-       err_touch = w8001_setup_touch(w8001);
+       /* For backwards-compatibility we compose the basename based on
+        * capabilities and then just append the tool type
+        */
+       strlcpy(basename, "Wacom Serial", sizeof(basename));
+
+       err_pen = w8001_setup_pen(w8001, basename, sizeof(basename));
+       err_touch = w8001_setup_touch(w8001, basename, sizeof(basename));
        if (err_pen && err_touch) {
                err = -ENXIO;
                goto fail3;
        }
 
-       input_dev->name = w8001->name;
-       input_dev->phys = w8001->phys;
-       input_dev->id.product = w8001->id;
-       input_dev->id.bustype = BUS_RS232;
-       input_dev->id.vendor = 0x056a;
-       input_dev->id.version = 0x0100;
-       input_dev->dev.parent = &serio->dev;
+       if (!err_pen) {
+               strlcpy(w8001->pen_name, basename, sizeof(w8001->pen_name));
+               strlcat(w8001->pen_name, " Pen", sizeof(w8001->pen_name));
+               input_dev_pen->name = w8001->pen_name;
 
-       input_dev->open = w8001_open;
-       input_dev->close = w8001_close;
+               w8001_set_devdata(input_dev_pen, w8001, serio);
 
-       input_set_drvdata(input_dev, w8001);
+               err = input_register_device(w8001->pen_dev);
+               if (err)
+                       goto fail3;
+       } else {
+               input_free_device(input_dev_pen);
+               input_dev_pen = NULL;
+               w8001->pen_dev = NULL;
+       }
 
-       err = input_register_device(w8001->dev);
-       if (err)
-               goto fail3;
+       if (!err_touch) {
+               strlcpy(w8001->touch_name, basename, sizeof(w8001->touch_name));
+               strlcat(w8001->touch_name, " Finger",
+                       sizeof(w8001->touch_name));
+               input_dev_touch->name = w8001->touch_name;
+
+               w8001_set_devdata(input_dev_touch, w8001, serio);
+
+               err = input_register_device(w8001->touch_dev);
+               if (err)
+                       goto fail4;
+       } else {
+               input_free_device(input_dev_touch);
+               input_dev_touch = NULL;
+               w8001->touch_dev = NULL;
+       }
 
        return 0;
 
+fail4:
+       if (w8001->pen_dev)
+               input_unregister_device(w8001->pen_dev);
 fail3:
        serio_close(serio);
 fail2:
        serio_set_drvdata(serio, NULL);
 fail1:
-       input_free_device(input_dev);
+       input_free_device(input_dev_pen);
+       input_free_device(input_dev_touch);
        kfree(w8001);
        return err;
 }
--

2.5.0

Signature · 1 line

Dmitry