* [PATCH] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework.
@ 2011-03-06 13:25 Richard Nauber
2011-03-07 10:07 ` Benjamin Tissoires
2011-03-08 13:12 ` [PATCH] " Jiri Kosina
0 siblings, 2 replies; 21+ messages in thread
From: Richard Nauber @ 2011-03-06 13:25 UTC (permalink / raw)
To: linux-input; +Cc: Jiri Kosina, Henrik Rydberg
This patch merges the egalax devices to hid-multitouch and
therefore adds an MT_QUIRK_EGALAX to work around the broken hid report.
It also fixes suspend/resume issues with the old driver.
Signed-off-by: Richard Nauber <Richard.Nauber@gmail.com>
---
drivers/hid/Kconfig | 9 +-
drivers/hid/Makefile | 1 -
drivers/hid/hid-egalax.c | 279 ------------------------------------------
drivers/hid/hid-multitouch.c | 48 +++++++-
4 files changed, 47 insertions(+), 290 deletions(-)
delete mode 100644 drivers/hid/hid-egalax.c
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 2560f01..34959a2 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -160,13 +160,6 @@ config HID_EMS_FF
Currently the following devices are known to be supported:
- Trio Linker Plus II
-config HID_EGALAX
- tristate "eGalax multi-touch panel"
- depends on USB_HID
- ---help---
- Support for the eGalax dual-touch panels, including the
- Joojoo and Wetab tablets.
-
config HID_ELECOM
tristate "ELECOM BM084 bluetooth mouse"
depends on BT_HIDP
@@ -306,6 +299,8 @@ config HID_MULTITOUCH
- Hanvon dual touch panels
- Pixcir dual touch panels
- 'Sensing Win7-TwoFinger' panel by GeneralTouch
+ - eGalax dual-touch panels, including the
+ Joojoo and Wetab tablets
If unsure, say N.
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 6efc2a0..29e9898 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -36,7 +36,6 @@ obj-$(CONFIG_HID_CHICONY) += hid-chicony.o
obj-$(CONFIG_HID_CYPRESS) += hid-cypress.o
obj-$(CONFIG_HID_DRAGONRISE) += hid-drff.o
obj-$(CONFIG_HID_EMS_FF) += hid-emsff.o
-obj-$(CONFIG_HID_EGALAX) += hid-egalax.o
obj-$(CONFIG_HID_ELECOM) += hid-elecom.o
obj-$(CONFIG_HID_EZKEY) += hid-ezkey.o
obj-$(CONFIG_HID_GYRATION) += hid-gyration.o
diff --git a/drivers/hid/hid-egalax.c b/drivers/hid/hid-egalax.c
deleted file mode 100644
index 03bee19..0000000
--- a/drivers/hid/hid-egalax.c
+++ /dev/null
@@ -1,279 +0,0 @@
-/*
- * HID driver for eGalax dual-touch panels
- *
- * Copyright (c) 2010 Stephane Chatty <chatty@enac.fr>
- * Copyright (c) 2010 Henrik Rydberg <rydberg@euromail.se>
- * 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/usb.h>
-#include <linux/input/mt.h>
-#include <linux/slab.h>
-#include "usbhid/usbhid.h"
-
-MODULE_AUTHOR("Stephane Chatty <chatty@enac.fr>");
-MODULE_DESCRIPTION("eGalax dual-touch panel");
-MODULE_LICENSE("GPL");
-
-#include "hid-ids.h"
-
-#define MAX_SLOTS 2
-
-/* estimated signal-to-noise ratios */
-#define SN_MOVE 4096
-#define SN_PRESSURE 32
-
-struct egalax_data {
- int valid;
- int slot;
- int touch;
- int x, y, z;
-};
-
-static void set_abs(struct input_dev *input, unsigned int code,
- struct hid_field *field, int snratio)
-{
- int fmin = field->logical_minimum;
- int fmax = field->logical_maximum;
- int fuzz = snratio ? (fmax - fmin) / snratio : 0;
- input_set_abs_params(input, code, fmin, fmax, fuzz, 0);
-}
-
-static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
- struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
-{
- struct input_dev *input = hi->input;
-
- switch (usage->hid & HID_USAGE_PAGE) {
-
- case HID_UP_GENDESK:
- switch (usage->hid) {
- case HID_GD_X:
- field->logical_maximum = 32760;
- hid_map_usage(hi, usage, bit, max,
- EV_ABS, ABS_MT_POSITION_X);
- set_abs(input, ABS_MT_POSITION_X, field, SN_MOVE);
- /* touchscreen emulation */
- set_abs(input, ABS_X, field, SN_MOVE);
- return 1;
- case HID_GD_Y:
- field->logical_maximum = 32760;
- hid_map_usage(hi, usage, bit, max,
- EV_ABS, ABS_MT_POSITION_Y);
- set_abs(input, ABS_MT_POSITION_Y, field, SN_MOVE);
- /* touchscreen emulation */
- set_abs(input, ABS_Y, field, SN_MOVE);
- return 1;
- }
- return 0;
-
- case HID_UP_DIGITIZER:
- switch (usage->hid) {
- case HID_DG_TIPSWITCH:
- /* touchscreen emulation */
- hid_map_usage(hi, usage, bit, max, EV_KEY, BTN_TOUCH);
- input_set_capability(input, EV_KEY, BTN_TOUCH);
- return 1;
- case HID_DG_INRANGE:
- case HID_DG_CONFIDENCE:
- case HID_DG_CONTACTCOUNT:
- case HID_DG_CONTACTMAX:
- return -1;
- case HID_DG_CONTACTID:
- input_mt_init_slots(input, MAX_SLOTS);
- return 1;
- case HID_DG_TIPPRESSURE:
- field->logical_minimum = 0;
- hid_map_usage(hi, usage, bit, max,
- EV_ABS, ABS_MT_PRESSURE);
- set_abs(input, ABS_MT_PRESSURE, field, SN_PRESSURE);
- /* touchscreen emulation */
- set_abs(input, ABS_PRESSURE, field, SN_PRESSURE);
- return 1;
- }
- return 0;
- }
-
- /* ignore others (from other reports we won't get anyway) */
- return -1;
-}
-
-static int egalax_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 finger has been parsed,
- * so that it can decide what to send to the input layer.
- */
-static void egalax_filter_event(struct egalax_data *td, struct input_dev *input)
-{
- input_mt_slot(input, td->slot);
- input_mt_report_slot_state(input, MT_TOOL_FINGER, td->touch);
- if (td->touch) {
- input_event(input, EV_ABS, ABS_MT_POSITION_X, td->x);
- input_event(input, EV_ABS, ABS_MT_POSITION_Y, td->y);
- input_event(input, EV_ABS, ABS_MT_PRESSURE, td->z);
- }
- input_mt_report_pointer_emulation(input, true);
-}
-
-static int egalax_event(struct hid_device *hid, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
-{
- struct egalax_data *td = hid_get_drvdata(hid);
-
- /* Note, eGalax has two product lines: the first is resistive and
- * uses a standard parallel multitouch protocol (product ID ==
- * 48xx). The second is capacitive and uses an unusual "serial"
- * protocol with a different message for each multitouch finger
- * (product ID == 72xx).
- */
- 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_CONFIDENCE:
- /* avoid interference from generic hidinput handling */
- break;
- case HID_DG_TIPSWITCH:
- td->touch = value;
- break;
- case HID_DG_TIPPRESSURE:
- td->z = value;
- break;
- case HID_DG_CONTACTID:
- td->slot = clamp_val(value, 0, MAX_SLOTS - 1);
- break;
- case HID_GD_X:
- td->x = value;
- break;
- case HID_GD_Y:
- td->y = value;
- /* this is the last field in a finger */
- if (td->valid)
- egalax_filter_event(td, input);
- break;
- case HID_DG_CONTACTCOUNT:
- /* touch emulation: this is the last field in a frame */
- 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 egalax_probe(struct hid_device *hdev, const struct hid_device_id *id)
-{
- int ret;
- struct egalax_data *td;
- struct hid_report *report;
-
- td = kzalloc(sizeof(struct egalax_data), GFP_KERNEL);
- if (!td) {
- hid_err(hdev, "cannot allocate eGalax data\n");
- return -ENOMEM;
- }
- hid_set_drvdata(hdev, td);
-
- ret = hid_parse(hdev);
- if (ret)
- goto end;
-
- ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
- if (ret)
- goto end;
-
- report = hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[5];
- if (report) {
- report->field[0]->value[0] = 2;
- usbhid_submit_report(hdev, report, USB_DIR_OUT);
- }
-
-end:
- if (ret)
- kfree(td);
-
- return ret;
-}
-
-static void egalax_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 egalax_devices[] = {
- { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
- USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH) },
- { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
- USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH1) },
- { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
- USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH2) },
- { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
- USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH3) },
- { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
- USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH4) },
- { }
-};
-MODULE_DEVICE_TABLE(hid, egalax_devices);
-
-static const struct hid_usage_id egalax_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 egalax_driver = {
- .name = "egalax-touch",
- .id_table = egalax_devices,
- .probe = egalax_probe,
- .remove = egalax_remove,
- .input_mapping = egalax_input_mapping,
- .input_mapped = egalax_input_mapped,
- .usage_table = egalax_grabbed_usages,
- .event = egalax_event,
-};
-
-static int __init egalax_init(void)
-{
- return hid_register_driver(&egalax_driver);
-}
-
-static void __exit egalax_exit(void)
-{
- hid_unregister_driver(&egalax_driver);
-}
-
-module_init(egalax_init);
-module_exit(egalax_exit);
-
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 07d3183..58df924 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -36,6 +36,8 @@ MODULE_LICENSE("GPL");
#define MT_QUIRK_SLOT_IS_CONTACTNUMBER (1 << 3)
#define MT_QUIRK_VALID_IS_INRANGE (1 << 4)
#define MT_QUIRK_VALID_IS_CONFIDENCE (1 << 5)
+#define MT_QUIRK_EGALAX (1 << 6)
+
struct mt_slot {
__s32 x, y, p, w, h;
@@ -69,6 +71,7 @@ struct mt_class {
#define MT_CLS_DUAL1 2
#define MT_CLS_DUAL2 3
#define MT_CLS_CYPRESS 4
+#define MT_CLS_EGALAX 5
/*
* these device-dependent functions determine what slot corresponds
@@ -118,7 +121,13 @@ struct mt_class mt_classes[] = {
.quirks = MT_QUIRK_NOT_SEEN_MEANS_UP |
MT_QUIRK_CYPRESS,
.maxcontacts = 10 },
-
+ { .name = MT_CLS_EGALAX,
+ .quirks = MT_QUIRK_SLOT_IS_CONTACTID |
+ MT_QUIRK_VALID_IS_INRANGE |
+ MT_QUIRK_EGALAX,
+ .maxcontacts = 2 ,
+ .sn_move = 4096,
+ .sn_pressure = 32},
{ }
};
@@ -146,9 +155,16 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
{
struct mt_device *td = hid_get_drvdata(hdev);
struct mt_class *cls = td->mtclass;
+
switch (usage->hid & HID_USAGE_PAGE) {
case HID_UP_GENDESK:
+
+ /* fix up the X/Y limits for egalax devices*/
+ if ((cls->quirks & MT_QUIRK_EGALAX)
+ && ((usage->hid == HID_GD_X) || (usage->hid == HID_GD_Y)))
+ field->logical_maximum = 32760;
+
switch (usage->hid) {
case HID_GD_X:
hid_map_usage(hi, usage, bit, max,
@@ -203,6 +219,10 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
td->last_slot_field = usage->hid;
return 1;
case HID_DG_TIPPRESSURE:
+ /* fix up the pressure range for egalax devices*/
+ if (cls->quirks & MT_QUIRK_EGALAX)
+ field->logical_minimum = 0;
+
hid_map_usage(hi, usage, bit, max,
EV_ABS, ABS_MT_PRESSURE);
set_abs(hi->input, ABS_MT_PRESSURE, field,
@@ -269,8 +289,10 @@ static void mt_complete_slot(struct mt_device *td)
if (slotnum >= 0 && slotnum < td->mtclass->maxcontacts)
td->slots[slotnum] = td->curdata;
+
}
td->num_received++;
+
}
@@ -366,12 +388,15 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
if (usage->hid == td->last_slot_field)
mt_complete_slot(td);
- if (field->index == td->last_field_index
+ if ((field->index == td->last_field_index
&& td->num_received >= td->num_expected)
+ || ((quirks & MT_QUIRK_EGALAX)
+ && (usage->hid == td->last_slot_field)))
+ /* for egalax devices: emit an event for every finger
+ to work around the wrong last_field_index in the report*/
mt_emit_event(td, field->hidinput->input);
}
-
/* 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);
@@ -478,6 +503,23 @@ static const struct hid_device_id mt_devices[] = {
HID_USB_DEVICE(USB_VENDOR_ID_CANDO,
USB_DEVICE_ID_CANDO_PIXCIR_MULTI_TOUCH) },
+ /* eGalax devices */
+ { .driver_data = MT_CLS_EGALAX,
+ HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
+ USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH) },
+ { .driver_data = MT_CLS_EGALAX,
+ HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
+ USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH1) },
+ { .driver_data = MT_CLS_EGALAX,
+ HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
+ USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH2) },
+ { .driver_data = MT_CLS_EGALAX,
+ HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
+ USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH3) },
+ { .driver_data = MT_CLS_EGALAX,
+ HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
+ USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH4) },
+
{ }
};
MODULE_DEVICE_TABLE(hid, mt_devices);
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework.
2011-03-06 13:25 [PATCH] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework Richard Nauber
@ 2011-03-07 10:07 ` Benjamin Tissoires
2011-03-08 6:08 ` [PATCH v2] " Richard Nauber
2011-03-08 13:12 ` [PATCH] " Jiri Kosina
1 sibling, 1 reply; 21+ messages in thread
From: Benjamin Tissoires @ 2011-03-07 10:07 UTC (permalink / raw)
To: Richard Nauber
Cc: linux-input, Jiri Kosina, Henrik Rydberg, Stéphane Chatty
Hi Richard,
On Sun, Mar 6, 2011 at 14:25, Richard Nauber
<richard.nauber@googlemail.com> wrote:
> This patch merges the egalax devices to hid-multitouch and
> therefore adds an MT_QUIRK_EGALAX to work around the broken hid report.
> It also fixes suspend/resume issues with the old driver.
>
>
> Signed-off-by: Richard Nauber <Richard.Nauber@gmail.com>
> ---
> drivers/hid/Kconfig | 9 +-
> drivers/hid/Makefile | 1 -
> drivers/hid/hid-egalax.c | 279 ------------------------------------------
> drivers/hid/hid-multitouch.c | 48 +++++++-
> 4 files changed, 47 insertions(+), 290 deletions(-)
> delete mode 100644 drivers/hid/hid-egalax.c
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 2560f01..34959a2 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -160,13 +160,6 @@ config HID_EMS_FF
> Currently the following devices are known to be supported:
> - Trio Linker Plus II
>
> -config HID_EGALAX
> - tristate "eGalax multi-touch panel"
> - depends on USB_HID
> - ---help---
> - Support for the eGalax dual-touch panels, including the
> - Joojoo and Wetab tablets.
> -
> config HID_ELECOM
> tristate "ELECOM BM084 bluetooth mouse"
> depends on BT_HIDP
> @@ -306,6 +299,8 @@ config HID_MULTITOUCH
> - Hanvon dual touch panels
> - Pixcir dual touch panels
> - 'Sensing Win7-TwoFinger' panel by GeneralTouch
> + - eGalax dual-touch panels, including the
> + Joojoo and Wetab tablets
>
> If unsure, say N.
>
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index 6efc2a0..29e9898 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -36,7 +36,6 @@ obj-$(CONFIG_HID_CHICONY) += hid-chicony.o
> obj-$(CONFIG_HID_CYPRESS) += hid-cypress.o
> obj-$(CONFIG_HID_DRAGONRISE) += hid-drff.o
> obj-$(CONFIG_HID_EMS_FF) += hid-emsff.o
> -obj-$(CONFIG_HID_EGALAX) += hid-egalax.o
> obj-$(CONFIG_HID_ELECOM) += hid-elecom.o
> obj-$(CONFIG_HID_EZKEY) += hid-ezkey.o
> obj-$(CONFIG_HID_GYRATION) += hid-gyration.o
> diff --git a/drivers/hid/hid-egalax.c b/drivers/hid/hid-egalax.c
> deleted file mode 100644
> index 03bee19..0000000
> --- a/drivers/hid/hid-egalax.c
> +++ /dev/null
> @@ -1,279 +0,0 @@
> -/*
> - * HID driver for eGalax dual-touch panels
> - *
> - * Copyright (c) 2010 Stephane Chatty <chatty@enac.fr>
> - * Copyright (c) 2010 Henrik Rydberg <rydberg@euromail.se>
> - * 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/usb.h>
> -#include <linux/input/mt.h>
> -#include <linux/slab.h>
> -#include "usbhid/usbhid.h"
> -
> -MODULE_AUTHOR("Stephane Chatty <chatty@enac.fr>");
> -MODULE_DESCRIPTION("eGalax dual-touch panel");
> -MODULE_LICENSE("GPL");
> -
> -#include "hid-ids.h"
> -
> -#define MAX_SLOTS 2
> -
> -/* estimated signal-to-noise ratios */
> -#define SN_MOVE 4096
> -#define SN_PRESSURE 32
> -
> -struct egalax_data {
> - int valid;
> - int slot;
> - int touch;
> - int x, y, z;
> -};
> -
> -static void set_abs(struct input_dev *input, unsigned int code,
> - struct hid_field *field, int snratio)
> -{
> - int fmin = field->logical_minimum;
> - int fmax = field->logical_maximum;
> - int fuzz = snratio ? (fmax - fmin) / snratio : 0;
> - input_set_abs_params(input, code, fmin, fmax, fuzz, 0);
> -}
> -
> -static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> - struct hid_field *field, struct hid_usage *usage,
> - unsigned long **bit, int *max)
> -{
> - struct input_dev *input = hi->input;
> -
> - switch (usage->hid & HID_USAGE_PAGE) {
> -
> - case HID_UP_GENDESK:
> - switch (usage->hid) {
> - case HID_GD_X:
> - field->logical_maximum = 32760;
> - hid_map_usage(hi, usage, bit, max,
> - EV_ABS, ABS_MT_POSITION_X);
> - set_abs(input, ABS_MT_POSITION_X, field, SN_MOVE);
> - /* touchscreen emulation */
> - set_abs(input, ABS_X, field, SN_MOVE);
> - return 1;
> - case HID_GD_Y:
> - field->logical_maximum = 32760;
> - hid_map_usage(hi, usage, bit, max,
> - EV_ABS, ABS_MT_POSITION_Y);
> - set_abs(input, ABS_MT_POSITION_Y, field, SN_MOVE);
> - /* touchscreen emulation */
> - set_abs(input, ABS_Y, field, SN_MOVE);
> - return 1;
> - }
> - return 0;
> -
> - case HID_UP_DIGITIZER:
> - switch (usage->hid) {
> - case HID_DG_TIPSWITCH:
> - /* touchscreen emulation */
> - hid_map_usage(hi, usage, bit, max, EV_KEY, BTN_TOUCH);
> - input_set_capability(input, EV_KEY, BTN_TOUCH);
> - return 1;
> - case HID_DG_INRANGE:
> - case HID_DG_CONFIDENCE:
> - case HID_DG_CONTACTCOUNT:
> - case HID_DG_CONTACTMAX:
> - return -1;
> - case HID_DG_CONTACTID:
> - input_mt_init_slots(input, MAX_SLOTS);
> - return 1;
> - case HID_DG_TIPPRESSURE:
> - field->logical_minimum = 0;
> - hid_map_usage(hi, usage, bit, max,
> - EV_ABS, ABS_MT_PRESSURE);
> - set_abs(input, ABS_MT_PRESSURE, field, SN_PRESSURE);
> - /* touchscreen emulation */
> - set_abs(input, ABS_PRESSURE, field, SN_PRESSURE);
> - return 1;
> - }
> - return 0;
> - }
> -
> - /* ignore others (from other reports we won't get anyway) */
> - return -1;
> -}
> -
> -static int egalax_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 finger has been parsed,
> - * so that it can decide what to send to the input layer.
> - */
> -static void egalax_filter_event(struct egalax_data *td, struct input_dev *input)
> -{
> - input_mt_slot(input, td->slot);
> - input_mt_report_slot_state(input, MT_TOOL_FINGER, td->touch);
> - if (td->touch) {
> - input_event(input, EV_ABS, ABS_MT_POSITION_X, td->x);
> - input_event(input, EV_ABS, ABS_MT_POSITION_Y, td->y);
> - input_event(input, EV_ABS, ABS_MT_PRESSURE, td->z);
> - }
> - input_mt_report_pointer_emulation(input, true);
> -}
> -
> -static int egalax_event(struct hid_device *hid, struct hid_field *field,
> - struct hid_usage *usage, __s32 value)
> -{
> - struct egalax_data *td = hid_get_drvdata(hid);
> -
> - /* Note, eGalax has two product lines: the first is resistive and
> - * uses a standard parallel multitouch protocol (product ID ==
> - * 48xx). The second is capacitive and uses an unusual "serial"
> - * protocol with a different message for each multitouch finger
> - * (product ID == 72xx).
> - */
> - 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_CONFIDENCE:
> - /* avoid interference from generic hidinput handling */
> - break;
> - case HID_DG_TIPSWITCH:
> - td->touch = value;
> - break;
> - case HID_DG_TIPPRESSURE:
> - td->z = value;
> - break;
> - case HID_DG_CONTACTID:
> - td->slot = clamp_val(value, 0, MAX_SLOTS - 1);
> - break;
> - case HID_GD_X:
> - td->x = value;
> - break;
> - case HID_GD_Y:
> - td->y = value;
> - /* this is the last field in a finger */
> - if (td->valid)
> - egalax_filter_event(td, input);
> - break;
> - case HID_DG_CONTACTCOUNT:
> - /* touch emulation: this is the last field in a frame */
> - 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 egalax_probe(struct hid_device *hdev, const struct hid_device_id *id)
> -{
> - int ret;
> - struct egalax_data *td;
> - struct hid_report *report;
> -
> - td = kzalloc(sizeof(struct egalax_data), GFP_KERNEL);
> - if (!td) {
> - hid_err(hdev, "cannot allocate eGalax data\n");
> - return -ENOMEM;
> - }
> - hid_set_drvdata(hdev, td);
> -
> - ret = hid_parse(hdev);
> - if (ret)
> - goto end;
> -
> - ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> - if (ret)
> - goto end;
> -
> - report = hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[5];
> - if (report) {
> - report->field[0]->value[0] = 2;
> - usbhid_submit_report(hdev, report, USB_DIR_OUT);
> - }
> -
> -end:
> - if (ret)
> - kfree(td);
> -
> - return ret;
> -}
> -
> -static void egalax_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 egalax_devices[] = {
> - { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> - USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH) },
> - { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> - USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH1) },
> - { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> - USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH2) },
> - { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> - USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH3) },
> - { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> - USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH4) },
> - { }
> -};
> -MODULE_DEVICE_TABLE(hid, egalax_devices);
> -
> -static const struct hid_usage_id egalax_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 egalax_driver = {
> - .name = "egalax-touch",
> - .id_table = egalax_devices,
> - .probe = egalax_probe,
> - .remove = egalax_remove,
> - .input_mapping = egalax_input_mapping,
> - .input_mapped = egalax_input_mapped,
> - .usage_table = egalax_grabbed_usages,
> - .event = egalax_event,
> -};
> -
> -static int __init egalax_init(void)
> -{
> - return hid_register_driver(&egalax_driver);
> -}
> -
> -static void __exit egalax_exit(void)
> -{
> - hid_unregister_driver(&egalax_driver);
> -}
> -
> -module_init(egalax_init);
> -module_exit(egalax_exit);
> -
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 07d3183..58df924 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -36,6 +36,8 @@ MODULE_LICENSE("GPL");
> #define MT_QUIRK_SLOT_IS_CONTACTNUMBER (1 << 3)
> #define MT_QUIRK_VALID_IS_INRANGE (1 << 4)
> #define MT_QUIRK_VALID_IS_CONFIDENCE (1 << 5)
> +#define MT_QUIRK_EGALAX (1 << 6)
> +
Please rebase against the branch for-next of Jiri's tree (at
http://git.kernel.org/?p=linux/kernel/git/jikos/hid.git;a=summary)
Also, I'm not in favor of this quirk. I know that I first introduced
MT_QUIRK_CYPRESS, but I was in a hurry and did not took the time to
find a better name. The problem here is that QUIRK_EGALAX infers a lot
of changes in the driver (X, Y, Z corrections, etc)
>
> struct mt_slot {
> __s32 x, y, p, w, h;
> @@ -69,6 +71,7 @@ struct mt_class {
> #define MT_CLS_DUAL1 2
> #define MT_CLS_DUAL2 3
> #define MT_CLS_CYPRESS 4
> +#define MT_CLS_EGALAX 5
please define MT_CLS_EGALAX_CAPACITIVE and MT_CLS_EGALAX_RESISTIVE
>
> /*
> * these device-dependent functions determine what slot corresponds
> @@ -118,7 +121,13 @@ struct mt_class mt_classes[] = {
> .quirks = MT_QUIRK_NOT_SEEN_MEANS_UP |
> MT_QUIRK_CYPRESS,
> .maxcontacts = 10 },
> -
> + { .name = MT_CLS_EGALAX,
> + .quirks = MT_QUIRK_SLOT_IS_CONTACTID |
> + MT_QUIRK_VALID_IS_INRANGE |
> + MT_QUIRK_EGALAX,
> + .maxcontacts = 2 ,
> + .sn_move = 4096,
> + .sn_pressure = 32},
MT_CLS_EGALAX_CAPACITIVE and MT_CLS_EGALAX_RESISTIVE
The differences are in the quirks (see later in the review)
> { }
> };
>
> @@ -146,9 +155,16 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> {
> struct mt_device *td = hid_get_drvdata(hdev);
> struct mt_class *cls = td->mtclass;
> +
> switch (usage->hid & HID_USAGE_PAGE) {
>
> case HID_UP_GENDESK:
> +
> + /* fix up the X/Y limits for egalax devices*/
> + if ((cls->quirks & MT_QUIRK_EGALAX)
> + && ((usage->hid == HID_GD_X) || (usage->hid == HID_GD_Y)))
> + field->logical_maximum = 32760;
> +
This should not be treated by a quirk. You should add fields in the
struct mt_class so that any other devices can expect the benefice. It
also removes the numerical constants in the code.
Also, this should go in the switch below.
> switch (usage->hid) {
> case HID_GD_X:
> hid_map_usage(hi, usage, bit, max,
> @@ -203,6 +219,10 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> td->last_slot_field = usage->hid;
> return 1;
> case HID_DG_TIPPRESSURE:
> + /* fix up the pressure range for egalax devices*/
> + if (cls->quirks & MT_QUIRK_EGALAX)
> + field->logical_minimum = 0;
> +
You can avoid the test here. Except eGalax, no known drivers set the
logical minimum for Pressure to a different value than 0.
> hid_map_usage(hi, usage, bit, max,
> EV_ABS, ABS_MT_PRESSURE);
> set_abs(hi->input, ABS_MT_PRESSURE, field,
> @@ -269,8 +289,10 @@ static void mt_complete_slot(struct mt_device *td)
>
> if (slotnum >= 0 && slotnum < td->mtclass->maxcontacts)
> td->slots[slotnum] = td->curdata;
> +
Please avoid adding blank lines, this complicates the reading of the patch.
> }
> td->num_received++;
> +
same here.
> }
>
>
> @@ -366,12 +388,15 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
> if (usage->hid == td->last_slot_field)
> mt_complete_slot(td);
>
> - if (field->index == td->last_field_index
> + if ((field->index == td->last_field_index
> && td->num_received >= td->num_expected)
> + || ((quirks & MT_QUIRK_EGALAX)
> + && (usage->hid == td->last_slot_field)))
> + /* for egalax devices: emit an event for every finger
> + to work around the wrong last_field_index in the report*/
checkpatch.pl warns here for a line over 80 characters.
Please use another quirk here for that. Maybe
MT_QUIRK_SEND_AT_EACH_REPORT. Henrik, any suggestions?
> mt_emit_event(td, field->hidinput->input);
>
> }
> -
Please keep the jump ;)
> /* 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);
> @@ -478,6 +503,23 @@ static const struct hid_device_id mt_devices[] = {
> HID_USB_DEVICE(USB_VENDOR_ID_CANDO,
> USB_DEVICE_ID_CANDO_PIXCIR_MULTI_TOUCH) },
>
> + /* eGalax devices */
> + { .driver_data = MT_CLS_EGALAX,
> + HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> + USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH) },
> + { .driver_data = MT_CLS_EGALAX,
> + HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> + USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH1) },
> + { .driver_data = MT_CLS_EGALAX,
> + HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> + USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH2) },
> + { .driver_data = MT_CLS_EGALAX,
> + HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> + USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH3) },
> + { .driver_data = MT_CLS_EGALAX,
> + HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> + USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH4) },
> +
Separate this list in two sections: one for resistive, 0x48xx, and one
for capacitive, 0x72xx.
Maybe we can also change them in the hid-ids.h? Jiri?
Now, we need testings for both types of hardware!
One last thing. When submitting patches, it's common use to add in CC
the authors, in case they do not read very carefully the mailing list.
Thanks,
Benjamin
> { }
> };
> MODULE_DEVICE_TABLE(hid, mt_devices);
> --
> 1.7.1
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework.
2011-03-07 10:07 ` Benjamin Tissoires
@ 2011-03-08 6:08 ` Richard Nauber
2011-03-08 8:04 ` Henrik Rydberg
0 siblings, 1 reply; 21+ messages in thread
From: Richard Nauber @ 2011-03-08 6:08 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: linux-input, Jiri Kosina, Henrik Rydberg, Stéphane Chatty
Hi Benjamin, Hendrick, Jiri and Stéphan,
I would like to give this patch another shot, after Benjamin reviewed it (thanks for that!).
As he pointed out, testing for the capacitive kind of egalax devices is needed. Maybe Stéphan,
has the hardware and would be so kind to test it? I have tried it with my Samsung MB30 display
and it worked:
Bus 002 Device 002: ID 0eef:480e D-WAV Scientific Co., Ltd
(thats USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH3)
I am looking forward to your comments,
Richard
PS: The patch should apply to 2fa403e8b49 of Jiris for-next branch, although I had to cheat
a little and rebase his tree to 2.6.38-rc7 because of some unrelated breakage (ecryptfs).
---- snip ----
[PATCH] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework.
This patch merges the hid-egalax driver into hid-multitouch and
therefore adds two device classes (MT_CLS_EGALAX_CAPACITIVE/RESISTIVE).
It also gains the capability to work around broken hid reports by
overriding X/Y limits and emitting events for each finger
(MT_QUIRK_SEND_AT_EACH_REPORT). As a side effect, this patch
fixes the broken suspend/resume behavior with the old driver.
Signed-off-by: Richard Nauber <Richard.Nauber@gmail.com>
---
drivers/hid/Kconfig | 9 +-
drivers/hid/Makefile | 1 -
drivers/hid/hid-egalax.c | 279 ------------------------------------------
drivers/hid/hid-multitouch.c | 68 ++++++++++-
4 files changed, 68 insertions(+), 289 deletions(-)
delete mode 100644 drivers/hid/hid-egalax.c
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index b152c91..87f0b03 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -160,13 +160,6 @@ config HID_EMS_FF
Currently the following devices are known to be supported:
- Trio Linker Plus II
-config HID_EGALAX
- tristate "eGalax multi-touch panel"
- depends on USB_HID
- ---help---
- Support for the eGalax dual-touch panels, including the
- Joojoo and Wetab tablets.
-
config HID_ELECOM
tristate "ELECOM BM084 bluetooth mouse"
depends on BT_HIDP
@@ -321,6 +314,8 @@ config HID_MULTITOUCH
- IrTouch Infrared USB panels
- Pixcir dual touch panels
- 'Sensing Win7-TwoFinger' panel by GeneralTouch
+ - eGalax dual-touch panels, including the
+ Joojoo and Wetab tablets
If unsure, say N.
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 04496ab..1b6ddd3 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -36,7 +36,6 @@ obj-$(CONFIG_HID_CHICONY) += hid-chicony.o
obj-$(CONFIG_HID_CYPRESS) += hid-cypress.o
obj-$(CONFIG_HID_DRAGONRISE) += hid-drff.o
obj-$(CONFIG_HID_EMS_FF) += hid-emsff.o
-obj-$(CONFIG_HID_EGALAX) += hid-egalax.o
obj-$(CONFIG_HID_ELECOM) += hid-elecom.o
obj-$(CONFIG_HID_EZKEY) += hid-ezkey.o
obj-$(CONFIG_HID_GYRATION) += hid-gyration.o
diff --git a/drivers/hid/hid-egalax.c b/drivers/hid/hid-egalax.c
deleted file mode 100644
index 03bee19..0000000
--- a/drivers/hid/hid-egalax.c
+++ /dev/null
@@ -1,279 +0,0 @@
-/*
- * HID driver for eGalax dual-touch panels
- *
- * Copyright (c) 2010 Stephane Chatty <chatty@enac.fr>
- * Copyright (c) 2010 Henrik Rydberg <rydberg@euromail.se>
- * 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/usb.h>
-#include <linux/input/mt.h>
-#include <linux/slab.h>
-#include "usbhid/usbhid.h"
-
-MODULE_AUTHOR("Stephane Chatty <chatty@enac.fr>");
-MODULE_DESCRIPTION("eGalax dual-touch panel");
-MODULE_LICENSE("GPL");
-
-#include "hid-ids.h"
-
-#define MAX_SLOTS 2
-
-/* estimated signal-to-noise ratios */
-#define SN_MOVE 4096
-#define SN_PRESSURE 32
-
-struct egalax_data {
- int valid;
- int slot;
- int touch;
- int x, y, z;
-};
-
-static void set_abs(struct input_dev *input, unsigned int code,
- struct hid_field *field, int snratio)
-{
- int fmin = field->logical_minimum;
- int fmax = field->logical_maximum;
- int fuzz = snratio ? (fmax - fmin) / snratio : 0;
- input_set_abs_params(input, code, fmin, fmax, fuzz, 0);
-}
-
-static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
- struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
-{
- struct input_dev *input = hi->input;
-
- switch (usage->hid & HID_USAGE_PAGE) {
-
- case HID_UP_GENDESK:
- switch (usage->hid) {
- case HID_GD_X:
- field->logical_maximum = 32760;
- hid_map_usage(hi, usage, bit, max,
- EV_ABS, ABS_MT_POSITION_X);
- set_abs(input, ABS_MT_POSITION_X, field, SN_MOVE);
- /* touchscreen emulation */
- set_abs(input, ABS_X, field, SN_MOVE);
- return 1;
- case HID_GD_Y:
- field->logical_maximum = 32760;
- hid_map_usage(hi, usage, bit, max,
- EV_ABS, ABS_MT_POSITION_Y);
- set_abs(input, ABS_MT_POSITION_Y, field, SN_MOVE);
- /* touchscreen emulation */
- set_abs(input, ABS_Y, field, SN_MOVE);
- return 1;
- }
- return 0;
-
- case HID_UP_DIGITIZER:
- switch (usage->hid) {
- case HID_DG_TIPSWITCH:
- /* touchscreen emulation */
- hid_map_usage(hi, usage, bit, max, EV_KEY, BTN_TOUCH);
- input_set_capability(input, EV_KEY, BTN_TOUCH);
- return 1;
- case HID_DG_INRANGE:
- case HID_DG_CONFIDENCE:
- case HID_DG_CONTACTCOUNT:
- case HID_DG_CONTACTMAX:
- return -1;
- case HID_DG_CONTACTID:
- input_mt_init_slots(input, MAX_SLOTS);
- return 1;
- case HID_DG_TIPPRESSURE:
- field->logical_minimum = 0;
- hid_map_usage(hi, usage, bit, max,
- EV_ABS, ABS_MT_PRESSURE);
- set_abs(input, ABS_MT_PRESSURE, field, SN_PRESSURE);
- /* touchscreen emulation */
- set_abs(input, ABS_PRESSURE, field, SN_PRESSURE);
- return 1;
- }
- return 0;
- }
-
- /* ignore others (from other reports we won't get anyway) */
- return -1;
-}
-
-static int egalax_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 finger has been parsed,
- * so that it can decide what to send to the input layer.
- */
-static void egalax_filter_event(struct egalax_data *td, struct input_dev *input)
-{
- input_mt_slot(input, td->slot);
- input_mt_report_slot_state(input, MT_TOOL_FINGER, td->touch);
- if (td->touch) {
- input_event(input, EV_ABS, ABS_MT_POSITION_X, td->x);
- input_event(input, EV_ABS, ABS_MT_POSITION_Y, td->y);
- input_event(input, EV_ABS, ABS_MT_PRESSURE, td->z);
- }
- input_mt_report_pointer_emulation(input, true);
-}
-
-static int egalax_event(struct hid_device *hid, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
-{
- struct egalax_data *td = hid_get_drvdata(hid);
-
- /* Note, eGalax has two product lines: the first is resistive and
- * uses a standard parallel multitouch protocol (product ID ==
- * 48xx). The second is capacitive and uses an unusual "serial"
- * protocol with a different message for each multitouch finger
- * (product ID == 72xx).
- */
- 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_CONFIDENCE:
- /* avoid interference from generic hidinput handling */
- break;
- case HID_DG_TIPSWITCH:
- td->touch = value;
- break;
- case HID_DG_TIPPRESSURE:
- td->z = value;
- break;
- case HID_DG_CONTACTID:
- td->slot = clamp_val(value, 0, MAX_SLOTS - 1);
- break;
- case HID_GD_X:
- td->x = value;
- break;
- case HID_GD_Y:
- td->y = value;
- /* this is the last field in a finger */
- if (td->valid)
- egalax_filter_event(td, input);
- break;
- case HID_DG_CONTACTCOUNT:
- /* touch emulation: this is the last field in a frame */
- 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 egalax_probe(struct hid_device *hdev, const struct hid_device_id *id)
-{
- int ret;
- struct egalax_data *td;
- struct hid_report *report;
-
- td = kzalloc(sizeof(struct egalax_data), GFP_KERNEL);
- if (!td) {
- hid_err(hdev, "cannot allocate eGalax data\n");
- return -ENOMEM;
- }
- hid_set_drvdata(hdev, td);
-
- ret = hid_parse(hdev);
- if (ret)
- goto end;
-
- ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
- if (ret)
- goto end;
-
- report = hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[5];
- if (report) {
- report->field[0]->value[0] = 2;
- usbhid_submit_report(hdev, report, USB_DIR_OUT);
- }
-
-end:
- if (ret)
- kfree(td);
-
- return ret;
-}
-
-static void egalax_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 egalax_devices[] = {
- { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
- USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH) },
- { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
- USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH1) },
- { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
- USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH2) },
- { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
- USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH3) },
- { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
- USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH4) },
- { }
-};
-MODULE_DEVICE_TABLE(hid, egalax_devices);
-
-static const struct hid_usage_id egalax_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 egalax_driver = {
- .name = "egalax-touch",
- .id_table = egalax_devices,
- .probe = egalax_probe,
- .remove = egalax_remove,
- .input_mapping = egalax_input_mapping,
- .input_mapped = egalax_input_mapped,
- .usage_table = egalax_grabbed_usages,
- .event = egalax_event,
-};
-
-static int __init egalax_init(void)
-{
- return hid_register_driver(&egalax_driver);
-}
-
-static void __exit egalax_exit(void)
-{
- hid_unregister_driver(&egalax_driver);
-}
-
-module_init(egalax_init);
-module_exit(egalax_exit);
-
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 65e7f20..a00f3d8 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -37,6 +37,7 @@ MODULE_LICENSE("GPL");
#define MT_QUIRK_SLOT_IS_CONTACTNUMBER (1 << 3)
#define MT_QUIRK_VALID_IS_INRANGE (1 << 4)
#define MT_QUIRK_VALID_IS_CONFIDENCE (1 << 5)
+#define MT_QUIRK_SEND_AT_EACH_REPORT (1 << 6)
struct mt_slot {
__s32 x, y, p, w, h;
@@ -63,6 +64,9 @@ struct mt_class {
__s32 sn_move; /* Signal/noise ratio for move events */
__s32 sn_pressure; /* Signal/noise ratio for pressure events */
__u8 maxcontacts;
+ __u8 override_logical_limits; /* correct the reported X/Y range */
+ __u32 logical_min[2];
+ __u32 logical_max[2];
};
/* classes of device behavior */
@@ -70,6 +74,8 @@ 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_EGALAX_CAPACITIVE 5
+#define MT_CLS_EGALAX_RESISTIVE 6
/*
* these device-dependent functions determine what slot corresponds
@@ -119,7 +125,25 @@ struct mt_class mt_classes[] = {
.quirks = MT_QUIRK_NOT_SEEN_MEANS_UP |
MT_QUIRK_CYPRESS,
.maxcontacts = 10 },
-
+ { .name = MT_CLS_EGALAX_CAPACITIVE,
+ .quirks = MT_QUIRK_SLOT_IS_CONTACTID |
+ MT_QUIRK_VALID_IS_INRANGE,
+ .maxcontacts = 2,
+ .sn_move = 4096,
+ .sn_pressure = 32,
+ .override_logical_limits = 1,
+ .logical_min = { 0, 0 },
+ .logical_max = { 32760, 32760 } },
+ { .name = MT_CLS_EGALAX_RESISTIVE,
+ .quirks = MT_QUIRK_SLOT_IS_CONTACTID |
+ MT_QUIRK_VALID_IS_INRANGE |
+ MT_QUIRK_SEND_AT_EACH_REPORT,
+ .maxcontacts = 2,
+ .sn_move = 4096,
+ .sn_pressure = 32,
+ .override_logical_limits = 1,
+ .logical_min = { 0, 0 },
+ .logical_max = { 32760, 32760 } },
{ }
};
@@ -147,11 +171,19 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
{
struct mt_device *td = hid_get_drvdata(hdev);
struct mt_class *cls = td->mtclass;
+
switch (usage->hid & HID_USAGE_PAGE) {
case HID_UP_GENDESK:
+
switch (usage->hid) {
case HID_GD_X:
+ /* fix up the reported X limits if necessary*/
+ if (cls->override_logical_limits) {
+ field->logical_minimum = cls->logical_min[0];
+ field->logical_maximum = cls->logical_max[0];
+ }
+
hid_map_usage(hi, usage, bit, max,
EV_ABS, ABS_MT_POSITION_X);
set_abs(hi->input, ABS_MT_POSITION_X, field,
@@ -161,6 +193,11 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
td->last_slot_field = usage->hid;
return 1;
case HID_GD_Y:
+ /* fix up the reported Y limits if nessecary*/
+ if (cls->override_logical_limits) {
+ field->logical_minimum = cls->logical_min[1];
+ field->logical_maximum = cls->logical_max[1];
+ }
hid_map_usage(hi, usage, bit, max,
EV_ABS, ABS_MT_POSITION_Y);
set_abs(hi->input, ABS_MT_POSITION_Y, field,
@@ -204,6 +241,10 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
td->last_slot_field = usage->hid;
return 1;
case HID_DG_TIPPRESSURE:
+ /* fix up the pressure range for some devices
+ with broken report */
+ field->logical_minimum = 0;
+
hid_map_usage(hi, usage, bit, max,
EV_ABS, ABS_MT_PRESSURE);
set_abs(hi->input, ABS_MT_PRESSURE, field,
@@ -367,8 +408,12 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
if (usage->hid == td->last_slot_field)
mt_complete_slot(td);
- if (field->index == td->last_field_index
+ if ((field->index == td->last_field_index
&& td->num_received >= td->num_expected)
+ || ((quirks & MT_QUIRK_SEND_AT_EACH_REPORT)
+ && (usage->hid == td->last_slot_field)))
+ /* emit an event for every finger to work around
+ a corrupt last_field_index in the hid report*/
mt_emit_event(td, field->hidinput->input);
}
@@ -484,6 +529,25 @@ static const struct hid_device_id mt_devices[] = {
HID_USB_DEVICE(USB_VENDOR_ID_CANDO,
USB_DEVICE_ID_CANDO_PIXCIR_MULTI_TOUCH) },
+ /* Resistive eGalax devices */
+ { .driver_data = MT_CLS_EGALAX_RESISTIVE,
+ HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
+ USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH) },
+ { .driver_data = MT_CLS_EGALAX_RESISTIVE,
+ HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
+ USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH3) },
+
+ /* Capacitive eGalax devices */
+ { .driver_data = MT_CLS_EGALAX_CAPACITIVE,
+ HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
+ USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH1) },
+ { .driver_data = MT_CLS_EGALAX_CAPACITIVE,
+ HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
+ USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH2) },
+ { .driver_data = MT_CLS_EGALAX_CAPACITIVE,
+ HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
+ USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH4) },
+
{ }
};
MODULE_DEVICE_TABLE(hid, mt_devices);
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework.
2011-03-08 6:08 ` [PATCH v2] " Richard Nauber
@ 2011-03-08 8:04 ` Henrik Rydberg
2011-03-08 8:55 ` Benjamin Tissoires
0 siblings, 1 reply; 21+ messages in thread
From: Henrik Rydberg @ 2011-03-08 8:04 UTC (permalink / raw)
To: Richard Nauber
Cc: Benjamin Tissoires, linux-input, Jiri Kosina,
Stéphane Chatty
Hi Richard,
> Hi Benjamin, Hendrick, Jiri and Stéphan,
The name is Henrik.
> I would like to give this patch another shot, after Benjamin reviewed it (thanks for that!).
> As he pointed out, testing for the capacitive kind of egalax devices is needed. Maybe Stéphan,
> has the hardware and would be so kind to test it? I have tried it with my Samsung MB30 display
> and it worked:
> Bus 002 Device 002: ID 0eef:480e D-WAV Scientific Co., Ltd
> (thats USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH3)
>
> I am looking forward to your comments,
> Richard
>
>
> PS: The patch should apply to 2fa403e8b49 of Jiris for-next branch, although I had to cheat
> a little and rebase his tree to 2.6.38-rc7 because of some unrelated breakage (ecryptfs).
Thanks for making this patch. Please find some comments inline. Also,
since you are removing copyright from something that was largely
copied into this driver, perhaps something should be done about that.
> [PATCH] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework.
>
> This patch merges the hid-egalax driver into hid-multitouch and
> therefore adds two device classes (MT_CLS_EGALAX_CAPACITIVE/RESISTIVE).
> It also gains the capability to work around broken hid reports by
> overriding X/Y limits and emitting events for each finger
> (MT_QUIRK_SEND_AT_EACH_REPORT). As a side effect, this patch
> fixes the broken suspend/resume behavior with the old driver.
>
> Signed-off-by: Richard Nauber <Richard.Nauber@gmail.com>
> ---
> drivers/hid/Kconfig | 9 +-
> drivers/hid/Makefile | 1 -
> drivers/hid/hid-egalax.c | 279 ------------------------------------------
> drivers/hid/hid-multitouch.c | 68 ++++++++++-
> 4 files changed, 68 insertions(+), 289 deletions(-)
> delete mode 100644 drivers/hid/hid-egalax.c
>
[...]
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 65e7f20..a00f3d8 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -37,6 +37,7 @@ MODULE_LICENSE("GPL");
> #define MT_QUIRK_SLOT_IS_CONTACTNUMBER (1 << 3)
> #define MT_QUIRK_VALID_IS_INRANGE (1 << 4)
> #define MT_QUIRK_VALID_IS_CONFIDENCE (1 << 5)
> +#define MT_QUIRK_SEND_AT_EACH_REPORT (1 << 6)
>
> struct mt_slot {
> __s32 x, y, p, w, h;
> @@ -63,6 +64,9 @@ struct mt_class {
> __s32 sn_move; /* Signal/noise ratio for move events */
> __s32 sn_pressure; /* Signal/noise ratio for pressure events */
> __u8 maxcontacts;
> + __u8 override_logical_limits; /* correct the reported X/Y range */
> + __u32 logical_min[2];
> + __u32 logical_max[2];
Please think about byte alignment here, keeping elements of the same
size together. Also, the override needs a specific name, given that it
applies to the whole class, not just the x and y positions. Perhaps
the override should be triggered with a quirk instead?
> };
>
> /* classes of device behavior */
> @@ -70,6 +74,8 @@ 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_EGALAX_CAPACITIVE 5
> +#define MT_CLS_EGALAX_RESISTIVE 6
>
> /*
> * these device-dependent functions determine what slot corresponds
> @@ -119,7 +125,25 @@ struct mt_class mt_classes[] = {
> .quirks = MT_QUIRK_NOT_SEEN_MEANS_UP |
> MT_QUIRK_CYPRESS,
> .maxcontacts = 10 },
> -
> + { .name = MT_CLS_EGALAX_CAPACITIVE,
> + .quirks = MT_QUIRK_SLOT_IS_CONTACTID |
> + MT_QUIRK_VALID_IS_INRANGE,
> + .maxcontacts = 2,
> + .sn_move = 4096,
> + .sn_pressure = 32,
> + .override_logical_limits = 1,
> + .logical_min = { 0, 0 },
> + .logical_max = { 32760, 32760 } },
> + { .name = MT_CLS_EGALAX_RESISTIVE,
> + .quirks = MT_QUIRK_SLOT_IS_CONTACTID |
> + MT_QUIRK_VALID_IS_INRANGE |
> + MT_QUIRK_SEND_AT_EACH_REPORT,
> + .maxcontacts = 2,
> + .sn_move = 4096,
> + .sn_pressure = 32,
> + .override_logical_limits = 1,
> + .logical_min = { 0, 0 },
> + .logical_max = { 32760, 32760 } },
> { }
> };
>
> @@ -147,11 +171,19 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> {
> struct mt_device *td = hid_get_drvdata(hdev);
> struct mt_class *cls = td->mtclass;
> +
Please check for patch for unrelated changes like this.
> switch (usage->hid & HID_USAGE_PAGE) {
>
> case HID_UP_GENDESK:
> +
> switch (usage->hid) {
> case HID_GD_X:
> + /* fix up the reported X limits if necessary*/
> + if (cls->override_logical_limits) {
> + field->logical_minimum = cls->logical_min[0];
> + field->logical_maximum = cls->logical_max[0];
> + }
> +
Something like "if (quirks & MT_QUIRK_FIX_X)", and then on next line "field->logical_maximum = cls->max_x".
> hid_map_usage(hi, usage, bit, max,
> EV_ABS, ABS_MT_POSITION_X);
> set_abs(hi->input, ABS_MT_POSITION_X, field,
> @@ -161,6 +193,11 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> td->last_slot_field = usage->hid;
> return 1;
> case HID_GD_Y:
> + /* fix up the reported Y limits if nessecary*/
> + if (cls->override_logical_limits) {
> + field->logical_minimum = cls->logical_min[1];
> + field->logical_maximum = cls->logical_max[1];
> + }
Ditto.
> hid_map_usage(hi, usage, bit, max,
> EV_ABS, ABS_MT_POSITION_Y);
> set_abs(hi->input, ABS_MT_POSITION_Y, field,
> @@ -204,6 +241,10 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> td->last_slot_field = usage->hid;
> return 1;
> case HID_DG_TIPPRESSURE:
> + /* fix up the pressure range for some devices
> + with broken report */
> + field->logical_minimum = 0;
> +
> hid_map_usage(hi, usage, bit, max,
> EV_ABS, ABS_MT_PRESSURE);
> set_abs(hi->input, ABS_MT_PRESSURE, field,
> @@ -367,8 +408,12 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
> if (usage->hid == td->last_slot_field)
> mt_complete_slot(td);
>
> - if (field->index == td->last_field_index
> + if ((field->index == td->last_field_index
> && td->num_received >= td->num_expected)
> + || ((quirks & MT_QUIRK_SEND_AT_EACH_REPORT)
> + && (usage->hid == td->last_slot_field)))
> + /* emit an event for every finger to work around
> + a corrupt last_field_index in the hid report*/
> mt_emit_event(td, field->hidinput->input);
>
> }
> @@ -484,6 +529,25 @@ static const struct hid_device_id mt_devices[] = {
> HID_USB_DEVICE(USB_VENDOR_ID_CANDO,
> USB_DEVICE_ID_CANDO_PIXCIR_MULTI_TOUCH) },
>
> + /* Resistive eGalax devices */
> + { .driver_data = MT_CLS_EGALAX_RESISTIVE,
> + HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> + USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH) },
> + { .driver_data = MT_CLS_EGALAX_RESISTIVE,
> + HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> + USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH3) },
> +
> + /* Capacitive eGalax devices */
> + { .driver_data = MT_CLS_EGALAX_CAPACITIVE,
> + HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> + USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH1) },
> + { .driver_data = MT_CLS_EGALAX_CAPACITIVE,
> + HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> + USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH2) },
> + { .driver_data = MT_CLS_EGALAX_CAPACITIVE,
> + HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> + USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH4) },
> +
> { }
> };
> MODULE_DEVICE_TABLE(hid, mt_devices);
> --
> 1.7.1
>
Thanks,
Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework.
2011-03-08 8:04 ` Henrik Rydberg
@ 2011-03-08 8:55 ` Benjamin Tissoires
2011-03-08 9:14 ` Henrik Rydberg
0 siblings, 1 reply; 21+ messages in thread
From: Benjamin Tissoires @ 2011-03-08 8:55 UTC (permalink / raw)
To: Henrik Rydberg
Cc: Richard Nauber, linux-input, Jiri Kosina, Stéphane Chatty
Hi Richard,
thanks for this new version. Just some more comments on top of Henrik's ones.
On Tue, Mar 8, 2011 at 09:04, Henrik Rydberg <rydberg@euromail.se> wrote:
> Hi Richard,
>
>> Hi Benjamin, Hendrick, Jiri and Stéphan,
>
> The name is Henrik.
>
>> I would like to give this patch another shot, after Benjamin reviewed it (thanks for that!).
>> As he pointed out, testing for the capacitive kind of egalax devices is needed. Maybe Stéphan,
>> has the hardware and would be so kind to test it? I have tried it with my Samsung MB30 display
>> and it worked:
>> Bus 002 Device 002: ID 0eef:480e D-WAV Scientific Co., Ltd
>> (thats USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH3)
>>
>> I am looking forward to your comments,
>> Richard
>>
>>
>> PS: The patch should apply to 2fa403e8b49 of Jiris for-next branch, although I had to cheat
>> a little and rebase his tree to 2.6.38-rc7 because of some unrelated breakage (ecryptfs).
>
> Thanks for making this patch. Please find some comments inline. Also,
> since you are removing copyright from something that was largely
> copied into this driver, perhaps something should be done about that.
>
>> [PATCH] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework.
>>
>> This patch merges the hid-egalax driver into hid-multitouch and
>> therefore adds two device classes (MT_CLS_EGALAX_CAPACITIVE/RESISTIVE).
>> It also gains the capability to work around broken hid reports by
>> overriding X/Y limits and emitting events for each finger
>> (MT_QUIRK_SEND_AT_EACH_REPORT). As a side effect, this patch
>> fixes the broken suspend/resume behavior with the old driver.
>>
>> Signed-off-by: Richard Nauber <Richard.Nauber@gmail.com>
>> ---
>> drivers/hid/Kconfig | 9 +-
>> drivers/hid/Makefile | 1 -
>> drivers/hid/hid-egalax.c | 279 ------------------------------------------
>> drivers/hid/hid-multitouch.c | 68 ++++++++++-
>> 4 files changed, 68 insertions(+), 289 deletions(-)
>> delete mode 100644 drivers/hid/hid-egalax.c
>>
> [...]
>> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
>> index 65e7f20..a00f3d8 100644
>> --- a/drivers/hid/hid-multitouch.c
>> +++ b/drivers/hid/hid-multitouch.c
>> @@ -37,6 +37,7 @@ MODULE_LICENSE("GPL");
>> #define MT_QUIRK_SLOT_IS_CONTACTNUMBER (1 << 3)
>> #define MT_QUIRK_VALID_IS_INRANGE (1 << 4)
>> #define MT_QUIRK_VALID_IS_CONFIDENCE (1 << 5)
>> +#define MT_QUIRK_SEND_AT_EACH_REPORT (1 << 6)
>>
>> struct mt_slot {
>> __s32 x, y, p, w, h;
>> @@ -63,6 +64,9 @@ struct mt_class {
>> __s32 sn_move; /* Signal/noise ratio for move events */
>> __s32 sn_pressure; /* Signal/noise ratio for pressure events */
>> __u8 maxcontacts;
>> + __u8 override_logical_limits; /* correct the reported X/Y range */
>> + __u32 logical_min[2];
>> + __u32 logical_max[2];
>
> Please think about byte alignment here, keeping elements of the same
> size together. Also, the override needs a specific name, given that it
> applies to the whole class, not just the x and y positions. Perhaps
> the override should be triggered with a quirk instead?
I'm not in favor of a quirk in this particular case: the information
is already here: max > 0.
For the specific name, the too fields logical_min/max are not well chosen:
either use an anonymous struct to have .x, .y so we can add .pressure,
.width, etc...
or rename logical_min to logical_min_xy.
I just saw that Henrik suggested max_x, max_y, etc... chose this one, please.
The alignment makes sense!
>
>> };
>>
>> /* classes of device behavior */
>> @@ -70,6 +74,8 @@ 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_EGALAX_CAPACITIVE 5
>> +#define MT_CLS_EGALAX_RESISTIVE 6
>>
>> /*
>> * these device-dependent functions determine what slot corresponds
>> @@ -119,7 +125,25 @@ struct mt_class mt_classes[] = {
>> .quirks = MT_QUIRK_NOT_SEEN_MEANS_UP |
>> MT_QUIRK_CYPRESS,
>> .maxcontacts = 10 },
>> -
>> + { .name = MT_CLS_EGALAX_CAPACITIVE,
>> + .quirks = MT_QUIRK_SLOT_IS_CONTACTID |
>> + MT_QUIRK_VALID_IS_INRANGE,
>> + .maxcontacts = 2,
>> + .sn_move = 4096,
>> + .sn_pressure = 32,
>> + .override_logical_limits = 1,
>> + .logical_min = { 0, 0 },
>> + .logical_max = { 32760, 32760 } },
>> + { .name = MT_CLS_EGALAX_RESISTIVE,
>> + .quirks = MT_QUIRK_SLOT_IS_CONTACTID |
>> + MT_QUIRK_VALID_IS_INRANGE |
>> + MT_QUIRK_SEND_AT_EACH_REPORT,
>> + .maxcontacts = 2,
>> + .sn_move = 4096,
>> + .sn_pressure = 32,
>> + .override_logical_limits = 1,
>> + .logical_min = { 0, 0 },
>> + .logical_max = { 32760, 32760 } },
>> { }
>> };
>>
>> @@ -147,11 +171,19 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>> {
>> struct mt_device *td = hid_get_drvdata(hdev);
>> struct mt_class *cls = td->mtclass;
>> +
>
> Please check for patch for unrelated changes like this.
>
>> switch (usage->hid & HID_USAGE_PAGE) {
>>
>> case HID_UP_GENDESK:
>> +
>> switch (usage->hid) {
>> case HID_GD_X:
>> + /* fix up the reported X limits if necessary*/
>> + if (cls->override_logical_limits) {
>> + field->logical_minimum = cls->logical_min[0];
>> + field->logical_maximum = cls->logical_max[0];
>> + }
>> +
>
> Something like "if (quirks & MT_QUIRK_FIX_X)", and then on next line "field->logical_maximum = cls->max_x".
or just "if (cls->max_x)" and then the two "field->logical_maximum =
cls->max_x" and "field->logical_minimum = cls->min_x".
>
>> hid_map_usage(hi, usage, bit, max,
>> EV_ABS, ABS_MT_POSITION_X);
>> set_abs(hi->input, ABS_MT_POSITION_X, field,
>> @@ -161,6 +193,11 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>> td->last_slot_field = usage->hid;
>> return 1;
>> case HID_GD_Y:
>> + /* fix up the reported Y limits if nessecary*/
>> + if (cls->override_logical_limits) {
>> + field->logical_minimum = cls->logical_min[1];
>> + field->logical_maximum = cls->logical_max[1];
>> + }
>
> Ditto.
>
>> hid_map_usage(hi, usage, bit, max,
>> EV_ABS, ABS_MT_POSITION_Y);
>> set_abs(hi->input, ABS_MT_POSITION_Y, field,
>> @@ -204,6 +241,10 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>> td->last_slot_field = usage->hid;
>> return 1;
>> case HID_DG_TIPPRESSURE:
>> + /* fix up the pressure range for some devices
>> + with broken report */
>> + field->logical_minimum = 0;
>> +
>> hid_map_usage(hi, usage, bit, max,
>> EV_ABS, ABS_MT_PRESSURE);
>> set_abs(hi->input, ABS_MT_PRESSURE, field,
>> @@ -367,8 +408,12 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
>> if (usage->hid == td->last_slot_field)
>> mt_complete_slot(td);
>>
>> - if (field->index == td->last_field_index
>> + if ((field->index == td->last_field_index
>> && td->num_received >= td->num_expected)
>> + || ((quirks & MT_QUIRK_SEND_AT_EACH_REPORT)
>> + && (usage->hid == td->last_slot_field)))
>> + /* emit an event for every finger to work around
>> + a corrupt last_field_index in the hid report*/
>> mt_emit_event(td, field->hidinput->input);
>>
>> }
>> @@ -484,6 +529,25 @@ static const struct hid_device_id mt_devices[] = {
>> HID_USB_DEVICE(USB_VENDOR_ID_CANDO,
>> USB_DEVICE_ID_CANDO_PIXCIR_MULTI_TOUCH) },
>>
>> + /* Resistive eGalax devices */
>> + { .driver_data = MT_CLS_EGALAX_RESISTIVE,
>> + HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
>> + USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH) },
>> + { .driver_data = MT_CLS_EGALAX_RESISTIVE,
>> + HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
>> + USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH3) },
>> +
>> + /* Capacitive eGalax devices */
>> + { .driver_data = MT_CLS_EGALAX_CAPACITIVE,
>> + HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
>> + USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH1) },
>> + { .driver_data = MT_CLS_EGALAX_CAPACITIVE,
>> + HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
>> + USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH2) },
>> + { .driver_data = MT_CLS_EGALAX_CAPACITIVE,
>> + HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
>> + USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH4) },
>> +
>> { }
>> };
>> MODULE_DEVICE_TABLE(hid, mt_devices);
>> --
>> 1.7.1
>>
>
> Thanks,
> Henrik
>
Thanks,
Benjamin
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework.
2011-03-08 8:55 ` Benjamin Tissoires
@ 2011-03-08 9:14 ` Henrik Rydberg
2011-03-08 9:23 ` Benjamin Tissoires
0 siblings, 1 reply; 21+ messages in thread
From: Henrik Rydberg @ 2011-03-08 9:14 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Richard Nauber, linux-input, Jiri Kosina, Stéphane Chatty
> >> struct mt_slot {
> >> __s32 x, y, p, w, h;
> >> @@ -63,6 +64,9 @@ struct mt_class {
> >> __s32 sn_move; /* Signal/noise ratio for move events */
> >> __s32 sn_pressure; /* Signal/noise ratio for pressure events */
> >> __u8 maxcontacts;
> >> + __u8 override_logical_limits; /* correct the reported X/Y range */
> >> + __u32 logical_min[2];
> >> + __u32 logical_max[2];
> >
> > Please think about byte alignment here, keeping elements of the same
> > size together. Also, the override needs a specific name, given that it
> > applies to the whole class, not just the x and y positions. Perhaps
> > the override should be triggered with a quirk instead?
>
> I'm not in favor of a quirk in this particular case: the information
> is already here: max > 0.
Only if min == 0, which is far from always the case. And setting a
special value for a special hardware does what a quirk does, so maybe
it is a quirk after all.
Thanks,
Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework.
2011-03-08 9:14 ` Henrik Rydberg
@ 2011-03-08 9:23 ` Benjamin Tissoires
2011-03-08 9:46 ` Henrik Rydberg
0 siblings, 1 reply; 21+ messages in thread
From: Benjamin Tissoires @ 2011-03-08 9:23 UTC (permalink / raw)
To: Henrik Rydberg
Cc: Richard Nauber, linux-input, Jiri Kosina, Stéphane Chatty
On Tue, Mar 8, 2011 at 10:14, Henrik Rydberg <rydberg@euromail.se> wrote:
>> >> struct mt_slot {
>> >> __s32 x, y, p, w, h;
>> >> @@ -63,6 +64,9 @@ struct mt_class {
>> >> __s32 sn_move; /* Signal/noise ratio for move events */
>> >> __s32 sn_pressure; /* Signal/noise ratio for pressure events */
>> >> __u8 maxcontacts;
>> >> + __u8 override_logical_limits; /* correct the reported X/Y range */
>> >> + __u32 logical_min[2];
>> >> + __u32 logical_max[2];
>> >
>> > Please think about byte alignment here, keeping elements of the same
>> > size together. Also, the override needs a specific name, given that it
>> > applies to the whole class, not just the x and y positions. Perhaps
>> > the override should be triggered with a quirk instead?
>>
>> I'm not in favor of a quirk in this particular case: the information
>> is already here: max > 0.
>
> Only if min == 0, which is far from always the case. And setting a
> special value for a special hardware does what a quirk does, so maybe
> it is a quirk after all.
>
I think there is a misunderstanding:
if the class provides max_x (so > 0), or min_x, that means that the
person in charge of adding the driver wants to replace the two values
min/max.
No need for a quirk that will be redundant with the hand-provided values.
It won't change anything to other devices as they do not provide min/max.
Cheers,
Benjamin
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework.
2011-03-08 9:23 ` Benjamin Tissoires
@ 2011-03-08 9:46 ` Henrik Rydberg
2011-03-08 23:38 ` [PATCH v3] " Richard Nauber
0 siblings, 1 reply; 21+ messages in thread
From: Henrik Rydberg @ 2011-03-08 9:46 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Richard Nauber, linux-input, Jiri Kosina, Stéphane Chatty
> >> I'm not in favor of a quirk in this particular case: the information
> >> is already here: max > 0.
> >
> > Only if min == 0, which is far from always the case. And setting a
> > special value for a special hardware does what a quirk does, so maybe
> > it is a quirk after all.
> >
>
> I think there is a misunderstanding:
> if the class provides max_x (so > 0), or min_x, that means that the
> person in charge of adding the driver wants to replace the two values
> min/max.
What I meant is that (min_x < max_x) is the proper test.
> No need for a quirk that will be redundant with the hand-provided values.
Yes, but I think we might be better off using a quirk than a generic
way to replace the values provided by the hardware.
Thanks,
Henrik
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework.
2011-03-06 13:25 [PATCH] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework Richard Nauber
2011-03-07 10:07 ` Benjamin Tissoires
@ 2011-03-08 13:12 ` Jiri Kosina
1 sibling, 0 replies; 21+ messages in thread
From: Jiri Kosina @ 2011-03-08 13:12 UTC (permalink / raw)
To: Richard Nauber; +Cc: linux-input, Henrik Rydberg, Stephane Chatty
On Sun, 6 Mar 2011, Richard Nauber wrote:
> --- a/drivers/hid/hid-egalax.c
> +++ /dev/null
> @@ -1,279 +0,0 @@
> -/*
> - * HID driver for eGalax dual-touch panels
> - *
> - * Copyright (c) 2010 Stephane Chatty <chatty@enac.fr>
> - * Copyright (c) 2010 Henrik Rydberg <rydberg@euromail.se>
> - * Copyright (c) 2010 Canonical, Ltd.
As this is (partial) code movement, I believe the copyrights should be
moved as well.
At least something along the lines 'based on hid-egalax.c copyrighted as
follows ...' would be nice.
Added Stephane (the other copyright holder) to CC here.
> - *
> - */
> -
> -/*
> - * 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/usb.h>
> -#include <linux/input/mt.h>
> -#include <linux/slab.h>
> -#include "usbhid/usbhid.h"
> -
> -MODULE_AUTHOR("Stephane Chatty <chatty@enac.fr>");
> -MODULE_DESCRIPTION("eGalax dual-touch panel");
> -MODULE_LICENSE("GPL");
> -
> -#include "hid-ids.h"
> -
> -#define MAX_SLOTS 2
> -
> -/* estimated signal-to-noise ratios */
> -#define SN_MOVE 4096
> -#define SN_PRESSURE 32
> -
> -struct egalax_data {
> - int valid;
> - int slot;
> - int touch;
> - int x, y, z;
> -};
> -
> -static void set_abs(struct input_dev *input, unsigned int code,
> - struct hid_field *field, int snratio)
> -{
> - int fmin = field->logical_minimum;
> - int fmax = field->logical_maximum;
> - int fuzz = snratio ? (fmax - fmin) / snratio : 0;
> - input_set_abs_params(input, code, fmin, fmax, fuzz, 0);
> -}
> -
> -static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> - struct hid_field *field, struct hid_usage *usage,
> - unsigned long **bit, int *max)
> -{
> - struct input_dev *input = hi->input;
> -
> - switch (usage->hid & HID_USAGE_PAGE) {
> -
> - case HID_UP_GENDESK:
> - switch (usage->hid) {
> - case HID_GD_X:
> - field->logical_maximum = 32760;
> - hid_map_usage(hi, usage, bit, max,
> - EV_ABS, ABS_MT_POSITION_X);
> - set_abs(input, ABS_MT_POSITION_X, field, SN_MOVE);
> - /* touchscreen emulation */
> - set_abs(input, ABS_X, field, SN_MOVE);
> - return 1;
> - case HID_GD_Y:
> - field->logical_maximum = 32760;
> - hid_map_usage(hi, usage, bit, max,
> - EV_ABS, ABS_MT_POSITION_Y);
> - set_abs(input, ABS_MT_POSITION_Y, field, SN_MOVE);
> - /* touchscreen emulation */
> - set_abs(input, ABS_Y, field, SN_MOVE);
> - return 1;
> - }
> - return 0;
> -
> - case HID_UP_DIGITIZER:
> - switch (usage->hid) {
> - case HID_DG_TIPSWITCH:
> - /* touchscreen emulation */
> - hid_map_usage(hi, usage, bit, max, EV_KEY, BTN_TOUCH);
> - input_set_capability(input, EV_KEY, BTN_TOUCH);
> - return 1;
> - case HID_DG_INRANGE:
> - case HID_DG_CONFIDENCE:
> - case HID_DG_CONTACTCOUNT:
> - case HID_DG_CONTACTMAX:
> - return -1;
> - case HID_DG_CONTACTID:
> - input_mt_init_slots(input, MAX_SLOTS);
> - return 1;
> - case HID_DG_TIPPRESSURE:
> - field->logical_minimum = 0;
> - hid_map_usage(hi, usage, bit, max,
> - EV_ABS, ABS_MT_PRESSURE);
> - set_abs(input, ABS_MT_PRESSURE, field, SN_PRESSURE);
> - /* touchscreen emulation */
> - set_abs(input, ABS_PRESSURE, field, SN_PRESSURE);
> - return 1;
> - }
> - return 0;
> - }
> -
> - /* ignore others (from other reports we won't get anyway) */
> - return -1;
> -}
> -
> -static int egalax_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 finger has been parsed,
> - * so that it can decide what to send to the input layer.
> - */
> -static void egalax_filter_event(struct egalax_data *td, struct input_dev *input)
> -{
> - input_mt_slot(input, td->slot);
> - input_mt_report_slot_state(input, MT_TOOL_FINGER, td->touch);
> - if (td->touch) {
> - input_event(input, EV_ABS, ABS_MT_POSITION_X, td->x);
> - input_event(input, EV_ABS, ABS_MT_POSITION_Y, td->y);
> - input_event(input, EV_ABS, ABS_MT_PRESSURE, td->z);
> - }
> - input_mt_report_pointer_emulation(input, true);
> -}
> -
> -static int egalax_event(struct hid_device *hid, struct hid_field *field,
> - struct hid_usage *usage, __s32 value)
> -{
> - struct egalax_data *td = hid_get_drvdata(hid);
> -
> - /* Note, eGalax has two product lines: the first is resistive and
> - * uses a standard parallel multitouch protocol (product ID ==
> - * 48xx). The second is capacitive and uses an unusual "serial"
> - * protocol with a different message for each multitouch finger
> - * (product ID == 72xx).
> - */
> - 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_CONFIDENCE:
> - /* avoid interference from generic hidinput handling */
> - break;
> - case HID_DG_TIPSWITCH:
> - td->touch = value;
> - break;
> - case HID_DG_TIPPRESSURE:
> - td->z = value;
> - break;
> - case HID_DG_CONTACTID:
> - td->slot = clamp_val(value, 0, MAX_SLOTS - 1);
> - break;
> - case HID_GD_X:
> - td->x = value;
> - break;
> - case HID_GD_Y:
> - td->y = value;
> - /* this is the last field in a finger */
> - if (td->valid)
> - egalax_filter_event(td, input);
> - break;
> - case HID_DG_CONTACTCOUNT:
> - /* touch emulation: this is the last field in a frame */
> - 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 egalax_probe(struct hid_device *hdev, const struct hid_device_id *id)
> -{
> - int ret;
> - struct egalax_data *td;
> - struct hid_report *report;
> -
> - td = kzalloc(sizeof(struct egalax_data), GFP_KERNEL);
> - if (!td) {
> - hid_err(hdev, "cannot allocate eGalax data\n");
> - return -ENOMEM;
> - }
> - hid_set_drvdata(hdev, td);
> -
> - ret = hid_parse(hdev);
> - if (ret)
> - goto end;
> -
> - ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> - if (ret)
> - goto end;
> -
> - report = hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[5];
> - if (report) {
> - report->field[0]->value[0] = 2;
> - usbhid_submit_report(hdev, report, USB_DIR_OUT);
> - }
> -
> -end:
> - if (ret)
> - kfree(td);
> -
> - return ret;
> -}
> -
> -static void egalax_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 egalax_devices[] = {
> - { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> - USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH) },
> - { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> - USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH1) },
> - { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> - USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH2) },
> - { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> - USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH3) },
> - { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> - USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH4) },
> - { }
> -};
> -MODULE_DEVICE_TABLE(hid, egalax_devices);
> -
> -static const struct hid_usage_id egalax_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 egalax_driver = {
> - .name = "egalax-touch",
> - .id_table = egalax_devices,
> - .probe = egalax_probe,
> - .remove = egalax_remove,
> - .input_mapping = egalax_input_mapping,
> - .input_mapped = egalax_input_mapped,
> - .usage_table = egalax_grabbed_usages,
> - .event = egalax_event,
> -};
> -
> -static int __init egalax_init(void)
> -{
> - return hid_register_driver(&egalax_driver);
> -}
> -
> -static void __exit egalax_exit(void)
> -{
> - hid_unregister_driver(&egalax_driver);
> -}
> -
> -module_init(egalax_init);
> -module_exit(egalax_exit);
> -
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 07d3183..58df924 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -36,6 +36,8 @@ MODULE_LICENSE("GPL");
> #define MT_QUIRK_SLOT_IS_CONTACTNUMBER (1 << 3)
> #define MT_QUIRK_VALID_IS_INRANGE (1 << 4)
> #define MT_QUIRK_VALID_IS_CONFIDENCE (1 << 5)
> +#define MT_QUIRK_EGALAX (1 << 6)
> +
>
> struct mt_slot {
> __s32 x, y, p, w, h;
> @@ -69,6 +71,7 @@ struct mt_class {
> #define MT_CLS_DUAL1 2
> #define MT_CLS_DUAL2 3
> #define MT_CLS_CYPRESS 4
> +#define MT_CLS_EGALAX 5
>
> /*
> * these device-dependent functions determine what slot corresponds
> @@ -118,7 +121,13 @@ struct mt_class mt_classes[] = {
> .quirks = MT_QUIRK_NOT_SEEN_MEANS_UP |
> MT_QUIRK_CYPRESS,
> .maxcontacts = 10 },
> -
> + { .name = MT_CLS_EGALAX,
> + .quirks = MT_QUIRK_SLOT_IS_CONTACTID |
> + MT_QUIRK_VALID_IS_INRANGE |
> + MT_QUIRK_EGALAX,
> + .maxcontacts = 2 ,
> + .sn_move = 4096,
> + .sn_pressure = 32},
> { }
> };
>
> @@ -146,9 +155,16 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> {
> struct mt_device *td = hid_get_drvdata(hdev);
> struct mt_class *cls = td->mtclass;
> +
> switch (usage->hid & HID_USAGE_PAGE) {
>
> case HID_UP_GENDESK:
> +
> + /* fix up the X/Y limits for egalax devices*/
> + if ((cls->quirks & MT_QUIRK_EGALAX)
> + && ((usage->hid == HID_GD_X) || (usage->hid == HID_GD_Y)))
> + field->logical_maximum = 32760;
> +
> switch (usage->hid) {
> case HID_GD_X:
> hid_map_usage(hi, usage, bit, max,
> @@ -203,6 +219,10 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> td->last_slot_field = usage->hid;
> return 1;
> case HID_DG_TIPPRESSURE:
> + /* fix up the pressure range for egalax devices*/
> + if (cls->quirks & MT_QUIRK_EGALAX)
> + field->logical_minimum = 0;
> +
> hid_map_usage(hi, usage, bit, max,
> EV_ABS, ABS_MT_PRESSURE);
> set_abs(hi->input, ABS_MT_PRESSURE, field,
> @@ -269,8 +289,10 @@ static void mt_complete_slot(struct mt_device *td)
>
> if (slotnum >= 0 && slotnum < td->mtclass->maxcontacts)
> td->slots[slotnum] = td->curdata;
> +
> }
> td->num_received++;
> +
> }
>
>
> @@ -366,12 +388,15 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
> if (usage->hid == td->last_slot_field)
> mt_complete_slot(td);
>
> - if (field->index == td->last_field_index
> + if ((field->index == td->last_field_index
> && td->num_received >= td->num_expected)
> + || ((quirks & MT_QUIRK_EGALAX)
> + && (usage->hid == td->last_slot_field)))
> + /* for egalax devices: emit an event for every finger
> + to work around the wrong last_field_index in the report*/
> mt_emit_event(td, field->hidinput->input);
>
> }
> -
> /* 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);
> @@ -478,6 +503,23 @@ static const struct hid_device_id mt_devices[] = {
> HID_USB_DEVICE(USB_VENDOR_ID_CANDO,
> USB_DEVICE_ID_CANDO_PIXCIR_MULTI_TOUCH) },
>
> + /* eGalax devices */
> + { .driver_data = MT_CLS_EGALAX,
> + HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> + USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH) },
> + { .driver_data = MT_CLS_EGALAX,
> + HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> + USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH1) },
> + { .driver_data = MT_CLS_EGALAX,
> + HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> + USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH2) },
> + { .driver_data = MT_CLS_EGALAX,
> + HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> + USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH3) },
> + { .driver_data = MT_CLS_EGALAX,
> + HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> + USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH4) },
> +
> { }
> };
> MODULE_DEVICE_TABLE(hid, mt_devices);
> --
> 1.7.1
>
>
>
--
Jiri Kosina
SUSE Labs, Novell Inc.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework.
2011-03-08 9:46 ` Henrik Rydberg
@ 2011-03-08 23:38 ` Richard Nauber
2011-03-09 8:24 ` Henrik Rydberg
0 siblings, 1 reply; 21+ messages in thread
From: Richard Nauber @ 2011-03-08 23:38 UTC (permalink / raw)
To: Henrik Rydberg
Cc: Benjamin Tissoires, linux-input, Jiri Kosina,
Stéphane Chatty
Hi Henrik :), Benjamin, Stephane and Jiri,
here is the third version of this patch, improved according to your suggestions.
> What I meant is that (min_x < max_x) is the proper test.
>
> > No need for a quirk that will be redundant with the hand-provided values.
>
> Yes, but I think we might be better off using a quirk than a generic
> way to replace the values provided by the hardware.
My personal opinion is that the override should be triggered by your
(min_x < max_x) test, which is safe, because the (min_x == max_x) case
makes no sense for a device and we are allowed to attach a special
meaning to it. Additionally this enables us to conveniently extend the
mechanism to more axis, whose could be individually enabled without a
bloat of quirk defines.
So tell me what you think of this...
Thanks again for the review,
Richard
----snip----
[PATCH] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework.
This patch merges the hid-egalax driver into hid-multitouch and
therefore adds two device classes (MT_CLS_EGALAX_CAPACITIVE/RESISTIVE).
It also gains the capability to work around broken hid reports by
overriding X/Y limits and emitting events for each finger
(MT_QUIRK_SEND_AT_EACH_REPORT). As a side effect, this patch
fixes the broken suspend/resume behavior with the old driver.
Signed-off-by: Richard Nauber <Richard.Nauber@gmail.com>
---
drivers/hid/Kconfig | 9 +-
drivers/hid/Makefile | 1 -
drivers/hid/hid-egalax.c | 279 ------------------------------------------
drivers/hid/hid-multitouch.c | 84 ++++++++++++-
4 files changed, 84 insertions(+), 289 deletions(-)
delete mode 100644 drivers/hid/hid-egalax.c
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 4b88bbe..3e25aa7 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -160,13 +160,6 @@ config HID_EMS_FF
Currently the following devices are known to be supported:
- Trio Linker Plus II
-config HID_EGALAX
- tristate "eGalax multi-touch panel"
- depends on USB_HID
- ---help---
- Support for the eGalax dual-touch panels, including the
- Joojoo and Wetab tablets.
-
config HID_ELECOM
tristate "ELECOM BM084 bluetooth mouse"
depends on BT_HIDP
@@ -321,6 +314,8 @@ config HID_MULTITOUCH
- IrTouch Infrared USB panels
- Pixcir dual touch panels
- 'Sensing Win7-TwoFinger' panel by GeneralTouch
+ - eGalax dual-touch panels, including the
+ Joojoo and Wetab tablets
If unsure, say N.
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 04496ab..1b6ddd3 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -36,7 +36,6 @@ obj-$(CONFIG_HID_CHICONY) += hid-chicony.o
obj-$(CONFIG_HID_CYPRESS) += hid-cypress.o
obj-$(CONFIG_HID_DRAGONRISE) += hid-drff.o
obj-$(CONFIG_HID_EMS_FF) += hid-emsff.o
-obj-$(CONFIG_HID_EGALAX) += hid-egalax.o
obj-$(CONFIG_HID_ELECOM) += hid-elecom.o
obj-$(CONFIG_HID_EZKEY) += hid-ezkey.o
obj-$(CONFIG_HID_GYRATION) += hid-gyration.o
diff --git a/drivers/hid/hid-egalax.c b/drivers/hid/hid-egalax.c
deleted file mode 100644
index 03bee19..0000000
--- a/drivers/hid/hid-egalax.c
+++ /dev/null
@@ -1,279 +0,0 @@
-/*
- * HID driver for eGalax dual-touch panels
- *
- * Copyright (c) 2010 Stephane Chatty <chatty@enac.fr>
- * Copyright (c) 2010 Henrik Rydberg <rydberg@euromail.se>
- * 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/usb.h>
-#include <linux/input/mt.h>
-#include <linux/slab.h>
-#include "usbhid/usbhid.h"
-
-MODULE_AUTHOR("Stephane Chatty <chatty@enac.fr>");
-MODULE_DESCRIPTION("eGalax dual-touch panel");
-MODULE_LICENSE("GPL");
-
-#include "hid-ids.h"
-
-#define MAX_SLOTS 2
-
-/* estimated signal-to-noise ratios */
-#define SN_MOVE 4096
-#define SN_PRESSURE 32
-
-struct egalax_data {
- int valid;
- int slot;
- int touch;
- int x, y, z;
-};
-
-static void set_abs(struct input_dev *input, unsigned int code,
- struct hid_field *field, int snratio)
-{
- int fmin = field->logical_minimum;
- int fmax = field->logical_maximum;
- int fuzz = snratio ? (fmax - fmin) / snratio : 0;
- input_set_abs_params(input, code, fmin, fmax, fuzz, 0);
-}
-
-static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
- struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
-{
- struct input_dev *input = hi->input;
-
- switch (usage->hid & HID_USAGE_PAGE) {
-
- case HID_UP_GENDESK:
- switch (usage->hid) {
- case HID_GD_X:
- field->logical_maximum = 32760;
- hid_map_usage(hi, usage, bit, max,
- EV_ABS, ABS_MT_POSITION_X);
- set_abs(input, ABS_MT_POSITION_X, field, SN_MOVE);
- /* touchscreen emulation */
- set_abs(input, ABS_X, field, SN_MOVE);
- return 1;
- case HID_GD_Y:
- field->logical_maximum = 32760;
- hid_map_usage(hi, usage, bit, max,
- EV_ABS, ABS_MT_POSITION_Y);
- set_abs(input, ABS_MT_POSITION_Y, field, SN_MOVE);
- /* touchscreen emulation */
- set_abs(input, ABS_Y, field, SN_MOVE);
- return 1;
- }
- return 0;
-
- case HID_UP_DIGITIZER:
- switch (usage->hid) {
- case HID_DG_TIPSWITCH:
- /* touchscreen emulation */
- hid_map_usage(hi, usage, bit, max, EV_KEY, BTN_TOUCH);
- input_set_capability(input, EV_KEY, BTN_TOUCH);
- return 1;
- case HID_DG_INRANGE:
- case HID_DG_CONFIDENCE:
- case HID_DG_CONTACTCOUNT:
- case HID_DG_CONTACTMAX:
- return -1;
- case HID_DG_CONTACTID:
- input_mt_init_slots(input, MAX_SLOTS);
- return 1;
- case HID_DG_TIPPRESSURE:
- field->logical_minimum = 0;
- hid_map_usage(hi, usage, bit, max,
- EV_ABS, ABS_MT_PRESSURE);
- set_abs(input, ABS_MT_PRESSURE, field, SN_PRESSURE);
- /* touchscreen emulation */
- set_abs(input, ABS_PRESSURE, field, SN_PRESSURE);
- return 1;
- }
- return 0;
- }
-
- /* ignore others (from other reports we won't get anyway) */
- return -1;
-}
-
-static int egalax_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 finger has been parsed,
- * so that it can decide what to send to the input layer.
- */
-static void egalax_filter_event(struct egalax_data *td, struct input_dev *input)
-{
- input_mt_slot(input, td->slot);
- input_mt_report_slot_state(input, MT_TOOL_FINGER, td->touch);
- if (td->touch) {
- input_event(input, EV_ABS, ABS_MT_POSITION_X, td->x);
- input_event(input, EV_ABS, ABS_MT_POSITION_Y, td->y);
- input_event(input, EV_ABS, ABS_MT_PRESSURE, td->z);
- }
- input_mt_report_pointer_emulation(input, true);
-}
-
-static int egalax_event(struct hid_device *hid, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
-{
- struct egalax_data *td = hid_get_drvdata(hid);
-
- /* Note, eGalax has two product lines: the first is resistive and
- * uses a standard parallel multitouch protocol (product ID ==
- * 48xx). The second is capacitive and uses an unusual "serial"
- * protocol with a different message for each multitouch finger
- * (product ID == 72xx).
- */
- 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_CONFIDENCE:
- /* avoid interference from generic hidinput handling */
- break;
- case HID_DG_TIPSWITCH:
- td->touch = value;
- break;
- case HID_DG_TIPPRESSURE:
- td->z = value;
- break;
- case HID_DG_CONTACTID:
- td->slot = clamp_val(value, 0, MAX_SLOTS - 1);
- break;
- case HID_GD_X:
- td->x = value;
- break;
- case HID_GD_Y:
- td->y = value;
- /* this is the last field in a finger */
- if (td->valid)
- egalax_filter_event(td, input);
- break;
- case HID_DG_CONTACTCOUNT:
- /* touch emulation: this is the last field in a frame */
- 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 egalax_probe(struct hid_device *hdev, const struct hid_device_id *id)
-{
- int ret;
- struct egalax_data *td;
- struct hid_report *report;
-
- td = kzalloc(sizeof(struct egalax_data), GFP_KERNEL);
- if (!td) {
- hid_err(hdev, "cannot allocate eGalax data\n");
- return -ENOMEM;
- }
- hid_set_drvdata(hdev, td);
-
- ret = hid_parse(hdev);
- if (ret)
- goto end;
-
- ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
- if (ret)
- goto end;
-
- report = hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[5];
- if (report) {
- report->field[0]->value[0] = 2;
- usbhid_submit_report(hdev, report, USB_DIR_OUT);
- }
-
-end:
- if (ret)
- kfree(td);
-
- return ret;
-}
-
-static void egalax_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 egalax_devices[] = {
- { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
- USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH) },
- { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
- USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH1) },
- { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
- USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH2) },
- { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
- USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH3) },
- { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
- USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH4) },
- { }
-};
-MODULE_DEVICE_TABLE(hid, egalax_devices);
-
-static const struct hid_usage_id egalax_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 egalax_driver = {
- .name = "egalax-touch",
- .id_table = egalax_devices,
- .probe = egalax_probe,
- .remove = egalax_remove,
- .input_mapping = egalax_input_mapping,
- .input_mapped = egalax_input_mapped,
- .usage_table = egalax_grabbed_usages,
- .event = egalax_event,
-};
-
-static int __init egalax_init(void)
-{
- return hid_register_driver(&egalax_driver);
-}
-
-static void __exit egalax_exit(void)
-{
- hid_unregister_driver(&egalax_driver);
-}
-
-module_init(egalax_init);
-module_exit(egalax_exit);
-
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 65e7f20..55a39ee 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -5,6 +5,12 @@
* Copyright (c) 2010-2011 Benjamin Tissoires <benjamin.tissoires@gmail.com>
* Copyright (c) 2010-2011 Ecole Nationale de l'Aviation Civile, France
*
+ * This code is partly based on hid-egalax.c:
+ *
+ * Copyright (c) 2010 Stephane Chatty <chatty@enac.fr>
+ * Copyright (c) 2010 Henrik Rydberg <rydberg@euromail.se>
+ * Copyright (c) 2010 Canonical, Ltd.
+ *
*/
/*
@@ -37,6 +43,7 @@ MODULE_LICENSE("GPL");
#define MT_QUIRK_SLOT_IS_CONTACTNUMBER (1 << 3)
#define MT_QUIRK_VALID_IS_INRANGE (1 << 4)
#define MT_QUIRK_VALID_IS_CONFIDENCE (1 << 5)
+#define MT_QUIRK_SEND_AT_EACH_REPORT (1 << 6)
struct mt_slot {
__s32 x, y, p, w, h;
@@ -62,7 +69,14 @@ struct mt_class {
__s32 quirks;
__s32 sn_move; /* Signal/noise ratio for move events */
__s32 sn_pressure; /* Signal/noise ratio for pressure events */
+ struct { /* correct the reported ranges for specified axis */
+ __s32 min_x;
+ __s32 max_x;
+ __s32 min_y;
+ __s32 max_y;
+ } axis;
__u8 maxcontacts;
+
};
/* classes of device behavior */
@@ -70,6 +84,8 @@ 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_EGALAX_CAPACITIVE 5
+#define MT_CLS_EGALAX_RESISTIVE 6
/*
* these device-dependent functions determine what slot corresponds
@@ -119,7 +135,33 @@ struct mt_class mt_classes[] = {
.quirks = MT_QUIRK_NOT_SEEN_MEANS_UP |
MT_QUIRK_CYPRESS,
.maxcontacts = 10 },
-
+ { .name = MT_CLS_EGALAX_CAPACITIVE,
+ .quirks = MT_QUIRK_SLOT_IS_CONTACTID |
+ MT_QUIRK_VALID_IS_INRANGE,
+ .maxcontacts = 2,
+ .sn_move = 4096,
+ .sn_pressure = 32,
+ .axis = {
+ .min_x = 0,
+ .max_x = 32760,
+ .min_y = 0,
+ .max_y = 32760
+ },
+ },
+ { .name = MT_CLS_EGALAX_RESISTIVE,
+ .quirks = MT_QUIRK_SLOT_IS_CONTACTID |
+ MT_QUIRK_VALID_IS_INRANGE |
+ MT_QUIRK_SEND_AT_EACH_REPORT,
+ .maxcontacts = 2,
+ .sn_move = 4096,
+ .sn_pressure = 32,
+ .axis = {
+ .min_x = 0,
+ .max_x = 32760,
+ .min_y = 0,
+ .max_y = 32760
+ },
+ },
{ }
};
@@ -152,6 +194,12 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
case HID_UP_GENDESK:
switch (usage->hid) {
case HID_GD_X:
+ /* fix up the reported X limits if necessary*/
+ if (cls->axis.min_x < cls->axis.max_x) {
+ field->logical_minimum = cls->axis.min_x;
+ field->logical_maximum = cls->axis.max_x;
+ }
+
hid_map_usage(hi, usage, bit, max,
EV_ABS, ABS_MT_POSITION_X);
set_abs(hi->input, ABS_MT_POSITION_X, field,
@@ -161,6 +209,11 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
td->last_slot_field = usage->hid;
return 1;
case HID_GD_Y:
+ /* fix up the reported Y limits if nessecary*/
+ if (cls->axis.min_y < cls->axis.max_y) {
+ field->logical_minimum = cls->axis.min_y;
+ field->logical_maximum = cls->axis.max_y;
+ }
hid_map_usage(hi, usage, bit, max,
EV_ABS, ABS_MT_POSITION_Y);
set_abs(hi->input, ABS_MT_POSITION_Y, field,
@@ -204,6 +257,10 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
td->last_slot_field = usage->hid;
return 1;
case HID_DG_TIPPRESSURE:
+ /* fix up the pressure range for some devices
+ with broken report */
+ field->logical_minimum = 0;
+
hid_map_usage(hi, usage, bit, max,
EV_ABS, ABS_MT_PRESSURE);
set_abs(hi->input, ABS_MT_PRESSURE, field,
@@ -367,8 +424,12 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
if (usage->hid == td->last_slot_field)
mt_complete_slot(td);
- if (field->index == td->last_field_index
+ if ((field->index == td->last_field_index
&& td->num_received >= td->num_expected)
+ || ((quirks & MT_QUIRK_SEND_AT_EACH_REPORT)
+ && (usage->hid == td->last_slot_field)))
+ /* emit an event for every finger to work around
+ a corrupt last_field_index in the hid report*/
mt_emit_event(td, field->hidinput->input);
}
@@ -484,6 +545,25 @@ static const struct hid_device_id mt_devices[] = {
HID_USB_DEVICE(USB_VENDOR_ID_CANDO,
USB_DEVICE_ID_CANDO_PIXCIR_MULTI_TOUCH) },
+ /* Resistive eGalax devices */
+ { .driver_data = MT_CLS_EGALAX_RESISTIVE,
+ HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
+ USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH) },
+ { .driver_data = MT_CLS_EGALAX_RESISTIVE,
+ HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
+ USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH3) },
+
+ /* Capacitive eGalax devices */
+ { .driver_data = MT_CLS_EGALAX_CAPACITIVE,
+ HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
+ USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH1) },
+ { .driver_data = MT_CLS_EGALAX_CAPACITIVE,
+ HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
+ USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH2) },
+ { .driver_data = MT_CLS_EGALAX_CAPACITIVE,
+ HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
+ USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH4) },
+
{ }
};
MODULE_DEVICE_TABLE(hid, mt_devices);
--
1.7.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v3] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework.
2011-03-08 23:38 ` [PATCH v3] " Richard Nauber
@ 2011-03-09 8:24 ` Henrik Rydberg
2011-03-09 8:54 ` Benjamin Tissoires
0 siblings, 1 reply; 21+ messages in thread
From: Henrik Rydberg @ 2011-03-09 8:24 UTC (permalink / raw)
To: Richard Nauber
Cc: Benjamin Tissoires, linux-input, Jiri Kosina,
Stéphane Chatty
Hi Richard,
> here is the third version of this patch, improved according to your suggestions.
>
> > What I meant is that (min_x < max_x) is the proper test.
> >
> > > No need for a quirk that will be redundant with the hand-provided values.
> >
> > Yes, but I think we might be better off using a quirk than a generic
> > way to replace the values provided by the hardware.
>
> My personal opinion is that the override should be triggered by your
> (min_x < max_x) test, which is safe, because the (min_x == max_x) case
> makes no sense for a device and we are allowed to attach a special
> meaning to it. Additionally this enables us to conveniently extend the
> mechanism to more axis, whose could be individually enabled without a
> bloat of quirk defines.
>
> So tell me what you think of this...
Having a mechanism to override device limits suggests this is
generally needed, which is not the case. Adding a specific quirk for
this specific hardware seems more appropriate. Also, it seems the
special behavior of the newer firmwares can be autodetected, so there
is no need for more than one class. I have added two patches below,
tested on a joojoo. Do you concur with the changes, and do they still
work for you?
Thanks,
Henrik
>From 67fd1e768341a8f67cd882b7603de62b5f250970 Mon Sep 17 00:00:00 2001
From: Henrik Rydberg <rydberg@euromail.se>
Date: Wed, 9 Mar 2011 06:35:25 +0100
Subject: [PATCH 1/2] HID: hid-multitouch: Send events per slot if CONTACTCOUNT is missing
The recent capacitive DWAV firmwares do not use the CONTACTCOUNT
field, and the touch frame boundary can therefore not be determined.
This patch makes the driver report the touch frame at each completed
slot instead.
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/hid/hid-multitouch.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 69f8744..4518006 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -364,8 +364,11 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
return 0;
}
- if (usage->hid == td->last_slot_field)
+ if (usage->hid == td->last_slot_field) {
mt_complete_slot(td);
+ if (!td->last_field_index)
+ mt_emit_event(td, field->hidinput->input);
+ }
if (field->index == td->last_field_index
&& td->num_received >= td->num_expected)
--
1.7.4.1
>From cd625e954a7f9774ae8f84a9eb2315c6da8b387c Mon Sep 17 00:00:00 2001
From: Richard Nauber <richard.nauber@googlemail.com>
Date: Wed, 9 Mar 2011 06:20:57 +0100
Subject: [PATCH 2/2] HID: merge hid-egalax into hid-multitouch
This patch merges the hid-egalax driver into hid-multitouch. There
are two types of devices support by the hid-egalax driver: resistive
and capacitive. Here, they are implicitly distinguished by the absence
of a HID_DG_CONTACTCOUNT field in the latter, so no special code path
needs to be introduced.
As a side effect, this patch fixes the broken suspend/resume behavior
in the old driver.
[rydberg@euromail.se: minor fixups]
Not-yet-Signed-off-by: Richard Nauber <Richard.Nauber@gmail.com>
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
drivers/hid/Kconfig | 9 +-
drivers/hid/Makefile | 1 -
drivers/hid/hid-egalax.c | 279 ------------------------------------------
drivers/hid/hid-multitouch.c | 43 +++++++
4 files changed, 45 insertions(+), 287 deletions(-)
delete mode 100644 drivers/hid/hid-egalax.c
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index a0b117d..b4b8b21 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -160,13 +160,6 @@ config HID_EMS_FF
Currently the following devices are known to be supported:
- Trio Linker Plus II
-config HID_EGALAX
- tristate "eGalax multi-touch panel"
- depends on USB_HID
- ---help---
- Support for the eGalax dual-touch panels, including the
- Joojoo and Wetab tablets.
-
config HID_ELECOM
tristate "ELECOM BM084 bluetooth mouse"
depends on BT_HIDP
@@ -307,6 +300,8 @@ config HID_MULTITOUCH
- IrTouch Infrared USB panels
- Pixcir dual touch panels
- 'Sensing Win7-TwoFinger' panel by GeneralTouch
+ - eGalax dual-touch panels, including the
+ Joojoo and Wetab tablets
If unsure, say N.
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 6efc2a0..29e9898 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -36,7 +36,6 @@ obj-$(CONFIG_HID_CHICONY) += hid-chicony.o
obj-$(CONFIG_HID_CYPRESS) += hid-cypress.o
obj-$(CONFIG_HID_DRAGONRISE) += hid-drff.o
obj-$(CONFIG_HID_EMS_FF) += hid-emsff.o
-obj-$(CONFIG_HID_EGALAX) += hid-egalax.o
obj-$(CONFIG_HID_ELECOM) += hid-elecom.o
obj-$(CONFIG_HID_EZKEY) += hid-ezkey.o
obj-$(CONFIG_HID_GYRATION) += hid-gyration.o
diff --git a/drivers/hid/hid-egalax.c b/drivers/hid/hid-egalax.c
deleted file mode 100644
index 03bee19..0000000
--- a/drivers/hid/hid-egalax.c
+++ /dev/null
@@ -1,279 +0,0 @@
-/*
- * HID driver for eGalax dual-touch panels
- *
- * Copyright (c) 2010 Stephane Chatty <chatty@enac.fr>
- * Copyright (c) 2010 Henrik Rydberg <rydberg@euromail.se>
- * 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/usb.h>
-#include <linux/input/mt.h>
-#include <linux/slab.h>
-#include "usbhid/usbhid.h"
-
-MODULE_AUTHOR("Stephane Chatty <chatty@enac.fr>");
-MODULE_DESCRIPTION("eGalax dual-touch panel");
-MODULE_LICENSE("GPL");
-
-#include "hid-ids.h"
-
-#define MAX_SLOTS 2
-
-/* estimated signal-to-noise ratios */
-#define SN_MOVE 4096
-#define SN_PRESSURE 32
-
-struct egalax_data {
- int valid;
- int slot;
- int touch;
- int x, y, z;
-};
-
-static void set_abs(struct input_dev *input, unsigned int code,
- struct hid_field *field, int snratio)
-{
- int fmin = field->logical_minimum;
- int fmax = field->logical_maximum;
- int fuzz = snratio ? (fmax - fmin) / snratio : 0;
- input_set_abs_params(input, code, fmin, fmax, fuzz, 0);
-}
-
-static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
- struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
-{
- struct input_dev *input = hi->input;
-
- switch (usage->hid & HID_USAGE_PAGE) {
-
- case HID_UP_GENDESK:
- switch (usage->hid) {
- case HID_GD_X:
- field->logical_maximum = 32760;
- hid_map_usage(hi, usage, bit, max,
- EV_ABS, ABS_MT_POSITION_X);
- set_abs(input, ABS_MT_POSITION_X, field, SN_MOVE);
- /* touchscreen emulation */
- set_abs(input, ABS_X, field, SN_MOVE);
- return 1;
- case HID_GD_Y:
- field->logical_maximum = 32760;
- hid_map_usage(hi, usage, bit, max,
- EV_ABS, ABS_MT_POSITION_Y);
- set_abs(input, ABS_MT_POSITION_Y, field, SN_MOVE);
- /* touchscreen emulation */
- set_abs(input, ABS_Y, field, SN_MOVE);
- return 1;
- }
- return 0;
-
- case HID_UP_DIGITIZER:
- switch (usage->hid) {
- case HID_DG_TIPSWITCH:
- /* touchscreen emulation */
- hid_map_usage(hi, usage, bit, max, EV_KEY, BTN_TOUCH);
- input_set_capability(input, EV_KEY, BTN_TOUCH);
- return 1;
- case HID_DG_INRANGE:
- case HID_DG_CONFIDENCE:
- case HID_DG_CONTACTCOUNT:
- case HID_DG_CONTACTMAX:
- return -1;
- case HID_DG_CONTACTID:
- input_mt_init_slots(input, MAX_SLOTS);
- return 1;
- case HID_DG_TIPPRESSURE:
- field->logical_minimum = 0;
- hid_map_usage(hi, usage, bit, max,
- EV_ABS, ABS_MT_PRESSURE);
- set_abs(input, ABS_MT_PRESSURE, field, SN_PRESSURE);
- /* touchscreen emulation */
- set_abs(input, ABS_PRESSURE, field, SN_PRESSURE);
- return 1;
- }
- return 0;
- }
-
- /* ignore others (from other reports we won't get anyway) */
- return -1;
-}
-
-static int egalax_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 finger has been parsed,
- * so that it can decide what to send to the input layer.
- */
-static void egalax_filter_event(struct egalax_data *td, struct input_dev *input)
-{
- input_mt_slot(input, td->slot);
- input_mt_report_slot_state(input, MT_TOOL_FINGER, td->touch);
- if (td->touch) {
- input_event(input, EV_ABS, ABS_MT_POSITION_X, td->x);
- input_event(input, EV_ABS, ABS_MT_POSITION_Y, td->y);
- input_event(input, EV_ABS, ABS_MT_PRESSURE, td->z);
- }
- input_mt_report_pointer_emulation(input, true);
-}
-
-static int egalax_event(struct hid_device *hid, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
-{
- struct egalax_data *td = hid_get_drvdata(hid);
-
- /* Note, eGalax has two product lines: the first is resistive and
- * uses a standard parallel multitouch protocol (product ID ==
- * 48xx). The second is capacitive and uses an unusual "serial"
- * protocol with a different message for each multitouch finger
- * (product ID == 72xx).
- */
- 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_CONFIDENCE:
- /* avoid interference from generic hidinput handling */
- break;
- case HID_DG_TIPSWITCH:
- td->touch = value;
- break;
- case HID_DG_TIPPRESSURE:
- td->z = value;
- break;
- case HID_DG_CONTACTID:
- td->slot = clamp_val(value, 0, MAX_SLOTS - 1);
- break;
- case HID_GD_X:
- td->x = value;
- break;
- case HID_GD_Y:
- td->y = value;
- /* this is the last field in a finger */
- if (td->valid)
- egalax_filter_event(td, input);
- break;
- case HID_DG_CONTACTCOUNT:
- /* touch emulation: this is the last field in a frame */
- 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 egalax_probe(struct hid_device *hdev, const struct hid_device_id *id)
-{
- int ret;
- struct egalax_data *td;
- struct hid_report *report;
-
- td = kzalloc(sizeof(struct egalax_data), GFP_KERNEL);
- if (!td) {
- hid_err(hdev, "cannot allocate eGalax data\n");
- return -ENOMEM;
- }
- hid_set_drvdata(hdev, td);
-
- ret = hid_parse(hdev);
- if (ret)
- goto end;
-
- ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
- if (ret)
- goto end;
-
- report = hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[5];
- if (report) {
- report->field[0]->value[0] = 2;
- usbhid_submit_report(hdev, report, USB_DIR_OUT);
- }
-
-end:
- if (ret)
- kfree(td);
-
- return ret;
-}
-
-static void egalax_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 egalax_devices[] = {
- { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
- USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH) },
- { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
- USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH1) },
- { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
- USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH2) },
- { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
- USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH3) },
- { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
- USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH4) },
- { }
-};
-MODULE_DEVICE_TABLE(hid, egalax_devices);
-
-static const struct hid_usage_id egalax_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 egalax_driver = {
- .name = "egalax-touch",
- .id_table = egalax_devices,
- .probe = egalax_probe,
- .remove = egalax_remove,
- .input_mapping = egalax_input_mapping,
- .input_mapped = egalax_input_mapped,
- .usage_table = egalax_grabbed_usages,
- .event = egalax_event,
-};
-
-static int __init egalax_init(void)
-{
- return hid_register_driver(&egalax_driver);
-}
-
-static void __exit egalax_exit(void)
-{
- hid_unregister_driver(&egalax_driver);
-}
-
-module_init(egalax_init);
-module_exit(egalax_exit);
-
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 4518006..c2e79dc 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -5,6 +5,12 @@
* Copyright (c) 2010-2011 Benjamin Tissoires <benjamin.tissoires@gmail.com>
* Copyright (c) 2010-2011 Ecole Nationale de l'Aviation Civile, France
*
+ * This code is partly based on hid-egalax.c:
+ *
+ * Copyright (c) 2010 Stephane Chatty <chatty@enac.fr>
+ * Copyright (c) 2010 Henrik Rydberg <rydberg@euromail.se>
+ * Copyright (c) 2010 Canonical, Ltd.
+ *
*/
/*
@@ -37,6 +43,7 @@ MODULE_LICENSE("GPL");
#define MT_QUIRK_SLOT_IS_CONTACTNUMBER (1 << 3)
#define MT_QUIRK_VALID_IS_INRANGE (1 << 4)
#define MT_QUIRK_VALID_IS_CONFIDENCE (1 << 5)
+#define MT_QUIRK_EGALAX_XYZ_FIXUP (1 << 6)
struct mt_slot {
__s32 x, y, p, w, h;
@@ -70,6 +77,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_EGALAX 5
/*
* these device-dependent functions determine what slot corresponds
@@ -120,6 +128,14 @@ struct mt_class mt_classes[] = {
MT_QUIRK_CYPRESS,
.maxcontacts = 10 },
+ { .name = MT_CLS_EGALAX,
+ .quirks = MT_QUIRK_SLOT_IS_CONTACTID |
+ MT_QUIRK_VALID_IS_INRANGE |
+ MT_QUIRK_EGALAX_XYZ_FIXUP
+ .maxcontacts = 2,
+ .sn_move = 4096,
+ .sn_pressure = 32,
+ },
{ }
};
@@ -147,11 +163,15 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
{
struct mt_device *td = hid_get_drvdata(hdev);
struct mt_class *cls = td->mtclass;
+ __s32 quirks = cls->quirks;
+
switch (usage->hid & HID_USAGE_PAGE) {
case HID_UP_GENDESK:
switch (usage->hid) {
case HID_GD_X:
+ if (quirks & MT_QUIRK_EGALAX_XYZ_FIXUP)
+ field->logical_maximum = 32760;
hid_map_usage(hi, usage, bit, max,
EV_ABS, ABS_MT_POSITION_X);
set_abs(hi->input, ABS_MT_POSITION_X, field,
@@ -161,6 +181,8 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
td->last_slot_field = usage->hid;
return 1;
case HID_GD_Y:
+ if (quirks & MT_QUIRK_EGALAX_XYZ_FIXUP)
+ field->logical_maximum = 32760;
hid_map_usage(hi, usage, bit, max,
EV_ABS, ABS_MT_POSITION_Y);
set_abs(hi->input, ABS_MT_POSITION_Y, field,
@@ -204,6 +226,8 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
td->last_slot_field = usage->hid;
return 1;
case HID_DG_TIPPRESSURE:
+ if (quirks & MT_QUIRK_EGALAX_XYZ_FIXUP)
+ field->logical_minimum = 0;
hid_map_usage(hi, usage, bit, max,
EV_ABS, ABS_MT_PRESSURE);
set_abs(hi->input, ABS_MT_PRESSURE, field,
@@ -487,6 +511,25 @@ static const struct hid_device_id mt_devices[] = {
HID_USB_DEVICE(USB_VENDOR_ID_CANDO,
USB_DEVICE_ID_CANDO_PIXCIR_MULTI_TOUCH) },
+ /* Resistive eGalax devices */
+ { .driver_data = MT_CLS_EGALAX,
+ HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
+ USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH) },
+ { .driver_data = MT_CLS_EGALAX,
+ HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
+ USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH3) },
+
+ /* Capacitive eGalax devices */
+ { .driver_data = MT_CLS_EGALAX,
+ HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
+ USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH1) },
+ { .driver_data = MT_CLS_EGALAX,
+ HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
+ USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH2) },
+ { .driver_data = MT_CLS_EGALAX,
+ HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
+ USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH4) },
+
{ }
};
MODULE_DEVICE_TABLE(hid, mt_devices);
--
1.7.4.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v3] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework.
2011-03-09 8:24 ` Henrik Rydberg
@ 2011-03-09 8:54 ` Benjamin Tissoires
2011-03-09 9:23 ` Henrik Rydberg
0 siblings, 1 reply; 21+ messages in thread
From: Benjamin Tissoires @ 2011-03-09 8:54 UTC (permalink / raw)
To: Henrik Rydberg
Cc: Richard Nauber, linux-input, Jiri Kosina, Stéphane Chatty
Hi Henrik,
On Wed, Mar 9, 2011 at 09:24, Henrik Rydberg <rydberg@euromail.se> wrote:
> Hi Richard,
>
>> here is the third version of this patch, improved according to your suggestions.
>>
>> > What I meant is that (min_x < max_x) is the proper test.
>> >
>> > > No need for a quirk that will be redundant with the hand-provided values.
>> >
>> > Yes, but I think we might be better off using a quirk than a generic
>> > way to replace the values provided by the hardware.
>>
>> My personal opinion is that the override should be triggered by your
>> (min_x < max_x) test, which is safe, because the (min_x == max_x) case
>> makes no sense for a device and we are allowed to attach a special
>> meaning to it. Additionally this enables us to conveniently extend the
>> mechanism to more axis, whose could be individually enabled without a
>> bloat of quirk defines.
>>
>> So tell me what you think of this...
>
> Having a mechanism to override device limits suggests this is
> generally needed, which is not the case. Adding a specific quirk for
> this specific hardware seems more appropriate. Also, it seems the
> special behavior of the newer firmwares can be autodetected, so there
> is no need for more than one class. I have added two patches below,
> tested on a joojoo. Do you concur with the changes, and do they still
> work for you?
>
I really like your approach:
for the newer firmwares, it can be autodetected -> no quirk
for the fixes in ranges, as it is autodetected in the previous patch
by the values given by the programmer -> insert a quirk.
You really know what you want! ;)
> From 67fd1e768341a8f67cd882b7603de62b5f250970 Mon Sep 17 00:00:00 2001
> From: Henrik Rydberg <rydberg@euromail.se>
> Date: Wed, 9 Mar 2011 06:35:25 +0100
> Subject: [PATCH 1/2] HID: hid-multitouch: Send events per slot if CONTACTCOUNT is missing
>
> The recent capacitive DWAV firmwares do not use the CONTACTCOUNT
> field, and the touch frame boundary can therefore not be determined.
> This patch makes the driver report the touch frame at each completed
> slot instead.
>
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> ---
> drivers/hid/hid-multitouch.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 69f8744..4518006 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -364,8 +364,11 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
> return 0;
> }
>
> - if (usage->hid == td->last_slot_field)
> + if (usage->hid == td->last_slot_field) {
> mt_complete_slot(td);
> + if (!td->last_field_index)
> + mt_emit_event(td, field->hidinput->input);
> + }
Fine with me
>
> if (field->index == td->last_field_index
> && td->num_received >= td->num_expected)
> --
> 1.7.4.1
>
> From cd625e954a7f9774ae8f84a9eb2315c6da8b387c Mon Sep 17 00:00:00 2001
> From: Richard Nauber <richard.nauber@googlemail.com>
> Date: Wed, 9 Mar 2011 06:20:57 +0100
> Subject: [PATCH 2/2] HID: merge hid-egalax into hid-multitouch
>
> This patch merges the hid-egalax driver into hid-multitouch. There
> are two types of devices support by the hid-egalax driver: resistive
> and capacitive. Here, they are implicitly distinguished by the absence
> of a HID_DG_CONTACTCOUNT field in the latter, so no special code path
> needs to be introduced.
>
> As a side effect, this patch fixes the broken suspend/resume behavior
> in the old driver.
>
> [rydberg@euromail.se: minor fixups]
> Not-yet-Signed-off-by: Richard Nauber <Richard.Nauber@gmail.com>
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> ---
> drivers/hid/Kconfig | 9 +-
> drivers/hid/Makefile | 1 -
> drivers/hid/hid-egalax.c | 279 ------------------------------------------
> drivers/hid/hid-multitouch.c | 43 +++++++
> 4 files changed, 45 insertions(+), 287 deletions(-)
> delete mode 100644 drivers/hid/hid-egalax.c
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index a0b117d..b4b8b21 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -160,13 +160,6 @@ config HID_EMS_FF
> Currently the following devices are known to be supported:
> - Trio Linker Plus II
>
> -config HID_EGALAX
> - tristate "eGalax multi-touch panel"
> - depends on USB_HID
> - ---help---
> - Support for the eGalax dual-touch panels, including the
> - Joojoo and Wetab tablets.
> -
> config HID_ELECOM
> tristate "ELECOM BM084 bluetooth mouse"
> depends on BT_HIDP
> @@ -307,6 +300,8 @@ config HID_MULTITOUCH
> - IrTouch Infrared USB panels
> - Pixcir dual touch panels
> - 'Sensing Win7-TwoFinger' panel by GeneralTouch
> + - eGalax dual-touch panels, including the
> + Joojoo and Wetab tablets
>
> If unsure, say N.
>
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index 6efc2a0..29e9898 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -36,7 +36,6 @@ obj-$(CONFIG_HID_CHICONY) += hid-chicony.o
> obj-$(CONFIG_HID_CYPRESS) += hid-cypress.o
> obj-$(CONFIG_HID_DRAGONRISE) += hid-drff.o
> obj-$(CONFIG_HID_EMS_FF) += hid-emsff.o
> -obj-$(CONFIG_HID_EGALAX) += hid-egalax.o
> obj-$(CONFIG_HID_ELECOM) += hid-elecom.o
> obj-$(CONFIG_HID_EZKEY) += hid-ezkey.o
> obj-$(CONFIG_HID_GYRATION) += hid-gyration.o
> diff --git a/drivers/hid/hid-egalax.c b/drivers/hid/hid-egalax.c
> deleted file mode 100644
> index 03bee19..0000000
> --- a/drivers/hid/hid-egalax.c
> +++ /dev/null
> @@ -1,279 +0,0 @@
> -/*
> - * HID driver for eGalax dual-touch panels
> - *
> - * Copyright (c) 2010 Stephane Chatty <chatty@enac.fr>
> - * Copyright (c) 2010 Henrik Rydberg <rydberg@euromail.se>
> - * 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/usb.h>
> -#include <linux/input/mt.h>
> -#include <linux/slab.h>
> -#include "usbhid/usbhid.h"
> -
> -MODULE_AUTHOR("Stephane Chatty <chatty@enac.fr>");
> -MODULE_DESCRIPTION("eGalax dual-touch panel");
> -MODULE_LICENSE("GPL");
> -
> -#include "hid-ids.h"
> -
> -#define MAX_SLOTS 2
> -
> -/* estimated signal-to-noise ratios */
> -#define SN_MOVE 4096
> -#define SN_PRESSURE 32
> -
> -struct egalax_data {
> - int valid;
> - int slot;
> - int touch;
> - int x, y, z;
> -};
> -
> -static void set_abs(struct input_dev *input, unsigned int code,
> - struct hid_field *field, int snratio)
> -{
> - int fmin = field->logical_minimum;
> - int fmax = field->logical_maximum;
> - int fuzz = snratio ? (fmax - fmin) / snratio : 0;
> - input_set_abs_params(input, code, fmin, fmax, fuzz, 0);
> -}
> -
> -static int egalax_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> - struct hid_field *field, struct hid_usage *usage,
> - unsigned long **bit, int *max)
> -{
> - struct input_dev *input = hi->input;
> -
> - switch (usage->hid & HID_USAGE_PAGE) {
> -
> - case HID_UP_GENDESK:
> - switch (usage->hid) {
> - case HID_GD_X:
> - field->logical_maximum = 32760;
> - hid_map_usage(hi, usage, bit, max,
> - EV_ABS, ABS_MT_POSITION_X);
> - set_abs(input, ABS_MT_POSITION_X, field, SN_MOVE);
> - /* touchscreen emulation */
> - set_abs(input, ABS_X, field, SN_MOVE);
> - return 1;
> - case HID_GD_Y:
> - field->logical_maximum = 32760;
> - hid_map_usage(hi, usage, bit, max,
> - EV_ABS, ABS_MT_POSITION_Y);
> - set_abs(input, ABS_MT_POSITION_Y, field, SN_MOVE);
> - /* touchscreen emulation */
> - set_abs(input, ABS_Y, field, SN_MOVE);
> - return 1;
> - }
> - return 0;
> -
> - case HID_UP_DIGITIZER:
> - switch (usage->hid) {
> - case HID_DG_TIPSWITCH:
> - /* touchscreen emulation */
> - hid_map_usage(hi, usage, bit, max, EV_KEY, BTN_TOUCH);
> - input_set_capability(input, EV_KEY, BTN_TOUCH);
> - return 1;
> - case HID_DG_INRANGE:
> - case HID_DG_CONFIDENCE:
> - case HID_DG_CONTACTCOUNT:
> - case HID_DG_CONTACTMAX:
> - return -1;
> - case HID_DG_CONTACTID:
> - input_mt_init_slots(input, MAX_SLOTS);
> - return 1;
> - case HID_DG_TIPPRESSURE:
> - field->logical_minimum = 0;
> - hid_map_usage(hi, usage, bit, max,
> - EV_ABS, ABS_MT_PRESSURE);
> - set_abs(input, ABS_MT_PRESSURE, field, SN_PRESSURE);
> - /* touchscreen emulation */
> - set_abs(input, ABS_PRESSURE, field, SN_PRESSURE);
> - return 1;
> - }
> - return 0;
> - }
> -
> - /* ignore others (from other reports we won't get anyway) */
> - return -1;
> -}
> -
> -static int egalax_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 finger has been parsed,
> - * so that it can decide what to send to the input layer.
> - */
> -static void egalax_filter_event(struct egalax_data *td, struct input_dev *input)
> -{
> - input_mt_slot(input, td->slot);
> - input_mt_report_slot_state(input, MT_TOOL_FINGER, td->touch);
> - if (td->touch) {
> - input_event(input, EV_ABS, ABS_MT_POSITION_X, td->x);
> - input_event(input, EV_ABS, ABS_MT_POSITION_Y, td->y);
> - input_event(input, EV_ABS, ABS_MT_PRESSURE, td->z);
> - }
> - input_mt_report_pointer_emulation(input, true);
> -}
> -
> -static int egalax_event(struct hid_device *hid, struct hid_field *field,
> - struct hid_usage *usage, __s32 value)
> -{
> - struct egalax_data *td = hid_get_drvdata(hid);
> -
> - /* Note, eGalax has two product lines: the first is resistive and
> - * uses a standard parallel multitouch protocol (product ID ==
> - * 48xx). The second is capacitive and uses an unusual "serial"
> - * protocol with a different message for each multitouch finger
> - * (product ID == 72xx).
> - */
> - 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_CONFIDENCE:
> - /* avoid interference from generic hidinput handling */
> - break;
> - case HID_DG_TIPSWITCH:
> - td->touch = value;
> - break;
> - case HID_DG_TIPPRESSURE:
> - td->z = value;
> - break;
> - case HID_DG_CONTACTID:
> - td->slot = clamp_val(value, 0, MAX_SLOTS - 1);
> - break;
> - case HID_GD_X:
> - td->x = value;
> - break;
> - case HID_GD_Y:
> - td->y = value;
> - /* this is the last field in a finger */
> - if (td->valid)
> - egalax_filter_event(td, input);
> - break;
> - case HID_DG_CONTACTCOUNT:
> - /* touch emulation: this is the last field in a frame */
> - 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 egalax_probe(struct hid_device *hdev, const struct hid_device_id *id)
> -{
> - int ret;
> - struct egalax_data *td;
> - struct hid_report *report;
> -
> - td = kzalloc(sizeof(struct egalax_data), GFP_KERNEL);
> - if (!td) {
> - hid_err(hdev, "cannot allocate eGalax data\n");
> - return -ENOMEM;
> - }
> - hid_set_drvdata(hdev, td);
> -
> - ret = hid_parse(hdev);
> - if (ret)
> - goto end;
> -
> - ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> - if (ret)
> - goto end;
> -
> - report = hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[5];
> - if (report) {
> - report->field[0]->value[0] = 2;
> - usbhid_submit_report(hdev, report, USB_DIR_OUT);
> - }
> -
> -end:
> - if (ret)
> - kfree(td);
> -
> - return ret;
> -}
> -
> -static void egalax_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 egalax_devices[] = {
> - { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> - USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH) },
> - { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> - USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH1) },
> - { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> - USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH2) },
> - { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> - USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH3) },
> - { HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> - USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH4) },
> - { }
> -};
> -MODULE_DEVICE_TABLE(hid, egalax_devices);
> -
> -static const struct hid_usage_id egalax_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 egalax_driver = {
> - .name = "egalax-touch",
> - .id_table = egalax_devices,
> - .probe = egalax_probe,
> - .remove = egalax_remove,
> - .input_mapping = egalax_input_mapping,
> - .input_mapped = egalax_input_mapped,
> - .usage_table = egalax_grabbed_usages,
> - .event = egalax_event,
> -};
> -
> -static int __init egalax_init(void)
> -{
> - return hid_register_driver(&egalax_driver);
> -}
> -
> -static void __exit egalax_exit(void)
> -{
> - hid_unregister_driver(&egalax_driver);
> -}
> -
> -module_init(egalax_init);
> -module_exit(egalax_exit);
> -
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 4518006..c2e79dc 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -5,6 +5,12 @@
> * Copyright (c) 2010-2011 Benjamin Tissoires <benjamin.tissoires@gmail.com>
> * Copyright (c) 2010-2011 Ecole Nationale de l'Aviation Civile, France
> *
> + * This code is partly based on hid-egalax.c:
> + *
> + * Copyright (c) 2010 Stephane Chatty <chatty@enac.fr>
> + * Copyright (c) 2010 Henrik Rydberg <rydberg@euromail.se>
> + * Copyright (c) 2010 Canonical, Ltd.
> + *
> */
>
> /*
> @@ -37,6 +43,7 @@ MODULE_LICENSE("GPL");
> #define MT_QUIRK_SLOT_IS_CONTACTNUMBER (1 << 3)
> #define MT_QUIRK_VALID_IS_INRANGE (1 << 4)
> #define MT_QUIRK_VALID_IS_CONFIDENCE (1 << 5)
> +#define MT_QUIRK_EGALAX_XYZ_FIXUP (1 << 6)
>
> struct mt_slot {
> __s32 x, y, p, w, h;
> @@ -70,6 +77,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_EGALAX 5
>
> /*
> * these device-dependent functions determine what slot corresponds
> @@ -120,6 +128,14 @@ struct mt_class mt_classes[] = {
> MT_QUIRK_CYPRESS,
> .maxcontacts = 10 },
>
> + { .name = MT_CLS_EGALAX,
> + .quirks = MT_QUIRK_SLOT_IS_CONTACTID |
> + MT_QUIRK_VALID_IS_INRANGE |
> + MT_QUIRK_EGALAX_XYZ_FIXUP
> + .maxcontacts = 2,
> + .sn_move = 4096,
> + .sn_pressure = 32,
> + },
> { }
> };
>
> @@ -147,11 +163,15 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> {
> struct mt_device *td = hid_get_drvdata(hdev);
> struct mt_class *cls = td->mtclass;
> + __s32 quirks = cls->quirks;
> +
> switch (usage->hid & HID_USAGE_PAGE) {
>
> case HID_UP_GENDESK:
> switch (usage->hid) {
> case HID_GD_X:
> + if (quirks & MT_QUIRK_EGALAX_XYZ_FIXUP)
> + field->logical_maximum = 32760;
Please do not add numerical constants in a generic code. If I want to
override the values for another device, I'll have to rename your quirk
to MT_QUIRK_EGALAX_XYZ_FIXUP_WITH_0_32760_0_32760_0_x and add mine.
These values have to be moved in the MT_CLS.
> hid_map_usage(hi, usage, bit, max,
> EV_ABS, ABS_MT_POSITION_X);
> set_abs(hi->input, ABS_MT_POSITION_X, field,
> @@ -161,6 +181,8 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> td->last_slot_field = usage->hid;
> return 1;
> case HID_GD_Y:
> + if (quirks & MT_QUIRK_EGALAX_XYZ_FIXUP)
> + field->logical_maximum = 32760;
ditto
> hid_map_usage(hi, usage, bit, max,
> EV_ABS, ABS_MT_POSITION_Y);
> set_abs(hi->input, ABS_MT_POSITION_Y, field,
> @@ -204,6 +226,8 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> td->last_slot_field = usage->hid;
> return 1;
> case HID_DG_TIPPRESSURE:
> + if (quirks & MT_QUIRK_EGALAX_XYZ_FIXUP)
> + field->logical_minimum = 0;
No really need for the test and the quirk. In addition, here you do
not override logical_max. This shows that the quirk is not
MT_QUIRK_EGALAX_XYZ_FIXUP but the combination of
MT_QUIRK_EGALAX_XY_FIXUP and MT_QUIRK_EGALAX_Z_MIN_FIXUP.
Just drop it here.
> hid_map_usage(hi, usage, bit, max,
> EV_ABS, ABS_MT_PRESSURE);
> set_abs(hi->input, ABS_MT_PRESSURE, field,
> @@ -487,6 +511,25 @@ static const struct hid_device_id mt_devices[] = {
> HID_USB_DEVICE(USB_VENDOR_ID_CANDO,
> USB_DEVICE_ID_CANDO_PIXCIR_MULTI_TOUCH) },
>
> + /* Resistive eGalax devices */
> + { .driver_data = MT_CLS_EGALAX,
> + HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> + USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH) },
> + { .driver_data = MT_CLS_EGALAX,
> + HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> + USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH3) },
> +
> + /* Capacitive eGalax devices */
> + { .driver_data = MT_CLS_EGALAX,
> + HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> + USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH1) },
> + { .driver_data = MT_CLS_EGALAX,
> + HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> + USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH2) },
> + { .driver_data = MT_CLS_EGALAX,
> + HID_USB_DEVICE(USB_VENDOR_ID_DWAV,
> + USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH4) },
> +
> { }
> };
> MODULE_DEVICE_TABLE(hid, mt_devices);
> --
> 1.7.4.1
>
Thanks,
Benjamin
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework.
2011-03-09 8:54 ` Benjamin Tissoires
@ 2011-03-09 9:23 ` Henrik Rydberg
2011-03-09 10:06 ` Benjamin Tissoires
0 siblings, 1 reply; 21+ messages in thread
From: Henrik Rydberg @ 2011-03-09 9:23 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Richard Nauber, linux-input, Jiri Kosina, Stéphane Chatty
> > @@ -147,11 +163,15 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> > {
> > struct mt_device *td = hid_get_drvdata(hdev);
> > struct mt_class *cls = td->mtclass;
> > + __s32 quirks = cls->quirks;
> > +
> > switch (usage->hid & HID_USAGE_PAGE) {
> >
> > case HID_UP_GENDESK:
> > switch (usage->hid) {
> > case HID_GD_X:
> > + if (quirks & MT_QUIRK_EGALAX_XYZ_FIXUP)
> > + field->logical_maximum = 32760;
>
> Please do not add numerical constants in a generic code. If I want to
> override the values for another device, I'll have to rename your quirk
> to MT_QUIRK_EGALAX_XYZ_FIXUP_WITH_0_32760_0_32760_0_x and add mine.
> These values have to be moved in the MT_CLS.
It is a specific value used for a specific code path, no need to
over-generalize. The quirk name could be specialized even more, i
suppose, but the point is that it makes the hid-multitouch driver do
exactly what is done in the hid-egalax driver, and it is easy to prove
that there are no side effects for other devices.
> > case HID_DG_TIPPRESSURE:
> > + if (quirks & MT_QUIRK_EGALAX_XYZ_FIXUP)
> > + field->logical_minimum = 0;
>
> No really need for the test and the quirk. In addition, here you do
> not override logical_max. This shows that the quirk is not
> MT_QUIRK_EGALAX_XYZ_FIXUP but the combination of
> MT_QUIRK_EGALAX_XY_FIXUP and MT_QUIRK_EGALAX_Z_MIN_FIXUP.
> Just drop it here.
The added code path simply reflects what is in the hid-egalax
driver. It is true that the statement _probably_ works for other
devices as well, but it cannot be proven just by looking at the
code. Also, we have no known additional case where it is needed.
I am trying to think of a better name for the quirk, but frankly, I
think it already says what it needs to say.
Thanks,
Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework.
2011-03-09 9:23 ` Henrik Rydberg
@ 2011-03-09 10:06 ` Benjamin Tissoires
2011-03-09 11:40 ` Henrik Rydberg
0 siblings, 1 reply; 21+ messages in thread
From: Benjamin Tissoires @ 2011-03-09 10:06 UTC (permalink / raw)
To: Henrik Rydberg
Cc: Richard Nauber, linux-input, Jiri Kosina, Stéphane Chatty
On Wed, Mar 9, 2011 at 10:23, Henrik Rydberg <rydberg@euromail.se> wrote:
>> > @@ -147,11 +163,15 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>> > {
>> > struct mt_device *td = hid_get_drvdata(hdev);
>> > struct mt_class *cls = td->mtclass;
>> > + __s32 quirks = cls->quirks;
>> > +
>> > switch (usage->hid & HID_USAGE_PAGE) {
>> >
>> > case HID_UP_GENDESK:
>> > switch (usage->hid) {
>> > case HID_GD_X:
>> > + if (quirks & MT_QUIRK_EGALAX_XYZ_FIXUP)
>> > + field->logical_maximum = 32760;
>>
>> Please do not add numerical constants in a generic code. If I want to
>> override the values for another device, I'll have to rename your quirk
>> to MT_QUIRK_EGALAX_XYZ_FIXUP_WITH_0_32760_0_32760_0_x and add mine.
>> These values have to be moved in the MT_CLS.
>
> It is a specific value used for a specific code path, no need to
> over-generalize. The quirk name could be specialized even more, i
> suppose, but the point is that it makes the hid-multitouch driver do
> exactly what is done in the hid-egalax driver, and it is easy to prove
> that there are no side effects for other devices.
Or it's a generic code path for generic devices: if someone want to do
the same thing that have been done in hid-egalax (correct the logical
min/max reported by the device), this could be done.
Concerning the prove, I'm pretty sure we can manage to prove that both
solutions are equivalent as the current code already relies on the
fact that static struct are null initialized.
>
>> > case HID_DG_TIPPRESSURE:
>> > + if (quirks & MT_QUIRK_EGALAX_XYZ_FIXUP)
>> > + field->logical_minimum = 0;
>>
>> No really need for the test and the quirk. In addition, here you do
>> not override logical_max. This shows that the quirk is not
>> MT_QUIRK_EGALAX_XYZ_FIXUP but the combination of
>> MT_QUIRK_EGALAX_XY_FIXUP and MT_QUIRK_EGALAX_Z_MIN_FIXUP.
>> Just drop it here.
>
> The added code path simply reflects what is in the hid-egalax
> driver. It is true that the statement _probably_ works for other
> devices as well, but it cannot be proven just by looking at the
> code. Also, we have no known additional case where it is needed.
I didn't told to remove the test just by looking at the code. I'm
taking in account all the devices that have been reported to us:
http://lii-enac.fr/en/architecture/linux-input/multitouch-devices.html
Among all of them, only asus, egalax (capacitive and resistive),
mosart (the same as asus), and stantum reports pressure.
Asus and mosart set only the logical maximum -> logical minimum is
therefore at 0.
Stantum does not set the min/max -> logical minimum is at 0 too.
EGalax set the minimum at 1 -> in your patch, you explained that was a
bug for the upper layers.
>
> I am trying to think of a better name for the quirk, but frankly, I
> think it already says what it needs to say.
What about MT_QUIRK_XYZ_CORRECTIONS_FOR_SPECIAL_EGALAX_DEVICES?
Cheers,
Benjamin
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework.
2011-03-09 10:06 ` Benjamin Tissoires
@ 2011-03-09 11:40 ` Henrik Rydberg
2011-03-11 6:02 ` Richard Nauber
0 siblings, 1 reply; 21+ messages in thread
From: Henrik Rydberg @ 2011-03-09 11:40 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Richard Nauber, linux-input, Jiri Kosina, Stéphane Chatty
> >> Please do not add numerical constants in a generic code. If I want to
> >> override the values for another device, I'll have to rename your quirk
> >> to MT_QUIRK_EGALAX_XYZ_FIXUP_WITH_0_32760_0_32760_0_x and add mine.
> >> These values have to be moved in the MT_CLS.
> >
> > It is a specific value used for a specific code path, no need to
> > over-generalize. The quirk name could be specialized even more, i
> > suppose, but the point is that it makes the hid-multitouch driver do
> > exactly what is done in the hid-egalax driver, and it is easy to prove
> > that there are no side effects for other devices.
>
> Or it's a generic code path for generic devices: if someone want to do
> the same thing that have been done in hid-egalax (correct the logical
> min/max reported by the device), this could be done.
The point is that this is not generally needed, but necessitated by a
bug in the firmware. Problems like this is why we have quirks.
> Concerning the prove, I'm pretty sure we can manage to prove that both
> solutions are equivalent as the current code already relies on the
> fact that static struct are null initialized.
The comment here was about using specific code paths enabled by quirks.
> >> > case HID_DG_TIPPRESSURE:
> >> > + if (quirks & MT_QUIRK_EGALAX_XYZ_FIXUP)
> >> > + field->logical_minimum = 0;
> >>
> >> No really need for the test and the quirk. In addition, here you do
> >> not override logical_max. This shows that the quirk is not
> >> MT_QUIRK_EGALAX_XYZ_FIXUP but the combination of
> >> MT_QUIRK_EGALAX_XY_FIXUP and MT_QUIRK_EGALAX_Z_MIN_FIXUP.
> >> Just drop it here.
> >
> > The added code path simply reflects what is in the hid-egalax
> > driver. It is true that the statement _probably_ works for other
> > devices as well, but it cannot be proven just by looking at the
> > code. Also, we have no known additional case where it is needed.
>
> I didn't told to remove the test just by looking at the code. I'm
> taking in account all the devices that have been reported to us:
> http://lii-enac.fr/en/architecture/linux-input/multitouch-devices.html
> Among all of them, only asus, egalax (capacitive and resistive),
> mosart (the same as asus), and stantum reports pressure.
> Asus and mosart set only the logical maximum -> logical minimum is
> therefore at 0.
> Stantum does not set the min/max -> logical minimum is at 0 too.
> EGalax set the minimum at 1 -> in your patch, you explained that was a
> bug for the upper layers.
Fine. Since we will always want the pressure to have zero intercept, I
think it is ok to always execute that line, but I still think it is
better to leave it conditional.
> > I am trying to think of a better name for the quirk, but frankly, I
> > think it already says what it needs to say.
>
> What about MT_QUIRK_XYZ_CORRECTIONS_FOR_SPECIAL_EGALAX_DEVICES?
Maybe at bit long, I guess you aren't really serious.
Thanks.
Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework.
2011-03-09 11:40 ` Henrik Rydberg
@ 2011-03-11 6:02 ` Richard Nauber
2011-03-11 6:53 ` Benjamin Tissoires
2011-03-14 12:06 ` Jiri Kosina
0 siblings, 2 replies; 21+ messages in thread
From: Richard Nauber @ 2011-03-11 6:02 UTC (permalink / raw)
To: Henrik Rydberg
Cc: Richard Nauber, linux-input, Jiri Kosina, Stéphane Chatty
Hi,
Except for a missing comma, Henriks patch works fine for me and I would
be glad to have it submitted. So feel free to add my SIGNED-OFF-BY.
(Or should I repost the patch?)
Greetings and thanks to all,
Richard
--- snip ---
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 57575ff..ee01e65 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -131,7 +131,7 @@ struct mt_class mt_classes[] = {
{ .name = MT_CLS_EGALAX,
.quirks = MT_QUIRK_SLOT_IS_CONTACTID |
MT_QUIRK_VALID_IS_INRANGE |
- MT_QUIRK_EGALAX_XYZ_FIXUP
+ MT_QUIRK_EGALAX_XYZ_FIXUP,
.maxcontacts = 2,
.sn_move = 4096,
.sn_pressure = 32,
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v3] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework.
2011-03-11 6:02 ` Richard Nauber
@ 2011-03-11 6:53 ` Benjamin Tissoires
2011-03-11 9:23 ` Richard Nauber
2011-03-14 12:06 ` Jiri Kosina
1 sibling, 1 reply; 21+ messages in thread
From: Benjamin Tissoires @ 2011-03-11 6:53 UTC (permalink / raw)
To: Richard Nauber
Cc: Henrik Rydberg, linux-input, Jiri Kosina, Stéphane Chatty
Hi Richard,
I'm really sorry, but it's a NACK for me:
what ever Henrik said, I do not want to have numerical constants
throughout the code. It would be the open door to many others. The
reason we have "mt_class" is to regroup each per-device parameters in
it. So there is no need to introduce those constants everywhere.
Cheers,
Benjamin
On Fri, Mar 11, 2011 at 07:02, Richard Nauber
<richard.nauber@googlemail.com> wrote:
> Hi,
>
> Except for a missing comma, Henriks patch works fine for me and I would
> be glad to have it submitted. So feel free to add my SIGNED-OFF-BY.
> (Or should I repost the patch?)
>
> Greetings and thanks to all,
> Richard
>
> --- snip ---
>
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 57575ff..ee01e65 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -131,7 +131,7 @@ struct mt_class mt_classes[] = {
> { .name = MT_CLS_EGALAX,
> .quirks = MT_QUIRK_SLOT_IS_CONTACTID |
> MT_QUIRK_VALID_IS_INRANGE |
> - MT_QUIRK_EGALAX_XYZ_FIXUP
> + MT_QUIRK_EGALAX_XYZ_FIXUP,
> .maxcontacts = 2,
> .sn_move = 4096,
> .sn_pressure = 32,
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework.
2011-03-11 6:53 ` Benjamin Tissoires
@ 2011-03-11 9:23 ` Richard Nauber
2011-03-11 9:52 ` Benjamin Tissoires
0 siblings, 1 reply; 21+ messages in thread
From: Richard Nauber @ 2011-03-11 9:23 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Henrik Rydberg, linux-input, Jiri Kosina, Stéphane Chatty
>On Fri, Mar 11, 2011 at 7:53 AM, Benjamin Tissoires
><benjamin.tissoires@gmail.com> wrote:
> Hi Richard,
>
> I'm really sorry, but it's a NACK for me:
> what ever Henrik said, I do not want to have numerical constants
> throughout the code. It would be the open door to many others. The
> reason we have "mt_class" is to regroup each per-device parameters in
> it. So there is no need to introduce those constants everywhere.
Hm, so should I make a patch that applies on top of Henriks "Send
events per slot if CONTACTCOUNT is missing" and uses the generic XY
fixup of my v3 patch?
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework.
2011-03-11 9:23 ` Richard Nauber
@ 2011-03-11 9:52 ` Benjamin Tissoires
2011-03-11 11:31 ` Henrik Rydberg
0 siblings, 1 reply; 21+ messages in thread
From: Benjamin Tissoires @ 2011-03-11 9:52 UTC (permalink / raw)
To: Richard Nauber
Cc: Henrik Rydberg, linux-input, Jiri Kosina, Stéphane Chatty
Hi Richard,
On Fri, Mar 11, 2011 at 10:23, Richard Nauber
<richard.nauber@googlemail.com> wrote:
>>On Fri, Mar 11, 2011 at 7:53 AM, Benjamin Tissoires
>><benjamin.tissoires@gmail.com> wrote:
>> Hi Richard,
>>
>> I'm really sorry, but it's a NACK for me:
>> what ever Henrik said, I do not want to have numerical constants
>> throughout the code. It would be the open door to many others. The
>> reason we have "mt_class" is to regroup each per-device parameters in
>> it. So there is no need to introduce those constants everywhere.
>
> Hm, so should I make a patch that applies on top of Henriks "Send
> events per slot if CONTACTCOUNT is missing" and uses the generic XY
> fixup of my v3 patch?
>
I would say so. Please just change the name of the field .axis by a better name.
Cheers,
Benjamin
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework.
2011-03-11 9:52 ` Benjamin Tissoires
@ 2011-03-11 11:31 ` Henrik Rydberg
0 siblings, 0 replies; 21+ messages in thread
From: Henrik Rydberg @ 2011-03-11 11:31 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Richard Nauber, linux-input, Jiri Kosina, Stéphane Chatty
On Fri, Mar 11, 2011 at 10:52:55AM +0100, Benjamin Tissoires wrote:
> Hi Richard,
>
> On Fri, Mar 11, 2011 at 10:23, Richard Nauber
> <richard.nauber@googlemail.com> wrote:
> >>On Fri, Mar 11, 2011 at 7:53 AM, Benjamin Tissoires
> >><benjamin.tissoires@gmail.com> wrote:
> >> Hi Richard,
> >>
> >> I'm really sorry, but it's a NACK for me:
> >> what ever Henrik said, I do not want to have numerical constants
> >> throughout the code. It would be the open door to many others. The
> >> reason we have "mt_class" is to regroup each per-device parameters in
> >> it. So there is no need to introduce those constants everywhere.
> >
> > Hm, so should I make a patch that applies on top of Henriks "Send
> > events per slot if CONTACTCOUNT is missing" and uses the generic XY
> > fixup of my v3 patch?
> >
>
> I would say so. Please just change the name of the field .axis by a better name.
I disagree with this approach. The code in question is per device bug,
not per device. The next firmware bug might be that the axis are
shifted, or that the value is given with wrong byte order, we just
don't know. Until we know what the next bug has in common with the old
ones, let's be simple.
Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework.
2011-03-11 6:02 ` Richard Nauber
2011-03-11 6:53 ` Benjamin Tissoires
@ 2011-03-14 12:06 ` Jiri Kosina
1 sibling, 0 replies; 21+ messages in thread
From: Jiri Kosina @ 2011-03-14 12:06 UTC (permalink / raw)
To: Richard Nauber; +Cc: Henrik Rydberg, linux-input, Stéphane Chatty
On Fri, 11 Mar 2011, Richard Nauber wrote:
> Hi,
>
> Except for a missing comma, Henriks patch works fine for me and I would
> be glad to have it submitted. So feel free to add my SIGNED-OFF-BY.
> (Or should I repost the patch?)
I have now queued the changes in the 'multitiouch' branch of my tree.
Thanks,
--
Jiri Kosina
SUSE Labs, Novell Inc.
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2011-03-14 12:06 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-06 13:25 [PATCH] [input-hid] Add hid-egalax driver to the unified hid-multitouch framework Richard Nauber
2011-03-07 10:07 ` Benjamin Tissoires
2011-03-08 6:08 ` [PATCH v2] " Richard Nauber
2011-03-08 8:04 ` Henrik Rydberg
2011-03-08 8:55 ` Benjamin Tissoires
2011-03-08 9:14 ` Henrik Rydberg
2011-03-08 9:23 ` Benjamin Tissoires
2011-03-08 9:46 ` Henrik Rydberg
2011-03-08 23:38 ` [PATCH v3] " Richard Nauber
2011-03-09 8:24 ` Henrik Rydberg
2011-03-09 8:54 ` Benjamin Tissoires
2011-03-09 9:23 ` Henrik Rydberg
2011-03-09 10:06 ` Benjamin Tissoires
2011-03-09 11:40 ` Henrik Rydberg
2011-03-11 6:02 ` Richard Nauber
2011-03-11 6:53 ` Benjamin Tissoires
2011-03-11 9:23 ` Richard Nauber
2011-03-11 9:52 ` Benjamin Tissoires
2011-03-11 11:31 ` Henrik Rydberg
2011-03-14 12:06 ` Jiri Kosina
2011-03-08 13:12 ` [PATCH] " Jiri Kosina
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).