[PATCH 3/4] input - wacom : support one finger touch the touchscreen way

[PATCH 3/4] input - wacom : support one finger touch the touchscreen way

[Ping Cheng]

There are two types of 1FGT devices supported in wacom_wac.c.
Changing them to follow the existing touchscreen format, i.e.,
only report BTN_TOUCH as a valid tool type.

Touch data will be ignored if pen is in proximity. This requires
a touch up sent if touch was down when pen comes in. The touch up
event should be sent before any pen events emitted. Otherwise,
two pointers would race for the cursor.

However, we can not send a touch up inside wacom_tpc_pen since
pen and touch are on different logical port. That is why we
have to check if touch is up before sending pen events.

Reviewed-by: Chris Bagwell <chris@cnpbagwell.com>
Signed-off-by: Ping Cheng <pingc@wacom.com>
---
 drivers/input/tablet/wacom_wac.c |  125 +++++++++++++------------------------
 drivers/input/tablet/wacom_wac.h |    1 +
 2 files changed, 45 insertions(+), 81 deletions(-)

diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
index 15bab99..fc0669d 100644
--- a/drivers/input/tablet/wacom_wac.c
+++ b/drivers/input/tablet/wacom_wac.c
@@ -675,51 +675,35 @@ static int wacom_intuos_irq(struct wacom_wac *wacom)
 	return 1;
 }
 
