diff mbox

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

Message ID 1448854696-32625-4-git-send-email-peter.hutterer@who-t.net (mailing list archive)
State Superseded
Headers show

Commit Message

Peter Hutterer Nov. 30, 2015, 3:38 a.m. UTC 
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 <peter.hutterer@who-t.net>
Acked-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
---
 drivers/input/touchscreen/wacom_w8001.c | 198 ++++++++++++++++++--------------
 1 file changed, 112 insertions(+), 86 deletions(-)

Comments

Dmitry Torokhov Dec. 1, 2015, 10:04 p.m. UTC | #1 
On Mon, Nov 30, 2015 at 01:38:15PM +1000, Peter Hutterer wrote:
> 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 <peter.hutterer@who-t.net>
> Acked-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
> ---
>  drivers/input/touchscreen/wacom_w8001.c | 198 ++++++++++++++++++--------------
>  1 file changed, 112 insertions(+), 86 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
> index cdebf8e..363e510 100644
> --- a/drivers/input/touchscreen/wacom_w8001.c
> +++ b/drivers/input/touchscreen/wacom_w8001.c
> @@ -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;

Let's just do
		err = -ENXIO;

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

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

Thanks.
diff mbox

Patch

diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
index cdebf8e..363e510 100644
--- a/drivers/input/touchscreen/wacom_w8001.c
+++ b/drivers/input/touchscreen/wacom_w8001.c
@@ -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;