* [PATCH 4/4] input - wacom: Support 2FGT in MT format @ 2011-02-11 1:33 Ping Cheng 2011-02-11 20:20 ` Henrik Rydberg 0 siblings, 1 reply; 7+ messages in thread From: Ping Cheng @ 2011-02-11 1:33 UTC (permalink / raw) To: linux-input; +Cc: Ping Cheng, Ping Cheng Reviewed-by: Chris Bagwell <chris@cnpbagwell.com> Signed-off-by: Ping Cheng <pingc@wacom.com> --- drivers/input/tablet/wacom_sys.c | 12 ++++---- drivers/input/tablet/wacom_wac.c | 61 ++++++++++++++++++++++++++++++------- 2 files changed, 55 insertions(+), 18 deletions(-) diff --git a/drivers/input/tablet/wacom_sys.c b/drivers/input/tablet/wacom_sys.c index fc38149..b97665f 100644 --- a/drivers/input/tablet/wacom_sys.c +++ b/drivers/input/tablet/wacom_sys.c @@ -193,16 +193,16 @@ static int wacom_parse_hid(struct usb_interface *intf, struct hid_descriptor *hi case HID_USAGE_X: if (usage == WCM_DESKTOP) { if (finger) { - features->device_type = BTN_TOOL_DOUBLETAP; + features->device_type = BTN_TOOL_FINGER; if (features->type == TABLETPC2FG) { /* need to reset back */ features->pktlen = WACOM_PKGLEN_TPC2FG; - features->device_type = BTN_TOOL_TRIPLETAP; + features->device_type = BTN_TOOL_DOUBLETAP; } if (features->type == BAMBOO_PT) { /* need to reset back */ features->pktlen = WACOM_PKGLEN_BBTOUCH; - features->device_type = BTN_TOOL_TRIPLETAP; + features->device_type = BTN_TOOL_DOUBLETAP; features->x_phy = get_unaligned_le16(&report[i + 5]); features->x_max = @@ -241,11 +241,11 @@ static int wacom_parse_hid(struct usb_interface *intf, struct hid_descriptor *hi case HID_USAGE_Y: if (usage == WCM_DESKTOP) { if (finger) { - features->device_type = BTN_TOOL_DOUBLETAP; + features->device_type = BTN_TOOL_FINGER; if (features->type == TABLETPC2FG) { /* need to reset back */ features->pktlen = WACOM_PKGLEN_TPC2FG; - features->device_type = BTN_TOOL_TRIPLETAP; + features->device_type = BTN_TOOL_DOUBLETAP; features->y_max = get_unaligned_le16(&report[i + 3]); features->y_phy = @@ -254,7 +254,7 @@ static int wacom_parse_hid(struct usb_interface *intf, struct hid_descriptor *hi } else if (features->type == BAMBOO_PT) { /* need to reset back */ features->pktlen = WACOM_PKGLEN_BBTOUCH; - features->device_type = BTN_TOOL_TRIPLETAP; + features->device_type = BTN_TOOL_DOUBLETAP; features->y_phy = get_unaligned_le16(&report[i + 3]); features->y_max = diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c index fc0669d..39c289d 100644 --- a/drivers/input/tablet/wacom_wac.c +++ b/drivers/input/tablet/wacom_wac.c @@ -675,6 +675,38 @@ static int wacom_intuos_irq(struct wacom_wac *wacom) return 1; } +static int wacom_tpc_mt_touch(struct wacom_wac *wacom) +{ + struct input_dev *input = wacom->input; + unsigned char *data = wacom->data; + int i; + + if (wacom->shared->stylus_in_proximity && !wacom->shared->touch_down) + return 0; + + for (i = 0; i < 2; i++) { + int p = data[1] & (1 << i); + bool touch = p && !wacom->shared->stylus_in_proximity; + + input_mt_slot(input, i); + input_mt_report_slot_state(input, MT_TOOL_FINGER, touch); + if (touch) { + int x = le16_to_cpup((__le16 *)&data[i * 2 + 2]) & 0x7fff; + int y = le16_to_cpup((__le16 *)&data[i * 2 + 6]) & 0x7fff; + + input_report_abs(input, ABS_MT_POSITION_X, x); + input_report_abs(input, ABS_MT_POSITION_Y, y); + } + + /* keep touch bit to send proper touch up event */ + wacom->shared->touch_down = max(touch, wacom->shared->touch_down); + } + + input_mt_report_pointer_emulation(input, true); + + return 1; +} + static int wacom_tpc_single_touch(struct wacom_wac *wacom, size_t len) { char *data = wacom->data; @@ -753,6 +785,8 @@ static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len) if (len == WACOM_PKGLEN_TPC1FG || data[0] == WACOM_REPORT_TPC1FG) return wacom_tpc_single_touch(wacom, len); + else if (data[0] == WACOM_REPORT_TPC2FG) + return wacom_tpc_mt_touch(wacom); else if (data[0] == WACOM_REPORT_PENABLED) return wacom_tpc_pen(wacom); @@ -979,7 +1013,7 @@ void wacom_setup_device_quirks(struct wacom_features *features) { /* touch device found but size is not defined. use default */ - if (features->device_type == BTN_TOOL_DOUBLETAP && !features->x_max) { + if (features->device_type == BTN_TOOL_FINGER && !features->x_max) { features->x_max = 1023; features->y_max = 1023; } @@ -991,7 +1025,7 @@ void wacom_setup_device_quirks(struct wacom_features *features) /* quirks for bamboo touch */ if (features->type == BAMBOO_PT && - features->device_type == BTN_TOOL_TRIPLETAP) { + features->device_type == BTN_TOOL_DOUBLETAP) { features->x_max <<= 5; features->y_max <<= 5; features->x_fuzz <<= 5; @@ -1127,28 +1161,31 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev, break; case TABLETPC2FG: - if (features->device_type == BTN_TOOL_TRIPLETAP) { - __set_bit(BTN_TOOL_TRIPLETAP, input_dev->keybit); - input_set_capability(input_dev, EV_MSC, MSC_SERIAL); + if (features->device_type == BTN_TOOL_DOUBLETAP) { + __set_bit(BTN_TOOL_DOUBLETAP, input_dev->keybit); + + input_mt_init_slots(input_dev, 2); + input_set_abs_params(input_dev, ABS_MT_TOOL_TYPE, + 0, MT_TOOL_MAX, 0, 0); + input_set_abs_params(input_dev, ABS_MT_POSITION_X, + 0, features->x_max, 0, 0); + input_set_abs_params(input_dev, ABS_MT_POSITION_Y, + 0, features->y_max, 0, 0); } /* fall through */ case TABLETPC: __clear_bit(ABS_MISC, input_dev->absbit); - if (features->device_type == BTN_TOOL_DOUBLETAP || - features->device_type == BTN_TOOL_TRIPLETAP) { + if (features->device_type != BTN_TOOL_PEN) { input_abs_set_res(input_dev, ABS_X, wacom_calculate_touch_res(features->x_max, features->x_phy)); input_abs_set_res(input_dev, ABS_Y, wacom_calculate_touch_res(features->y_max, features->y_phy)); - } - - if (features->device_type != BTN_TOOL_PEN) break; /* no need to process stylus stuff */ - + } /* fall through */ case PL: @@ -1166,7 +1203,7 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev, case BAMBOO_PT: __clear_bit(ABS_MISC, input_dev->absbit); - if (features->device_type == BTN_TOOL_TRIPLETAP) { + if (features->device_type == BTN_TOOL_DOUBLETAP) { __set_bit(BTN_LEFT, input_dev->keybit); __set_bit(BTN_FORWARD, input_dev->keybit); __set_bit(BTN_BACK, input_dev->keybit); -- 1.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 4/4] input - wacom: Support 2FGT in MT format 2011-02-11 1:33 [PATCH 4/4] input - wacom: Support 2FGT in MT format Ping Cheng @ 2011-02-11 20:20 ` Henrik Rydberg 2011-02-11 20:35 ` Ping Cheng 0 siblings, 1 reply; 7+ messages in thread From: Henrik Rydberg @ 2011-02-11 20:20 UTC (permalink / raw) To: Ping Cheng; +Cc: linux-input, Ping Cheng Hi Ping, On Thu, Feb 10, 2011 at 05:33:03PM -0800, Ping Cheng wrote: Given the nice patch below, there has to be _something_ to say about it here, hmm? > Reviewed-by: Chris Bagwell <chris@cnpbagwell.com> > Signed-off-by: Ping Cheng <pingc@wacom.com> > --- > drivers/input/tablet/wacom_sys.c | 12 ++++---- > drivers/input/tablet/wacom_wac.c | 61 ++++++++++++++++++++++++++++++------- > 2 files changed, 55 insertions(+), 18 deletions(-) > > diff --git a/drivers/input/tablet/wacom_sys.c b/drivers/input/tablet/wacom_sys.c > index fc38149..b97665f 100644 > --- a/drivers/input/tablet/wacom_sys.c > +++ b/drivers/input/tablet/wacom_sys.c > @@ -193,16 +193,16 @@ static int wacom_parse_hid(struct usb_interface *intf, struct hid_descriptor *hi > case HID_USAGE_X: > if (usage == WCM_DESKTOP) { > if (finger) { > - features->device_type = BTN_TOOL_DOUBLETAP; > + features->device_type = BTN_TOOL_FINGER; > if (features->type == TABLETPC2FG) { > /* need to reset back */ > features->pktlen = WACOM_PKGLEN_TPC2FG; > - features->device_type = BTN_TOOL_TRIPLETAP; > + features->device_type = BTN_TOOL_DOUBLETAP; > } > if (features->type == BAMBOO_PT) { > /* need to reset back */ > features->pktlen = WACOM_PKGLEN_BBTOUCH; > - features->device_type = BTN_TOOL_TRIPLETAP; > + features->device_type = BTN_TOOL_DOUBLETAP; > features->x_phy = > get_unaligned_le16(&report[i + 5]); > features->x_max = > @@ -241,11 +241,11 @@ static int wacom_parse_hid(struct usb_interface *intf, struct hid_descriptor *hi > case HID_USAGE_Y: > if (usage == WCM_DESKTOP) { > if (finger) { > - features->device_type = BTN_TOOL_DOUBLETAP; > + features->device_type = BTN_TOOL_FINGER; > if (features->type == TABLETPC2FG) { > /* need to reset back */ > features->pktlen = WACOM_PKGLEN_TPC2FG; > - features->device_type = BTN_TOOL_TRIPLETAP; > + features->device_type = BTN_TOOL_DOUBLETAP; > features->y_max = > get_unaligned_le16(&report[i + 3]); > features->y_phy = > @@ -254,7 +254,7 @@ static int wacom_parse_hid(struct usb_interface *intf, struct hid_descriptor *hi > } else if (features->type == BAMBOO_PT) { > /* need to reset back */ > features->pktlen = WACOM_PKGLEN_BBTOUCH; > - features->device_type = BTN_TOOL_TRIPLETAP; > + features->device_type = BTN_TOOL_DOUBLETAP; > features->y_phy = > get_unaligned_le16(&report[i + 3]); > features->y_max = > diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c > index fc0669d..39c289d 100644 > --- a/drivers/input/tablet/wacom_wac.c > +++ b/drivers/input/tablet/wacom_wac.c > @@ -675,6 +675,38 @@ static int wacom_intuos_irq(struct wacom_wac *wacom) > return 1; > } > > +static int wacom_tpc_mt_touch(struct wacom_wac *wacom) > +{ > + struct input_dev *input = wacom->input; > + unsigned char *data = wacom->data; > + int i; > + > + if (wacom->shared->stylus_in_proximity && !wacom->shared->touch_down) > + return 0; Seeing one case handled out of four possible always makes me nervous. > + > + for (i = 0; i < 2; i++) { > + int p = data[1] & (1 << i); > + bool touch = p && !wacom->shared->stylus_in_proximity; > + > + input_mt_slot(input, i); > + input_mt_report_slot_state(input, MT_TOOL_FINGER, touch); > + if (touch) { > + int x = le16_to_cpup((__le16 *)&data[i * 2 + 2]) & 0x7fff; > + int y = le16_to_cpup((__le16 *)&data[i * 2 + 6]) & 0x7fff; > + > + input_report_abs(input, ABS_MT_POSITION_X, x); > + input_report_abs(input, ABS_MT_POSITION_Y, y); > + } > + > + /* keep touch bit to send proper touch up event */ > + wacom->shared->touch_down = max(touch, wacom->shared->touch_down); So only false->true is possible here. What I can see from the patchset, only wacom_tpc_single_touch() will ever set touch_down to false. Is that sufficient? > + } > + > + input_mt_report_pointer_emulation(input, true); > + > + return 1; > +} > + > static int wacom_tpc_single_touch(struct wacom_wac *wacom, size_t len) > { > char *data = wacom->data; > @@ -753,6 +785,8 @@ static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len) > > if (len == WACOM_PKGLEN_TPC1FG || data[0] == WACOM_REPORT_TPC1FG) > return wacom_tpc_single_touch(wacom, len); > + else if (data[0] == WACOM_REPORT_TPC2FG) > + return wacom_tpc_mt_touch(wacom); > else if (data[0] == WACOM_REPORT_PENABLED) > return wacom_tpc_pen(wacom); > > @@ -979,7 +1013,7 @@ void wacom_setup_device_quirks(struct wacom_features *features) > { > > /* touch device found but size is not defined. use default */ > - if (features->device_type == BTN_TOOL_DOUBLETAP && !features->x_max) { > + if (features->device_type == BTN_TOOL_FINGER && !features->x_max) { > features->x_max = 1023; > features->y_max = 1023; > } > @@ -991,7 +1025,7 @@ void wacom_setup_device_quirks(struct wacom_features *features) > > /* quirks for bamboo touch */ > if (features->type == BAMBOO_PT && > - features->device_type == BTN_TOOL_TRIPLETAP) { > + features->device_type == BTN_TOOL_DOUBLETAP) { > features->x_max <<= 5; > features->y_max <<= 5; > features->x_fuzz <<= 5; > @@ -1127,28 +1161,31 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev, > break; > > case TABLETPC2FG: > - if (features->device_type == BTN_TOOL_TRIPLETAP) { > - __set_bit(BTN_TOOL_TRIPLETAP, input_dev->keybit); > - input_set_capability(input_dev, EV_MSC, MSC_SERIAL); > + if (features->device_type == BTN_TOOL_DOUBLETAP) { > + __set_bit(BTN_TOOL_DOUBLETAP, input_dev->keybit); > + > + input_mt_init_slots(input_dev, 2); > + input_set_abs_params(input_dev, ABS_MT_TOOL_TYPE, > + 0, MT_TOOL_MAX, 0, 0); > + input_set_abs_params(input_dev, ABS_MT_POSITION_X, > + 0, features->x_max, 0, 0); > + input_set_abs_params(input_dev, ABS_MT_POSITION_Y, > + 0, features->y_max, 0, 0); > } > /* fall through */ > > case TABLETPC: > __clear_bit(ABS_MISC, input_dev->absbit); > > - if (features->device_type == BTN_TOOL_DOUBLETAP || > - features->device_type == BTN_TOOL_TRIPLETAP) { > + if (features->device_type != BTN_TOOL_PEN) { > input_abs_set_res(input_dev, ABS_X, > wacom_calculate_touch_res(features->x_max, > features->x_phy)); > input_abs_set_res(input_dev, ABS_Y, > wacom_calculate_touch_res(features->y_max, > features->y_phy)); > - } > - > - if (features->device_type != BTN_TOOL_PEN) > break; /* no need to process stylus stuff */ > - > + } > /* fall through */ > > case PL: > @@ -1166,7 +1203,7 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev, > case BAMBOO_PT: > __clear_bit(ABS_MISC, input_dev->absbit); > > - if (features->device_type == BTN_TOOL_TRIPLETAP) { > + if (features->device_type == BTN_TOOL_DOUBLETAP) { > __set_bit(BTN_LEFT, input_dev->keybit); > __set_bit(BTN_FORWARD, input_dev->keybit); > __set_bit(BTN_BACK, input_dev->keybit); > -- Thanks, Henrik ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 4/4] input - wacom: Support 2FGT in MT format 2011-02-11 20:20 ` Henrik Rydberg @ 2011-02-11 20:35 ` Ping Cheng 2011-02-11 20:47 ` Henrik Rydberg 0 siblings, 1 reply; 7+ messages in thread From: Ping Cheng @ 2011-02-11 20:35 UTC (permalink / raw) To: Henrik Rydberg; +Cc: linux-input On Fri, Feb 11, 2011 at 12:20 PM, Henrik Rydberg <rydberg@euromail.se> wrote: > Hi Ping, > > On Thu, Feb 10, 2011 at 05:33:03PM -0800, Ping Cheng wrote: > > Given the nice patch below, there has to be _something_ to say about it here, hmm? > >> Reviewed-by: Chris Bagwell <chris@cnpbagwell.com> >> Signed-off-by: Ping Cheng <pingc@wacom.com> >> --- >> drivers/input/tablet/wacom_sys.c | 12 ++++---- >> drivers/input/tablet/wacom_wac.c | 61 ++++++++++++++++++++++++++++++------- >> 2 files changed, 55 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/input/tablet/wacom_sys.c b/drivers/input/tablet/wacom_sys.c >> index fc38149..b97665f 100644 >> --- a/drivers/input/tablet/wacom_sys.c >> +++ b/drivers/input/tablet/wacom_sys.c >> @@ -193,16 +193,16 @@ static int wacom_parse_hid(struct usb_interface *intf, struct hid_descriptor *hi >> case HID_USAGE_X: >> if (usage == WCM_DESKTOP) { >> if (finger) { >> - features->device_type = BTN_TOOL_DOUBLETAP; >> + features->device_type = BTN_TOOL_FINGER; >> if (features->type == TABLETPC2FG) { >> /* need to reset back */ >> features->pktlen = WACOM_PKGLEN_TPC2FG; >> - features->device_type = BTN_TOOL_TRIPLETAP; >> + features->device_type = BTN_TOOL_DOUBLETAP; >> } >> if (features->type == BAMBOO_PT) { >> /* need to reset back */ >> features->pktlen = WACOM_PKGLEN_BBTOUCH; >> - features->device_type = BTN_TOOL_TRIPLETAP; >> + features->device_type = BTN_TOOL_DOUBLETAP; >> features->x_phy = >> get_unaligned_le16(&report[i + 5]); >> features->x_max = >> @@ -241,11 +241,11 @@ static int wacom_parse_hid(struct usb_interface *intf, struct hid_descriptor *hi >> case HID_USAGE_Y: >> if (usage == WCM_DESKTOP) { >> if (finger) { >> - features->device_type = BTN_TOOL_DOUBLETAP; >> + features->device_type = BTN_TOOL_FINGER; >> if (features->type == TABLETPC2FG) { >> /* need to reset back */ >> features->pktlen = WACOM_PKGLEN_TPC2FG; >> - features->device_type = BTN_TOOL_TRIPLETAP; >> + features->device_type = BTN_TOOL_DOUBLETAP; >> features->y_max = >> get_unaligned_le16(&report[i + 3]); >> features->y_phy = >> @@ -254,7 +254,7 @@ static int wacom_parse_hid(struct usb_interface *intf, struct hid_descriptor *hi >> } else if (features->type == BAMBOO_PT) { >> /* need to reset back */ >> features->pktlen = WACOM_PKGLEN_BBTOUCH; >> - features->device_type = BTN_TOOL_TRIPLETAP; >> + features->device_type = BTN_TOOL_DOUBLETAP; >> features->y_phy = >> get_unaligned_le16(&report[i + 3]); >> features->y_max = >> diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c >> index fc0669d..39c289d 100644 >> --- a/drivers/input/tablet/wacom_wac.c >> +++ b/drivers/input/tablet/wacom_wac.c >> @@ -675,6 +675,38 @@ static int wacom_intuos_irq(struct wacom_wac *wacom) >> return 1; >> } >> >> +static int wacom_tpc_mt_touch(struct wacom_wac *wacom) >> +{ >> + struct input_dev *input = wacom->input; >> + unsigned char *data = wacom->data; >> + int i; >> + >> + if (wacom->shared->stylus_in_proximity && !wacom->shared->touch_down) >> + return 0; > > Seeing one case handled out of four possible always makes me nervous. The above statement is to avoid going through the input_mt_slot and input_mt_report_slot_state routines without posting any meaningful events. I guess it could be considered as a performance enhancement? Which case makes you nervous? I'll take care of it ;). >> + >> + for (i = 0; i < 2; i++) { >> + int p = data[1] & (1 << i); >> + bool touch = p && !wacom->shared->stylus_in_proximity; >> + >> + input_mt_slot(input, i); >> + input_mt_report_slot_state(input, MT_TOOL_FINGER, touch); >> + if (touch) { >> + int x = le16_to_cpup((__le16 *)&data[i * 2 + 2]) & 0x7fff; >> + int y = le16_to_cpup((__le16 *)&data[i * 2 + 6]) & 0x7fff; >> + >> + input_report_abs(input, ABS_MT_POSITION_X, x); >> + input_report_abs(input, ABS_MT_POSITION_Y, y); >> + } >> + >> + /* keep touch bit to send proper touch up event */ >> + wacom->shared->touch_down = max(touch, wacom->shared->touch_down); > > So only false->true is possible here. Yeah, both are bools. What else can they take? > What I can see from the patchset, only wacom_tpc_single_touch() will ever set touch_down to > false. Is that sufficient? No, wacom_tpc_mt_touch can set touch_down to false too. When touch is false, touch_down will be false. This happens when pen in prox or when both fingers are up. Where else do you see it can be changed? tpc_pen() can not do that since it is on a different port, that's the reason of touch_down. Ping >> + } >> + >> + input_mt_report_pointer_emulation(input, true); >> + >> + return 1; >> +} >> + >> static int wacom_tpc_single_touch(struct wacom_wac *wacom, size_t len) >> { >> char *data = wacom->data; >> @@ -753,6 +785,8 @@ static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len) >> >> if (len == WACOM_PKGLEN_TPC1FG || data[0] == WACOM_REPORT_TPC1FG) >> return wacom_tpc_single_touch(wacom, len); >> + else if (data[0] == WACOM_REPORT_TPC2FG) >> + return wacom_tpc_mt_touch(wacom); >> else if (data[0] == WACOM_REPORT_PENABLED) >> return wacom_tpc_pen(wacom); >> >> @@ -979,7 +1013,7 @@ void wacom_setup_device_quirks(struct wacom_features *features) >> { >> >> /* touch device found but size is not defined. use default */ >> - if (features->device_type == BTN_TOOL_DOUBLETAP && !features->x_max) { >> + if (features->device_type == BTN_TOOL_FINGER && !features->x_max) { >> features->x_max = 1023; >> features->y_max = 1023; >> } >> @@ -991,7 +1025,7 @@ void wacom_setup_device_quirks(struct wacom_features *features) >> >> /* quirks for bamboo touch */ >> if (features->type == BAMBOO_PT && >> - features->device_type == BTN_TOOL_TRIPLETAP) { >> + features->device_type == BTN_TOOL_DOUBLETAP) { >> features->x_max <<= 5; >> features->y_max <<= 5; >> features->x_fuzz <<= 5; >> @@ -1127,28 +1161,31 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev, >> break; >> >> case TABLETPC2FG: >> - if (features->device_type == BTN_TOOL_TRIPLETAP) { >> - __set_bit(BTN_TOOL_TRIPLETAP, input_dev->keybit); >> - input_set_capability(input_dev, EV_MSC, MSC_SERIAL); >> + if (features->device_type == BTN_TOOL_DOUBLETAP) { >> + __set_bit(BTN_TOOL_DOUBLETAP, input_dev->keybit); >> + >> + input_mt_init_slots(input_dev, 2); >> + input_set_abs_params(input_dev, ABS_MT_TOOL_TYPE, >> + 0, MT_TOOL_MAX, 0, 0); >> + input_set_abs_params(input_dev, ABS_MT_POSITION_X, >> + 0, features->x_max, 0, 0); >> + input_set_abs_params(input_dev, ABS_MT_POSITION_Y, >> + 0, features->y_max, 0, 0); >> } >> /* fall through */ >> >> case TABLETPC: >> __clear_bit(ABS_MISC, input_dev->absbit); >> >> - if (features->device_type == BTN_TOOL_DOUBLETAP || >> - features->device_type == BTN_TOOL_TRIPLETAP) { >> + if (features->device_type != BTN_TOOL_PEN) { >> input_abs_set_res(input_dev, ABS_X, >> wacom_calculate_touch_res(features->x_max, >> features->x_phy)); >> input_abs_set_res(input_dev, ABS_Y, >> wacom_calculate_touch_res(features->y_max, >> features->y_phy)); >> - } >> - >> - if (features->device_type != BTN_TOOL_PEN) >> break; /* no need to process stylus stuff */ >> - >> + } >> /* fall through */ >> >> case PL: >> @@ -1166,7 +1203,7 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev, >> case BAMBOO_PT: >> __clear_bit(ABS_MISC, input_dev->absbit); >> >> - if (features->device_type == BTN_TOOL_TRIPLETAP) { >> + if (features->device_type == BTN_TOOL_DOUBLETAP) { >> __set_bit(BTN_LEFT, input_dev->keybit); >> __set_bit(BTN_FORWARD, input_dev->keybit); >> __set_bit(BTN_BACK, input_dev->keybit); >> -- > > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 4/4] input - wacom: Support 2FGT in MT format 2011-02-11 20:35 ` Ping Cheng @ 2011-02-11 20:47 ` Henrik Rydberg 2011-02-11 21:07 ` Ping Cheng 0 siblings, 1 reply; 7+ messages in thread From: Henrik Rydberg @ 2011-02-11 20:47 UTC (permalink / raw) To: Ping Cheng; +Cc: linux-input > >> +static int wacom_tpc_mt_touch(struct wacom_wac *wacom) > >> +{ > >> + struct input_dev *input = wacom->input; > >> + unsigned char *data = wacom->data; > >> + int i; > >> + > >> + if (wacom->shared->stylus_in_proximity && !wacom->shared->touch_down) > >> + return 0; > > > > Seeing one case handled out of four possible always makes me nervous. > > The above statement is to avoid going through the input_mt_slot and > input_mt_report_slot_state routines without posting any meaningful > events. I guess it could be considered as a performance enhancement? It won't be posting events unless something changed. > Which case makes you nervous? I'll take care of it ;). Well, removing the logic above would suffice. :-) > > So only false->true is possible here. > > Yeah, both are bools. What else can they take? Only the transition false-to-true, that is. > > What I can see from the patchset, only wacom_tpc_single_touch() will ever set touch_down to > > false. Is that sufficient? > > No, wacom_tpc_mt_touch can set touch_down to false too. When touch is > false, touch_down will be false. This happens when pen in prox or when > both fingers are up. Now, that is what cannot happen, because of the max() function. > Where else do you see it can be changed? tpc_pen() can not do that > since it is on a different port, that's the reason of touch_down. 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 4/4] input - wacom: Support 2FGT in MT format 2011-02-11 20:47 ` Henrik Rydberg @ 2011-02-11 21:07 ` Ping Cheng 2011-02-11 22:05 ` Henrik Rydberg 0 siblings, 1 reply; 7+ messages in thread From: Ping Cheng @ 2011-02-11 21:07 UTC (permalink / raw) To: Henrik Rydberg; +Cc: linux-input On Fri, Feb 11, 2011 at 12:47 PM, Henrik Rydberg <rydberg@euromail.se> wrote: >> >> +static int wacom_tpc_mt_touch(struct wacom_wac *wacom) >> >> +{ >> >> + struct input_dev *input = wacom->input; >> >> + unsigned char *data = wacom->data; >> >> + int i; >> >> + >> >> + if (wacom->shared->stylus_in_proximity && !wacom->shared->touch_down) >> >> + return 0; >> > >> > Seeing one case handled out of four possible always makes me nervous. >> >> The above statement is to avoid going through the input_mt_slot and >> input_mt_report_slot_state routines without posting any meaningful >> events. I guess it could be considered as a performance enhancement? > > It won't be posting events unless something changed. But it still go through the input_mt_slot and input_mt_report_slot_state routines, which is unnecessary. If we know up front there is no events posting, what benefit do we get to go through the routines? >> Which case makes you nervous? I'll take care of it ;). > > Well, removing the logic above would suffice. :-) Yeah, I see one benefit here ;). I can remove that if others see the same issue. >> > So only false->true is possible here. >> >> Yeah, both are bools. What else can they take? > > Only the transition false-to-true, that is. > >> > What I can see from the patchset, only wacom_tpc_single_touch() will ever set touch_down to >> > false. Is that sufficient? >> >> No, wacom_tpc_mt_touch can set touch_down to false too. When touch is >> false, touch_down will be false. This happens when pen in prox or when >> both fingers are up. > > Now, that is what cannot happen, because of the max() function. This time is real. How about this one? diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c index 39c289d..3cdafb9 100644 --- a/drivers/input/tablet/wacom_wac.c +++ b/drivers/input/tablet/wacom_wac.c @@ -699,7 +699,10 @@ static int wacom_tpc_mt_touch(struct wacom_wac *wacom) } /* keep touch bit to send proper touch up event */ - wacom->shared->touch_down = max(touch, wacom->shared->touch_down); + if (i == 1) + wacom->shared->touch_down = max(touch, wacom->shared->touch_down); + else + wacom->shared->touch_down = touch; } input_mt_report_pointer_emulation(input, true); --------- Thank you. Ping -- 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 4/4] input - wacom: Support 2FGT in MT format 2011-02-11 21:07 ` Ping Cheng @ 2011-02-11 22:05 ` Henrik Rydberg 2011-02-11 23:47 ` Ping Cheng 0 siblings, 1 reply; 7+ messages in thread From: Henrik Rydberg @ 2011-02-11 22:05 UTC (permalink / raw) To: Ping Cheng; +Cc: linux-input > >> The above statement is to avoid going through the input_mt_slot and > >> input_mt_report_slot_state routines without posting any meaningful > >> events. I guess it could be considered as a performance enhancement? > > > > It won't be posting events unless something changed. > > But it still go through the input_mt_slot and > input_mt_report_slot_state routines, which is unnecessary. If we know > up front there is no events posting, what benefit do we get to go > through the routines? Clarity. :-) > >> Which case makes you nervous? I'll take care of it ;). > > > > Well, removing the logic above would suffice. :-) > > Yeah, I see one benefit here ;). I can remove that if others see the same issue. Thanks. > >> No, wacom_tpc_mt_touch can set touch_down to false too. When touch is > >> false, touch_down will be false. This happens when pen in prox or when > >> both fingers are up. > > > > Now, that is what cannot happen, because of the max() function. > > This time is real. How about this one? > > > diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c > index 39c289d..3cdafb9 100644 > --- a/drivers/input/tablet/wacom_wac.c > +++ b/drivers/input/tablet/wacom_wac.c > @@ -699,7 +699,10 @@ static int wacom_tpc_mt_touch(struct wacom_wac *wacom) > } > > /* keep touch bit to send proper touch up event */ > - wacom->shared->touch_down = max(touch, > wacom->shared->touch_down); > + if (i == 1) > + wacom->shared->touch_down = max(touch, > wacom->shared->touch_down); > + else > + wacom->shared->touch_down = touch; > } > > input_mt_report_pointer_emulation(input, true); Well... it might help to return to what the main goal is here. If the purpose is to fix userspace, maybe it should actually be done in userspace instead. 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 4/4] input - wacom: Support 2FGT in MT format 2011-02-11 22:05 ` Henrik Rydberg @ 2011-02-11 23:47 ` Ping Cheng 0 siblings, 0 replies; 7+ messages in thread From: Ping Cheng @ 2011-02-11 23:47 UTC (permalink / raw) To: Henrik Rydberg; +Cc: linux-input On Fri, Feb 11, 2011 at 2:05 PM, Henrik Rydberg <rydberg@euromail.se> wrote: >> >> The above statement is to avoid going through the input_mt_slot and >> >> input_mt_report_slot_state routines without posting any meaningful >> >> events. I guess it could be considered as a performance enhancement? >> > >> > It won't be posting events unless something changed. >> >> But it still go through the input_mt_slot and >> input_mt_report_slot_state routines, which is unnecessary. If we know >> up front there is no events posting, what benefit do we get to go >> through the routines? > > Clarity. :-) Oh, that. We could add some comments to make it clear. In fact, Chris has suggested to add more comments in general. But I have seen review comments like "the code tells the others what you are saying". So, we deferred. How about this: + + /* no touch events should be posted when pen is in proximity and touch was up */ + if (wacom->shared->stylus_in_proximity && !wacom->shared->touch_down) + return 0; >> >> No, wacom_tpc_mt_touch can set touch_down to false too. When touch is >> >> false, touch_down will be false. This happens when pen in prox or when >> >> both fingers are up. >> > >> > Now, that is what cannot happen, because of the max() function. >> >> This time is real. How about this one? >> >> >> diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c >> index 39c289d..3cdafb9 100644 >> --- a/drivers/input/tablet/wacom_wac.c >> +++ b/drivers/input/tablet/wacom_wac.c >> @@ -699,7 +699,10 @@ static int wacom_tpc_mt_touch(struct wacom_wac *wacom) >> } >> >> /* keep touch bit to send proper touch up event */ >> - wacom->shared->touch_down = max(touch, >> wacom->shared->touch_down); >> + if (i == 1) >> + wacom->shared->touch_down = max(touch, >> wacom->shared->touch_down); >> + else >> + wacom->shared->touch_down = touch; >> } >> >> input_mt_report_pointer_emulation(input, true); > > Well... it might help to return to what the main goal is here. If the > purpose is to fix userspace, maybe it should actually be done in > userspace instead. The goal is to support legacy ST client. A clean support would be to send pen data and pen data only whenever pen is in prox, no matter touch is down or not; sending ST touch data only when pen is out-prox. The concept of ST is that in any single moment, there would be only one type of data in action. In that sense, avoiding both pen and touch in action at the same time is our (kernel's) responsibility. ST clients do not have the ability to deal with dual types. Can I put your reviewed-by if I make a v2 of this patch? Thank you for your help. Ping -- 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-02-11 23:47 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-02-11 1:33 [PATCH 4/4] input - wacom: Support 2FGT in MT format Ping Cheng 2011-02-11 20:20 ` Henrik Rydberg 2011-02-11 20:35 ` Ping Cheng 2011-02-11 20:47 ` Henrik Rydberg 2011-02-11 21:07 ` Ping Cheng 2011-02-11 22:05 ` Henrik Rydberg 2011-02-11 23:47 ` Ping Cheng
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).