Hi guys,
As mentioned on the list, I think that the following drivers can safely be moved to hid-multitouch:
3M, Stantum and Cando.
This let Quanta and Mosart in the queue.
In addition, I also added the auto-detection of the maximum contact number a device can handle based on the report descriptors.
Cheers,
Benjamin
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]>
---
drivers/hid/hid-multitouch.c | 43 ++++++++++++++++++++++++++++-------------
1 files changed, 29 insertions(+), 14 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 65e7f20..363ca08 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -38,6 +38,8 @@ MODULE_LICENSE("GPL");
#define MT_QUIRK_VALID_IS_INRANGE (1 << 4)
#define MT_QUIRK_VALID_IS_CONFIDENCE (1 << 5)
+#define MT_CONTACTMAX_DEFAULT 11
+
struct mt_slot {
__s32 x, y, p, w, h;
__s32 contactid; /* the device ContactID assigned to this slot */
@@ -53,8 +55,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 +90,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;
@@ -105,8 +108,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 },
+ .quirks = MT_QUIRK_NOT_SEEN_MEANS_UP },
{ .name = MT_CLS_DUAL_INRANGE_CONTACTID,
.quirks = MT_QUIRK_VALID_IS_INRANGE |
MT_QUIRK_SLOT_IS_CONTACTID,
@@ -126,9 +128,22 @@ 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)
+ /* check if the maxcontacts is given by the class */
+ td->maxcontacts = td->mtclass->maxcontacts;
+
+ if (!td->maxcontacts)
+ td->maxcontacts = MT_CONTACTMAX_DEFAULT;
+
+ break;
}
}
@@ -186,8 +201,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 +282,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 +297,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) {
@@ -415,9 +429,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 +446,9 @@ 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);
+
mt_set_input_mode(hdev);
return 0;
--
1.7.4
Signed-off-by: Benjamin Tissoires <[email protected]>
---
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 363ca08..e7a4f83 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -73,6 +73,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
@@ -121,6 +122,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 },
{ }
};
@@ -499,6 +502,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
Signed-off-by: Benjamin Tissoires <[email protected]>
---
drivers/hid/Kconfig | 7 +-
drivers/hid/Makefile | 1 -
drivers/hid/hid-3m-pct.c | 305 ------------------------------------------
drivers/hid/hid-multitouch.c | 19 +++
4 files changed, 20 insertions(+), 312 deletions(-)
delete mode 100644 drivers/hid/hid-3m-pct.c
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 1c9ca3ad..a1fbb36 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
@@ -316,6 +310,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
- Cypress TrueTouch panels
- Hanvon dual touch panels
- IrTouch Infrared USB panels
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 397344a..bf7e623 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 e7a4f83..f32bf46 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.
+ *
*/
/*
@@ -74,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
@@ -124,6 +130,11 @@ 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_pressure = 128 },
{ }
};
@@ -479,6 +490,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) },
+
/* Cypress panel */
{ .driver_data = MT_CLS_CYPRESS,
HID_USB_DEVICE(USB_VENDOR_ID_CYPRESS,
--
1.7.4
Signed-off-by: Benjamin Tissoires <[email protected]>
---
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 a1fbb36..3bf1001 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -88,12 +88,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
@@ -311,6 +305,7 @@ config HID_MULTITOUCH
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
- IrTouch Infrared USB panels
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index bf7e623..9d63d2c 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -29,7 +29,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 f32bf46..945b798 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -498,6 +498,20 @@ static const struct hid_device_id mt_devices[] = {
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,
+ 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
Hi Benjamin,
> 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]>
It seems quite alright to let the classes contain the expected number
of contacts, so I do not really see the reason for that part of the
patch. How about keeping the maxcontacts in the class, and then do
max(hid-provided-maxcontacts, default-maxcontacts)?
Thanks,
Henrik
Hi Benjamin,
On Tue, Mar 08, 2011 at 05:32:58PM +0100, Benjamin Tissoires wrote:
> Signed-off-by: Benjamin Tissoires <[email protected]>
> ---
Semms this patch is missing a commit message.
[...]
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index e7a4f83..f32bf46 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.
> + *
> */
>
> /*
> @@ -74,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
> @@ -124,6 +130,11 @@ 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_pressure = 128 },
>
> { }
> };
> @@ -479,6 +490,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) },
> +
> /* Cypress panel */
> { .driver_data = MT_CLS_CYPRESS,
> HID_USB_DEVICE(USB_VENDOR_ID_CYPRESS,
The touch width correction from the original driver has gone missing
here. Otherwise, looks good, but will defer testing to the next round.
Cheers,
Henrik
On Wed, Mar 9, 2011 at 09:42, Henrik Rydberg <[email protected]> wrote:
> Hi Benjamin,
>
>> 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]>
>
> It seems quite alright to let the classes contain the expected number
> of contacts, so I do not really see the reason for that part of the
> patch. How about keeping the maxcontacts in the class, and then do
> max(hid-provided-maxcontacts, default-maxcontacts)?
>
Yep, I've got three particular reasons:
- 3M: there are two devices now, 1968 and 2256. The first one is a 10
touches only, whereas the second one is a 60 touches.
- autodetection of multitouch devices. I have some patches on my tree
(that we do not want to go upstream right now for some reasons) that
allows us to plug any unknown multitouch devices and to let
hid-multitouch handling it. As most of the devices are 2 touches only,
and as the generic way to work with a multitouch devices is to iterate
over all the slots, using 10 touches by default infers a lot of
instructions that can be avoided.
- finally, it simplifies the writing of the new CLS (we just need to
know how the device works to add the right quirks).
Cheers,
Benjamin
Hi Henrik
On Wed, Mar 9, 2011 at 09:46, Henrik Rydberg <[email protected]> wrote:
> Hi Benjamin,
>
> On Tue, Mar 08, 2011 at 05:32:58PM +0100, Benjamin Tissoires wrote:
>> Signed-off-by: Benjamin Tissoires <[email protected]>
>> ---
>
> Semms this patch is missing a commit message.
You can copy/paste this point to the other patches as well.
How about:
"3M PCT touch screens can be handled by the unified driver. This will
ease future evolution. So, just move it."
Then I'll copy/paste for the others.
>
> [...]
>> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
>> index e7a4f83..f32bf46 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.
>> + *
>> ? */
>>
>> ?/*
>> @@ -74,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
>> @@ -124,6 +130,11 @@ 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_pressure = 128 },
>>
>> ? ? ? { }
>> ?};
>> @@ -479,6 +490,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) },
>> +
>> ? ? ? /* Cypress panel */
>> ? ? ? { .driver_data = MT_CLS_CYPRESS,
>> ? ? ? ? ? ? ? HID_USB_DEVICE(USB_VENDOR_ID_CYPRESS,
>
> The touch width correction from the original driver has gone missing
> here. Otherwise, looks good, but will defer testing to the next round.
Oops, it's not missing, it has been transformed into sn_pressure...
Will correct it.
By looking at it, I just saw that we override values for HID_DG_HEIGHT
to the range [0-1].
I'll have to dig into.
For the tests, I've tested it on the 1968.
Thanks,
Benjamin
On Wed, Mar 09, 2011 at 10:03:45AM +0100, Benjamin Tissoires wrote:
> On Wed, Mar 9, 2011 at 09:42, Henrik Rydberg <[email protected]> wrote:
> > Hi Benjamin,
> >
> >> 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]>
> >
> > It seems quite alright to let the classes contain the expected number
> > of contacts, so I do not really see the reason for that part of the
> > patch. How about keeping the maxcontacts in the class, and then do
> > max(hid-provided-maxcontacts, default-maxcontacts)?
> >
>
> Yep, I've got three particular reasons:
> - 3M: there are two devices now, 1968 and 2256. The first one is a 10
> touches only, whereas the second one is a 60 touches.
Right, so increasing the number of touches based on device information
seems like a good idea.
> - autodetection of multitouch devices. I have some patches on my tree
> (that we do not want to go upstream right now for some reasons) that
> allows us to plug any unknown multitouch devices and to let
> hid-multitouch handling it. As most of the devices are 2 touches only,
> and as the generic way to work with a multitouch devices is to iterate
> over all the slots, using 10 touches by default infers a lot of
> instructions that can be avoided.
Right, so keeping the default number of touches per class seems like a
good idea.
> - finally, it simplifies the writing of the new CLS (we just need to
> know how the device works to add the right quirks).
Right, we always need to know how the device works. :-)
Cheers,
Henrik
On Wed, Mar 09, 2011 at 10:13:55AM +0100, Benjamin Tissoires wrote:
> Hi Henrik
>
> On Wed, Mar 9, 2011 at 09:46, Henrik Rydberg <[email protected]> wrote:
> > Hi Benjamin,
> >
> > On Tue, Mar 08, 2011 at 05:32:58PM +0100, Benjamin Tissoires wrote:
> >> Signed-off-by: Benjamin Tissoires <[email protected]>
> >> ---
> >
> > Semms this patch is missing a commit message.
>
> You can copy/paste this point to the other patches as well.
>
> How about:
> "3M PCT touch screens can be handled by the unified driver. This will
> ease future evolution. So, just move it."
>
> Then I'll copy/paste for the others.
I bet every driver merge has its own considerations that deserve
comment. A good commit message gives an introduction to the code
change and gives some context to the reader. Richard made a nice one
for the egalax driver.
Henrik
On Wed, Mar 9, 2011 at 10:38, Henrik Rydberg <[email protected]> wrote:
> On Wed, Mar 09, 2011 at 10:03:45AM +0100, Benjamin Tissoires wrote:
>> On Wed, Mar 9, 2011 at 09:42, Henrik Rydberg <[email protected]> wrote:
>> > Hi Benjamin,
>> >
>> >> 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]>
>> >
>> > It seems quite alright to let the classes contain the expected number
>> > of contacts, so I do not really see the reason for that part of the
>> > patch. How about keeping the maxcontacts in the class, and then do
>> > max(hid-provided-maxcontacts, default-maxcontacts)?
>> >
>>
>> Yep, I've got three particular reasons:
>> - 3M: there are two devices now, 1968 and 2256. The first one is a 10
>> touches only, whereas the second one is a 60 touches.
>
> Right, so increasing the number of touches based on device information
> seems like a good idea.
So the patch is useful.
>
>> - autodetection of multitouch devices. I have some patches on my tree
>> (that we do not want to go upstream right now for some reasons) that
>> allows us to plug any unknown multitouch devices and to let
>> hid-multitouch handling it. As most of the devices are 2 touches only,
>> and as the generic way to work with a multitouch devices is to iterate
>> over all the slots, using 10 touches by default infers a lot of
>> instructions that can be avoided.
>
> Right, so keeping the default number of touches per class seems like a
> good idea.
That's the way the patch works: we can still manually provide the
maxcontact per class, but if it's not needed (the device sends proper
value), then we can skip it.
>
>> - finally, it simplifies the writing of the new CLS (we just need to
>> know how the device works to add the right quirks).
>
> Right, we always need to know how the device works. :-)
What I meant was the dynamic behavior of the device, not the static
capabilities. ;)
Am I right if I take your reply as an Ack?
Cheers,
Benjamin
On Wed, Mar 9, 2011 at 10:47, Henrik Rydberg <[email protected]> wrote:
> On Wed, Mar 09, 2011 at 10:13:55AM +0100, Benjamin Tissoires wrote:
>> Hi Henrik
>>
>> On Wed, Mar 9, 2011 at 09:46, Henrik Rydberg <[email protected]> wrote:
>> > Hi Benjamin,
>> >
>> > On Tue, Mar 08, 2011 at 05:32:58PM +0100, Benjamin Tissoires wrote:
>> >> Signed-off-by: Benjamin Tissoires <[email protected]>
>> >> ---
>> >
>> > Semms this patch is missing a commit message.
>>
>> You can copy/paste this point to the other patches as well.
>>
>> How about:
>> "3M PCT touch screens can be handled by the unified driver. This will
>> ease future evolution. So, just move it."
>>
>> Then I'll copy/paste for the others.
>
> I bet every driver merge has its own considerations that deserve
> comment. A good commit message gives an introduction to the code
> change and gives some context to the reader. Richard made a nice one
> for the egalax driver.
>
Now that We have to introduce .sn_width / .sn_height for 3M, this need
indeed a better commit message.
Cheers,
Benjamin
> >> Yep, I've got three particular reasons:
> >> - 3M: there are two devices now, 1968 and 2256. The first one is a 10
> >> touches only, whereas the second one is a 60 touches.
> >
> > Right, so increasing the number of touches based on device information
> > seems like a good idea.
>
> So the patch is useful.
Indeed. :-)
> >> - autodetection of multitouch devices. I have some patches on my tree
> >> (that we do not want to go upstream right now for some reasons) that
> >> allows us to plug any unknown multitouch devices and to let
> >> hid-multitouch handling it. As most of the devices are 2 touches only,
> >> and as the generic way to work with a multitouch devices is to iterate
> >> over all the slots, using 10 touches by default infers a lot of
> >> instructions that can be avoided.
> >
> > Right, so keeping the default number of touches per class seems like a
> > good idea.
>
> That's the way the patch works: we can still manually provide the
> maxcontact per class, but if it's not needed (the device sends proper
> value), then we can skip it.
I misread the original patch, the maxcontacts are still there, so this
point is moot. Sorry about that. :-)
> >> - finally, it simplifies the writing of the new CLS (we just need to
> >> know how the device works to add the right quirks).
> >
> > Right, we always need to know how the device works. :-)
>
> What I meant was the dynamic behavior of the device, not the static
> capabilities. ;)
>
> Am I right if I take your reply as an Ack?
I will reply to the original patch with some comments.
Cheers,
Henrik
On Tue, Mar 08, 2011 at 05:32:56PM +0100, Benjamin Tissoires wrote:
> 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]>
> ---
> drivers/hid/hid-multitouch.c | 43 ++++++++++++++++++++++++++++-------------
> 1 files changed, 29 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 65e7f20..363ca08 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -38,6 +38,8 @@ MODULE_LICENSE("GPL");
> #define MT_QUIRK_VALID_IS_INRANGE (1 << 4)
> #define MT_QUIRK_VALID_IS_CONFIDENCE (1 << 5)
>
> +#define MT_CONTACTMAX_DEFAULT 11
> +
I think this one can be removed, se below.
> struct mt_slot {
> __s32 x, y, p, w, h;
> __s32 contactid; /* the device ContactID assigned to this slot */
> @@ -53,8 +55,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 +90,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;
> @@ -105,8 +108,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 },
> + .quirks = MT_QUIRK_NOT_SEEN_MEANS_UP },
Keeping .maxcontacts at two here seems sensible
> { .name = MT_CLS_DUAL_INRANGE_CONTACTID,
> .quirks = MT_QUIRK_VALID_IS_INRANGE |
> MT_QUIRK_SLOT_IS_CONTACTID,
> @@ -126,9 +128,22 @@ 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)
if (td->mtclass->maxcontacts > td->maxcontacts)
> + /* check if the maxcontacts is given by the class */
> + td->maxcontacts = td->mtclass->maxcontacts;
> +
> + if (!td->maxcontacts)
> + td->maxcontacts = MT_CONTACTMAX_DEFAULT;
this part can be then dropped
> +
> + break;
> }
> }
>
> @@ -186,8 +201,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 +282,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 +297,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) {
> @@ -415,9 +429,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 +446,9 @@ 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);
> +
Don't we have a race problem here? It seems the device is started at
this point, so I worry that events will be handled when slots is still
NULL.
> mt_set_input_mode(hdev);
>
> return 0;
> --
> 1.7.4
>
Thanks,
Henrik
On Wed, Mar 9, 2011 at 12:22, Henrik Rydberg <[email protected]> wrote:
> On Tue, Mar 08, 2011 at 05:32:56PM +0100, Benjamin Tissoires wrote:
>> 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]>
>> ---
>> ?drivers/hid/hid-multitouch.c | ? 43 ++++++++++++++++++++++++++++-------------
>> ?1 files changed, 29 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
>> index 65e7f20..363ca08 100644
>> --- a/drivers/hid/hid-multitouch.c
>> +++ b/drivers/hid/hid-multitouch.c
>> @@ -38,6 +38,8 @@ MODULE_LICENSE("GPL");
>> ?#define MT_QUIRK_VALID_IS_INRANGE ? ?(1 << 4)
>> ?#define MT_QUIRK_VALID_IS_CONFIDENCE (1 << 5)
>>
>> +#define MT_CONTACTMAX_DEFAULT ? ? ? ?11
>> +
>
> I think this one can be removed, se below.
>
>> ?struct mt_slot {
>> ? ? ? __s32 x, y, p, w, h;
>> ? ? ? __s32 contactid; ? ? ? ?/* the device ContactID assigned to this slot */
>> @@ -53,8 +55,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 +90,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;
>> @@ -105,8 +108,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 },
>> + ? ? ? ? ? ? .quirks = MT_QUIRK_NOT_SEEN_MEANS_UP },
>
> Keeping .maxcontacts at two here seems sensible
>
>> ? ? ? { .name = MT_CLS_DUAL_INRANGE_CONTACTID,
>> ? ? ? ? ? ? ? .quirks = MT_QUIRK_VALID_IS_INRANGE |
>> ? ? ? ? ? ? ? ? ? ? ? MT_QUIRK_SLOT_IS_CONTACTID,
>> @@ -126,9 +128,22 @@ 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)
>
> if (td->mtclass->maxcontacts > td->maxcontacts)
>
>> + ? ? ? ? ? ? ? ? ? ? /* check if the maxcontacts is given by the class */
>> + ? ? ? ? ? ? ? ? ? ? td->maxcontacts = td->mtclass->maxcontacts;
>> +
>> + ? ? ? ? ? ? if (!td->maxcontacts)
>> + ? ? ? ? ? ? ? ? ? ? td->maxcontacts = MT_CONTACTMAX_DEFAULT;
>
> this part can be then dropped
Well, it works the way you are suggesting. BTW this let the corner
case where someone adds a device (MT_CLS) that does not send the
contact max and does not initialize the .maxcontact field.
>
>> +
>> + ? ? ? ? ? ? break;
>> ? ? ? }
>> ?}
>>
>> @@ -186,8 +201,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 +282,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 +297,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) {
>> @@ -415,9 +429,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 +446,9 @@ 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);
>> +
>
> Don't we have a race problem here? ?It seems the device is started at
> this point, so I worry that events will be handled when slots is still
> NULL.
I tried again yesterday: if I put this line above the hid_hw_start ->
kernel oops at first touch.
The point is that hid_hw_start calls hid_connect that do the actual
calls to input_mapping and feature_mapping.
Cheers,
Benjamin
>
>> ? ? ? mt_set_input_mode(hdev);
>>
>> ? ? ? return 0;
>> --
>> 1.7.4
>>
>
> Thanks,
> Henrik
>
> > if (td->mtclass->maxcontacts > td->maxcontacts)
> >
> >> + ? ? ? ? ? ? ? ? ? ? /* check if the maxcontacts is given by the class */
> >> + ? ? ? ? ? ? ? ? ? ? td->maxcontacts = td->mtclass->maxcontacts;
> >> +
> >> + ? ? ? ? ? ? if (!td->maxcontacts)
> >> + ? ? ? ? ? ? ? ? ? ? td->maxcontacts = MT_CONTACTMAX_DEFAULT;
> >
> > this part can be then dropped
>
> Well, it works the way you are suggesting. BTW this let the corner
> case where someone adds a device (MT_CLS) that does not send the
> contact max and does not initialize the .maxcontact field.
Yes, the patch changes the current, perfectly reasonable assumption
that maxcontact is set. If that change is removed from the patch, it
makes the logic simpler, without changing the semantics of the current
code.
> >> + ? ? td->slots = kzalloc(td->maxcontacts * sizeof(struct mt_slot),
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? GFP_KERNEL);
> >> +
> >
> > Don't we have a race problem here? ?It seems the device is started at
> > this point, so I worry that events will be handled when slots is still
> > NULL.
>
> I tried again yesterday: if I put this line above the hid_hw_start ->
> kernel oops at first touch.
> The point is that hid_hw_start calls hid_connect that do the actual
> calls to input_mapping and feature_mapping.
Yes, that is clear, but the urbs are running, and as fas as I can see,
irqs can be delivered to mt_event(). Minute time window, testing is
not likely to hit this. Adding a test for completed initialization in
mt_event() would make sure.
Oh, and there is no test for failed memory allocation.
Thanks.
Henrik