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@xxxxxxxxxxxxxx] Sent: Tuesday, July 05, 2011 7:32 PM To: Ping Cheng Cc: linux-input@xxxxxxxxxxxxxxx; dmitry.torokhov@xxxxxxxxx; rydberg@xxxxxxxxxxx; 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@xxxxxxxxx> 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@xxxxxxxxx> > --- > 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@xxxxxxxxxxxxxx> > 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@xxxxxxxxxxxxxxx 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html