[PATCH 0/3] Change in handling different input device in hid-multitouch

[PATCH 0/3] Change in handling different input device in hid-multitouch

[Benjamin Tissoires]

Hi guys,

well, I have been reported a brand new device, which, as always presents a new
way of dealing with multitouch and mouse and soever.
The new fancy stuff is capable to report mouse and touch, whereas by default,
we consider mouse as garbage.

if you want to get an idea of the device:
http://www.pearl.de/a-PX2569-1002.shtml

So, we need to add a quirk for it, as usual, but I also cleaned up the way we
treat the different input devices within the driver. I expect some bikesheding
here, so I fire this right now until it's too late for 3.14 or 3.15 :)

Cheers,
Benjamin

Benjamin Tissoires (3):
  HID: multitouch: switch to a callback system for handling events
  HID: multitouch: introduce mt_protocol_generic
  HID: multitouch: add support of other generic collections in hid-mt

 drivers/hid/hid-ids.h        |   3 +
 drivers/hid/hid-multitouch.c | 205 +++++++++++++++++++++++++++++++++----------
 include/linux/hid.h          |   3 +
 3 files changed, 166 insertions(+), 45 deletions(-)

-- 
1.8.3.1

[PATCH 1/3] HID: multitouch: switch to a callback system for handling events

[Benjamin Tissoires]

In kernel v3.10, we added the support of pen in addition to touch because
some devices presents both pen and touch on the same HID device.
Now, we are seeing at least one device (ANTON Touch Pad) which presents
a multitouch collection and a mouse collection which are both valid.

Instead of adding a lot of ifs and messing up with the code, we can switch
to a callback system which allows to define in a better way each protocol
used by each input report.

Tested-by: Edel Maks <edelmaks@gmail.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-multitouch.c | 136 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 101 insertions(+), 35 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 1f0ba5c..1cdc21a 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -91,6 +91,21 @@ struct mt_fields {
 	unsigned int length;
 };
 
