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

[Drews, Paul]

> -----Original Message-----
> From: linux-kernel-owner@xxxxxxxxxxxxxxx [mailto:linux-kernel-
> owner@xxxxxxxxxxxxxxx] On Behalf Of Benjamin Tissoires
> Sent: Wednesday, May 09, 2012 12:04 PM
> To: Henrik Rydberg
> Cc: Dmitry Torokhov; Jiri Kosina; Stephane Chatty; linux-input@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 1/5] HID: hid-multitouch: fix wrong protocol detection
> 

> On Sun, May 6, 2012 at 9:01 PM, Henrik Rydberg <rydberg@xxxxxxxxxxx> wrote:
> > Hi Benjamin,
> >
> >> 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.
> >>

> >> +
> >>  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;
> >> +     }
> >> +}
> >> +

It sounds as though:

() Reviewers are a little uncomfortable with the memory footprint and
    allocation/free
() The patch as it stands relies on the pattern of "usage" values repeating
    for each touch, and deeming the last one in the repetition pattern to
    be the last-slot-field marker.

If this is the case, how about avoiding storing all the slot-field values
and just detecting the point of repetition to use the most-recently-seen
usage value as the last-slot-field marker.  I have been successfully using
the patch below based on this notion.  It took the failure rate from about
1-per-10 boots to 250+ boots with no failures on an Atmel MaXTouch.
I don't have others to try it with, including the "buggy" one that led
to all this trouble in the first place.

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