[PATCH v3 0/3] HID: Win 8 multitouch panels detection in core

[PATCH v3 0/3] HID: Win 8 multitouch panels detection in core

[Benjamin Tissoires]

Hi guys,

this is the v3 of this patch series.

Srinivas, I still did not added your tested-by as I made minors modifications of
the patches 1 and 2. Now only the patch 1 will impact sensor_hub, and patch 2
will not impact this. Maybe just wait for Henrik to put his "Reviewed-by" before
giving one last test.

Cheers,
Benjamin

Changes in v3:
- remove one remnant duplicate message
- amended commit messages
- re-add initializer of hid->group (set to HID_GROUP_GENERIC)
- removed ugly #define #undef :)
- add back scan_flags in hid_parser
- simpler post processing of Win 8 certified multitouch panels

Changes in v2:
- moved "flags" processing in patch 2/3
- do not introduce parser->flags, but use hid->group as a temporary flag placeholder
- hid_scan_report() is less verbose when errors are found in the descriptor
- hid_scan_report() is tolerant to parsing errors
- fixed usage_page handling in hid_scan_collection(), which fixes sensors detection
- amended commit messages
- #define and #undef HID_FLAG_* in hid-core.c instead of hid.h

Benjamin Tissoires (3):
  HID: Use hid_parser for pre-scanning the report descriptors
  HID: detect Win 8 multitouch devices in core
  HID: do not init input reports for Win 8 multitouch devices

 drivers/hid/hid-core.c        | 118 ++++++++++++++++++++++++++++--------------
 drivers/hid/hid-multitouch.c  |  36 ++++++++-----
 drivers/hid/usbhid/hid-core.c |  11 ++--
 include/linux/hid.h           |   5 ++
 4 files changed, 116 insertions(+), 54 deletions(-)

-- 
1.8.3.1

[PATCH v3 1/3] HID: Use hid_parser for pre-scanning the report descriptors

[Benjamin Tissoires]

The Win 8 detection is sufficiently complex to warrant use of the full
parser code, in spite of the inferred memory usage. Therefore, we can use
the existing HID parser in hid-core for hid_scan_report() by re-using the
code from hid_open_report(). hid_parser_global, hid_parser_local and
hid_parser_reserved does not have any side effects. We just need to
reimplement the MAIN_ITEM callback to have a proper parsing without side
effects.

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

changes in v3:
- remove one remnant duplicate message
- amended commit message
- re-add initializer of hid->group (set to HID_GROUP_GENERIC)

changes in v2:
- moved "flags" processing in patch 2/3 (so use hid->group in this patch)
- hid_scan_report() is less verbose when errors are found in the descriptor
- hid_scan_report() is tolerant to parsing errors
- fixed usage_page handling in hid_scan_collection(), which fixes sensors detection
- amended commit message

 drivers/hid/hid-core.c | 102 +++++++++++++++++++++++++++++++------------------
 1 file changed, 64 insertions(+), 38 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 3efe19f..9fc1220 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -677,12 +677,52 @@ static u8 *fetch_item(__u8 *start, __u8 *end, struct hid_item *item)
 	return NULL;
 }
 
-static void hid_scan_usage(struct hid_device *hid, u32 usage)
+static void hid_scan_input_usage(struct hid_parser *parser, u32 usage)
 {
+	struct hid_device *hid = parser->device;
+
 	if (usage == HID_DG_CONTACTID)
 		hid->group = HID_GROUP_MULTITOUCH;
 }
 
