Re: [PATCH] HID: wacom: Stop mapping touch usages after processing HID_DG_CONTACTCOUNT

[Jason Gerecke <killertofu@gmail.com>]

On 07/12/2016 12:32 AM, Benjamin Tissoires wrote:
> Hi Jason,
> 
> On Jul 11 2016 or thereabouts, Jason Gerecke wrote:
>> If a touchscreen contains both multitouch and single-touch reports in its
>> descriptor in that order, the driver may overwrite information it saved
>> about the format of the multitouch report. This can cause the report
>> processing code to get tripped up and send an incorrect event stream to
>> userspace.
>>
>> In particular, this can cause last_slot_field to be overwritten with the
>> result that the driver prematurely assumes it has finished processing a
>> slot and sending the ABS_MT_SLOT event at the wrong point in time,
>> associating events for the current contact with the following contact
>> instead.
>>
>> To prevent this from occuring, we stop mapping usages after having seen
>> HID_DG_CONTACTCOUNT. This usage is only present in multitouch reports,
>> so the format of any following single-touch reports will have no effect.
> 
> I had a quick look at the driver, and it looks like the Cintiq Companion
> 2 has more than one multitouch collection (see 499522c "HID: wacom: Tie
> cached HID_DG_CONTACTCOUNT indices to report ID").
> 
> So if this doesn't break the cintiq companion 2, you have my
> reviewed-by, but I'd rather be sure (and please see if we really need to
> keep 499522c then).
> 
> Cheers,
> Benjamin
> 

Thanks for the reminder about 499522c. This patch should supersede it in
terms of functionality, so I can make a patch to remove it instead.

As far as compatibility goes, I don't have a Companion 2 to actually
test, but looking at the descriptor [1], it shouldn't pose any problems:
the second report containing HID_DG_CONTACTCOUNT is the single-touch
report that we don't want to use anyway.

[1]:
https://github.com/linuxwacom/wacom-hid-descriptors/blob/master/Wacom%20Cintiq%20Companion%202/0003:056A:0326.0002.hid.txt

Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one /
(That is to say, eight) to the two, /
But you can’t take seven from three, /
So you look at the sixty-fours....

>>
>> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
>> ---
>>  drivers/hid/wacom_wac.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
>> index fcf2264..e499cdb 100644
>> --- a/drivers/hid/wacom_wac.c
>> +++ b/drivers/hid/wacom_wac.c
>> @@ -1560,6 +1560,11 @@ static void wacom_wac_finger_usage_mapping(struct hid_device *hdev,
>>  	struct input_dev *input = wacom_wac->touch_input;
>>  	unsigned touch_max = wacom_wac->features.touch_max;
>>  
>> +	/* stop processing after the first multitouch report */
>> +	if (wacom_wac->hid_data.cc_report &&
>> +	    wacom_wac->hid_data.cc_report != field->report->id)
>> +		return;
>> +
>>  	switch (usage->hid) {
>>  	case HID_GD_X:
>>  		features->last_slot_field = usage->hid;
>> -- 
>> 2.9.0
>>
--
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