On Mon, Nov 21, 2011 at 3:44 PM, Przemo Firszt <[email protected]> wrote:
> This patch adds parsing pad buttons for Intuos4. The change of "buttstate"
> type is required, because there are 9 buttons on Intuos4, so "char" wasn't
> enough.
>
> Signed-off-by: Przemo Firszt <[email protected]>
> ---
>  drivers/hid/hid-wacom.c |   31 ++++++++++++++++++++++++++++++-
>  1 files changed, 30 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/hid/hid-wacom.c b/drivers/hid/hid-wacom.c
> index f218348..378b26f 100644
> --- a/drivers/hid/hid-wacom.c
> +++ b/drivers/hid/hid-wacom.c
> @@ -33,7 +33,7 @@
>
>  struct wacom_data {
>        __u16 tool;
> -       unsigned char butstate;
> +       __u16 butstate;
>        __u8 features;
>        unsigned char high_speed;
>  #ifdef CONFIG_HID_WACOM_POWER_SUPPLY
> @@ -306,6 +306,27 @@ static int wacom_gr_parse_report(struct hid_device *hdev,
>        return 1;
>  }
>
> +static void wacom_i4_parse_button_report(struct wacom_data *wdata,
> +                      struct input_dev *input, unsigned char *data)
> +{
> +       __u16 new_butstate;
> +
> +       new_butstate = (data[3] << 1) | (data[2] & 0x01);
> +       if (new_butstate != wdata->butstate) {
> +               wdata->butstate = new_butstate;
> +               input_report_key(input, BTN_0, new_butstate & 0x001);
> +               input_report_key(input, BTN_1, new_butstate & 0x002);
> +               input_report_key(input, BTN_2, new_butstate & 0x004);
> +               input_report_key(input, BTN_3, new_butstate & 0x008);
> +               input_report_key(input, BTN_4, new_butstate & 0x010);
> +               input_report_key(input, BTN_5, new_butstate & 0x020);
> +               input_report_key(input, BTN_6, new_butstate & 0x040);
> +               input_report_key(input, BTN_7, new_butstate & 0x080);
> +               input_report_key(input, BTN_8, new_butstate & 0x100);
> +               input_sync(input);
> +       }
> +}

The code all looks good but its following style of "generic" tablet.
Thats good in my book but for this to work with no chance of issues I
would think you need to conditionally declare MSC_SERIAL.  If you
decide to make that improvement, I'd make it its own patch.

Here is extra info to help you out in case you are seeing any button
issues.  As long as your declaring MSC_SERIAL, xf86-input-wacom is
using it to choose an internal channel to store button info in.  If
you do not declare MSC_SERIAL then xf86-input-wacom will use some hard
coded logic to put BTN_STYLUS* in channel 0 and all other buttons in
channel 2.

Since BTN_STYLUS and BTN_0 both map to X button 1, they would
overwrite each other if not routed to separate channels.

I believe MSC_SERIAL is initialized to zero by kernel so your always
sending that and therefore BTN_STYLUS/BTN_STYLUS1 and BTN_0/BTN_2 are
in conflict with each other inside xf86-input-wacom.

OK, having said that, I consider this more a xf86-input-wacom issue
than a kernel side issue ("generic" tablets should be allowed to
declare serial #'s) and its unlikely that user will click stylus and
tablet buttons at same time.  Come to think of it, we may should
modify xf86-input-wacom to stop basing Protocol 4 support based on
MSC_SERIAL and instead rely on hard coded list of tablet_id's.

So I'll leave it for you to decide how best to proceed and for this
patch you have my:

Reviewed-by: Chris Bagwell <[email protected]>

Chris

> +
>  static void wacom_i4_parse_pen_report(struct wacom_data *wdata,
>                        struct input_dev *input, unsigned char *data)
>  {
> @@ -369,6 +390,7 @@ static void wacom_i4_parse_report(struct hid_device *hdev,
>                wdata->features = data[2];
>                break;
>        case 0x0C: /* Button report */
> +               wacom_i4_parse_button_report(wdata, input, data);
>                break;
>        default:
>                hid_err(hdev, "Unknown report: %d,%d\n", data[0], data[1]);
> @@ -463,6 +485,13 @@ static int wacom_input_mapped(struct hid_device *hdev, 
> struct hid_input *hi,
>                input_set_abs_params(input, ABS_DISTANCE, 0, 32, 0, 0);
>                break;
>        case USB_DEVICE_ID_WACOM_INTUOS4_BLUETOOTH:
> +               __set_bit(BTN_2, input->keybit);
> +               __set_bit(BTN_3, input->keybit);
> +               __set_bit(BTN_4, input->keybit);
> +               __set_bit(BTN_5, input->keybit);
> +               __set_bit(BTN_6, input->keybit);
> +               __set_bit(BTN_7, input->keybit);
> +               __set_bit(BTN_8, input->keybit);
>                input_set_abs_params(input, ABS_X, 0, 40640, 4, 0);
>                input_set_abs_params(input, ABS_Y, 0, 25400, 4, 0);
>                input_set_abs_params(input, ABS_PRESSURE, 0, 2047, 0, 0);
> --
> 1.7.6.4
>
>

------------------------------------------------------------------------------
All the data continuously generated in your IT infrastructure 
contains a definitive record of customers, application performance, 
security threats, fraudulent activity, and more. Splunk takes this 
data and makes sense of it. IT sense. And common sense.
http://p.sf.net/sfu/splunk-novd2d
_______________________________________________
Linuxwacom-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel

Reply via email to