+struct mt_protocol {
+	char *suffix;
+	int (*event)(struct hid_device *hdev, struct hid_field *field,
+			struct hid_usage *usage, __s32 value);
+	void (*report)(struct hid_device *hdev, struct hid_report *report);
+	int (*input_mapping)(struct hid_device *hdev,
+			struct hid_input *hidinput, struct hid_field *field,
+			struct hid_usage *usage, 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);
+	void (*input_configured)(struct hid_device *hdev,
+				 struct hid_input *hidinput);
+};
+
 struct mt_device {
 	struct mt_slot curdata;	/* placeholder of incoming data */
 	struct mt_class mtclass;	/* our mt device class */
@@ -99,8 +114,7 @@ struct mt_device {
 	int cc_index;	/* contact count field index in the report */
 	int cc_value_index;	/* contact count value index in the field */
 	unsigned last_slot_field;	/* the last field of a slot */
-	unsigned mt_report_id;	/* the report ID of the multitouch device */
-	unsigned pen_report_id;	/* the report ID of the pen device */
+	struct mt_protocol protocols[HID_MAX_IDS]; /* per-id callbacks */
 	__s16 inputmode;	/* InputMode HID feature, -1 if non-existent */
 	__s16 inputmode_index;	/* InputMode HID feature index in the report */
 	__s16 maxcontact_report_id;	/* Maximum Contact Number HID feature,
@@ -119,6 +133,54 @@ struct mt_device {
 static void mt_post_parse_default_settings(struct mt_device *td);
 static void mt_post_parse(struct mt_device *td);
 
+static int mt_touch_input_mapping(struct hid_device *hdev,
+		struct hid_input *hi, struct hid_field *field,
+		struct hid_usage *usage, unsigned long **bit, int *max);
+static int mt_touch_input_mapped(struct hid_device *hdev,
+		struct hid_input *hi, struct hid_field *field,
+		struct hid_usage *usage, unsigned long **bit, int *max);
+static int mt_touch_event(struct hid_device *hid,
+		struct hid_field *field, struct hid_usage *usage,
+		__s32 value);
+static void mt_touch_report(struct hid_device *hid,
+		struct hid_report *report);
+static void mt_touch_input_configured(struct hid_device *hdev,
+		struct hid_input *hi);
+
+static struct mt_protocol mt_protocol_touch = {
+	.suffix = NULL,
+	.event = mt_touch_event,
+	.report = mt_touch_report,
+	.input_mapping = mt_touch_input_mapping,
+	.input_mapped = mt_touch_input_mapped,
+	.input_configured = mt_touch_input_configured,
+};
+
+static int mt_pen_input_mapping(struct hid_device *hdev,
+		struct hid_input *hi, struct hid_field *field,
+		struct hid_usage *usage, unsigned long **bit, int *max);
+static int mt_pen_input_mapped(struct hid_device *hdev,
+		struct hid_input *hi, struct hid_field *field,
+		struct hid_usage *usage, unsigned long **bit, int *max);
+static int mt_pen_event(struct hid_device *hid,
+		struct hid_field *field, struct hid_usage *usage,
+		__s32 value);
+static void mt_pen_report(struct hid_device *hid,
+		struct hid_report *report);
+static void mt_pen_input_configured(struct hid_device *hdev,
+		struct hid_input *hi);
+
+static struct mt_protocol mt_protocol_pen = {
+	.suffix = "Pen",
+	.event = mt_pen_event,
+	.report = mt_pen_report,
+	.input_mapping = mt_pen_input_mapping,
+	.input_mapped = mt_pen_input_mapped,
+	.input_configured = mt_pen_input_configured,
+};
+
+static struct mt_protocol mt_protocol_ignore = { 0 };
+
 /* classes of device behavior */
 #define MT_CLS_DEFAULT				0x0001
 
@@ -364,10 +426,6 @@ static int mt_pen_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		struct hid_field *field, struct hid_usage *usage,
 		unsigned long **bit, int *max)
 {
-	struct mt_device *td = hid_get_drvdata(hdev);
-
-	td->pen_report_id = field->report->id;
-
 	return 0;
 }
 
@@ -480,7 +538,6 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 		case HID_DG_CONTACTID:
 			mt_store_field(usage, td, hi);
 			td->touches_by_report++;
-			td->mt_report_id = field->report->id;
 			return 1;
 		case HID_DG_WIDTH:
 			hid_map_usage(hi, usage, bit, max,
@@ -776,57 +833,71 @@ 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)
 {
+	struct mt_device *td = hid_get_drvdata(hdev);
+	unsigned report_id = field->report->id;
+
 	/* Only map fields from TouchScreen or TouchPad collections.
 	* We need to ignore fields that belong to other collections
 	* such as Mouse that might have the same GenericDesktop usages. */
 	if (field->application != HID_DG_TOUCHSCREEN &&
 	    field->application != HID_DG_PEN &&
 	    field->application != HID_DG_TOUCHPAD)
-		return -1;
+		td->protocols[report_id] = mt_protocol_ignore;
 
 	if (field->physical == HID_DG_STYLUS)
-		return mt_pen_input_mapping(hdev, hi, field, usage, bit, max);
+		td->protocols[report_id] = mt_protocol_pen;
+
+	else if (field->application == HID_DG_TOUCHSCREEN ||
+	    field->application == HID_DG_PEN ||
+	    field->application == HID_DG_TOUCHPAD)
+		td->protocols[report_id] = mt_protocol_touch;
 
-	return mt_touch_input_mapping(hdev, hi, field, usage, bit, max);
+	if (td->protocols[report_id].input_mapping)
+		return td->protocols[report_id].input_mapping(hdev, hi,
+				field, usage, bit, max);
+
+	/* ignore if no protocol is given */
+	return -1;
 }
 
 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 (field->physical == HID_DG_STYLUS)
-		return mt_pen_input_mapped(hdev, hi, field, usage, bit, max);
+	struct mt_device *td = hid_get_drvdata(hdev);
+	unsigned report_id = field->report->id;
+
+	if (td->protocols[report_id].input_mapped)
+		return td->protocols[report_id].input_mapped(hdev, hi, field,
+				usage, bit, max);
 
-	return mt_touch_input_mapped(hdev, hi, field, usage, bit, max);
+	/* ignore if no protocol is given */
+	return -1;
 }
 
 static int mt_event(struct hid_device *hid, struct hid_field *field,
 				struct hid_usage *usage, __s32 value)
 {
 	struct mt_device *td = hid_get_drvdata(hid);
+	unsigned report_id = field->report->id;
 
-	if (field->report->id == td->mt_report_id)
-		return mt_touch_event(hid, field, usage, value);
-
-	if (field->report->id == td->pen_report_id)
-		return mt_pen_event(hid, field, usage, value);
+	if (td->protocols[report_id].event)
+		return td->protocols[report_id].event(hid, field, usage, value);
 
-	/* ignore other reports */
+	/* ignore if no protocol is given */
 	return 1;
 }
 
 static void mt_report(struct hid_device *hid, struct hid_report *report)
 {
 	struct mt_device *td = hid_get_drvdata(hid);
+	unsigned report_id = report->id;
 
 	if (!(hid->claimed & HID_CLAIMED_INPUT))
 		return;
 
-	if (report->id == td->mt_report_id)
-		mt_touch_report(hid, report);
-
-	if (report->id == td->pen_report_id)
-		mt_pen_report(hid, report);
+	if (td->protocols[report_id].report)
+		return td->protocols[report_id].report(hid, report);
 }
 
 static void mt_set_input_mode(struct hid_device *hdev)
