2011-03-09 16:01:00

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH 0/4] Migrations to hid-multitouch, round 2

Hi guys,

This is the second version for this patch series.

Since last time, I integrated Henrik's comments.

Cheers,
Benjamin

drivers/hid/Kconfig | 21 +---
drivers/hid/Makefile | 3 -
drivers/hid/hid-3m-pct.c | 305 ------------------------------------------
drivers/hid/hid-cando.c | 276 --------------------------------------
drivers/hid/hid-multitouch.c | 110 +++++++++++++---
drivers/hid/hid-stantum.c | 286 ---------------------------------------
6 files changed, 97 insertions(+), 904 deletions(-)

;-)


2011-03-09 16:01:07

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH v2 1/4] hid-multitouch: Auto detection of maxcontacts

This patch enables support of autodetection of maxcontacts.
We can still manually provide maxcontact in case the device
lies on it.

Signed-off-by: Benjamin Tissoires <[email protected]>
---

Changes since v1:
- removed MT_CONTACTMAX_DEFAULT
- test the return value of kzalloc
- free td->slots when the device is removed
- check for td->slots in mt_event to avoid a race as it is
initialized after the device has been claimed.

drivers/hid/hid-multitouch.c | 45 ++++++++++++++++++++++++++++-------------
1 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 65e7f20..8a5fcc0 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -53,8 +53,9 @@ struct mt_device {
__s8 inputmode; /* InputMode HID feature, -1 if non-existent */
__u8 num_received; /* how many contacts we received */
__u8 num_expected; /* expected last contact index */
+ __u8 maxcontacts;
bool curvalid; /* is the current contact valid? */
- struct mt_slot slots[0]; /* first slot */
+ struct mt_slot *slots;
};

