Re: [PATCH 2/2 v2] input: wacom_w8001 - cleanup 2FG touch code

On Sat, Jul 16, 2016 at 2:58 PM, Dmitry Torokhov
[off-list ref] wrote:

On July 16, 2016 2:32:27 PM PDT, Ping Cheng [off-list ref] wrote:

quoted

On Fri, Jul 15, 2016 at 10:33 PM, Dmitry Torokhov
[off-list ref] wrote:

quoted

On Fri, Jul 15, 2016 at 06:19:41PM -0700, Ping Cheng wrote:

quoted

On Friday, July 15, 2016, Dmitry Torokhov

[off-list ref] wrote:

quoted

quoted

quoted

On Fri, Jul 15, 2016 at 04:26:25PM -0700, Ping Cheng wrote:

quoted

input_mt_sync_frame is used by other wacom devices in

wacom_wac.c

quoted

quoted

quoted

quoted

to close the frame and emulate pointer events. Let's follow

them.

quoted

quoted

quoted

quoted

Touch events aren't multiplexed over the same device anymore,

the

quoted

quoted

quoted

quoted

use of ABS_MT_TOOL_TYPE is superfluous.

All of our touchscreen report the finger tool events, even if it

is the

quoted

quoted

quoted

only "tool" that is being used. Does it cause problems?

No, it does not cause problem, with/without ABS_MT_TOOL_TYPE being

set.

quoted

quoted

Testing result shows all touch data are reported whether we

explicitly set

quoted

quoted

ABS_MT_TOOL_TYPE upfront or not. Plus, ABS_MT_TOOL_TYPE is not set

in

quoted

quoted

wacom.ko. We'd like to be in sync with wacom.ko.

The following comments for input_mt_report_slot_state in input-mt.c
explains the reason:

"* Reports a contact via ABS_MT_TRACKING_ID, and optionally
 * ABS_MT_TOOL_TYPE. If active is true and the slot is currently
 * inactive, or if the tool type is changed, a new tracking id is
 * assigned to the slot. The tool type is only reported if the
 * corresponding absbit field is set."

MT_TOOL_TYPE is necessary if we report both pen and touch on the

same

quoted

quoted

interface. That is not true for this series of devices any more..

quoted

quoted

Signed-off-by: Ping Cheng <redacted>
Signed-off-by: Peter Hutterer <redacted>
---
v2: moved input_abs_set_res into a separate patch, as suggested

by

quoted

quoted

quoted

quoted

Dmitry.
---
 drivers/input/touchscreen/wacom_w8001.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/input/touchscreen/wacom_w8001.c

b/drivers/input/touchscreen/wacom_w8001.c

quoted

index 7e807af..541a8df 100644

--- a/drivers/input/touchscreen/wacom_w8001.c
+++ b/drivers/input/touchscreen/wacom_w8001.c

@@ -170,16 +170,8 @@ static void parse_multi_touch(struct w8001

*w8001)

quoted

quoted

quoted

quoted

              }
      }

-     /* emulate single touch events when stylus is out of

proximity.

quoted

quoted

quoted

quoted

-      * This is to make single touch backward support

consistent

quoted

quoted

quoted

quoted

-      * across all Wacom single touch devices.
-      */
-     if (w8001->type != BTN_TOOL_PEN &&
-                         w8001->type != BTN_TOOL_RUBBER) {
-             w8001->type = count == 1 ? BTN_TOOL_FINGER :

KEY_RESERVED;

quoted

quoted

quoted

quoted

-             input_mt_report_pointer_emulation(dev, true);
-     }
-
+     w8001->type = KEY_RESERVED;
+     input_mt_sync_frame(dev);
      input_sync(dev);
 }

@@ -508,7 +500,6 @@ static int w8001_setup_touch(struct w8001

*w8001,

quoted

quoted

quoted

char *basename,

quoted

      case 5:
              w8001->pktlen = W8001_PKTLEN_TOUCH2FG;

-             __set_bit(BTN_TOOL_DOUBLETAP, dev->keybit);

Who is setting this bit then?

All MT related bits are set by input_mt_init_slots in input-mt.c,

including

quoted

quoted

BTN_TOOL_DOUBLETAP, if necessary.

Doesn't this only happen if you call input_mt_init_slots with
INPUT_MT_POINTER flag?

You are right. And that is what we want this driver to do - to be in
sync with wacom.ko and rely on input-mt for common actions.

But unless I missed some other patch from you this driver passes 0 as flags to input_mt_init_slots() and by removing the double-tap bit we are breaking the driver.

Oh, I see where you are ;). I assumed INPUT_MT_DIRECT was there. My
bad eyes ;-)!  We'd be better to change

input_mt_init_slots(dev, 2, 0);

to

input_mt_init_slots(dev, 2, INPUT_MT_DIRECT);

Do you want me to submit a new version or can you update it locally?

Thank you for your time.

Ping