+static void hid_scan_collection(struct hid_parser *parser, unsigned type)
+{
+	struct hid_device *hid = parser->device;
+
+	if (((parser->global.usage_page << 16) == HID_UP_SENSOR) &&
+	    type == HID_COLLECTION_PHYSICAL)
+		hid->group = HID_GROUP_SENSOR_HUB;
+}
+
+static int hid_scan_main(struct hid_parser *parser, struct hid_item *item)
+{
+	__u32 data;
+	int i;
+
+	data = item_udata(item);
+
+	switch (item->tag) {
+	case HID_MAIN_ITEM_TAG_BEGIN_COLLECTION:
+		hid_scan_collection(parser, data & 0xff);
+		break;
+	case HID_MAIN_ITEM_TAG_END_COLLECTION:
+		break;
+	case HID_MAIN_ITEM_TAG_INPUT:
+		for (i = 0; i < parser->local.usage_index; i++)
+			hid_scan_input_usage(parser, parser->local.usage[i]);
+		break;
+	case HID_MAIN_ITEM_TAG_OUTPUT:
+		break;
+	case HID_MAIN_ITEM_TAG_FEATURE:
+		break;
+	}
+
+	/* Reset the local parser environment */
+	memset(&parser->local, 0, sizeof(parser->local));
+
+	return 0;
+}
+
 /*
  * Scan a report descriptor before the device is added to the bus.
  * Sets device groups and other properties that determine what driver
@@ -690,48 +730,34 @@ static void hid_scan_usage(struct hid_device *hid, u32 usage)
  */
 static int hid_scan_report(struct hid_device *hid)
 {
-	unsigned int page = 0, delim = 0;
+	struct hid_parser *parser;
+	struct hid_item item;
 	__u8 *start = hid->dev_rdesc;
 	__u8 *end = start + hid->dev_rsize;
-	unsigned int u, u_min = 0, u_max = 0;
-	struct hid_item item;
+	static int (*dispatch_type[])(struct hid_parser *parser,
+				      struct hid_item *item) = {
+		hid_scan_main,
+		hid_parser_global,
+		hid_parser_local,
+		hid_parser_reserved
+	};
+
+	parser = vzalloc(sizeof(struct hid_parser));
+	if (!parser)
+		return -ENOMEM;
 
+	parser->device = hid;
 	hid->group = HID_GROUP_GENERIC;
-	while ((start = fetch_item(start, end, &item)) != NULL) {
-		if (item.format != HID_ITEM_FORMAT_SHORT)
-			return -EINVAL;
-		if (item.type == HID_ITEM_TYPE_GLOBAL) {
-			if (item.tag == HID_GLOBAL_ITEM_TAG_USAGE_PAGE)
-				page = item_udata(&item) << 16;
-		} else if (item.type == HID_ITEM_TYPE_LOCAL) {
-			if (delim > 1)
-				break;
-			u = item_udata(&item);
-			if (item.size <= 2)
-				u += page;
-			switch (item.tag) {
-			case HID_LOCAL_ITEM_TAG_DELIMITER:
-				delim += !!u;
-				break;
-			case HID_LOCAL_ITEM_TAG_USAGE:
-				hid_scan_usage(hid, u);
-				break;
-			case HID_LOCAL_ITEM_TAG_USAGE_MINIMUM:
-				u_min = u;
-				break;
-			case HID_LOCAL_ITEM_TAG_USAGE_MAXIMUM:
-				u_max = u;
-				for (u = u_min; u <= u_max; u++)
-					hid_scan_usage(hid, u);
-				break;
-			}
-		} else if (page == HID_UP_SENSOR &&
-			item.type == HID_ITEM_TYPE_MAIN &&
-			item.tag == HID_MAIN_ITEM_TAG_BEGIN_COLLECTION &&
-			(item_udata(&item) & 0xff) == HID_COLLECTION_PHYSICAL)
-			hid->group = HID_GROUP_SENSOR_HUB;
-	}
 
+	/*
+	 * The parsing is simpler than the one in hid_open_report() as we should
+	 * be robust against hid errors. Those errors will be raised by
+	 * hid_open_report() anyway.
+	 */
+	while ((start = fetch_item(start, end, &item)) != NULL)
+		dispatch_type[item.type](parser, &item);
+
+	vfree(parser);
 	return 0;
 }
 
-- 
1.8.3.1

[PATCH v3 2/3] HID: detect Win 8 multitouch devices in core

[Benjamin Tissoires]

Detecting Win 8 multitouch devices in core allows us to set quirks
before the device is parsed through hid_hw_start().
It also simplifies the detection of those devices in hid-multitouch and
makes the handling of those devices cleaner.

As Win 8 multitouch panels are in the group multitouch and rely on a
special feature to be detected, this patch adds a bitfield in the parser.

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

