Hi Chris,
Please see my comments inline.
Ping
On Wed, Mar 16, 2011 at 6:43 AM, <[email protected]> wrote:
> From: Chris Bagwell <[email protected]>
>
> Previously, buttons and ring events from tablet pad itself
> where sent as a tool called BTN_TOOL_FINGER and that
>
nitpicking, a typo.
> finger event never toggled back to zero. Its serial
> number also never transition to zero.
>
> This approach had some issues in user land when switching
> between stylus and pad buttons and initial event was usually
> sent before user apps ever started. This could be improved
> by toggling BTN_TOOL_FINGER back to zero when all buttons
> released; like most other wacom drivers are doing.
>
> Instead, convert to more "standard" approach where tablet pad buttons
> are sent always without announcing them as tools. This aligns
> with newer Bamboo P&T in same file.
>
> Note: REL_WHEEL can be on both mouse tool and tablet pad.
> Userland must always treat them as coming from tablet pad to avoid
> confusion (don't ignore while mouse tool is out of proximity).
>
Can we use another bit for the pad wheel, such are REL_HWHEEL so both
wheels can be reported and processed in the userland?
Signed-off-by: Chris Bagwell <[email protected]>
> ---
> drivers/input/tablet/wacom_wac.c | 148
> ++++++++++++++++++--------------------
> 1 files changed, 70 insertions(+), 78 deletions(-)
>
> diff --git a/drivers/input/tablet/wacom_wac.c
> b/drivers/input/tablet/wacom_wac.c
> index 74056b9..15bb104 100644
> --- a/drivers/input/tablet/wacom_wac.c
> +++ b/drivers/input/tablet/wacom_wac.c
> @@ -195,109 +195,104 @@ static int wacom_dtu_irq(struct wacom_wac *wacom)
> static int wacom_graphire_irq(struct wacom_wac *wacom)
> {
> struct wacom_features *features = &wacom->features;
> - unsigned char *data = wacom->data;
> struct input_dev *input = wacom->input;
> - int prox;
> - int rw = 0;
> - int retval = 0;
> + unsigned char *data = wacom->data;
> + int prox = 0, x = 0, y = 0, p = 0, d = 0, rw = 0, aw = 0,
> + pen = 0, btns1 = 0, btns2 = 0, btnl = 0, btnm = 0, btnr = 0,
> + btn0 = 0, btn1 = 0, btn3 = 0, btn4 = 0;
>
> if (data[0] != WACOM_REPORT_PENABLED) {
> dbg("wacom_graphire_irq: received unknown report #%d",
> data[0]);
> - goto exit;
> + return 0;
> }
>
> prox = data[1] & 0x80;
> - if (prox || wacom->id[0]) {
> - if (prox) {
> - switch ((data[1] >> 5) & 3) {
>
> + /*
> + * firmware can send invalid data when out of proximity so
> + * send zeros to be safe. Also gives apps a fresh starting
> + * point when entering proximity.
> + */
> + if (prox) {
> + switch ((data[1] >> 5) & 3) {
> case 0: /* Pen */
> wacom->tool[0] = BTN_TOOL_PEN;
> - wacom->id[0] = STYLUS_DEVICE_ID;
> break;
>
> case 1: /* Rubber */
> wacom->tool[0] = BTN_TOOL_RUBBER;
> - wacom->id[0] = ERASER_DEVICE_ID;
> break;
>
> case 2: /* Mouse with wheel */
> - input_report_key(input, BTN_MIDDLE, data[1]
> & 0x04);
>
Shouldn't we assign btnm to data[1] & 0x04 here?
To reduce regression testing effort, do you mind we only change the lines
that we have to change? I know the code can be improved. But there will be
no new devices going through this routine. Minimal changes to fix the in/out
prox issue and to bring the routine to the generic format would require
least effort in testing.
What do you think?
Ping
> - /* fall through */
> -
> case 3: /* Mouse without wheel */
> wacom->tool[0] = BTN_TOOL_MOUSE;
> - wacom->id[0] = CURSOR_DEVICE_ID;
> break;
> - }
> }
> - input_report_abs(input, ABS_X, le16_to_cpup((__le16
> *)&data[2]));
> - input_report_abs(input, ABS_Y, le16_to_cpup((__le16
> *)&data[4]));
> - if (wacom->tool[0] != BTN_TOOL_MOUSE) {
> - input_report_abs(input, ABS_PRESSURE, data[6] |
> ((data[7] & 0x01) << 8));
> - input_report_key(input, BTN_TOUCH, data[1] & 0x01);
> - input_report_key(input, BTN_STYLUS, data[1] &
> 0x02);
> - input_report_key(input, BTN_STYLUS2, data[1] &
> 0x04);
> - } else {
> - input_report_key(input, BTN_LEFT, data[1] & 0x01);
> - input_report_key(input, BTN_RIGHT, data[1] & 0x02);
> +
> + x = le16_to_cpup((__le16 *)&data[2]);
> + y = le16_to_cpup((__le16 *)&data[4]);
> +
> + if (wacom->tool[0] == BTN_TOOL_MOUSE ) {
> if (features->type == WACOM_G4 ||
> - features->type == WACOM_MO) {
> - input_report_abs(input, ABS_DISTANCE,
> data[6] & 0x3f);
> + features->type == WACOM_MO) {
> + d = data[6] & 0x3f;
> rw = (data[7] & 0x04) - (data[7] & 0x03);
> } else {
> - input_report_abs(input, ABS_DISTANCE,
> data[7] & 0x3f);
> + d = data[7] & 0x3f;
> rw = -(signed char)data[6];
> }
> - input_report_rel(input, REL_WHEEL, rw);
> + btnl = data[1] & 0x01;
> + btnr = data[1] & 0x02;
> + btnm = data[1] & 0x04;
> + } else {
> + p = data[6] | ((data[7] & 0x01) << 8);
> + pen = data[1] & 0x01;
> + btns1 = data[1] & 0x02;
> + btns2 = data[1] & 0x04;
> }
> -
> - if (!prox)
> - wacom->id[0] = 0;
> - input_report_abs(input, ABS_MISC, wacom->id[0]); /* report
> tool id */
> - input_report_key(input, wacom->tool[0], prox);
> - input_sync(input); /* sync last event */
> }
>
> - /* send pad data */
> - switch (features->type) {
> - case WACOM_G4:
> - prox = data[7] & 0xf8;
> - if (prox || wacom->id[1]) {
> - wacom->id[1] = PAD_DEVICE_ID;
> - input_report_key(input, BTN_0, (data[7] & 0x40));
> - input_report_key(input, BTN_4, (data[7] & 0x80));
> + /* Buttons and wheels on tablet are always reported */
> + if (features->type == WACOM_G4) {
> + btn0 = data[7] & 0x40;
> + btn1 = data[7] & 0x80;
> + /*
> + * Mouse wheel and tablet wheel share event but mouse
> + * event is only valid when tool is in proximity.
> + * When both are in use, give priority to mouse.
> + */
> + if (!rw)
> rw = ((data[7] & 0x18) >> 3) - ((data[7] & 0x20) >>
> 3);
> - input_report_rel(input, REL_WHEEL, rw);
> - input_report_key(input, BTN_TOOL_FINGER, 0xf0);
> - if (!prox)
> - wacom->id[1] = 0;
> - input_report_abs(input, ABS_MISC, wacom->id[1]);
> - input_event(input, EV_MSC, MSC_SERIAL, 0xf0);
> - retval = 1;
> - }
> - break;
> -
> - case WACOM_MO:
> - prox = (data[7] & 0xf8) || data[8];
> - if (prox || wacom->id[1]) {
> - wacom->id[1] = PAD_DEVICE_ID;
> - input_report_key(input, BTN_0, (data[7] & 0x08));
> - input_report_key(input, BTN_1, (data[7] & 0x20));
> - input_report_key(input, BTN_4, (data[7] & 0x10));
> - input_report_key(input, BTN_5, (data[7] & 0x40));
> - input_report_abs(input, ABS_WHEEL, (data[8] &
> 0x7f));
> - input_report_key(input, BTN_TOOL_FINGER, 0xf0);
> - if (!prox)
> - wacom->id[1] = 0;
> - input_report_abs(input, ABS_MISC, wacom->id[1]);
> - input_event(input, EV_MSC, MSC_SERIAL, 0xf0);
> - }
> - retval = 1;
> - break;
> + } else if (features->type == WACOM_MO) {
> + btn0 = data[7] & 0x08;
> + btn1 = data[7] & 0x10;
> + btn3 = data[7] & 0x20;
> + btn4 = data[7] & 0x40;
> + aw = data[8] & 0x7f;
> }
> -exit:
> - return retval;
> +
> + input_report_key(input, BTN_TOUCH, pen);
> + input_report_key(input, BTN_STYLUS, btns1);
> + input_report_key(input, BTN_STYLUS2, btns2);
> + input_report_key(input, BTN_LEFT, btnl);
> + input_report_key(input, BTN_MIDDLE, btnm);
> + input_report_key(input, BTN_RIGHT, btnr);
> + input_report_key(input, BTN_0, btn0);
> + input_report_key(input, BTN_1, btn1);
> + input_report_key(input, BTN_3, btn3);
> + input_report_key(input, BTN_4, btn4);
> +
> + input_report_abs(input, ABS_X, x);
> + input_report_abs(input, ABS_Y, y);
> + input_report_abs(input, ABS_PRESSURE, p);
> + input_report_abs(input, ABS_DISTANCE, d);
> + input_report_abs(input, ABS_WHEEL, aw);
> +
> + input_report_rel(input, REL_WHEEL, rw);
> +
> + input_report_key(input, wacom->tool[0], prox);
> +
> + return 1;
> }
>
> static int wacom_intuos_inout(struct wacom_wac *wacom)
> @@ -1055,18 +1050,15 @@ void wacom_setup_input_capabilities(struct
> input_dev *input_dev,
>
> switch (wacom_wac->features.type) {
> case WACOM_MO:
> - __set_bit(BTN_1, input_dev->keybit);
> - __set_bit(BTN_5, input_dev->keybit);
> + __set_bit(BTN_3, input_dev->keybit);
> + __set_bit(BTN_4, input_dev->keybit);
>
> input_set_abs_params(input_dev, ABS_WHEEL, 0, 71, 0, 0);
> /* fall through */
>
> case WACOM_G4:
> - input_set_capability(input_dev, EV_MSC, MSC_SERIAL);
> -
> - __set_bit(BTN_TOOL_FINGER, input_dev->keybit);
> __set_bit(BTN_0, input_dev->keybit);
> - __set_bit(BTN_4, input_dev->keybit);
> + __set_bit(BTN_1, input_dev->keybit);
> /* fall through */
>
> case GRAPHIRE:
> --
> 1.7.4
>
>
>
> ------------------------------------------------------------------------------
> Colocation vs. Managed Hosting
> A question and answer guide to determining the best fit
> for your organization - today and in the future.
> http://p.sf.net/sfu/internap-sfd2d
> _______________________________________________
> Linuxwacom-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
>
------------------------------------------------------------------------------
Colocation vs. Managed Hosting
A question and answer guide to determining the best fit
for your organization - today and in the future.
http://p.sf.net/sfu/internap-sfd2d
_______________________________________________
Linuxwacom-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
