linux-kernel - Re: [PATCH 19/19] HID: multitouch: Remove the redundant touch state

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 20 Aug 2012 18:01:50 +0200
From:	"Henrik Rydberg" <rydberg@...math.se>
To:	Benjamin Tissoires <benjamin.tissoires@...c.fr>
Cc:	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Jiri Kosina <jkosina@...e.cz>, linux-input@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 19/19] HID: multitouch: Remove the redundant touch state

Hi Benjamin,

> This patch seems to be a little bit complex.
> It has very good things, but also many things that hinders the readability.
> 
> And you should also remove the /* touchscreen emulation */ things in
> mt_input_mapping as input_mt_init_slots handles now the init of ABS_X
> ABS_Y and ABS_PRESSURE.

Yes, it could be changed as well, like the bit patterns. As you
mention below, the logic around the bits could be enhanced, and the
ABS_X/Y should go in that set of changes.

> > -struct mt_slot {
> > -       __s32 x, y, p, w, h;
> > -       __s32 contactid;        /* the device ContactID assigned to this slot */
> > -       bool touch_state;       /* is the touch valid? */
> > -       bool seen_in_this_frame;/* has this slot been updated */
> > -};
> 
> Why removing this struct?
> Removing it infers a lot of unneeded changes in the patch.
> 
> As the mt_sync_frame handle the quirk NOT_SEEN_MEANS_UP, we should
> just remove the field seen_in_this_frame.

Well, it is no longer needed, but sure, one could keep it and just
remove the unused fields.

> > @@ -464,12 +438,31 @@ static int mt_input_mapped(struct hid_device *hdev, struct hid_input *hi,
> >         return -1;
> >  }
> >
> > -static int mt_compute_slot(struct mt_device *td)
> > +static void mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
> > +
> > +{
> > +       struct mt_device *td = hid_get_drvdata(hdev);
> > +       struct mt_class *cls = &td->mtclass;
> > +       struct input_dev *input = hi->input;
> > +       unsigned int flags = 0;
> > +
> > +       if (test_bit(INPUT_PROP_POINTER, input->propbit))
> > +               flags |= INPUT_MT_POINTER;
> > +       if (test_bit(INPUT_PROP_DIRECT, input->propbit))
> > +               flags |= INPUT_MT_DIRECT;
> 
> These two tests are really strange: the function input_mt_init_slots
> already sets those bits....
> Maybe we should handle INPUT_PROP_POINTER, INPUT_PROP_DIRECT by
> keeping the flag instead of setting the bits and re-read them to
> finally re-set them...

Ok, I will extend the patchset for hid-multitouch to include such
changes as well.

Thanks,
Henrik

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/