linux-kernel - Re: [PATCH v1] xen/input: add multi-touch support

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]

Message-ID: <75F63113-9756-4A21-930B-CDE24871BCA3@gmail.com>
Date:   Thu, 29 Jun 2017 12:24:42 -0700
From:   Dmitry Torokhov <dmitry.torokhov@...il.com>
To:     Oleksandr Andrushchenko <andr2000@...il.com>
CC:     xen-devel@...ts.xenproject.org, linux-kernel@...r.kernel.org,
        joculator@...il.com, al1img@...il.com, vlad.babchuk@...il.com,
        andrii.anisov@...il.com, olekstysh@...il.com,
        Oleksandr Andrushchenko <oleksandr_andrushchenko@...m.com>
Subject: Re: [PATCH v1] xen/input: add multi-touch support

On June 29, 2017 11:40:30 AM PDT, Oleksandr Andrushchenko <andr2000@...il.com> wrote:
>Hi, Dmitry!
>
>First of all thank you for both the comments and the patch
>
>On 06/29/2017 11:17 AM, Dmitry Torokhov wrote:
>> Hi Oleksandr,
>>
>> On Fri, Jun 23, 2017 at 09:09:55AM +0300, Oleksandr Andrushchenko
>wrote:
>>> +			switch (event->mtouch.event_type) {
>>> +			case XENKBD_MT_EV_DOWN:
>>> +				input_mt_report_slot_state(dev, MT_TOOL_FINGER,
>>> +							   true);
>>> +				input_event(dev, EV_ABS, ABS_MT_POSITION_X,
>>> +					    event->mtouch.u.pos.abs_x);
>>> +				input_event(dev, EV_ABS, ABS_MT_POSITION_Y,
>>> +					    event->mtouch.u.pos.abs_y);
>>> +				input_event(dev, EV_ABS, ABS_X,
>>> +					    event->mtouch.u.pos.abs_x);
>>> +				input_event(dev, EV_ABS, ABS_Y,
>>> +					    event->mtouch.u.pos.abs_y);
>> I was looking at this and realized that this breaks the single touch
>> emulation for MT interface: for ST you are supposed to report the
>oldest
>> contact, here you report data for all of them. Luckily
>> input_mt_report_pointer_emulation() that is called as part of
>> input_mt_sync_frame() reports the correct ABS_X/ABS_Y data and fixes
>> that for you.
>>
>> We should simply remove reporting ABS_X/ABS_Y here and in
>> XENKBD_MT_EV_MOTION as well.
>>
>>> +
>>> +		input_set_capability(mtouch, EV_KEY, BTN_TOUCH);
>>> +		input_set_abs_params(mtouch, ABS_X,
>>> +				     0, width, 0, 0);
>>> +		input_set_abs_params(mtouch, ABS_Y,
>>> +				     0, height, 0, 0);
>>> +		input_set_abs_params(mtouch, ABS_PRESSURE,
>>> +				     0, 255, 0, 0);
>> This is done automatically by input_mt_init_slots() when called with
>> INPUT_MT_DIRECT (as in your case) or INPUT_MT_POINTER, so this can be
>> removed as well.
>Great, I was not actually convinced that ABS is really needed
>to be put here while dealing with MT devices,
>so the above can be removed
>> Does the patch below (on top of yours) work for you?
>Unfortunately I didn't have time to test the patch today, but will try
>to do so tomorrow.
>
>Beside that, do you think that the removals above should go into my
>patch
>and the rest of yours (it looks like needed refactoring to me) should
>go 
>into
>a separate one, not named "MT support fixups", but rather "Xen input
>driver refactoring"? Because part of the changes seems to be MT
>relevant
>and part is pure refactoring.
>If so, do you want me to rework your patch with these changes and add
>on
>top of mine (I will put your signed off) or you will handle it on your
>own?

I was planning on simply folding my changes into your patch and calling it a day, unless your testing would show there is an issue. It wasn't intended to be a separate patch in it's own right, I simply sent it out this way to show what exactly I was changing.


Thanks.

-- 
Dmitry