Re: [PATCH 1/5] HID: hid-multitouch: fix wrong protocol detection

[Drews, Paul]

>>> The previous implementation introduced a randomness in the splitting
>>> of the different touches reported by the device. This version is more
>>> robust as we don't rely on hi->input->absbit, but on our own structure.
>>>
>>> This also prepares hid-multitouch to better support Win8 devices.
>>>
>>> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@xxxxxxx>
>
>>>  struct mt_device {
>>>       struct mt_slot curdata; /* placeholder of incoming data */
>>>       struct mt_class mtclass;        /* our mt device class */
>>> +     struct mt_fields *fields;       /* temporary placeholder for storing the
>>> +                                        multitouch fields */
>>
>> Why not skip the pointer here?
>
>well, the idea was to keep the memory footprint low. As these values
>are only needed at init, then I freed them once I finished using them.
>I can of course skip the pointer, but in that case, wouldn't the
>struct declaration be worthless?
>
>
>>> +static void mt_post_parse(struct mt_device *td)
>>> +{
>>> +     struct mt_fields *f = td->fields;
>>> +
>>> +     if (td->touches_by_report > 0) {
>>> +             int field_count_per_touch = f->length / td->touches_by_report;
>>> +             td->last_slot_field = f->usages[field_count_per_touch - 1]->hid;
>>> +     }
>>> +}
>>> +
>

The patch as it stands more-or-less depends on the group of usage->hid
values repeating for each touch in the multi-touch report, and choosing
the last usage->hid seen in the first group as the ultimate last_slot_field
value.  A suggestion: as long as we're relying on this group repetition
anyway, why not take advantage of the repetition wrap-around to
detect the last_slot_field without having to allocate memory and store
everything?  I've been using the following patch that does it this way
with an Atmel MaXTouch Digitizer (3EB:211C).

Prior to this patch I was getting a MTBF of about 1 failure in 10 boots
due to the out-of-range bitmap lookup coming up with an unlucky
result and making the wrong last_slot_field conclusion.  Symptom:
touch events get reported to user-space with previous x,y coordinates.
Also confirmed using a printk to instrument the kernel for this.

With this patch, I have tested beyond 10X the MTBF on 3.4-rc7 with no failures.
I don't have a touchscreen other than that Atmel to test with.  Will this
method work with the buggy touchscreen that the original patch was
intended to fix?

Patch follows:
========================================================