| 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: Fri, 6 Oct 2017 10:13:39 +0200
From: Benjamin Tissoires <benjamin.tissoires@...hat.com>
To: Jiri Kosina <jikos@...nel.org>,
Wei-Ning Huang <wnhuang@...gle.com>,
Wei-Ning Huang <wnhuang@...omium.org>
Cc: LKML <linux-kernel@...r.kernel.org>,
Linux Input <linux-input@...r.kernel.org>,
Dmitry Torokhov <dtor@...omium.org>
Subject: Re: [PATCH v3] HID: hid-multitouch: support fine-grain orientation
reporting
Hi,
On Oct 05 2017 or thereabouts, Jiri Kosina wrote:
>
> [ adding Benjamin to CC ]
>
> On Thu, 28 Sep 2017, Wei-Ning Huang wrote:
>
> > The current hid-multitouch driver only allow the report of two
> > orientations, vertical and horizontal. We use the Azimuth orientation
> > usage 0x3F under the Digitizer usage page to report orientation if the
> > device supports it.
> >
> > Changelog:
> > v1 -> v2:
> > - Fix commit message.
> > - Remove resolution reporting for ABS_MT_ORIENTATION.
> > v2 -> v3:
> > - Fix commit message.
> >
> > Signed-off-by: Wei-Ning Huang <wnhuang@...gle.com>
> > Reviewed-by: Dmitry Torokhov <dtor@...omium.org>
> > ---
> > drivers/hid/hid-multitouch.c | 35 +++++++++++++++++++++++++++++++++--
> > include/linux/hid.h | 1 +
> > 2 files changed, 34 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> > index 440b999304a5..f9801d1b1eae 100644
> > --- a/drivers/hid/hid-multitouch.c
> > +++ b/drivers/hid/hid-multitouch.c
> > @@ -84,11 +84,12 @@ MODULE_LICENSE("GPL");
> > #define MT_IO_FLAGS_PENDING_SLOTS 2
> >
> > struct mt_slot {
> > - __s32 x, y, cx, cy, p, w, h;
> > + __s32 x, y, cx, cy, p, w, h, a;
> > __s32 contactid; /* the device ContactID assigned to this slot */
> > bool touch_state; /* is the touch valid? */
> > bool inrange_state; /* is the finger in proximity of the sensor? */
> > bool confidence_state; /* is the touch made by a finger? */
> > + bool has_azimuth; /* the contact reports azimuth */
> > };
> >
> > struct mt_class {
> > @@ -591,6 +592,20 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> > td->cc_index = field->index;
> > td->cc_value_index = usage->usage_index;
> > return 1;
> > + case HID_DG_AZIMUTH:
> > + hid_map_usage(hi, usage, bit, max,
> > + EV_ABS, ABS_MT_ORIENTATION);
> > + /*
> > + * Azimuth has the range of [0, MAX) representing a full
> > + * revolution. Set ABS_MT_ORIENTATION to a quarter of
> > + * MAX according the definition of ABS_MT_ORIENTATION
> > + */
> > + input_set_abs_params(hi->input, ABS_MT_ORIENTATION,
> > + 0, field->logical_maximum / 4,
Shouldn't this be "-field->logical_maximum / 4, field->logical_maximum / 4,"?
Also, if HID_DG_HEIGHT is seen after HID_DG_AZIMUTH, it'll change the
ABS_MT_ORIENTATION you just set here.
> > + cls->sn_move ?
> > + field->logical_maximum / cls->sn_move : 0, 0);
> > + mt_store_field(usage, td, hi);
> > + return 1;
> > case HID_DG_CONTACTMAX:
> > /* we don't set td->last_slot_field as contactcount and
> > * contact max are global to the report */
> > @@ -683,6 +698,10 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
> > int wide = (s->w > s->h);
> > int major = max(s->w, s->h);
> > int minor = min(s->w, s->h);
> > + int orientation = wide;
> > +
> > + if (s->has_azimuth)
> > + orientation = s->a;
> >
> > /*
> > * divided by two to match visual scale of touch
> > @@ -699,7 +718,8 @@ 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->touch_state);
> > - input_event(input, EV_ABS, ABS_MT_ORIENTATION, wide);
> > + input_event(input, EV_ABS, ABS_MT_ORIENTATION,
> > + orientation);
> > input_event(input, EV_ABS, ABS_MT_PRESSURE, s->p);
> > input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
> > input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
> > @@ -789,6 +809,17 @@ static void mt_process_mt_event(struct hid_device *hid, struct hid_field *field,
> > break;
> > case HID_DG_CONTACTCOUNT:
> > break;
> > + case HID_DG_AZIMUTH:
> > + /*
> > + * Azimuth is counter-clockwise and ranges from [0, MAX)
> > + * (a full revolution). Convert it to clockwise ranging
> > + * [-MAX/2, MAX/2].
It took me a while to get this right. The problem is the definition of
ABS_MT_ORIENTATION says that we report [-MAX/4, MAX/4], *but* we can go
out of range to [-MAX/2, MAX/2] to report upside down ellipsis.
So this is correct, but I'd like to have some mention of [-MAX/4, MAX/4]
in this comment, to ease further rereading of the code.
Cheers,
Benjamin
> > + */
> > + if (value > field->logical_maximum / 2)
> > + value -= field->logical_maximum;
> > + td->curdata.a = -value;
> > + td->curdata.has_azimuth = true;
> > + break;
> > case HID_DG_TOUCH:
> > /* do nothing */
> > break;
> > diff --git a/include/linux/hid.h b/include/linux/hid.h
> > index ab05a86269dc..d81b9b6fd83a 100644
> > --- a/include/linux/hid.h
> > +++ b/include/linux/hid.h
> > @@ -281,6 +281,7 @@ struct hid_item {
> >
> > #define HID_DG_DEVICECONFIG 0x000d000e
> > #define HID_DG_DEVICESETTINGS 0x000d0023
> > +#define HID_DG_AZIMUTH 0x000d003f
> > #define HID_DG_CONFIDENCE 0x000d0047
> > #define HID_DG_WIDTH 0x000d0048
> > #define HID_DG_HEIGHT 0x000d0049
> > --
> > 2.12.2
> >
>
> --
> Jiri Kosina
> SUSE Labs
>
Powered by blists - more mailing lists