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

[Drews, Paul]

Hi Benjamin,

> Hi Paul,
> 
> On Fri, May 18, 2012 at 11:14 PM, Drews, Paul <paul.drews@xxxxxxxxx> wrote:
> >
> >
> >> -----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
> >>
...
> > 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.
> 
> Thank you very much for this patch. However, Jiri already applied mine
> with the allocation/free mechanism.
> 
> You're idea is good but it has one big problem with Win8 devices:
> As we can have 2 X and 2 Y per touch report, if these dual-X reporting
> or dual-Y reporting is present in the report, we will stop at the
> second X or the second Y seen, which will lead to a buggy touchscreen
> (the first touch won't get it's second coordinate). However, without
> this particularity, the patch would have worked ;-)
> 
> If the Win8 norm has came earlier, the initial implementation that
> relies on the collection would have suffice, but some hardware makers
> made a bad use of it, leading us to stop using this, and relying on a
> more brutal approach.

Oops.  Didn't know about that.  If the first item is duplicated somewhere
in the sequence, that's a fatal problem for my approach.

> > +static void update_last_slot_field(struct hid_usage *usage,
> > +               struct mt_device *td)
> >  {
> > -       if (!test_bit(usage->hid, hi->input->absbit))
> > -               td->last_slot_field = usage->hid;
> > +       if (!td->last_slot_field_found) {
> > +               if (td->first_slot_field_found) {
> > +                       if (td->last_slot_field == usage->hid)
> 
> I'm sure you wanted to put here:
>                        if (td->first_slot_field == usage->hid)
> 
> Cheers,
> Benjamin

Good catch.  And as you point out, irrelevant since your patch is in
linux-next already.  I tested your commit 3ac36d1 from there with
a 3.4 (final) kernel on a Atmel MaXTouch Digitizer tablet and it is
working fine.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/