[PATCH 03/10] HID: hid-multitouch: support arrays for the split of the touches in a report

[Benjamin Tissoires]

Win8 certification introduced the ability to transmit two X and two Y per
touch. The specification precises that it must be in an array, with a
report count == 2.

This test guarantees that we split the touches on the last element
in this array.

Signed-off-by: Benjamin Tissoires <benjamin....@enac.fr>
---
drivers/hid/hid-multitouch.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 1382554..e96ecc3 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -577,12 +577,16 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
return 0;
}

- if (usage->hid == td->last_slot_field)
- mt_complete_slot(td, field->hidinput->input);
-
- if (field->index == td->last_field_index
- && td->num_received >= td->num_expected)
- mt_sync_frame(td, field->hidinput->input);
+ if (usage_index + 1 == field->report_count) {
+ /* we only take into account the last report
+ * of a field if report_count > 1 */
+ if (usage->hid == td->last_slot_field)
+ mt_complete_slot(td, field->hidinput->input);
+
+ if (field->index == td->last_field_index
+ && td->num_received >= td->num_expected)
+ mt_sync_frame(td, field->hidinput->input);
+ }

}

--
1.7.11.7

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

[Benjamin Tissoires]

Win8 input specification clarifies the X and Y sent by devices.
It distincts the position where the user wants to Touch (T) from
the center of the ellipsoide (C). This patch enable supports for this
distinction in hid-multitouch.

We recognize Win8 certified devices from their vendor feature 0xff0000c5
where Microsoft put a signed blob in the report to check if the device
passed the certification.

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---
drivers/hid/hid-multitouch.c | 53 ++++++++++++++++++++++++++++++++++++--------
1 file changed, 44 insertions(+), 9 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 38a4feb..ff2354c 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -52,9 +52,10 @@ MODULE_LICENSE("GPL");
#define MT_QUIRK_VALID_IS_CONFIDENCE (1 << 6)
#define MT_QUIRK_SLOT_IS_CONTACTID_MINUS_ONE (1 << 8)
#define MT_QUIRK_NO_AREA (1 << 9)
+#define MT_QUIRK_WIN_8_CERTIFIED (1 << 10)

struct mt_slot {
- __s32 x, y, p, w, h;
+ __s32 x, y, cx, cy, p, w, h;
__s32 contactid; /* the device ContactID assigned to this slot */
bool touch_state; /* is the touch valid? */
};
@@ -292,6 +293,10 @@ static void mt_feature_mapping(struct hid_device *hdev,
td->maxcontacts = td->mtclass.maxcontacts;

break;
+ case 0xff0000c5:
+ if (field->report_count == 256 && field->report_size == 8)
+ td->mtclass.quirks |= MT_QUIRK_WIN_8_CERTIFIED;
+ break;
}
}

