[PATCH 1/3] HID: hid-multitouch: fix input mode feature command

From: Benjamin Tissoires <redacted>

Zytronic panels shows a new way of setting the Input Mode feature.
This feature is put in the second usage in the HID feature, instead
of the first, as the majority of the multitouch devices.

This patch adds a detection step when the feature is presented to know
where the feature is located in the report. We can then trigger the right
command to the device. This removes the magic number "0" in the function
mt_set_input_mode.

Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/hid/hid-multitouch.c |   13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 61cc4cb..9a3891e 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c

@@ -83,6 +83,7 @@ struct mt_device {
 	unsigned last_field_index;	/* last field index of the report */
 	unsigned last_slot_field;	/* the last field of a slot */
 	__s8 inputmode;		/* InputMode HID feature, -1 if non-existent */
+	__s8 inputmode_index;	/* InputMode HID feature index in the report */
 	__s8 maxcontact_report_id;	/* Maximum Contact Number HID feature,
 				   -1 if non-existent */
 	__u8 num_received;	/* how many contacts we received */

@@ -260,10 +261,20 @@ static void mt_feature_mapping(struct hid_device *hdev,
 		struct hid_field *field, struct hid_usage *usage)
 {
 	struct mt_device *td = hid_get_drvdata(hdev);
+	int i;
 
 	switch (usage->hid) {
 	case HID_DG_INPUTMODE:
 		td->inputmode = field->report->id;
+		td->inputmode_index = 0; /* has to be updated below */
+
+		for (i=0; i < field->maxusage; i++) {
+			if (field->usage[i].hid == usage->hid) {
+				td->inputmode_index = i;
+				break;
+			}
+		}
+
 		break;
 	case HID_DG_CONTACTMAX:
 		td->maxcontact_report_id = field->report->id;

@@ -618,7 +629,7 @@ static void mt_set_input_mode(struct hid_device *hdev)
 	re = &(hdev->report_enum[HID_FEATURE_REPORT]);
 	r = re->report_id_hash[td->inputmode];
 	if (r) {
-		r->field[0]->value[0] = 0x02;
+		r->field[0]->value[td->inputmode_index] = 0x02;
 		usbhid_submit_report(hdev, r, USB_DIR_OUT);
 	}
 }

-- 
1.7.10.2

From: Benjamin Tissoires <redacted>

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 <redacted>
---
 drivers/hid/hid-multitouch.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 9a3891e..a5ef877 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c

@@ -601,7 +601,10 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
 			return 0;
 		}
 
-		if (usage->hid == td->last_slot_field)
+		if (usage->hid == td->last_slot_field &&
+			usage == &field->usage[field->maxusage - 1])
+			/* we only take into account the last report
+			 * of a field if report_count > 1 */
 			mt_complete_slot(td);
 
 		if (field->index == td->last_field_index

-- 
1.7.10.2

From: Benjamin Tissoires <redacted>

Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/hid/Kconfig          |    1 +
 drivers/hid/hid-ids.h        |    3 +++
 drivers/hid/hid-multitouch.c |    5 +++++
 3 files changed, 9 insertions(+)

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index e9c68fe..bcaf3fa 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig

@@ -393,6 +393,7 @@ config HID_MULTITOUCH
 	  - Unitec Panels
 	  - XAT optical touch panels
 	  - Xiroku optical touch panels
+	  - Zytronic touch panels
 
 	  If unsure, say N.
 

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 734a2b9..c77bfdd 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h

@@ -802,6 +802,9 @@
 #define USB_VENDOR_ID_ZYDACRON	0x13EC
 #define USB_DEVICE_ID_ZYDACRON_REMOTE_CONTROL	0x0006
 
+#define USB_VENDOR_ID_ZYTRONIC		0x14c8
+#define USB_DEVICE_ID_ZYTRONIC_ZXY100	0x0005
+
 #define USB_VENDOR_ID_PRIMAX	0x0461
 #define USB_DEVICE_ID_PRIMAX_KEYBOARD	0x4e05
 

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index a5ef877..eb66abd 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c

@@ -1067,6 +1067,11 @@ static const struct hid_device_id mt_devices[] = {
 		MT_USB_DEVICE(USB_VENDOR_ID_XIROKU,
 			USB_DEVICE_ID_XIROKU_CSR2) },
 
+	/* Zytronic panels */
+	{ .driver_data = MT_CLS_SERIAL,
+		MT_USB_DEVICE(USB_VENDOR_ID_ZYTRONIC,
+			USB_DEVICE_ID_ZYTRONIC_ZXY100) },
+
 	/* Generic MT device */
 	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_MULTITOUCH, HID_ANY_ID, HID_ANY_ID) },
 	{ }

