[PATCH 3/3] HID: wacom: set stylus_in_proximity before checking touch_down

[PATCH 3/3] HID: wacom: set stylus_in_proximity before checking touch_down

[Ping Cheng]

In wacom_bpt_pen, we checked touch_down before assigning new
stylus_in_proximity value. This would cause stylus_in_proximity
not updated properly if touch is down before pen is in proximity.

Signed-off-by: Ping Cheng <pingc@wacom.com>
---
 drivers/hid/wacom_wac.c | 35 ++++++++++++++++-------------------
 1 file changed, 16 insertions(+), 19 deletions(-)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 60d9ccb..63e767e 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1752,9 +1752,6 @@ static int wacom_bpt_pen(struct wacom_wac *wacom)
 		return 0;
 	}
 
-	if (wacom->shared->touch_down)
-		return 0;
-
 	prox = (data[1] & 0x20) == 0x20;
 
 	/*
@@ -1767,17 +1764,21 @@ static int wacom_bpt_pen(struct wacom_wac *wacom)
 	 *
 	 * Hardware does report zero in most out-of-prox cases but not all.
 	 */
-	if (prox) {
-		if (!wacom->shared->stylus_in_proximity) {
-			if (data[1] & 0x08) {
-				wacom->tool[0] = BTN_TOOL_RUBBER;
-				wacom->id[0] = ERASER_DEVICE_ID;
-			} else {
-				wacom->tool[0] = BTN_TOOL_PEN;
-				wacom->id[0] = STYLUS_DEVICE_ID;
-			}
-			wacom->shared->stylus_in_proximity = true;
+	if (!wacom->shared->stylus_in_proximity) {
+		if (data[1] & 0x08) {
+			wacom->tool[0] = BTN_TOOL_RUBBER;
+			wacom->id[0] = ERASER_DEVICE_ID;
+		} else {
+			wacom->tool[0] = BTN_TOOL_PEN;
+			wacom->id[0] = STYLUS_DEVICE_ID;
 		}
+	}
+
+	wacom->shared->stylus_in_proximity = prox;
+	if (wacom->shared->touch_down)
+		return 0;
+
+	if (prox) {
 		x = le16_to_cpup((__le16 *)&data[2]);
 		y = le16_to_cpup((__le16 *)&data[4]);
 		p = le16_to_cpup((__le16 *)&data[6]);
@@ -1793,7 +1794,8 @@ static int wacom_bpt_pen(struct wacom_wac *wacom)
 		pen = data[1] & 0x01;
 		btn1 = data[1] & 0x02;
 		btn2 = data[1] & 0x04;
-	}
+	} else
+		wacom->id[0] = 0;
 
 	input_report_key(input, BTN_TOUCH, pen);
 	input_report_key(input, BTN_STYLUS, btn1);
@@ -1804,11 +1806,6 @@ static int wacom_bpt_pen(struct wacom_wac *wacom)
 	input_report_abs(input, ABS_PRESSURE, p);
 	input_report_abs(input, ABS_DISTANCE, d);
 
-	if (!prox) {
-		wacom->id[0] = 0;
-		wacom->shared->stylus_in_proximity = false;
-	}
-
 	input_report_key(input, wacom->tool[0], prox); /* PEN or RUBBER */
 	input_report_abs(input, ABS_MISC, wacom->id[0]); /* TOOL ID */
 
-- 
1.9.1

Re: [PATCH 3/3] HID: wacom: set stylus_in_proximity before checking touch_down

[Jason Gerecke]

On 3/20/2015 2:58 PM, Ping Cheng wrote:
> In wacom_bpt_pen, we checked touch_down before assigning new
> stylus_in_proximity value. This would cause stylus_in_proximity
> not updated properly if touch is down before pen is in proximity.
>
> Signed-off-by: Ping Cheng <pingc@wacom.com>
> ---
>   drivers/hid/wacom_wac.c | 35 ++++++++++++++++-------------------
>   1 file changed, 16 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 60d9ccb..63e767e 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -1752,9 +1752,6 @@ static int wacom_bpt_pen(struct wacom_wac *wacom)
>   		return 0;
>   	}
>
> -	if (wacom->shared->touch_down)
> -		return 0;
> -
>   	prox = (data[1] & 0x20) == 0x20;
>
>   	/*
> @@ -1767,17 +1764,21 @@ static int wacom_bpt_pen(struct wacom_wac *wacom)
>   	 *
>   	 * Hardware does report zero in most out-of-prox cases but not all.
>   	 */
> -	if (prox) {
> -		if (!wacom->shared->stylus_in_proximity) {
> -			if (data[1] & 0x08) {
> -				wacom->tool[0] = BTN_TOOL_RUBBER;
> -				wacom->id[0] = ERASER_DEVICE_ID;
> -			} else {
> -				wacom->tool[0] = BTN_TOOL_PEN;
> -				wacom->id[0] = STYLUS_DEVICE_ID;
> -			}
> -			wacom->shared->stylus_in_proximity = true;
> +	if (!wacom->shared->stylus_in_proximity) {
> +		if (data[1] & 0x08) {
> +			wacom->tool[0] = BTN_TOOL_RUBBER;
> +			wacom->id[0] = ERASER_DEVICE_ID;
> +		} else {
> +			wacom->tool[0] = BTN_TOOL_PEN;
> +			wacom->id[0] = STYLUS_DEVICE_ID;
>   		}
> +	}
> +
> +	wacom->shared->stylus_in_proximity = prox;
> +	if (wacom->shared->touch_down)
> +		return 0;
> +
> +	if (prox) {
>   		x = le16_to_cpup((__le16 *)&data[2]);
>   		y = le16_to_cpup((__le16 *)&data[4]);
>   		p = le16_to_cpup((__le16 *)&data[6]);
> @@ -1793,7 +1794,8 @@ static int wacom_bpt_pen(struct wacom_wac *wacom)
>   		pen = data[1] & 0x01;
>   		btn1 = data[1] & 0x02;
>   		btn2 = data[1] & 0x04;
> -	}
> +	} else
> +		wacom->id[0] = 0;

Nitpick: Add braces around this final block. Otherwise:

Reviewed-by: Jason Gerecke <jason.gerecke@wacom.com>

---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one /
(That is to say, eight) to the two, /
But you can’t take seven from three, /
So you look at the sixty-fours....

>
>   	input_report_key(input, BTN_TOUCH, pen);
>   	input_report_key(input, BTN_STYLUS, btn1);
> @@ -1804,11 +1806,6 @@ static int wacom_bpt_pen(struct wacom_wac *wacom)
>   	input_report_abs(input, ABS_PRESSURE, p);
>   	input_report_abs(input, ABS_DISTANCE, d);
>
> -	if (!prox) {
> -		wacom->id[0] = 0;
> -		wacom->shared->stylus_in_proximity = false;
> -	}
> -
>   	input_report_key(input, wacom->tool[0], prox); /* PEN or RUBBER */
>   	input_report_abs(input, ABS_MISC, wacom->id[0]); /* TOOL ID */
>
>
--
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