Hi Benjamin, > This device is the worst device I saw. It keeps TipSwitch and InRange > at 1 for fingers that are not touching the panel. > The solution is to rely on the field ContactCount, which is accurate > as the correct information are packed at the begining of the frame. > > Unfortunately, CountactCount is most of the time at the end of the report. > The solution is to pick it when we have the whole report in raw_event. > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxx> > --- > drivers/hid/hid-ids.h | 3 ++ > drivers/hid/hid-multitouch.c | 91 ++++++++++++++++++++++++++++++++++++-------- > 2 files changed, 78 insertions(+), 16 deletions(-) I think it would make more sense to introduce a method where the driver sees all report values at once. We have had reasonable cause to add it in the past, but never did. > > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h > index dad56aa..0935012 100644 > --- a/drivers/hid/hid-ids.h > +++ b/drivers/hid/hid-ids.h > @@ -597,6 +597,9 @@ > #define USB_VENDOR_ID_NEC 0x073e > #define USB_DEVICE_ID_NEC_USB_GAME_PAD 0x0301 > > +#define USB_VENDOR_ID_NEXIO 0x1870 > +#define USB_DEVICE_ID_NEXIO_MULTITOUCH_420 0x010d > + > #define USB_VENDOR_ID_NEXTWINDOW 0x1926 > #define USB_DEVICE_ID_NEXTWINDOW_TOUCHSCREEN 0x0003 > > diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c > index 46d8136..c4acdd0 100644 > --- a/drivers/hid/hid-multitouch.c > +++ b/drivers/hid/hid-multitouch.c > @@ -32,6 +32,8 @@ > #include <linux/slab.h> > #include <linux/usb.h> > #include <linux/input/mt.h> > +#include <asm/unaligned.h> > +#include <asm/byteorder.h> > #include "usbhid/usbhid.h" > > > @@ -54,6 +56,7 @@ MODULE_LICENSE("GPL"); > #define MT_QUIRK_NO_AREA (1 << 9) > #define MT_QUIRK_IGNORE_DUPLICATES (1 << 10) > #define MT_QUIRK_HOVERING (1 << 11) > +#define MT_QUIRK_CONTACT_CNT_ACCURATE (1 << 12) > > struct mt_slot { > __s32 x, y, cx, cy, p, w, h; > @@ -83,6 +86,10 @@ struct mt_device { > struct mt_class mtclass; /* our mt device class */ > struct mt_fields *fields; /* temporary placeholder for storing the > multitouch fields */ > + struct hid_field *contactcount; /* the hid_field contact count that > + will be picked in mt_raw_event */ > + __s8 contactcount_index; /* the index of the usage contact count > + in its hid_field. */ > 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 */ > @@ -111,6 +118,7 @@ struct mt_device { > #define MT_CLS_DUAL_INRANGE_CONTACTNUMBER 0x0007 > #define MT_CLS_DUAL_NSMU_CONTACTID 0x0008 > #define MT_CLS_INRANGE_CONTACTNUMBER 0x0009 > +#define MT_CLS_ALWAYS_TRUE 0x000a > > /* vendor specific classes */ > #define MT_CLS_3M 0x0101 > @@ -170,6 +178,9 @@ static struct mt_class mt_classes[] = { > { .name = MT_CLS_INRANGE_CONTACTNUMBER, > .quirks = MT_QUIRK_VALID_IS_INRANGE | > MT_QUIRK_SLOT_IS_CONTACTNUMBER }, > + { .name = MT_CLS_ALWAYS_TRUE, > + .quirks = MT_QUIRK_ALWAYS_VALID | > + MT_QUIRK_CONTACT_CNT_ACCURATE }, > > /* > * vendor specific classes > @@ -250,6 +261,9 @@ static ssize_t mt_set_quirks(struct device *dev, > > td->mtclass.quirks = val; > > + if (!td->contactcount) > + td->mtclass.quirks &= ~MT_QUIRK_CONTACT_CNT_ACCURATE; > + > return count; > } > > @@ -264,24 +278,26 @@ static struct attribute_group mt_attribute_group = { > .attrs = sysfs_attrs > }; > > +static int mt_find_usage_index(struct hid_field *field, struct hid_usage *usage) > +{ > + int i; > + for (i = 0; i < field->maxusage; i++) { > + if (field->usage[i].hid == usage->hid) > + return i; > + } > + return -1; > +} > + > static void mt_feature_mapping(struct hid_device *hdev, > struct hid_field *field, struct hid_usage *usage) > { > struct mt_device *td = hid_get_drvdata(hdev); > - int i; > > switch (usage->hid) { > case HID_DG_INPUTMODE: > td->inputmode = field->report->id; > - td->inputmode_index = 0; /* has to be updated below */ > - > - for (i=0; i < field->maxusage; i++) { > - if (field->usage[i].hid == usage->hid) { > - td->inputmode_index = i; > - break; > - } > - } > - > + td->inputmode_index = mt_find_usage_index(field, usage); > + /* inputmode_index can't be set at -1 */ > break; > case HID_DG_CONTACTMAX: > td->maxcontact_report_id = field->report->id; > @@ -459,6 +475,10 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi, > td->last_field_index = field->index; > return 1; > case HID_DG_CONTACTCOUNT: > + td->contactcount = field; > + td->contactcount_index = mt_find_usage_index(field, > + usage); > + /* contactcount_index can't be set at -1 */ > td->last_field_index = field->index; > return 1; > case HID_DG_CONTACTMAX: > @@ -523,6 +543,10 @@ static int mt_compute_slot(struct mt_device *td, struct input_dev *input) > */ > static void mt_complete_slot(struct mt_device *td, struct input_dev *input) > { > + if ((td->mtclass.quirks & MT_QUIRK_CONTACT_CNT_ACCURATE) && > + td->num_received >= td->num_expected) > + return; > + > if (td->curvalid || (td->mtclass.quirks & MT_QUIRK_ALWAYS_VALID)) { > int slotnum = mt_compute_slot(td, input); > struct mt_slot *s = &td->curdata; > @@ -623,12 +647,6 @@ static int mt_event(struct hid_device *hid, struct hid_field *field, > td->curdata.h = value; > break; > case HID_DG_CONTACTCOUNT: > - /* > - * Includes multi-packet support where subsequent > - * packets are sent with zero contactcount. > - */ > - if (value) > - td->num_expected = value; > break; > case HID_DG_TOUCH: > /* do nothing */ > @@ -658,6 +676,37 @@ static int mt_event(struct hid_device *hid, struct hid_field *field, > return 1; > } > > +static int mt_raw_event(struct hid_device *hid, struct hid_report *report, > + u8 *data, int size) > +{ > + struct mt_device *td = hid_get_drvdata(hid); > + struct hid_field *field = td->contactcount; > + s32 *value; > + > + if (field && report->id == field->report->id) { > + /* > + * Pick in advance the field HID_DG_CONTACTCOUNT as it is > + * often placed at the end of the report. > + */ > + if (report->id) > + data++; > + > + value = hid_extract_field(hid, field, data); > + if (!value) > + return 0; > + > + /* > + * Includes multi-packet support where subsequent > + * packets are sent with zero contactcount. > + */ > + if (value[td->contactcount_index]) > + td->num_expected = value[td->contactcount_index]; > + > + kfree(value); > + } > + return 0; > +} This is a lot of cycles for something that is already available in the core. > + > static void mt_set_input_mode(struct hid_device *hdev) > { > struct mt_device *td = hid_get_drvdata(hdev); > @@ -719,11 +768,15 @@ static void mt_post_parse_default_settings(struct mt_device *td) > static void mt_post_parse(struct mt_device *td) > { > struct mt_fields *f = td->fields; > + struct mt_class *cls = &td->mtclass; > > if (td->touches_by_report > 0) { > int field_count_per_touch = f->length / td->touches_by_report; > td->last_slot_field = f->usages[field_count_per_touch - 1]; > } > + > + if (!td->contactcount) > + cls->quirks &= ~MT_QUIRK_CONTACT_CNT_ACCURATE; > } > > static void mt_input_configured(struct hid_device *hdev, struct hid_input *hi) > @@ -1056,6 +1109,11 @@ static const struct hid_device_id mt_devices[] = { > MT_USB_DEVICE(USB_VENDOR_ID_TURBOX, > USB_DEVICE_ID_TURBOX_TOUCHSCREEN_MOSART) }, > > + /* Nexio panels */ > + { .driver_data = MT_CLS_ALWAYS_TRUE, > + MT_USB_DEVICE(USB_VENDOR_ID_NEXIO, > + USB_DEVICE_ID_NEXIO_MULTITOUCH_420)}, > + > /* Panasonic panels */ > { .driver_data = MT_CLS_PANASONIC, > MT_USB_DEVICE(USB_VENDOR_ID_PANASONIC, > @@ -1193,6 +1251,7 @@ static struct hid_driver mt_driver = { > .feature_mapping = mt_feature_mapping, > .usage_table = mt_grabbed_usages, > .event = mt_event, > + .raw_event = mt_raw_event, Rather a new and simpler event method here, in other words. > #ifdef CONFIG_PM > .reset_resume = mt_reset_resume, > .resume = mt_resume, > -- > 1.8.1 > Thanks, Henrik -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html