-- 
1.7.10.2

Hi Benjamin,

Or a separate function to keep thing neat?

 
 	switch (usage->hid) {
 	case HID_DG_INPUTMODE:
 		td->inputmode = field->report->id;
+		td->inputmode_index = 0; /* has to be updated below */

Isn't this zero already?

+
+		for (i=0; i < field->maxusage; i++) {
+			if (field->usage[i].hid == usage->hid) {
+				td->inputmode_index = i;
+				break;

The break here is an optimization of the dubious kind. ;-)
Or do we expect more than one occurence?

Thanks,
Henrik

Hi Benjamin,

This just looks so fragile... The report index should really be amount
the arguments to mt_event(). Since we rely on the pointer to reveal
the index, why not try something like (usage - &field->usage[0]) ==
(field->report_count - 1)? At least it shows clearly what we use the
point for.

 			mt_complete_slot(td);
 
 		if (field->index == td->last_field_index
-- 
1.7.10.2

Thanks,
Henrik

On Tue, 19 Jun 2012, benjamin.tissoires wrote:

From: Benjamin Tissoires <redacted>

Zytronic panels shows a new way of setting the Input Mode feature.
This feature is put in the second usage in the HID feature, instead
of the first, as the majority of the multitouch devices.

This patch adds a detection step when the feature is presented to know
where the feature is located in the report. We can then trigger the right
command to the device. This removes the magic number "0" in the function
mt_set_input_mode.

I have applied 1/3 and 3/3. For 2/3, Henrik raised concern which I'd like 
to have resolved first.

Thanks!

-- 
Jiri Kosina
SUSE Labs

Hi Jiri,

Thanks a lot for that.
My apologies. I didn't thanked Henrik and commented his review. So
thank you Henrik. (the "break" here is not an optimisation as
field->maxusage may be greater than the real report count).

I'm currently quite overloaded with the hid over i2c bus, and I'm
getting some very goods results. I'm stuck with the acpi part, but the
rest of it is working pretty good for a draft. I'll keep you inform
soon.

Cheers,
Benjamin

On Thu, Jun 28, 2012 at 10:31 AM, Jiri Kosina [off-list ref] wrote:

On Tue, 19 Jun 2012, benjamin.tissoires wrote:

quoted

From: Benjamin Tissoires <redacted>

Zytronic panels shows a new way of setting the Input Mode feature.
This feature is put in the second usage in the HID feature, instead
of the first, as the majority of the multitouch devices.

This patch adds a detection step when the feature is presented to know
where the feature is located in the report. We can then trigger the right
command to the device. This removes the magic number "0" in the function
mt_set_input_mode.

I have applied 1/3 and 3/3. For 2/3, Henrik raised concern which I'd like
to have resolved first.

Thanks!

--
Jiri Kosina
SUSE Labs

On Thu, 28 Jun 2012, Benjamin Tissoires wrote:

Thanks a lot for that.
My apologies. I didn't thanked Henrik and commented his review. So
thank you Henrik. (the "break" here is not an optimisation as
field->maxusage may be greater than the real report count).

Yes, I thought so, and therefore applied that patch. 2/3 I am still 
keeping unapplied.

I'm currently quite overloaded with the hid over i2c bus, and I'm 
getting some very goods results. I'm stuck with the acpi part, but the 
rest of it is working pretty good for a draft. I'll keep you inform 
soon.

Sounds great, looking forward to seeing the implementation :)

Thanks,

-- 
Jiri Kosina
SUSE Labs