diff mbox

[2/3,v2] input : wacom - Update Graphire4 and old Bamboo tablet buttons

Message ID 1309908332-18829-1-git-send-email-pinglinux@gmail.com (mailing list archive)
State Accepted
Commit 0bd10ef8f8a29d824561a4678f5e63350751407a
Headers show

Commit Message

Ping Cheng July 5, 2011, 11:25 p.m. UTC
Bamboo touch sets BTN_BACK, BTN_FORWARD, BTN_LEFT, and BTN_RIGHT
as the default button events for tablet buttons. Change Graphire4
and old Bamboo to the same settings.

Signed-off-by: Ping Cheng <pingc@wacom.com>
---
 drivers/input/tablet/wacom_wac.c |   19 ++++++++-----------
 1 files changed, 8 insertions(+), 11 deletions(-)

Comments

chris@cnpbagwell.com July 6, 2011, 2:32 a.m. UTC | #1
On Tue, Jul 5, 2011 at 6:25 PM, Ping Cheng <pinglinux@gmail.com> wrote:
> Bamboo touch sets BTN_BACK, BTN_FORWARD, BTN_LEFT, and BTN_RIGHT
> as the default button events for tablet buttons. Change Graphire4
> and old Bamboo to the same settings.

This should make the tablets more useful out of the box.

At least with xf86-input-wacom, I believe today's default mapping
causes an almost direct mapping to X button #'s so that means BTN_0 is
dropped (X buttons start at 1), BTN_1 treated as LEFT and BTN_4/5
treated as BACK/FORWARD (it skips over buttons 4/5/6/7 when creating
default mappings).

So for most people, its only the BTN_LEFT/RIGHT that would cause a
visible difference and I'd think for the better.

I have one question below.

>
> Signed-off-by: Ping Cheng <pingc@wacom.com>
> ---
>  drivers/input/tablet/wacom_wac.c |   19 ++++++++-----------
>  1 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
> index b93501a..19554a4 100644
> --- a/drivers/input/tablet/wacom_wac.c
> +++ b/drivers/input/tablet/wacom_wac.c
> @@ -275,8 +275,8 @@ static int wacom_graphire_irq(struct wacom_wac *wacom)
>                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));
> +                       input_report_key(input, BTN_BACK, (data[7] & 0x40));
> +                       input_report_key(input, BTN_FORWARD, (data[7] & 0x80));
>                        rw = ((data[7] & 0x18) >> 3) - ((data[7] & 0x20) >> 3);
>                        input_report_rel(input, REL_WHEEL, rw);
>                        if (!prox)
> @@ -291,10 +291,10 @@ static int wacom_graphire_irq(struct wacom_wac *wacom)
>                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_key(input, BTN_BACK, (data[7] & 0x08));
> +                       input_report_key(input, BTN_LEFT, (data[7] & 0x20));
> +                       input_report_key(input, BTN_FORWARD, (data[7] & 0x10));
> +                       input_report_key(input, BTN_RIGHT, (data[7] & 0x40));
>                        input_report_abs(input, ABS_WHEEL, (data[8] & 0x7f));
>                        if (!prox)
>                                wacom->id[1] = 0;
> @@ -1076,17 +1076,14 @@ 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);
> -

Can you add a message to commit on why this is deleted instead of
changed?  I'm guessing WACOM_MO series must support mice pucks and so
already declare BTN_LEFT and BTN_RIGHT?

Other than that:

Reviewed-by: Chris Bagwell <chris@cnpbagwell.com>

>                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_0, input_dev->keybit);
> -               __set_bit(BTN_4, input_dev->keybit);
> +               __set_bit(BTN_BACK, input_dev->keybit);
> +               __set_bit(BTN_FORWARD, input_dev->keybit);
>                /* fall through */
>
>        case GRAPHIRE:
> --
> 1.7.5.4
>
> --
> 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
>
--
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
Ping Cheng July 6, 2011, 5:38 a.m. UTC | #2
Chris,

Thank you for the review. BTN_LEFT/RIGHT is declared in the code inside the " case GRAPHIRE:" block if you take a look at the whole routine.

Ping

-----Original Message-----
From: Chris Bagwell [mailto:chris@cnpbagwell.com] 
Sent: Tuesday, July 05, 2011 7:32 PM
To: Ping Cheng
Cc: linux-input@vger.kernel.org; dmitry.torokhov@gmail.com; rydberg@euromail.se; Ping Cheng
Subject: Re: [PATCH 2/3 v2] input : wacom - Update Graphire4 and old Bamboo tablet buttons