-static void wacom_tpc_touch_out(struct wacom_wac *wacom, int idx)
-{
-	struct input_dev *input = wacom->input;
-	int finger = idx + 1;
-
-	input_report_abs(input, ABS_X, 0);
-	input_report_abs(input, ABS_Y, 0);
-	input_report_abs(input, ABS_MISC, 0);
-	input_report_key(input, wacom->tool[finger], 0);
-	if (!idx)
-		input_report_key(input, BTN_TOUCH, 0);
-	input_event(input, EV_MSC, MSC_SERIAL, finger);
-	input_sync(input);
-}
-
-static void wacom_tpc_touch_in(struct wacom_wac *wacom, size_t len)
+static int wacom_tpc_single_touch(struct wacom_wac *wacom, size_t len)
 {
 	char *data = wacom->data;
 	struct input_dev *input = wacom->input;
+	bool prox = 0;
 
-	wacom->tool[1] = BTN_TOOL_DOUBLETAP;
-	wacom->id[0] = TOUCH_DEVICE_ID;
-
-	if (len != WACOM_PKGLEN_TPC1FG) {
-
-		switch (data[0]) {
+	if (!wacom->shared->stylus_in_proximity) {
+		if (len == WACOM_PKGLEN_TPC1FG)
+			prox = data[0] & 0x01;
+		else  /* with capacity */
+			prox = data[1] & 0x01;
+	} else if (wacom->shared->touch_down)
+		prox = 0;
+	else
+		return 0;
 
-		case WACOM_REPORT_TPC1FG:
-			input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
-			input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
-			input_report_abs(input, ABS_PRESSURE, le16_to_cpup((__le16 *)&data[6]));
-			input_report_key(input, BTN_TOUCH, le16_to_cpup((__le16 *)&data[6]));
-			input_report_abs(input, ABS_MISC, wacom->id[0]);
-			input_report_key(input, wacom->tool[1], 1);
-			input_sync(input);
-			break;
-		}
-	} else {
+	if (len == WACOM_PKGLEN_TPC1FG) {
 		input_report_abs(input, ABS_X, get_unaligned_le16(&data[1]));
 		input_report_abs(input, ABS_Y, get_unaligned_le16(&data[3]));
-		input_report_key(input, BTN_TOUCH, 1);
-		input_report_abs(input, ABS_MISC, wacom->id[1]);
-		input_report_key(input, wacom->tool[1], 1);
-		input_sync(input);
+	} else {
+		input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
+		input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
 	}
+	input_report_key(input, BTN_TOUCH, prox);
+
+	/* keep prox bit to send proper out-prox event */
+	wacom->shared->touch_down = prox;
+
+	return 1;
 }
 
 static int wacom_tpc_pen(struct wacom_wac *wacom)
@@ -731,7 +715,7 @@ static int wacom_tpc_pen(struct wacom_wac *wacom)
 
 	prox = data[1] & 0x20;
 
-	if (!wacom->shared->stylus_in_proximity) { /* first in prox */
+	if (!wacom->shared->stylus_in_proximity) {
 		/* Going into proximity select tool */
 		wacom->tool[0] = (data[1] & 0x0c) ? BTN_TOOL_RUBBER : BTN_TOOL_PEN;
 		if (wacom->tool[0] == BTN_TOOL_PEN)
@@ -740,58 +724,36 @@ static int wacom_tpc_pen(struct wacom_wac *wacom)
 			wacom->id[0] = ERASER_DEVICE_ID;
 
 		wacom->shared->stylus_in_proximity = true;
+	} else if (!wacom->shared->touch_down) {
+		input_report_key(input, BTN_STYLUS, data[1] & 0x02);
+		input_report_key(input, BTN_STYLUS2, data[1] & 0x10);
+		input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
+		input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
+		pressure = ((data[7] & 0x01) << 8) | data[6];
+		if (pressure < 0)
+			pressure = features->pressure_max + pressure + 1;
+		input_report_abs(input, ABS_PRESSURE, pressure);
+		input_report_key(input, BTN_TOUCH, data[1] & 0x05);
+		if (!prox) { /* out-prox */
+			wacom->id[0] = 0;
+			wacom->shared->stylus_in_proximity = false;
+		}
+		input_report_key(input, wacom->tool[0], prox);
+		return 1;
 	}
-	input_report_key(input, BTN_STYLUS, data[1] & 0x02);
-	input_report_key(input, BTN_STYLUS2, data[1] & 0x10);
-	input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
-	input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
-	pressure = ((data[7] & 0x01) << 8) | data[6];
-	if (pressure < 0)
-		pressure = features->pressure_max + pressure + 1;
-	input_report_abs(input, ABS_PRESSURE, pressure);
-	input_report_key(input, BTN_TOUCH, data[1] & 0x05);
-	if (!prox) { /* out-prox */
-		wacom->id[0] = 0;
-		wacom->shared->stylus_in_proximity = false;
-	}
-	input_report_key(input, wacom->tool[0], prox);
 
-	return 1;
+	return 0;
 }
 
 static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len)
 {
 	char *data = wacom->data;
-	int prox = 0;
 
 	dbg("wacom_tpc_irq: received report #%d", data[0]);
 
-	if (len == WACOM_PKGLEN_TPC1FG ||
-	    data[0] == WACOM_REPORT_TPC1FG) {	/* single touch */
-
-		if (wacom->shared->stylus_in_proximity) {
-			if (wacom->id[1] & 0x01)
-				wacom_tpc_touch_out(wacom, 0);
-
-			wacom->id[1] = 0;
-			return 0;
-		}
-
-		if (len == WACOM_PKGLEN_TPC1FG)
-			prox = data[0] & 0x01;
-		else  /* with capacity */
-			prox = data[1] & 0x01;
-
-		if (prox)
-			wacom_tpc_touch_in(wacom, len);
-		else {
-			wacom_tpc_touch_out(wacom, 0);
-
-			wacom->id[0] = 0;
-		}
-		/* keep prox bit to send proper out-prox event */
-		wacom->id[1] = prox;
-	} else if (data[0] == WACOM_REPORT_PENABLED)
+	if (len == WACOM_PKGLEN_TPC1FG || data[0] == WACOM_REPORT_TPC1FG)
+		return wacom_tpc_single_touch(wacom, len);
+	else if (data[0] == WACOM_REPORT_PENABLED)
 		return wacom_tpc_pen(wacom);
 
 	return 0;
@@ -1172,6 +1134,8 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
 		/* fall through */
 
 	case TABLETPC:
+		__clear_bit(ABS_MISC, input_dev->absbit);
+
 		if (features->device_type == BTN_TOOL_DOUBLETAP ||
 		    features->device_type == BTN_TOOL_TRIPLETAP) {
 			input_abs_set_res(input_dev, ABS_X,
@@ -1180,7 +1144,6 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
 			input_abs_set_res(input_dev, ABS_Y,
 				wacom_calculate_touch_res(features->y_max,
 							features->y_phy));
-			__set_bit(BTN_TOOL_DOUBLETAP, input_dev->keybit);
 		}
 
 		if (features->device_type != BTN_TOOL_PEN)
diff --git a/drivers/input/tablet/wacom_wac.h b/drivers/input/tablet/wacom_wac.h
index 8f747dd..835f756 100644
--- a/drivers/input/tablet/wacom_wac.h
+++ b/drivers/input/tablet/wacom_wac.h
@@ -88,6 +88,7 @@ struct wacom_features {
 
 struct wacom_shared {
 	bool stylus_in_proximity;
+	bool touch_down;
 };
 
 struct wacom_wac {
-- 
1.7.4

Re: [PATCH 3/4] input - wacom : support one finger touch the touchscreen way

[Henrik Rydberg]

On Thu, Feb 10, 2011 at 05:32:42PM -0800, Ping Cheng wrote:
> There are two types of 1FGT devices supported in wacom_wac.c.
> Changing them to follow the existing touchscreen format, i.e.,
> only report BTN_TOUCH as a valid tool type.
> 
> Touch data will be ignored if pen is in proximity. This requires
> a touch up sent if touch was down when pen comes in. The touch up
> event should be sent before any pen events emitted. Otherwise,
> two pointers would race for the cursor.
> 
> However, we can not send a touch up inside wacom_tpc_pen since
> pen and touch are on different logical port. That is why we
> have to check if touch is up before sending pen events.
> 
> Reviewed-by: Chris Bagwell <chris@cnpbagwell.com>
> Signed-off-by: Ping Cheng <pingc@wacom.com>
> ---

Hi Ping,

>  drivers/input/tablet/wacom_wac.c |  125 +++++++++++++------------------------
>  drivers/input/tablet/wacom_wac.h |    1 +
>  2 files changed, 45 insertions(+), 81 deletions(-)
> 
> diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
> index 15bab99..fc0669d 100644
> --- a/drivers/input/tablet/wacom_wac.c
> +++ b/drivers/input/tablet/wacom_wac.c
> @@ -675,51 +675,35 @@ static int wacom_intuos_irq(struct wacom_wac *wacom)
>  	return 1;
>  }
>  
> -static void wacom_tpc_touch_out(struct wacom_wac *wacom, int idx)
> -{
> -	struct input_dev *input = wacom->input;
> -	int finger = idx + 1;
> -
> -	input_report_abs(input, ABS_X, 0);
> -	input_report_abs(input, ABS_Y, 0);
> -	input_report_abs(input, ABS_MISC, 0);
> -	input_report_key(input, wacom->tool[finger], 0);
> -	if (!idx)
> -		input_report_key(input, BTN_TOUCH, 0);
> -	input_event(input, EV_MSC, MSC_SERIAL, finger);
> -	input_sync(input);
> -}
> -
> -static void wacom_tpc_touch_in(struct wacom_wac *wacom, size_t len)
> +static int wacom_tpc_single_touch(struct wacom_wac *wacom, size_t len)
>  {
>  	char *data = wacom->data;
>  	struct input_dev *input = wacom->input;
> +	bool prox = 0;
>  
> -	wacom->tool[1] = BTN_TOOL_DOUBLETAP;
> -	wacom->id[0] = TOUCH_DEVICE_ID;
> -
> -	if (len != WACOM_PKGLEN_TPC1FG) {
> -
> -		switch (data[0]) {
> +	if (!wacom->shared->stylus_in_proximity) {
> +		if (len == WACOM_PKGLEN_TPC1FG)
> +			prox = data[0] & 0x01;
> +		else  /* with capacity */
> +			prox = data[1] & 0x01;
> +	} else if (wacom->shared->touch_down)
> +		prox = 0;
> +	else
> +		return 0;
>  
> -		case WACOM_REPORT_TPC1FG:
> -			input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
> -			input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
> -			input_report_abs(input, ABS_PRESSURE, le16_to_cpup((__le16 *)&data[6]));
> -			input_report_key(input, BTN_TOUCH, le16_to_cpup((__le16 *)&data[6]));
> -			input_report_abs(input, ABS_MISC, wacom->id[0]);
> -			input_report_key(input, wacom->tool[1], 1);
> -			input_sync(input);
> -			break;
> -		}
> -	} else {
> +	if (len == WACOM_PKGLEN_TPC1FG) {
>  		input_report_abs(input, ABS_X, get_unaligned_le16(&data[1]));
>  		input_report_abs(input, ABS_Y, get_unaligned_le16(&data[3]));
> -		input_report_key(input, BTN_TOUCH, 1);
> -		input_report_abs(input, ABS_MISC, wacom->id[1]);
> -		input_report_key(input, wacom->tool[1], 1);
> -		input_sync(input);
> +	} else {
> +		input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
> +		input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
>  	}
> +	input_report_key(input, BTN_TOUCH, prox);
> +
> +	/* keep prox bit to send proper out-prox event */
> +	wacom->shared->touch_down = prox;
> +
> +	return 1;
>  }
>  
>  static int wacom_tpc_pen(struct wacom_wac *wacom)
> @@ -731,7 +715,7 @@ static int wacom_tpc_pen(struct wacom_wac *wacom)
>  
>  	prox = data[1] & 0x20;
>  
> -	if (!wacom->shared->stylus_in_proximity) { /* first in prox */
> +	if (!wacom->shared->stylus_in_proximity) {
>  		/* Going into proximity select tool */
>  		wacom->tool[0] = (data[1] & 0x0c) ? BTN_TOOL_RUBBER : BTN_TOOL_PEN;
>  		if (wacom->tool[0] == BTN_TOOL_PEN)
> @@ -740,58 +724,36 @@ static int wacom_tpc_pen(struct wacom_wac *wacom)
>  			wacom->id[0] = ERASER_DEVICE_ID;
>  
>  		wacom->shared->stylus_in_proximity = true;

Relying on additional events coming in to execute the part below this
comment is not causing any troubles, then?

> +	} else if (!wacom->shared->touch_down) {
> +		input_report_key(input, BTN_STYLUS, data[1] & 0x02);
> +		input_report_key(input, BTN_STYLUS2, data[1] & 0x10);
> +		input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
> +		input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
> +		pressure = ((data[7] & 0x01) << 8) | data[6];
> +		if (pressure < 0)
> +			pressure = features->pressure_max + pressure + 1;

And that conversion again...

> +		input_report_abs(input, ABS_PRESSURE, pressure);
> +		input_report_key(input, BTN_TOUCH, data[1] & 0x05);
> +		if (!prox) { /* out-prox */
> +			wacom->id[0] = 0;
> +			wacom->shared->stylus_in_proximity = false;
> +		}
> +		input_report_key(input, wacom->tool[0], prox);
> +		return 1;
>  	}
> -	input_report_key(input, BTN_STYLUS, data[1] & 0x02);
> -	input_report_key(input, BTN_STYLUS2, data[1] & 0x10);
> -	input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
> -	input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
> -	pressure = ((data[7] & 0x01) << 8) | data[6];
> -	if (pressure < 0)
> -		pressure = features->pressure_max + pressure + 1;
> -	input_report_abs(input, ABS_PRESSURE, pressure);
> -	input_report_key(input, BTN_TOUCH, data[1] & 0x05);
> -	if (!prox) { /* out-prox */
> -		wacom->id[0] = 0;
> -		wacom->shared->stylus_in_proximity = false;
> -	}
> -	input_report_key(input, wacom->tool[0], prox);
>  
> -	return 1;
> +	return 0;
>  }
>  
>  static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len)
>  {
>  	char *data = wacom->data;
> -	int prox = 0;
>  
>  	dbg("wacom_tpc_irq: received report #%d", data[0]);
>  
> -	if (len == WACOM_PKGLEN_TPC1FG ||
> -	    data[0] == WACOM_REPORT_TPC1FG) {	/* single touch */
> -
> -		if (wacom->shared->stylus_in_proximity) {
> -			if (wacom->id[1] & 0x01)
> -				wacom_tpc_touch_out(wacom, 0);
> -
> -			wacom->id[1] = 0;
> -			return 0;
> -		}
> -
> -		if (len == WACOM_PKGLEN_TPC1FG)
> -			prox = data[0] & 0x01;
> -		else  /* with capacity */
> -			prox = data[1] & 0x01;
> -
> -		if (prox)
> -			wacom_tpc_touch_in(wacom, len);
> -		else {
> -			wacom_tpc_touch_out(wacom, 0);
> -
> -			wacom->id[0] = 0;
> -		}
> -		/* keep prox bit to send proper out-prox event */
> -		wacom->id[1] = prox;
> -	} else if (data[0] == WACOM_REPORT_PENABLED)
> +	if (len == WACOM_PKGLEN_TPC1FG || data[0] == WACOM_REPORT_TPC1FG)
> +		return wacom_tpc_single_touch(wacom, len);
> +	else if (data[0] == WACOM_REPORT_PENABLED)
>  		return wacom_tpc_pen(wacom);
>  
>  	return 0;
> @@ -1172,6 +1134,8 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
>  		/* fall through */
>  
>  	case TABLETPC:
> +		__clear_bit(ABS_MISC, input_dev->absbit);
> +
>  		if (features->device_type == BTN_TOOL_DOUBLETAP ||
>  		    features->device_type == BTN_TOOL_TRIPLETAP) {
>  			input_abs_set_res(input_dev, ABS_X,
> @@ -1180,7 +1144,6 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
>  			input_abs_set_res(input_dev, ABS_Y,
>  				wacom_calculate_touch_res(features->y_max,
>  							features->y_phy));
> -			__set_bit(BTN_TOOL_DOUBLETAP, input_dev->keybit);

Where is doubletap set now?

>  		}
>  
>  		if (features->device_type != BTN_TOOL_PEN)
> diff --git a/drivers/input/tablet/wacom_wac.h b/drivers/input/tablet/wacom_wac.h
> index 8f747dd..835f756 100644
> --- a/drivers/input/tablet/wacom_wac.h
> +++ b/drivers/input/tablet/wacom_wac.h
> @@ -88,6 +88,7 @@ struct wacom_features {
>  
>  struct wacom_shared {
>  	bool stylus_in_proximity;
> +	bool touch_down;
>  };
>  
>  struct wacom_wac {
> -- 

Thanks,
Henrik

Re: [PATCH 3/4] input - wacom : support one finger touch the touchscreen way

[Ping Cheng]

On Fri, Feb 11, 2011 at 11:56 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
> On Thu, Feb 10, 2011 at 05:32:42PM -0800, Ping Cheng wrote:
>> There are two types of 1FGT devices supported in wacom_wac.c.
>> Changing them to follow the existing touchscreen format, i.e.,
>> only report BTN_TOUCH as a valid tool type.
>>
>> Touch data will be ignored if pen is in proximity. This requires
>> a touch up sent if touch was down when pen comes in. The touch up
>> event should be sent before any pen events emitted. Otherwise,
>> two pointers would race for the cursor.
>>
>> However, we can not send a touch up inside wacom_tpc_pen since
>> pen and touch are on different logical port. That is why we
>> have to check if touch is up before sending pen events.
>>
>> Reviewed-by: Chris Bagwell <chris@cnpbagwell.com>
>> Signed-off-by: Ping Cheng <pingc@wacom.com>
>> ---
>
> Hi Ping,
>
>>  drivers/input/tablet/wacom_wac.c |  125 +++++++++++++------------------------
>>  drivers/input/tablet/wacom_wac.h |    1 +
>>  2 files changed, 45 insertions(+), 81 deletions(-)
>>
>> diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
>> index 15bab99..fc0669d 100644
>> --- a/drivers/input/tablet/wacom_wac.c
>> +++ b/drivers/input/tablet/wacom_wac.c
>> @@ -675,51 +675,35 @@ static int wacom_intuos_irq(struct wacom_wac *wacom)
>>       return 1;
>>  }
>>
>> -static void wacom_tpc_touch_out(struct wacom_wac *wacom, int idx)
>> -{
>> -     struct input_dev *input = wacom->input;
>> -     int finger = idx + 1;
>> -
>> -     input_report_abs(input, ABS_X, 0);
>> -     input_report_abs(input, ABS_Y, 0);
>> -     input_report_abs(input, ABS_MISC, 0);
>> -     input_report_key(input, wacom->tool[finger], 0);
>> -     if (!idx)
>> -             input_report_key(input, BTN_TOUCH, 0);
>> -     input_event(input, EV_MSC, MSC_SERIAL, finger);
>> -     input_sync(input);
>> -}
>> -
>> -static void wacom_tpc_touch_in(struct wacom_wac *wacom, size_t len)
>> +static int wacom_tpc_single_touch(struct wacom_wac *wacom, size_t len)
>>  {
>>       char *data = wacom->data;
>>       struct input_dev *input = wacom->input;
>> +     bool prox = 0;
>>
>> -     wacom->tool[1] = BTN_TOOL_DOUBLETAP;
>> -     wacom->id[0] = TOUCH_DEVICE_ID;
>> -
>> -     if (len != WACOM_PKGLEN_TPC1FG) {
>> -
>> -             switch (data[0]) {
>> +     if (!wacom->shared->stylus_in_proximity) {
>> +             if (len == WACOM_PKGLEN_TPC1FG)
>> +                     prox = data[0] & 0x01;
>> +             else  /* with capacity */
>> +                     prox = data[1] & 0x01;
>> +     } else if (wacom->shared->touch_down)
>> +             prox = 0;
>> +     else
>> +             return 0;
>>
>> -             case WACOM_REPORT_TPC1FG:
>> -                     input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
>> -                     input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
>> -                     input_report_abs(input, ABS_PRESSURE, le16_to_cpup((__le16 *)&data[6]));
>> -                     input_report_key(input, BTN_TOUCH, le16_to_cpup((__le16 *)&data[6]));
>> -                     input_report_abs(input, ABS_MISC, wacom->id[0]);
>> -                     input_report_key(input, wacom->tool[1], 1);
>> -                     input_sync(input);
>> -                     break;
>> -             }
>> -     } else {
>> +     if (len == WACOM_PKGLEN_TPC1FG) {
>>               input_report_abs(input, ABS_X, get_unaligned_le16(&data[1]));
>>               input_report_abs(input, ABS_Y, get_unaligned_le16(&data[3]));
>> -             input_report_key(input, BTN_TOUCH, 1);
>> -             input_report_abs(input, ABS_MISC, wacom->id[1]);
>> -             input_report_key(input, wacom->tool[1], 1);
>> -             input_sync(input);
>> +     } else {
>> +             input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
>> +             input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
>>       }
>> +     input_report_key(input, BTN_TOUCH, prox);
>> +
>> +     /* keep prox bit to send proper out-prox event */
>> +     wacom->shared->touch_down = prox;
>> +
>> +     return 1;
>>  }
>>
>>  static int wacom_tpc_pen(struct wacom_wac *wacom)
>> @@ -731,7 +715,7 @@ static int wacom_tpc_pen(struct wacom_wac *wacom)
>>
>>       prox = data[1] & 0x20;
>>
>> -     if (!wacom->shared->stylus_in_proximity) { /* first in prox */
>> +     if (!wacom->shared->stylus_in_proximity) {
>>               /* Going into proximity select tool */
>>               wacom->tool[0] = (data[1] & 0x0c) ? BTN_TOOL_RUBBER : BTN_TOOL_PEN;
>>               if (wacom->tool[0] == BTN_TOOL_PEN)
>> @@ -740,58 +724,36 @@ static int wacom_tpc_pen(struct wacom_wac *wacom)
>>                       wacom->id[0] = ERASER_DEVICE_ID;
>>
>>               wacom->shared->stylus_in_proximity = true;
>
> Relying on additional events coming in to execute the part below this
> comment is not causing any troubles, then?

That is right. My testing result shows a smooth transition between
touch and pen with this addition. This logical should be applied to
Bamboo pen and touch as well (I see the same issue there). But, it is
outside of this patchset. So, hopefully someone will make that change
later.

>> +     } else if (!wacom->shared->touch_down) {
>> +             input_report_key(input, BTN_STYLUS, data[1] & 0x02);
>> +             input_report_key(input, BTN_STYLUS2, data[1] & 0x10);
>> +             input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
>> +             input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
>> +             pressure = ((data[7] & 0x01) << 8) | data[6];
>> +             if (pressure < 0)
>> +                     pressure = features->pressure_max + pressure + 1;
>
> And that conversion again...

Well, no code change. Just move them around.

>> +             input_report_abs(input, ABS_PRESSURE, pressure);
>> +             input_report_key(input, BTN_TOUCH, data[1] & 0x05);
>> +             if (!prox) { /* out-prox */
>> +                     wacom->id[0] = 0;
>> +                     wacom->shared->stylus_in_proximity = false;
>> +             }
>> +             input_report_key(input, wacom->tool[0], prox);
>> +             return 1;
>>       }
>> -     input_report_key(input, BTN_STYLUS, data[1] & 0x02);
>> -     input_report_key(input, BTN_STYLUS2, data[1] & 0x10);
>> -     input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
>> -     input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
>> -     pressure = ((data[7] & 0x01) << 8) | data[6];
>> -     if (pressure < 0)
>> -             pressure = features->pressure_max + pressure + 1;
>> -     input_report_abs(input, ABS_PRESSURE, pressure);
>> -     input_report_key(input, BTN_TOUCH, data[1] & 0x05);
>> -     if (!prox) { /* out-prox */
>> -             wacom->id[0] = 0;
>> -             wacom->shared->stylus_in_proximity = false;
>> -     }
>> -     input_report_key(input, wacom->tool[0], prox);
>>
>> -     return 1;
>> +     return 0;
>>  }
>>
>>  static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len)
>>  {
>>       char *data = wacom->data;
>> -     int prox = 0;
>>
>>       dbg("wacom_tpc_irq: received report #%d", data[0]);
>>
>> -     if (len == WACOM_PKGLEN_TPC1FG ||
>> -         data[0] == WACOM_REPORT_TPC1FG) {   /* single touch */
>> -
>> -             if (wacom->shared->stylus_in_proximity) {
>> -                     if (wacom->id[1] & 0x01)
>> -                             wacom_tpc_touch_out(wacom, 0);
>> -
>> -                     wacom->id[1] = 0;
>> -                     return 0;
>> -             }
>> -
>> -             if (len == WACOM_PKGLEN_TPC1FG)
>> -                     prox = data[0] & 0x01;
>> -             else  /* with capacity */
>> -                     prox = data[1] & 0x01;
>> -
>> -             if (prox)
>> -                     wacom_tpc_touch_in(wacom, len);
>> -             else {
>> -                     wacom_tpc_touch_out(wacom, 0);
>> -
>> -                     wacom->id[0] = 0;
>> -             }
>> -             /* keep prox bit to send proper out-prox event */
>> -             wacom->id[1] = prox;
>> -     } else if (data[0] == WACOM_REPORT_PENABLED)
>> +     if (len == WACOM_PKGLEN_TPC1FG || data[0] == WACOM_REPORT_TPC1FG)
>> +             return wacom_tpc_single_touch(wacom, len);
>> +     else if (data[0] == WACOM_REPORT_PENABLED)
>>               return wacom_tpc_pen(wacom);
>>
>>       return 0;
>> @@ -1172,6 +1134,8 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
>>               /* fall through */
>>
>>       case TABLETPC:
>> +             __clear_bit(ABS_MISC, input_dev->absbit);
>> +
>>               if (features->device_type == BTN_TOOL_DOUBLETAP ||
>>                   features->device_type == BTN_TOOL_TRIPLETAP) {
>>                       input_abs_set_res(input_dev, ABS_X,
>> @@ -1180,7 +1144,6 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
>>                       input_abs_set_res(input_dev, ABS_Y,
>>                               wacom_calculate_touch_res(features->y_max,
>>                                                       features->y_phy));
>> -                     __set_bit(BTN_TOOL_DOUBLETAP, input_dev->keybit);
>
> Where is doubletap set now?

Oops. I must have missed that in 4/4. I'll add it in the new version
after I hear from you for the other comments.

I didn't think touchscreen needs a DOUBLETAP. It is meant for
touchpad. However, I have agreed with Chris to add DOUBLETAP anyway.
Having worked on this set for so long, I guess I must have
accidentally removed it somehow...

>>               }
>>
>>               if (features->device_type != BTN_TOOL_PEN)
>> diff --git a/drivers/input/tablet/wacom_wac.h b/drivers/input/tablet/wacom_wac.h
>> index 8f747dd..835f756 100644
>> --- a/drivers/input/tablet/wacom_wac.h
>> +++ b/drivers/input/tablet/wacom_wac.h
>> @@ -88,6 +88,7 @@ struct wacom_features {
>>
>>  struct wacom_shared {
>>       bool stylus_in_proximity;
>> +     bool touch_down;
>>  };
>>
>>  struct wacom_wac {
>> --
>
> Thanks,
> Henrik
>
--
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

Re: [PATCH 3/4] input - wacom : support one finger touch the touchscreen way

[Ping Cheng]

On Fri, Feb 11, 2011 at 12:24 PM, Ping Cheng <pinglinux@gmail.com> wrote:
> On Fri, Feb 11, 2011 at 11:56 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
>> On Thu, Feb 10, 2011 at 05:32:42PM -0800, Ping Cheng wrote:
>>> There are two types of 1FGT devices supported in wacom_wac.c.
>>> Changing them to follow the existing touchscreen format, i.e.,
>>> only report BTN_TOUCH as a valid tool type.
>>>
>>> Touch data will be ignored if pen is in proximity. This requires
>>> a touch up sent if touch was down when pen comes in. The touch up
>>> event should be sent before any pen events emitted. Otherwise,
>>> two pointers would race for the cursor.
>>>
>>> However, we can not send a touch up inside wacom_tpc_pen since
>>> pen and touch are on different logical port. That is why we
>>> have to check if touch is up before sending pen events.
>>>
>>> Reviewed-by: Chris Bagwell <chris@cnpbagwell.com>
>>> Signed-off-by: Ping Cheng <pingc@wacom.com>
>>> ---
>>
>> Hi Ping,
>>
>>>  drivers/input/tablet/wacom_wac.c |  125 +++++++++++++------------------------
>>>  drivers/input/tablet/wacom_wac.h |    1 +
>>>  2 files changed, 45 insertions(+), 81 deletions(-)
>>>
>>> diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
>>> index 15bab99..fc0669d 100644
>>> --- a/drivers/input/tablet/wacom_wac.c
>>> +++ b/drivers/input/tablet/wacom_wac.c
>>> @@ -675,51 +675,35 @@ static int wacom_intuos_irq(struct wacom_wac *wacom)
>>>       return 1;
>>>  }
>>>
>>> -static void wacom_tpc_touch_out(struct wacom_wac *wacom, int idx)
>>> -{
>>> -     struct input_dev *input = wacom->input;
>>> -     int finger = idx + 1;
>>> -
>>> -     input_report_abs(input, ABS_X, 0);
>>> -     input_report_abs(input, ABS_Y, 0);
>>> -     input_report_abs(input, ABS_MISC, 0);
>>> -     input_report_key(input, wacom->tool[finger], 0);
>>> -     if (!idx)
>>> -             input_report_key(input, BTN_TOUCH, 0);
>>> -     input_event(input, EV_MSC, MSC_SERIAL, finger);
>>> -     input_sync(input);
>>> -}
>>> -
>>> -static void wacom_tpc_touch_in(struct wacom_wac *wacom, size_t len)
>>> +static int wacom_tpc_single_touch(struct wacom_wac *wacom, size_t len)
>>>  {
>>>       char *data = wacom->data;
>>>       struct input_dev *input = wacom->input;
>>> +     bool prox = 0;
>>>
>>> -     wacom->tool[1] = BTN_TOOL_DOUBLETAP;
>>> -     wacom->id[0] = TOUCH_DEVICE_ID;
>>> -
>>> -     if (len != WACOM_PKGLEN_TPC1FG) {
>>> -
>>> -             switch (data[0]) {
>>> +     if (!wacom->shared->stylus_in_proximity) {
>>> +             if (len == WACOM_PKGLEN_TPC1FG)
>>> +                     prox = data[0] & 0x01;
>>> +             else  /* with capacity */
>>> +                     prox = data[1] & 0x01;
>>> +     } else if (wacom->shared->touch_down)
>>> +             prox = 0;
>>> +     else
>>> +             return 0;
>>>
>>> -             case WACOM_REPORT_TPC1FG:
>>> -                     input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
>>> -                     input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
>>> -                     input_report_abs(input, ABS_PRESSURE, le16_to_cpup((__le16 *)&data[6]));
>>> -                     input_report_key(input, BTN_TOUCH, le16_to_cpup((__le16 *)&data[6]));
>>> -                     input_report_abs(input, ABS_MISC, wacom->id[0]);
>>> -                     input_report_key(input, wacom->tool[1], 1);
>>> -                     input_sync(input);
>>> -                     break;
>>> -             }
>>> -     } else {
>>> +     if (len == WACOM_PKGLEN_TPC1FG) {
>>>               input_report_abs(input, ABS_X, get_unaligned_le16(&data[1]));
>>>               input_report_abs(input, ABS_Y, get_unaligned_le16(&data[3]));
>>> -             input_report_key(input, BTN_TOUCH, 1);
>>> -             input_report_abs(input, ABS_MISC, wacom->id[1]);
>>> -             input_report_key(input, wacom->tool[1], 1);
>>> -             input_sync(input);
>>> +     } else {
>>> +             input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
>>> +             input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
>>>       }
>>> +     input_report_key(input, BTN_TOUCH, prox);
>>> +
>>> +     /* keep prox bit to send proper out-prox event */
>>> +     wacom->shared->touch_down = prox;
>>> +
>>> +     return 1;
>>>  }
>>>
>>>  static int wacom_tpc_pen(struct wacom_wac *wacom)
>>> @@ -731,7 +715,7 @@ static int wacom_tpc_pen(struct wacom_wac *wacom)
>>>
>>>       prox = data[1] & 0x20;
>>>
>>> -     if (!wacom->shared->stylus_in_proximity) { /* first in prox */
>>> +     if (!wacom->shared->stylus_in_proximity) {
>>>               /* Going into proximity select tool */
>>>               wacom->tool[0] = (data[1] & 0x0c) ? BTN_TOOL_RUBBER : BTN_TOOL_PEN;
>>>               if (wacom->tool[0] == BTN_TOOL_PEN)
>>> @@ -740,58 +724,36 @@ static int wacom_tpc_pen(struct wacom_wac *wacom)
>>>                       wacom->id[0] = ERASER_DEVICE_ID;
>>>
>>>               wacom->shared->stylus_in_proximity = true;
>>
>> Relying on additional events coming in to execute the part below this
>> comment is not causing any troubles, then?
>
> That is right. My testing result shows a smooth transition between
> touch and pen with this addition. This logical should be applied to
> Bamboo pen and touch as well (I see the same issue there). But, it is
> outside of this patchset. So, hopefully someone will make that change
> later.
>
>>> +     } else if (!wacom->shared->touch_down) {
>>> +             input_report_key(input, BTN_STYLUS, data[1] & 0x02);
>>> +             input_report_key(input, BTN_STYLUS2, data[1] & 0x10);
>>> +             input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
>>> +             input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
>>> +             pressure = ((data[7] & 0x01) << 8) | data[6];
>>> +             if (pressure < 0)
>>> +                     pressure = features->pressure_max + pressure + 1;
>>
>> And that conversion again...
>
> Well, no code change. Just move them around.
>
>>> +             input_report_abs(input, ABS_PRESSURE, pressure);
>>> +             input_report_key(input, BTN_TOUCH, data[1] & 0x05);
>>> +             if (!prox) { /* out-prox */
>>> +                     wacom->id[0] = 0;
>>> +                     wacom->shared->stylus_in_proximity = false;
>>> +             }
>>> +             input_report_key(input, wacom->tool[0], prox);
>>> +             return 1;
>>>       }
>>> -     input_report_key(input, BTN_STYLUS, data[1] & 0x02);
>>> -     input_report_key(input, BTN_STYLUS2, data[1] & 0x10);
>>> -     input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
>>> -     input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
>>> -     pressure = ((data[7] & 0x01) << 8) | data[6];
>>> -     if (pressure < 0)
>>> -             pressure = features->pressure_max + pressure + 1;
>>> -     input_report_abs(input, ABS_PRESSURE, pressure);
>>> -     input_report_key(input, BTN_TOUCH, data[1] & 0x05);
>>> -     if (!prox) { /* out-prox */
>>> -             wacom->id[0] = 0;
>>> -             wacom->shared->stylus_in_proximity = false;
>>> -     }
>>> -     input_report_key(input, wacom->tool[0], prox);
>>>
>>> -     return 1;
>>> +     return 0;
>>>  }
>>>
>>>  static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len)
>>>  {
>>>       char *data = wacom->data;
>>> -     int prox = 0;
>>>
>>>       dbg("wacom_tpc_irq: received report #%d", data[0]);
>>>
>>> -     if (len == WACOM_PKGLEN_TPC1FG ||
>>> -         data[0] == WACOM_REPORT_TPC1FG) {   /* single touch */
>>> -
>>> -             if (wacom->shared->stylus_in_proximity) {
>>> -                     if (wacom->id[1] & 0x01)
>>> -                             wacom_tpc_touch_out(wacom, 0);
>>> -
>>> -                     wacom->id[1] = 0;
>>> -                     return 0;
>>> -             }
>>> -
>>> -             if (len == WACOM_PKGLEN_TPC1FG)
>>> -                     prox = data[0] & 0x01;
>>> -             else  /* with capacity */
>>> -                     prox = data[1] & 0x01;
>>> -
>>> -             if (prox)
>>> -                     wacom_tpc_touch_in(wacom, len);
>>> -             else {
>>> -                     wacom_tpc_touch_out(wacom, 0);
>>> -
>>> -                     wacom->id[0] = 0;
>>> -             }
>>> -             /* keep prox bit to send proper out-prox event */
>>> -             wacom->id[1] = prox;
>>> -     } else if (data[0] == WACOM_REPORT_PENABLED)
>>> +     if (len == WACOM_PKGLEN_TPC1FG || data[0] == WACOM_REPORT_TPC1FG)
>>> +             return wacom_tpc_single_touch(wacom, len);
>>> +     else if (data[0] == WACOM_REPORT_PENABLED)
>>>               return wacom_tpc_pen(wacom);
>>>
>>>       return 0;
>>> @@ -1172,6 +1134,8 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
>>>               /* fall through */
>>>
>>>       case TABLETPC:
>>> +             __clear_bit(ABS_MISC, input_dev->absbit);
>>> +
>>>               if (features->device_type == BTN_TOOL_DOUBLETAP ||
>>>                   features->device_type == BTN_TOOL_TRIPLETAP) {
>>>                       input_abs_set_res(input_dev, ABS_X,
>>> @@ -1180,7 +1144,6 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
>>>                       input_abs_set_res(input_dev, ABS_Y,
>>>                               wacom_calculate_touch_res(features->y_max,
>>>                                                       features->y_phy));
>>> -                     __set_bit(BTN_TOOL_DOUBLETAP, input_dev->keybit);
>>
>> Where is doubletap set now?
>
> Oops. I must have missed that in 4/4. I'll add it in the new version
> after I hear from you for the other comments.

Well, DOUBLETAP has been added in 4/4 already. Please take a close
look. Too soon to admit an error ;).

Ping

> I didn't think touchscreen needs a DOUBLETAP. It is meant for
> touchpad. However, I have agreed with Chris to add DOUBLETAP anyway.
> Having worked on this set for so long, I guess I must have
> accidentally removed it somehow...
>
>>>               }
>>>
>>>               if (features->device_type != BTN_TOOL_PEN)
>>> diff --git a/drivers/input/tablet/wacom_wac.h b/drivers/input/tablet/wacom_wac.h
>>> index 8f747dd..835f756 100644
>>> --- a/drivers/input/tablet/wacom_wac.h
>>> +++ b/drivers/input/tablet/wacom_wac.h
>>> @@ -88,6 +88,7 @@ struct wacom_features {
>>>
>>>  struct wacom_shared {
>>>       bool stylus_in_proximity;
>>> +     bool touch_down;
>>>  };
>>>
>>>  struct wacom_wac {
>>> --
>>
>> Thanks,
>> Henrik
>>
>
--
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

Re: [PATCH 3/4] input - wacom : support one finger touch the touchscreen way

[Chris Bagwell]

On Fri, Feb 11, 2011 at 2:40 PM, Ping Cheng <pinglinux@gmail.com> wrote:
> On Fri, Feb 11, 2011 at 12:24 PM, Ping Cheng <pinglinux@gmail.com> wrote:
>> On Fri, Feb 11, 2011 at 11:56 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
>>>> @@ -1180,7 +1144,6 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
>>>>                       input_abs_set_res(input_dev, ABS_Y,
>>>>                               wacom_calculate_touch_res(features->y_max,
>>>>                                                       features->y_phy));
>>>> -                     __set_bit(BTN_TOOL_DOUBLETAP, input_dev->keybit);
>>>
>>> Where is doubletap set now?
>>
>> Oops. I must have missed that in 4/4. I'll add it in the new version
>> after I hear from you for the other comments.
>
> Well, DOUBLETAP has been added in 4/4 already. Please take a close
> look. Too soon to admit an error ;).
>

Extra clarification on whats going on here:

Patch #1 removes BTN_TOOL_TRIPLETAP; among other supporting 2 finger
touch code.  In that context, it was used as an awkward indication to
xf86-input-wacom that device supported 2 finger touch.

In this patch #3, it removes BTN_TOOL_DOUBLETAP; which was an awkward
indication to xf86-input-wacom that device supports 1 finger touch.
Once this patch is applied, applications must deduce touchscreen input
supports 1 finger touch same way as they do for other touchscreens
(not seeing BTN_TOOL_FINGER and no stylus related BTN_'s as well).

Patch #4 adds BTN_TOOL_DOUBLETAP back but using normal meaning that
the device supports 2 finger touch.

I had reviewed this patch series in context of some related changes
from Ping to xf86-input-wacom.  I wanted to document doubletap part of
that review on this list though.

Ping originally didn't want to add that back in.  At least one of the
reasons was that there were no known applications that supported both
"touchscreens" (absolute mode) and the BTN_TOOL_DOUBLETAP at same
time.

I did look around quite a bit after Ping's comment and I couldn't come
up with anything (mtdev, xf86-input-evdev, etc).  So its a good point.
 When something supports ABS_MT_*, thats given much more weight then
BTN_TOOL_DOUPLETAP would; which is the case here.

Anyways, I requested if Ping would add BTN_TOOL_DOUBLETAP in to align
with current hid-ntrig and since it didn't seem to hurt and could
possibly help some future app.  Please double check me here if it is
in fact a good idea to add DOUBLETAP in for touchscreens.

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

Re: [PATCH 3/4] input - wacom : support one finger touch the touchscreen way

[Henrik Rydberg]

Hi Chis,

> Extra clarification on whats going on here:
> 
> Patch #1 removes BTN_TOOL_TRIPLETAP; among other supporting 2 finger
> touch code.  In that context, it was used as an awkward indication to
> xf86-input-wacom that device supported 2 finger touch.

Which is great, no argument there.

> In this patch #3, it removes BTN_TOOL_DOUBLETAP; which was an awkward
> indication to xf86-input-wacom that device supports 1 finger touch.
> Once this patch is applied, applications must deduce touchscreen input
> supports 1 finger touch same way as they do for other touchscreens
> (not seeing BTN_TOOL_FINGER and no stylus related BTN_'s as well).

Also makes sense - I only noticed that it was gone without
replacement, and Ping's and your answer gives ample rationale for its
removal.

> Patch #4 adds BTN_TOOL_DOUBLETAP back but using normal meaning that
> the device supports 2 finger touch.

Right, thanks.

> I had reviewed this patch series in context of some related changes
> from Ping to xf86-input-wacom.  I wanted to document doubletap part of
> that review on this list though.
> 
> Ping originally didn't want to add that back in.  At least one of the
> reasons was that there were no known applications that supported both
> "touchscreens" (absolute mode) and the BTN_TOOL_DOUBLETAP at same
> time.

Given the current state of userland, a touchscreen should not emit
doubletaps, true.

> I did look around quite a bit after Ping's comment and I couldn't come
> up with anything (mtdev, xf86-input-evdev, etc).  So its a good point.
>  When something supports ABS_MT_*, thats given much more weight then
> BTN_TOOL_DOUPLETAP would; which is the case here.

Yes - unless the device is of the SEMI_MT kind, the finger-counting
BTN_TOOL_* events really have no meaning.
 
> Anyways, I requested if Ping would add BTN_TOOL_DOUBLETAP in to align
> with current hid-ntrig and since it didn't seem to hurt and could
> possibly help some future app.  Please double check me here if it is
> in fact a good idea to add DOUBLETAP in for touchscreens.

In fact, I think it hurts a bit, so removing it is a good idea.

Thanks,
Henrik

[PATCH 3/4] input - wacom : support one finger touch the touchscreen way

[Ping Cheng]

There are two types of 1FGT devices supported in wacom_wac.c.
Changing them to follow the existing touchscreen format, i.e.,
only report BTN_TOUCH as a valid tool type.

Touch data will be ignored if pen is in proximity. This requires
a touch up sent if touch was down when pen comes in. The touch up
event should be sent before any pen events emitted. Otherwise,
two pointers would race for the cursor.

However, we can not send a touch up inside wacom_tpc_pen since
pen and touch are on different logical port. That is why we
have to check if touch is up before sending pen events.

Reviewed-by: Chris Bagwell <chris@cnpbagwell.com>
Signed-off-by: Ping Cheng <pingc@wacom.com>
---
 drivers/input/tablet/wacom_wac.c |  125 +++++++++++++------------------------
 drivers/input/tablet/wacom_wac.h |    1 +
 2 files changed, 45 insertions(+), 81 deletions(-)

diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
index 15bab99..fc0669d 100644
--- a/drivers/input/tablet/wacom_wac.c
+++ b/drivers/input/tablet/wacom_wac.c
@@ -675,51 +675,35 @@ static int wacom_intuos_irq(struct wacom_wac *wacom)
 	return 1;
 }
 
-static void wacom_tpc_touch_out(struct wacom_wac *wacom, int idx)
-{
-	struct input_dev *input = wacom->input;
-	int finger = idx + 1;
-
-	input_report_abs(input, ABS_X, 0);
-	input_report_abs(input, ABS_Y, 0);
-	input_report_abs(input, ABS_MISC, 0);
-	input_report_key(input, wacom->tool[finger], 0);
-	if (!idx)
-		input_report_key(input, BTN_TOUCH, 0);
-	input_event(input, EV_MSC, MSC_SERIAL, finger);
-	input_sync(input);
-}
-
-static void wacom_tpc_touch_in(struct wacom_wac *wacom, size_t len)
+static int wacom_tpc_single_touch(struct wacom_wac *wacom, size_t len)
 {
 	char *data = wacom->data;
 	struct input_dev *input = wacom->input;
+	bool prox = 0;
 
-	wacom->tool[1] = BTN_TOOL_DOUBLETAP;
-	wacom->id[0] = TOUCH_DEVICE_ID;
-
-	if (len != WACOM_PKGLEN_TPC1FG) {
-
-		switch (data[0]) {
+	if (!wacom->shared->stylus_in_proximity) {
+		if (len == WACOM_PKGLEN_TPC1FG)
+			prox = data[0] & 0x01;
+		else  /* with capacity */
+			prox = data[1] & 0x01;
+	} else if (wacom->shared->touch_down)
+		prox = 0;
+	else
+		return 0;
 
-		case WACOM_REPORT_TPC1FG:
-			input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
-			input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
-			input_report_abs(input, ABS_PRESSURE, le16_to_cpup((__le16 *)&data[6]));
-			input_report_key(input, BTN_TOUCH, le16_to_cpup((__le16 *)&data[6]));
-			input_report_abs(input, ABS_MISC, wacom->id[0]);
-			input_report_key(input, wacom->tool[1], 1);
-			input_sync(input);
-			break;
-		}
-	} else {
+	if (len == WACOM_PKGLEN_TPC1FG) {
 		input_report_abs(input, ABS_X, get_unaligned_le16(&data[1]));
 		input_report_abs(input, ABS_Y, get_unaligned_le16(&data[3]));
-		input_report_key(input, BTN_TOUCH, 1);
-		input_report_abs(input, ABS_MISC, wacom->id[1]);
-		input_report_key(input, wacom->tool[1], 1);
-		input_sync(input);
+	} else {
+		input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
+		input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
 	}
+	input_report_key(input, BTN_TOUCH, prox);
+
+	/* keep prox bit to send proper out-prox event */
+	wacom->shared->touch_down = prox;
+
+	return 1;
 }
 
 static int wacom_tpc_pen(struct wacom_wac *wacom)
@@ -731,7 +715,7 @@ static int wacom_tpc_pen(struct wacom_wac *wacom)
 
 	prox = data[1] & 0x20;
 
-	if (!wacom->shared->stylus_in_proximity) { /* first in prox */
+	if (!wacom->shared->stylus_in_proximity) {
 		/* Going into proximity select tool */
 		wacom->tool[0] = (data[1] & 0x0c) ? BTN_TOOL_RUBBER : BTN_TOOL_PEN;
 		if (wacom->tool[0] == BTN_TOOL_PEN)
@@ -740,58 +724,36 @@ static int wacom_tpc_pen(struct wacom_wac *wacom)
 			wacom->id[0] = ERASER_DEVICE_ID;
 
 		wacom->shared->stylus_in_proximity = true;
+	} else if (!wacom->shared->touch_down) {
+		input_report_key(input, BTN_STYLUS, data[1] & 0x02);
+		input_report_key(input, BTN_STYLUS2, data[1] & 0x10);
+		input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
+		input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
+		pressure = ((data[7] & 0x01) << 8) | data[6];
+		if (pressure < 0)
+			pressure = features->pressure_max + pressure + 1;
+		input_report_abs(input, ABS_PRESSURE, pressure);
+		input_report_key(input, BTN_TOUCH, data[1] & 0x05);
+		if (!prox) { /* out-prox */
+			wacom->id[0] = 0;
+			wacom->shared->stylus_in_proximity = false;
+		}
+		input_report_key(input, wacom->tool[0], prox);
+		return 1;
 	}
-	input_report_key(input, BTN_STYLUS, data[1] & 0x02);
-	input_report_key(input, BTN_STYLUS2, data[1] & 0x10);
-	input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
-	input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
-	pressure = ((data[7] & 0x01) << 8) | data[6];
-	if (pressure < 0)
-		pressure = features->pressure_max + pressure + 1;
-	input_report_abs(input, ABS_PRESSURE, pressure);
-	input_report_key(input, BTN_TOUCH, data[1] & 0x05);
-	if (!prox) { /* out-prox */
-		wacom->id[0] = 0;
-		wacom->shared->stylus_in_proximity = false;
-	}
-	input_report_key(input, wacom->tool[0], prox);
 
-	return 1;
+	return 0;
 }
 
 static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len)
 {
 	char *data = wacom->data;
-	int prox = 0;
 
 	dbg("wacom_tpc_irq: received report #%d", data[0]);
 
-	if (len == WACOM_PKGLEN_TPC1FG ||
-	    data[0] == WACOM_REPORT_TPC1FG) {	/* single touch */
-
-		if (wacom->shared->stylus_in_proximity) {
-			if (wacom->id[1] & 0x01)
-				wacom_tpc_touch_out(wacom, 0);
-
-			wacom->id[1] = 0;
-			return 0;
-		}
-
-		if (len == WACOM_PKGLEN_TPC1FG)
-			prox = data[0] & 0x01;
-		else  /* with capacity */
-			prox = data[1] & 0x01;
-
-		if (prox)
-			wacom_tpc_touch_in(wacom, len);
-		else {
-			wacom_tpc_touch_out(wacom, 0);
-
-			wacom->id[0] = 0;
-		}
-		/* keep prox bit to send proper out-prox event */
-		wacom->id[1] = prox;
-	} else if (data[0] == WACOM_REPORT_PENABLED)
+	if (len == WACOM_PKGLEN_TPC1FG || data[0] == WACOM_REPORT_TPC1FG)
+		return wacom_tpc_single_touch(wacom, len);
+	else if (data[0] == WACOM_REPORT_PENABLED)
 		return wacom_tpc_pen(wacom);
 
 	return 0;
@@ -1172,6 +1134,8 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
 		/* fall through */
 
 	case TABLETPC:
+		__clear_bit(ABS_MISC, input_dev->absbit);
+
 		if (features->device_type == BTN_TOOL_DOUBLETAP ||
 		    features->device_type == BTN_TOOL_TRIPLETAP) {
 			input_abs_set_res(input_dev, ABS_X,
@@ -1180,7 +1144,6 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
 			input_abs_set_res(input_dev, ABS_Y,
 				wacom_calculate_touch_res(features->y_max,
 							features->y_phy));
-			__set_bit(BTN_TOOL_DOUBLETAP, input_dev->keybit);
 		}
 
 		if (features->device_type != BTN_TOOL_PEN)
diff --git a/drivers/input/tablet/wacom_wac.h b/drivers/input/tablet/wacom_wac.h
index 8f747dd..835f756 100644
--- a/drivers/input/tablet/wacom_wac.h
+++ b/drivers/input/tablet/wacom_wac.h
@@ -88,6 +88,7 @@ struct wacom_features {
 
 struct wacom_shared {
 	bool stylus_in_proximity;
+	bool touch_down;
 };
 
 struct wacom_wac {
-- 
1.7.4

Re: [PATCH 3/4] input - wacom : support one finger touch the touchscreen way

[Henrik Rydberg]

Hi Ping,

On Wed, Feb 16, 2011 at 02:53:11PM -0800, Ping Cheng wrote:
> There are two types of 1FGT devices supported in wacom_wac.c.
> Changing them to follow the existing touchscreen format, i.e.,
> only report BTN_TOUCH as a valid tool type.
> 
> Touch data will be ignored if pen is in proximity. This requires
> a touch up sent if touch was down when pen comes in. The touch up
> event should be sent before any pen events emitted. Otherwise,
> two pointers would race for the cursor.
> 
> However, we can not send a touch up inside wacom_tpc_pen since
> pen and touch are on different logical port. That is why we
> have to check if touch is up before sending pen events.
> 
> Reviewed-by: Chris Bagwell <chris@cnpbagwell.com>
> Signed-off-by: Ping Cheng <pingc@wacom.com>
> ---
>  drivers/input/tablet/wacom_wac.c |  125 +++++++++++++------------------------
>  drivers/input/tablet/wacom_wac.h |    1 +
>  2 files changed, 45 insertions(+), 81 deletions(-)
> 
> diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
> index 15bab99..fc0669d 100644
> --- a/drivers/input/tablet/wacom_wac.c
> +++ b/drivers/input/tablet/wacom_wac.c
> @@ -675,51 +675,35 @@ static int wacom_intuos_irq(struct wacom_wac *wacom)
>  	return 1;
>  }
>  
> -static void wacom_tpc_touch_out(struct wacom_wac *wacom, int idx)
> -{
> -	struct input_dev *input = wacom->input;
> -	int finger = idx + 1;
> -
> -	input_report_abs(input, ABS_X, 0);
> -	input_report_abs(input, ABS_Y, 0);
> -	input_report_abs(input, ABS_MISC, 0);
> -	input_report_key(input, wacom->tool[finger], 0);
> -	if (!idx)
> -		input_report_key(input, BTN_TOUCH, 0);
> -	input_event(input, EV_MSC, MSC_SERIAL, finger);
> -	input_sync(input);
> -}
> -
> -static void wacom_tpc_touch_in(struct wacom_wac *wacom, size_t len)
> +static int wacom_tpc_single_touch(struct wacom_wac *wacom, size_t len)
>  {
>  	char *data = wacom->data;
>  	struct input_dev *input = wacom->input;
> +	bool prox = 0;
>  
> -	wacom->tool[1] = BTN_TOOL_DOUBLETAP;
> -	wacom->id[0] = TOUCH_DEVICE_ID;
> -
> -	if (len != WACOM_PKGLEN_TPC1FG) {
> -
> -		switch (data[0]) {
> +	if (!wacom->shared->stylus_in_proximity) {
> +		if (len == WACOM_PKGLEN_TPC1FG)
> +			prox = data[0] & 0x01;
> +		else  /* with capacity */
> +			prox = data[1] & 0x01;
> +	} else if (wacom->shared->touch_down)
> +		prox = 0;
> +	else
> +		return 0;
>  
> -		case WACOM_REPORT_TPC1FG:
> -			input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
> -			input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
> -			input_report_abs(input, ABS_PRESSURE, le16_to_cpup((__le16 *)&data[6]));
> -			input_report_key(input, BTN_TOUCH, le16_to_cpup((__le16 *)&data[6]));
> -			input_report_abs(input, ABS_MISC, wacom->id[0]);
> -			input_report_key(input, wacom->tool[1], 1);
> -			input_sync(input);
> -			break;
> -		}
> -	} else {
> +	if (len == WACOM_PKGLEN_TPC1FG) {
>  		input_report_abs(input, ABS_X, get_unaligned_le16(&data[1]));
>  		input_report_abs(input, ABS_Y, get_unaligned_le16(&data[3]));
> -		input_report_key(input, BTN_TOUCH, 1);
> -		input_report_abs(input, ABS_MISC, wacom->id[1]);
> -		input_report_key(input, wacom->tool[1], 1);
> -		input_sync(input);
> +	} else {
> +		input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
> +		input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
>  	}
> +	input_report_key(input, BTN_TOUCH, prox);
> +
> +	/* keep prox bit to send proper out-prox event */
> +	wacom->shared->touch_down = prox;

So in this function, touch-first-then-pen leads to touch_down =
touch_is_actually_down && !pen_in_prox. Conversely,
pen-first-then-touch second leads to no state transition at all, which
seems equivalent. However, the code mixes the actual pen and touch
state with what gets reported. It seems cleaner to let touch_down =
touch_is_actually_down, and instead define what should be reported in
that case.

> +
> +	return 1;
>  }
>  
>  static int wacom_tpc_pen(struct wacom_wac *wacom)
> @@ -731,7 +715,7 @@ static int wacom_tpc_pen(struct wacom_wac *wacom)
>  
>  	prox = data[1] & 0x20;
>  
> -	if (!wacom->shared->stylus_in_proximity) { /* first in prox */
> +	if (!wacom->shared->stylus_in_proximity) {
>  		/* Going into proximity select tool */
>  		wacom->tool[0] = (data[1] & 0x0c) ? BTN_TOOL_RUBBER : BTN_TOOL_PEN;
>  		if (wacom->tool[0] == BTN_TOOL_PEN)
> @@ -740,58 +724,36 @@ static int wacom_tpc_pen(struct wacom_wac *wacom)
>  			wacom->id[0] = ERASER_DEVICE_ID;
>  
>  		wacom->shared->stylus_in_proximity = true;
> +	} else if (!wacom->shared->touch_down) {
> +		input_report_key(input, BTN_STYLUS, data[1] & 0x02);
> +		input_report_key(input, BTN_STYLUS2, data[1] & 0x10);
> +		input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
> +		input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
> +		pressure = ((data[7] & 0x01) << 8) | data[6];
> +		if (pressure < 0)
> +			pressure = features->pressure_max + pressure + 1;
> +		input_report_abs(input, ABS_PRESSURE, pressure);
> +		input_report_key(input, BTN_TOUCH, data[1] & 0x05);
> +		if (!prox) { /* out-prox */
> +			wacom->id[0] = 0;
> +			wacom->shared->stylus_in_proximity = false;
> +		}
> +		input_report_key(input, wacom->tool[0], prox);
> +		return 1;

If pen_in_prox is false on entry, and prox is false, this results in
pen_in_prox being true.

>  	}
> -	input_report_key(input, BTN_STYLUS, data[1] & 0x02);
> -	input_report_key(input, BTN_STYLUS2, data[1] & 0x10);
> -	input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
> -	input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
> -	pressure = ((data[7] & 0x01) << 8) | data[6];
> -	if (pressure < 0)
> -		pressure = features->pressure_max + pressure + 1;
> -	input_report_abs(input, ABS_PRESSURE, pressure);
> -	input_report_key(input, BTN_TOUCH, data[1] & 0x05);
> -	if (!prox) { /* out-prox */
> -		wacom->id[0] = 0;
> -		wacom->shared->stylus_in_proximity = false;
> -	}
> -	input_report_key(input, wacom->tool[0], prox);
>  
> -	return 1;
> +	return 0;
>  }
>  
>  static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len)
>  {
>  	char *data = wacom->data;
> -	int prox = 0;
>  
>  	dbg("wacom_tpc_irq: received report #%d", data[0]);
>  
> -	if (len == WACOM_PKGLEN_TPC1FG ||
> -	    data[0] == WACOM_REPORT_TPC1FG) {	/* single touch */
> -
> -		if (wacom->shared->stylus_in_proximity) {
> -			if (wacom->id[1] & 0x01)
> -				wacom_tpc_touch_out(wacom, 0);
> -
> -			wacom->id[1] = 0;
> -			return 0;
> -		}
> -
> -		if (len == WACOM_PKGLEN_TPC1FG)
> -			prox = data[0] & 0x01;
> -		else  /* with capacity */
> -			prox = data[1] & 0x01;
> -
> -		if (prox)
> -			wacom_tpc_touch_in(wacom, len);
> -		else {
> -			wacom_tpc_touch_out(wacom, 0);
> -
> -			wacom->id[0] = 0;
> -		}
> -		/* keep prox bit to send proper out-prox event */
> -		wacom->id[1] = prox;
> -	} else if (data[0] == WACOM_REPORT_PENABLED)
> +	if (len == WACOM_PKGLEN_TPC1FG || data[0] == WACOM_REPORT_TPC1FG)
> +		return wacom_tpc_single_touch(wacom, len);
> +	else if (data[0] == WACOM_REPORT_PENABLED)
>  		return wacom_tpc_pen(wacom);
>  
>  	return 0;
> @@ -1172,6 +1134,8 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
>  		/* fall through */
>  
>  	case TABLETPC:
> +		__clear_bit(ABS_MISC, input_dev->absbit);
> +
>  		if (features->device_type == BTN_TOOL_DOUBLETAP ||
>  		    features->device_type == BTN_TOOL_TRIPLETAP) {
>  			input_abs_set_res(input_dev, ABS_X,
> @@ -1180,7 +1144,6 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
>  			input_abs_set_res(input_dev, ABS_Y,
>  				wacom_calculate_touch_res(features->y_max,
>  							features->y_phy));
> -			__set_bit(BTN_TOOL_DOUBLETAP, input_dev->keybit);
>  		}
>  
>  		if (features->device_type != BTN_TOOL_PEN)
> diff --git a/drivers/input/tablet/wacom_wac.h b/drivers/input/tablet/wacom_wac.h
> index 8f747dd..835f756 100644
> --- a/drivers/input/tablet/wacom_wac.h
> +++ b/drivers/input/tablet/wacom_wac.h
> @@ -88,6 +88,7 @@ struct wacom_features {
>  
>  struct wacom_shared {
>  	bool stylus_in_proximity;
> +	bool touch_down;
>  };
>  
>  struct wacom_wac {
> -- 
> 1.7.4
> 

[PATCH 3/4] input - wacom : support one finger touch the touchscreen way

[Ping Cheng]

On Sun, Feb 20, 2011 at 8:21 PM, Henrik Rydberg <rydberg@euromail.se> wrote:

> Hi Ping,
>
> On Wed, Feb 16, 2011 at 02:53:11PM -0800, Ping Cheng wrote:
> > There are two types of 1FGT devices supported in wacom_wac.c.
> > Changing them to follow the existing touchscreen format, i.e.,
> > only report BTN_TOUCH as a valid tool type.
> >
> > Touch data will be ignored if pen is in proximity. This requires
> > a touch up sent if touch was down when pen comes in. The touch up
> > event should be sent before any pen events emitted. Otherwise,
> > two pointers would race for the cursor.
> >
> > However, we can not send a touch up inside wacom_tpc_pen since
> > pen and touch are on different logical port. That is why we
> > have to check if touch is up before sending pen events.
> >
> > Reviewed-by: Chris Bagwell <chris@cnpbagwell.com>
> > Signed-off-by: Ping Cheng <pingc@wacom.com>
> > ---
> >  drivers/input/tablet/wacom_wac.c |  125
> +++++++++++++------------------------
> >  drivers/input/tablet/wacom_wac.h |    1 +
> >  2 files changed, 45 insertions(+), 81 deletions(-)
> >
> > diff --git a/drivers/input/tablet/wacom_wac.c
> b/drivers/input/tablet/wacom_wac.c
> > index 15bab99..fc0669d 100644
> > --- a/drivers/input/tablet/wacom_wac.c
> > +++ b/drivers/input/tablet/wacom_wac.c
> > @@ -675,51 +675,35 @@ static int wacom_intuos_irq(struct wacom_wac
> *wacom)
> >       return 1;
> >  }
> >
> > -static void wacom_tpc_touch_out(struct wacom_wac *wacom, int idx)
> > -{
> > -     struct input_dev *input = wacom->input;
> > -     int finger = idx + 1;
> > -
> > -     input_report_abs(input, ABS_X, 0);
> > -     input_report_abs(input, ABS_Y, 0);
> > -     input_report_abs(input, ABS_MISC, 0);
> > -     input_report_key(input, wacom->tool[finger], 0);
> > -     if (!idx)
> > -             input_report_key(input, BTN_TOUCH, 0);
> > -     input_event(input, EV_MSC, MSC_SERIAL, finger);
> > -     input_sync(input);
> > -}
> > -
> > -static void wacom_tpc_touch_in(struct wacom_wac *wacom, size_t len)
> > +static int wacom_tpc_single_touch(struct wacom_wac *wacom, size_t len)
> >  {
> >       char *data = wacom->data;
> >       struct input_dev *input = wacom->input;
> > +     bool prox = 0;
> >
> > -     wacom->tool[1] = BTN_TOOL_DOUBLETAP;
> > -     wacom->id[0] = TOUCH_DEVICE_ID;
> > -
> > -     if (len != WACOM_PKGLEN_TPC1FG) {
> > -
> > -             switch (data[0]) {
> > +     if (!wacom->shared->stylus_in_proximity) {
> > +             if (len == WACOM_PKGLEN_TPC1FG)
> > +                     prox = data[0] & 0x01;
> > +             else  /* with capacity */
> > +                     prox = data[1] & 0x01;
> > +     } else if (wacom->shared->touch_down)
> > +             prox = 0;
> > +     else
> > +             return 0;
> >
> > -             case WACOM_REPORT_TPC1FG:
> > -                     input_report_abs(input, ABS_X, le16_to_cpup((__le16
> *)&data[2]));
> > -                     input_report_abs(input, ABS_Y, le16_to_cpup((__le16
> *)&data[4]));
> > -                     input_report_abs(input, ABS_PRESSURE,
> le16_to_cpup((__le16 *)&data[6]));
> > -                     input_report_key(input, BTN_TOUCH,
> le16_to_cpup((__le16 *)&data[6]));
> > -                     input_report_abs(input, ABS_MISC, wacom->id[0]);
> > -                     input_report_key(input, wacom->tool[1], 1);
> > -                     input_sync(input);
> > -                     break;
> > -             }
> > -     } else {
> > +     if (len == WACOM_PKGLEN_TPC1FG) {
> >               input_report_abs(input, ABS_X,
> get_unaligned_le16(&data[1]));
> >               input_report_abs(input, ABS_Y,
> get_unaligned_le16(&data[3]));
> > -             input_report_key(input, BTN_TOUCH, 1);
> > -             input_report_abs(input, ABS_MISC, wacom->id[1]);
> > -             input_report_key(input, wacom->tool[1], 1);
> > -             input_sync(input);
> > +     } else {
> > +             input_report_abs(input, ABS_X, le16_to_cpup((__le16
> *)&data[2]));
> > +             input_report_abs(input, ABS_Y, le16_to_cpup((__le16
> *)&data[4]));
> >       }
> > +     input_report_key(input, BTN_TOUCH, prox);
> > +
> > +     /* keep prox bit to send proper out-prox event */
> > +     wacom->shared->touch_down = prox;
>

touch_down represents the touch status that the driver gets/sets, not the
actual touch down. Driver touch down (touch_down) is decided by both the
actual touch and pen status, while it won't be changed by actual touch
status when pen is in.


> So in this function, touch-first-then-pen leads to touch_down =
> touch_is_actually_down && !pen_in_prox.


We need to think of the status in the driver's perspective, not the actual
touch state.

As soon as the pen is in, i.e., pen data is processed,  pen_in_prox will be
true. The !pen_in_prox can only happen when pen data is not processed. So,
to the driver, it is not in although it is actual in.

In this block, there are two possible cases for touch-first-then-pen:

touch in first (pen was not), so

touch_down = true;
pen_in_prox = false;

touch still in and pen is in also:

touch_down = true;
pen_in_prox = true;


> Conversely,
> pen-first-then-touch second leads to no state transition at all, which
> seems equivalent.


pen-first-then-touch (and pen stays in) will always have:

touch_down = false;
pen_in_prox = true;


> However, the code mixes the actual pen and touch
> state with what gets reported. It seems cleaner to let touch_down =
> touch_is_actually_down, and instead define what should be reported in
> that case.
>

No, we are not reporting the actual down. The actual touch can be down while
the driver touch_down is false since pen is in-prox.

It is a logic generated by/for the driver, not from the firmware. Maybe I
missed your point? Can you post a pesuo-code to show your idea?


>
> > +
> > +     return 1;
> >  }
> >
> >  static int wacom_tpc_pen(struct wacom_wac *wacom)
> > @@ -731,7 +715,7 @@ static int wacom_tpc_pen(struct wacom_wac *wacom)
> >
> >       prox = data[1] & 0x20;
> >
> > -     if (!wacom->shared->stylus_in_proximity) { /* first in prox */
> > +     if (!wacom->shared->stylus_in_proximity) {
> >               /* Going into proximity select tool */
> >               wacom->tool[0] = (data[1] & 0x0c) ? BTN_TOOL_RUBBER :
> BTN_TOOL_PEN;
> >               if (wacom->tool[0] == BTN_TOOL_PEN)
> > @@ -740,58 +724,36 @@ static int wacom_tpc_pen(struct wacom_wac *wacom)
> >                       wacom->id[0] = ERASER_DEVICE_ID;
> >
> >               wacom->shared->stylus_in_proximity = true;
> > +     } else if (!wacom->shared->touch_down) {
> > +             input_report_key(input, BTN_STYLUS, data[1] & 0x02);
> > +             input_report_key(input, BTN_STYLUS2, data[1] & 0x10);
> > +             input_report_abs(input, ABS_X, le16_to_cpup((__le16
> *)&data[2]));
> > +             input_report_abs(input, ABS_Y, le16_to_cpup((__le16
> *)&data[4]));
> > +             pressure = ((data[7] & 0x01) << 8) | data[6];
> > +             if (pressure < 0)
> > +                     pressure = features->pressure_max + pressure + 1;
> > +             input_report_abs(input, ABS_PRESSURE, pressure);
> > +             input_report_key(input, BTN_TOUCH, data[1] & 0x05);
> > +             if (!prox) { /* out-prox */
> > +                     wacom->id[0] = 0;
> > +                     wacom->shared->stylus_in_proximity = false;
> > +             }
> > +             input_report_key(input, wacom->tool[0], prox);
> > +             return 1;
>
> If pen_in_prox is false on entry, and prox is false, this results in
> pen_in_prox being true.
>

How?

We get prox = false event from FW for pen only once when tool leaves prox.
So, if prox is false, it means pen was in since without pen in, we do not
get pen out event.

If pen_in_prox was true, touch_down has to be false. Otherwise, we won't
post any events for pen. The main purpose here is that we will not post pen
(and/or touch) events before the driver sets touch_down to false.

Firmware posts pen and touch data continuously as long as they both are
in/down. It is the driver's responsibility to decide which ST to post so the
events won't confuse the ST clients.

It is a very small corner case. But it eliminates an obvious jumpy cursor
during the touch and pen transition.

I wish you have a Bamboo pen and touch to see what I mean.

Thank you for the comments.

Ping


> >       }
> > -     input_report_key(input, BTN_STYLUS, data[1] & 0x02);
> > -     input_report_key(input, BTN_STYLUS2, data[1] & 0x10);
> > -     input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
> > -     input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
> > -     pressure = ((data[7] & 0x01) << 8) | data[6];
> > -     if (pressure < 0)
> > -             pressure = features->pressure_max + pressure + 1;
> > -     input_report_abs(input, ABS_PRESSURE, pressure);
> > -     input_report_key(input, BTN_TOUCH, data[1] & 0x05);
> > -     if (!prox) { /* out-prox */
> > -             wacom->id[0] = 0;
> > -             wacom->shared->stylus_in_proximity = false;
> > -     }
> > -     input_report_key(input, wacom->tool[0], prox);
> >
> > -     return 1;
> > +     return 0;
> >  }
> >
> >  static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len)
> >  {
> >       char *data = wacom->data;
> > -     int prox = 0;
> >
> >       dbg("wacom_tpc_irq: received report #%d", data[0]);
> >
> > -     if (len == WACOM_PKGLEN_TPC1FG ||
> > -         data[0] == WACOM_REPORT_TPC1FG) {   /* single touch */
> > -
> > -             if (wacom->shared->stylus_in_proximity) {
> > -                     if (wacom->id[1] & 0x01)
> > -                             wacom_tpc_touch_out(wacom, 0);
> > -
> > -                     wacom->id[1] = 0;
> > -                     return 0;
> > -             }
> > -
> > -             if (len == WACOM_PKGLEN_TPC1FG)
> > -                     prox = data[0] & 0x01;
> > -             else  /* with capacity */
> > -                     prox = data[1] & 0x01;
> > -
> > -             if (prox)
> > -                     wacom_tpc_touch_in(wacom, len);
> > -             else {
> > -                     wacom_tpc_touch_out(wacom, 0);
> > -
> > -                     wacom->id[0] = 0;
> > -             }
> > -             /* keep prox bit to send proper out-prox event */
> > -             wacom->id[1] = prox;
> > -     } else if (data[0] == WACOM_REPORT_PENABLED)
> > +     if (len == WACOM_PKGLEN_TPC1FG || data[0] == WACOM_REPORT_TPC1FG)
> > +             return wacom_tpc_single_touch(wacom, len);
> > +     else if (data[0] == WACOM_REPORT_PENABLED)
> >               return wacom_tpc_pen(wacom);
> >
> >       return 0;
> > @@ -1172,6 +1134,8 @@ void wacom_setup_input_capabilities(struct
> input_dev *input_dev,
> >               /* fall through */
> >
> >       case TABLETPC:
> > +             __clear_bit(ABS_MISC, input_dev->absbit);
> > +
> >               if (features->device_type == BTN_TOOL_DOUBLETAP ||
> >                   features->device_type == BTN_TOOL_TRIPLETAP) {
> >                       input_abs_set_res(input_dev, ABS_X,
> > @@ -1180,7 +1144,6 @@ void wacom_setup_input_capabilities(struct
> input_dev *input_dev,
> >                       input_abs_set_res(input_dev, ABS_Y,
> >                               wacom_calculate_touch_res(features->y_max,
> >                                                       features->y_phy));
> > -                     __set_bit(BTN_TOOL_DOUBLETAP, input_dev->keybit);
> >               }
> >
> >               if (features->device_type != BTN_TOOL_PEN)
> > diff --git a/drivers/input/tablet/wacom_wac.h
> b/drivers/input/tablet/wacom_wac.h
> > index 8f747dd..835f756 100644
> > --- a/drivers/input/tablet/wacom_wac.h
> > +++ b/drivers/input/tablet/wacom_wac.h
> > @@ -88,6 +88,7 @@ struct wacom_features {
> >
> >  struct wacom_shared {
> >       bool stylus_in_proximity;
> > +     bool touch_down;
> >  };
> >
> >  struct wacom_wac {
> > --
> > 1.7.4
> >
>

Re: [PATCH 3/4] input - wacom : support one finger touch the touchscreen way

[Henrik Rydberg]

> touch_down represents the touch status that the driver gets/sets, not the
> actual touch down. Driver touch down (touch_down) is decided by both the
> actual touch and pen status, while it won't be changed by actual touch
> status when pen is in.

Understood, but it hides complex state transitions and therefore
becomes error prone. I believe your code is correct, but less
maintainable than it could be. I would like to take the opportunity to
express that here, with the following argument.

Let us assume we have two variables, A and B, representing the actual
touch and pen down. We can then try to write down the boolean function
for when touch and pen events should not be sent. Starting out simple,
we could try, for an arbitrary time step n,

inhibit_touch(n) = T(A(n), B(n))
inhibit_pen(n) = P(A(n), B(n))

Now, you already stated that this does not work very well, because if
B changes state, for instance, and inhibit_touch changes state at the
same time, we get a race for the pointer events. So, expanding on
this, we could take history into account and try

inhibit_touch(n) = T(A(n), B(n), A(n-1), B(n-1))
inhibit_pen(n) = P(A(n), B(n), A(n-1), B(n-1))

This seems like it could work, since we can detect the state change
and decide how to deal with the pointer events by choosing the
functions T and P appropriately.

The point of all this is that code using simple input variables (A, B)
and a complex boolean function (T, P) has no hidden error
states. Conversely, code using complex input variables and a simple
boolean function may contain an arbitrary number of hidden error
states. As time goes by, those error states may be enabled by accident
during a code change. Sure, both approaches lead to correct code if
performed properly, but the former is less error prone and therefore
easier to maintain.

Henrik