| 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
| ||
|
Date: Tue, 30 Oct 2012 11:43:28 +0100
From: Benjamin Tissoires <benjamin.tissoires@...il.com>
To: Henrik Rydberg <rydberg@...omail.se>
Cc: Dmitry Torokhov <dmitry.torokhov@...il.com>,
Jiri Kosina <jkosina@...e.cz>,
Stephane Chatty <chatty@...c.fr>, linux-input@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 09/11] HID: hid-multitouch: support for hovering devices
On Mon, Oct 29, 2012 at 11:31 PM, Henrik Rydberg <rydberg@...omail.se> wrote:
> Hi Benjamin,
>
>> Win8 devices supporting hovering must provides InRange HID field.
>> The information that the finger is here but is not touching the surface
>> is sent to the user space through ABS_MT_DISTANCE as required by the
>> multitouch protocol.
>>
>> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@...il.com>
>> ---
>> drivers/hid/hid-multitouch.c | 21 +++++++++++++++++++--
>> 1 file changed, 19 insertions(+), 2 deletions(-)
>
> I suppose the idea here is to send position and distance values even
> when there is no touch, but the code does not seem to do that?
Well, the code does it, but I agree it's not very clear.
The touch_state field is not for win8 with hovering the fact that the
finger is touching the surface, but the fact that a finger is in range
of the device.
This is why tipswitch is filling z instead of touch_state.
I agree, it's not that clear, I'll try to do better in v3.
>
>>
>> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
>> index bd23f19..c0ab1c6 100644
>> --- a/drivers/hid/hid-multitouch.c
>> +++ b/drivers/hid/hid-multitouch.c
>> @@ -55,7 +55,7 @@ MODULE_LICENSE("GPL");
>> #define MT_QUIRK_WIN_8_CERTIFIED (1 << 10)
>>
>> struct mt_slot {
>> - __s32 x, y, cx, cy, p, w, h;
>> + __s32 x, y, cx, cy, z, p, w, h;
>> __s32 contactid; /* the device ContactID assigned to this slot */
>> bool touch_state; /* is the touch valid? */
>> };
>> @@ -394,6 +394,12 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>> case HID_UP_DIGITIZER:
>> switch (usage->hid) {
>> case HID_DG_INRANGE:
>> + if (cls->quirks & MT_QUIRK_WIN_8_CERTIFIED) {
>> + hid_map_usage(hi, usage, bit, max,
>> + EV_ABS, ABS_MT_DISTANCE);
>> + input_set_abs_params(hi->input,
>> + ABS_MT_DISTANCE, 0, 1, 0, 0);
>> + }
>> mt_store_field(usage, td, hi);
>> td->last_field_index = field->index;
>> return 1;
>> @@ -511,6 +517,11 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
>> struct mt_slot *s = &td->curdata;
>>
>> if (td->mtclass.quirks & MT_QUIRK_WIN_8_CERTIFIED &&
>> + !test_bit(ABS_MT_DISTANCE, input->absbit))
>> + /* If InRange is not present, rely on TipSwitch */
>> + s->touch_state = !s->z;
>> +
>> + if (td->mtclass.quirks & MT_QUIRK_WIN_8_CERTIFIED &&
>> !s->touch_state) {
>> struct input_mt_slot *slot = &input->mt->slots[slotnum];
>> int prv_x = input_mt_get_value(slot, ABS_MT_POSITION_X);
>> @@ -543,6 +554,7 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
>> input_event(input, EV_ABS, ABS_MT_TOOL_Y,
>> s->cy);
>> }
>> + input_event(input, EV_ABS, ABS_MT_DISTANCE, s->z);
>
> This only happens if touch_state is true?
mmm, yes, we should move it outside this condition.
>
>> input_event(input, EV_ABS, ABS_MT_ORIENTATION, wide);
>> input_event(input, EV_ABS, ABS_MT_PRESSURE, s->p);
>> input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
>> @@ -575,11 +587,16 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
>> case HID_DG_INRANGE:
>> if (quirks & MT_QUIRK_VALID_IS_INRANGE)
>> td->curvalid = value;
>> + if (quirks & MT_QUIRK_WIN_8_CERTIFIED)
>> + td->curdata.touch_state = value;
>> break;
>> case HID_DG_TIPSWITCH:
>> if (quirks & MT_QUIRK_NOT_SEEN_MEANS_UP)
>> td->curvalid = value;
>> - td->curdata.touch_state = value;
>> + if (quirks & MT_QUIRK_WIN_8_CERTIFIED)
>> + td->curdata.z = !value;
>
> Here, z is a boolean?
this is what is written in Win 8 specification. So ABS_MT_DISTANCE
will have a range of [0..1] and this behavior is right. When we will
have device with real Z hovering measures, then it will be the time to
change this, but currently, we only have a boolean in InRange.
Cheers,
Benjamin
>
>> + else
>> + td->curdata.touch_state = value;
>> break;
>> case HID_DG_CONFIDENCE:
>> if (quirks & MT_QUIRK_VALID_IS_CONFIDENCE)
>> --
>> 1.7.11.7
>>
>
> 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/
Powered by blists - more mailing lists