On Tue, Jul 5, 2011 at 6:25 PM, Ping Cheng <pinglinux@gmail.com> wrote:
> Bamboo touch sets BTN_BACK, BTN_FORWARD, BTN_LEFT, and BTN_RIGHT as 
> the default button events for tablet buttons. Change Graphire4 and old 
> Bamboo to the same settings.

This should make the tablets more useful out of the box.

At least with xf86-input-wacom, I believe today's default mapping causes an almost direct mapping to X button #'s so that means BTN_0 is dropped (X buttons start at 1), BTN_1 treated as LEFT and BTN_4/5 treated as BACK/FORWARD (it skips over buttons 4/5/6/7 when creating default mappings).

So for most people, its only the BTN_LEFT/RIGHT that would cause a visible difference and I'd think for the better.

I have one question below.

>
> Signed-off-by: Ping Cheng <pingc@wacom.com>
> ---
>  drivers/input/tablet/wacom_wac.c |   19 ++++++++-----------
>  1 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/input/tablet/wacom_wac.c 
> b/drivers/input/tablet/wacom_wac.c
> index b93501a..19554a4 100644
> --- a/drivers/input/tablet/wacom_wac.c
> +++ b/drivers/input/tablet/wacom_wac.c
> @@ -275,8 +275,8 @@ static int wacom_graphire_irq(struct wacom_wac 
> *wacom)
>                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));
> +                       input_report_key(input, BTN_BACK, (data[7] & 
> + 0x40));
> +                       input_report_key(input, BTN_FORWARD, (data[7] 
> + & 0x80));
>                        rw = ((data[7] & 0x18) >> 3) - ((data[7] & 
> 0x20) >> 3);
>                        input_report_rel(input, REL_WHEEL, rw);
>                        if (!prox)
> @@ -291,10 +291,10 @@ static int wacom_graphire_irq(struct wacom_wac 
> *wacom)
>                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_key(input, BTN_BACK, (data[7] & 
> + 0x08));
> +                       input_report_key(input, BTN_LEFT, (data[7] & 
> + 0x20));
> +                       input_report_key(input, BTN_FORWARD, (data[7] 
> + & 0x10));
> +                       input_report_key(input, BTN_RIGHT, (data[7] & 
> + 0x40));
>                        input_report_abs(input, ABS_WHEEL, (data[8] & 
> 0x7f));
>                        if (!prox)
>                                wacom->id[1] = 0; @@ -1076,17 +1076,14 
> @@ 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);
> -

Can you add a message to commit on why this is deleted instead of changed?  I'm guessing WACOM_MO series must support mice pucks and so already declare BTN_LEFT and BTN_RIGHT?

Other than that:

Reviewed-by: Chris Bagwell <chris@cnpbagwell.com>

>                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_0, input_dev->keybit);
> -               __set_bit(BTN_4, input_dev->keybit);
> +               __set_bit(BTN_BACK, input_dev->keybit);
> +               __set_bit(BTN_FORWARD, input_dev->keybit);
>                /* fall through */
>
>        case GRAPHIRE:
> --
> 1.7.5.4
>
> --
> 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
>
--
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
diff mbox

Patch

diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
index b93501a..19554a4 100644
--- a/drivers/input/tablet/wacom_wac.c
+++ b/drivers/input/tablet/wacom_wac.c
@@ -275,8 +275,8 @@  static int wacom_graphire_irq(struct wacom_wac *wacom)
 		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));
+			input_report_key(input, BTN_BACK, (data[7] & 0x40));
+			input_report_key(input, BTN_FORWARD, (data[7] & 0x80));
 			rw = ((data[7] & 0x18) >> 3) - ((data[7] & 0x20) >> 3);
 			input_report_rel(input, REL_WHEEL, rw);
 			if (!prox)
@@ -291,10 +291,10 @@  static int wacom_graphire_irq(struct wacom_wac *wacom)
 		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_key(input, BTN_BACK, (data[7] & 0x08));
+			input_report_key(input, BTN_LEFT, (data[7] & 0x20));
+			input_report_key(input, BTN_FORWARD, (data[7] & 0x10));
+			input_report_key(input, BTN_RIGHT, (data[7] & 0x40));
 			input_report_abs(input, ABS_WHEEL, (data[8] & 0x7f));
 			if (!prox)
 				wacom->id[1] = 0;
@@ -1076,17 +1076,14 @@  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);
-
 		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_0, input_dev->keybit);
-		__set_bit(BTN_4, input_dev->keybit);
+		__set_bit(BTN_BACK, input_dev->keybit);
+		__set_bit(BTN_FORWARD, input_dev->keybit);
 		/* fall through */
 
 	case GRAPHIRE: