From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Henrik Rydberg" Subject: Re: [RFC v3 3/5] hid-multitouch: support for PixCir-based panels Date: Fri, 7 Jan 2011 20:49:18 +0100 Message-ID: <20110107194918.GB29913@polaris.bitmath.org> References: <1294425762-29730-1-git-send-email-benjamin.tissoires@enac.fr> <1294425762-29730-4-git-send-email-benjamin.tissoires@enac.fr> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Content-Disposition: inline In-Reply-To: <1294425762-29730-4-git-send-email-benjamin.tissoires@enac.fr> Sender: linux-kernel-owner@vger.kernel.org To: Benjamin Tissoires Cc: Stephane Chatty , Dmitry Torokhov , Jiri Kosina , linux-input@vger.kernel.org, linux-kernel@vger.kernel.org List-Id: linux-input@vger.kernel.org On Fri, Jan 07, 2011 at 07:42:40PM +0100, Benjamin Tissoires wrote: > Created a driver for PixCir based dual-touch panels, including the on= e > in the Hanvon tablet. This is done in a code structure aimed at unif= ying > support for several existing HID multitouch panels. >=20 > Signed-off-by: Benjamin Tissoires > Signed-off-by: St=E9phane Chatty > --- It looks much better now. Just a few comments and some nit-picks inline= =2E > drivers/hid/Kconfig | 9 + > drivers/hid/Makefile | 1 + > drivers/hid/hid-core.c | 2 + > drivers/hid/hid-ids.h | 4 + > drivers/hid/hid-multitouch.c | 468 ++++++++++++++++++++++++++++++++= ++++++++++ > 5 files changed, 484 insertions(+), 0 deletions(-) > create mode 100644 drivers/hid/hid-multitouch.c >=20 > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig > index 401acec..511554d 100644 > --- a/drivers/hid/Kconfig > +++ b/drivers/hid/Kconfig > @@ -285,6 +285,15 @@ config HID_MONTEREY > ---help--- > Support for Monterey Genius KB29E. > =20 > +config HID_MULTITOUCH > + tristate "HID Multitouch panels" > + depends on USB_HID > + ---help--- > + Generic support for HID multitouch panels. > + > + Say Y here if you have one of the following devices: > + - PixCir touchscreen Should also mention how to compile as module, and what the module will = be called. > + > config HID_NTRIG > tristate "N-Trig touch screen" > depends on USB_HID > diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile > index c335605..ec991d4 100644 > --- a/drivers/hid/Makefile > +++ b/drivers/hid/Makefile > @@ -46,6 +46,7 @@ obj-$(CONFIG_HID_MAGICMOUSE) +=3D hid-magicmouse= =2Eo > obj-$(CONFIG_HID_MICROSOFT) +=3D hid-microsoft.o > obj-$(CONFIG_HID_MONTEREY) +=3D hid-monterey.o > obj-$(CONFIG_HID_MOSART) +=3D hid-mosart.o > +obj-$(CONFIG_HID_MULTITOUCH) +=3D hid-multitouch.o > obj-$(CONFIG_HID_NTRIG) +=3D hid-ntrig.o > obj-$(CONFIG_HID_ORTEK) +=3D hid-ortek.o > obj-$(CONFIG_HID_PRODIKEYS) +=3D hid-prodikeys.o > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > index 88668ae..2b4d9b9 100644 > --- a/drivers/hid/hid-core.c > +++ b/drivers/hid/hid-core.c > @@ -1286,6 +1286,7 @@ static const struct hid_device_id hid_blacklist= [] =3D { > { HID_USB_DEVICE(USB_VENDOR_ID_BELKIN, USB_DEVICE_ID_FLIP_KVM) }, > { HID_USB_DEVICE(USB_VENDOR_ID_BTC, USB_DEVICE_ID_BTC_EMPREX_REMOTE= ) }, > { HID_USB_DEVICE(USB_VENDOR_ID_BTC, USB_DEVICE_ID_BTC_EMPREX_REMOTE= _2) }, > + { HID_USB_DEVICE(USB_VENDOR_ID_CANDO, USB_DEVICE_ID_CANDO_PIXCIR_MU= LTI_TOUCH) }, > { HID_USB_DEVICE(USB_VENDOR_ID_CANDO, USB_DEVICE_ID_CANDO_MULTI_TOU= CH) }, > { HID_USB_DEVICE(USB_VENDOR_ID_CANDO, USB_DEVICE_ID_CANDO_MULTI_TOU= CH_11_6) }, > { HID_USB_DEVICE(USB_VENDOR_ID_CANDO, USB_DEVICE_ID_CANDO_MULTI_TOU= CH_15_6) }, > @@ -1312,6 +1313,7 @@ static const struct hid_device_id hid_blacklist= [] =3D { > { HID_USB_DEVICE(USB_VENDOR_ID_GYRATION, USB_DEVICE_ID_GYRATION_REM= OTE) }, > { HID_USB_DEVICE(USB_VENDOR_ID_GYRATION, USB_DEVICE_ID_GYRATION_REM= OTE_2) }, > { HID_USB_DEVICE(USB_VENDOR_ID_GYRATION, USB_DEVICE_ID_GYRATION_REM= OTE_3) }, > + { HID_USB_DEVICE(USB_VENDOR_ID_HANVON, USB_DEVICE_ID_HANVON_MULTITO= UCH) }, > { HID_USB_DEVICE(USB_VENDOR_ID_KENSINGTON, USB_DEVICE_ID_KS_SLIMBLA= DE) }, > { HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_ERGO_525V) }, > { HID_USB_DEVICE(USB_VENDOR_ID_LABTEC, USB_DEVICE_ID_LABTEC_WIRELES= S_KEYBOARD) }, > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h > index 0f150c7..17b444b 100644 > --- a/drivers/hid/hid-ids.h > +++ b/drivers/hid/hid-ids.h > @@ -134,6 +134,7 @@ > #define USB_DEVICE_ID_BTC_EMPREX_REMOTE_2 0x5577 > =20 > #define USB_VENDOR_ID_CANDO 0x2087 > +#define USB_DEVICE_ID_CANDO_PIXCIR_MULTI_TOUCH 0x0703 > #define USB_DEVICE_ID_CANDO_MULTI_TOUCH 0x0a01 > #define USB_DEVICE_ID_CANDO_MULTI_TOUCH_11_6 0x0b03 > #define USB_DEVICE_ID_CANDO_MULTI_TOUCH_15_6 0x0f01 > @@ -306,6 +307,9 @@ > #define USB_DEVICE_ID_HANWANG_TABLET_FIRST 0x5000 > #define USB_DEVICE_ID_HANWANG_TABLET_LAST 0x8fff > =20 > +#define USB_VENDOR_ID_HANVON 0x20b3 > +#define USB_DEVICE_ID_HANVON_MULTITOUCH 0x0a18 > + > #define USB_VENDOR_ID_HAPP 0x078b > #define USB_DEVICE_ID_UGCI_DRIVING 0x0010 > #define USB_DEVICE_ID_UGCI_FLYING 0x0020 > diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouc= h.c > new file mode 100644 > index 0000000..3b05dfe > --- /dev/null > +++ b/drivers/hid/hid-multitouch.c > @@ -0,0 +1,468 @@ > +/* > + * HID driver for multitouch panels > + * > + * Copyright (c) 2010-2011 Stephane Chatty > + * Copyright (c) 2010-2011 Benjamin Tissoires > + * Copyright (c) 2010-2011 Ecole Nationale de l'Aviation Civile, Fr= ance > + * > + */ > + > +/* > + * This program is free software; you can redistribute it and/or mod= ify 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 > +#include > +#include > +#include > +#include > +#include > +#include "usbhid/usbhid.h" > + > + > +MODULE_AUTHOR("Stephane Chatty "); > +MODULE_DESCRIPTION("HID multitouch panels"); > +MODULE_LICENSE("GPL"); > + > +#include "hid-ids.h" > + > +/* quirks to control the device */ > +#define MT_QUIRK_NOT_SEEN_MEANS_UP (1 << 0) > +#define MT_QUIRK_SLOT_IS_CONTACTID (1 << 1) > + > +struct mt_slot { > + __s32 x, y, p, w, h; > + __s32 contactid; /* the device ContactID assigned to this slot */ > + bool touch_state; /* is the touch valid? */ > + bool seen_in_this_frame;/* has this slot been updated */ > +}; > + > +struct mt_device { > + struct mt_slot curdata; /* placeholder of incoming data */ > + struct mt_class *mtclass; /* our mt device class */ > + unsigned last_field_index; /* last field index of the report */ > + unsigned last_slot_field; /* the last field of a slot */ > + __s8 inputmode; /* InputMode HID feature, -1 if non-existent */ > + __u8 num_received; /* how many contacts we received */ > + __u8 num_expected; /* expected last contact index */ Any particular reason these are u8? > + bool curvalid; /* is the current contact valid? */ > + struct mt_slot slots[0]; /* first slot */ > +}; > + > +struct mt_class { > + __s32 quirks; > + __s32 sn_move; /* Signal/noise ratio for move events */ > + __s32 sn_pressure; /* Signal/noise ratio for pressure events */ > + __u8 maxcontacts; Same here - its not like we allocate a million of these anyways. Simple= ints should do fine. > +}; > + > +/* classes of device behavior */ > +#define MT_CLS_DEFAULT 0 > +#define MT_CLS_DUAL1 1 > + > +/* > + * these device-dependent functions determine what slot corresponds > + * to a valid contact that was just read. > + */ > + > +static int slot_is_contactid(struct mt_device *td) > +{ > + return td->curdata.contactid; > +} > + > +static int find_slot_from_contactid(struct mt_device *td) > +{ > + int i; > + for (i =3D 0; i < td->mtclass->maxcontacts; ++i) { > + if (td->slots[i].contactid =3D=3D td->curdata.contactid && > + td->slots[i].touch_state) > + return i; > + } > + for (i =3D 0; i < td->mtclass->maxcontacts; ++i) { > + if (!td->slots[i].seen_in_this_frame && > + !td->slots[i].touch_state) > + return i; > + } > + return -1; > + /* should not occurs. If this happens that means > + * that the device sent more touches that it says > + * in the report descriptor. It is ignored then. */ I would put the comment above the return statement. > +} > + > +struct mt_class mt_classes[] =3D { > + { 0, 0, 0, 10 }, /* MT_CLS_DEFAULT */ > + { MT_QUIRK_SLOT_IS_CONTACTID, 0, 0, 2 }, /* MT_CLS_DUAL1 */ > +}; > + > +static void mt_feature_mapping(struct hid_device *hdev, struct hid_i= nput *hi, > + struct hid_field *field, struct hid_usage *usage) > +{ > + if (usage->hid =3D=3D HID_DG_INPUTMODE) { > + struct mt_device *td =3D hid_get_drvdata(hdev); > + td->inputmode =3D field->report->id; > + } > +} > + > +static void set_abs(struct input_dev *input, unsigned int code, > + struct hid_field *field, int snratio) > +{ > + int fmin =3D field->logical_minimum; > + int fmax =3D field->logical_maximum; > + int fuzz =3D snratio ? (fmax - fmin) / snratio : 0; > + input_set_abs_params(input, code, fmin, fmax, fuzz, 0); > +} > + > +static int mt_input_mapping(struct hid_device *hdev, struct hid_inpu= t *hi, > + struct hid_field *field, struct hid_usage *usage, > + unsigned long **bit, int *max) > +{ > + struct mt_device *td =3D hid_get_drvdata(hdev); > + struct mt_class *cls =3D td->mtclass; > + switch (usage->hid & HID_USAGE_PAGE) { > + > + case HID_UP_GENDESK: > + switch (usage->hid) { > + case HID_GD_X: > + hid_map_usage(hi, usage, bit, max, > + EV_ABS, ABS_MT_POSITION_X); > + set_abs(hi->input, ABS_MT_POSITION_X, field, > + cls->sn_move); > + /* touchscreen emulation */ > + set_abs(hi->input, ABS_X, field, cls->sn_move); > + td->last_slot_field =3D usage->hid; > + return 1; > + case HID_GD_Y: > + hid_map_usage(hi, usage, bit, max, > + EV_ABS, ABS_MT_POSITION_Y); > + set_abs(hi->input, ABS_MT_POSITION_Y, field, > + cls->sn_move); > + /* touchscreen emulation */ > + set_abs(hi->input, ABS_Y, field, cls->sn_move); > + td->last_slot_field =3D usage->hid; > + return 1; > + } > + return 0; > + > + case HID_UP_DIGITIZER: > + switch (usage->hid) { > + case HID_DG_INRANGE: > + td->last_slot_field =3D usage->hid; > + return 1; > + case HID_DG_CONFIDENCE: > + td->last_slot_field =3D usage->hid; > + return 1; > + case HID_DG_TIPSWITCH: > + hid_map_usage(hi, usage, bit, max, EV_KEY, BTN_TOUCH); > + input_set_capability(hi->input, EV_KEY, BTN_TOUCH); > + td->last_slot_field =3D usage->hid; > + return 1; > + case HID_DG_CONTACTID: > + input_mt_init_slots(hi->input, > + td->mtclass->maxcontacts); > + td->last_slot_field =3D usage->hid; > + return 1; > + case HID_DG_WIDTH: > + hid_map_usage(hi, usage, bit, max, > + EV_ABS, ABS_MT_TOUCH_MAJOR); > + td->last_slot_field =3D usage->hid; > + return 1; > + case HID_DG_HEIGHT: > + hid_map_usage(hi, usage, bit, max, > + EV_ABS, ABS_MT_TOUCH_MINOR); > + field->logical_maximum =3D 1; > + field->logical_minimum =3D 1; minimum should be zero here. > + set_abs(hi->input, ABS_MT_ORIENTATION, field, 0); > + td->last_slot_field =3D usage->hid; > + return 1; > + case HID_DG_TIPPRESSURE: > + hid_map_usage(hi, usage, bit, max, > + EV_ABS, ABS_MT_PRESSURE); > + set_abs(hi->input, ABS_MT_PRESSURE, field, > + cls->sn_pressure); > + /* touchscreen emulation */ > + set_abs(hi->input, ABS_PRESSURE, field, > + cls->sn_pressure); > + td->last_slot_field =3D usage->hid; > + return 1; > + case HID_DG_CONTACTCOUNT: > + td->last_field_index =3D field->report->maxfield - 1; > + return 1; > + case HID_DG_CONTACTMAX: > + /* we don't set td->last_slot_field as contactcount and > + * contact max are global to the report */ > + return -1; > + } > + /* let hid-input decide for the others */ > + return 0; > + > + case 0xff000000: > + /* we do not want to map these: no input-oriented meaning */ > + return -1; > + } > + > + return 0; > +} > + > +static int mt_input_mapped(struct hid_device *hdev, struct hid_input= *hi, > + struct hid_field *field, struct hid_usage *usage, > + unsigned long **bit, int *max) > +{ > + if (usage->type =3D=3D EV_KEY || usage->type =3D=3D EV_ABS) > + set_bit(usage->type, hi->input->evbit); > + > + return -1; > +} > + > +static int mt_compute_slot(struct mt_device *td) > +{ > + struct mt_class *cls =3D td->mtclass; > + > + if (cls->quirks & MT_QUIRK_SLOT_IS_CONTACTID) > + return slot_is_contactid(td); No real need for a function call here... > + > + return find_slot_from_contactid(td); > +} > + > +/* > + * this function is called when a whole contact has been processed, > + * so that it can assign it to a slot and store the data there > + */ > +static void mt_complete_slot(struct mt_device *td) > +{ Adding "td->curdata.seen_in_this_frame =3D true;" here... > + if (td->curvalid) { > + struct mt_slot *slot; Skipping this... > + int slotnum =3D mt_compute_slot(td); > + > + if (slotnum >=3D 0 && slotnum < td->mtclass->maxcontacts) { > + slot =3D td->slots + slotnum; and this... > + > + memcpy(slot, &(td->curdata), sizeof(struct mt_slot)); and "td->slots[slotnum] =3D td->curdata" here... > + slot->seen_in_this_frame =3D true; and dropping this... looks simpler, ne? > + } > + } > + td->num_received++; > +} Writing it explicitly here so you can judge for yourself: td->curdata.seen_in_this_frame =3D true; if (td->curvalid) { int slot =3D mt_compute_slot(td); if (slot >=3D 0 && slot < td->mtclass->maxcontacts) td->slots[slot] =3D td->curdata; } td->num_received++; > + > + > +/* > + * this function is called when a whole packet has been received and= processed, > + * so that it can decide what to send to the input layer. > + */ > +static void mt_emit_event(struct mt_device *td, struct input_dev *in= put) > +{ > + int i; > + > + for (i =3D 0; i < td->mtclass->maxcontacts; ++i) { > + struct mt_slot *s =3D &(td->slots[i]); > + if ((td->mtclass->quirks & MT_QUIRK_NOT_SEEN_MEANS_UP) && > + !s->seen_in_this_frame) { > + /* > + * this slot does not contain useful data, > + * notify its closure > + */ It does contain useful data, it is just assumed to be in the up state. I would drop the comment and the brackets here, the quirk name speaks for itself. > + s->touch_state =3D false; > + } > + > + input_mt_slot(input, i); > + input_mt_report_slot_state(input, MT_TOOL_FINGER, > + s->touch_state); The values below should not be updated for an inactive slot. > + input_event(input, EV_ABS, ABS_MT_POSITION_X, s->x); > + input_event(input, EV_ABS, ABS_MT_POSITION_Y, s->y); > + input_event(input, EV_ABS, ABS_MT_PRESSURE, s->p); > + input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, s->w); > + input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, s->h); > + s->seen_in_this_frame =3D false; > + > + } > + > + input_mt_report_pointer_emulation(input, true); > + input_sync(input); > + td->num_received =3D 0; > +} > + > + > + > +static int mt_event(struct hid_device *hid, struct hid_field *field, > + struct hid_usage *usage, __s32 value) > +{ > + struct mt_device *td =3D hid_get_drvdata(hid); > + > + if (hid->claimed & HID_CLAIMED_INPUT) { > + switch (usage->hid) { > + case HID_DG_INRANGE: > + break; > + case HID_DG_TIPSWITCH: > + td->curvalid =3D value; I do not think curvalid should depend on the touch state (which is what tipswitch is). Either move to INRANGE, or simply set to true. > + td->curdata.touch_state =3D value; > + break; > + case HID_DG_CONFIDENCE: > + break; > + case HID_DG_CONTACTID: > + td->curdata.contactid =3D value; > + break; > + case HID_DG_TIPPRESSURE: > + td->curdata.p =3D value; > + break; > + case HID_GD_X: > + td->curdata.x =3D value; > + break; > + case HID_GD_Y: > + td->curdata.y =3D value; > + break; > + case HID_DG_WIDTH: > + td->curdata.w =3D value; > + break; > + case HID_DG_HEIGHT: > + td->curdata.h =3D value; > + break; > + case HID_DG_CONTACTCOUNT: > + /* > + * We must not overwrite the previous value (some > + * devices send one sequence splitted over several > + * messages) > + */ How about "Includes multi-packet support where subsequent packets are s= ent with zero contactcount." > + if (value) > + td->num_expected =3D value - 1; The - 1 should be dropped here. > + break; > + > + default: > + /* fallback to the generic hidinput handling */ > + return 0; > + } > + } > + > + if (usage->hid =3D=3D td->last_slot_field) > + mt_complete_slot(td); > + > + if (field->index =3D=3D td->last_field_index > + && td->num_received > td->num_expected) A ">=3D" here. > + 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); > + > + return 1; > +} > + > +static void mt_set_input_mode(struct hid_device *hdev) > +{ > + struct mt_device *td =3D hid_get_drvdata(hdev); > + struct hid_report *r; > + struct hid_report_enum *re; > + > + if (td->inputmode < 0) > + return; > + > + re =3D &(hdev->report_enum[HID_FEATURE_REPORT]); > + r =3D re->report_id_hash[td->inputmode]; > + if (r) { > + r->field[0]->value[0] =3D 0x02; > + usbhid_submit_report(hdev, r, USB_DIR_OUT); > + } > +} > + > +static int mt_probe(struct hid_device *hdev, const struct hid_device= _id *id) > +{ > + int ret; > + struct mt_device *td; > + struct mt_class *mtclass =3D mt_classes + id->driver_data; > + > + /* This allows the driver to correctly support devices > + * that emit events over several HID messages. > + */ > + hdev->quirks |=3D HID_QUIRK_NO_INPUT_SYNC; > + > + td =3D kzalloc(sizeof(struct mt_device) + > + mtclass->maxcontacts * sizeof(struct mt_slot), > + GFP_KERNEL); > + if (!td) { > + dev_err(&hdev->dev, "cannot allocate multitouch data\n"); > + return -ENOMEM; > + } > + td->mtclass =3D mtclass; > + td->inputmode =3D -1; > + hid_set_drvdata(hdev, td); > + > + ret =3D hid_parse(hdev); > + if (ret !=3D 0) > + goto fail; "if (ret)" is very standard. > + > + ret =3D hid_hw_start(hdev, HID_CONNECT_DEFAULT); > + if (ret !=3D 0) > + goto fail; > + > + mt_set_input_mode(hdev); > + > + return 0; > + > +fail: > + kfree(td); > + return ret; > +} > + > +#ifdef CONFIG_PM > +static int mt_reset_resume(struct hid_device *hdev) > +{ > + mt_set_input_mode(hdev); > + return 0; > +} > +#endif > + > +static void mt_remove(struct hid_device *hdev) > +{ > + struct mt_device *td =3D hid_get_drvdata(hdev); > + hid_hw_stop(hdev); > + kfree(td); > + hid_set_drvdata(hdev, NULL); > +} > + > +static const struct hid_device_id mt_devices[] =3D { > + > + /* PixCir-based panels */ > + { .driver_data =3D MT_CLS_DUAL1, > + HID_USB_DEVICE(USB_VENDOR_ID_HANVON, > + USB_DEVICE_ID_HANVON_MULTITOUCH) }, > + { .driver_data =3D MT_CLS_DUAL1, > + HID_USB_DEVICE(USB_VENDOR_ID_CANDO, > + USB_DEVICE_ID_CANDO_PIXCIR_MULTI_TOUCH) }, > + > + { } > +}; > +MODULE_DEVICE_TABLE(hid, mt_devices); > + > +static const struct hid_usage_id mt_grabbed_usages[] =3D { > + { 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 mt_driver =3D { > + .name =3D "hid-multitouch", > + .id_table =3D mt_devices, > + .probe =3D mt_probe, > + .remove =3D mt_remove, > + .input_mapping =3D mt_input_mapping, > + .input_mapped =3D mt_input_mapped, > + .feature_mapping =3D mt_feature_mapping, > + .usage_table =3D mt_grabbed_usages, > + .event =3D mt_event, > +#ifdef CONFIG_PM > + .reset_resume =3D mt_reset_resume, > +#endif > +}; > + > +static int __init mt_init(void) > +{ > + return hid_register_driver(&mt_driver); > +} > + > +static void __exit mt_exit(void) > +{ > + hid_unregister_driver(&mt_driver); > +} > + > +module_init(mt_init); > +module_exit(mt_exit); > --=20 > 1.7.3.4 >=20 Thanks! Henrik