struct mt_class {
@@ -87,12 +88,12 @@ static int cypress_compute_slot(struct mt_device *td)
static int find_slot_from_contactid(struct mt_device *td)
{
int i;
- for (i = 0; i < td->mtclass->maxcontacts; ++i) {
+ for (i = 0; i < td->maxcontacts; ++i) {
if (td->slots[i].contactid == td->curdata.contactid &&
td->slots[i].touch_state)
return i;
}
- for (i = 0; i < td->mtclass->maxcontacts; ++i) {
+ for (i = 0; i < td->maxcontacts; ++i) {
if (!td->slots[i].seen_in_this_frame &&
!td->slots[i].touch_state)
return i;
@@ -106,7 +107,7 @@ static int find_slot_from_contactid(struct mt_device *td)
struct mt_class mt_classes[] = {
{ .name = MT_CLS_DEFAULT,
.quirks = MT_QUIRK_NOT_SEEN_MEANS_UP,
- .maxcontacts = 10 },
+ .maxcontacts = 2 },
{ .name = MT_CLS_DUAL_INRANGE_CONTACTID,
.quirks = MT_QUIRK_VALID_IS_INRANGE |
MT_QUIRK_SLOT_IS_CONTACTID,
@@ -126,9 +127,19 @@ struct mt_class mt_classes[] = {
static void mt_feature_mapping(struct hid_device *hdev,
struct hid_field *field, struct hid_usage *usage)
{
- if (usage->hid == HID_DG_INPUTMODE) {
- struct mt_device *td = hid_get_drvdata(hdev);
+ struct mt_device *td = hid_get_drvdata(hdev);
+
+ switch (usage->hid) {
+ case HID_DG_INPUTMODE:
td->inputmode = field->report->id;
+ break;
+ case HID_DG_CONTACTMAX:
+ td->maxcontacts = *(field->value);
+ if (td->mtclass->maxcontacts > td->maxcontacts)
+ /* check if the maxcontacts is given by the class */
+ td->maxcontacts = td->mtclass->maxcontacts;
+
+ break;
}
}

@@ -186,8 +197,7 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
td->last_slot_field = usage->hid;
return 1;
case HID_DG_CONTACTID:
- input_mt_init_slots(hi->input,
- td->mtclass->maxcontacts);
+ input_mt_init_slots(hi->input, td->maxcontacts);
td->last_slot_field = usage->hid;
return 1;
case HID_DG_WIDTH:
@@ -268,7 +278,7 @@ static void mt_complete_slot(struct mt_device *td)
if (td->curvalid) {
int slotnum = mt_compute_slot(td);

- if (slotnum >= 0 && slotnum < td->mtclass->maxcontacts)
+ if (slotnum >= 0 && slotnum < td->maxcontacts)
td->slots[slotnum] = td->curdata;
}
td->num_received++;
@@ -283,7 +293,7 @@ static void mt_emit_event(struct mt_device *td, struct input_dev *input)
{
int i;

- for (i = 0; i < td->mtclass->maxcontacts; ++i) {
+ for (i = 0; i < td->maxcontacts; ++i) {
struct mt_slot *s = &(td->slots[i]);
if ((td->mtclass->quirks & MT_QUIRK_NOT_SEEN_MEANS_UP) &&
!s->seen_in_this_frame) {
@@ -317,7 +327,7 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
struct mt_device *td = hid_get_drvdata(hid);
__s32 quirks = td->mtclass->quirks;

- if (hid->claimed & HID_CLAIMED_INPUT) {
+ if (hid->claimed & HID_CLAIMED_INPUT && td->slots) {
switch (usage->hid) {
case HID_DG_INRANGE:
if (quirks & MT_QUIRK_VALID_IS_INRANGE)
@@ -415,9 +425,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
*/
hdev->quirks |= HID_QUIRK_NO_INPUT_SYNC;

- td = kzalloc(sizeof(struct mt_device) +
- mtclass->maxcontacts * sizeof(struct mt_slot),
- GFP_KERNEL);
+ td = kzalloc(sizeof(struct mt_device), GFP_KERNEL);
if (!td) {
dev_err(&hdev->dev, "cannot allocate multitouch data\n");
return -ENOMEM;
@@ -434,6 +442,14 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
if (ret)
goto fail;

+ td->slots = kzalloc(td->maxcontacts * sizeof(struct mt_slot),
+ GFP_KERNEL);
+ if (!td->slots) {
+ dev_err(&hdev->dev, "cannot allocate multitouch slots\n");
+ ret = -ENOMEM;
+ goto fail;
+ }
+
mt_set_input_mode(hdev);

return 0;
@@ -455,6 +471,7 @@ static void mt_remove(struct hid_device *hdev)
{
struct mt_device *td = hid_get_drvdata(hdev);
hid_hw_stop(hdev);
+ kfree(td->slots);
kfree(td);
hid_set_drvdata(hdev, NULL);
}
--
1.7.4

2011-03-09 16:01:11

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH v2 2/4] hid-multitouch: migrate support for Stantum panels to the unified driver.

This patch merges hid-stantum to the generic multitouch driver.

Signed-off-by: Benjamin Tissoires <[email protected]>
---

no changes since v1 ;)

drivers/hid/Kconfig | 7 +-
drivers/hid/Makefile | 1 -
drivers/hid/hid-multitouch.c | 14 ++
drivers/hid/hid-stantum.c | 286 ------------------------------------------
4 files changed, 15 insertions(+), 293 deletions(-)
delete mode 100644 drivers/hid/hid-stantum.c

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index b152c91..1c9ca3ad 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -321,6 +321,7 @@ config HID_MULTITOUCH
- IrTouch Infrared USB panels
- Pixcir dual touch panels
- 'Sensing Win7-TwoFinger' panel by GeneralTouch
+ - Stantum multitouch panel

If unsure, say N.

@@ -487,12 +488,6 @@ config HID_SONY
---help---
Support for Sony PS3 controller.

-config HID_STANTUM
- tristate "Stantum multitouch panel"
- depends on USB_HID
- ---help---
- Support for Stantum multitouch panel.
-
config HID_SUNPLUS
tristate "Sunplus wireless desktop"
depends on USB_HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 04496ab..397344a 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -67,7 +67,6 @@ obj-$(CONFIG_HID_ROCCAT_PYRA) += hid-roccat-pyra.o
obj-$(CONFIG_HID_SAMSUNG) += hid-samsung.o
obj-$(CONFIG_HID_SMARTJOYPLUS) += hid-sjoy.o
obj-$(CONFIG_HID_SONY) += hid-sony.o
-obj-$(CONFIG_HID_STANTUM) += hid-stantum.o
obj-$(CONFIG_HID_SUNPLUS) += hid-sunplus.o
obj-$(CONFIG_HID_GREENASIA) += hid-gaff.o
obj-$(CONFIG_HID_THRUSTMASTER) += hid-tmff.o
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 8a5fcc0..525529c 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -71,6 +71,7 @@ struct mt_class {
#define MT_CLS_DUAL_INRANGE_CONTACTID 2
#define MT_CLS_DUAL_INRANGE_CONTACTNUMBER 3
#define MT_CLS_CYPRESS 4
+#define MT_CLS_STANTUM 5

/*
* these device-dependent functions determine what slot corresponds
@@ -120,6 +121,8 @@ struct mt_class mt_classes[] = {
.quirks = MT_QUIRK_NOT_SEEN_MEANS_UP |
MT_QUIRK_CYPRESS,
.maxcontacts = 10 },
+ { .name = MT_CLS_STANTUM,
+ .quirks = MT_QUIRK_VALID_IS_CONFIDENCE },

{ }
};
@@ -501,6 +504,17 @@ static const struct hid_device_id mt_devices[] = {
HID_USB_DEVICE(USB_VENDOR_ID_CANDO,
USB_DEVICE_ID_CANDO_PIXCIR_MULTI_TOUCH) },

+ /* Stantum panels */
+ { .driver_data = MT_CLS_STANTUM,
+ HID_USB_DEVICE(USB_VENDOR_ID_STANTUM,
+ USB_DEVICE_ID_MTP)},
+ { .driver_data = MT_CLS_STANTUM,
+ HID_USB_DEVICE(USB_VENDOR_ID_STANTUM,
+ USB_DEVICE_ID_MTP_STM)},
+ { .driver_data = MT_CLS_STANTUM,
+ HID_USB_DEVICE(USB_VENDOR_ID_STANTUM,
+ USB_DEVICE_ID_MTP_SITRONIX)},
+
{ }
};
MODULE_DEVICE_TABLE(hid, mt_devices);
diff --git a/drivers/hid/hid-stantum.c b/drivers/hid/hid-stantum.c
deleted file mode 100644
index b2be1d1..0000000
--- a/drivers/hid/hid-stantum.c
+++ /dev/null
@@ -1,286 +0,0 @@
-/*
- * HID driver for Stantum multitouch panels
- *
- * Copyright (c) 2009 Stephane Chatty <[email protected]>
- *
- */
-
-/*
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License as published by the Free
- * Software Foundation; either version 2 of the License, or (at your option)
- * any later version.
- */
-
-#include <linux/device.h>
-#include <linux/hid.h>
-#include <linux/module.h>
-#include <linux/slab.h>
-
-MODULE_AUTHOR("Stephane Chatty <[email protected]>");
-MODULE_DESCRIPTION("Stantum HID multitouch panels");
-MODULE_LICENSE("GPL");
-
-#include "hid-ids.h"
-
-struct stantum_data {
- __s32 x, y, z, w, h; /* x, y, pressure, width, height */
- __u16 id; /* touch id */
- bool valid; /* valid finger data, or just placeholder? */
- bool first; /* first finger in the HID packet? */
- bool activity; /* at least one active finger so far? */
-};
-
-static int stantum_input_mapping(struct hid_device *hdev, struct hid_input *hi,
- struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
-{
- switch (usage->hid & HID_USAGE_PAGE) {
-
- case HID_UP_GENDESK:
- switch (usage->hid) {
- case HID_GD_X:
- hid_map_usage(hi, usage, bit, max,
- EV_ABS, ABS_MT_POSITION_X);
- /* touchscreen emulation */
- input_set_abs_params(hi->input, ABS_X,
- field->logical_minimum,
- field->logical_maximum, 0, 0);
- return 1;
- case HID_GD_Y:
- hid_map_usage(hi, usage, bit, max,
- EV_ABS, ABS_MT_POSITION_Y);
- /* touchscreen emulation */
- input_set_abs_params(hi->input, ABS_Y,
- field->logical_minimum,
- field->logical_maximum, 0, 0);
- return 1;
- }
- return 0;
-
- case HID_UP_DIGITIZER:
- switch (usage->hid) {
- case HID_DG_INRANGE:
- case HID_DG_CONFIDENCE:
- case HID_DG_INPUTMODE:
- case HID_DG_DEVICEINDEX:
- case HID_DG_CONTACTCOUNT:
- case HID_DG_CONTACTMAX:
- return -1;
-
- case HID_DG_TIPSWITCH:
- /* touchscreen emulation */
- hid_map_usage(hi, usage, bit, max, EV_KEY, BTN_TOUCH);
- return 1;
-
- case HID_DG_WIDTH:
- hid_map_usage(hi, usage, bit, max,
- EV_ABS, ABS_MT_TOUCH_MAJOR);
- return 1;
- case HID_DG_HEIGHT:
- hid_map_usage(hi, usage, bit, max,
- EV_ABS, ABS_MT_TOUCH_MINOR);
- input_set_abs_params(hi->input, ABS_MT_ORIENTATION,
- 1, 1, 0, 0);
- return 1;
- case HID_DG_TIPPRESSURE:
- hid_map_usage(hi, usage, bit, max,
- EV_ABS, ABS_MT_PRESSURE);
- return 1;
-
- case HID_DG_CONTACTID:
- hid_map_usage(hi, usage, bit, max,
- EV_ABS, ABS_MT_TRACKING_ID);
- return 1;
-
- }
- return 0;
-
- case 0xff000000:
- /* no input-oriented meaning */
- return -1;
- }
-
- return 0;
-}
-
-static int stantum_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)
- clear_bit(usage->code, *bit);
-
- return 0;
-}
-
-/*
- * this function is called when a whole finger has been parsed,
- * so that it can decide what to send to the input layer.
- */
-static void stantum_filter_event(struct stantum_data *sd,
- struct input_dev *input)
-{
- bool wide;
-
- if (!sd->valid) {
- /*
- * touchscreen emulation: if the first finger is not valid and
- * there previously was finger activity, this is a release
- */
- if (sd->first && sd->activity) {
- input_event(input, EV_KEY, BTN_TOUCH, 0);
- sd->activity = false;
- }
- return;
- }
-
- input_event(input, EV_ABS, ABS_MT_TRACKING_ID, sd->id);
- input_event(input, EV_ABS, ABS_MT_POSITION_X, sd->x);
- input_event(input, EV_ABS, ABS_MT_POSITION_Y, sd->y);
-
- wide = (sd->w > sd->h);
- input_event(input, EV_ABS, ABS_MT_ORIENTATION, wide);
- input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, wide ? sd->w : sd->h);
- input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, wide ? sd->h : sd->w);
-
- input_event(input, EV_ABS, ABS_MT_PRESSURE, sd->z);
-
- input_mt_sync(input);
- sd->valid = false;
-
- /* touchscreen emulation */
- if (sd->first) {
- if (!sd->activity) {
- input_event(input, EV_KEY, BTN_TOUCH, 1);
- sd->activity = true;
- }
- input_event(input, EV_ABS, ABS_X, sd->x);
- input_event(input, EV_ABS, ABS_Y, sd->y);
- }
- sd->first = false;
-}
-
-
-static int stantum_event(struct hid_device *hid, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
-{
- struct stantum_data *sd = hid_get_drvdata(hid);
-
- if (hid->claimed & HID_CLAIMED_INPUT) {
- struct input_dev *input = field->hidinput->input;
-
- switch (usage->hid) {
- case HID_DG_INRANGE:
- /* this is the last field in a finger */
- stantum_filter_event(sd, input);
- break;
- case HID_DG_WIDTH:
- sd->w = value;
- break;
- case HID_DG_HEIGHT:
- sd->h = value;
- break;
- case HID_GD_X:
- sd->x = value;
- break;
- case HID_GD_Y:
- sd->y = value;
- break;
- case HID_DG_TIPPRESSURE:
- sd->z = value;
- break;
- case HID_DG_CONTACTID:
- sd->id = value;
- break;
- case HID_DG_CONFIDENCE:
- sd->valid = !!value;
- break;
- case 0xff000002:
- /* this comes only before the first finger */
- sd->first = true;
- break;
-
- default:
- /* ignore the others */
- return 1;
- }
- }
-
- /* we have handled the hidinput part, now remains hiddev */
- if (hid->claimed & HID_CLAIMED_HIDDEV && hid->hiddev_hid_event)
- hid->hiddev_hid_event(hid, field, usage, value);
-
- return 1;
-}
-
-static int stantum_probe(struct hid_device *hdev,
- const struct hid_device_id *id)
-{
- int ret;
- struct stantum_data *sd;
-
- sd = kmalloc(sizeof(struct stantum_data), GFP_KERNEL);
- if (!sd) {
- hid_err(hdev, "cannot allocate Stantum data\n");
- return -ENOMEM;
- }
- sd->valid = false;
- sd->first = false;
- sd->activity = false;
- hid_set_drvdata(hdev, sd);
-
- ret = hid_parse(hdev);
- if (!ret)
- ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
-
- if (ret)
- kfree(sd);
-
- return ret;
-}
-
-static void stantum_remove(struct hid_device *hdev)
-{
- hid_hw_stop(hdev);
- kfree(hid_get_drvdata(hdev));
- hid_set_drvdata(hdev, NULL);
-}
-
-static const struct hid_device_id stantum_devices[] = {
- { HID_USB_DEVICE(USB_VENDOR_ID_STANTUM, USB_DEVICE_ID_MTP) },
- { HID_USB_DEVICE(USB_VENDOR_ID_STANTUM_STM, USB_DEVICE_ID_MTP_STM) },
- { HID_USB_DEVICE(USB_VENDOR_ID_STANTUM_SITRONIX, USB_DEVICE_ID_MTP_SITRONIX) },
- { }
-};
-MODULE_DEVICE_TABLE(hid, stantum_devices);
-
-static const struct hid_usage_id stantum_grabbed_usages[] = {
- { HID_ANY_ID, HID_ANY_ID, HID_ANY_ID },
- { HID_ANY_ID - 1, HID_ANY_ID - 1, HID_ANY_ID - 1}
-};
-
-static struct hid_driver stantum_driver = {
- .name = "stantum",
- .id_table = stantum_devices,
- .probe = stantum_probe,
- .remove = stantum_remove,
- .input_mapping = stantum_input_mapping,
- .input_mapped = stantum_input_mapped,
- .usage_table = stantum_grabbed_usages,
- .event = stantum_event,
-};
-
-static int __init stantum_init(void)
-{
- return hid_register_driver(&stantum_driver);
-}
-
-static void __exit stantum_exit(void)
-{
- hid_unregister_driver(&stantum_driver);
-}
-
-module_init(stantum_init);
-module_exit(stantum_exit);
-
--
1.7.4

2011-03-09 16:01:16

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH v2 3/4] hid-multitouch: migrate Cando dual touch panels to hid-multitouch

This patch merges hid-cando into the unified multitouch driver.

Signed-off-by: Benjamin Tissoires <[email protected]>
---

no changes since v1 ;)

drivers/hid/Kconfig | 7 +-
drivers/hid/Makefile | 1 -
drivers/hid/hid-cando.c | 276 ------------------------------------------
drivers/hid/hid-multitouch.c | 14 ++
4 files changed, 15 insertions(+), 283 deletions(-)
delete mode 100644 drivers/hid/hid-cando.c

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 1c9ca3ad..187c05e 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -94,12 +94,6 @@ config HID_BELKIN
---help---
Support for Belkin Flip KVM and Wireless keyboard.

-config HID_CANDO
- tristate "Cando dual touch panel"
- depends on USB_HID
- ---help---
- Support for Cando dual touch panel.
-
config HID_CHERRY
tristate "Cherry Cymotion keyboard" if EMBEDDED
depends on USB_HID
@@ -316,6 +310,7 @@ config HID_MULTITOUCH
Generic support for HID multitouch panels.

Say Y here if you have one of the following devices:
+ - Cando dual touch panel
- Cypress TrueTouch panels
- Hanvon dual touch panels
- IrTouch Infrared USB panels
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 397344a..571ebb9 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -30,7 +30,6 @@ obj-$(CONFIG_HID_A4TECH) += hid-a4tech.o
obj-$(CONFIG_HID_ACRUX_FF) += hid-axff.o
obj-$(CONFIG_HID_APPLE) += hid-apple.o
obj-$(CONFIG_HID_BELKIN) += hid-belkin.o
-obj-$(CONFIG_HID_CANDO) += hid-cando.o
obj-$(CONFIG_HID_CHERRY) += hid-cherry.o
obj-$(CONFIG_HID_CHICONY) += hid-chicony.o
obj-$(CONFIG_HID_CYPRESS) += hid-cypress.o
diff --git a/drivers/hid/hid-cando.c b/drivers/hid/hid-cando.c
deleted file mode 100644
index 1ea066c..0000000
--- a/drivers/hid/hid-cando.c
+++ /dev/null
@@ -1,276 +0,0 @@
-/*
- * HID driver for Cando dual-touch panels
- *
- * Copyright (c) 2010 Stephane Chatty <[email protected]>
- *
- */
-
-/*
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License as published by the Free
- * Software Foundation; either version 2 of the License, or (at your option)
- * any later version.
- */
-
-#include <linux/device.h>
-#include <linux/hid.h>
-#include <linux/module.h>
-#include <linux/slab.h>
-
-MODULE_AUTHOR("Stephane Chatty <[email protected]>");
-MODULE_DESCRIPTION("Cando dual-touch panel");
-MODULE_LICENSE("GPL");
-
-#include "hid-ids.h"
-
-struct cando_data {
- __u16 x, y;
- __u8 id;
- __s8 oldest; /* id of the oldest finger in previous frame */
- bool valid; /* valid finger data, or just placeholder? */
- bool first; /* is this the first finger in this frame? */
- __s8 firstid; /* id of the first finger in the frame */
- __u16 firstx, firsty; /* (x, y) of the first finger in the frame */
-};
-
-static int cando_input_mapping(struct hid_device *hdev, struct hid_input *hi,
- struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
-{
- switch (usage->hid & HID_USAGE_PAGE) {
-
- case HID_UP_GENDESK:
- switch (usage->hid) {
- case HID_GD_X:
- hid_map_usage(hi, usage, bit, max,
- EV_ABS, ABS_MT_POSITION_X);
- /* touchscreen emulation */
- input_set_abs_params(hi->input, ABS_X,
- field->logical_minimum,
- field->logical_maximum, 0, 0);
- return 1;
- case HID_GD_Y:
- hid_map_usage(hi, usage, bit, max,
- EV_ABS, ABS_MT_POSITION_Y);
- /* touchscreen emulation */
- input_set_abs_params(hi->input, ABS_Y,
- field->logical_minimum,
- field->logical_maximum, 0, 0);
- return 1;
- }
- return 0;
-
- case HID_UP_DIGITIZER:
- switch (usage->hid) {
- case HID_DG_TIPSWITCH:
- case HID_DG_CONTACTMAX:
- return -1;
- case HID_DG_INRANGE:
- /* touchscreen emulation */
- hid_map_usage(hi, usage, bit, max, EV_KEY, BTN_TOUCH);
- return 1;
- case HID_DG_CONTACTID:
- hid_map_usage(hi, usage, bit, max,
- EV_ABS, ABS_MT_TRACKING_ID);
- return 1;
- }
- return 0;
- }
-
- return 0;
-}
-
-static int cando_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)
- clear_bit(usage->code, *bit);
-
- return 0;
-}
-
-/*
- * this function is called when a whole finger has been parsed,
- * so that it can decide what to send to the input layer.
- */
-static void cando_filter_event(struct cando_data *td, struct input_dev *input)
-{
- td->first = !td->first; /* touchscreen emulation */
-
- if (!td->valid) {
- /*
- * touchscreen emulation: if this is the second finger and
- * the first was valid, the first was the oldest; if the
- * first was not valid and there was a valid finger in the
- * previous frame, this is a release.
- */
- if (td->first) {
- td->firstid = -1;
- } else if (td->firstid >= 0) {
- input_event(input, EV_ABS, ABS_X, td->firstx);
- input_event(input, EV_ABS, ABS_Y, td->firsty);
- td->oldest = td->firstid;
- } else if (td->oldest >= 0) {
- input_event(input, EV_KEY, BTN_TOUCH, 0);
- td->oldest = -1;
- }
-
- return;
- }
-
- input_event(input, EV_ABS, ABS_MT_TRACKING_ID, td->id);
- input_event(input, EV_ABS, ABS_MT_POSITION_X, td->x);
- input_event(input, EV_ABS, ABS_MT_POSITION_Y, td->y);
-
- input_mt_sync(input);
-
- /*
- * touchscreen emulation: if there was no touching finger previously,
- * emit touch event
- */
- if (td->oldest < 0) {
- input_event(input, EV_KEY, BTN_TOUCH, 1);
- td->oldest = td->id;
- }
-
- /*
- * touchscreen emulation: if this is the first finger, wait for the
- * second; the oldest is then the second if it was the oldest already
- * or if there was no first, the first otherwise.
- */
- if (td->first) {
- td->firstx = td->x;
- td->firsty = td->y;
- td->firstid = td->id;
- } else {
- int x, y, oldest;
- if (td->id == td->oldest || td->firstid < 0) {
- x = td->x;
- y = td->y;
- oldest = td->id;
- } else {
- x = td->firstx;
- y = td->firsty;
- oldest = td->firstid;
- }
- input_event(input, EV_ABS, ABS_X, x);
- input_event(input, EV_ABS, ABS_Y, y);
- td->oldest = oldest;
- }
-}
-
-
-static int cando_event(struct hid_device *hid, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
-{
- struct cando_data *td = hid_get_drvdata(hid);
-
- if (hid->claimed & HID_CLAIMED_INPUT) {
- struct input_dev *input = field->hidinput->input;
-
- switch (usage->hid) {
- case HID_DG_INRANGE:
- td->valid = value;
- break;
- case HID_DG_CONTACTID:
- td->id = value;
- break;
- case HID_GD_X:
- td->x = value;
- break;
- case HID_GD_Y:
- td->y = value;
- cando_filter_event(td, input);
- break;
- case HID_DG_TIPSWITCH:
- /* avoid interference from generic hidinput handling */
- break;
-
- default:
- /* fallback to the generic hidinput handling */
- return 0;
- }
- }
-
- /* we have handled the hidinput part, now remains hiddev */
- if (hid->claimed & HID_CLAIMED_HIDDEV && hid->hiddev_hid_event)
- hid->hiddev_hid_event(hid, field, usage, value);
-
- return 1;
-}
-
-static int cando_probe(struct hid_device *hdev, const struct hid_device_id *id)
-{
- int ret;
- struct cando_data *td;
-
- td = kmalloc(sizeof(struct cando_data), GFP_KERNEL);
- if (!td) {
- hid_err(hdev, "cannot allocate Cando Touch data\n");
- return -ENOMEM;
- }
- hid_set_drvdata(hdev, td);
- td->first = false;
- td->oldest = -1;
- td->valid = false;
-
- ret = hid_parse(hdev);
- if (!ret)
- ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
-
- if (ret)
- kfree(td);
-
- return ret;
-}
-
-static void cando_remove(struct hid_device *hdev)
-{
- hid_hw_stop(hdev);
- kfree(hid_get_drvdata(hdev));
- hid_set_drvdata(hdev, NULL);
-}
-
-static const struct hid_device_id cando_devices[] = {
- { HID_USB_DEVICE(USB_VENDOR_ID_CANDO,
- USB_DEVICE_ID_CANDO_MULTI_TOUCH) },
- { HID_USB_DEVICE(USB_VENDOR_ID_CANDO,
- USB_DEVICE_ID_CANDO_MULTI_TOUCH_10_1) },
- { HID_USB_DEVICE(USB_VENDOR_ID_CANDO,
- USB_DEVICE_ID_CANDO_MULTI_TOUCH_11_6) },
- { HID_USB_DEVICE(USB_VENDOR_ID_CANDO,
- USB_DEVICE_ID_CANDO_MULTI_TOUCH_15_6) },
- { }
-};
-MODULE_DEVICE_TABLE(hid, cando_devices);
-
-static const struct hid_usage_id cando_grabbed_usages[] = {
- { HID_ANY_ID, HID_ANY_ID, HID_ANY_ID },
- { HID_ANY_ID - 1, HID_ANY_ID - 1, HID_ANY_ID - 1}
-};
-
-static struct hid_driver cando_driver = {
- .name = "cando-touch",
- .id_table = cando_devices,
- .probe = cando_probe,
- .remove = cando_remove,
- .input_mapping = cando_input_mapping,
- .input_mapped = cando_input_mapped,
- .usage_table = cando_grabbed_usages,
- .event = cando_event,
-};
-
-static int __init cando_init(void)
-{
- return hid_register_driver(&cando_driver);
-}
-
-static void __exit cando_exit(void)
-{
- hid_unregister_driver(&cando_driver);
-}
-
-module_init(cando_init);
-module_exit(cando_exit);
-
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 525529c..02a77f9 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -481,6 +481,20 @@ static void mt_remove(struct hid_device *hdev)

static const struct hid_device_id mt_devices[] = {

+ /* Cando panels */
+ { .driver_data = MT_CLS_DUAL_INRANGE_CONTACTNUMBER,
+ HID_USB_DEVICE(USB_VENDOR_ID_CANDO,
+ USB_DEVICE_ID_CANDO_MULTI_TOUCH) },
+ { .driver_data = MT_CLS_DUAL_INRANGE_CONTACTNUMBER,
+ HID_USB_DEVICE(USB_VENDOR_ID_CANDO,
+ USB_DEVICE_ID_CANDO_MULTI_TOUCH_10_1) },
+ { .driver_data = MT_CLS_DUAL_INRANGE_CONTACTNUMBER,
+ HID_USB_DEVICE(USB_VENDOR_ID_CANDO,
+ USB_DEVICE_ID_CANDO_MULTI_TOUCH_11_6) },
+ { .driver_data = MT_CLS_DUAL_INRANGE_CONTACTNUMBER,
+ HID_USB_DEVICE(USB_VENDOR_ID_CANDO,
+ USB_DEVICE_ID_CANDO_MULTI_TOUCH_15_6) },
+
/* Cypress panel */
{ .driver_data = MT_CLS_CYPRESS,
HID_USB_DEVICE(USB_VENDOR_ID_CYPRESS,
--
1.7.4

2011-03-09 16:01:21

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH v2 4/4] hid-multitouch: migrate 3M PCT touch screens to hid-multitouch

This patch merges the hid-3m-pct driver into hid-multitouch.
To keep devices working the same way they used to with hid-3m-pct,
we need to add two signal/noise ratios for width and height.
We also need to work on width/height to send proper
ABS_MT_ORIENTATION flag.

Importing 3M into hid-multitouch also solved the bug in which
devices handling width and height in their report descriptors
did not show ABS_MT_TOUCH_MAJOR and ABS_MT_TOUCH_MINOR.

Signed-off-by: Benjamin Tissoires <[email protected]>
---

changes since v1:
- introduce .sn_width and .sn_height
- copy/paste width and height treatment from hid-3m-pct.c

The width and height has been tested against the two other known devices
that report them (stantum and cypress). But I was not able to test it
against a 3M.

And many thanks Henrik for the review of the v1.

Cheers,
Benjamin

drivers/hid/Kconfig | 7 +-
drivers/hid/Makefile | 1 -
drivers/hid/hid-3m-pct.c | 305 ------------------------------------------
drivers/hid/hid-multitouch.c | 37 +++++-
4 files changed, 36 insertions(+), 314 deletions(-)
delete mode 100644 drivers/hid/hid-3m-pct.c

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 187c05e..3bf1001 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -55,12 +55,6 @@ source "drivers/hid/usbhid/Kconfig"
menu "Special HID drivers"
depends on HID

-config HID_3M_PCT
- tristate "3M PCT touchscreen"
- depends on USB_HID
- ---help---
- Support for 3M PCT touch screens.
-
config HID_A4TECH
tristate "A4 tech mice" if EMBEDDED
depends on USB_HID
@@ -310,6 +304,7 @@ config HID_MULTITOUCH
Generic support for HID multitouch panels.

Say Y here if you have one of the following devices:
+ - 3M PCT touch screens
- Cando dual touch panel
- Cypress TrueTouch panels
- Hanvon dual touch panels
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 571ebb9..9d63d2c 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -25,7 +25,6 @@ ifdef CONFIG_LOGIWII_FF
hid-logitech-y += hid-lg4ff.o
endif

-obj-$(CONFIG_HID_3M_PCT) += hid-3m-pct.o
obj-$(CONFIG_HID_A4TECH) += hid-a4tech.o
obj-$(CONFIG_HID_ACRUX_FF) += hid-axff.o
obj-$(CONFIG_HID_APPLE) += hid-apple.o
diff --git a/drivers/hid/hid-3m-pct.c b/drivers/hid/hid-3m-pct.c
deleted file mode 100644
index 5243ae2..0000000
--- a/drivers/hid/hid-3m-pct.c
+++ /dev/null
@@ -1,305 +0,0 @@
-/*
- * HID driver for 3M PCT multitouch panels
- *
- * Copyright (c) 2009-2010 Stephane Chatty <[email protected]>
- * Copyright (c) 2010 Henrik Rydberg <[email protected]>
- * Copyright (c) 2010 Canonical, Ltd.
- *
- */
-
-/*
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License as published by the Free
- * Software Foundation; either version 2 of the License, or (at your option)
- * any later version.
- */
-
-#include <linux/device.h>
-#include <linux/hid.h>
-#include <linux/module.h>
-#include <linux/slab.h>
-#include <linux/usb.h>
-#include <linux/input/mt.h>
-
-MODULE_AUTHOR("Stephane Chatty <[email protected]>");
-MODULE_DESCRIPTION("3M PCT multitouch panels");
-MODULE_LICENSE("GPL");
-
-#include "hid-ids.h"
-
-#define MAX_SLOTS 60
-
-/* estimated signal-to-noise ratios */
-#define SN_MOVE 2048
-#define SN_WIDTH 128
-
-struct mmm_finger {
- __s32 x, y, w, h;
- bool touch, valid;
-};
-
-struct mmm_data {
- struct mmm_finger f[MAX_SLOTS];
- __u8 curid;
- __u8 nexp, nreal;
- bool touch, valid;
-};
-
-static int mmm_input_mapping(struct hid_device *hdev, struct hid_input *hi,
- struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
-{
- int f1 = field->logical_minimum;
- int f2 = field->logical_maximum;
- int df = f2 - f1;
-
- switch (usage->hid & HID_USAGE_PAGE) {
-
- case HID_UP_BUTTON:
- return -1;
-
- case HID_UP_GENDESK:
- switch (usage->hid) {
- case HID_GD_X:
- hid_map_usage(hi, usage, bit, max,
- EV_ABS, ABS_MT_POSITION_X);
- input_set_abs_params(hi->input, ABS_MT_POSITION_X,
- f1, f2, df / SN_MOVE, 0);
- /* touchscreen emulation */
- input_set_abs_params(hi->input, ABS_X,
- f1, f2, df / SN_MOVE, 0);
- return 1;
- case HID_GD_Y:
- hid_map_usage(hi, usage, bit, max,
- EV_ABS, ABS_MT_POSITION_Y);
- input_set_abs_params(hi->input, ABS_MT_POSITION_Y,
- f1, f2, df / SN_MOVE, 0);
- /* touchscreen emulation */
- input_set_abs_params(hi->input, ABS_Y,
- f1, f2, df / SN_MOVE, 0);
- return 1;
- }
- return 0;
-
- case HID_UP_DIGITIZER:
- switch (usage->hid) {
- /* we do not want to map these: no input-oriented meaning */
- case 0x14:
- case 0x23:
- case HID_DG_INPUTMODE:
- case HID_DG_DEVICEINDEX:
- case HID_DG_CONTACTCOUNT:
- case HID_DG_CONTACTMAX:
- case HID_DG_INRANGE:
- case HID_DG_CONFIDENCE:
- return -1;
- case HID_DG_TIPSWITCH:
- /* touchscreen emulation */
- hid_map_usage(hi, usage, bit, max, EV_KEY, BTN_TOUCH);
- input_set_capability(hi->input, EV_KEY, BTN_TOUCH);
- return 1;
- case HID_DG_WIDTH:
- hid_map_usage(hi, usage, bit, max,
- EV_ABS, ABS_MT_TOUCH_MAJOR);
- input_set_abs_params(hi->input, ABS_MT_TOUCH_MAJOR,
- f1, f2, df / SN_WIDTH, 0);
- return 1;
- case HID_DG_HEIGHT:
- hid_map_usage(hi, usage, bit, max,
- EV_ABS, ABS_MT_TOUCH_MINOR);
- input_set_abs_params(hi->input, ABS_MT_TOUCH_MINOR,
- f1, f2, df / SN_WIDTH, 0);
- input_set_abs_params(hi->input, ABS_MT_ORIENTATION,
- 0, 1, 0, 0);
- return 1;
- case HID_DG_CONTACTID:
- input_mt_init_slots(hi->input, MAX_SLOTS);
- return 1;
- }
- /* let hid-input decide for the others */
- return 0;
-
- case 0xff000000:
- /* we do not want to map these: no input-oriented meaning */
- return -1;
- }
-
- return 0;
-}
-
-static int mmm_input_mapped(struct hid_device *hdev, struct hid_input *hi,
- struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
-{
- /* tell hid-input to skip setup of these event types */
- if (usage->type == EV_KEY || usage->type == EV_ABS)
- set_bit(usage->type, hi->input->evbit);
- return -1;
-}
-
-/*
- * this function is called when a whole packet has been received and processed,
- * so that it can decide what to send to the input layer.
- */
-static void mmm_filter_event(struct mmm_data *md, struct input_dev *input)
-{
- int i;
- for (i = 0; i < MAX_SLOTS; ++i) {
- struct mmm_finger *f = &md->f[i];
- if (!f->valid) {
- /* this finger is just placeholder data, ignore */
- continue;
- }
- input_mt_slot(input, i);
- input_mt_report_slot_state(input, MT_TOOL_FINGER, f->touch);
- if (f->touch) {
- /* this finger is on the screen */
- int wide = (f->w > f->h);
- /* divided by two to match visual scale of touch */
- int major = max(f->w, f->h) >> 1;
- int minor = min(f->w, f->h) >> 1;
-
- input_event(input, EV_ABS, ABS_MT_POSITION_X, f->x);
- input_event(input, EV_ABS, ABS_MT_POSITION_Y, f->y);
- input_event(input, EV_ABS, ABS_MT_ORIENTATION, wide);
- input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
- input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
- }
- f->valid = 0;
- }
-
- input_mt_report_pointer_emulation(input, true);
- input_sync(input);
-}
-
-/*
- * this function is called upon all reports
- * so that we can accumulate contact point information,
- * and call input_mt_sync after each point.
- */
-static int mmm_event(struct hid_device *hid, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
-{
- struct mmm_data *md = hid_get_drvdata(hid);
- /*
- * strangely, this function can be called before
- * field->hidinput is initialized!
- */
- if (hid->claimed & HID_CLAIMED_INPUT) {
- struct input_dev *input = field->hidinput->input;
- switch (usage->hid) {
- case HID_DG_TIPSWITCH:
- md->touch = value;
- break;
- case HID_DG_CONFIDENCE:
- md->valid = value;
- break;
- case HID_DG_WIDTH:
- if (md->valid)
- md->f[md->curid].w = value;
- break;
- case HID_DG_HEIGHT:
- if (md->valid)
- md->f[md->curid].h = value;
- break;
- case HID_DG_CONTACTID:
- value = clamp_val(value, 0, MAX_SLOTS - 1);
- if (md->valid) {
- md->curid = value;
- md->f[value].touch = md->touch;
- md->f[value].valid = 1;
- md->nreal++;
- }
- break;
- case HID_GD_X:
- if (md->valid)
- md->f[md->curid].x = value;
- break;
- case HID_GD_Y:
- if (md->valid)
- md->f[md->curid].y = value;
- break;
- case HID_DG_CONTACTCOUNT:
- if (value)
- md->nexp = value;
- if (md->nreal >= md->nexp) {
- mmm_filter_event(md, input);
- md->nreal = 0;
- }
- break;
- }
- }
-
- /* we have handled the hidinput part, now remains hiddev */
- if (hid->claimed & HID_CLAIMED_HIDDEV && hid->hiddev_hid_event)
- hid->hiddev_hid_event(hid, field, usage, value);
-
- return 1;
-}
-
-static int mmm_probe(struct hid_device *hdev, const struct hid_device_id *id)
-{
- int ret;
- struct mmm_data *md;
-
- hdev->quirks |= HID_QUIRK_NO_INPUT_SYNC;
-
- md = kzalloc(sizeof(struct mmm_data), GFP_KERNEL);
- if (!md) {
- hid_err(hdev, "cannot allocate 3M data\n");
- return -ENOMEM;
- }
- hid_set_drvdata(hdev, md);
-
- ret = hid_parse(hdev);
- if (!ret)
- ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
-
- if (ret)
- kfree(md);
- return ret;
-}
-
-static void mmm_remove(struct hid_device *hdev)
-{
- hid_hw_stop(hdev);
- kfree(hid_get_drvdata(hdev));
- hid_set_drvdata(hdev, NULL);
-}
-
-static const struct hid_device_id mmm_devices[] = {
- { HID_USB_DEVICE(USB_VENDOR_ID_3M, USB_DEVICE_ID_3M1968) },
- { HID_USB_DEVICE(USB_VENDOR_ID_3M, USB_DEVICE_ID_3M2256) },
- { }
-};
-MODULE_DEVICE_TABLE(hid, mmm_devices);
-
-static const struct hid_usage_id mmm_grabbed_usages[] = {
- { HID_ANY_ID, HID_ANY_ID, HID_ANY_ID },
- { HID_ANY_ID - 1, HID_ANY_ID - 1, HID_ANY_ID - 1}
-};
-
-static struct hid_driver mmm_driver = {
- .name = "3m-pct",
- .id_table = mmm_devices,
- .probe = mmm_probe,
- .remove = mmm_remove,
- .input_mapping = mmm_input_mapping,
- .input_mapped = mmm_input_mapped,
- .usage_table = mmm_grabbed_usages,
- .event = mmm_event,
-};
-
-static int __init mmm_init(void)
-{
- return hid_register_driver(&mmm_driver);
-}
-
-static void __exit mmm_exit(void)
-{
- hid_unregister_driver(&mmm_driver);
-}
-
-module_init(mmm_init);
-module_exit(mmm_exit);
-
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 02a77f9..0b92dfc 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -5,6 +5,11 @@
* Copyright (c) 2010-2011 Benjamin Tissoires <[email protected]>
* Copyright (c) 2010-2011 Ecole Nationale de l'Aviation Civile, France
*
+ * based on hid-3m-pct.c copyrighted as follows:
+ * Copyright (c) 2009-2010 Stephane Chatty <[email protected]>
+ * Copyright (c) 2010 Henrik Rydberg <[email protected]>
+ * Copyright (c) 2010 Canonical, Ltd.
+ *
*/

/*
@@ -62,6 +67,8 @@ struct mt_class {
__s32 name; /* MT_CLS */
__s32 quirks;
__s32 sn_move; /* Signal/noise ratio for move events */
+ __s32 sn_width; /* Signal/noise ratio for width events */
+ __s32 sn_height; /* Signal/noise ratio for height events */
__s32 sn_pressure; /* Signal/noise ratio for pressure events */
__u8 maxcontacts;
};
@@ -72,6 +79,7 @@ struct mt_class {
#define MT_CLS_DUAL_INRANGE_CONTACTNUMBER 3
#define MT_CLS_CYPRESS 4
#define MT_CLS_STANTUM 5
+#define MT_CLS_3M 6

/*
* these device-dependent functions determine what slot corresponds
@@ -123,6 +131,12 @@ struct mt_class mt_classes[] = {
.maxcontacts = 10 },
{ .name = MT_CLS_STANTUM,
.quirks = MT_QUIRK_VALID_IS_CONFIDENCE },
+ { .name = MT_CLS_3M,
+ .quirks = MT_QUIRK_VALID_IS_CONFIDENCE |
+ MT_QUIRK_SLOT_IS_CONTACTID,
+ .sn_move = 2048,
+ .sn_width = 128,
+ .sn_height = 128 },

{ }
};
@@ -206,11 +220,15 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
case HID_DG_WIDTH:
hid_map_usage(hi, usage, bit, max,
EV_ABS, ABS_MT_TOUCH_MAJOR);
+ set_abs(hi->input, ABS_MT_TOUCH_MAJOR, field,
+ cls->sn_width);
td->last_slot_field = usage->hid;
return 1;
case HID_DG_HEIGHT:
hid_map_usage(hi, usage, bit, max,
EV_ABS, ABS_MT_TOUCH_MINOR);
+ set_abs(hi->input, ABS_MT_TOUCH_MINOR, field,
+ cls->sn_height);
field->logical_maximum = 1;
field->logical_minimum = 0;
set_abs(hi->input, ABS_MT_ORIENTATION, field, 0);
@@ -307,11 +325,18 @@ static void mt_emit_event(struct mt_device *td, struct input_dev *input)
input_mt_report_slot_state(input, MT_TOOL_FINGER,
s->touch_state);
if (s->touch_state) {
+ /* this finger is on the screen */
+ int wide = (s->w > s->h);
+ /* divided by two to match visual scale of touch */
+ int major = max(s->w, s->h) >> 1;
+ int minor = min(s->w, s->h) >> 1;
+
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_ORIENTATION, wide);
input_event(input, EV_ABS, ABS_MT_PRESSURE, s->p);
- input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, s->w);
- input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, s->h);
+ input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
+ input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
}
s->seen_in_this_frame = false;

@@ -481,6 +506,14 @@ static void mt_remove(struct hid_device *hdev)

static const struct hid_device_id mt_devices[] = {

+ /* 3M panels */
+ { .driver_data = MT_CLS_3M,
+ HID_USB_DEVICE(USB_VENDOR_ID_3M,
+ USB_DEVICE_ID_3M1968) },
+ { .driver_data = MT_CLS_3M,
+ HID_USB_DEVICE(USB_VENDOR_ID_3M,
+ USB_DEVICE_ID_3M2256) },
+
/* Cando panels */
{ .driver_data = MT_CLS_DUAL_INRANGE_CONTACTNUMBER,
HID_USB_DEVICE(USB_VENDOR_ID_CANDO,
--
1.7.4

2011-03-10 12:44:24

by Henrik Rydberg

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] hid-multitouch: Auto detection of maxcontacts

Hi Benjamin,

thanks for this, just a couple of things...

> @@ -126,9 +127,19 @@ struct mt_class mt_classes[] = {
> static void mt_feature_mapping(struct hid_device *hdev,
> struct hid_field *field, struct hid_usage *usage)
> {
> - if (usage->hid == HID_DG_INPUTMODE) {
> - struct mt_device *td = hid_get_drvdata(hdev);
> + struct mt_device *td = hid_get_drvdata(hdev);
> +
> + switch (usage->hid) {
> + case HID_DG_INPUTMODE:
> td->inputmode = field->report->id;
> + break;
> + case HID_DG_CONTACTMAX:
> + td->maxcontacts = *(field->value);

field->value[0] seems more standard-ish

> + if (td->mtclass->maxcontacts > td->maxcontacts)
> + /* check if the maxcontacts is given by the class */
> + td->maxcontacts = td->mtclass->maxcontacts;
> +
> + break;
> }
> }
>
> @@ -317,7 +327,7 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
> struct mt_device *td = hid_get_drvdata(hid);
> __s32 quirks = td->mtclass->quirks;
>
> - if (hid->claimed & HID_CLAIMED_INPUT) {
> + if (hid->claimed & HID_CLAIMED_INPUT && td->slots) {

The extra test could in principle be placed after the usage switch, which would prevent partially filled contact data, should this ever occur. Something like

if (!td->slots)
break;

> switch (usage->hid) {
> case HID_DG_INRANGE:
> if (quirks & MT_QUIRK_VALID_IS_INRANGE)
> @@ -415,9 +425,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
> */
> hdev->quirks |= HID_QUIRK_NO_INPUT_SYNC;
>
> - td = kzalloc(sizeof(struct mt_device) +
> - mtclass->maxcontacts * sizeof(struct mt_slot),
> - GFP_KERNEL);
> + td = kzalloc(sizeof(struct mt_device), GFP_KERNEL);
> if (!td) {
> dev_err(&hdev->dev, "cannot allocate multitouch data\n");
> return -ENOMEM;
> @@ -434,6 +442,14 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
> if (ret)
> goto fail;
>
> + td->slots = kzalloc(td->maxcontacts * sizeof(struct mt_slot),
> + GFP_KERNEL);
> + if (!td->slots) {
> + dev_err(&hdev->dev, "cannot allocate multitouch slots\n");
> + ret = -ENOMEM;
> + goto fail;

This exit leaves the device running.

> + }
> +
> mt_set_input_mode(hdev);
>
> return 0;
> @@ -455,6 +471,7 @@ static void mt_remove(struct hid_device *hdev)
> {
> struct mt_device *td = hid_get_drvdata(hdev);
> hid_hw_stop(hdev);
> + kfree(td->slots);
> kfree(td);
> hid_set_drvdata(hdev, NULL);
> }
> --
> 1.7.4
>

Thanks,
Henrik

2011-03-10 15:50:01

by Henrik Rydberg

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] hid-multitouch: migrate 3M PCT touch screens to hid-multitouch

Hi Benjamin,

> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 02a77f9..0b92dfc 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -5,6 +5,11 @@
> * Copyright (c) 2010-2011 Benjamin Tissoires <[email protected]>
> * Copyright (c) 2010-2011 Ecole Nationale de l'Aviation Civile, France
> *
> + * based on hid-3m-pct.c copyrighted as follows:
> + * Copyright (c) 2009-2010 Stephane Chatty <[email protected]>
> + * Copyright (c) 2010 Henrik Rydberg <[email protected]>
> + * Copyright (c) 2010 Canonical, Ltd.
> + *
> */
>
> /*
> @@ -62,6 +67,8 @@ struct mt_class {
> __s32 name; /* MT_CLS */
> __s32 quirks;
> __s32 sn_move; /* Signal/noise ratio for move events */
> + __s32 sn_width; /* Signal/noise ratio for width events */
> + __s32 sn_height; /* Signal/noise ratio for height events */
> __s32 sn_pressure; /* Signal/noise ratio for pressure events */
> __u8 maxcontacts;
> };
> @@ -72,6 +79,7 @@ struct mt_class {
> #define MT_CLS_DUAL_INRANGE_CONTACTNUMBER 3
> #define MT_CLS_CYPRESS 4
> #define MT_CLS_STANTUM 5
> +#define MT_CLS_3M 6
>
> /*
> * these device-dependent functions determine what slot corresponds
> @@ -123,6 +131,12 @@ struct mt_class mt_classes[] = {
> .maxcontacts = 10 },
> { .name = MT_CLS_STANTUM,
> .quirks = MT_QUIRK_VALID_IS_CONFIDENCE },

I realize several of the entries are missing maxcontacts now, so all
patches needs to be checked...

> + { .name = MT_CLS_3M,
> + .quirks = MT_QUIRK_VALID_IS_CONFIDENCE |
> + MT_QUIRK_SLOT_IS_CONTACTID,
> + .sn_move = 2048,
> + .sn_width = 128,
> + .sn_height = 128 },

And this one does too
>
> { }
> };
> @@ -206,11 +220,15 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> case HID_DG_WIDTH:
> hid_map_usage(hi, usage, bit, max,
> EV_ABS, ABS_MT_TOUCH_MAJOR);
> + set_abs(hi->input, ABS_MT_TOUCH_MAJOR, field,
> + cls->sn_width);
> td->last_slot_field = usage->hid;
> return 1;
> case HID_DG_HEIGHT:
> hid_map_usage(hi, usage, bit, max,
> EV_ABS, ABS_MT_TOUCH_MINOR);
> + set_abs(hi->input, ABS_MT_TOUCH_MINOR, field,
> + cls->sn_height);
> field->logical_maximum = 1;
> field->logical_minimum = 0;

These limits are not right - I doubt they are for any device.

> set_abs(hi->input, ABS_MT_ORIENTATION, field, 0);
> @@ -307,11 +325,18 @@ static void mt_emit_event(struct mt_device *td, struct input_dev *input)
> input_mt_report_slot_state(input, MT_TOOL_FINGER,
> s->touch_state);
> if (s->touch_state) {
> + /* this finger is on the screen */
> + int wide = (s->w > s->h);
> + /* divided by two to match visual scale of touch */
> + int major = max(s->w, s->h) >> 1;
> + int minor = min(s->w, s->h) >> 1;
> +
> 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_ORIENTATION, wide);
> input_event(input, EV_ABS, ABS_MT_PRESSURE, s->p);
> - input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, s->w);
> - input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, s->h);
> + input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
> + input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
> }
> s->seen_in_this_frame = false;
>
> @@ -481,6 +506,14 @@ static void mt_remove(struct hid_device *hdev)
>
> static const struct hid_device_id mt_devices[] = {
>
> + /* 3M panels */
> + { .driver_data = MT_CLS_3M,
> + HID_USB_DEVICE(USB_VENDOR_ID_3M,
> + USB_DEVICE_ID_3M1968) },
> + { .driver_data = MT_CLS_3M,
> + HID_USB_DEVICE(USB_VENDOR_ID_3M,
> + USB_DEVICE_ID_3M2256) },
> +
> /* Cando panels */
> { .driver_data = MT_CLS_DUAL_INRANGE_CONTACTNUMBER,
> HID_USB_DEVICE(USB_VENDOR_ID_CANDO,
> --
> 1.7.4
>

Also, this line needs to be added in case no feature reports are sent:

@@ -481,6 +481,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
return -ENOMEM;
}
td->mtclass = mtclass;
+ td->maxcontacts = mtclass->maxcontacts;
td->inputmode = -1;
hid_set_drvdata(hdev, td);

Thanks,
Henrik

2011-03-11 07:07:23

by Benjamin Tissoires

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] hid-multitouch: Auto detection of maxcontacts

Hi Henrik,

thanks for the review.

On Thu, Mar 10, 2011 at 13:45, Henrik Rydberg <[email protected]> wrote:
> Hi Benjamin,
>
> thanks for this, just a couple of things...
>
>> @@ -126,9 +127,19 @@ struct mt_class mt_classes[] = {
>> ?static void mt_feature_mapping(struct hid_device *hdev,
>> ? ? ? ? ? ? ? struct hid_field *field, struct hid_usage *usage)
>> ?{
>> - ? ? if (usage->hid == HID_DG_INPUTMODE) {
>> - ? ? ? ? ? ? struct mt_device *td = hid_get_drvdata(hdev);
>> + ? ? struct mt_device *td = hid_get_drvdata(hdev);
>> +
>> + ? ? switch (usage->hid) {
>> + ? ? case HID_DG_INPUTMODE:
>> ? ? ? ? ? ? ? td->inputmode = field->report->id;
>> + ? ? ? ? ? ? break;
>> + ? ? case HID_DG_CONTACTMAX:
>> + ? ? ? ? ? ? td->maxcontacts = *(field->value);
>
> field->value[0] seems more standard-ish

If you want

>
>> + ? ? ? ? ? ? if (td->mtclass->maxcontacts > td->maxcontacts)
>> + ? ? ? ? ? ? ? ? ? ? /* check if the maxcontacts is given by the class */
>> + ? ? ? ? ? ? ? ? ? ? td->maxcontacts = td->mtclass->maxcontacts;
>> +
>> + ? ? ? ? ? ? break;
>> ? ? ? }
>> ?}
>>
>> @@ -317,7 +327,7 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
>> ? ? ? struct mt_device *td = hid_get_drvdata(hid);
>> ? ? ? __s32 quirks = td->mtclass->quirks;
>>
>> - ? ? if (hid->claimed & HID_CLAIMED_INPUT) {
>> + ? ? if (hid->claimed & HID_CLAIMED_INPUT && td->slots) {
>
> The extra test could in principle be placed after the usage switch, which would prevent partially filled contact data, should this ever occur. Something like
>
> if (!td->slots)
> ? break;

I don't know where these "break" comes from. We can add the test after
the switch before the mt_complete_slot and mt_emit_event. But please
note than it can in all cases introduce truncated slots reports as all
the slots in the report may not have been completed. To be honest, I'm
not sure this will ever matter as it's a race at the connection of the
device, and the reports will come to correct anything wrong.

>
>> ? ? ? ? ? ? ? switch (usage->hid) {
>> ? ? ? ? ? ? ? case HID_DG_INRANGE:
>> ? ? ? ? ? ? ? ? ? ? ? if (quirks & MT_QUIRK_VALID_IS_INRANGE)
>> @@ -415,9 +425,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
>> ? ? ? ?*/
>> ? ? ? hdev->quirks |= HID_QUIRK_NO_INPUT_SYNC;
>>
>> - ? ? td = kzalloc(sizeof(struct mt_device) +
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? mtclass->maxcontacts * sizeof(struct mt_slot),
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? GFP_KERNEL);
>> + ? ? td = kzalloc(sizeof(struct mt_device), GFP_KERNEL);
>> ? ? ? if (!td) {
>> ? ? ? ? ? ? ? dev_err(&hdev->dev, "cannot allocate multitouch data\n");
>> ? ? ? ? ? ? ? return -ENOMEM;
>> @@ -434,6 +442,14 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
>> ? ? ? if (ret)
>> ? ? ? ? ? ? ? goto fail;
>>
>> + ? ? td->slots = kzalloc(td->maxcontacts * sizeof(struct mt_slot),
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? GFP_KERNEL);
>> + ? ? if (!td->slots) {
>> + ? ? ? ? ? ? dev_err(&hdev->dev, "cannot allocate multitouch slots\n");
>> + ? ? ? ? ? ? ret = -ENOMEM;
>> + ? ? ? ? ? ? goto fail;
>
> This exit leaves the device running.

Yep, will do sth.

Thanks
Benjamin

>
>> + ? ? }
>> +
>> ? ? ? mt_set_input_mode(hdev);
>>
>> ? ? ? return 0;
>> @@ -455,6 +471,7 @@ static void mt_remove(struct hid_device *hdev)
>> ?{
>> ? ? ? struct mt_device *td = hid_get_drvdata(hdev);
>> ? ? ? hid_hw_stop(hdev);
>> + ? ? kfree(td->slots);
>> ? ? ? kfree(td);
>> ? ? ? hid_set_drvdata(hdev, NULL);
>> ?}
>> --
>> 1.7.4
>>
>
> Thanks,
> Henrik
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>

2011-03-11 07:26:44

by Benjamin Tissoires

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] hid-multitouch: migrate 3M PCT touch screens to hid-multitouch

Hi Henrik,

On Thu, Mar 10, 2011 at 16:52, Henrik Rydberg <[email protected]> wrote:
> Hi Benjamin,
>
>> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
>> index 02a77f9..0b92dfc 100644
>> --- a/drivers/hid/hid-multitouch.c
>> +++ b/drivers/hid/hid-multitouch.c
>> @@ -5,6 +5,11 @@
>> ? * ?Copyright (c) 2010-2011 Benjamin Tissoires <[email protected]>
>> ? * ?Copyright (c) 2010-2011 Ecole Nationale de l'Aviation Civile, France
>> ? *
>> + * ?based on hid-3m-pct.c copyrighted as follows:
>> + * ? ?Copyright (c) 2009-2010 Stephane Chatty <[email protected]>
>> + * ? ?Copyright (c) 2010 ? ? ?Henrik Rydberg <[email protected]>
>> + * ? ?Copyright (c) 2010 ? ? ?Canonical, Ltd.
>> + *
>> ? */
>>
>> ?/*
>> @@ -62,6 +67,8 @@ struct mt_class {
>> ? ? ? __s32 name; ? ? /* MT_CLS */
>> ? ? ? __s32 quirks;
>> ? ? ? __s32 sn_move; ?/* Signal/noise ratio for move events */
>> + ? ? __s32 sn_width; /* Signal/noise ratio for width events */
>> + ? ? __s32 sn_height; ? ? ? ?/* Signal/noise ratio for height events */
>> ? ? ? __s32 sn_pressure; ? ? ?/* Signal/noise ratio for pressure events */
>> ? ? ? __u8 maxcontacts;
>> ?};
>> @@ -72,6 +79,7 @@ struct mt_class {
>> ?#define MT_CLS_DUAL_INRANGE_CONTACTNUMBER ? ?3
>> ?#define MT_CLS_CYPRESS ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 4
>> ?#define MT_CLS_STANTUM ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 5
>> +#define MT_CLS_3M ? ? ? ? ? ? ? ? ? ? ? ? ? ?6
>>
>> ?/*
>> ? * these device-dependent functions determine what slot corresponds
>> @@ -123,6 +131,12 @@ struct mt_class mt_classes[] = {
>> ? ? ? ? ? ? ? .maxcontacts = 10 },
>> ? ? ? { .name = MT_CLS_STANTUM,
>> ? ? ? ? ? ? ? .quirks = MT_QUIRK_VALID_IS_CONFIDENCE },
>
> I realize several of the entries are missing maxcontacts now, so all
> patches needs to be checked...

This is really annoying. The idea behind the auto-detection was to
simplify the writing of the driver and to let the device inform us on
its capabilities.
It's not a bug, it's a feature in this particular case. My idea was to
remove all those .maxcontact (for instance, I know that it works for
cypress, stantum and 3M) but I didn't want to bother my testers about
such a little change.

Can I remember you that you where the first complaining about the
maxcontact? You asked if it could not be given by the device. And as
we where in a hurry, we didn't include it and said that we will do it
later.

>
>> + ? ? { .name = MT_CLS_3M,
>> + ? ? ? ? ? ? .quirks = MT_QUIRK_VALID_IS_CONFIDENCE |
>> + ? ? ? ? ? ? ? ? ? ? MT_QUIRK_SLOT_IS_CONTACTID,
>> + ? ? ? ? ? ? .sn_move = 2048,
>> + ? ? ? ? ? ? .sn_width = 128,
>> + ? ? ? ? ? ? .sn_height = 128 },
>
> And this one does too

And this one would be false: should I write 10 or 60? As it depends on
the device, we'll have to let the device decide.
If you remember the commit message of the auto-detection patch, I have
written that we keep the .maxcontact field in case the device _lies_.
And 3M does not lie.

>>
>> ? ? ? { }
>> ?};
>> @@ -206,11 +220,15 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>> ? ? ? ? ? ? ? case HID_DG_WIDTH:
>> ? ? ? ? ? ? ? ? ? ? ? hid_map_usage(hi, usage, bit, max,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? EV_ABS, ABS_MT_TOUCH_MAJOR);
>> + ? ? ? ? ? ? ? ? ? ? set_abs(hi->input, ABS_MT_TOUCH_MAJOR, field,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? cls->sn_width);
>> ? ? ? ? ? ? ? ? ? ? ? td->last_slot_field = usage->hid;
>> ? ? ? ? ? ? ? ? ? ? ? return 1;
>> ? ? ? ? ? ? ? case HID_DG_HEIGHT:
>> ? ? ? ? ? ? ? ? ? ? ? hid_map_usage(hi, usage, bit, max,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? EV_ABS, ABS_MT_TOUCH_MINOR);
>> + ? ? ? ? ? ? ? ? ? ? set_abs(hi->input, ABS_MT_TOUCH_MINOR, field,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? cls->sn_height);
>> ? ? ? ? ? ? ? ? ? ? ? field->logical_maximum = 1;
>> ? ? ? ? ? ? ? ? ? ? ? field->logical_minimum = 0;
>
> These limits are not right - I doubt they are for any device.

I was a little surprise too while looking at these. But This is not
related to ABS_MT_TOUCH_MINOR, but ABS_MT_ORIENTATION.
And if I'm forced to modify those values this way it's because _you_
made me introduce the set_abs function which takes in parameter the
field. Thus, it's more complicated to introduce new fields and usages.

>
>> ? ? ? ? ? ? ? ? ? ? ? set_abs(hi->input, ABS_MT_ORIENTATION, field, 0);
>> @@ -307,11 +325,18 @@ static void mt_emit_event(struct mt_device *td, struct input_dev *input)
>> ? ? ? ? ? ? ? input_mt_report_slot_state(input, MT_TOOL_FINGER,
>> ? ? ? ? ? ? ? ? ? ? ? s->touch_state);
>> ? ? ? ? ? ? ? if (s->touch_state) {
>> + ? ? ? ? ? ? ? ? ? ? /* this finger is on the screen */
>> + ? ? ? ? ? ? ? ? ? ? int wide = (s->w > s->h);
>> + ? ? ? ? ? ? ? ? ? ? /* divided by two to match visual scale of touch */
>> + ? ? ? ? ? ? ? ? ? ? int major = max(s->w, s->h) >> 1;
>> + ? ? ? ? ? ? ? ? ? ? int minor = min(s->w, s->h) >> 1;
>> +
>> ? ? ? ? ? ? ? ? ? ? ? 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_ORIENTATION, wide);
>> ? ? ? ? ? ? ? ? ? ? ? input_event(input, EV_ABS, ABS_MT_PRESSURE, s->p);
>> - ? ? ? ? ? ? ? ? ? ? input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, s->w);
>> - ? ? ? ? ? ? ? ? ? ? input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, s->h);
>> + ? ? ? ? ? ? ? ? ? ? input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
>> + ? ? ? ? ? ? ? ? ? ? input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
>> ? ? ? ? ? ? ? }
>> ? ? ? ? ? ? ? s->seen_in_this_frame = false;
>>
>> @@ -481,6 +506,14 @@ static void mt_remove(struct hid_device *hdev)
>>
>> ?static const struct hid_device_id mt_devices[] = {
>>
>> + ? ? /* 3M panels */
>> + ? ? { .driver_data = MT_CLS_3M,
>> + ? ? ? ? ? ? HID_USB_DEVICE(USB_VENDOR_ID_3M,
>> + ? ? ? ? ? ? ? ? ? ? USB_DEVICE_ID_3M1968) },
>> + ? ? { .driver_data = MT_CLS_3M,
>> + ? ? ? ? ? ? HID_USB_DEVICE(USB_VENDOR_ID_3M,
>> + ? ? ? ? ? ? ? ? ? ? USB_DEVICE_ID_3M2256) },
>> +
>> ? ? ? /* Cando panels */
>> ? ? ? { .driver_data = MT_CLS_DUAL_INRANGE_CONTACTNUMBER,
>> ? ? ? ? ? ? ? HID_USB_DEVICE(USB_VENDOR_ID_CANDO,
>> --
>> 1.7.4
>>
>
> Also, this line needs to be added in case no feature reports are sent:
>
> @@ -481,6 +481,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
> ? ? ? ? ? ? ? ?return -ENOMEM;
> ? ? ? ?}
> ? ? ? ?td->mtclass = mtclass;
> + ? ? ? td->maxcontacts = mtclass->maxcontacts;
> ? ? ? ?td->inputmode = -1;
> ? ? ? ?hid_set_drvdata(hdev, td);
>

So:
1) If a device does not send the feature report related to maxcontact,
then it won't pass the Win7 certification, then we will need to write
a special driver for it.
2) This is wrong because I do not want to add in all those classes the
field maxcontact. So I'll revert the MT_DEFAULT_MAXCONTACT, and I will
add this sort of line just before allocating the slots.
Oh, and I'll remove the .maxcontact = 2 for MT_CLS_DEFAULT.

Cheers,
Benjamin

2011-03-11 11:05:01

by Henrik Rydberg

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] hid-multitouch: migrate 3M PCT touch screens to hid-multitouch

> > I realize several of the entries are missing maxcontacts now, so all
> > patches needs to be checked...
>
> This is really annoying. The idea behind the auto-detection was to
> simplify the writing of the driver and to let the device inform us on
> its capabilities.
> It's not a bug, it's a feature in this particular case. My idea was to
> remove all those .maxcontact (for instance, I know that it works for
> cypress, stantum and 3M) but I didn't want to bother my testers about
> such a little change.
>
> Can I remember you that you where the first complaining about the
> maxcontact? You asked if it could not be given by the device. And as
> we where in a hurry, we didn't include it and said that we will do it
> later.

*sigh*

The generally problem seems to be that the patch cycles are too short,
leading to buggy implementations.

> And this one would be false: should I write 10 or 60? As it depends on
> the device, we'll have to let the device decide.
> If you remember the commit message of the auto-detection patch, I have
> written that we keep the .maxcontact field in case the device _lies_.
> And 3M does not lie.

Which is why max(default-value, device-value) was suggested.

> >> ? ? ? ? ? ? ? ? ? ? ? field->logical_maximum = 1;
> >> ? ? ? ? ? ? ? ? ? ? ? field->logical_minimum = 0;
> >> ? ? ? ? ? ? ? ? ? ? ? set_abs(hi->input, ABS_MT_ORIENTATION, field, 0);
> >
> > These limits are not right - I doubt they are for any device.
>
> I was a little surprise too while looking at these. But This is not
> related to ABS_MT_TOUCH_MINOR, but ABS_MT_ORIENTATION.
> And if I'm forced to modify those values this way it's because _you_
> made me introduce the set_abs function which takes in parameter the
> field. Thus, it's more complicated to introduce new fields and usages.

Enough with the accusations. Simply replace those lines with

input_set_abs_params(hi->input, ABS_MT_ORIENTATION, 0, 1, 0, 0);

> > Also, this line needs to be added in case no feature reports are sent:
> >
> > @@ -481,6 +481,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
> > ? ? ? ? ? ? ? ?return -ENOMEM;
> > ? ? ? ?}
> > ? ? ? ?td->mtclass = mtclass;
> > + ? ? ? td->maxcontacts = mtclass->maxcontacts;
> > ? ? ? ?td->inputmode = -1;
> > ? ? ? ?hid_set_drvdata(hdev, td);
> >
>
> So:
> 1) If a device does not send the feature report related to maxcontact,
> then it won't pass the Win7 certification, then we will need to write
> a special driver for it.
> 2) This is wrong because I do not want to add in all those classes the
> field maxcontact. So I'll revert the MT_DEFAULT_MAXCONTACT, and I will
> add this sort of line just before allocating the slots.
> Oh, and I'll remove the .maxcontact = 2 for MT_CLS_DEFAULT.

Take a deep breath and take your time. We don't want to waste anybodys
time by going through more cycles than necessary.

Henrik

2011-03-14 12:08:43

by Jiri Kosina

[permalink] [raw]
Subject: Re: [PATCH 0/4] Migrations to hid-multitouch, round 2

On Wed, 9 Mar 2011, Benjamin Tissoires wrote:

> Hi guys,
>
> This is the second version for this patch series.
>
> Since last time, I integrated Henrik's comments.
>
> Cheers,
> Benjamin
>
> drivers/hid/Kconfig | 21 +---
> drivers/hid/Makefile | 3 -
> drivers/hid/hid-3m-pct.c | 305 ------------------------------------------
> drivers/hid/hid-cando.c | 276 --------------------------------------
> drivers/hid/hid-multitouch.c | 110 +++++++++++++---
> drivers/hid/hid-stantum.c | 286 ---------------------------------------
> 6 files changed, 97 insertions(+), 904 deletions(-)
>
> ;-)

Hi Benjamin,

thanks a lot for this work you have done.

Is there going to be second round of the patches with a few minor fixes
coming out from Henrik's review?

Thanks,

--
Jiri Kosina
SUSE Labs, Novell Inc.

2011-03-14 13:10:23

by Benjamin Tissoires

[permalink] [raw]
Subject: Re: [PATCH 0/4] Migrations to hid-multitouch, round 2

On Mon, Mar 14, 2011 at 13:08, Jiri Kosina <[email protected]> wrote:
> On Wed, 9 Mar 2011, Benjamin Tissoires wrote:
>
>> Hi guys,
>>
>> This is the second version for this patch series.
>>
>> Since last time, I integrated Henrik's comments.
>>
>> Cheers,
>> Benjamin
>>
>> ?drivers/hid/Kconfig ? ? ? ? ?| ? 21 +---
>> ?drivers/hid/Makefile ? ? ? ? | ? ?3 -
>> ?drivers/hid/hid-3m-pct.c ? ? | ?305 ------------------------------------------
>> ?drivers/hid/hid-cando.c ? ? ?| ?276 --------------------------------------
>> ?drivers/hid/hid-multitouch.c | ?110 +++++++++++++---
>> ?drivers/hid/hid-stantum.c ? ?| ?286 ---------------------------------------
>> ?6 files changed, 97 insertions(+), 904 deletions(-)
>>
>> ;-)
>
> Hi Benjamin,
>
> thanks a lot for this work you have done.
>
> Is there going to be second round of the patches with a few minor fixes
> coming out from Henrik's review?
>
> Thanks,

Hi Jiri,

yes of course. I don't know if I'll have the time to do it today, but
I'll do my best.

Cheers,
Benjamin

>
> --
> Jiri Kosina
> SUSE Labs, Novell Inc.
>