changes in v3:
- removed ugly #define #undef :)
- add back scan_flags in hid_parser
- simpler post processing of Win 8 certified multitouch panels
- amended commit message

changes in v2:
- moved "flags" processing here
- do not introduce parser->flags, but use hid->group as a temporary flag placeholder
- #define and #undef HID_FLAG_* in hid-core.c instead of hid.h
- amended commit message

 drivers/hid/hid-core.c       | 16 ++++++++++++++++
 drivers/hid/hid-multitouch.c | 24 +++++++++++-------------
 include/linux/hid.h          |  4 ++++
 3 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 9fc1220..db99900 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -685,6 +685,13 @@ static void hid_scan_input_usage(struct hid_parser *parser, u32 usage)
 		hid->group = HID_GROUP_MULTITOUCH;
 }
 
+static void hid_scan_feature_usage(struct hid_parser *parser, u32 usage)
+{
+	if (usage == 0xff0000c5 && parser->global.report_count == 256 &&
+	    parser->global.report_size == 8)
+		parser->scan_flags |= HID_SCAN_FLAG_MT_WIN_8;
+}
+
 static void hid_scan_collection(struct hid_parser *parser, unsigned type)
 {
 	struct hid_device *hid = parser->device;
@@ -714,6 +721,8 @@ static int hid_scan_main(struct hid_parser *parser, struct hid_item *item)
 	case HID_MAIN_ITEM_TAG_OUTPUT:
 		break;
 	case HID_MAIN_ITEM_TAG_FEATURE:
+		for (i = 0; i < parser->local.usage_index; i++)
+			hid_scan_feature_usage(parser, parser->local.usage[i]);
 		break;
 	}
 
@@ -757,6 +766,13 @@ static int hid_scan_report(struct hid_device *hid)
 	while ((start = fetch_item(start, end, &item)) != NULL)
 		dispatch_type[item.type](parser, &item);
 
+	/*
+	 * Handle special flags set during scanning.
+	 */
+	if ((parser->scan_flags & HID_SCAN_FLAG_MT_WIN_8) &&
+	    (hid->group == HID_GROUP_MULTITOUCH))
+		hid->group = HID_GROUP_MULTITOUCH_WIN_8;
+
 	vfree(parser);
 	return 0;
 }
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 0fe00e2..c28ef86 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -133,6 +133,7 @@ static void mt_post_parse(struct mt_device *td);
 #define MT_CLS_NSMU				0x000a
 #define MT_CLS_DUAL_CONTACT_NUMBER		0x0010
 #define MT_CLS_DUAL_CONTACT_ID			0x0011
+#define MT_CLS_WIN_8				0x0012
 
 /* vendor specific classes */
 #define MT_CLS_3M				0x0101
@@ -205,6 +206,11 @@ static struct mt_class mt_classes[] = {
 			MT_QUIRK_CONTACT_CNT_ACCURATE |
 			MT_QUIRK_SLOT_IS_CONTACTID,
 		.maxcontacts = 2 },
+	{ .name = MT_CLS_WIN_8,
+		.quirks = MT_QUIRK_ALWAYS_VALID |
+			MT_QUIRK_IGNORE_DUPLICATES |
+			MT_QUIRK_HOVERING |
+			MT_QUIRK_CONTACT_CNT_ACCURATE },
 
 	/*
 	 * vendor specific classes
@@ -332,19 +338,6 @@ 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) {
-			/* Win 8 devices need special quirks */
-			__s32 *quirks = &td->mtclass.quirks;
-			*quirks |= MT_QUIRK_ALWAYS_VALID;
-			*quirks |= MT_QUIRK_IGNORE_DUPLICATES;
-			*quirks |= MT_QUIRK_HOVERING;
-			*quirks |= MT_QUIRK_CONTACT_CNT_ACCURATE;
-			*quirks &= ~MT_QUIRK_NOT_SEEN_MEANS_UP;
-			*quirks &= ~MT_QUIRK_VALID_IS_INRANGE;
-			*quirks &= ~MT_QUIRK_VALID_IS_CONFIDENCE;
-		}
-		break;
 	}
 }
 
@@ -1346,6 +1339,11 @@ static const struct hid_device_id mt_devices[] = {
 
 	/* Generic MT device */
 	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_MULTITOUCH, HID_ANY_ID, HID_ANY_ID) },
+
+	/* Generic Win 8 certified MT device */
+	{  .driver_data = MT_CLS_WIN_8,
+		HID_DEVICE(HID_BUS_ANY, HID_GROUP_MULTITOUCH_WIN_8,
+			HID_ANY_ID, HID_ANY_ID) },
 	{ }
 };
 MODULE_DEVICE_TABLE(hid, mt_devices);
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 5a4e789..24f1197 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -295,6 +295,7 @@ struct hid_item {
 #define HID_GROUP_GENERIC			0x0001
 #define HID_GROUP_MULTITOUCH			0x0002
 #define HID_GROUP_SENSOR_HUB			0x0003
+#define HID_GROUP_MULTITOUCH_WIN_8		0x0004
 
 /*
  * This is the global environment of the parser. This information is
@@ -533,6 +534,8 @@ static inline void hid_set_drvdata(struct hid_device *hdev, void *data)
 #define HID_GLOBAL_STACK_SIZE 4
 #define HID_COLLECTION_STACK_SIZE 4
 
+#define HID_SCAN_FLAG_MT_WIN_8			0x00000001
+
 struct hid_parser {
 	struct hid_global     global;
 	struct hid_global     global_stack[HID_GLOBAL_STACK_SIZE];
@@ -541,6 +544,7 @@ struct hid_parser {
 	unsigned              collection_stack[HID_COLLECTION_STACK_SIZE];
 	unsigned              collection_stack_ptr;
 	struct hid_device    *device;
+	unsigned              scan_flags;
 };
 
 struct hid_class_descriptor {
-- 
1.8.3.1

[PATCH v3 3/3] HID: do not init input reports for Win 8 multitouch devices

[Benjamin Tissoires]

Some multitouch screens do not like to be polled for input reports.
However, the Win8 spec says that all touches should be sent during
each report, making the initialization of reports unnecessary.
The Win7 spec is less precise, so do not use this for those devices.

Add the quirk HID_QUIRK_NO_INIT_INPUT_REPORTS so that we do not have to
introduce a quirk for each problematic device. This quirk makes the driver
behave the same way the Win 8 does. It actually retrieves the features,
but not the inputs.

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

no changes in v3
no changes in v2

 drivers/hid/hid-multitouch.c  | 12 ++++++++++++
 drivers/hid/usbhid/hid-core.c | 11 ++++++++---
 include/linux/hid.h           |  1 +
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index c28ef86..ac28f08 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -951,6 +951,18 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	hdev->quirks |= HID_QUIRK_MULTI_INPUT;
 	hdev->quirks |= HID_QUIRK_NO_EMPTY_INPUT;
 
+	/*
+	 * Handle special quirks for Windows 8 certified devices.
+	 */
+	if (id->group == HID_GROUP_MULTITOUCH_WIN_8)
+		/*
+		 * Some multitouch screens do not like to be polled for input
+		 * reports. Fortunately, the Win8 spec says that all touches
+		 * should be sent during each report, making the initialization
+		 * of input reports unnecessary.
+		 */
+		hdev->quirks |= HID_QUIRK_NO_INIT_INPUT_REPORTS;
+
 	td = devm_kzalloc(&hdev->dev, sizeof(struct mt_device), GFP_KERNEL);
 	if (!td) {
 		dev_err(&hdev->dev, "cannot allocate multitouch data\n");
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index bd38cdf..44df131 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -750,12 +750,17 @@ void usbhid_init_reports(struct hid_device *hid)
 {
 	struct hid_report *report;
 	struct usbhid_device *usbhid = hid->driver_data;
+	struct hid_report_enum *report_enum;
 	int err, ret;
 
-	list_for_each_entry(report, &hid->report_enum[HID_INPUT_REPORT].report_list, list)
-		usbhid_submit_report(hid, report, USB_DIR_IN);
+	if (!(hid->quirks & HID_QUIRK_NO_INIT_INPUT_REPORTS)) {
+		report_enum = &hid->report_enum[HID_INPUT_REPORT];
+		list_for_each_entry(report, &report_enum->report_list, list)
+			usbhid_submit_report(hid, report, USB_DIR_IN);
+	}
 
-	list_for_each_entry(report, &hid->report_enum[HID_FEATURE_REPORT].report_list, list)
+	report_enum = &hid->report_enum[HID_FEATURE_REPORT];
+	list_for_each_entry(report, &report_enum->report_list, list)
 		usbhid_submit_report(hid, report, USB_DIR_IN);
 
 	err = 0;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 24f1197..d421848 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -283,6 +283,7 @@ struct hid_item {
 #define HID_QUIRK_MULTI_INPUT			0x00000040
 #define HID_QUIRK_HIDINPUT_FORCE		0x00000080
 #define HID_QUIRK_NO_EMPTY_INPUT		0x00000100
+#define HID_QUIRK_NO_INIT_INPUT_REPORTS		0x00000200
 #define HID_QUIRK_SKIP_OUTPUT_REPORTS		0x00010000
 #define HID_QUIRK_FULLSPEED_INTERVAL		0x10000000
 #define HID_QUIRK_NO_INIT_REPORTS		0x20000000
-- 
1.8.3.1

Re: [PATCH v3 0/3] HID: Win 8 multitouch panels detection in core

[Henrik Rydberg]

Hi Benjamin,

thanks for the patches, it all looks fine now.

> this is the v3 of this patch series.
> 
> Srinivas, I still did not added your tested-by as I made minors modifications of
> the patches 1 and 2. Now only the patch 1 will impact sensor_hub, and patch 2
> will not impact this. Maybe just wait for Henrik to put his "Reviewed-by" before
> giving one last test.

Here goes:

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

Cheers,
Henrik

Re: [PATCH v3 0/3] HID: Win 8 multitouch panels detection in core

[Jiri Kosina]

On Thu, 22 Aug 2013, Henrik Rydberg wrote:

> > this is the v3 of this patch series.
> > 
> > Srinivas, I still did not added your tested-by as I made minors modifications of
> > the patches 1 and 2. Now only the patch 1 will impact sensor_hub, and patch 2
> > will not impact this. Maybe just wait for Henrik to put his "Reviewed-by" before
> > giving one last test.
> 
> Here goes:
> 
>     Reviewed-by: Henrik Rydberg <rydberg@euromail.se>

Thanks guys. Not applying this just yet, waiting for Tested-by: from 
Srinivas.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v3 0/3] HID: Win 8 multitouch panels detection in core

[Srinivas Pandruvada]

On 08/26/2013 04:43 AM, Jiri Kosina wrote:
> On Thu, 22 Aug 2013, Henrik Rydberg wrote:
>
>>> this is the v3 of this patch series.
>>>
>>> Srinivas, I still did not added your tested-by as I made minors modifications of
>>> the patches 1 and 2. Now only the patch 1 will impact sensor_hub, and patch 2
>>> will not impact this. Maybe just wait for Henrik to put his "Reviewed-by" before
>>> giving one last test.
>> Here goes:
>>
>>      Reviewed-by: Henrik Rydberg <rydberg@euromail.se>
> Thanks guys. Not applying this just yet, waiting for Tested-by: from
> Srinivas.
>
Tested-by: Srinivas Pandruvada<srinivas.pandruvada@linux.intel.com>

Re: [PATCH v3 0/3] HID: Win 8 multitouch panels detection in core

[Jiri Kosina]

On Mon, 26 Aug 2013, Srinivas Pandruvada wrote:

> > > > Srinivas, I still did not added your tested-by as I made minors
> > > > modifications of
> > > > the patches 1 and 2. Now only the patch 1 will impact sensor_hub, and
> > > > patch 2
> > > > will not impact this. Maybe just wait for Henrik to put his
> > > > "Reviewed-by" before
> > > > giving one last test.
> > > Here goes:
> > > 
> > >      Reviewed-by: Henrik Rydberg <rydberg@euromail.se>
> > Thanks guys. Not applying this just yet, waiting for Tested-by: from
> > Srinivas.
> > 
> Tested-by: Srinivas Pandruvada<srinivas.pandruvada@linux.intel.com>

Applied, thanks everybody.

-- 
Jiri Kosina
SUSE Labs