2012-06-19 12:40:12

by Benjamin Tissoires

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

From: Benjamin Tissoires <[email protected]>

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 <[email protected]>
---
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


2012-06-19 12:40:17

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH 2/3] HID: hid-multitouch: support arrays for the split of the touches in a report

From: Benjamin Tissoires <[email protected]>

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 <[email protected]>
---
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

2012-06-19 12:40:24

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH 3/3] HID: hid-multitouch: add support for Zytronic panels

From: Benjamin Tissoires <[email protected]>

Signed-off-by: Benjamin Tissoires <[email protected]>
---
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

2012-06-20 16:29:59

by Henrik Rydberg

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

Hi Benjamin,

> 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 <[email protected]>
> ---
> 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;

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?

> + }
> + }
> +
> 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
>

Thanks,
Henrik

2012-06-20 16:51:17

by Henrik Rydberg

[permalink] [raw]
Subject: Re: [PATCH 2/3] HID: hid-multitouch: support arrays for the split of the touches in a report

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.
>
> This test guarantees that we split the touches on the last element
> in this array.
>
> Signed-off-by: Benjamin Tissoires <[email protected]>
> ---
> 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 */

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

2012-06-28 08:31:21

by Jiri Kosina

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

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

> From: Benjamin Tissoires <[email protected]>
>
> 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

2012-06-28 10:08:09

by Benjamin Tissoires

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

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 <[email protected]> wrote:
> On Tue, 19 Jun 2012, benjamin.tissoires wrote:
>
>> From: Benjamin Tissoires <[email protected]>
>>
>> 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

2012-06-28 10:10:48

by Jiri Kosina

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

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