On Mon, Nov 21, 2011 at 2:25 PM, Chris Bagwell <[email protected]> wrote:
> 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.

Chris is right. We need MSC_SERIAL to report P5 (I4 is a P5 device) tools.

> 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.

I agree. All future P4 devices should be claimed as generic tablets. I
thought we've updated that already. After checking into the code I saw
MSC_SERIAL is still there (sigh). So, we should not report MSC_SERIAL
for P4 devices. Chris, are you interested in making patches to fix
issue, or do you prefer me to take care of it?

We'll need users to test the patches, especially for older P4 tablets,
before pushing the patch upstream.

Ping

------------------------------------------------------------------------------
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