@@ -906,17 +977,14 @@ static void mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
 {
 	struct mt_device *td = hid_get_drvdata(hdev);
 	char *name;
-	const char *suffix = NULL;
-
-	if (hi->report->id == td->mt_report_id)
-		mt_touch_input_configured(hdev, hi);
+	const char *suffix;
+	unsigned report_id = hi->report->id;
 
-	if (hi->report->field[0]->physical == HID_DG_STYLUS) {
-		suffix = "Pen";
-		mt_pen_input_configured(hdev, hi);
-	}
+	if (td->protocols[report_id].input_configured)
+		td->protocols[report_id].input_configured(hdev, hi);
 
-	if (suffix) {
+	if (td->protocols[report_id].suffix) {
+		suffix = td->protocols[report_id].suffix;
 		name = devm_kzalloc(&hi->input->dev,
 				    strlen(hdev->name) + strlen(suffix) + 2,
 				    GFP_KERNEL);
@@ -974,8 +1042,6 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	td->inputmode = -1;
 	td->maxcontact_report_id = -1;
 	td->cc_index = -1;
-	td->mt_report_id = -1;
-	td->pen_report_id = -1;
 	hid_set_drvdata(hdev, td);
 
 	td->fields = devm_kzalloc(&hdev->dev, sizeof(struct mt_fields),
-- 
1.8.3.1

[PATCH 2/3] HID: multitouch: introduce mt_protocol_generic

[Benjamin Tissoires]

most of the protocol callbacks used by the pen protocol are generic,
they just ask hid-input to treat the incoming data.
Introduce a generic protocol to avoid having to copy/paste the pens
callbacks for other kinds of input reports.

Tested-by: Edel Maks <edelmaks@gmail.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-multitouch.c | 42 ++++++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 1cdc21a..4dd6c6c 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -156,26 +156,36 @@ static struct mt_protocol mt_protocol_touch = {
 	.input_configured = mt_touch_input_configured,
 };
 
-static int mt_pen_input_mapping(struct hid_device *hdev,
+static int mt_generic_input_mapping(struct hid_device *hdev,
 		struct hid_input *hi, struct hid_field *field,
 		struct hid_usage *usage, unsigned long **bit, int *max);
-static int mt_pen_input_mapped(struct hid_device *hdev,
+static int mt_generic_input_mapped(struct hid_device *hdev,
 		struct hid_input *hi, struct hid_field *field,
 		struct hid_usage *usage, unsigned long **bit, int *max);
-static int mt_pen_event(struct hid_device *hid,
+static int mt_generic_event(struct hid_device *hid,
 		struct hid_field *field, struct hid_usage *usage,
 		__s32 value);
-static void mt_pen_report(struct hid_device *hid,
+static void mt_generic_report(struct hid_device *hid,
 		struct hid_report *report);
+
+static struct mt_protocol mt_protocol_generic = {
+	.suffix = NULL,
+	.event = mt_generic_event,
+	.report = mt_generic_report,
+	.input_mapping = mt_generic_input_mapping,
+	.input_mapped = mt_generic_input_mapped,
+	.input_configured = NULL,
+};
+
 static void mt_pen_input_configured(struct hid_device *hdev,
 		struct hid_input *hi);
 
 static struct mt_protocol mt_protocol_pen = {
 	.suffix = "Pen",
-	.event = mt_pen_event,
-	.report = mt_pen_report,
-	.input_mapping = mt_pen_input_mapping,
-	.input_mapped = mt_pen_input_mapped,
+	.event = mt_generic_event,
+	.report = mt_generic_report,
+	.input_mapping = mt_generic_input_mapping,
+	.input_mapped = mt_generic_input_mapped,
 	.input_configured = mt_pen_input_configured,
 };
 
@@ -422,28 +432,28 @@ static void mt_store_field(struct hid_usage *usage, struct mt_device *td,
 	f->usages[f->length++] = usage->hid;
 }
 
-static int mt_pen_input_mapping(struct hid_device *hdev, struct hid_input *hi,
-		struct hid_field *field, struct hid_usage *usage,
-		unsigned long **bit, int *max)
+static int mt_generic_input_mapping(struct hid_device *hdev,
+		struct hid_input *hi, struct hid_field *field,
+		struct hid_usage *usage, unsigned long **bit, int *max)
 {
 	return 0;
 }
 
-static int mt_pen_input_mapped(struct hid_device *hdev, struct hid_input *hi,
-		struct hid_field *field, struct hid_usage *usage,
-		unsigned long **bit, int *max)
+static int mt_generic_input_mapped(struct hid_device *hdev,
+		struct hid_input *hi, struct hid_field *field,
+		struct hid_usage *usage, unsigned long **bit, int *max)
 {
 	return 0;
 }
 
-static int mt_pen_event(struct hid_device *hid, struct hid_field *field,
+static int mt_generic_event(struct hid_device *hid, struct hid_field *field,
 				struct hid_usage *usage, __s32 value)
 {
 	/* let hid-input handle it */
 	return 0;
 }
 
-static void mt_pen_report(struct hid_device *hid, struct hid_report *report)
+static void mt_generic_report(struct hid_device *hid, struct hid_report *report)
 {
 	struct hid_field *field = report->field[0];
 
-- 
1.8.3.1

[patch 3/3] HID: multitouch: add support of other generic collections in hid-mt

[Benjamin Tissoires]

The ANTEC Touch Pad is a device which can switch from a multitouch
touchpad to a mouse. It thus presents several generic collections which
are currently ignored by hid-multitouch. Enable them by using the generic
protocol. Adding also a suffix for them depending on their application.

Reported-by: Edel Maks <edelmaks@gmail.com>
Tested-by: Edel Maks <edelmaks@gmail.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-ids.h        |  3 +++
 drivers/hid/hid-multitouch.c | 43 ++++++++++++++++++++++++++++++++++++++++---
 include/linux/hid.h          |  3 +++
 3 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 2bf397f..4221494 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -67,6 +67,9 @@
 #define USB_VENDOR_ID_ALPS		0x0433
 #define USB_DEVICE_ID_IBM_GAMEPAD	0x1101
 
+#define USB_VENDOR_ID_ANTON		0x1130
+#define USB_DEVICE_ID_ANTON_TOUCH_PAD	0x3101
+
 #define USB_VENDOR_ID_APPLE		0x05ac
 #define USB_DEVICE_ID_APPLE_MIGHTYMOUSE	0x0304
 #define USB_DEVICE_ID_APPLE_MAGICMOUSE	0x030d
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 4dd6c6c..502e5bb 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -84,6 +84,7 @@ struct mt_class {
 	__s32 sn_pressure;	/* Signal/noise ratio for pressure events */
 	__u8 maxcontacts;
 	bool is_indirect;	/* true for touchpads */
+	bool export_all_inputs;	/* do not ignore mouse, keyboards, etc... */
 };
 
 struct mt_fields {
@@ -217,6 +218,7 @@ static struct mt_protocol mt_protocol_ignore = { 0 };
 #define MT_CLS_FLATFROG				0x0107
 #define MT_CLS_GENERALTOUCH_TWOFINGERS		0x0108
 #define MT_CLS_GENERALTOUCH_PWT_TENFINGERS	0x0109
+#define MT_CLS_ANTON_TP				0x010a
 
 #define MT_DEFAULT_MAXCONTACT	10
 #define MT_MAX_MAXCONTACT	250
@@ -329,13 +331,18 @@ static struct mt_class mt_classes[] = {
 		.quirks	= MT_QUIRK_NOT_SEEN_MEANS_UP |
 			MT_QUIRK_SLOT_IS_CONTACTID
 	},
-
 	{ .name = MT_CLS_FLATFROG,
 		.quirks = MT_QUIRK_NOT_SEEN_MEANS_UP |
 			MT_QUIRK_NO_AREA,
 		.sn_move = 2048,
 		.maxcontacts = 40,
 	},
+	{ .name = MT_CLS_ANTON_TP,
+		.quirks = MT_QUIRK_ALWAYS_VALID |
+			MT_QUIRK_CONTACT_CNT_ACCURATE,
+		.is_indirect = true,
+		.export_all_inputs = true,
+	},
 	{ }
 };
 
@@ -846,10 +853,32 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 	struct mt_device *td = hid_get_drvdata(hdev);
 	unsigned report_id = field->report->id;
 
+	if (td->mtclass.export_all_inputs) {
+		td->protocols[report_id] = mt_protocol_generic;
+		switch (field->application) {
+		case HID_GD_KEYPAD:
+			td->protocols[report_id].suffix = "Keypad";
+			break;
+		case HID_GD_MOUSE:
+			td->protocols[report_id].suffix = "Mouse";
+			break;
+		case HID_DG_TOUCHSCREEN:
+			td->protocols[report_id].suffix = "Touchscreen";
+			break;
+		case HID_GD_SYSTEM_CONTROL:
+			td->protocols[report_id].suffix = "System Control";
+			break;
+		case HID_CP_CONSUMER_CONTROL:
+			td->protocols[report_id].suffix = "Consumer Control";
+			break;
+		}
+	}
+
 	/* Only map fields from TouchScreen or TouchPad collections.
 	* We need to ignore fields that belong to other collections
 	* such as Mouse that might have the same GenericDesktop usages. */
-	if (field->application != HID_DG_TOUCHSCREEN &&
+	if (!td->mtclass.export_all_inputs &&
+	    field->application != HID_DG_TOUCHSCREEN &&
 	    field->application != HID_DG_PEN &&
 	    field->application != HID_DG_TOUCHPAD)
 		td->protocols[report_id] = mt_protocol_ignore;
@@ -859,8 +888,11 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 
 	else if (field->application == HID_DG_TOUCHSCREEN ||
 	    field->application == HID_DG_PEN ||
-	    field->application == HID_DG_TOUCHPAD)
+	    field->application == HID_DG_TOUCHPAD) {
 		td->protocols[report_id] = mt_protocol_touch;
+		if (td->mtclass.export_all_inputs)
+			td->protocols[report_id].suffix = "Touch";
+	}
 
 	if (td->protocols[report_id].input_mapping)
 		return td->protocols[report_id].input_mapping(hdev, hi,
@@ -1128,6 +1160,11 @@ static const struct hid_device_id mt_devices[] = {
 		MT_USB_DEVICE(USB_VENDOR_ID_ACTIONSTAR,
 			USB_DEVICE_ID_ACTIONSTAR_1011) },
 
+	/* Anton Touch Pad */
+	{ .driver_data = MT_CLS_ANTON_TP,
+		MT_USB_DEVICE(USB_VENDOR_ID_ANTON,
+			USB_DEVICE_ID_ANTON_TOUCH_PAD) },
+
 	/* Atmel panels */
 	{ .driver_data = MT_CLS_SERIAL,
 		MT_USB_DEVICE(USB_VENDOR_ID_ATMEL,
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 31b9d29..2b2041a 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -201,6 +201,7 @@ struct hid_item {
 #define HID_GD_VBRZ		0x00010045
 #define HID_GD_VNO		0x00010046
 #define HID_GD_FEATURE		0x00010047
+#define HID_GD_SYSTEM_CONTROL	0x00010080
 #define HID_GD_UP		0x00010090
 #define HID_GD_DOWN		0x00010091
 #define HID_GD_RIGHT		0x00010092
@@ -208,6 +209,8 @@ struct hid_item {
 
 #define HID_DC_BATTERYSTRENGTH	0x00060020
 
+#define HID_CP_CONSUMER_CONTROL	0x000c0001
+
 #define HID_DG_DIGITIZER	0x000d0001
 #define HID_DG_PEN		0x000d0002
 #define HID_DG_LIGHTPEN		0x000d0003
-- 
1.8.3.1

Re: [patch 3/3] HID: multitouch: add support of other generic collections in hid-mt

[Henrik Rydberg]

Hi Benjamin,

> The ANTEC Touch Pad is a device which can switch from a multitouch
> touchpad to a mouse. It thus presents several generic collections which
> are currently ignored by hid-multitouch. Enable them by using the generic
> protocol. Adding also a suffix for them depending on their application.

In what way does this and the preceeding patches differ from "else if (is_pen ||
export_all_inputs)"? Adding a functional pattern which then is converted to a
generic case, such that the usual branching is bound to occur anyways, seems
unnecessary.

Thanks,
Henrik

Re: [patch 3/3] HID: multitouch: add support of other generic collections in hid-mt

[Jiri Kosina]

On Sat, 21 Dec 2013, Henrik Rydberg wrote:

> > The ANTEC Touch Pad is a device which can switch from a multitouch
> > touchpad to a mouse. It thus presents several generic collections which
> > are currently ignored by hid-multitouch. Enable them by using the generic
> > protocol. Adding also a suffix for them depending on their application.
> 
> In what way does this and the preceeding patches differ from "else if (is_pen ||
> export_all_inputs)"? Adding a functional pattern which then is converted to a
> generic case, such that the usual branching is bound to occur anyways, seems
> unnecessary.

Benjamin,

did you have chance to do any followup work on this?

If this is to go into 3.14, it's the time to have the thing finalized.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [patch 3/3] HID: multitouch: add support of other generic collections in hid-mt

[Benjamin Tissoires]

On 03/01/14 04:56, Jiri Kosina wrote:
> On Sat, 21 Dec 2013, Henrik Rydberg wrote:
> 
>>> The ANTEC Touch Pad is a device which can switch from a multitouch
>>> touchpad to a mouse. It thus presents several generic collections which
>>> are currently ignored by hid-multitouch. Enable them by using the generic
>>> protocol. Adding also a suffix for them depending on their application.
>>
>> In what way does this and the preceeding patches differ from "else if (is_pen ||
>> export_all_inputs)"? Adding a functional pattern which then is converted to a
>> generic case, such that the usual branching is bound to occur anyways, seems
>> unnecessary.
> 
> Benjamin,
> 
> did you have chance to do any followup work on this?
> 
> If this is to go into 3.14, it's the time to have the thing finalized.
> 

Hi Jiri,

well, I'm still not sure what to answer to address Henrik's comments. I
will work on that later in the week I think, but currently I am focusing
on hid-logitech-dj and others, which is rather time consuming.

I think Maks is fine with the current status (he still have to use a
backport due to an arm platform), so let's skip this for 3.15.

Cheers,
Benjamin

[PATCH 3/3] HID: multitouch: add support of other generic collections in hid-mt

[Benjamin Tissoires]

The ANTON Touch Pad is a device which can switch from a multitouch
touchpad to a mouse. It thus presents several generic collections which
are currently ignored by hid-multitouch. Enable them by not ignoring
them in mt_input_mapping.
Adding also a suffix for them depending on their application.

Reported-by: Edel Maks <edelmaks@gmail.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-ids.h        |  3 ++
 drivers/hid/hid-multitouch.c | 82 ++++++++++++++++++++++++++++++++++++++++----
 include/linux/hid.h          |  3 ++
 3 files changed, 82 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 00be0d0..7045a71 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -67,6 +67,9 @@
 #define USB_VENDOR_ID_ALPS		0x0433
 #define USB_DEVICE_ID_IBM_GAMEPAD	0x1101
 
+#define USB_VENDOR_ID_ANTON		0x1130
+#define USB_DEVICE_ID_ANTON_TOUCH_PAD	0x3101
+
 #define USB_VENDOR_ID_APPLE		0x05ac
 #define USB_DEVICE_ID_APPLE_MIGHTYMOUSE	0x0304
 #define USB_DEVICE_ID_APPLE_MAGICMOUSE	0x030d
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 8250cc0..0d31139 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -84,6 +84,7 @@ struct mt_class {
 	__s32 sn_pressure;	/* Signal/noise ratio for pressure events */
 	__u8 maxcontacts;
 	bool is_indirect;	/* true for touchpads */
+	bool export_all_inputs;	/* do not ignore mouse, keyboards, etc... */
 };
 
 struct mt_fields {
@@ -133,6 +134,7 @@ static void mt_post_parse(struct mt_device *td);
 /* reserved					0x0010 */
 /* reserved					0x0011 */
 #define MT_CLS_WIN_8				0x0012
+#define MT_CLS_EXPORT_ALL_INPUTS		0x0013
 
 /* vendor specific classes */
 #define MT_CLS_3M				0x0101
@@ -196,6 +198,10 @@ static struct mt_class mt_classes[] = {
 			MT_QUIRK_IGNORE_DUPLICATES |
 			MT_QUIRK_HOVERING |
 			MT_QUIRK_CONTACT_CNT_ACCURATE },
+	{ .name = MT_CLS_EXPORT_ALL_INPUTS,
+		.quirks = MT_QUIRK_ALWAYS_VALID |
+			MT_QUIRK_CONTACT_CNT_ACCURATE,
+		.export_all_inputs = true },
 
 	/*
 	 * vendor specific classes
@@ -718,28 +724,52 @@ 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)
 {
-	/* Only map fields from TouchScreen or TouchPad collections.
-	* We need to ignore fields that belong to other collections
-	* such as Mouse that might have the same GenericDesktop usages. */
-	if (field->application != HID_DG_TOUCHSCREEN &&
+	struct mt_device *td = hid_get_drvdata(hdev);
+
+	/*
+	 * If mtclass.export_all_inputs is not set, only map fields from
+	 * TouchScreen or TouchPad collections. We need to ignore fields
+	 * that belong to other collections such as Mouse that might have
+	 * the same GenericDesktop usages.
+	 */
+	if (!td->mtclass.export_all_inputs &&
+	    field->application != HID_DG_TOUCHSCREEN &&
 	    field->application != HID_DG_PEN &&
 	    field->application != HID_DG_TOUCHPAD)
 		return -1;
 
+	/*
+	 * some egalax touchscreens have "application == HID_DG_TOUCHSCREEN"
+	 * for the stylus.
+	 */
 	if (field->physical == HID_DG_STYLUS)
 		return 0;
 
-	return mt_touch_input_mapping(hdev, hi, field, usage, bit, max);
+	if (field->application == HID_DG_TOUCHSCREEN ||
+	    field->application == HID_DG_TOUCHPAD)
+		return mt_touch_input_mapping(hdev, hi, field, usage, bit, max);
+
+	/* let hid-core decide for the others */
+	return 0;
 }
 
 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)
 {
+	/*
+	 * some egalax touchscreens have "application == HID_DG_TOUCHSCREEN"
+	 * for the stylus.
+	 */
 	if (field->physical == HID_DG_STYLUS)
 		return 0;
 
-	return mt_touch_input_mapped(hdev, hi, field, usage, bit, max);
+	if (field->application == HID_DG_TOUCHSCREEN ||
+	    field->application == HID_DG_TOUCHPAD)
+		return mt_touch_input_mapped(hdev, hi, field, usage, bit, max);
+
+	/* let hid-core decide for the others */
+	return 0;
 }
 
 static int mt_event(struct hid_device *hid, struct hid_field *field,
@@ -846,14 +876,49 @@ static void mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
 	struct mt_device *td = hid_get_drvdata(hdev);
 	char *name;
 	const char *suffix = NULL;
+	struct hid_field *field = hi->report->field[0];
 
 	if (hi->report->id == td->mt_report_id)
 		mt_touch_input_configured(hdev, hi);
 
+	/*
+	 * some egalax touchscreens have "application == HID_DG_TOUCHSCREEN"
+	 * for the stylus. Check this first, and then rely on the application
+	 * field.
+	 */
 	if (hi->report->field[0]->physical == HID_DG_STYLUS) {
 		suffix = "Pen";
 		/* force BTN_STYLUS to allow tablet matching in udev */
 		__set_bit(BTN_STYLUS, hi->input->keybit);
+	} else {
+		switch (field->application) {
+		case HID_GD_KEYBOARD:
+			suffix = "Keyboard";
+			break;
+		case HID_GD_KEYPAD:
+			suffix = "Keypad";
+			break;
+		case HID_GD_MOUSE:
+			suffix = "Mouse";
+			break;
+		case HID_DG_STYLUS:
+			suffix = "Pen";
+			/* force BTN_STYLUS to allow tablet matching in udev */
+			__set_bit(BTN_STYLUS, hi->input->keybit);
+			break;
+		case HID_DG_TOUCHSCREEN:
+			/* we do not set suffix = "Touchscreen" */
+			break;
+		case HID_GD_SYSTEM_CONTROL:
+			suffix = "System Control";
+			break;
+		case HID_CP_CONSUMER_CONTROL:
+			suffix = "Consumer Control";
+			break;
+		default:
+			suffix = "UNKNOWN";
+			break;
+		}
 	}
 
 	if (suffix) {
@@ -992,6 +1057,11 @@ static const struct hid_device_id mt_devices[] = {
 		MT_USB_DEVICE(USB_VENDOR_ID_3M,
 			USB_DEVICE_ID_3M3266) },
 
+	/* Anton devices */
+	{ .driver_data = MT_CLS_EXPORT_ALL_INPUTS,
+		MT_USB_DEVICE(USB_VENDOR_ID_ANTON,
+			USB_DEVICE_ID_ANTON_TOUCH_PAD) },
+
 	/* Atmel panels */
 	{ .driver_data = MT_CLS_SERIAL,
 		MT_USB_DEVICE(USB_VENDOR_ID_ATMEL,
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 5eb282e..e224516 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -201,6 +201,7 @@ struct hid_item {
 #define HID_GD_VBRZ		0x00010045
 #define HID_GD_VNO		0x00010046
 #define HID_GD_FEATURE		0x00010047
+#define HID_GD_SYSTEM_CONTROL	0x00010080
 #define HID_GD_UP		0x00010090
 #define HID_GD_DOWN		0x00010091
 #define HID_GD_RIGHT		0x00010092
@@ -208,6 +209,8 @@ struct hid_item {
 
 #define HID_DC_BATTERYSTRENGTH	0x00060020
 
+#define HID_CP_CONSUMER_CONTROL	0x000c0001
+
 #define HID_DG_DIGITIZER	0x000d0001
 #define HID_DG_PEN		0x000d0002
 #define HID_DG_LIGHTPEN		0x000d0003
-- 
1.8.5.3