@@ -350,18 +355,36 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
case HID_UP_GENDESK:
switch (usage->hid) {
case HID_GD_X:
- hid_map_usage(hi, usage, bit, max,
+ if (cls->quirks & MT_QUIRK_WIN_8_CERTIFIED &&
+ usage_index) {
+ hid_map_usage(hi, usage, bit, max,
+ EV_ABS, ABS_MT_TOOL_X);
+ set_abs(hi->input, ABS_MT_TOOL_X, field,
+ cls->sn_move);
+ } else {
+ hid_map_usage(hi, usage, bit, max,
EV_ABS, ABS_MT_POSITION_X);
- set_abs(hi->input, ABS_MT_POSITION_X, field,
- cls->sn_move);
+ set_abs(hi->input, ABS_MT_POSITION_X, field,
+ cls->sn_move);
+ }
+
mt_store_field(usage, td, hi);
td->last_field_index = field->index;
return 1;
case HID_GD_Y:
- hid_map_usage(hi, usage, bit, max,
+ if (cls->quirks & MT_QUIRK_WIN_8_CERTIFIED &&
+ usage_index) {
+ hid_map_usage(hi, usage, bit, max,
+ EV_ABS, ABS_MT_TOOL_Y);
+ set_abs(hi->input, ABS_MT_TOOL_Y, field,
+ cls->sn_move);
+ } else {
+ hid_map_usage(hi, usage, bit, max,
EV_ABS, ABS_MT_POSITION_Y);
- set_abs(hi->input, ABS_MT_POSITION_Y, field,
- cls->sn_move);
+ set_abs(hi->input, ABS_MT_POSITION_Y, field,
+ cls->sn_move);
+ }
+
mt_store_field(usage, td, hi);
td->last_field_index = field->index;
return 1;
@@ -502,6 +525,12 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)

input_event(input, EV_ABS, ABS_MT_POSITION_X, s->x);
input_event(input, EV_ABS, ABS_MT_POSITION_Y, s->y);
+ if (td->mtclass.quirks & MT_QUIRK_WIN_8_CERTIFIED) {
+ input_event(input, EV_ABS, ABS_MT_TOOL_X,
+ s->cx);
+ input_event(input, EV_ABS, ABS_MT_TOOL_Y,
+ s->cy);
+ }
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);
@@ -553,10 +582,16 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
td->curdata.p = value;
break;
case HID_GD_X:
- td->curdata.x = value;
+ if (usage->code == ABS_MT_POSITION_X)
+ td->curdata.x = value;
+ else
+ td->curdata.cx = value;
break;
case HID_GD_Y:
- td->curdata.y = value;
+ if (usage->code == ABS_MT_POSITION_Y)
+ td->curdata.y = value;
+ else
+ td->curdata.cy = value;
break;
case HID_DG_WIDTH:
td->curdata.w = value;

[Benjamin Tissoires]

Of course, I was in a rush to send the patches, and they don't apply
smoothly on top of Jiri's tree.
Sorry for that, I'll send an update tomorrow.

Cheers,
Benjamin

On Thu, Oct 25, 2012 at 4:09 PM, Benjamin Tissoires
<benjamin....@gmail.com> wrote:
> Hi guys,
>
> well, this is my first series as a Red Hat employee. I should be able to
> communicate my email address soon (once I'm able to read it).
>
> So, this is an update for supporting Win 8 multitouch devices in the kernel.
> As I wanted to reliably forward the resolution, I noticed a bug in the
> processing of the unit exponent in the hid core layer.
> Thus the fixes for hid-core and hid-input.
>
> The last patch removes the dependency for usbhid as now touchscreen may
> be connected through i2c.
>
> Cheers,
> Benjamin

[Benjamin Tissoires]

HID spec details special values for the HID field unit exponent.
Basically, the range [0x8..0xf] correspond to [-8..-1].

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

drivers/hid/hid-core.c | 10 +++++++++-
drivers/hid/hid-input.c | 19 +++++++++++++------
2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 5de3bb3..b2533f1 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -315,6 +315,7 @@ static s32 item_sdata(struct hid_item *item)

static int hid_parser_global(struct hid_parser *parser, struct hid_item *item)
{
+ __u32 raw_value;
switch (item->tag) {
case HID_GLOBAL_ITEM_TAG_PUSH:

@@ -365,7 +366,14 @@ static int hid_parser_global(struct hid_parser *parser, struct hid_item *item)
return 0;

case HID_GLOBAL_ITEM_TAG_UNIT_EXPONENT:
- parser->global.unit_exponent = item_sdata(item);
+ /* Units exponent negative numbers are given through a special
+ * table.
+ * See "6.2.2.7 Global Items" for more information. */
+ raw_value = item_udata(item);
+ if ((raw_value >> 3) == 1)
+ parser->global.unit_exponent = raw_value - 16;
+ else
+ parser->global.unit_exponent = raw_value;
return 0;

case HID_GLOBAL_ITEM_TAG_UNIT:
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 1b0adc3..fc9f2b5 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -215,7 +215,7 @@ __s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code)
field->logical_minimum;
__s32 physical_extents = field->physical_maximum -
field->physical_minimum;
- __s32 prev;
+ __s32 prev, tmp_exponent;

/* Check if the extents are sane */
if (logical_extents <= 0 || physical_extents <= 0)
@@ -235,17 +235,24 @@ __s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code)
case ABS_MT_TOOL_Y:
case ABS_MT_TOUCH_MAJOR:
case ABS_MT_TOUCH_MINOR:
- if (field->unit == 0x11) { /* If centimeters */
+ if (field->unit & 0xffffff00) /* Not a length */
+ return 0;
+ tmp_exponent = field->unit >> 4;
+ if ((tmp_exponent >> 3) == 1)
+ tmp_exponent -= 16;
+ switch (field->unit & 0xf) {
+ case 0x1: /* If centimeters */
/* Convert to millimeters */
- unit_exponent += 1;
- } else if (field->unit == 0x13) { /* If inches */
+ unit_exponent += tmp_exponent;
+ break;
+ case 0x3: /* If inches */
/* Convert to millimeters */
prev = physical_extents;
physical_extents *= 254;
if (physical_extents < prev)
return 0;
- unit_exponent -= 1;
- } else {
+ unit_exponent += tmp_exponent - 2;
+ default:
return 0;
}
break;
--
1.7.11.7

[Benjamin Tissoires]

Win 8 specification is much more precise than the Win 7 one.
Moreover devices that need to take certification must be submitted
to Microsoft.

The result is a better protocol support and we can rely on that to
skip all the messy tests we used to do.

The protocol specify the fact that each valid touch must be reported
within a frame, and that the release touch coordinate must be the
same than the last known touch.
So we can use the always_valid quirk and dismiss reports when we
touch coordiante do not follow this rule.

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

drivers/hid/hid-multitouch.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index c8290e2..6d4ad30 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -510,6 +510,18 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
int slotnum = mt_compute_slot(td, input);
struct mt_slot *s = &td->curdata;

+ 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);
+ int prv_y = input_mt_get_value(slot, ABS_MT_POSITION_Y);
+ /* valid releases occurs when the last x and y positions
+ * are the same as the last known touch. */
+ if (!input_mt_is_active(slot) ||
+ prv_x != s->x || prv_y != s->y)
+ return;
+ }
+
if (slotnum < 0 || slotnum >= td->maxcontacts)
return;

@@ -681,8 +693,8 @@ static void mt_post_parse_default_settings(struct mt_device *td)
{
__s32 quirks = td->mtclass.quirks;

- /* unknown serial device needs special quirks */
- if (td->touches_by_report == 1) {
+ /* unknown serial devices or win8 ones need special quirks */
+ if (td->touches_by_report == 1 || quirks & MT_QUIRK_WIN_8_CERTIFIED) {
quirks |= MT_QUIRK_ALWAYS_VALID;
quirks &= ~MT_QUIRK_NOT_SEEN_MEANS_UP;
quirks &= ~MT_QUIRK_VALID_IS_INRANGE;

[Benjamin Tissoires]

Hi guys,

well, this is my first series as a Red Hat employee. I should be able to
communicate my email address soon (once I'm able to read it).

So, this is an update for supporting Win 8 multitouch devices in the kernel.
As I wanted to reliably forward the resolution, I noticed a bug in the
processing of the unit exponent in the hid core layer.
Thus the fixes for hid-core and hid-input.

The last patch removes the dependency for usbhid as now touchscreen may
be connected through i2c.

Cheers,
Benjamin

[Benjamin Tissoires]

Win8 devices are required to present the feature "Maximum Contact Number".
Fortunately all win7 devices I've seen presents this feature.
If the current value is 0, then, the driver can get the actual supported
contact count by refering to the logical_max.
This win8 specification ensures that logical_max may not be above 250.
This also allows us to detect when devices like irtouch or stantum reports
an obviously wrong value of 255.

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

drivers/hid/hid-multitouch.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index e96ecc3..38a4feb 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -121,6 +121,7 @@ struct mt_device {
#define MT_CLS_GENERALTOUCH_PWT_TENFINGERS 0x0109

#define MT_DEFAULT_MAXCONTACT 10
+#define MT_MAX_MAXCONTACT 250

#define MT_USB_DEVICE(v, p) HID_DEVICE(BUS_USB, HID_GROUP_MULTITOUCH, v, p)
#define MT_BT_DEVICE(v, p) HID_DEVICE(BUS_BLUETOOTH, HID_GROUP_MULTITOUCH, v, p)
@@ -283,6 +284,9 @@ static void mt_feature_mapping(struct hid_device *hdev,
case HID_DG_CONTACTMAX:
td->maxcontact_report_id = field->report->id;
td->maxcontacts = field->value[0];
+ if (!td->maxcontacts &&
+ field->logical_maximum <= MT_MAX_MAXCONTACT)
+ td->maxcontacts = field->logical_maximum;
if (td->mtclass.maxcontacts)
/* check if the maxcontacts is given by the class */

td->maxcontacts = td->mtclass.maxcontacts;

--
1.7.11.7

[Benjamin Tissoires]

Win 8 digitizer devices provides the actual scan time computed by the
hardware itself. The value is global to the frame and is not specific
to the multitouch protocol (though only touch, not pen, should use it
according to the specification).

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

Documentation/input/event-codes.txt | 7 +++++++
drivers/hid/hid-input.c | 4 ++++
drivers/hid/hid-multitouch.c | 13 ++++++++++---
include/linux/hid.h | 1 +
include/linux/input.h | 1 +
5 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/Documentation/input/event-codes.txt b/Documentation/input/event-codes.txt
index 53305bd..8f8c908 100644
--- a/Documentation/input/event-codes.txt
+++ b/Documentation/input/event-codes.txt
@@ -174,6 +174,13 @@ A few EV_ABS codes have special meanings:
the input device may be used freely in three dimensions, consider ABS_Z
instead.

+* ABS_SCAN_TIME:
+ - Used when the device provides a timestamp for each frame. The unit must be
+ 100us, and may be reset when the device don't send any events for a
+ period of time. The values increment at each frame and thus, it can roll
+ back to 0 when reach logical_max. If the device does not provide this
+ information, the driver must not provide it to the user space.
+
* ABS_MT_<name>:
- Used to describe multitouch input events. Please see
multi-touch-protocol.txt for details.
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 16cc89a..5fe7bd3 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -675,6 +675,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
map_key_clear(BTN_STYLUS2);
break;

+ case 0x56: /* Scan Time */
+ map_abs(ABS_SCAN_TIME);
+ break;
+
default: goto unknown;
}
break;
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 5aebbff..5d7fd39 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -447,19 +447,26 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,

mt_store_field(usage, td, hi);
td->last_field_index = field->index;
return 1;

+ case HID_DG_SCANTIME:

+ hid_map_usage(hi, usage, bit, max,

+ EV_ABS, ABS_SCAN_TIME);
+ set_abs(hi->input, ABS_SCAN_TIME, field, 0);
+ td->last_field_index = field->index;
+ return 1;
case HID_DG_CONTACTCOUNT:

td->last_field_index = field->index;
return 1;

case HID_DG_CONTACTMAX:
- /* we don't set td->last_slot_field as contactcount and
- * contact max are global to the report */
+ /* we don't set td->last_slot_field as scan time,
+ * contactcount and contact max are global to the
+ * report */

td->last_field_index = field->index;

return -1;
- }
case HID_DG_TOUCH:
/* Legacy devices use TIPSWITCH and not TOUCH.
* Let's just ignore this field. */
return -1;
+ }
/* let hid-input decide for the others */
return 0;

diff --git a/include/linux/hid.h b/include/linux/hid.h
index 6216529..99a6418 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -279,6 +279,7 @@ struct hid_item {
#define HID_DG_DEVICEINDEX 0x000d0053
#define HID_DG_CONTACTCOUNT 0x000d0054
#define HID_DG_CONTACTMAX 0x000d0055
+#define HID_DG_SCANTIME 0x000d0056

/*
* HID report types --- Ouch! HID spec says 1 2 3!
diff --git a/include/linux/input.h b/include/linux/input.h
index ba48743..73c3a96 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -796,6 +796,7 @@ struct input_keymap_entry {
#define ABS_TILT_X 0x1a
#define ABS_TILT_Y 0x1b
#define ABS_TOOL_WIDTH 0x1c
+#define ABS_SCAN_TIME 0x1d

#define ABS_VOLUME 0x20

--
1.7.11.7

[Benjamin Tissoires]

This patch factorizes the hid set_feature command by using
hid_device->hid_output_raw_report instead of direclty relying on
usbhid. This makes the driver usb independant.

However I still can't remove the 2 usb related headers because the
function mt_resume has a specific patch for usb devices.

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

drivers/hid/hid-multitouch.c | 63 ++++++++++++++++++++++++++------------------
1 file changed, 37 insertions(+), 26 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 5d7fd39..8033a6b 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -670,47 +670,58 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
return 1;
}

-static void mt_set_input_mode(struct hid_device *hdev)
+static void mt_set_feature(struct hid_device *hdev, __s8 feature_id,
+ u8 value, size_t index)
{
- struct mt_device *td = hid_get_drvdata(hdev);
struct hid_report *r;
struct hid_report_enum *re;
+ u8 *data;
+ u8 max_value;
+ int len;
+
+ if (feature_id < 0 || !hdev->hid_output_raw_report)
+ return;
+
+ re = &hdev->report_enum[HID_FEATURE_REPORT];
+ r = re->report_id_hash[feature_id];
+ if (!r)
+ return;
+
+ len = ((r->size - 1) >> 3) + 1 + (r->id > 0);
+ max_value = r->field[0]->logical_maximum;
+ value = min(value, max_value);

- if (td->inputmode < 0)
+ if (r->field[0]->value[index] == value || len < 2 || index + 1 >= len)
return;

- re = &(hdev->report_enum[HID_FEATURE_REPORT]);
- r = re->report_id_hash[td->inputmode];
- if (r) {
- r->field[0]->value[td->inputmode_index] = 0x02;
- usbhid_submit_report(hdev, r, USB_DIR_OUT);
+ data = kmalloc(len, GFP_ATOMIC);
+ if (!data) {
+ hid_warn(hdev, "output queueing failed\n");
+ return;
}
+
+ data[0] = r->id;
+ data[index + 1] = value;
+ hdev->hid_output_raw_report(hdev, data, len, HID_FEATURE_REPORT);
+ kfree(data);
}

-static void mt_set_maxcontacts(struct hid_device *hdev)
+static void mt_set_input_mode(struct hid_device *hdev)
{
struct mt_device *td = hid_get_drvdata(hdev);
- struct hid_report *r;
- struct hid_report_enum *re;
- int fieldmax, max;

- if (td->maxcontact_report_id < 0)
- return;
+ mt_set_feature(hdev, td->inputmode, 0x02, td->inputmode_index);
+}

- if (!td->mtclass.maxcontacts)
+static void mt_set_maxcontacts(struct hid_device *hdev)
+{
+ struct mt_device *td = hid_get_drvdata(hdev);
+ int max = td->mtclass.maxcontacts;
+
+ if (!max)
return;

- re = &hdev->report_enum[HID_FEATURE_REPORT];
- r = re->report_id_hash[td->maxcontact_report_id];
- if (r) {
- max = td->mtclass.maxcontacts;
- fieldmax = r->field[0]->logical_maximum;
- max = min(fieldmax, max);
- if (r->field[0]->value[0] != max) {
- r->field[0]->value[0] = max;
- usbhid_submit_report(hdev, r, USB_DIR_OUT);
- }
- }
+ mt_set_feature(hdev, td->maxcontact_report_id, max, 0);

}

static void mt_post_parse_default_settings(struct mt_device *td)

--
1.7.11.7

[Benjamin Tissoires]

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....@gmail.com>
---

drivers/hid/hid-multitouch.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 6d4ad30..5aebbff 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);

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;
+ else
+ td->curdata.touch_state = value;
break;
case HID_DG_CONFIDENCE:
if (quirks & MT_QUIRK_VALID_IS_CONFIDENCE)
--
1.7.11.7

[Benjamin Tissoires]

Win 8 device specification changed the requirements for the hid usages
of the multitouch devices. Now InRange is optional and must be only
used when the device supports hovering.

This ensures that the quirk ALWAYS_VALID is taken into account and
also ensures its precedence over the other VALID* quirks.

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

drivers/hid/hid-multitouch.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index ff2354c..c8290e2 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -506,7 +506,7 @@ static int mt_compute_slot(struct mt_device *td, struct input_dev *input)
*/

static void mt_complete_slot(struct mt_device *td, struct input_dev *input)

{
- if (td->curvalid) {
+ if (td->curvalid || td->mtclass.quirks & MT_QUIRK_ALWAYS_VALID) {

int slotnum = mt_compute_slot(td, input);

struct mt_slot *s = &td->curdata;

@@ -561,9 +561,7 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
if (hid->claimed & HID_CLAIMED_INPUT) {

switch (usage->hid) {
case HID_DG_INRANGE:

- if (quirks & MT_QUIRK_ALWAYS_VALID)
- td->curvalid = true;
- else if (quirks & MT_QUIRK_VALID_IS_INRANGE)
+ if (quirks & MT_QUIRK_VALID_IS_INRANGE)
td->curvalid = value;
break;
case HID_DG_TIPSWITCH:
--
1.7.11.7

[Benjamin Tissoires]

Currently, there is no way to know the index of the current field
in the .input_mapping and .event callbacks when this field is inside
an array of HID fields.
This patch transfers this index to the input_mapping and event
callbacks.

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

drivers/hid/hid-a4tech.c | 2 +-
drivers/hid/hid-apple.c | 4 ++--
drivers/hid/hid-belkin.c | 2 +-
drivers/hid/hid-cherry.c | 2 +-
drivers/hid/hid-chicony.c | 2 +-
drivers/hid/hid-core.c | 16 +++++++++++-----
drivers/hid/hid-cypress.c | 2 +-
drivers/hid/hid-ezkey.c | 4 ++--
drivers/hid/hid-gyration.c | 4 ++--
drivers/hid/hid-input.c | 10 ++++++----
drivers/hid/hid-kensington.c | 2 +-
drivers/hid/hid-lcpower.c | 2 +-
drivers/hid/hid-lenovo-tpkbd.c | 3 ++-
drivers/hid/hid-lg.c | 4 ++--
drivers/hid/hid-magicmouse.c | 3 ++-
drivers/hid/hid-microsoft.c | 4 ++--
drivers/hid/hid-monterey.c | 2 +-
drivers/hid/hid-multitouch.c | 4 ++--
drivers/hid/hid-ntrig.c | 4 +++-
drivers/hid/hid-petalynx.c | 2 +-
drivers/hid/hid-prodikeys.c | 2 +-
drivers/hid/hid-samsung.c | 2 +-
drivers/hid/hid-speedlink.c | 6 +++---
drivers/hid/hid-sunplus.c | 2 +-
drivers/hid/hid-tivo.c | 2 +-
drivers/hid/hid-topseed.c | 2 +-
drivers/hid/hid-twinhan.c | 2 +-
include/linux/hid.h | 6 ++++--
28 files changed, 58 insertions(+), 44 deletions(-)

diff --git a/drivers/hid/hid-a4tech.c b/drivers/hid/hid-a4tech.c
index 902d1df..b7aa5a2 100644
--- a/drivers/hid/hid-a4tech.c
+++ b/drivers/hid/hid-a4tech.c
@@ -49,7 +49,7 @@ static int a4_input_mapped(struct hid_device *hdev, struct hid_input *hi,
}

static int a4_event(struct hid_device *hdev, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
+ struct hid_usage *usage, unsigned int usage_index, __s32 value)
{
struct a4tech_sc *a4 = hid_get_drvdata(hdev);
struct input_dev *input;
diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index 585344b..cb5eeae 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -248,7 +248,7 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
}

static int apple_event(struct hid_device *hdev, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
+ struct hid_usage *usage, unsigned int usage_index, __s32 value)
{
struct apple_sc *asc = hid_get_drvdata(hdev);

@@ -311,7 +311,7 @@ static void apple_setup_input(struct input_dev *input)

static int apple_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{
if (usage->hid == (HID_UP_CUSTOM | 0x0003)) {
/* The fn key on Apple USB keyboards */
diff --git a/drivers/hid/hid-belkin.c b/drivers/hid/hid-belkin.c
index a1a765a..5a68ad4 100644
--- a/drivers/hid/hid-belkin.c
+++ b/drivers/hid/hid-belkin.c
@@ -29,7 +29,7 @@
EV_KEY, (c))
static int belkin_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{
unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);

diff --git a/drivers/hid/hid-cherry.c b/drivers/hid/hid-cherry.c
index 888ece6..1b26dd0 100644
--- a/drivers/hid/hid-cherry.c
+++ b/drivers/hid/hid-cherry.c
@@ -41,7 +41,7 @@ static __u8 *ch_report_fixup(struct hid_device *hdev, __u8 *rdesc,
EV_KEY, (c))
static int ch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{
if ((usage->hid & HID_USAGE_PAGE) != HID_UP_CONSUMER)
return 0;
diff --git a/drivers/hid/hid-chicony.c b/drivers/hid/hid-chicony.c
index a2abb8e..4510ea5 100644
--- a/drivers/hid/hid-chicony.c
+++ b/drivers/hid/hid-chicony.c
@@ -27,7 +27,7 @@
EV_KEY, (c))
static int ch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{
if ((usage->hid & HID_USAGE_PAGE) != HID_UP_MSVENDOR)
return 0;
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index b2533f1..93c893b 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1009,7 +1009,8 @@ static int hid_match_usage(struct hid_device *hid, struct hid_usage *usage)
}

static void hid_process_event(struct hid_device *hid, struct hid_field *field,
- struct hid_usage *usage, __s32 value, int interrupt)
+ struct hid_usage *usage, unsigned int usage_index, __s32 value,
+ int interrupt)
{
struct hid_driver *hdrv = hid->driver;
int ret;
@@ -1018,7 +1019,7 @@ static void hid_process_event(struct hid_device *hid, struct hid_field *field,
hid_dump_input(hid, usage, value);

if (hdrv && hdrv->event && hid_match_usage(hid, usage)) {
- ret = hdrv->event(hid, field, usage, value);
+ ret = hdrv->event(hid, field, usage, usage_index, value);
if (ret != 0) {
if (ret < 0)
hid_err(hid, "%s's event failed with %d\n",
@@ -1071,19 +1072,24 @@ static void hid_input_field(struct hid_device *hid, struct hid_field *field,
for (n = 0; n < count; n++) {

if (HID_MAIN_ITEM_VARIABLE & field->flags) {
- hid_process_event(hid, field, &field->usage[n], value[n], interrupt);
+ hid_process_event(hid, field, &field->usage[n], n,
+ value[n], interrupt);
continue;
}

if (field->value[n] >= min && field->value[n] <= max
&& field->usage[field->value[n] - min].hid
&& search(value, field->value[n], count))
- hid_process_event(hid, field, &field->usage[field->value[n] - min], 0, interrupt);
+ hid_process_event(hid, field,
+ &field->usage[field->value[n] - min], n,
+ 0, interrupt);

if (value[n] >= min && value[n] <= max
&& field->usage[value[n] - min].hid
&& search(field->value, value[n], count))
- hid_process_event(hid, field, &field->usage[value[n] - min], 1, interrupt);
+ hid_process_event(hid, field,
+ &field->usage[value[n] - min], n,
+ 1, interrupt);
}

memcpy(field->value, value, count * sizeof(__s32));
diff --git a/drivers/hid/hid-cypress.c b/drivers/hid/hid-cypress.c
index 9e43aac..e3c8569 100644
--- a/drivers/hid/hid-cypress.c
+++ b/drivers/hid/hid-cypress.c
@@ -71,7 +71,7 @@ static int cp_input_mapped(struct hid_device *hdev, struct hid_input *hi,
}

static int cp_event(struct hid_device *hdev, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
+ struct hid_usage *usage, unsigned int usage_index, __s32 value)
{
unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);

diff --git a/drivers/hid/hid-ezkey.c b/drivers/hid/hid-ezkey.c
index ca1163e..664f4fd 100644
--- a/drivers/hid/hid-ezkey.c
+++ b/drivers/hid/hid-ezkey.c
@@ -28,7 +28,7 @@

static int ez_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{
if ((usage->hid & HID_USAGE_PAGE) != HID_UP_CONSUMER)
return 0;
@@ -49,7 +49,7 @@ static int ez_input_mapping(struct hid_device *hdev, struct hid_input *hi,
}

static int ez_event(struct hid_device *hdev, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
+ struct hid_usage *usage, unsigned int usage_index, __s32 value)
{
if (!(hdev->claimed & HID_CLAIMED_INPUT) || !field->hidinput ||
!usage->type)
diff --git a/drivers/hid/hid-gyration.c b/drivers/hid/hid-gyration.c
index e88b951..7d27943 100644
--- a/drivers/hid/hid-gyration.c
+++ b/drivers/hid/hid-gyration.c
@@ -27,7 +27,7 @@
EV_KEY, (c))
static int gyration_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{
if ((usage->hid & HID_USAGE_PAGE) != HID_UP_LOGIVENDOR)
return 0;
@@ -56,7 +56,7 @@ static int gyration_input_mapping(struct hid_device *hdev, struct hid_input *hi,
}

static int gyration_event(struct hid_device *hdev, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
+ struct hid_usage *usage, unsigned int usage_index, __s32 value)
{

if (!(hdev->claimed & HID_CLAIMED_INPUT) || !field->hidinput)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index fc9f2b5..16cc89a 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -464,7 +464,8 @@ static void hidinput_cleanup_battery(struct hid_device *dev)
#endif /* CONFIG_HID_BATTERY_STRENGTH */

static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_field *field,
- struct hid_usage *usage)
+ struct hid_usage *usage,
+ unsigned int usage_index)
{
struct input_dev *input = hidinput->input;
struct hid_device *device = input_get_drvdata(input);
@@ -484,7 +485,7 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel

if (device->driver->input_mapping) {
int ret = device->driver->input_mapping(device, hidinput, field,
- usage, &bit, &max);
+ usage, usage_index, &bit, &max);
if (ret > 0)
goto mapped;
if (ret < 0)
@@ -1233,8 +1234,9 @@ int hidinput_connect(struct hid_device *hid, unsigned int force)

for (i = 0; i < report->maxfield; i++)
for (j = 0; j < report->field[i]->maxusage; j++)
- hidinput_configure_usage(hidinput, report->field[i],
- report->field[i]->usage + j);
+ hidinput_configure_usage(hidinput,
+ report->field[i],
+ report->field[i]->usage + j, j);

if (hid->quirks & HID_QUIRK_MULTI_INPUT) {
/* This will leave hidinput NULL, so that it
diff --git a/drivers/hid/hid-kensington.c b/drivers/hid/hid-kensington.c
index a5b4016..af9b9da 100644
--- a/drivers/hid/hid-kensington.c
+++ b/drivers/hid/hid-kensington.c
@@ -22,7 +22,7 @@

static int ks_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{
if ((usage->hid & HID_USAGE_PAGE) != HID_UP_MSVENDOR)
return 0;
diff --git a/drivers/hid/hid-lcpower.c b/drivers/hid/hid-lcpower.c
index c4fe9bd0..d499a0a 100644
--- a/drivers/hid/hid-lcpower.c
+++ b/drivers/hid/hid-lcpower.c
@@ -22,7 +22,7 @@
EV_KEY, (c))
static int ts_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{
if ((usage->hid & HID_USAGE_PAGE) != 0x0ffbc0000)
return 0;
diff --git a/drivers/hid/hid-lenovo-tpkbd.c b/drivers/hid/hid-lenovo-tpkbd.c
index 60c4e1e..86ea802 100644
--- a/drivers/hid/hid-lenovo-tpkbd.c
+++ b/drivers/hid/hid-lenovo-tpkbd.c
@@ -39,7 +39,8 @@ struct tpkbd_data_pointer {

static int tpkbd_input_mapping(struct hid_device *hdev,
struct hid_input *hi, struct hid_field *field,
- struct hid_usage *usage, unsigned long **bit, int *max)
+ struct hid_usage *usage, unsigned int usage_index,
+ unsigned long **bit, int *max)
{
struct usbhid_device *uhdev;

diff --git a/drivers/hid/hid-lg.c b/drivers/hid/hid-lg.c
index fc37ed6..cdcd4d5 100644
--- a/drivers/hid/hid-lg.c
+++ b/drivers/hid/hid-lg.c
@@ -264,7 +264,7 @@ static int lg_wireless_mapping(struct hid_input *hi, struct hid_usage *usage,

static int lg_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{
/* extended mapping for certain Logitech hardware (Logitech cordless
desktop LX500) */
@@ -333,7 +333,7 @@ static int lg_input_mapped(struct hid_device *hdev, struct hid_input *hi,
}

static int lg_event(struct hid_device *hdev, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
+ struct hid_usage *usage, unsigned int usage_index, __s32 value)
{
struct lg_drv_data *drv_data = (struct lg_drv_data *)hid_get_drvdata(hdev);

diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 25ddf3e..aca56ee 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -447,7 +447,8 @@ static int magicmouse_setup_input(struct input_dev *input, struct hid_device *hd

static int magicmouse_input_mapping(struct hid_device *hdev,
struct hid_input *hi, struct hid_field *field,
- struct hid_usage *usage, unsigned long **bit, int *max)
+ struct hid_usage *usage, unsigned int usage_index,
+ unsigned long **bit, int *max)
{
struct magicmouse_sc *msc = hid_get_drvdata(hdev);

diff --git a/drivers/hid/hid-microsoft.c b/drivers/hid/hid-microsoft.c
index e5c699b..62304e1 100644
--- a/drivers/hid/hid-microsoft.c
+++ b/drivers/hid/hid-microsoft.c
@@ -90,7 +90,7 @@ static int ms_presenter_8k_quirk(struct hid_input *hi, struct hid_usage *usage,

static int ms_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{
unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);

@@ -123,7 +123,7 @@ static int ms_input_mapped(struct hid_device *hdev, struct hid_input *hi,
}

static int ms_event(struct hid_device *hdev, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
+ struct hid_usage *usage, unsigned int usage_index, __s32 value)
{
unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);

diff --git a/drivers/hid/hid-monterey.c b/drivers/hid/hid-monterey.c
index dedf757..513cd1b 100644
--- a/drivers/hid/hid-monterey.c
+++ b/drivers/hid/hid-monterey.c
@@ -36,7 +36,7 @@ static __u8 *mr_report_fixup(struct hid_device *hdev, __u8 *rdesc,
EV_KEY, (c))
static int mr_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{
if ((usage->hid & HID_USAGE_PAGE) != HID_UP_CONSUMER)
return 0;
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 9a0804b..1382554 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -314,7 +314,7 @@ static void mt_store_field(struct hid_usage *usage, struct mt_device *td,

static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,

struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{

struct mt_device *td = hid_get_drvdata(hdev);

struct mt_class *cls = &td->mtclass;
@@ -520,7 +520,7 @@ static void mt_sync_frame(struct mt_device *td, struct input_dev *input)

}

static int mt_event(struct hid_device *hid, struct hid_field *field,

- struct hid_usage *usage, __s32 value)
+ struct hid_usage *usage, unsigned int usage_index, __s32 value)
{
struct mt_device *td = hid_get_drvdata(hid);

__s32 quirks = td->mtclass.quirks;

diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
index 9fae2eb..b0b66606 100644
--- a/drivers/hid/hid-ntrig.c
+++ b/drivers/hid/hid-ntrig.c
@@ -456,6 +456,7 @@ static struct attribute_group ntrig_attribute_group = {

static int ntrig_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
+ unsigned int usage_index,
unsigned long **bit, int *max)
{
struct ntrig_data *nd = hid_get_drvdata(hdev);
@@ -567,7 +568,8 @@ static int ntrig_input_mapped(struct hid_device *hdev, struct hid_input *hi,
* and call input_mt_sync after each point if necessary
*/
static int ntrig_event (struct hid_device *hid, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
+ struct hid_usage *usage, unsigned int usage_index,
+ __s32 value)
{
struct ntrig_data *nd = hid_get_drvdata(hid);
struct input_dev *input;
diff --git a/drivers/hid/hid-petalynx.c b/drivers/hid/hid-petalynx.c
index f1ea3ff..30aacb8 100644
--- a/drivers/hid/hid-petalynx.c
+++ b/drivers/hid/hid-petalynx.c
@@ -40,7 +40,7 @@ static __u8 *pl_report_fixup(struct hid_device *hdev, __u8 *rdesc,
EV_KEY, (c))
static int pl_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{
if ((usage->hid & HID_USAGE_PAGE) == HID_UP_LOGIVENDOR) {
switch (usage->hid & HID_USAGE) {
diff --git a/drivers/hid/hid-prodikeys.c b/drivers/hid/hid-prodikeys.c
index b71b77a..2faf83a 100644
--- a/drivers/hid/hid-prodikeys.c
+++ b/drivers/hid/hid-prodikeys.c
@@ -757,7 +757,7 @@ static __u8 *pk_report_fixup(struct hid_device *hdev, __u8 *rdesc,

static int pk_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{
struct pk_device *pk = (struct pk_device *)hid_get_drvdata(hdev);
struct pcmidi_snd *pm;
diff --git a/drivers/hid/hid-samsung.c b/drivers/hid/hid-samsung.c
index 3c1fd8a..d0dd311 100644
--- a/drivers/hid/hid-samsung.c
+++ b/drivers/hid/hid-samsung.c
@@ -141,7 +141,7 @@ static __u8 *samsung_report_fixup(struct hid_device *hdev, __u8 *rdesc,

static int samsung_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{
int ret = 0;

diff --git a/drivers/hid/hid-speedlink.c b/drivers/hid/hid-speedlink.c
index 6020137..da50735 100644
--- a/drivers/hid/hid-speedlink.c
+++ b/drivers/hid/hid-speedlink.c
@@ -27,8 +27,8 @@ static const struct hid_device_id speedlink_devices[] = {
};

static int speedlink_input_mapping(struct hid_device *hdev,
- struct hid_input *hi,
- struct hid_field *field, struct hid_usage *usage,
+ struct hid_input *hi, struct hid_field *field,
+ struct hid_usage *usage, unsigned int usage_index,
unsigned long **bit, int *max)
{
/*
@@ -45,7 +45,7 @@ static int speedlink_input_mapping(struct hid_device *hdev,
}

static int speedlink_event(struct hid_device *hdev, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
+ struct hid_usage *usage, unsigned int usage_index, __s32 value)
{
/* No other conditions due to usage_table. */
/* Fix "jumpy" cursor (invalid events sent by device). */
diff --git a/drivers/hid/hid-sunplus.c b/drivers/hid/hid-sunplus.c
index d484a00..4e62a26 100644
--- a/drivers/hid/hid-sunplus.c
+++ b/drivers/hid/hid-sunplus.c
@@ -38,7 +38,7 @@ static __u8 *sp_report_fixup(struct hid_device *hdev, __u8 *rdesc,
EV_KEY, (c))
static int sp_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{
if ((usage->hid & HID_USAGE_PAGE) != HID_UP_CONSUMER)
return 0;
diff --git a/drivers/hid/hid-tivo.c b/drivers/hid/hid-tivo.c
index 9f85f82..e7684fd 100644
--- a/drivers/hid/hid-tivo.c
+++ b/drivers/hid/hid-tivo.c
@@ -24,7 +24,7 @@

static int tivo_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{
switch (usage->hid & HID_USAGE_PAGE) {
case HID_UP_TIVOVENDOR:
diff --git a/drivers/hid/hid-topseed.c b/drivers/hid/hid-topseed.c
index 613ff7b..e924b7d 100644
--- a/drivers/hid/hid-topseed.c
+++ b/drivers/hid/hid-topseed.c
@@ -28,7 +28,7 @@
EV_KEY, (c))
static int ts_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{
if ((usage->hid & HID_USAGE_PAGE) != HID_UP_LOGIVENDOR)
return 0;
diff --git a/drivers/hid/hid-twinhan.c b/drivers/hid/hid-twinhan.c
index f23456b..b38e6b3 100644
--- a/drivers/hid/hid-twinhan.c
+++ b/drivers/hid/hid-twinhan.c
@@ -62,7 +62,7 @@
EV_KEY, (c))
static int twinhan_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{
if ((usage->hid & HID_USAGE_PAGE) != HID_UP_KEYBOARD)

return 0;
diff --git a/include/linux/hid.h b/include/linux/hid.h

index 9edb06c..6216529 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -660,14 +660,16 @@ struct hid_driver {
u8 *data, int size);
const struct hid_usage_id *usage_table;
int (*event)(struct hid_device *hdev, struct hid_field *field,
- struct hid_usage *usage, __s32 value);
+ struct hid_usage *usage, unsigned int usage_index,
+ __s32 value);

__u8 *(*report_fixup)(struct hid_device *hdev, __u8 *buf,
unsigned int *size);

int (*input_mapping)(struct hid_device *hdev,
struct hid_input *hidinput, struct hid_field *field,
- struct hid_usage *usage, unsigned long **bit, int *max);
+ struct hid_usage *usage, unsigned int usage_index,
+ unsigned long **bit, int *max);
int (*input_mapped)(struct hid_device *hdev,
struct hid_input *hidinput, struct hid_field *field,
struct hid_usage *usage, unsigned long **bit, int *max);
--
1.7.11.7

[Benjamin Tissoires]

Win 8 device specification changed the requirements for the hid usages
of the multitouch devices. Now InRange is optional and must be only
used when the device supports hovering.

This ensures that the quirk ALWAYS_VALID is taken into account and
also ensures its precedence over the other VALID* quirks.

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

drivers/hid/hid-multitouch.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 000c979..43bd8e8 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c

@@ -506,7 +506,7 @@ static int mt_compute_slot(struct mt_device *td, struct input_dev *input)
*/
static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
{
- if (td->curvalid) {
+ if (td->curvalid || td->mtclass.quirks & MT_QUIRK_ALWAYS_VALID) {
int slotnum = mt_compute_slot(td, input);
struct mt_slot *s = &td->curdata;

@@ -561,9 +561,7 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
if (hid->claimed & HID_CLAIMED_INPUT) {
switch (usage->hid) {
case HID_DG_INRANGE:
- if (quirks & MT_QUIRK_ALWAYS_VALID)
- td->curvalid = true;
- else if (quirks & MT_QUIRK_VALID_IS_INRANGE)
+ if (quirks & MT_QUIRK_VALID_IS_INRANGE)
td->curvalid = value;
break;
case HID_DG_TIPSWITCH:

[Benjamin Tissoires]

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....@gmail.com>
---

drivers/hid/hid-multitouch.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)

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);
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;
+ else
+ td->curdata.touch_state = value;
break;
case HID_DG_CONFIDENCE:
if (quirks & MT_QUIRK_VALID_IS_CONFIDENCE)

[Benjamin Tissoires]

This patch factorizes the hid set_feature command by using
hid_device->hid_output_raw_report instead of direclty relying on
usbhid. This makes the driver usb independant.

However I still can't remove the 2 usb related headers because the
function mt_resume has a specific patch for usb devices.

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

drivers/hid/hid-multitouch.c | 63 ++++++++++++++++++++++++++------------------
1 file changed, 37 insertions(+), 26 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 21a120b..33038c5 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c

+ data = kzalloc(len, GFP_ATOMIC);

+ if (!data) {
+ hid_warn(hdev, "output queueing failed\n");
+ return;
}
+
+ data[0] = r->id;
+ data[index + 1] = value;
+ hdev->hid_output_raw_report(hdev, data, len, HID_FEATURE_REPORT);
+ kfree(data);
}

-static void mt_set_maxcontacts(struct hid_device *hdev)
+static void mt_set_input_mode(struct hid_device *hdev)
{

struct mt_device *td = hid_get_drvdata(hdev);

[Benjamin Tissoires]

Win 8 specification is much more precise than the Win 7 one.
Moreover devices that need to take certification must be submitted
to Microsoft.

The result is a better protocol support and we can rely on that to
skip all the messy tests we used to do.

The protocol specify the fact that each valid touch must be reported
within a frame, and that the release touch coordinate must be the
same than the last known touch.
So we can use the always_valid quirk and dismiss reports when we
touch coordiante do not follow this rule.

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

drivers/hid/hid-multitouch.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 43bd8e8..bd23f19 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -510,6 +510,18 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)

int slotnum = mt_compute_slot(td, input);

struct mt_slot *s = &td->curdata;

+ 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);
+ int prv_y = input_mt_get_value(slot, ABS_MT_POSITION_Y);
+ /* valid releases occurs when the last x and y positions
+ * are the same as the last known touch. */
+ if (!input_mt_is_active(slot) ||
+ prv_x != s->x || prv_y != s->y)
+ return;
+ }
+
if (slotnum < 0 || slotnum >= td->maxcontacts)
return;

@@ -681,8 +693,8 @@ static void mt_post_parse_default_settings(struct mt_device *td)
{

__s32 quirks = td->mtclass.quirks;

- /* unknown serial device needs special quirks */
- if (td->touches_by_report == 1) {
+ /* unknown serial devices or win8 ones need special quirks */
+ if (td->touches_by_report == 1 || quirks & MT_QUIRK_WIN_8_CERTIFIED) {
quirks |= MT_QUIRK_ALWAYS_VALID;
quirks &= ~MT_QUIRK_NOT_SEEN_MEANS_UP;
quirks &= ~MT_QUIRK_VALID_IS_INRANGE;

[Benjamin Tissoires]

HID spec details special values for the HID field unit exponent.
Basically, the range [0x8..0xf] correspond to [-8..-1].

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

drivers/hid/hid-core.c | 10 +++++++++-
drivers/hid/hid-input.c | 19 +++++++++++++------
2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 9904776..6bde6e4 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c

@@ -315,6 +315,7 @@ static s32 item_sdata(struct hid_item *item)

static int hid_parser_global(struct hid_parser *parser, struct hid_item *item)
{
+ __u32 raw_value;
switch (item->tag) {
case HID_GLOBAL_ITEM_TAG_PUSH:

@@ -365,7 +366,14 @@ static int hid_parser_global(struct hid_parser *parser, struct hid_item *item)
return 0;

case HID_GLOBAL_ITEM_TAG_UNIT_EXPONENT:
- parser->global.unit_exponent = item_sdata(item);
+ /* Units exponent negative numbers are given through a special
+ * table.
+ * See "6.2.2.7 Global Items" for more information. */
+ raw_value = item_udata(item);
+ if ((raw_value >> 3) == 1)
+ parser->global.unit_exponent = raw_value - 16;
+ else
+ parser->global.unit_exponent = raw_value;
return 0;

case HID_GLOBAL_ITEM_TAG_UNIT:

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 1b0adc3..fc9f2b5 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c

[Benjamin Tissoires]

Currently, there is no way to know the index of the current field
in the .input_mapping and .event callbacks when this field is inside
an array of HID fields.

This patch forwards this index to the input_mapping and event
callbacks.

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

drivers/hid/hid-a4tech.c | 2 +-
drivers/hid/hid-apple.c | 4 ++--
drivers/hid/hid-belkin.c | 2 +-
drivers/hid/hid-cherry.c | 2 +-
drivers/hid/hid-chicony.c | 2 +-
drivers/hid/hid-core.c | 16 +++++++++++-----
drivers/hid/hid-cypress.c | 2 +-
drivers/hid/hid-ezkey.c | 4 ++--
drivers/hid/hid-gyration.c | 4 ++--
drivers/hid/hid-input.c | 10 ++++++----
drivers/hid/hid-kensington.c | 2 +-
drivers/hid/hid-lcpower.c | 2 +-
drivers/hid/hid-lenovo-tpkbd.c | 3 ++-
drivers/hid/hid-lg.c | 4 ++--
drivers/hid/hid-magicmouse.c | 3 ++-
drivers/hid/hid-microsoft.c | 4 ++--
drivers/hid/hid-monterey.c | 2 +-
drivers/hid/hid-multitouch.c | 4 ++--
drivers/hid/hid-ntrig.c | 4 +++-
drivers/hid/hid-petalynx.c | 2 +-
drivers/hid/hid-prodikeys.c | 2 +-

drivers/hid/hid-ps3remote.c | 3 ++-

drivers/hid/hid-samsung.c | 2 +-
drivers/hid/hid-speedlink.c | 6 +++---
drivers/hid/hid-sunplus.c | 2 +-
drivers/hid/hid-tivo.c | 2 +-
drivers/hid/hid-topseed.c | 2 +-
drivers/hid/hid-twinhan.c | 2 +-

drivers/hid/hid-zydacron.c | 2 +-
include/linux/hid.h | 6 ++++--
30 files changed, 61 insertions(+), 46 deletions(-)

diff --git a/drivers/hid/hid-a4tech.c b/drivers/hid/hid-a4tech.c
index 0a23988..62e2aa5 100644
--- a/drivers/hid/hid-a4tech.c
+++ b/drivers/hid/hid-a4tech.c
@@ -48,7 +48,7 @@ static int a4_input_mapped(struct hid_device *hdev, struct hid_input *hi,

}

static int a4_event(struct hid_device *hdev, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
+ struct hid_usage *usage, unsigned int usage_index, __s32 value)
{
struct a4tech_sc *a4 = hid_get_drvdata(hdev);
struct input_dev *input;
diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c

index 06ebdbb..01e8592 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -247,7 +247,7 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,

}

static int apple_event(struct hid_device *hdev, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
+ struct hid_usage *usage, unsigned int usage_index, __s32 value)
{
struct apple_sc *asc = hid_get_drvdata(hdev);

@@ -310,7 +310,7 @@ static void apple_setup_input(struct input_dev *input)

static int apple_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{
if (usage->hid == (HID_UP_CUSTOM | 0x0003)) {
/* The fn key on Apple USB keyboards */
diff --git a/drivers/hid/hid-belkin.c b/drivers/hid/hid-belkin.c

index a1a5a12..6999a64 100644
--- a/drivers/hid/hid-belkin.c
+++ b/drivers/hid/hid-belkin.c

@@ -28,7 +28,7 @@
EV_KEY, (c))

static int belkin_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{
unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);

diff --git a/drivers/hid/hid-cherry.c b/drivers/hid/hid-cherry.c

index af034d3..1feec49 100644
--- a/drivers/hid/hid-cherry.c
+++ b/drivers/hid/hid-cherry.c
@@ -40,7 +40,7 @@ static __u8 *ch_report_fixup(struct hid_device *hdev, __u8 *rdesc,
EV_KEY, (c))
static int ch_input_mapping(struct hid_device *hdev, struct hid_input *hi,

struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)

+ unsigned int usage_index, unsigned long **bit, int *max)
{
if ((usage->hid & HID_USAGE_PAGE) != HID_UP_CONSUMER)
return 0;
diff --git a/drivers/hid/hid-chicony.c b/drivers/hid/hid-chicony.c
index a2abb8e..4510ea5 100644
--- a/drivers/hid/hid-chicony.c
+++ b/drivers/hid/hid-chicony.c
@@ -27,7 +27,7 @@
EV_KEY, (c))

static int ch_input_mapping(struct hid_device *hdev, struct hid_input *hi,

struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)

+ unsigned int usage_index, unsigned long **bit, int *max)
{
if ((usage->hid & HID_USAGE_PAGE) != HID_UP_MSVENDOR)
return 0;

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 6bde6e4..f1720a0 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c

index 3e159a5..453548a 100644
--- a/drivers/hid/hid-cypress.c
+++ b/drivers/hid/hid-cypress.c
@@ -70,7 +70,7 @@ static int cp_input_mapped(struct hid_device *hdev, struct hid_input *hi,

}

static int cp_event(struct hid_device *hdev, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
+ struct hid_usage *usage, unsigned int usage_index, __s32 value)
{
unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);

diff --git a/drivers/hid/hid-ezkey.c b/drivers/hid/hid-ezkey.c

index 6540af2..3bd9675 100644
--- a/drivers/hid/hid-ezkey.c
+++ b/drivers/hid/hid-ezkey.c
@@ -27,7 +27,7 @@

static int ez_input_mapping(struct hid_device *hdev, struct hid_input *hi,

struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)

+ unsigned int usage_index, unsigned long **bit, int *max)
{
if ((usage->hid & HID_USAGE_PAGE) != HID_UP_CONSUMER)
return 0;

@@ -48,7 +48,7 @@ static int ez_input_mapping(struct hid_device *hdev, struct hid_input *hi,

}

static int ez_event(struct hid_device *hdev, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
+ struct hid_usage *usage, unsigned int usage_index, __s32 value)
{
if (!(hdev->claimed & HID_CLAIMED_INPUT) || !field->hidinput ||
!usage->type)
diff --git a/drivers/hid/hid-gyration.c b/drivers/hid/hid-gyration.c

index 4442c30..255e5dd 100644
--- a/drivers/hid/hid-gyration.c
+++ b/drivers/hid/hid-gyration.c
@@ -26,7 +26,7 @@
EV_KEY, (c))
static int gyration_input_mapping(struct hid_device *hdev, struct hid_input *hi,

struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)

+ unsigned int usage_index, unsigned long **bit, int *max)
{
if ((usage->hid & HID_USAGE_PAGE) != HID_UP_LOGIVENDOR)
return 0;

@@ -55,7 +55,7 @@ static int gyration_input_mapping(struct hid_device *hdev, struct hid_input *hi,

}

static int gyration_event(struct hid_device *hdev, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
+ struct hid_usage *usage, unsigned int usage_index, __s32 value)
{

if (!(hdev->claimed & HID_CLAIMED_INPUT) || !field->hidinput)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index fc9f2b5..16cc89a 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c

static int ks_input_mapping(struct hid_device *hdev, struct hid_input *hi,

struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)

+ unsigned int usage_index, unsigned long **bit, int *max)
{
if ((usage->hid & HID_USAGE_PAGE) != HID_UP_MSVENDOR)
return 0;
diff --git a/drivers/hid/hid-lcpower.c b/drivers/hid/hid-lcpower.c

index 22bc14a..5cfbef8 100644

--- a/drivers/hid/hid-lcpower.c
+++ b/drivers/hid/hid-lcpower.c
@@ -22,7 +22,7 @@
EV_KEY, (c))

static int ts_input_mapping(struct hid_device *hdev, struct hid_input *hi,

struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)

+ unsigned int usage_index, unsigned long **bit, int *max)
{
if ((usage->hid & HID_USAGE_PAGE) != HID_UP_LOGIVENDOR)
return 0;

diff --git a/drivers/hid/hid-lenovo-tpkbd.c b/drivers/hid/hid-lenovo-tpkbd.c
index cea016e..7c12fd4 100644

--- a/drivers/hid/hid-lenovo-tpkbd.c
+++ b/drivers/hid/hid-lenovo-tpkbd.c
@@ -39,7 +39,8 @@ struct tpkbd_data_pointer {

static int tpkbd_input_mapping(struct hid_device *hdev,
struct hid_input *hi, struct hid_field *field,
- struct hid_usage *usage, unsigned long **bit, int *max)
+ struct hid_usage *usage, unsigned int usage_index,
+ unsigned long **bit, int *max)
{
struct usbhid_device *uhdev;

diff --git a/drivers/hid/hid-lg.c b/drivers/hid/hid-lg.c

index a2f8e88..0c5acc6 100644
--- a/drivers/hid/hid-lg.c
+++ b/drivers/hid/hid-lg.c
@@ -263,7 +263,7 @@ static int lg_wireless_mapping(struct hid_input *hi, struct hid_usage *usage,

static int lg_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{
/* extended mapping for certain Logitech hardware (Logitech cordless
desktop LX500) */

@@ -332,7 +332,7 @@ static int lg_input_mapped(struct hid_device *hdev, struct hid_input *hi,

}

static int lg_event(struct hid_device *hdev, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
+ struct hid_usage *usage, unsigned int usage_index, __s32 value)
{

struct lg_drv_data *drv_data = hid_get_drvdata(hdev);

diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 25ddf3e..aca56ee 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -447,7 +447,8 @@ static int magicmouse_setup_input(struct input_dev *input, struct hid_device *hd

static int magicmouse_input_mapping(struct hid_device *hdev,
struct hid_input *hi, struct hid_field *field,
- struct hid_usage *usage, unsigned long **bit, int *max)
+ struct hid_usage *usage, unsigned int usage_index,
+ unsigned long **bit, int *max)
{
struct magicmouse_sc *msc = hid_get_drvdata(hdev);

diff --git a/drivers/hid/hid-microsoft.c b/drivers/hid/hid-microsoft.c

index 3acdcfc..2dfe9fa 100644
--- a/drivers/hid/hid-microsoft.c
+++ b/drivers/hid/hid-microsoft.c
@@ -89,7 +89,7 @@ static int ms_presenter_8k_quirk(struct hid_input *hi, struct hid_usage *usage,

static int ms_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{
unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);

@@ -122,7 +122,7 @@ static int ms_input_mapped(struct hid_device *hdev, struct hid_input *hi,

}

static int ms_event(struct hid_device *hdev, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
+ struct hid_usage *usage, unsigned int usage_index, __s32 value)
{
unsigned long quirks = (unsigned long)hid_get_drvdata(hdev);

diff --git a/drivers/hid/hid-monterey.c b/drivers/hid/hid-monterey.c

index cd3643e..cbcbcc7 100644
--- a/drivers/hid/hid-monterey.c
+++ b/drivers/hid/hid-monterey.c
@@ -35,7 +35,7 @@ static __u8 *mr_report_fixup(struct hid_device *hdev, __u8 *rdesc,
EV_KEY, (c))
static int mr_input_mapping(struct hid_device *hdev, struct hid_input *hi,

struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)

+ unsigned int usage_index, unsigned long **bit, int *max)
{
if ((usage->hid & HID_USAGE_PAGE) != HID_UP_CONSUMER)
return 0;

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 375a38d..725d155 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c

@@ -314,7 +314,7 @@ static void mt_store_field(struct hid_usage *usage, struct mt_device *td,

static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,

struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{

struct mt_device *td = hid_get_drvdata(hdev);

struct mt_class *cls = &td->mtclass;

@@ -520,7 +520,7 @@ static void mt_sync_frame(struct mt_device *td, struct input_dev *input)

}

static int mt_event(struct hid_device *hid, struct hid_field *field,

- struct hid_usage *usage, __s32 value)
+ struct hid_usage *usage, unsigned int usage_index, __s32 value)
{
struct mt_device *td = hid_get_drvdata(hid);

__s32 quirks = td->mtclass.quirks;

diff --git a/drivers/hid/hid-ntrig.c b/drivers/hid/hid-ntrig.c
index 86a969f..14794b9 100644

--- a/drivers/hid/hid-ntrig.c
+++ b/drivers/hid/hid-ntrig.c
@@ -456,6 +456,7 @@ static struct attribute_group ntrig_attribute_group = {

static int ntrig_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
+ unsigned int usage_index,
unsigned long **bit, int *max)
{
struct ntrig_data *nd = hid_get_drvdata(hdev);
@@ -567,7 +568,8 @@ static int ntrig_input_mapped(struct hid_device *hdev, struct hid_input *hi,
* and call input_mt_sync after each point if necessary
*/
static int ntrig_event (struct hid_device *hid, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
+ struct hid_usage *usage, unsigned int usage_index,
+ __s32 value)
{
struct ntrig_data *nd = hid_get_drvdata(hid);
struct input_dev *input;
diff --git a/drivers/hid/hid-petalynx.c b/drivers/hid/hid-petalynx.c

index 4c521de..590db7d 100644
--- a/drivers/hid/hid-petalynx.c
+++ b/drivers/hid/hid-petalynx.c
@@ -39,7 +39,7 @@ static __u8 *pl_report_fixup(struct hid_device *hdev, __u8 *rdesc,
EV_KEY, (c))
static int pl_input_mapping(struct hid_device *hdev, struct hid_input *hi,

struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)

+ unsigned int usage_index, unsigned long **bit, int *max)
{
if ((usage->hid & HID_USAGE_PAGE) == HID_UP_LOGIVENDOR) {
switch (usage->hid & HID_USAGE) {
diff --git a/drivers/hid/hid-prodikeys.c b/drivers/hid/hid-prodikeys.c

index ec8ca33..b687f4e 100644

--- a/drivers/hid/hid-prodikeys.c
+++ b/drivers/hid/hid-prodikeys.c
@@ -757,7 +757,7 @@ static __u8 *pk_report_fixup(struct hid_device *hdev, __u8 *rdesc,

static int pk_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{

struct pk_device *pk = hid_get_drvdata(hdev);
struct pcmidi_snd *pm;
diff --git a/drivers/hid/hid-ps3remote.c b/drivers/hid/hid-ps3remote.c
index 03811e5..24d3196 100644
--- a/drivers/hid/hid-ps3remote.c
+++ b/drivers/hid/hid-ps3remote.c
@@ -151,7 +151,8 @@ static __u8 *ps3remote_fixup(struct hid_device *hdev, __u8 *rdesc,

static int ps3remote_mapping(struct hid_device *hdev, struct hid_input *hi,

struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit,

+ int *max)
{
unsigned int key = usage->hid & HID_USAGE;

diff --git a/drivers/hid/hid-samsung.c b/drivers/hid/hid-samsung.c
index a5821d3..b88a27a 100644
--- a/drivers/hid/hid-samsung.c
+++ b/drivers/hid/hid-samsung.c
@@ -140,7 +140,7 @@ static __u8 *samsung_report_fixup(struct hid_device *hdev, __u8 *rdesc,

static int samsung_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{
int ret = 0;

diff --git a/drivers/hid/hid-speedlink.c b/drivers/hid/hid-speedlink.c
index 6020137..da50735 100644
--- a/drivers/hid/hid-speedlink.c
+++ b/drivers/hid/hid-speedlink.c
@@ -27,8 +27,8 @@ static const struct hid_device_id speedlink_devices[] = {
};

static int speedlink_input_mapping(struct hid_device *hdev,
- struct hid_input *hi,
- struct hid_field *field, struct hid_usage *usage,
+ struct hid_input *hi, struct hid_field *field,
+ struct hid_usage *usage, unsigned int usage_index,
unsigned long **bit, int *max)
{
/*
@@ -45,7 +45,7 @@ static int speedlink_input_mapping(struct hid_device *hdev,
}

static int speedlink_event(struct hid_device *hdev, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
+ struct hid_usage *usage, unsigned int usage_index, __s32 value)
{
/* No other conditions due to usage_table. */
/* Fix "jumpy" cursor (invalid events sent by device). */
diff --git a/drivers/hid/hid-sunplus.c b/drivers/hid/hid-sunplus.c

index 45b4b06..9d36bc0 100644
--- a/drivers/hid/hid-sunplus.c
+++ b/drivers/hid/hid-sunplus.c
@@ -37,7 +37,7 @@ static __u8 *sp_report_fixup(struct hid_device *hdev, __u8 *rdesc,
EV_KEY, (c))
static int sp_input_mapping(struct hid_device *hdev, struct hid_input *hi,

struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)

+ unsigned int usage_index, unsigned long **bit, int *max)
{
if ((usage->hid & HID_USAGE_PAGE) != HID_UP_CONSUMER)
return 0;
diff --git a/drivers/hid/hid-tivo.c b/drivers/hid/hid-tivo.c
index 9f85f82..e7684fd 100644
--- a/drivers/hid/hid-tivo.c
+++ b/drivers/hid/hid-tivo.c
@@ -24,7 +24,7 @@

static int tivo_input_mapping(struct hid_device *hdev, struct hid_input *hi,
struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{
switch (usage->hid & HID_USAGE_PAGE) {
case HID_UP_TIVOVENDOR:
diff --git a/drivers/hid/hid-topseed.c b/drivers/hid/hid-topseed.c
index 613ff7b..e924b7d 100644
--- a/drivers/hid/hid-topseed.c
+++ b/drivers/hid/hid-topseed.c
@@ -28,7 +28,7 @@
EV_KEY, (c))

static int ts_input_mapping(struct hid_device *hdev, struct hid_input *hi,

struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)

+ unsigned int usage_index, unsigned long **bit, int *max)
{
if ((usage->hid & HID_USAGE_PAGE) != HID_UP_LOGIVENDOR)
return 0;
diff --git a/drivers/hid/hid-twinhan.c b/drivers/hid/hid-twinhan.c
index f23456b..b38e6b3 100644
--- a/drivers/hid/hid-twinhan.c
+++ b/drivers/hid/hid-twinhan.c
@@ -62,7 +62,7 @@
EV_KEY, (c))

static int twinhan_input_mapping(struct hid_device *hdev, struct hid_input *hi,

struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)

+ unsigned int usage_index, unsigned long **bit, int *max)
{
if ((usage->hid & HID_USAGE_PAGE) != HID_UP_KEYBOARD)
return 0;

diff --git a/drivers/hid/hid-zydacron.c b/drivers/hid/hid-zydacron.c
index 1ad85f2..af0e171 100644
--- a/drivers/hid/hid-zydacron.c
+++ b/drivers/hid/hid-zydacron.c
@@ -47,7 +47,7 @@ static __u8 *zc_report_fixup(struct hid_device *hdev, __u8 *rdesc,

static int zc_input_mapping(struct hid_device *hdev, struct hid_input *hi,

struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ unsigned int usage_index, unsigned long **bit, int *max)
{

int i;
struct zc_device *zc = hid_get_drvdata(hdev);

diff --git a/include/linux/hid.h b/include/linux/hid.h
index 9edb06c..6216529 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -660,14 +660,16 @@ struct hid_driver {
u8 *data, int size);
const struct hid_usage_id *usage_table;
int (*event)(struct hid_device *hdev, struct hid_field *field,
- struct hid_usage *usage, __s32 value);
+ struct hid_usage *usage, unsigned int usage_index,
+ __s32 value);

__u8 *(*report_fixup)(struct hid_device *hdev, __u8 *buf,
unsigned int *size);

int (*input_mapping)(struct hid_device *hdev,
struct hid_input *hidinput, struct hid_field *field,
- struct hid_usage *usage, unsigned long **bit, int *max);
+ struct hid_usage *usage, unsigned int usage_index,
+ unsigned long **bit, int *max);
int (*input_mapped)(struct hid_device *hdev,
struct hid_input *hidinput, struct hid_field *field,
struct hid_usage *usage, unsigned long **bit, int *max);

[Benjamin Tissoires]

Win8 certification introduced the ability to transmit two X and two Y per
touch. The specification precises that it must be in an array, with a
report count == 2.

This test guarantees that we split the touches on the last element
in this array.

Signed-off-by: Benjamin Tissoires <benjamin....@enac.fr>
---
drivers/hid/hid-multitouch.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 725d155..95562d8 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c

@@ -577,12 +577,16 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
return 0;
}

- if (usage->hid == td->last_slot_field)
- mt_complete_slot(td, field->hidinput->input);
-
- if (field->index == td->last_field_index
- && td->num_received >= td->num_expected)
- mt_sync_frame(td, field->hidinput->input);
+ if (usage_index + 1 == field->report_count) {
+ /* we only take into account the last report
+ * of a field if report_count > 1 */
+ if (usage->hid == td->last_slot_field)
+ mt_complete_slot(td, field->hidinput->input);
+
+ if (field->index == td->last_field_index
+ && td->num_received >= td->num_expected)
+ mt_sync_frame(td, field->hidinput->input);
+ }

}

[Benjamin Tissoires]

Win8 devices are required to present the feature "Maximum Contact Number".
Fortunately all win7 devices I've seen presents this feature.
If the current value is 0, then, the driver can get the actual supported
contact count by refering to the logical_max.
This win8 specification ensures that logical_max may not be above 250.
This also allows us to detect when devices like irtouch or stantum reports
an obviously wrong value of 255.

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

drivers/hid/hid-multitouch.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 95562d8..41f2981 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c

@@ -121,6 +121,7 @@ struct mt_device {
#define MT_CLS_GENERALTOUCH_PWT_TENFINGERS 0x0109

#define MT_DEFAULT_MAXCONTACT 10
+#define MT_MAX_MAXCONTACT 250

#define MT_USB_DEVICE(v, p) HID_DEVICE(BUS_USB, HID_GROUP_MULTITOUCH, v, p)
#define MT_BT_DEVICE(v, p) HID_DEVICE(BUS_BLUETOOTH, HID_GROUP_MULTITOUCH, v, p)
@@ -283,6 +284,9 @@ static void mt_feature_mapping(struct hid_device *hdev,
case HID_DG_CONTACTMAX:
td->maxcontact_report_id = field->report->id;
td->maxcontacts = field->value[0];
+ if (!td->maxcontacts &&
+ field->logical_maximum <= MT_MAX_MAXCONTACT)
+ td->maxcontacts = field->logical_maximum;
if (td->mtclass.maxcontacts)
/* check if the maxcontacts is given by the class */
td->maxcontacts = td->mtclass.maxcontacts;

[Benjamin Tissoires]

Win 8 digitizer devices provides the actual scan time computed by the
hardware itself. The value is global to the frame and is not specific
to the multitouch protocol (though only touch, not pen, should use it
according to the specification).

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

Documentation/input/event-codes.txt | 7 +++++++
drivers/hid/hid-input.c | 4 ++++

drivers/hid/hid-multitouch.c | 11 +++++++++--

include/linux/hid.h | 1 +
include/linux/input.h | 1 +

5 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/Documentation/input/event-codes.txt b/Documentation/input/event-codes.txt
index 53305bd..8f8c908 100644
--- a/Documentation/input/event-codes.txt
+++ b/Documentation/input/event-codes.txt
@@ -174,6 +174,13 @@ A few EV_ABS codes have special meanings:
the input device may be used freely in three dimensions, consider ABS_Z
instead.

+* ABS_SCAN_TIME:
+ - Used when the device provides a timestamp for each frame. The unit must be
+ 100us, and may be reset when the device don't send any events for a
+ period of time. The values increment at each frame and thus, it can roll
+ back to 0 when reach logical_max. If the device does not provide this
+ information, the driver must not provide it to the user space.
+
* ABS_MT_<name>:
- Used to describe multitouch input events. Please see
multi-touch-protocol.txt for details.

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 16cc89a..5fe7bd3 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c

@@ -675,6 +675,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
map_key_clear(BTN_STYLUS2);
break;

+ case 0x56: /* Scan Time */
+ map_abs(ABS_SCAN_TIME);
+ break;
+
default: goto unknown;
}
break;

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index c0ab1c6..21a120b 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -447,12 +447,19 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,

mt_store_field(usage, td, hi);
td->last_field_index = field->index;
return 1;

+ case HID_DG_SCANTIME:

+ hid_map_usage(hi, usage, bit, max,

+ EV_ABS, ABS_SCAN_TIME);
+ set_abs(hi->input, ABS_SCAN_TIME, field, 0);
+ td->last_field_index = field->index;
+ return 1;
case HID_DG_CONTACTCOUNT:

td->last_field_index = field->index;
return 1;

case HID_DG_CONTACTMAX:
- /* we don't set td->last_slot_field as contactcount and
- * contact max are global to the report */
+ /* we don't set td->last_slot_field as scan time,
+ * contactcount and contact max are global to the
+ * report */

td->last_field_index = field->index;

return -1;
case HID_DG_TOUCH:
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 6216529..99a6418 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h

@@ -279,6 +279,7 @@ struct hid_item {
#define HID_DG_DEVICEINDEX 0x000d0053
#define HID_DG_CONTACTCOUNT 0x000d0054
#define HID_DG_CONTACTMAX 0x000d0055
+#define HID_DG_SCANTIME 0x000d0056

/*
* HID report types --- Ouch! HID spec says 1 2 3!
diff --git a/include/linux/input.h b/include/linux/input.h
index ba48743..73c3a96 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -796,6 +796,7 @@ struct input_keymap_entry {
#define ABS_TILT_X 0x1a
#define ABS_TILT_Y 0x1b
#define ABS_TOOL_WIDTH 0x1c
+#define ABS_SCAN_TIME 0x1d

#define ABS_VOLUME 0x20

[Benjamin Tissoires]

Win8 input specification clarifies the X and Y sent by devices.
It distincts the position where the user wants to Touch (T) from
the center of the ellipsoide (C). This patch enable supports for this
distinction in hid-multitouch.

We recognize Win8 certified devices from their vendor feature 0xff0000c5
where Microsoft put a signed blob in the report to check if the device
passed the certification.

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

drivers/hid/hid-multitouch.c | 53 ++++++++++++++++++++++++++++++++++++--------
1 file changed, 44 insertions(+), 9 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 41f2981..000c979 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c

@@ -52,9 +52,10 @@ MODULE_LICENSE("GPL");
#define MT_QUIRK_VALID_IS_CONFIDENCE (1 << 6)
#define MT_QUIRK_SLOT_IS_CONTACTID_MINUS_ONE (1 << 8)
#define MT_QUIRK_NO_AREA (1 << 9)

+#define MT_QUIRK_WIN_8_CERTIFIED (1 << 10)

struct mt_slot {
- __s32 x, y, p, w, h;
+ __s32 x, y, cx, cy, p, w, h;

__s32 contactid; /* the device ContactID assigned to this slot */
bool touch_state; /* is the touch valid? */
};

@@ -292,6 +293,10 @@ static void mt_feature_mapping(struct hid_device *hdev,

td->maxcontacts = td->mtclass.maxcontacts;

break;
+ case 0xff0000c5:
+ if (field->report_count == 256 && field->report_size == 8)
+ td->mtclass.quirks |= MT_QUIRK_WIN_8_CERTIFIED;
+ break;
}
}

@@ -350,18 +355,36 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
case HID_UP_GENDESK:
switch (usage->hid) {
case HID_GD_X:
- hid_map_usage(hi, usage, bit, max,
+ if (cls->quirks & MT_QUIRK_WIN_8_CERTIFIED &&
+ usage_index) {

+ hid_map_usage(hi, usage, bit, max,

+ EV_ABS, ABS_MT_TOOL_X);
+ set_abs(hi->input, ABS_MT_TOOL_X, field,
+ cls->sn_move);
+ } else {

+ hid_map_usage(hi, usage, bit, max,

EV_ABS, ABS_MT_POSITION_X);
- set_abs(hi->input, ABS_MT_POSITION_X, field,
- cls->sn_move);
+ set_abs(hi->input, ABS_MT_POSITION_X, field,
+ cls->sn_move);
+ }
+

mt_store_field(usage, td, hi);
td->last_field_index = field->index;
return 1;

case HID_GD_Y:

- hid_map_usage(hi, usage, bit, max,
+ if (cls->quirks & MT_QUIRK_WIN_8_CERTIFIED &&
+ usage_index) {

+ hid_map_usage(hi, usage, bit, max,

+ EV_ABS, ABS_MT_TOOL_Y);
+ set_abs(hi->input, ABS_MT_TOOL_Y, field,
+ cls->sn_move);
+ } else {

+ hid_map_usage(hi, usage, bit, max,

EV_ABS, ABS_MT_POSITION_Y);
- set_abs(hi->input, ABS_MT_POSITION_Y, field,
- cls->sn_move);
+ set_abs(hi->input, ABS_MT_POSITION_Y, field,
+ cls->sn_move);
+ }
+

mt_store_field(usage, td, hi);
td->last_field_index = field->index;
return 1;

@@ -502,6 +525,12 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)

input_event(input, EV_ABS, ABS_MT_POSITION_X, s->x);
input_event(input, EV_ABS, ABS_MT_POSITION_Y, s->y);
+ if (td->mtclass.quirks & MT_QUIRK_WIN_8_CERTIFIED) {
+ input_event(input, EV_ABS, ABS_MT_TOOL_X,
+ s->cx);
+ input_event(input, EV_ABS, ABS_MT_TOOL_Y,

+ s->cy);
+ }

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);

@@ -553,10 +582,16 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
td->curdata.p = value;
break;
case HID_GD_X:
- td->curdata.x = value;
+ if (usage->code == ABS_MT_POSITION_X)
+ td->curdata.x = value;
+ else
+ td->curdata.cx = value;
break;
case HID_GD_Y:
- td->curdata.y = value;
+ if (usage->code == ABS_MT_POSITION_Y)
+ td->curdata.y = value;
+ else
+ td->curdata.cy = value;
break;
case HID_DG_WIDTH:
td->curdata.w = value;

[Benjamin Tissoires]

Exporting this function allows us to calculate the resolution in third
party drivers like hid-multitouch.
This patch also complete the function with additional valid axes.

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

drivers/hid/hid-input.c | 11 +++++++++--
drivers/hid/hid-multitouch.c | 1 +
include/linux/hid.h | 1 +
3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index d917c0d..1b0adc3 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -208,7 +208,7 @@ static int hidinput_setkeycode(struct input_dev *dev,
* Only exponent 1 length units are processed. Centimeters and inches are
* converted to millimeters. Degrees are converted to radians.
*/
-static __s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code)
+__s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code)
{
__s32 unit_exponent = field->unit_exponent;
__s32 logical_extents = field->logical_maximum -
@@ -229,6 +229,12 @@ static __s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code)
case ABS_X:
case ABS_Y:
case ABS_Z:
+ case ABS_MT_POSITION_X:
+ case ABS_MT_POSITION_Y:
+ case ABS_MT_TOOL_X:
+ case ABS_MT_TOOL_Y:
+ case ABS_MT_TOUCH_MAJOR:
+ case ABS_MT_TOUCH_MINOR:

if (field->unit == 0x11) { /* If centimeters */

/* Convert to millimeters */

unit_exponent += 1;
@@ -281,8 +287,9 @@ static __s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code)
}

/* Calculate resolution */
- return logical_extents / physical_extents;
+ return DIV_ROUND_CLOSEST(logical_extents, physical_extents);
}
+EXPORT_SYMBOL_GPL(hidinput_calc_abs_res);

#ifdef CONFIG_HID_BATTERY_STRENGTH
static enum power_supply_property hidinput_battery_props[] = {
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index c97011c..375a38d 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -298,6 +298,7 @@ static void set_abs(struct input_dev *input, unsigned int code,
int fmax = field->logical_maximum;
int fuzz = snratio ? (fmax - fmin) / snratio : 0;
input_set_abs_params(input, code, fmin, fmax, fuzz, 0);
+ input_abs_set_res(input, code, hidinput_calc_abs_res(field, code));

}

static void mt_store_field(struct hid_usage *usage, struct mt_device *td,

diff --git a/include/linux/hid.h b/include/linux/hid.h
index 7e1f37d..9edb06c 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -743,6 +743,7 @@ int hid_input_report(struct hid_device *, int type, u8 *, int, int);
int hidinput_find_field(struct hid_device *hid, unsigned int type, unsigned int code, struct hid_field **field);
struct hid_field *hidinput_get_led_field(struct hid_device *hid);
unsigned int hidinput_count_leds(struct hid_device *hid);
+__s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code);
void hid_output_report(struct hid_report *report, __u8 *data);
struct hid_device *hid_allocate_device(void);
struct hid_report *hid_register_report(struct hid_device *device, unsigned type, unsigned id);

[Benjamin Tissoires]

Hi guys,

Ok, now, it should work with Jiri's tree.

v1 introduction:

So, this is an update for supporting Win 8 multitouch devices in the kernel.
As I wanted to reliably forward the resolution, I noticed a bug in the

processing of the unit_exponent in the hid core layer.

Thus the fixes for hid-core and hid-input.

v2 changes:
* added missing initial patch that prevents the series to be applied on top of Jiri's tree
* update to include latest hid changes
* taken into account Alan's patch: "hid: put the case in the right switch statement"

Sorry for the noise...

Cheers,
Benjamin

---

Benjamin Tissoires (11):
HID: hid-input: export hidinput_calc_abs_res
HID: core: fix unit exponent parsing
HID: hid-input: add usage_index argument in input_mapping and event.
HID: hid-multitouch: support arrays for the split of the touches in a
report
HID: hid-multitouch: get maxcontacts also from logical_max value
HID: hid-multitouch: support T and C for win8 devices
HID: hid-multitouch: move ALWAYS_VALID quirk check
HID: hid-multitouch: fix Win 8 protocol
HID: hid-multitouch: support for hovering devices
HID: introduce Scan Time
HID: hid-multitouch: get rid of usbhid depedency for general path

Documentation/input/event-codes.txt | 7 ++
drivers/hid/hid-a4tech.c | 2 +-
drivers/hid/hid-apple.c | 4 +-

drivers/hid/hid-belkin.c | 2 +-
drivers/hid/hid-cherry.c | 2 +-
drivers/hid/hid-chicony.c | 2 +-

drivers/hid/hid-core.c | 26 +++--
drivers/hid/hid-cypress.c | 2 +-
drivers/hid/hid-ezkey.c | 4 +-
drivers/hid/hid-gyration.c | 4 +-
drivers/hid/hid-input.c | 44 ++++++---

drivers/hid/hid-kensington.c | 2 +-
drivers/hid/hid-lcpower.c | 2 +-

drivers/hid/hid-lenovo-tpkbd.c | 3 +-
drivers/hid/hid-lg.c | 4 +-
drivers/hid/hid-magicmouse.c | 3 +-
drivers/hid/hid-microsoft.c | 4 +-
drivers/hid/hid-monterey.c | 2 +-
drivers/hid/hid-multitouch.c | 191 ++++++++++++++++++++++++++----------
drivers/hid/hid-ntrig.c | 4 +-

drivers/hid/hid-petalynx.c | 2 +-
drivers/hid/hid-prodikeys.c | 2 +-

drivers/hid/hid-ps3remote.c | 3 +-
drivers/hid/hid-samsung.c | 2 +-
drivers/hid/hid-speedlink.c | 6 +-

drivers/hid/hid-sunplus.c | 2 +-
drivers/hid/hid-tivo.c | 2 +-
drivers/hid/hid-topseed.c | 2 +-
drivers/hid/hid-twinhan.c | 2 +-
drivers/hid/hid-zydacron.c | 2 +-

include/linux/hid.h | 8 +-
include/linux/input.h | 1 +
32 files changed, 244 insertions(+), 104 deletions(-)

[Henrik Rydberg]

Hi Benjamin,

> Exporting this function allows us to calculate the resolution in third
> party drivers like hid-multitouch.
> This patch also complete the function with additional valid axes.
>
> Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
> ---

Nice, but please see comment below.

This line might be best left alone; if the round-off matters, you now
risk regressing a carefully tuned userland.

Thanks,
Henrik

[Henrik Rydberg]

Hi Benjamin,

> HID spec details special values for the HID field unit exponent.
> Basically, the range [0x8..0xf] correspond to [-8..-1].
>
> Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
> ---

Standard two's complement, in other words?

> drivers/hid/hid-core.c | 10 +++++++++-
> drivers/hid/hid-input.c | 19 +++++++++++++------
> 2 files changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 9904776..6bde6e4 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -315,6 +315,7 @@ static s32 item_sdata(struct hid_item *item)
>
> static int hid_parser_global(struct hid_parser *parser, struct hid_item *item)
> {
> + __u32 raw_value;
> switch (item->tag) {
> case HID_GLOBAL_ITEM_TAG_PUSH:
>
> @@ -365,7 +366,14 @@ static int hid_parser_global(struct hid_parser *parser, struct hid_item *item)
> return 0;
>
> case HID_GLOBAL_ITEM_TAG_UNIT_EXPONENT:
> - parser->global.unit_exponent = item_sdata(item);
> + /* Units exponent negative numbers are given through a special
> + * table.
> + * See "6.2.2.7 Global Items" for more information. */
> + raw_value = item_udata(item);
> + if ((raw_value >> 3) == 1)
> + parser->global.unit_exponent = raw_value - 16;
> + else
> + parser->global.unit_exponent = raw_value;

I beleive the function you want is snto32(value, 4).

> return 0;
>
> case HID_GLOBAL_ITEM_TAG_UNIT:
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 1b0adc3..fc9f2b5 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -215,7 +215,7 @@ __s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code)
> field->logical_minimum;
> __s32 physical_extents = field->physical_maximum -
> field->physical_minimum;
> - __s32 prev;
> + __s32 prev, tmp_exponent;
>
> /* Check if the extents are sane */
> if (logical_extents <= 0 || physical_extents <= 0)
> @@ -235,17 +235,24 @@ __s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code)
> case ABS_MT_TOOL_Y:
> case ABS_MT_TOUCH_MAJOR:
> case ABS_MT_TOUCH_MINOR:
> - if (field->unit == 0x11) { /* If centimeters */
> + if (field->unit & 0xffffff00) /* Not a length */
> + return 0;
> + tmp_exponent = field->unit >> 4;
> + if ((tmp_exponent >> 3) == 1)
> + tmp_exponent -= 16;

Ditto.

> + switch (field->unit & 0xf) {
> + case 0x1: /* If centimeters */
> /* Convert to millimeters */
> - unit_exponent += 1;
> - } else if (field->unit == 0x13) { /* If inches */
> + unit_exponent += tmp_exponent;
> + break;
> + case 0x3: /* If inches */
> /* Convert to millimeters */
> prev = physical_extents;
> physical_extents *= 254;
> if (physical_extents < prev)
> return 0;
> - unit_exponent -= 1;
> - } else {
> + unit_exponent += tmp_exponent - 2;
> + default:
> return 0;
> }
> break;
> --
> 1.7.11.7
>

Thanks,
Henrik

[Henrik Rydberg]

Hi Benjamin,

> Currently, there is no way to know the index of the current field
> in the .input_mapping and .event callbacks when this field is inside
> an array of HID fields.
> This patch forwards this index to the input_mapping and event
> callbacks.

I agree with the idea, but the function argument list is becoming
ridiculously long... Could we remove the usage pointer argument, at
least?

int (*event)(struct hid_device *hdev, struct hid_field *field,

unsigned int usage_index, __s32 value);

> @@ -1071,19 +1072,24 @@ static void hid_input_field(struct hid_device *hid, struct hid_field *field,
> for (n = 0; n < count; n++) {
>
> if (HID_MAIN_ITEM_VARIABLE & field->flags) {
> - hid_process_event(hid, field, &field->usage[n], value[n], interrupt);
> + hid_process_event(hid, field, &field->usage[n], n,
> + value[n], interrupt);
> continue;
> }
>
> if (field->value[n] >= min && field->value[n] <= max
> && field->usage[field->value[n] - min].hid
> && search(value, field->value[n], count))
> - hid_process_event(hid, field, &field->usage[field->value[n] - min], 0, interrupt);
> + hid_process_event(hid, field,
> + &field->usage[field->value[n] - min], n,
> + 0, interrupt);

Wrong index?

>
> if (value[n] >= min && value[n] <= max
> && field->usage[value[n] - min].hid
> && search(field->value, value[n], count))
> - hid_process_event(hid, field, &field->usage[value[n] - min], 1, interrupt);
> + hid_process_event(hid, field,
> + &field->usage[value[n] - min], n,
> + 1, interrupt);

Wrong index?

> }
>
> memcpy(field->value, value, count * sizeof(__s32));

Thanks,
Henrik

[Henrik Rydberg]

Hi Benjamin,

> Win8 certification introduced the ability to transmit two X and two Y per
> touch. The specification precises that it must be in an array, with a
> report count == 2.

The number two never really enters the patch, so maybe it should be
dropped to avoid confusion. It probably makes more sense to comment on
in a later patch, when the reports are actually used.

>
> This test guarantees that we split the touches on the last element
> in this array.
>
> Signed-off-by: Benjamin Tissoires <benjamin....@enac.fr>
> ---
> drivers/hid/hid-multitouch.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 725d155..95562d8 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -577,12 +577,16 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
> return 0;
> }
>
> - if (usage->hid == td->last_slot_field)
> - mt_complete_slot(td, field->hidinput->input);
> -
> - if (field->index == td->last_field_index
> - && td->num_received >= td->num_expected)
> - mt_sync_frame(td, field->hidinput->input);
> + if (usage_index + 1 == field->report_count) {
> + /* we only take into account the last report
> + * of a field if report_count > 1 */

Seems we could drop "of a field if report_count > 1" here, and be even
more correct.

> + if (usage->hid == td->last_slot_field)
> + mt_complete_slot(td, field->hidinput->input);
> +
> + if (field->index == td->last_field_index
> + && td->num_received >= td->num_expected)
> + mt_sync_frame(td, field->hidinput->input);
> + }
>
> }
>
> --
> 1.7.11.7

Thanks,
Henrik

[Henrik Rydberg]

Acked-by: Henrik Rydberg <ryd...@euromail.se>

[Henrik Rydberg]

Hi Benjamin,

> Win8 input specification clarifies the X and Y sent by devices.
> It distincts the position where the user wants to Touch (T) from
> the center of the ellipsoide (C). This patch enable supports for this
> distinction in hid-multitouch.
>
> We recognize Win8 certified devices from their vendor feature 0xff0000c5
> where Microsoft put a signed blob in the report to check if the device
> passed the certification.
>
> Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
> ---
> drivers/hid/hid-multitouch.c | 53 ++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 44 insertions(+), 9 deletions(-)

This is great, just a few comments below.

Parenthesis, please. Precedence is not always enough.

> + usage_index) {
> + hid_map_usage(hi, usage, bit, max,
> + EV_ABS, ABS_MT_TOOL_X);
> + set_abs(hi->input, ABS_MT_TOOL_X, field,
> + cls->sn_move);
> + } else {
> + hid_map_usage(hi, usage, bit, max,
> EV_ABS, ABS_MT_POSITION_X);
> - set_abs(hi->input, ABS_MT_POSITION_X, field,
> - cls->sn_move);
> + set_abs(hi->input, ABS_MT_POSITION_X, field,
> + cls->sn_move);
> + }
> +

Do we really want to do the latter several times, even if the device is not a win8 one?

> mt_store_field(usage, td, hi);
> td->last_field_index = field->index;
> return 1;
> case HID_GD_Y:
> - hid_map_usage(hi, usage, bit, max,
> + if (cls->quirks & MT_QUIRK_WIN_8_CERTIFIED &&

Ditto.

> + usage_index) {
> + hid_map_usage(hi, usage, bit, max,
> + EV_ABS, ABS_MT_TOOL_Y);
> + set_abs(hi->input, ABS_MT_TOOL_Y, field,
> + cls->sn_move);
> + } else {
> + hid_map_usage(hi, usage, bit, max,
> EV_ABS, ABS_MT_POSITION_Y);
> - set_abs(hi->input, ABS_MT_POSITION_Y, field,
> - cls->sn_move);
> + set_abs(hi->input, ABS_MT_POSITION_Y, field,
> + cls->sn_move);
> + }
> +

Ditto.

> mt_store_field(usage, td, hi);
> td->last_field_index = field->index;
> return 1;
> @@ -502,6 +525,12 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
>
> input_event(input, EV_ABS, ABS_MT_POSITION_X, s->x);
> input_event(input, EV_ABS, ABS_MT_POSITION_Y, s->y);
> + if (td->mtclass.quirks & MT_QUIRK_WIN_8_CERTIFIED) {
> + input_event(input, EV_ABS, ABS_MT_TOOL_X,
> + s->cx);

Won't this fit on one line?

> + input_event(input, EV_ABS, ABS_MT_TOOL_Y,
> + s->cy);
> + }
> 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);
> @@ -553,10 +582,16 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
> td->curdata.p = value;
> break;
> case HID_GD_X:
> - td->curdata.x = value;
> + if (usage->code == ABS_MT_POSITION_X)
> + td->curdata.x = value;
> + else
> + td->curdata.cx = value;

Since cx is the new value, reversing the logic would make sense here.

> break;
> case HID_GD_Y:
> - td->curdata.y = value;
> + if (usage->code == ABS_MT_POSITION_Y)
> + td->curdata.y = value;
> + else
> + td->curdata.cy = value;
> break;
> case HID_DG_WIDTH:
> td->curdata.w = value;
> --
> 1.7.11.7
>

Thanks,
Henrik

[Henrik Rydberg]

Hi Benjamin,

> Win 8 device specification changed the requirements for the hid usages
> of the multitouch devices. Now InRange is optional and must be only
> used when the device supports hovering.
>
> This ensures that the quirk ALWAYS_VALID is taken into account and
> also ensures its precedence over the other VALID* quirks.
>
> Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
> ---
> drivers/hid/hid-multitouch.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)

Since we seem to never actually reset td, this seems equivalent,
unless some device added ALWAYS VALID by mistake, even if INRANGE is
not part of the report descriptor.

> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 000c979..43bd8e8 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -506,7 +506,7 @@ static int mt_compute_slot(struct mt_device *td, struct input_dev *input)
> */
> static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
> {
> - if (td->curvalid) {
> + if (td->curvalid || td->mtclass.quirks & MT_QUIRK_ALWAYS_VALID) {

I found at least one presence of this construct in the kernel, but I
think the overwhelming majority use parenthesis.

> int slotnum = mt_compute_slot(td, input);
> struct mt_slot *s = &td->curdata;
>
> @@ -561,9 +561,7 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
> if (hid->claimed & HID_CLAIMED_INPUT) {
> switch (usage->hid) {
> case HID_DG_INRANGE:
> - if (quirks & MT_QUIRK_ALWAYS_VALID)
> - td->curvalid = true;
> - else if (quirks & MT_QUIRK_VALID_IS_INRANGE)
> + if (quirks & MT_QUIRK_VALID_IS_INRANGE)
> td->curvalid = value;
> break;
> case HID_DG_TIPSWITCH:
> --
> 1.7.11.7
>

Thanks,
Henrik

[Henrik Rydberg]

On Fri, Oct 26, 2012 at 10:44:24AM +0200, Benjamin Tissoires wrote:
> Win 8 specification is much more precise than the Win 7 one.
> Moreover devices that need to take certification must be submitted
> to Microsoft.
>
> The result is a better protocol support and we can rely on that to
> skip all the messy tests we used to do.
>
> The protocol specify the fact that each valid touch must be reported
> within a frame, and that the release touch coordinate must be the
> same than the last known touch.
> So we can use the always_valid quirk and dismiss reports when we
> touch coordiante do not follow this rule.
>
> Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
> ---
> drivers/hid/hid-multitouch.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)

Why do we need this patch?

Henrik

[Henrik Rydberg]

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....@gmail.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?

This only happens if touch_state is true?

> 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?

> + else
> + td->curdata.touch_state = value;
> break;
> case HID_DG_CONFIDENCE:
> if (quirks & MT_QUIRK_VALID_IS_CONFIDENCE)
> --
> 1.7.11.7
>

Henrik

[Henrik Rydberg]

Hi Benjamin,

> Win 8 digitizer devices provides the actual scan time computed by the
> hardware itself. The value is global to the frame and is not specific
> to the multitouch protocol (though only touch, not pen, should use it
> according to the specification).
>
> Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
> ---
> Documentation/input/event-codes.txt | 7 +++++++
> drivers/hid/hid-input.c | 4 ++++
> drivers/hid/hid-multitouch.c | 11 +++++++++--
> include/linux/hid.h | 1 +
> include/linux/input.h | 1 +
> 5 files changed, 22 insertions(+), 2 deletions(-)

This is a nice feature, useful in many other contexts. As such, I
think it should be defined in the context of the input subsystem, with
a more specific definition added to the documentation. For instance,
is 100us suitable? When should it start at zero, at BTN_TOUCH? Or
should it perhaps wrap around on unsigned integer instead? Or display
the difference from the last event?

Is it not enough to map it in the case below? Or you mean this is
picked up by hid core?

Thanks,
Henrik

[Henrik Rydberg]

Hi Benjamin,

> This patch factorizes the hid set_feature command by using
> hid_device->hid_output_raw_report instead of direclty relying on
> usbhid. This makes the driver usb independant.
>
> However I still can't remove the 2 usb related headers because the
> function mt_resume has a specific patch for usb devices.
>
> Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
> ---
> drivers/hid/hid-multitouch.c | 63 ++++++++++++++++++++++++++------------------
> 1 file changed, 37 insertions(+), 26 deletions(-)

In my drawer, I have a patchset that aims to remove all usbhid
dependence, from all the drivers. Perhaps the attached patch is
something to consider here?

Thanks,
Henrik

---

From 5d9a791e0a9e41bcea0fcb286e2849b403675f37 Mon Sep 17 00:00:00 2001
From: Henrik Rydberg <ryd...@euromail.se>
Date: Mon, 2 Jul 2012 20:38:21 +0200
Subject: [PATCH 3/7] hid: extend interface with report requests

---
drivers/hid/usbhid/hid-core.c | 14 ++++++++++++++
include/linux/hid.h | 33 +++++++++++++++++++++++++++++++++
2 files changed, 47 insertions(+)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index de1f9ac..3c618da 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1251,6 +1251,18 @@ static int usbhid_power(struct hid_device *hid, int lvl)
return r;
}

+static void usbhid_send(struct hid_device *hid, struct hid_report *rep, int req)
+{
+ switch (req) {
+ case HID_REQ_GET_REPORT:
+ usbhid_submit_report(hid, rep, USB_DIR_IN);
+ break;
+ case HID_REQ_SET_REPORT:
+ usbhid_submit_report(hid, rep, USB_DIR_OUT);
+ break;
+ }
+}
+
static struct hid_ll_driver usb_hid_driver = {
.parse = usbhid_parse,
.start = usbhid_start,
@@ -1259,6 +1271,8 @@ static struct hid_ll_driver usb_hid_driver = {
.close = usbhid_close,
.power = usbhid_power,
.hidinput_input_event = usb_hidinput_input_event,
+ .send = usbhid_send,
+ .wait = usbhid_wait_io,
};

static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *id)
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 241eb40..5e169c1 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -686,6 +686,8 @@ struct hid_driver {
* @hidinput_input_event: event input event (e.g. ff or leds)
* @parse: this method is called only once to parse the device data,
* shouldn't allocate anything to not leak memory
+ * @send: send report request to device (e.g. feature report)
+ * @wait: wait for buffered io to complete (send/recv reports)
*/
struct hid_ll_driver {
int (*start)(struct hid_device *hdev);
@@ -700,6 +702,11 @@ struct hid_ll_driver {
unsigned int code, int value);

int (*parse)(struct hid_device *hdev);
+
+ void (*send)(struct hid_device *hdev,
+ struct hid_report *report, int req);
+ int (*wait)(struct hid_device *hdev);
+
};

#define PM_HINT_FULLON 1<<5
@@ -892,6 +899,32 @@ static inline int hid_hw_power(struct hid_device *hdev, int level)
return hdev->ll_driver->power ? hdev->ll_driver->power(hdev, level) : 0;
}

+
+/**
+ * hid_hw_send - send report request to device
+ *
+ * @hdev: hid device
+ * @report: report to send
+ * @req: hid request type
+ */
+static inline void hid_hw_send(struct hid_device *hdev,
+ struct hid_report *report, int req)
+{
+ if (hdev->ll_driver->send)
+ hdev->ll_driver->send(hdev, report, req);
+}
+
+/**
+ * hid_hw_wait - wait for buffered io to complete
+ *
+ * @hdev: hid device
+ */
+static inline void hid_hw_wait(struct hid_device *hdev)
+{
+ if (hdev->ll_driver->wait)
+ hdev->ll_driver->wait(hdev);
+}
+
int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size,
int interrupt);

--
1.7.11.1

[Benjamin Tissoires]

Hi Henrik,

On Mon, Oct 29, 2012 at 8:05 PM, Henrik Rydberg <ryd...@euromail.se> wrote:
> Hi Benjamin,
>
>> HID spec details special values for the HID field unit exponent.
>> Basically, the range [0x8..0xf] correspond to [-8..-1].
>>
>> Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
>> ---
>
> Standard two's complement, in other words?

yes, but for 4-bits values. The thing is that I didn't managed to
figure out if it was allowed to give unit exponent with more than 4
bits...

but to be sure, we should still do a test against (raw_value & 0xf0)
for values with more than 4 bits, no?

Cheers,
Benjamin

[Benjamin Tissoires]

On Mon, Oct 29, 2012 at 8:25 PM, Henrik Rydberg <ryd...@euromail.se> wrote:
> Hi Benjamin,
>
>> Currently, there is no way to know the index of the current field
>> in the .input_mapping and .event callbacks when this field is inside
>> an array of HID fields.
>> This patch forwards this index to the input_mapping and event
>> callbacks.
>
> I agree with the idea, but the function argument list is becoming
> ridiculously long... Could we remove the usage pointer argument, at
> least?

yeah, totally agree. Let me just check whether it will not introduce
more problems than it solves for my driver.

>
> int (*event)(struct hid_device *hdev, struct hid_field *field,
> unsigned int usage_index, __s32 value);
>
>
>> @@ -1071,19 +1072,24 @@ static void hid_input_field(struct hid_device *hid, struct hid_field *field,
>> for (n = 0; n < count; n++) {
>>
>> if (HID_MAIN_ITEM_VARIABLE & field->flags) {
>> - hid_process_event(hid, field, &field->usage[n], value[n], interrupt);
>> + hid_process_event(hid, field, &field->usage[n], n,
>> + value[n], interrupt);
>> continue;
>> }
>>
>> if (field->value[n] >= min && field->value[n] <= max
>> && field->usage[field->value[n] - min].hid
>> && search(value, field->value[n], count))
>> - hid_process_event(hid, field, &field->usage[field->value[n] - min], 0, interrupt);
>> + hid_process_event(hid, field,
>> + &field->usage[field->value[n] - min], n,
>> + 0, interrupt);
>
> Wrong index?

oops, I'll have to check that.

Thanks,
Benjamin

[Benjamin Tissoires]

Hi Henrik,

first thanks for the full review of the patch series.

If you think it's better, I'll split the patch in 2 to put aside the
DIV_ROUND_CLOSEST.

Cheers,
Benjamin

[Benjamin Tissoires]

Hi Henrik,

oops

>
>> + usage_index) {
>> + hid_map_usage(hi, usage, bit, max,
>> + EV_ABS, ABS_MT_TOOL_X);
>> + set_abs(hi->input, ABS_MT_TOOL_X, field,
>> + cls->sn_move);
>> + } else {
>> + hid_map_usage(hi, usage, bit, max,
>> EV_ABS, ABS_MT_POSITION_X);
>> - set_abs(hi->input, ABS_MT_POSITION_X, field,
>> - cls->sn_move);
>> + set_abs(hi->input, ABS_MT_POSITION_X, field,
>> + cls->sn_move);
>> + }
>> +
>
> Do we really want to do the latter several times, even if the device is not a win8 one?

I don't get your point here. The only difference with the previous
release is that it will treat differently the first in the array than
the others. For non win8 devices, there is no changes in the behavior.
Could you elaborate a little bit more, please?

I'm afraid not: 81 columns... ;-)

>
>> + input_event(input, EV_ABS, ABS_MT_TOOL_Y,
>> + s->cy);
>> + }
>> 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);
>> @@ -553,10 +582,16 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
>> td->curdata.p = value;
>> break;
>> case HID_GD_X:
>> - td->curdata.x = value;
>> + if (usage->code == ABS_MT_POSITION_X)
>> + td->curdata.x = value;
>> + else
>> + td->curdata.cx = value;
>
> Since cx is the new value, reversing the logic would make sense here.

ok

Cheers,
Benjamin

[Benjamin Tissoires]

Hi Henrik,

On Mon, Oct 29, 2012 at 10:49 PM, Henrik Rydberg <ryd...@euromail.se> wrote:
> Hi Benjamin,
>
>> Win8 certification introduced the ability to transmit two X and two Y per
>> touch. The specification precises that it must be in an array, with a
>> report count == 2.
>
> The number two never really enters the patch, so maybe it should be
> dropped to avoid confusion. It probably makes more sense to comment on
> in a later patch, when the reports are actually used.

Yep, it seems that the commit message is not good. I'll rewrite it in v3.

>
>>
>> This test guarantees that we split the touches on the last element
>> in this array.
>>
>> Signed-off-by: Benjamin Tissoires <benjamin....@enac.fr>
>> ---
>> drivers/hid/hid-multitouch.c | 16 ++++++++++------
>> 1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
>> index 725d155..95562d8 100644
>> --- a/drivers/hid/hid-multitouch.c
>> +++ b/drivers/hid/hid-multitouch.c
>> @@ -577,12 +577,16 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
>> return 0;
>> }
>>
>> - if (usage->hid == td->last_slot_field)
>> - mt_complete_slot(td, field->hidinput->input);
>> -
>> - if (field->index == td->last_field_index
>> - && td->num_received >= td->num_expected)
>> - mt_sync_frame(td, field->hidinput->input);
>> + if (usage_index + 1 == field->report_count) {
>> + /* we only take into account the last report
>> + * of a field if report_count > 1 */
>
> Seems we could drop "of a field if report_count > 1" here, and be even
> more correct.

oops ;-)

Cheers,
Benjamin

[Benjamin Tissoires]

On Mon, Oct 29, 2012 at 11:16 PM, Henrik Rydberg <ryd...@euromail.se> wrote:
> Hi Benjamin,
>
>> Win 8 device specification changed the requirements for the hid usages
>> of the multitouch devices. Now InRange is optional and must be only
>> used when the device supports hovering.
>>
>> This ensures that the quirk ALWAYS_VALID is taken into account and
>> also ensures its precedence over the other VALID* quirks.
>>
>> Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
>> ---
>> drivers/hid/hid-multitouch.c | 6 ++----
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> Since we seem to never actually reset td, this seems equivalent,
> unless some device added ALWAYS VALID by mistake, even if INRANGE is
> not part of the report descriptor.

Yes, it is equivalent for Win7 devices. The field InRange was
mandatory, and currently, nobody mixed ALWAYS_VALID with other quirks.
But now, InRange is optional, and device not supporting hovering
should not use it, so it's mandatory to move it at a place we can be
sure it will be taken into account.

>
>> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
>> index 000c979..43bd8e8 100644
>> --- a/drivers/hid/hid-multitouch.c
>> +++ b/drivers/hid/hid-multitouch.c
>> @@ -506,7 +506,7 @@ static int mt_compute_slot(struct mt_device *td, struct input_dev *input)
>> */
>> static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
>> {
>> - if (td->curvalid) {
>> + if (td->curvalid || td->mtclass.quirks & MT_QUIRK_ALWAYS_VALID) {
>
> I found at least one presence of this construct in the kernel, but I
> think the overwhelming majority use parenthesis.

oops, I was really angry against parenthesis in this patch series :)

Cheers,
Benjamin

[Benjamin Tissoires]

Hi Henrik,

On Mon, Oct 29, 2012 at 11:19 PM, Henrik Rydberg <ryd...@euromail.se> wrote:
> On Fri, Oct 26, 2012 at 10:44:24AM +0200, Benjamin Tissoires wrote:
>> Win 8 specification is much more precise than the Win 7 one.
>> Moreover devices that need to take certification must be submitted
>> to Microsoft.
>>
>> The result is a better protocol support and we can rely on that to
>> skip all the messy tests we used to do.
>>
>> The protocol specify the fact that each valid touch must be reported
>> within a frame, and that the release touch coordinate must be the
>> same than the last known touch.
>> So we can use the always_valid quirk and dismiss reports when we
>> touch coordiante do not follow this rule.
>>
>> Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
>> ---
>> drivers/hid/hid-multitouch.c | 16 ++++++++++++++--
>> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> Why do we need this patch?

This patch prevents a corner case where the device use contactID 0 for
it's first reported touch.
Once you got the invalid touches, most of the time, contactID will be
0, x, y, and other fields too. So this ensures to avoid conflict
between valid values and garbage values. The problem lies in the fact
that we don't have the whole overview of the frame (with the contact
count) to decide which contacts are good and which are not.

Cheers,
Benjamin

[Benjamin Tissoires]

On Mon, Oct 29, 2012 at 11:31 PM, Henrik Rydberg <ryd...@euromail.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....@gmail.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.

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

[Benjamin Tissoires]

On Mon, Oct 29, 2012 at 11:43 PM, Henrik Rydberg <ryd...@euromail.se> wrote:
> Hi Benjamin,
>
>> Win 8 digitizer devices provides the actual scan time computed by the
>> hardware itself. The value is global to the frame and is not specific
>> to the multitouch protocol (though only touch, not pen, should use it
>> according to the specification).
>>
>> Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
>> ---
>> Documentation/input/event-codes.txt | 7 +++++++
>> drivers/hid/hid-input.c | 4 ++++
>> drivers/hid/hid-multitouch.c | 11 +++++++++--
>> include/linux/hid.h | 1 +
>> include/linux/input.h | 1 +
>> 5 files changed, 22 insertions(+), 2 deletions(-)
>
> This is a nice feature, useful in many other contexts. As such, I
> think it should be defined in the context of the input subsystem, with
> a more specific definition added to the documentation. For instance,
> is 100us suitable? When should it start at zero, at BTN_TOUCH? Or
> should it perhaps wrap around on unsigned integer instead? Or display
> the difference from the last event?

Well, the thing is that I didn't wanted to copy/paste win 8 definition
for ScanTime. So I put it with my words and not in a very
understandable way :)
This allows me to forward the incoming events without having to do
anything on them...

- 100us: suitable, not sure, but it would be good to define a standard
unit for time too
- start at zero at BTN_TOUCH: no. This information allows us to do
much better false release detection. If we reset this counter, then we
will loose the time between the two touch/release. The spec says that
it is up to the device to reset it after a period of time (not
defined, but can be one second for instance). Last, BTN_TOUCH is not
reliable for hovering devices because we will still get finger
information without BTN_TOUCH.
- difference from the last event: not sure how it is implemented in
windows, but I'm not sure it's a good way of doing if the gesture
engine needs the time from the beginning of the touch...

Anyway, I would be happy to have other comments/proposals for this event code.

in hidinput_configure_usage, it's enough to just map it.

In hid-multitouch, We also just need to map it, and it will be handled
by hid-core in the .event callback.

Cheers,
Benjamin

[Benjamin Tissoires]

Hi Henrik,

On Mon, Oct 29, 2012 at 11:57 PM, Henrik Rydberg <ryd...@euromail.se> wrote:
> Hi Benjamin,
>
>> This patch factorizes the hid set_feature command by using
>> hid_device->hid_output_raw_report instead of direclty relying on
>> usbhid. This makes the driver usb independant.
>>
>> However I still can't remove the 2 usb related headers because the
>> function mt_resume has a specific patch for usb devices.
>>
>> Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
>> ---
>> drivers/hid/hid-multitouch.c | 63 ++++++++++++++++++++++++++------------------
>> 1 file changed, 37 insertions(+), 26 deletions(-)
>
> In my drawer, I have a patchset that aims to remove all usbhid
> dependence, from all the drivers. Perhaps the attached patch is
> something to consider here?

yep, removing usbhid dependencies is a good thing.
See my review below :)

the name here is a little bit misleading. You are using "send" to also
"get" reports...
Maybe "hid_request" is a better name. This will allows us to add the
missing function:
Get_Descriptor, Set_Descriptor, Get_Report Request, Set_Report
Request, Get_Idle, Set_Idle, Get_Protocol, Set_Protocol and maybe
others - even WAIT for instance :)

> + .wait = usbhid_wait_io,

is it really necessary to wait for IO? (I think it will not be one
line for hid over i2c...).

Cheers,
Benjamin

[Henrik Rydberg]

Hi Benjamin,

> On Mon, Oct 29, 2012 at 11:19 PM, Henrik Rydberg <ryd...@euromail.se> wrote:
> > On Fri, Oct 26, 2012 at 10:44:24AM +0200, Benjamin Tissoires wrote:
> >> Win 8 specification is much more precise than the Win 7 one.
> >> Moreover devices that need to take certification must be submitted
> >> to Microsoft.
> >>
> >> The result is a better protocol support and we can rely on that to
> >> skip all the messy tests we used to do.
> >>
> >> The protocol specify the fact that each valid touch must be reported
> >> within a frame, and that the release touch coordinate must be the
> >> same than the last known touch.
> >> So we can use the always_valid quirk and dismiss reports when we
> >> touch coordiante do not follow this rule.
> >>
> >> Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
> >> ---
> >> drivers/hid/hid-multitouch.c | 16 ++++++++++++++--
> >> 1 file changed, 14 insertions(+), 2 deletions(-)
> >
> > Why do we need this patch?
>
> This patch prevents a corner case where the device use contactID 0 for
> it's first reported touch.
> Once you got the invalid touches, most of the time, contactID will be
> 0, x, y, and other fields too. So this ensures to avoid conflict
> between valid values and garbage values. The problem lies in the fact
> that we don't have the whole overview of the frame (with the contact
> count) to decide which contacts are good and which are not.

I am sorry, but your explanation did not make me any wiser. :-) Are
you saying that touch state changes and touch property changes are
mutually exclusive? For all win8 devices, or just for the serial
protocol ones? For what devices is the current implementation a
problem?

I am asking because this looks more like a device quirk than anything
else.

Thanks,

[Henrik Rydberg]

> >> + usage_index) {
> >> + hid_map_usage(hi, usage, bit, max,
> >> + EV_ABS, ABS_MT_TOOL_X);
> >> + set_abs(hi->input, ABS_MT_TOOL_X, field,
> >> + cls->sn_move);
> >> + } else {
> >> + hid_map_usage(hi, usage, bit, max,
> >> EV_ABS, ABS_MT_POSITION_X);
> >> - set_abs(hi->input, ABS_MT_POSITION_X, field,
> >> - cls->sn_move);
> >> + set_abs(hi->input, ABS_MT_POSITION_X, field,
> >> + cls->sn_move);
> >> + }
> >> +
> >
> > Do we really want to do the latter several times, even if the device is not a win8 one?
>
> I don't get your point here. The only difference with the previous
> release is that it will treat differently the first in the array than
> the others. For non win8 devices, there is no changes in the behavior.
> Could you elaborate a little bit more, please?

I was wondering what we want to do about multiple reports in the
general casel. Not that important though, the patch will probably look
fine in your next version.

[Henrik Rydberg]

Hi Benjamin,

> > This is a nice feature, useful in many other contexts. As such, I
> > think it should be defined in the context of the input subsystem, with
> > a more specific definition added to the documentation. For instance,
> > is 100us suitable? When should it start at zero, at BTN_TOUCH? Or
> > should it perhaps wrap around on unsigned integer instead? Or display
> > the difference from the last event?
>
> Well, the thing is that I didn't wanted to copy/paste win 8 definition
> for ScanTime. So I put it with my words and not in a very
> understandable way :)
> This allows me to forward the incoming events without having to do
> anything on them...
>
> - 100us: suitable, not sure, but it would be good to define a standard
> unit for time too

Units of 100us might be fine, but maybe 64 or 128 or even 1 might be
better suited for implementations.

> - start at zero at BTN_TOUCH: no. This information allows us to do
> much better false release detection. If we reset this counter, then we
> will loose the time between the two touch/release.

Good point.

> The spec says that
> it is up to the device to reset it after a period of time (not
> defined, but can be one second for instance). Last, BTN_TOUCH is not
> reliable for hovering devices because we will still get finger
> information without BTN_TOUCH.

Agreed.

> - difference from the last event: not sure how it is implemented in
> windows, but I'm not sure it's a good way of doing if the gesture
> engine needs the time from the beginning of the touch...

Probably more complicated than it needs to be, yes.

> Anyway, I would be happy to have other comments/proposals for this event code.

Here is my proposal: Let ABS_SCAN_TIME be the number of microseconds
since the last reset. Let it be coded as an uint32 value, which is
allowed to wrap around with no special consequence. It is assumed that
the time difference between two consecutive events is reliable on a
reasonable time scale (hours). A reset to zero can happen, in which
case the time since the last event is unknown. A definition like
time_valid = (time || (time - prev_time) < MAX_SCAN_INTERVAL) ought to
work for most cases.

I think you should intercept it, convert it, and send it out with the touch frame.

[Henrik Rydberg]

> > In my drawer, I have a patchset that aims to remove all usbhid
> > dependence, from all the drivers. Perhaps the attached patch is
> > something to consider here?
>
> yep, removing usbhid dependencies is a good thing.
> See my review below :)
>

Sounds good, I'll ponder this a bit.

>
> > + .wait = usbhid_wait_io,
>
> is it really necessary to wait for IO? (I think it will not be one
> line for hid over i2c...).

We can certainly skip it for the scope of your patchset, but last time
I checked, it was needed in quite a few places.

Thanks,
Henrik

[Benjamin Tissoires]

Hi Henrik,

Sorry, for that. Let's try with other words.

> you saying that touch state changes and touch property changes are
> mutually exclusive? For all win8 devices, or just for the serial
> protocol ones? For what devices is the current implementation a
> problem?

The goal of this patch is to implement in a reliable way Win 8
multitouch protocol (to avoid quirking many devices). Thanks to the
precision they made in the specification, I think it is feasible:
they add the dynamic part that were missing in Win 7 spec:
"""
When sending data in hybrid or parallel mode, a contact that is
delivered in one report must be delivered in all subsequent reports
until it is lifted off the screen. If time is needed to adequate
determine if the contact was lifted off the surface, the device must
report the last known position of the contact and then deliver the
�UP� state of the contact in a subsequent report. Devices should not
send a report without the information for that contact while trying to
determine its current state.
"""

Thus, the quirk ALWAYS_VALID fits very well with win 8 devices (the
device has to send the touch until it is lifted and out of range, and
the device must send the 'up' state).

The problem lies that some devices may reuse contact id 0 within the
frame for the end of the report (my Win8 device doesn't has this kind
of problem):

With the following hid usages:
I -> contact Id
T -> tip switch
X, Y -> X, Y
S -> scan time
C -> contact count

a friendly device would report:

I:1 T:1 X:125 X:130 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:0 C:01
I:1 T:1 X:130 X:135 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:1 C:01
I:1 T:1 X:135 X:140 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:2 C:01
I:1 T:1 X:140 X:145 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:3 C:01
I:1 T:0 X:140 X:145 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:4 C:01

*but*, I've already seen win 7 devices, that do send:

I:0 T:1 X:125 X:130 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:0 C:01
I:0 T:1 X:130 X:135 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:1 C:01
I:0 T:1 X:135 X:140 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:2 C:01
I:0 T:1 X:140 X:145 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:3 C:01
I:0 T:0 X:140 X:145 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:4 C:01

The difference lies in the first bit, contact id is 0.

So, the quirk always valid is not sufficient because the second touch
in the frame will override the values of the first (the valid one).

As Microsoft says that "the device must report the last known position
of the contact and then deliver the �UP� state of the contact", so we
can safely discard the second touch because X and Y do not match the
current state of the valid touch.

Then, as exposed in the "How to Design and Test Multitouch Hardware
Solutions for Windows 8" document here:
http://msdn.microsoft.com/en-us/library/windows/hardware/hh872968.aspx
when the device attempt the certification, if the "up" is not valid,
the error "Last move location different" raises, which, I hope will
prevent the device to get the certification.

I hope it is clearer now.

>
> I am asking because this looks more like a device quirk than anything
> else.

and yes, it looks like a quirk, we could make the "Last move location
different" presented like a quirk, but it is only followed by win 8
devices (or it is by luck).

Cheers,
Benjamin

[Benjamin Tissoires]

all right, I'm fine with it.

With the definition inspired from Win 8, I didn't need to convert it,
thus the hid-core could handle it.
Now, it's clear that if we want to transform it, it needs to be intercepted.

Cheers,
Benjamin

[Henrik Rydberg]

Hi Benjamin,

> The goal of this patch is to implement in a reliable way Win 8
> multitouch protocol (to avoid quirking many devices). Thanks to the
> precision they made in the specification, I think it is feasible:
> they add the dynamic part that were missing in Win 7 spec:
> """
> When sending data in hybrid or parallel mode, a contact that is
> delivered in one report must be delivered in all subsequent reports
> until it is lifted off the screen. If time is needed to adequate
> determine if the contact was lifted off the surface, the device must
> report the last known position of the contact and then deliver the

> “UP” state of the contact in a subsequent report. Devices should not

> send a report without the information for that contact while trying to
> determine its current state.
> """

The text seems to say that devices are not required to send touch
state information in a separate frame, but if the device needs time to
determine the touch state, the touch properties should stay the same
during that time.

> Thus, the quirk ALWAYS_VALID fits very well with win 8 devices (the
> device has to send the touch until it is lifted and out of range, and
> the device must send the 'up' state).

One could simply add another quirk which fits the win8 case exactly
instead. No need to change the existing one.

> The problem lies that some devices may reuse contact id 0 within the
> frame for the end of the report (my Win8 device doesn't has this kind
> of problem):
>
> With the following hid usages:
> I -> contact Id
> T -> tip switch
> X, Y -> X, Y
> S -> scan time
> C -> contact count
>
> a friendly device would report:
>
> I:1 T:1 X:125 X:130 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:0 C:01
> I:1 T:1 X:130 X:135 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:1 C:01
> I:1 T:1 X:135 X:140 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:2 C:01
> I:1 T:1 X:140 X:145 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:3 C:01
> I:1 T:0 X:140 X:145 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:4 C:01
>
> *but*, I've already seen win 7 devices, that do send:
>
> I:0 T:1 X:125 X:130 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:0 C:01
> I:0 T:1 X:130 X:135 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:1 C:01
> I:0 T:1 X:135 X:140 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:2 C:01
> I:0 T:1 X:140 X:145 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:3 C:01
> I:0 T:0 X:140 X:145 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:4 C:01

I see, more brain-damaged usage. :-) Still, there seems to be a
simpler way to distinguish this case: if there are more than one touch
with the same id in the frame, use the one with T=1.

> The difference lies in the first bit, contact id is 0.
>
> So, the quirk always valid is not sufficient because the second touch
> in the frame will override the values of the first (the valid one).
>
> As Microsoft says that "the device must report the last known position

> of the contact and then deliver the “UP” state of the contact", so we

> can safely discard the second touch because X and Y do not match the
> current state of the valid touch.
>
> Then, as exposed in the "How to Design and Test Multitouch Hardware
> Solutions for Windows 8" document here:
> http://msdn.microsoft.com/en-us/library/windows/hardware/hh872968.aspx
> when the device attempt the certification, if the "up" is not valid,
> the error "Last move location different" raises, which, I hope will
> prevent the device to get the certification.

I think it would be too fragile to rely on this assumption. Hopefully
the suggestion above will work equally well.

[Benjamin Tissoires]

On Sun, Nov 4, 2012 at 9:54 PM, Henrik Rydberg <ryd...@euromail.se> wrote:
> Hi Benjamin,
>
>> The goal of this patch is to implement in a reliable way Win 8
>> multitouch protocol (to avoid quirking many devices). Thanks to the
>> precision they made in the specification, I think it is feasible:
>> they add the dynamic part that were missing in Win 7 spec:
>> """
>> When sending data in hybrid or parallel mode, a contact that is
>> delivered in one report must be delivered in all subsequent reports
>> until it is lifted off the screen. If time is needed to adequate
>> determine if the contact was lifted off the surface, the device must
>> report the last known position of the contact and then deliver the
>> “UP” state of the contact in a subsequent report. Devices should not
>> send a report without the information for that contact while trying to
>> determine its current state.
>> """
>
> The text seems to say that devices are not required to send touch
> state information in a separate frame, but if the device needs time to
> determine the touch state, the touch properties should stay the same
> during that time.

Yes and no:
- yes, if the device needs time to determine the "up" state, it should
send at every frame the last known properties.
- no, I never said that the information should be split in separate
frames: I think you are confused by the serial protocol. I'm talking
here about the parallel or the hybrid one.

>
>> Thus, the quirk ALWAYS_VALID fits very well with win 8 devices (the
>> device has to send the touch until it is lifted and out of range, and
>> the device must send the 'up' state).
>
> One could simply add another quirk which fits the win8 case exactly
> instead. No need to change the existing one.

I never changed the existing one. The quirk ALWAYS_VALID means that
whatever the device sends, the information are valid. The fact is that
the serial protocol can be handled with this quirk (otherwise, we
would have called this quirk SERIAL, and not ALWAYS_VALID). So the Win
8 case doesn't need a new quirk, the ALWAYS_VALID fits.

>
>> The problem lies that some devices may reuse contact id 0 within the
>> frame for the end of the report (my Win8 device doesn't has this kind
>> of problem):
>>
>> With the following hid usages:
>> I -> contact Id
>> T -> tip switch
>> X, Y -> X, Y
>> S -> scan time
>> C -> contact count
>>
>> a friendly device would report:
>>
>> I:1 T:1 X:125 X:130 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:0 C:01
>> I:1 T:1 X:130 X:135 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:1 C:01
>> I:1 T:1 X:135 X:140 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:2 C:01
>> I:1 T:1 X:140 X:145 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:3 C:01
>> I:1 T:0 X:140 X:145 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:4 C:01
>>
>> *but*, I've already seen win 7 devices, that do send:
>>
>> I:0 T:1 X:125 X:130 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:0 C:01
>> I:0 T:1 X:130 X:135 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:1 C:01
>> I:0 T:1 X:135 X:140 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:2 C:01
>> I:0 T:1 X:140 X:145 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:3 C:01
>> I:0 T:0 X:140 X:145 Y:750 Y:755 I:0 T:0 X:0 X:0 Y:0 Y:0 S:4 C:01
>
> I see, more brain-damaged usage. :-) Still, there seems to be a
> simpler way to distinguish this case: if there are more than one touch
> with the same id in the frame, use the one with T=1.

sigh... the problem in your sentence is there: "with the same id in the frame".
This forces me to introduce the function input_mt_is_used in mt.h...

>
>> The difference lies in the first bit, contact id is 0.
>>
>> So, the quirk always valid is not sufficient because the second touch
>> in the frame will override the values of the first (the valid one).
>>
>> As Microsoft says that "the device must report the last known position
>> of the contact and then deliver the “UP” state of the contact", so we
>> can safely discard the second touch because X and Y do not match the
>> current state of the valid touch.
>>
>> Then, as exposed in the "How to Design and Test Multitouch Hardware
>> Solutions for Windows 8" document here:
>> http://msdn.microsoft.com/en-us/library/windows/hardware/hh872968.aspx
>> when the device attempt the certification, if the "up" is not valid,
>> the error "Last move location different" raises, which, I hope will
>> prevent the device to get the certification.
>
> I think it would be too fragile to rely on this assumption. Hopefully
> the suggestion above will work equally well.

I'm not sure it is fragile, because otherwise that means that the
certification is worthless.
But anyway, your solution is valid too, so I'll implement it.

Cheers,
Benjamin

[Henrik Rydberg]

Hi Benjamin,

> >> This patch factorizes the hid set_feature command by using
> >> hid_device->hid_output_raw_report instead of direclty relying on
> >> usbhid. This makes the driver usb independant.
> >>
> >> However I still can't remove the 2 usb related headers because the
> >> function mt_resume has a specific patch for usb devices.
> >>
> >> Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
> >> ---
> >> drivers/hid/hid-multitouch.c | 63 ++++++++++++++++++++++++++------------------
> >> 1 file changed, 37 insertions(+), 26 deletions(-)
> >
> > In my drawer, I have a patchset that aims to remove all usbhid
> > dependence, from all the drivers. Perhaps the attached patch is
> > something to consider here?
>
> yep, removing usbhid dependencies is a good thing.
> See my review below :)

I have a tentative patch taking your comments into account, and it is
likely that we want to go that way. However, as to not hold up your
patchset, perhaps we could do without it for now.

Regarding the hardwired usbhid dependency, I think the solution is to
move that code to usbhid itself.

Thanks,
Henrik

[Benjamin Tissoires]

Hi Henrik,

[Benjamin Tissoires]

Hi Henrik,

grrr.... damn new gmail interface, I clicked on the wrong button.

sorry for the noise.

On Mon, Nov 5, 2012 at 2:28 PM, Benjamin Tissoires
<benjamin....@gmail.com> wrote:
> Hi Henrik,
>
> On Mon, Nov 5, 2012 at 1:57 PM, Henrik Rydberg <ryd...@euromail.se> wrote:
>> Hi Benjamin,
>>
>>> >> This patch factorizes the hid set_feature command by using
>>> >> hid_device->hid_output_raw_report instead of direclty relying on
>>> >> usbhid. This makes the driver usb independant.
>>> >>
>>> >> However I still can't remove the 2 usb related headers because the
>>> >> function mt_resume has a specific patch for usb devices.
>>> >>
>>> >> Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
>>> >> ---
>>> >> drivers/hid/hid-multitouch.c | 63 ++++++++++++++++++++++++++------------------
>>> >> 1 file changed, 37 insertions(+), 26 deletions(-)
>>> >
>>> > In my drawer, I have a patchset that aims to remove all usbhid
>>> > dependence, from all the drivers. Perhaps the attached patch is
>>> > something to consider here?
>>>
>>> yep, removing usbhid dependencies is a good thing.
>>> See my review below :)
>>
>> I have a tentative patch taking your comments into account, and it is
>> likely that we want to go that way. However, as to not hold up your
>> patchset, perhaps we could do without it for now.

so, Yes, this is not my blocking patch that prevents me from sending
the new patchset. I intend to let you clean this up with your new
patch.
The v3 is on it's way!

>>
>> Regarding the hardwired usbhid dependency, I think the solution is to
>> move that code to usbhid itself.

yes, maybe, but at the beginning, we didn't want to patch it that way
because it was only specific to one hid-multitouch device (though
armless for the other multitouch devices).
So maybe you will have to add a quirk in usbhid or sth like that.

Cheers,
Benjamin

[Benjamin Tissoires]

On Tue, Oct 30, 2012 at 11:09 AM, Benjamin Tissoires
<benjamin....@gmail.com> wrote:
> On Mon, Oct 29, 2012 at 8:25 PM, Henrik Rydberg <ryd...@euromail.se> wrote:
>> Hi Benjamin,
>>
>>> Currently, there is no way to know the index of the current field
>>> in the .input_mapping and .event callbacks when this field is inside
>>> an array of HID fields.
>>> This patch forwards this index to the input_mapping and event
>>> callbacks.
>>
>> I agree with the idea, but the function argument list is becoming
>> ridiculously long... Could we remove the usage pointer argument, at
>> least?
>
> yeah, totally agree. Let me just check whether it will not introduce
> more problems than it solves for my driver.

Well, after a deeper look, it's a really bad idea. Every drivers that
implements input_mapping, input_mapped, event use at least once the
struct usage. So this would require change nearly every hid driver.

To solve that, and to minimize the impact on the other drivers, I'm
going to add a field in struct usage with the appropriate index.

Cheers,
Benjamin

[Jiri Kosina]

On Tue, 6 Nov 2012, Benjamin Tissoires wrote:

> >>> Currently, there is no way to know the index of the current field
> >>> in the .input_mapping and .event callbacks when this field is inside
> >>> an array of HID fields.
> >>> This patch forwards this index to the input_mapping and event
> >>> callbacks.
> >>
> >> I agree with the idea, but the function argument list is becoming
> >> ridiculously long... Could we remove the usage pointer argument, at
> >> least?
> >
> > yeah, totally agree. Let me just check whether it will not introduce
> > more problems than it solves for my driver.
>
> Well, after a deeper look, it's a really bad idea. Every drivers that
> implements input_mapping, input_mapped, event use at least once the
> struct usage. So this would require change nearly every hid driver.
>
> To solve that, and to minimize the impact on the other drivers, I'm
> going to add a field in struct usage with the appropriate index.

Yes please, I like that solution more as well.

Thanks,

--
Jiri Kosina
SUSE Labs

[Benjamin Tissoires]

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....@gmail.com>
---

drivers/hid/hid-multitouch.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index b393c6c..1f3d1e0 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -59,6 +59,7 @@ struct mt_slot {

__s32 x, y, cx, cy, p, w, h;

__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? */
};

struct mt_class {
@@ -397,6 +398,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, -1, 1, 0, 0);
+ }

mt_store_field(usage, td, hi);
td->last_field_index = field->index;
return 1;

@@ -516,6 +523,10 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)

if (slotnum < 0 || slotnum >= td->maxcontacts)
return;

+ if (!test_bit(ABS_MT_DISTANCE, input->absbit))

+ /* If InRange is not present, rely on TipSwitch */

+ s->inrange_state = s->touch_state;
+
if (td->mtclass.quirks & MT_QUIRK_IGNORE_DUPLICATES &&
input_mt_is_active(&input->mt->slots[slotnum]) &&
input_mt_is_used(input->mt, &input->mt->slots[slotnum]))
@@ -523,9 +534,9 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)

input_mt_slot(input, slotnum);
input_mt_report_slot_state(input, MT_TOOL_FINGER,
- s->touch_state);
- if (s->touch_state) {
- /* this finger is on the screen */
+ s->inrange_state);
+ if (s->inrange_state) {
+ /* this finger is in proximity of the sensor */
int wide = (s->w > s->h);
/* divided by two to match visual scale of touch */
int major = max(s->w, s->h) >> 1;
@@ -535,11 +546,14 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
input_event(input, EV_ABS, ABS_MT_POSITION_Y, s->y);
input_event(input, EV_ABS, ABS_MT_TOOL_X, s->cx);

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_PRESSURE, s->p);
input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);

input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
- }
+ } else
+ input_event(input, EV_ABS, ABS_MT_DISTANCE, -1);
}

td->num_received++;
@@ -567,6 +581,8 @@ 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.inrange_state = value;

break;
case HID_DG_TIPSWITCH:
if (quirks & MT_QUIRK_NOT_SEEN_MEANS_UP)

--
1.7.11.7

[Benjamin Tissoires]

this prevents a device to reuse contact id 0 several time when
sending garbage values to complete the report.

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

drivers/hid/hid-multitouch.c | 6 ++++++
include/linux/input/mt.h | 6 ++++++
2 files changed, 12 insertions(+)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 2352770..351c814 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -52,6 +52,7 @@ MODULE_LICENSE("GPL");

#define MT_QUIRK_VALID_IS_CONFIDENCE (1 << 6)
#define MT_QUIRK_SLOT_IS_CONTACTID_MINUS_ONE (1 << 8)
#define MT_QUIRK_NO_AREA (1 << 9)

+#define MT_QUIRK_IGNORE_DUPLICATES (1 << 10)

struct mt_slot {
__s32 x, y, cx, cy, p, w, h;

@@ -510,6 +511,11 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)

if (slotnum < 0 || slotnum >= td->maxcontacts)
return;

+ if (td->mtclass.quirks & MT_QUIRK_IGNORE_DUPLICATES &&

+ input_mt_is_active(&input->mt->slots[slotnum]) &&
+ input_mt_is_used(input->mt, &input->mt->slots[slotnum]))
+ return;
+
input_mt_slot(input, slotnum);
input_mt_report_slot_state(input, MT_TOOL_FINGER,
s->touch_state);
diff --git a/include/linux/input/mt.h b/include/linux/input/mt.h
index cc5cca7..2e86bd0 100644
--- a/include/linux/input/mt.h
+++ b/include/linux/input/mt.h
@@ -69,6 +69,12 @@ static inline bool input_mt_is_active(const struct input_mt_slot *slot)
return input_mt_get_value(slot, ABS_MT_TRACKING_ID) >= 0;
}

+static inline bool input_mt_is_used(const struct input_mt *mt,
+ const struct input_mt_slot *slot)
+{
+ return slot->frame == mt->frame;
+}
+
int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots,
unsigned int flags);
void input_mt_destroy_slots(struct input_dev *dev);

[Benjamin Tissoires]

Win 8 digitizer devices provides the actual scan time computed by the
hardware itself. The value is global to the frame and is not specific
to the multitouch protocol (though only touch, not pen, should use it
according to the specification).

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

Documentation/input/event-codes.txt | 9 +++++++++
drivers/hid/hid-input.c | 4 ++++

include/linux/hid.h | 1 +
include/linux/input.h | 1 +

4 files changed, 15 insertions(+)

diff --git a/Documentation/input/event-codes.txt b/Documentation/input/event-codes.txt
index 53305bd..80c06e5 100644
--- a/Documentation/input/event-codes.txt
+++ b/Documentation/input/event-codes.txt
@@ -174,6 +174,15 @@ A few EV_ABS codes have special meanings:

the input device may be used freely in three dimensions, consider ABS_Z
instead.

+* ABS_SCAN_TIME:

+ - Used to report the number of microseconds since the last reset. This event
+ should be coded as an uint32 value, which is allowed to wrap around with
+ no special consequence. It is assumed that the time difference between two
+ consecutive events is reliable on a reasonable time scale (hours).
+ A reset to zero can happen, in which case the time since the last event is
+ unknown. If the device does not provide this information, the driver must
+ not provide it to the user space.

+
* ABS_MT_<name>:
- Used to describe multitouch input events. Please see
multi-touch-protocol.txt for details.
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c

index 7015080..1c96238 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -677,6 +677,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel

map_key_clear(BTN_STYLUS2);
break;

+ case 0x56: /* Scan Time */
+ map_abs(ABS_SCAN_TIME);
+ break;
+

default: goto unknown;
}
break;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 6b4f322..0337e50 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h

@@ -279,6 +279,7 @@ struct hid_item {
#define HID_DG_DEVICEINDEX 0x000d0053
#define HID_DG_CONTACTCOUNT 0x000d0054
#define HID_DG_CONTACTMAX 0x000d0055
+#define HID_DG_SCANTIME 0x000d0056

/*
* HID report types --- Ouch! HID spec says 1 2 3!
diff --git a/include/linux/input.h b/include/linux/input.h
index ba48743..73c3a96 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -796,6 +796,7 @@ struct input_keymap_entry {
#define ABS_TILT_X 0x1a
#define ABS_TILT_Y 0x1b
#define ABS_TOOL_WIDTH 0x1c
+#define ABS_SCAN_TIME 0x1d

#define ABS_VOLUME 0x20

[Benjamin Tissoires]

Win 8 device specification changed the requirements for the hid usages
of the multitouch devices. Now InRange is optional and must be only
used when the device supports hovering.

This ensures that the quirk ALWAYS_VALID is taken into account and
also ensures its precedence over the other VALID* quirks.

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

drivers/hid/hid-multitouch.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 54367f4..2352770 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -503,7 +503,7 @@ static int mt_compute_slot(struct mt_device *td, struct input_dev *input)
*/

static void mt_complete_slot(struct mt_device *td, struct input_dev *input)

{
- if (td->curvalid) {
+ if (td->curvalid || (td->mtclass.quirks & MT_QUIRK_ALWAYS_VALID)) {

int slotnum = mt_compute_slot(td, input);

struct mt_slot *s = &td->curdata;

@@ -554,9 +554,7 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,

if (hid->claimed & HID_CLAIMED_INPUT) {

switch (usage->hid) {
case HID_DG_INRANGE:

- if (quirks & MT_QUIRK_ALWAYS_VALID)
- td->curvalid = true;
- else if (quirks & MT_QUIRK_VALID_IS_INRANGE)

+ if (quirks & MT_QUIRK_VALID_IS_INRANGE)
td->curvalid = value;
break;
case HID_DG_TIPSWITCH:

[Benjamin Tissoires]

Computes the scan time according to the specification.
It also ensures that if the time between two events is greater
than MAX_SCAN_INTERVAL, the scan time will be reset.

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

drivers/hid/hid-multitouch.c | 45 ++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 1f3d1e0..5902567 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -32,6 +32,7 @@
#include <linux/slab.h>
#include <linux/usb.h>
#include <linux/input/mt.h>
+#include <linux/jiffies.h>
#include "usbhid/usbhid.h"


@@ -98,6 +99,9 @@ struct mt_device {
bool serial_maybe; /* need to check for serial protocol */
bool curvalid; /* is the current contact valid? */
unsigned mt_flags; /* flags to pass to input-mt */
+ __s32 dev_time; /* the scan time provided by the device */
+ unsigned long jiffies; /* the frame's jiffies */
+ unsigned scan_time; /* the scan time to be sent */
};

/* classes of device behavior */
@@ -126,6 +130,9 @@ struct mt_device {
#define MT_DEFAULT_MAXCONTACT 10
#define MT_MAX_MAXCONTACT 250

+#define MAX_SCAN_TIME INT_MAX
+#define MAX_SCAN_INTERVAL 500000
+
#define MT_USB_DEVICE(v, p) HID_DEVICE(BUS_USB, HID_GROUP_MULTITOUCH, v, p)
#define MT_BT_DEVICE(v, p) HID_DEVICE(BUS_BLUETOOTH, HID_GROUP_MULTITOUCH, v, p)

@@ -451,12 +458,20 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,

mt_store_field(usage, td, hi);
td->last_field_index = field->index;
return 1;

+ case HID_DG_SCANTIME:

+ hid_map_usage(hi, usage, bit, max,

+ EV_ABS, ABS_SCAN_TIME);
+ input_set_abs_params(hi->input,
+ ABS_SCAN_TIME, 0, MAX_SCAN_TIME, 0, 0);

+ td->last_field_index = field->index;
+ return 1;
case HID_DG_CONTACTCOUNT:

td->last_field_index = field->index;
return 1;

case HID_DG_CONTACTMAX:
- /* we don't set td->last_slot_field as contactcount and
- * contact max are global to the report */
+ /* we don't set td->last_slot_field as scan time,
+ * contactcount and contact max are global to the
+ * report */

td->last_field_index = field->index;

return -1;
}
@@ -565,11 +580,34 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
*/
static void mt_sync_frame(struct mt_device *td, struct input_dev *input)
{
+ input_event(input, EV_ABS, ABS_SCAN_TIME, td->scan_time);
input_mt_sync_frame(input);
input_sync(input);
td->num_received = 0;
}

+static void mt_compute_scan_time(struct mt_device *td, struct hid_field *field,
+ __s32 value)
+{
+ long delta = value - td->dev_time;
+ unsigned long jdelta = jiffies_to_usecs(jiffies - td->jiffies);
+
+ td->jiffies = jiffies;
+ td->dev_time = value;
+
+ if (delta < 0)
+ delta += field->logical_maximum;
+
+ /* HID_DG_SCANTIME is expressed in 100us, we want it in ms. */
+ delta *= 100;
+
+ if (abs(delta - jdelta) > MAX_SCAN_INTERVAL)
+ /* obviously wrong clock -> the device time has been reset */
+ td->scan_time = 0;
+ else
+ td->scan_time += delta;
+}
+

static int mt_event(struct hid_device *hid, struct hid_field *field,

struct hid_usage *usage, __s32 value)
{
@@ -617,6 +655,9 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
case HID_DG_HEIGHT:
td->curdata.h = value;
break;
+ case HID_DG_SCANTIME:
+ mt_compute_scan_time(td, field, value);
+ break;
case HID_DG_CONTACTCOUNT:
/*
* Includes multi-packet support where subsequent

[Benjamin Tissoires]

Win 8 specification is much more precise than the Win 7 one.
Moreover devices that need to take certification must be submitted
to Microsoft.

The result is a better protocol support and we can rely on that to
skip all the messy tests we used to do.

The protocol specify the fact that each valid touch must be reported

within a frame until it is released.
So we can use the always_valid quirk and dismiss reports when we see
duplicates contact ID.

We recognize Win8 certified devices from their vendor feature 0xff0000c5
where Microsoft put a signed blob in the report to check if the device
passed the certification.

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

drivers/hid/hid-multitouch.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 351c814..b393c6c 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -53,6 +53,7 @@ MODULE_LICENSE("GPL");

#define MT_QUIRK_SLOT_IS_CONTACTID_MINUS_ONE (1 << 8)
#define MT_QUIRK_NO_AREA (1 << 9)

#define MT_QUIRK_IGNORE_DUPLICATES (1 << 10)
+#define MT_QUIRK_WIN_8_CERTIFIED (1 << 11)

struct mt_slot {
__s32 x, y, cx, cy, p, w, h;

@@ -293,6 +294,10 @@ static void mt_feature_mapping(struct hid_device *hdev,

td->maxcontacts = td->mtclass.maxcontacts;

break;
+ case 0xff0000c5:
+ if (field->report_count == 256 && field->report_size == 8)
+ td->mtclass.quirks |= MT_QUIRK_WIN_8_CERTIFIED;
+ break;
}
}

@@ -679,14 +684,17 @@ static void mt_post_parse_default_settings(struct mt_device *td)

{
__s32 quirks = td->mtclass.quirks;

- /* unknown serial device needs special quirks */
- if (td->touches_by_report == 1) {
+ /* unknown serial devices or win8 ones need special quirks */

+ if (td->touches_by_report == 1 || (quirks & MT_QUIRK_WIN_8_CERTIFIED)) {

quirks |= MT_QUIRK_ALWAYS_VALID;
quirks &= ~MT_QUIRK_NOT_SEEN_MEANS_UP;
quirks &= ~MT_QUIRK_VALID_IS_INRANGE;

quirks &= ~MT_QUIRK_VALID_IS_CONFIDENCE;

}

+ if (quirks & MT_QUIRK_WIN_8_CERTIFIED)

+ quirks |= MT_QUIRK_IGNORE_DUPLICATES;
+
td->mtclass.quirks = quirks;

[Benjamin Tissoires]

Win8 input specification clarifies the X and Y sent by devices.
It distincts the position where the user wants to Touch (T) from
the center of the ellipsoide (C). This patch enable supports for this
distinction in hid-multitouch.

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

drivers/hid/hid-multitouch.c | 46 +++++++++++++++++++++++++++++++++++---------
1 file changed, 37 insertions(+), 9 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index ae57b8f..54367f4 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -54,7 +54,7 @@ MODULE_LICENSE("GPL");
#define MT_QUIRK_NO_AREA (1 << 9)

struct mt_slot {
- __s32 x, y, p, w, h;
+ __s32 x, y, cx, cy, p, w, h;

__s32 contactid; /* the device ContactID assigned to this slot */
bool touch_state; /* is the touch valid? */

};
@@ -323,6 +323,7 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,

struct mt_device *td = hid_get_drvdata(hdev);

struct mt_class *cls = &td->mtclass;
int code;
+ struct hid_usage *prev_usage = NULL;

/* Only map fields from TouchScreen or TouchPad collections.
* We need to ignore fields that belong to other collections
@@ -345,23 +346,42 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
if (field->physical == HID_DG_STYLUS)
return -1;

+ if (usage->usage_index)
+ prev_usage = &field->usage[usage->usage_index - 1];
+
switch (usage->hid & HID_USAGE_PAGE) {

case HID_UP_GENDESK:
switch (usage->hid) {
case HID_GD_X:
- hid_map_usage(hi, usage, bit, max,
+ if (prev_usage && (prev_usage->hid == usage->hid)) {

+ hid_map_usage(hi, usage, bit, max,

+ EV_ABS, ABS_MT_TOOL_X);
+ set_abs(hi->input, ABS_MT_TOOL_X, field,
+ cls->sn_move);
+ } else {

+ hid_map_usage(hi, usage, bit, max,

EV_ABS, ABS_MT_POSITION_X);
- set_abs(hi->input, ABS_MT_POSITION_X, field,
- cls->sn_move);
+ set_abs(hi->input, ABS_MT_POSITION_X, field,
+ cls->sn_move);
+ }
+

mt_store_field(usage, td, hi);
td->last_field_index = field->index;
return 1;

case HID_GD_Y:
- hid_map_usage(hi, usage, bit, max,
+ if (prev_usage && (prev_usage->hid == usage->hid)) {

+ hid_map_usage(hi, usage, bit, max,

+ EV_ABS, ABS_MT_TOOL_Y);
+ set_abs(hi->input, ABS_MT_TOOL_Y, field,

+ cls->sn_move);
+ } else {

+ hid_map_usage(hi, usage, bit, max,

EV_ABS, ABS_MT_POSITION_Y);
- set_abs(hi->input, ABS_MT_POSITION_Y, field,
- cls->sn_move);
+ set_abs(hi->input, ABS_MT_POSITION_Y, field,

+ cls->sn_move);
+ }
+

mt_store_field(usage, td, hi);
td->last_field_index = field->index;
return 1;

@@ -502,6 +522,8 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)

input_event(input, EV_ABS, ABS_MT_POSITION_X, s->x);

input_event(input, EV_ABS, ABS_MT_POSITION_Y, s->y);

+ input_event(input, EV_ABS, ABS_MT_TOOL_X, s->cx);
+ input_event(input, EV_ABS, ABS_MT_TOOL_Y, s->cy);

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);

@@ -553,10 +575,16 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
td->curdata.p = value;
break;
case HID_GD_X:
- td->curdata.x = value;
+ if (usage->code == ABS_MT_TOOL_X)
+ td->curdata.cx = value;
+ else
+ td->curdata.x = value;
break;
case HID_GD_Y:
- td->curdata.y = value;
+ if (usage->code == ABS_MT_TOOL_Y)
+ td->curdata.cy = value;
+ else
+ td->curdata.y = value;
break;
case HID_DG_WIDTH:
td->curdata.w = value;

[Benjamin Tissoires]

Currently, there is no way to know the index of the current field
in the .input_mapping and .event callbacks when this field is inside
an array of HID fields.

This patch adds this index to the struct hid_usage so that this
information is available to input_mapping and event callbacks.

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

drivers/hid/hid-core.c | 4 ++++
include/linux/hid.h | 1 +
2 files changed, 5 insertions(+)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 9da3007..8f82c4c 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -92,6 +92,7 @@ EXPORT_SYMBOL_GPL(hid_register_report);
static struct hid_field *hid_register_field(struct hid_report *report, unsigned usages, unsigned values)
{
struct hid_field *field;
+ int i;

if (report->maxfield == HID_MAX_FIELDS) {
hid_err(report->device, "too many fields in report\n");
@@ -110,6 +111,9 @@ static struct hid_field *hid_register_field(struct hid_report *report, unsigned
field->value = (s32 *)(field->usage + usages);
field->report = report;

+ for (i = 0; i < usages; i++)
+ field->usage[i].usage_index = i;
+
return field;
}

diff --git a/include/linux/hid.h b/include/linux/hid.h
index a110a3e..6b4f322 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -380,6 +380,7 @@ struct hid_usage {
unsigned hid; /* hid usage code */
unsigned collection_index; /* index into collection array */
/* hidinput data */
+ unsigned usage_index; /* index into usage array */
__u16 code; /* input driver code */
__u8 type; /* input driver type */
__s8 hat_min; /* hat switch fun */

[Benjamin Tissoires]

Exporting the function allows us to calculate the resolution in third
party drivers like hid-multitouch.
This patch also complete the function with additional valid axes.

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

drivers/hid/hid-input.c | 9 ++++++++-
drivers/hid/hid-multitouch.c | 1 +
include/linux/hid.h | 1 +
3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 10248cf..f5b1d57 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -208,7 +208,7 @@ static int hidinput_setkeycode(struct input_dev *dev,
* Only exponent 1 length units are processed. Centimeters and inches are
* converted to millimeters. Degrees are converted to radians.
*/
-static __s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code)
+__s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code)
{
__s32 unit_exponent = field->unit_exponent;
__s32 logical_extents = field->logical_maximum -
@@ -229,6 +229,12 @@ static __s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code)
case ABS_X:
case ABS_Y:
case ABS_Z:
+ case ABS_MT_POSITION_X:
+ case ABS_MT_POSITION_Y:
+ case ABS_MT_TOOL_X:
+ case ABS_MT_TOOL_Y:
+ case ABS_MT_TOUCH_MAJOR:
+ case ABS_MT_TOUCH_MINOR:
if (field->unit == 0x11) { /* If centimeters */
/* Convert to millimeters */
unit_exponent += 1;
@@ -283,6 +289,7 @@ static __s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code)
/* Calculate resolution */
return logical_extents / physical_extents;
}
+EXPORT_SYMBOL_GPL(hidinput_calc_abs_res);

#ifdef CONFIG_HID_BATTERY_STRENGTH
static enum power_supply_property hidinput_battery_props[] = {
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 3eb02b9..9f64e36 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -298,6 +298,7 @@ static void set_abs(struct input_dev *input, unsigned int code,
int fmax = field->logical_maximum;
int fuzz = snratio ? (fmax - fmin) / snratio : 0;
input_set_abs_params(input, code, fmin, fmax, fuzz, 0);
+ input_abs_set_res(input, code, hidinput_calc_abs_res(field, code));
}

static void mt_store_field(struct hid_usage *usage, struct mt_device *td,
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 7e1f37d..9edb06c 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -743,6 +743,7 @@ int hid_input_report(struct hid_device *, int type, u8 *, int, int);
int hidinput_find_field(struct hid_device *hid, unsigned int type, unsigned int code, struct hid_field **field);
struct hid_field *hidinput_get_led_field(struct hid_device *hid);
unsigned int hidinput_count_leds(struct hid_device *hid);
+__s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code);
void hid_output_report(struct hid_report *report, __u8 *data);
struct hid_device *hid_allocate_device(void);
struct hid_report *hid_register_report(struct hid_device *device, unsigned type, unsigned id);

[Benjamin Tissoires]

Hi Guys,

here is the third version of this patchset.
I think I included most of Henrik's comments.

Happy reviewing :)

Cheers,
Benjamin

v1 introduction:
So, this is an update for supporting Win 8 multitouch devices in the kernel.
As I wanted to reliably forward the resolution, I noticed a bug in the
processing of the unit_exponent in the hid core layer.
Thus the fixes for hid-core and hid-input.

v2 changes:
* added missing initial patch that prevents the series to be applied on top of Jiri's tree
* update to include latest hid changes
* taken into account Alan's patch: "hid: put the case in the right switch statement"

v3 changes:
* splitted "round return value of hidinput_calc_abs_res" in a separate patch
* export snto32 in hid.h as we need to use it in hid-input.c
* didn't change all drivers, but add a field in hid_usage instead
* add quirk MT_QUIRK_IGNORE_DUPLICATES so that any device can rely on it
* easier understandable support of hovering devices
* changed scan time definition
* applied new definition of scan time in hid-multitouch
* some other few things.

Benjamin Tissoires (13):
HID: hid-input: export hidinput_calc_abs_res
HID: hid-input: round return value of hidinput_calc_abs_res
HID: core: fix unit exponent parsing
HID: hid-input: add usage_index in struct hid_usage.
HID: hid-multitouch: support arrays for the split of the touches in a
report
HID: hid-multitouch: get maxcontacts also from logical_max value
HID: hid-multitouch: support T and C for win8 devices
HID: hid-multitouch: move ALWAYS_VALID quirk check
HID: hid-multitouch: add MT_QUIRK_IGNORE_DUPLICATES
HID: hid-multitouch: fix Win 8 protocol
HID: hid-multitouch: support for hovering devices
HID: introduce Scan Time
HID: hid-multitouch: forwards ABS_SCAN_TIME

Documentation/input/event-codes.txt | 9 +++
drivers/hid/hid-core.c | 20 ++++-
drivers/hid/hid-input.c | 28 +++++--
drivers/hid/hid-multitouch.c | 157 ++++++++++++++++++++++++++++++------
include/linux/hid.h | 4 +
include/linux/input.h | 1 +
include/linux/input/mt.h | 6 ++
7 files changed, 192 insertions(+), 33 deletions(-)

[Benjamin Tissoires]

HID spec details special values for the HID field unit exponent.
Basically, the range [0x8..0xf] correspond to [-8..-1], so this is
a standard two's complement on a half-byte.

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

drivers/hid/hid-core.c | 16 +++++++++++++++-
drivers/hid/hid-input.c | 13 +++++++++----
include/linux/hid.h | 1 +
3 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 3d0da29..9da3007 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -315,6 +315,7 @@ static s32 item_sdata(struct hid_item *item)

static int hid_parser_global(struct hid_parser *parser, struct hid_item *item)
{
+ __u32 raw_value;
switch (item->tag) {
case HID_GLOBAL_ITEM_TAG_PUSH:

@@ -365,7 +366,14 @@ static int hid_parser_global(struct hid_parser *parser, struct hid_item *item)
return 0;

case HID_GLOBAL_ITEM_TAG_UNIT_EXPONENT:
- parser->global.unit_exponent = item_sdata(item);
+ /* Units exponent negative numbers are given through a
+ * two's complement.
+ * See "6.2.2.7 Global Items" for more information. */
+ raw_value = item_udata(item);
+ if (!(raw_value & 0xfffffff0))
+ parser->global.unit_exponent = hid_snto32(raw_value, 4);
+ else
+ parser->global.unit_exponent = raw_value;
return 0;

case HID_GLOBAL_ITEM_TAG_UNIT:
@@ -865,6 +873,12 @@ static s32 snto32(__u32 value, unsigned n)
return value & (1 << (n - 1)) ? value | (-1 << n) : value;
}

+s32 hid_snto32(__u32 value, unsigned n)
+{
+ return snto32(value, n);
+}
+EXPORT_SYMBOL_GPL(hid_snto32);
+
/*
* Convert a signed 32-bit integer to a signed n-bit integer.
*/
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 67044f3..7015080 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -192,7 +192,6 @@ static int hidinput_setkeycode(struct input_dev *dev,
return -EINVAL;
}

-
/**
* hidinput_calc_abs_res - calculate an absolute axis resolution
* @field: the HID report field to calculate resolution for
@@ -235,17 +234,23 @@ __s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code)
case ABS_MT_TOOL_Y:
case ABS_MT_TOUCH_MAJOR:
case ABS_MT_TOUCH_MINOR:
- if (field->unit == 0x11) { /* If centimeters */
+ if (field->unit & 0xffffff00) /* Not a length */
+ return 0;
+ unit_exponent += hid_snto32(field->unit >> 4, 4) - 1;
+ switch (field->unit & 0xf) {
+ case 0x1: /* If centimeters */

/* Convert to millimeters */
unit_exponent += 1;

- } else if (field->unit == 0x13) { /* If inches */
+ break;
+ case 0x3: /* If inches */

/* Convert to millimeters */

prev = physical_extents;
physical_extents *= 254;
if (physical_extents < prev)
return 0;
unit_exponent -= 1;
- } else {
+ break;
+ default:
return 0;
}
break;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 9edb06c..a110a3e 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -754,6 +754,7 @@ int hid_connect(struct hid_device *hid, unsigned int connect_mask);
void hid_disconnect(struct hid_device *hid);
const struct hid_device_id *hid_match_id(struct hid_device *hdev,
const struct hid_device_id *id);
+s32 hid_snto32(__u32 value, unsigned n);

/**
* hid_map_usage - map usage input bits

[Benjamin Tissoires]

Win8 devices are required to present the feature "Maximum Contact Number".
Fortunately all win7 devices I've seen presents this feature.
If the current value is 0, then, the driver can get the actual supported
contact count by refering to the logical_max.
This win8 specification ensures that logical_max may not be above 250.
This also allows us to detect when devices like irtouch or stantum reports
an obviously wrong value of 255.

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
Acked-by: Henrik Rydberg <ryd...@euromail.se>
---
drivers/hid/hid-multitouch.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index a6a4e0a..ae57b8f 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -121,6 +121,7 @@ struct mt_device {
#define MT_CLS_GENERALTOUCH_PWT_TENFINGERS 0x0109

#define MT_DEFAULT_MAXCONTACT 10
+#define MT_MAX_MAXCONTACT 250

#define MT_USB_DEVICE(v, p) HID_DEVICE(BUS_USB, HID_GROUP_MULTITOUCH, v, p)
#define MT_BT_DEVICE(v, p) HID_DEVICE(BUS_BLUETOOTH, HID_GROUP_MULTITOUCH, v, p)

@@ -283,6 +284,9 @@ static void mt_feature_mapping(struct hid_device *hdev,
case HID_DG_CONTACTMAX:
td->maxcontact_report_id = field->report->id;
td->maxcontacts = field->value[0];
+ if (!td->maxcontacts &&
+ field->logical_maximum <= MT_MAX_MAXCONTACT)
+ td->maxcontacts = field->logical_maximum;
if (td->mtclass.maxcontacts)
/* check if the maxcontacts is given by the class */

td->maxcontacts = td->mtclass.maxcontacts;

[Benjamin Tissoires]

Win8 certification introduced the ability to transmit two X and two Y per
touch. The specification precises that it must be used in an array.

This test guarantees that we split the touches on the last element
in this array.

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

drivers/hid/hid-multitouch.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 9f64e36..a6a4e0a 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -577,12 +577,15 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
return 0;
}

- if (usage->hid == td->last_slot_field)
- mt_complete_slot(td, field->hidinput->input);
-
- if (field->index == td->last_field_index
- && td->num_received >= td->num_expected)
- mt_sync_frame(td, field->hidinput->input);
+ if (usage->usage_index + 1 == field->report_count) {
+ /* we only take into account the last report. */
+ if (usage->hid == td->last_slot_field)
+ mt_complete_slot(td, field->hidinput->input);
+
+ if (field->index == td->last_field_index
+ && td->num_received >= td->num_expected)
+ mt_sync_frame(td, field->hidinput->input);
+ }

[Benjamin Tissoires]

hidinput_calc_abs_res should return the closest int in the division
instead of the floor.
On a device with a logical_max of 3008 and a physical_max of 255mm,
previous implementation gave a resolution of 11 instead of 12.
With 11, user-space computes a physical size of 273.5mm and the
round_closest results gives 250.6mm.
The old implementation introduced an error of 2cm in this example.

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

drivers/hid/hid-input.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index f5b1d57..67044f3 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -287,7 +287,7 @@ __s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code)
}

/* Calculate resolution */
- return logical_extents / physical_extents;
+ return DIV_ROUND_CLOSEST(logical_extents, physical_extents);
}
EXPORT_SYMBOL_GPL(hidinput_calc_abs_res);

[Dmitry Torokhov]

Hi Benjamin,

This should not be an absolute event but rather EV_MSC/MSC_TIMESTAMP.

Thanks.

--
Dmitry

[Jiri Kosina]

On Wed, 7 Nov 2012, Benjamin Tissoires wrote:

> Currently, there is no way to know the index of the current field
> in the .input_mapping and .event callbacks when this field is inside
> an array of HID fields.
> This patch adds this index to the struct hid_usage so that this
> information is available to input_mapping and event callbacks.
>
> Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>

Acked-by: Jiri Kosina <jko...@suse.cz>

Jiri Kosina
SUSE Labs

[Jiri Kosina]

On Wed, 7 Nov 2012, Benjamin Tissoires wrote:

> HID spec details special values for the HID field unit exponent.
> Basically, the range [0x8..0xf] correspond to [-8..-1], so this is
> a standard two's complement on a half-byte.
>
> Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>

Acked-by: Jiri Kosina <jko...@suse.cz>

Jiri Kosina
SUSE Labs

[Jiri Kosina]

On Wed, 7 Nov 2012, Benjamin Tissoires wrote:

> hidinput_calc_abs_res should return the closest int in the division
> instead of the floor.
> On a device with a logical_max of 3008 and a physical_max of 255mm,
> previous implementation gave a resolution of 11 instead of 12.
> With 11, user-space computes a physical size of 273.5mm and the
> round_closest results gives 250.6mm.
> The old implementation introduced an error of 2cm in this example.
>
> Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>

Acked-by: Jiri Kosina <jko...@suse.cz>

> ---
> drivers/hid/hid-input.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index f5b1d57..67044f3 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -287,7 +287,7 @@ __s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code)
> }
>
> /* Calculate resolution */
> - return logical_extents / physical_extents;
> + return DIV_ROUND_CLOSEST(logical_extents, physical_extents);
> }
> EXPORT_SYMBOL_GPL(hidinput_calc_abs_res);
>
> --
> 1.7.11.7
>

--

Jiri Kosina
SUSE Labs

[Jiri Kosina]

On Wed, 7 Nov 2012, Benjamin Tissoires wrote:

> Exporting the function allows us to calculate the resolution in third
> party drivers like hid-multitouch.
> This patch also complete the function with additional valid axes.
>
> Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>

Acked-by: Jiri Kosina <jko...@suse.cz>

Jiri Kosina
SUSE Labs

[Jiri Kosina]

On Wed, 7 Nov 2012, Benjamin Tissoires wrote:

Benjamin,

thanks for the patchset.

I am fine with the HID-specific changes (and I have sent out my Acks to
the invidivual patches).

For hid-multitouch changes, I'd like to have Reviewed-by: from Henrik as
well. Henrik, are you planning to review this patchset, please?

Also the note from Dmitry on turning ABS_SCAN_TIME into
EV_MSC/MSC_TIMESTAMP should be addressed.

Once this is finalized, I'd like to queue this for 3.8 in my tree.

Thanks,

--
Jiri Kosina
SUSE Labs

[Benjamin Tissoires]

Hi Jiri,

Thanks :)

>
> For hid-multitouch changes, I'd like to have Reviewed-by: from Henrik as
> well. Henrik, are you planning to review this patchset, please?
>
> Also the note from Dmitry on turning ABS_SCAN_TIME into
> EV_MSC/MSC_TIMESTAMP should be addressed.

Yes, sure. I'll do it today.

>
> Once this is finalized, I'd like to queue this for 3.8 in my tree.

Thanks,

Benjamin

[Benjamin Tissoires]

Hi Dmitry,

alright.
However, since we are using EV_MSC now and the hid layer is not so friendly with them,
the change in hid-input can not be done in the same way, so let's drop it for now.

How about this?


From: Benjamin Tissoires <benjamin....@gmail.com>
Date: Tue, 13 Nov 2012 15:11:22 +0100
Subject: [PATCH] Input: introduce EV_MSC Timestamp

Win 8 digitizer devices provides the actual timestamp (hid_dg_scan_time)

computed by the hardware itself. The value is global to the frame and is
not specific to the multitouch protocol (though only touch, not pen,
should use it according to the specification).

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

Documentation/input/event-codes.txt | 11 +++++++++++
include/linux/input.h | 1 +
2 files changed, 12 insertions(+)

diff --git a/Documentation/input/event-codes.txt b/Documentation/input/event-codes.txt
index 53305bd..6f28e11 100644
--- a/Documentation/input/event-codes.txt
+++ b/Documentation/input/event-codes.txt
@@ -196,6 +196,17 @@ EV_MSC:
EV_MSC events are used for input and output events that do not fall under other
categories.

+A few EV_MSC codes have special meanings:
+
+* MSC_TIMESTAMP:

+ - Used to report the number of microseconds since the last reset. This event
+ should be coded as an uint32 value, which is allowed to wrap around with
+ no special consequence. It is assumed that the time difference between two
+ consecutive events is reliable on a reasonable time scale (hours).
+ A reset to zero can happen, in which case the time since the last event is
+ unknown. If the device does not provide this information, the driver must
+ not provide it to the user space.
+

EV_LED:
----------
EV_LED events are used for input and output to set and query the state of
diff --git a/include/linux/input.h b/include/linux/input.h
index ba48743..25354f3 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -858,6 +858,7 @@ struct input_keymap_entry {
#define MSC_GESTURE 0x02
#define MSC_RAW 0x03
#define MSC_SCAN 0x04
+#define MSC_TIMESTAMP 0x05
#define MSC_MAX 0x07
#define MSC_CNT (MSC_MAX+1)

--
1.8.0

Cheers,
Benjamin

[Benjamin Tissoires]

On 11/07/2012 05:37 PM, Benjamin Tissoires wrote:
> Computes the scan time according to the specification.
> It also ensures that if the time between two events is greater
> than MAX_SCAN_INTERVAL, the scan time will be reset.
>
> Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
> ---
> drivers/hid/hid-multitouch.c | 45 ++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 43 insertions(+), 2 deletions(-)
>

[snipped]

> /*
> * Includes multi-packet support where subsequent
>

Since ABS_SCAN_TIME has been replaced by MSC_TIMESTAMP, this patch is modified as this:

From: Benjamin Tissoires <benjamin....@gmail.com>
Date: Tue, 13 Nov 2012 15:12:17 +0100
Subject: [PATCH v4] HID: hid-multitouch: forwards MSC_TIMESTAMP

Computes the device timestamp according to the specification.

It also ensures that if the time between two events is greater

than MAX_TIMESTAMP_INTERVAL, the timestamp will be reset.

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
---

drivers/hid/hid-multitouch.c | 46 +++++++++++++++++++++++++++++++++++++++++---
include/linux/hid.h | 1 +
2 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index caf0f0b..3f8432d 100644

--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -32,6 +32,7 @@
#include <linux/slab.h>
#include <linux/usb.h>
#include <linux/input/mt.h>
+#include <linux/jiffies.h>
#include "usbhid/usbhid.h"


@@ -98,6 +99,9 @@ struct mt_device {
bool serial_maybe; /* need to check for serial protocol */
bool curvalid; /* is the current contact valid? */
unsigned mt_flags; /* flags to pass to input-mt */
+ __s32 dev_time; /* the scan time provided by the device */
+ unsigned long jiffies; /* the frame's jiffies */

+ unsigned timestamp; /* the timestamp to be sent */

};

/* classes of device behavior */

@@ -126,6 +130,8 @@ struct mt_device {

#define MT_DEFAULT_MAXCONTACT 10
#define MT_MAX_MAXCONTACT 250

+#define MAX_TIMESTAMP_INTERVAL 500000

+
#define MT_USB_DEVICE(v, p) HID_DEVICE(BUS_USB, HID_GROUP_MULTITOUCH, v, p)
#define MT_BT_DEVICE(v, p) HID_DEVICE(BUS_BLUETOOTH, HID_GROUP_MULTITOUCH, v, p)

@@ -451,12 +457,19 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,

mt_store_field(usage, td, hi);
td->last_field_index = field->index;
return 1;
+ case HID_DG_SCANTIME:
+ hid_map_usage(hi, usage, bit, max,

+ EV_MSC, MSC_TIMESTAMP);
+ set_bit(MSC_TIMESTAMP, hi->input->mscbit);

+ td->last_field_index = field->index;
+ return 1;
case HID_DG_CONTACTCOUNT:
td->last_field_index = field->index;
return 1;
case HID_DG_CONTACTMAX:
- /* we don't set td->last_slot_field as contactcount and
- * contact max are global to the report */
+ /* we don't set td->last_slot_field as scan time,
+ * contactcount and contact max are global to the
+ * report */
td->last_field_index = field->index;
return -1;
}

@@ -485,7 +498,8 @@ static int mt_input_mapped(struct hid_device *hdev, struct hid_input *hi,

struct hid_field *field, struct hid_usage *usage,

unsigned long **bit, int *max)
{
- if (usage->type == EV_KEY || usage->type == EV_ABS)
+ if (usage->type == EV_KEY || usage->type == EV_ABS ||
+ usage->type == EV_MSC)
set_bit(usage->type, hi->input->evbit);

return -1;
@@ -565,11 +579,34 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)

*/
static void mt_sync_frame(struct mt_device *td, struct input_dev *input)
{

+ input_event(input, EV_MSC, MSC_TIMESTAMP, td->timestamp);

input_mt_sync_frame(input);
input_sync(input);
td->num_received = 0;
}

+static void mt_compute_timestamp(struct mt_device *td, struct hid_field *field,

+ __s32 value)
+{
+ long delta = value - td->dev_time;
+ unsigned long jdelta = jiffies_to_usecs(jiffies - td->jiffies);
+
+ td->jiffies = jiffies;
+ td->dev_time = value;
+
+ if (delta < 0)
+ delta += field->logical_maximum;
+
+ /* HID_DG_SCANTIME is expressed in 100us, we want it in ms. */
+ delta *= 100;
+

+ if (abs(delta - jdelta) > MAX_TIMESTAMP_INTERVAL)

+ /* obviously wrong clock -> the device time has been reset */

+ td->timestamp = 0;
+ else
+ td->timestamp += delta;

+}
+
static int mt_event(struct hid_device *hid, struct hid_field *field,
struct hid_usage *usage, __s32 value)
{

@@ -617,6 +654,9 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,

case HID_DG_HEIGHT:
td->curdata.h = value;
break;
+ case HID_DG_SCANTIME:

+ mt_compute_timestamp(td, field, value);

+ break;
case HID_DG_CONTACTCOUNT:
/*
* Includes multi-packet support where subsequent

diff --git a/include/linux/hid.h b/include/linux/hid.h
index 6b4f322..0337e50 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h

@@ -279,6 +279,7 @@ struct hid_item {
#define HID_DG_DEVICEINDEX 0x000d0053
#define HID_DG_CONTACTCOUNT 0x000d0054
#define HID_DG_CONTACTMAX 0x000d0055
+#define HID_DG_SCANTIME 0x000d0056

/*
* HID report types --- Ouch! HID spec says 1 2 3!

--
1.8.0

Cheers,
Benjamin

[Henrik Rydberg]

Hi Benjamin,

Is this really hidinput data? Should it not be one line above?

> __u16 code; /* input driver code */
> __u8 type; /* input driver type */
> __s8 hat_min; /* hat switch fun */
> --
> 1.7.11.7
>

Thanks,
Henrik

[Henrik Rydberg]

Reviewed-by: Henrik Rydberg <ryd...@euromail.se>

Thanks,
Henrik

[Henrik Rydberg]

Reviewed-by: Henrik Rydberg <ryd...@euromail.se>

Thanks,
Henrik

[Henrik Rydberg]

Reviewed-by: Henrik Rydberg <ryd...@euromail.se>

Thanks,
Henrik

[Henrik Rydberg]

Hi Benjamin,

> this prevents a device to reuse contact id 0 several time when
> sending garbage values to complete the report.

This quirk allows a device to reuse a contact id when sending garbage
inactive contacts at the end of a report.

>
> Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
> ---

> drivers/hid/hid-multitouch.c | 6 ++++++
> include/linux/input/mt.h | 6 ++++++

> 2 files changed, 12 insertions(+)
>

> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 2352770..351c814 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -52,6 +52,7 @@ MODULE_LICENSE("GPL");
> #define MT_QUIRK_VALID_IS_CONFIDENCE (1 << 6)

> #define MT_QUIRK_SLOT_IS_CONTACTID_MINUS_ONE (1 << 8)
> #define MT_QUIRK_NO_AREA (1 << 9)

> +#define MT_QUIRK_IGNORE_DUPLICATES (1 << 10)
>
> struct mt_slot {

> __s32 x, y, cx, cy, p, w, h;

> @@ -510,6 +511,11 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
> if (slotnum < 0 || slotnum >= td->maxcontacts)
> return;
>
> + if (td->mtclass.quirks & MT_QUIRK_IGNORE_DUPLICATES &&

Don't like parenthesis, i see :-)

> + input_mt_is_active(&input->mt->slots[slotnum]) &&
> + input_mt_is_used(input->mt, &input->mt->slots[slotnum]))
> + return;
> +

It init_mt_slots failed earlier (no checks), input->mt can be NULL
here. Also, a local variable instead of repetition would be nice.

> input_mt_slot(input, slotnum);
> input_mt_report_slot_state(input, MT_TOOL_FINGER,
> s->touch_state);
> diff --git a/include/linux/input/mt.h b/include/linux/input/mt.h
> index cc5cca7..2e86bd0 100644
> --- a/include/linux/input/mt.h
> +++ b/include/linux/input/mt.h
> @@ -69,6 +69,12 @@ static inline bool input_mt_is_active(const struct input_mt_slot *slot)
> return input_mt_get_value(slot, ABS_MT_TRACKING_ID) >= 0;
> }
>
> +static inline bool input_mt_is_used(const struct input_mt *mt,
> + const struct input_mt_slot *slot)
> +{
> + return slot->frame == mt->frame;
> +}
> +

This is ok, although I would prefer to also change input-mt.c to use
this helper function. Possibly that turns this hunk into a separate patch.

> int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots,
> unsigned int flags);
> void input_mt_destroy_slots(struct input_dev *dev);
> --
> 1.7.11.7
>

Thanks,
Henrik

[Henrik Rydberg]

Hi Benjamin,

> Win 8 specification is much more precise than the Win 7 one.
> Moreover devices that need to take certification must be submitted
> to Microsoft.
>
> The result is a better protocol support and we can rely on that to
> skip all the messy tests we used to do.
>

You could possibly start the commit message here..

> The protocol specify the fact that each valid touch must be reported

The win8 protocol...

> within a frame until it is released.
> So we can use the always_valid quirk and dismiss reports when we see

We can therefore...

> duplicates contact ID.

duplicate contacts.

Won't this line interfere with the hovering functionality?

> quirks &= ~MT_QUIRK_VALID_IS_CONFIDENCE;
> }
>
> + if (quirks & MT_QUIRK_WIN_8_CERTIFIED)
> + quirks |= MT_QUIRK_IGNORE_DUPLICATES;
> +
> td->mtclass.quirks = quirks;
> }
>
> --
> 1.7.11.7
>

Thanks,
Henrik

[Henrik Rydberg]

On Wed, Nov 07, 2012 at 05:37:34PM +0100, Benjamin Tissoires wrote:
> Win8 devices supporting hovering must provides InRange HID field.

provide the

> 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.

In the absence of more detailed information, use ABS_MT_DISTANCE with
a [0,1] interval to distinguish between presence (1) and touch (0).

>
> Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
> ---

> drivers/hid/hid-multitouch.c | 24 ++++++++++++++++++++----
> 1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index b393c6c..1f3d1e0 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -59,6 +59,7 @@ struct mt_slot {

> __s32 x, y, cx, cy, p, w, h;

> __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? */
> };
>
> struct mt_class {
> @@ -397,6 +398,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, -1, 1, 0, 0);

Why [-1,1] here?

> + }
> mt_store_field(usage, td, hi);
> td->last_field_index = field->index;
> return 1;

> @@ -516,6 +523,10 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)

> if (slotnum < 0 || slotnum >= td->maxcontacts)
> return;
>

> + if (!test_bit(ABS_MT_DISTANCE, input->absbit))
> + /* If InRange is not present, rely on TipSwitch */
> + s->inrange_state = s->touch_state;
> +

This could be skipped, see below.

> if (td->mtclass.quirks & MT_QUIRK_IGNORE_DUPLICATES &&

> input_mt_is_active(&input->mt->slots[slotnum]) &&

> input_mt_is_used(input->mt, &input->mt->slots[slotnum]))

> @@ -523,9 +534,9 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
>
> input_mt_slot(input, slotnum);
> input_mt_report_slot_state(input, MT_TOOL_FINGER,
> - s->touch_state);
> - if (s->touch_state) {
> - /* this finger is on the screen */
> + s->inrange_state);
> + if (s->inrange_state) {
> + /* this finger is in proximity of the sensor */

Using (s->touch_state || s->inrange_state) here seems simpler.

> int wide = (s->w > s->h);
> /* divided by two to match visual scale of touch */
> int major = max(s->w, s->h) >> 1;
> @@ -535,11 +546,14 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)

> input_event(input, EV_ABS, ABS_MT_POSITION_Y, s->y);

> input_event(input, EV_ABS, ABS_MT_TOOL_X, s->cx);

> 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_PRESSURE, s->p);
> input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);

> input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
> - }
> + } else
> + input_event(input, EV_ABS, ABS_MT_DISTANCE, -1);

Just skip this, please.

> }
>
> td->num_received++;
> @@ -567,6 +581,8 @@ 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.inrange_state = value;
> break;
> case HID_DG_TIPSWITCH:
> if (quirks & MT_QUIRK_NOT_SEEN_MEANS_UP)
> --
> 1.7.11.7
>

Thanks,
Henrik

[Henrik Rydberg]

Hi Benjamin,

> From: Benjamin Tissoires <benjamin....@gmail.com>
> Date: Tue, 13 Nov 2012 15:11:22 +0100
> Subject: [PATCH] Input: introduce EV_MSC Timestamp
>
> Win 8 digitizer devices provides the actual timestamp (hid_dg_scan_time)
> computed by the hardware itself. The value is global to the frame and is
> not specific to the multitouch protocol (though only touch, not pen,
> should use it according to the specification).

I think it is good to use win8 as an example here, but this feature is
present in many other devices as well, and this feature is generic to
all of them. It would thus make sense to start off a bit more general here.

>
> Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
> ---
> Documentation/input/event-codes.txt | 11 +++++++++++
> include/linux/input.h | 1 +
> 2 files changed, 12 insertions(+)
>
> diff --git a/Documentation/input/event-codes.txt b/Documentation/input/event-codes.txt
> index 53305bd..6f28e11 100644
> --- a/Documentation/input/event-codes.txt
> +++ b/Documentation/input/event-codes.txt
> @@ -196,6 +196,17 @@ EV_MSC:
> EV_MSC events are used for input and output events that do not fall under other
> categories.
>
> +A few EV_MSC codes have special meanings:

meaning:

> +
> +* MSC_TIMESTAMP:
> + - Used to report the number of microseconds since the last reset. This event
> + should be coded as an uint32 value, which is allowed to wrap around with
> + no special consequence. It is assumed that the time difference between two
> + consecutive events is reliable on a reasonable time scale (hours).
> + A reset to zero can happen, in which case the time since the last event is
> + unknown. If the device does not provide this information, the driver must
> + not provide it to the user space.

to user space.

> +
> EV_LED:
> ----------
> EV_LED events are used for input and output to set and query the state of
> diff --git a/include/linux/input.h b/include/linux/input.h
> index ba48743..25354f3 100644
> --- a/include/linux/input.h
> +++ b/include/linux/input.h
> @@ -858,6 +858,7 @@ struct input_keymap_entry {
> #define MSC_GESTURE 0x02
> #define MSC_RAW 0x03
> #define MSC_SCAN 0x04
> +#define MSC_TIMESTAMP 0x05
> #define MSC_MAX 0x07
> #define MSC_CNT (MSC_MAX+1)
>
> --
> 1.8.0
>
> Cheers,
> Benjamin

Reviewed-by: Henrik Rydberg <ryd...@euromail.se>

Thanks,
Henrik

[Henrik Rydberg]

Hi Benjamin,

> From: Benjamin Tissoires <benjamin....@gmail.com>
> Date: Tue, 13 Nov 2012 15:12:17 +0100
> Subject: [PATCH v4] HID: hid-multitouch: forwards MSC_TIMESTAMP
>
> Computes the device timestamp according to the specification.
> It also ensures that if the time between two events is greater
> than MAX_TIMESTAMP_INTERVAL, the timestamp will be reset.
>
> Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
> ---
> drivers/hid/hid-multitouch.c | 46 +++++++++++++++++++++++++++++++++++++++++---
> include/linux/hid.h | 1 +
> 2 files changed, 44 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index caf0f0b..3f8432d 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -32,6 +32,7 @@
> #include <linux/slab.h>
> #include <linux/usb.h>
> #include <linux/input/mt.h>
> +#include <linux/jiffies.h>

Why?

> #include "usbhid/usbhid.h"
>
>
> @@ -98,6 +99,9 @@ struct mt_device {
> bool serial_maybe; /* need to check for serial protocol */
> bool curvalid; /* is the current contact valid? */
> unsigned mt_flags; /* flags to pass to input-mt */
> + __s32 dev_time; /* the scan time provided by the device */
> + unsigned long jiffies; /* the frame's jiffies */
> + unsigned timestamp; /* the timestamp to be sent */

Why not just dev_time here?

> };
>
> /* classes of device behavior */
> @@ -126,6 +130,8 @@ struct mt_device {
> #define MT_DEFAULT_MAXCONTACT 10
> #define MT_MAX_MAXCONTACT 250
>
> +#define MAX_TIMESTAMP_INTERVAL 500000
> +

Needs to be done in userland anyways, so no need to check this in the kernel.

I think this should go after the frame sync,

> input_mt_sync_frame(input);

And the computation (100 * td->dev_time) should work fine. It will wrap
properly.

> input_sync(input);
> td->num_received = 0;
> }
>
> +static void mt_compute_timestamp(struct mt_device *td, struct hid_field *field,
> + __s32 value)
> +{
> + long delta = value - td->dev_time;
> + unsigned long jdelta = jiffies_to_usecs(jiffies - td->jiffies);
> +
> + td->jiffies = jiffies;
> + td->dev_time = value;
> +
> + if (delta < 0)
> + delta += field->logical_maximum;
> +
> + /* HID_DG_SCANTIME is expressed in 100us, we want it in ms. */
> + delta *= 100;
> +
> + if (abs(delta - jdelta) > MAX_TIMESTAMP_INTERVAL)
> + /* obviously wrong clock -> the device time has been reset */
> + td->timestamp = 0;
> + else
> + td->timestamp += delta;
> +}
> +

Can be skipped entirely.

> static int mt_event(struct hid_device *hid, struct hid_field *field,
> struct hid_usage *usage, __s32 value)
> {
> @@ -617,6 +654,9 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
> case HID_DG_HEIGHT:
> td->curdata.h = value;
> break;
> + case HID_DG_SCANTIME:
> + mt_compute_timestamp(td, field, value);

Just td->dev_time = value should work fine here.

> + break;
> case HID_DG_CONTACTCOUNT:
> /*
> * Includes multi-packet support where subsequent
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index 6b4f322..0337e50 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -279,6 +279,7 @@ struct hid_item {
> #define HID_DG_DEVICEINDEX 0x000d0053
> #define HID_DG_CONTACTCOUNT 0x000d0054
> #define HID_DG_CONTACTMAX 0x000d0055
> +#define HID_DG_SCANTIME 0x000d0056

Was this missing this the old patch, or did it get moved here?

>
> /*
> * HID report types --- Ouch! HID spec says 1 2 3!
> --
> 1.8.0

Thanks,
Henrik

[Benjamin Tissoires]

Hi Henrik,

thanks for the review of the patchset. I'll do my best for the next version :)

At first, I only used [0,1]. However, it's still the same problem with
the information being kept between the touches:
if you start an application after having touched your device, you
normally have to ask for the different per-touche states to get x, y,
distance, pressure, etc...
However, this is not much mandatory because the protocol in its
current form ensures that you will get the new states of the axes when
a new touch occurs.

ABS_MT_DISTANCE is a little bit different here because the protocol
guarantees that once you get the "not in range" state through
tracking_id == -1, distance should be 1.
When the new touch of the very same slot occurs, you also have the
guarantee that distance is at 1 too.

So basically, if you don't ask for the slot states, you will never get
that very first distance.

I know that it's a user-space problem, but honestly, I don't want to
fix several user-space problems when we could fix it through the
protocol.

I can see 2 solutions:
- set the range to: [0,1] and still sending -1 (or 2) for the invalid
distance value (if it's not clamped)
- set the range to: [0,2] and having: 0 for touch, 1 for hovering, and
2 for not in range

Does that make sense?

agree.

>
>> int wide = (s->w > s->h);
>> /* divided by two to match visual scale of touch */
>> int major = max(s->w, s->h) >> 1;
>> @@ -535,11 +546,14 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
>> input_event(input, EV_ABS, ABS_MT_POSITION_Y, s->y);
>> input_event(input, EV_ABS, ABS_MT_TOOL_X, s->cx);
>> 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_PRESSURE, s->p);
>> input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
>> input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
>> - }
>> + } else
>> + input_event(input, EV_ABS, ABS_MT_DISTANCE, -1);
>
> Just skip this, please.

see above.

Cheers,
Benjamin

[Henrik Rydberg]

Hi Benjamin,

> > here is the third version of this patchset.
> > I think I included most of Henrik's comments.
> >
> > Happy reviewing :)

thanks for the changes :-)

Jiri,

> I am fine with the HID-specific changes (and I have sent out my Acks to
> the invidivual patches).

Looks reasonable to me too (although I have not looked through the exponent
code enough to want to formally ack it).

> For hid-multitouch changes, I'd like to have Reviewed-by: from Henrik as
> well. Henrik, are you planning to review this patchset, please?

Yes, and finally done.

Cheers,
Henrik

[Benjamin Tissoires]

Hi Henrik,

because max dev_time is at least 65535 according to the norm, and the
win 8 device I have has his max value of 65535.
Which means that every 6 seconds and a half the counter resets, and
the value is not properly reset in the way I understand the
specification. The device just forwards an internal clock that is
never reset.
So if you wait 653500 us + 8ms, everything happens as if the device
sent this frame right after the previous one (you get the same value).

I think that it's the job of the kernel to provide clean and coherent
values through evdev, which won't be the case if the jiffies thing is
not in place: every devices will have a different behavior, leading to
complicate things in the user-space.

Sorry for that. I moved it there because I cleaned a little the other
patch by removing the hid things.

Cheers,
Benjamin

[Henrik Rydberg]

Hi Benjamin,

> > Why [-1,1] here?
>
> At first, I only used [0,1]. However, it's still the same problem with
> the information being kept between the touches:
> if you start an application after having touched your device, you
> normally have to ask for the different per-touche states to get x, y,
> distance, pressure, etc...
> However, this is not much mandatory because the protocol in its
> current form ensures that you will get the new states of the axes when
> a new touch occurs.

Right, the stateful communication requires a full state read after
opening the deivce, although in most cases this is not really
necessary. This is no different for ABS_MT_DISTANCE, of course.

> ABS_MT_DISTANCE is a little bit different here because the protocol
> guarantees that once you get the "not in range" state through
> tracking_id == -1, distance should be 1.
> When the new touch of the very same slot occurs, you also have the
> guarantee that distance is at 1 too.

ABS_MT_DISTANCE is just another attribute of the slot, so it really is
no different.

> So basically, if you don't ask for the slot states, you will never get
> that very first distance.

Which will not be important either; as long as the slot is unused, it
does not matter what the attributes of that slot are.

> I know that it's a user-space problem, but honestly, I don't want to
> fix several user-space problems when we could fix it through the
> protocol.
>
> I can see 2 solutions:
> - set the range to: [0,1] and still sending -1 (or 2) for the invalid
> distance value (if it's not clamped)
> - set the range to: [0,2] and having: 0 for touch, 1 for hovering, and
> 2 for not in range
>
> Does that make sense?

I just do not see what problem you want to solve here.

[Benjamin Tissoires]

Hi Henrik,

On Tue, Nov 13, 2012 at 6:33 PM, Henrik Rydberg <ryd...@euromail.se> wrote:
> Hi Benjamin,
>
>> > here is the third version of this patchset.
>> > I think I included most of Henrik's comments.
>> >
>> > Happy reviewing :)
>
> thanks for the changes :-)

and thanks for the review.

>
> Jiri,
>
>> I am fine with the HID-specific changes (and I have sent out my Acks to
>> the invidivual patches).
>
> Looks reasonable to me too (although I have not looked through the exponent
> code enough to want to formally ack it).
>
>> For hid-multitouch changes, I'd like to have Reviewed-by: from Henrik as
>> well. Henrik, are you planning to review this patchset, please?
>
> Yes, and finally done.
>
> Cheers,
> Henrik

Jiri,

I'll send a new patchset tomorrow with the different acked, reviewed
and requested changes.

Some patches at the end may need an other review, so I'll make sure to
get the reviewed patches at the beginning of the set so that you can
pick them for 3.8.
It will also allow me not to send dozens of patches in a row :)

Is it good for you?

Cheers,
Benjamin

[Benjamin Tissoires]

And if the slot is unused, but has been used since the plug of the
device, the state is kept between the touch.

>
>> I know that it's a user-space problem, but honestly, I don't want to
>> fix several user-space problems when we could fix it through the
>> protocol.
>>
>> I can see 2 solutions:
>> - set the range to: [0,1] and still sending -1 (or 2) for the invalid
>> distance value (if it's not clamped)
>> - set the range to: [0,2] and having: 0 for touch, 1 for hovering, and
>> 2 for not in range
>>
>> Does that make sense?
>
> I just do not see what problem you want to solve here.

I just intend to force the update of the distance value at the
beginning of the touch.
This is just to send a coherent state when the finger goes in range,
and to make sure that user-space application do not rely on the
undefined value (0) whereas the kernel thought it was set to 1.

Cheers,
Benjamin

[Henrik Rydberg]

> >> @@ -98,6 +99,9 @@ struct mt_device {
> >> bool serial_maybe; /* need to check for serial protocol */
> >> bool curvalid; /* is the current contact valid? */
> >> unsigned mt_flags; /* flags to pass to input-mt */
> >> + __s32 dev_time; /* the scan time provided by the device */
> >> + unsigned long jiffies; /* the frame's jiffies */
> >> + unsigned timestamp; /* the timestamp to be sent */
> >
> > Why not just dev_time here?
>
> because max dev_time is at least 65535 according to the norm, and the
> win 8 device I have has his max value of 65535.
> Which means that every 6 seconds and a half the counter resets, and
> the value is not properly reset in the way I understand the
> specification. The device just forwards an internal clock that is
> never reset.

Ok, I though it was a 32-bit value, and that it would wrap with a
longer period. It does not change the essence of the definition,
though. If we say "seconds" instead of "hours", we should still be
fine, no?

> So if you wait 653500 us + 8ms, everything happens as if the device
> sent this frame right after the previous one (you get the same value).

Yes, but we have this effect on a 32-bit counter as well.

> I think that it's the job of the kernel to provide clean and coherent
> values through evdev, which won't be the case if the jiffies thing is
> not in place: every devices will have a different behavior, leading to
> complicate things in the user-space.

The whole point is to provide the device clock to userland when it
exists, isn't it? Thus, the jiffies would never be used. If a future
device needs additions to work conformly, we just have to deal with it
at that point in time.

To conclude, we obviously have devices with a rather short wrap-around
time. However, since the normal inter-frame time is in the millisecond
range, it should not be overly restrictive to change the definition of
the minimum wraparound time from hours to seconds.

[Henrik Rydberg]

> To conclude, we obviously have devices with a rather short wrap-around
> time. However, since the normal inter-frame time is in the millisecond
> range, it should not be overly restrictive to change the definition of
> the minimum wraparound time from hours to seconds.

Sorry, Benjamin, you are right about the counter. To be useful, we do
need to know the number of bits of the counter, so that differences
can be computed properly. In other words, either ABS_SCAN_TIME is
be the better choice, or we need to make sure that the counter wraps
on the full 32 bit boundary.

[Henrik Rydberg]

> I just intend to force the update of the distance value at the
> beginning of the touch.
> This is just to send a coherent state when the finger goes in range,
> and to make sure that user-space application do not rely on the
> undefined value (0) whereas the kernel thought it was set to 1.

Right, so we use EVIOCGMTSLOTS. This "problem" occurs with
ABS_MT_ORIENTATION as well.

[Benjamin Tissoires]

HID spec details special values for the HID field unit exponent.
Basically, the range [0x8..0xf] correspond to [-8..-1], so this is
a standard two's complement on a half-byte.

Signed-off-by: Benjamin Tissoires <benjamin....@gmail.com>
Acked-by: Jiri Kosina <jko...@suse.cz>
---

drivers/hid/hid-core.c | 16 +++++++++++++++-
drivers/hid/hid-input.c | 13 +++++++++----

include/linux/hid.h | 1 +
3 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 3d0da29..9da3007 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 67044f3..7015080 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c

@@ -192,7 +192,6 @@ static int hidinput_setkeycode(struct input_dev *dev,
return -EINVAL;
}

-
/**
* hidinput_calc_abs_res - calculate an absolute axis resolution
* @field: the HID report field to calculate resolution for
@@ -235,17 +234,23 @@ __s32 hidinput_calc_abs_res(const struct hid_field *field, __u16 code)
case ABS_MT_TOOL_Y:
case ABS_MT_TOUCH_MAJOR:
case ABS_MT_TOUCH_MINOR:
- if (field->unit == 0x11) { /* If centimeters */
+ if (field->unit & 0xffffff00) /* Not a length */
+ return 0;
+ unit_exponent += hid_snto32(field->unit >> 4, 4) - 1;
+ switch (field->unit & 0xf) {

+ case 0x1: /* If centimeters */

/* Convert to millimeters */
unit_exponent += 1;

- } else if (field->unit == 0x13) { /* If inches */
+ break;

+ case 0x3: /* If inches */

/* Convert to millimeters */

prev = physical_extents;
physical_extents *= 254;
if (physical_extents < prev)
return 0;
unit_exponent -= 1;
- } else {
+ break;
+ default:
return 0;
}
break;

diff --git a/include/linux/hid.h b/include/linux/hid.h
index 9edb06c..a110a3e 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h

@@ -754,6 +754,7 @@ int hid_connect(struct hid_device *hid, unsigned int connect_mask);
void hid_disconnect(struct hid_device *hid);
const struct hid_device_id *hid_match_id(struct hid_device *hdev,
const struct hid_device_id *id);
+s32 hid_snto32(__u32 value, unsigned n);

/**
* hid_map_usage - map usage input bits
--

1.8.0