| Message ID | 1359649351-11092-4-git-send-email-benjamin.tissoires@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jiri Kosina |
| Headers | show |
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@gmail.com> > --- > drivers/hid/hid-ids.h | 3 +++ > drivers/hid/hid-multitouch.c | 36 ++++++++++++++++++++++++++++++------ > 2 files changed, 33 insertions(+), 6 deletions(-) > > 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 092c09b..c9b8fe5 100644 > --- a/drivers/hid/hid-multitouch.c > +++ b/drivers/hid/hid-multitouch.c > @@ -54,6 +54,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 +84,7 @@ struct mt_device { > struct mt_class mtclass; /* our mt device class */ > struct mt_fields *fields; /* temporary placeholder for storing the > multitouch fields */ > + __s32 *contactcount; /* contact count value in the report */ Why not an index here? > unsigned last_field_index; /* last field index of the report */ > unsigned last_slot_field; /* the last field of a slot */ > unsigned mt_report_id; /* the report ID of the multitouch device */ > @@ -112,6 +114,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 > @@ -171,6 +174,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 > @@ -251,6 +257,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; > + Why override the overrider here? > return count; > } > > @@ -461,6 +470,7 @@ 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->value + usage->usage_index; An index into the the struct actually passed in mt_report() feels safer. > td->last_field_index = field->index; > return 1; > case HID_DG_CONTACTMAX: > @@ -525,6 +535,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; > @@ -635,12 +649,6 @@ static void mt_process_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 */ > @@ -676,6 +684,13 @@ static void mt_report(struct hid_device *hid, struct hid_report *report) > if (!(hid->claimed & HID_CLAIMED_INPUT)) > return; > > + /* > + * Includes multi-packet support where subsequent > + * packets are sent with zero contactcount. > + */ > + if (td->contactcount && *td->contactcount) > + td->num_expected = *td->contactcount; > + Here, that is. > for (r = 0; r < report->maxfield; r++) { > field = report->field[r]; > count = field->report_count; > @@ -750,11 +765,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; Since MT_QUIRK_CONTACT_CNT_ACCURATE is a quirk, modifiable by the user, it should probably not validate num_expected in the code. Better use the contact count index or something equivalent for that. > } > > static void mt_input_configured(struct hid_device *hdev, struct hid_input *hi) > @@ -1087,6 +1106,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, > -- > 1.8.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
Hi Henrik, On Sun, Feb 3, 2013 at 2:00 PM, Henrik Rydberg <rydberg@euromail.se> wrote: > 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@gmail.com> >> --- >> drivers/hid/hid-ids.h | 3 +++ >> drivers/hid/hid-multitouch.c | 36 ++++++++++++++++++++++++++++++------ >> 2 files changed, 33 insertions(+), 6 deletions(-) >> >> 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 092c09b..c9b8fe5 100644 >> --- a/drivers/hid/hid-multitouch.c >> +++ b/drivers/hid/hid-multitouch.c >> @@ -54,6 +54,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 +84,7 @@ struct mt_device { >> struct mt_class mtclass; /* our mt device class */ >> struct mt_fields *fields; /* temporary placeholder for storing the >> multitouch fields */ >> + __s32 *contactcount; /* contact count value in the report */ > > Why not an index here? Just because an index is not sufficient. You need two things: an index within the field, and the actual field (a pointer to a struct hid_field). Each .value is local to a field, and even if in most of the case, the contact count is alone in its field, it would mean to take the risk that a new device does not follow this logic. So the actual pointer to the contact count value seemed to be the shortest way to do it. But it can be easily changed. > >> unsigned last_field_index; /* last field index of the report */ >> unsigned last_slot_field; /* the last field of a slot */ >> unsigned mt_report_id; /* the report ID of the multitouch device */ >> @@ -112,6 +114,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 >> @@ -171,6 +174,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 >> @@ -251,6 +257,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; >> + > > Why override the overrider here? This callback is called from the user-space through the sysfs attribute. So, it is not called in the same time that the mt_post_parse function. This is just to avoid a user setting this quirk once the device is up and running leading to a potential oops. > >> return count; >> } >> >> @@ -461,6 +470,7 @@ 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->value + usage->usage_index; > > An index into the the struct actually passed in mt_report() feels safer. again, you need to store "field" and "usage->usage_index". Agree, it would be safer but it will take more space... :) > >> td->last_field_index = field->index; >> return 1; >> case HID_DG_CONTACTMAX: >> @@ -525,6 +535,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; >> @@ -635,12 +649,6 @@ static void mt_process_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 */ >> @@ -676,6 +684,13 @@ static void mt_report(struct hid_device *hid, struct hid_report *report) >> if (!(hid->claimed & HID_CLAIMED_INPUT)) >> return; >> >> + /* >> + * Includes multi-packet support where subsequent >> + * packets are sent with zero contactcount. >> + */ >> + if (td->contactcount && *td->contactcount) >> + td->num_expected = *td->contactcount; >> + > > Here, that is. > >> for (r = 0; r < report->maxfield; r++) { >> field = report->field[r]; >> count = field->report_count; >> @@ -750,11 +765,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; > > Since MT_QUIRK_CONTACT_CNT_ACCURATE is a quirk, modifiable by the > user, it should probably not validate num_expected in the code. Better > use the contact count index or something equivalent for that. As when the user changes the quirk, we validate it, this is not required. Cheers, Benjamin > >> } >> >> static void mt_input_configured(struct hid_device *hdev, struct hid_input *hi) >> @@ -1087,6 +1106,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, >> -- >> 1.8.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
Hi Benjamin, > > Why not an index here? > > Just because an index is not sufficient. You need two things: an index > within the field, and the actual field (a pointer to a struct > hid_field). Each .value is local to a field, and even if in most of > the case, the contact count is alone in its field, it would mean to > take the risk that a new device does not follow this logic. The field value is passed to process_mt_event() in a fairly straight-forward fashion, I was imagining that behavior could be copied somehow. > So the actual pointer to the contact count value seemed to be the > shortest way to do it. But it can be easily changed. Keeping a pointer into the core structure creates unwanted dependencies to the scope of that value, making an eventual core refactoring even harder, not to mention trickier to debug. So even though it looks neat in the code, it pushes the problem forward. > >> @@ -251,6 +257,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; > >> + > > > > Why override the overrider here? > > This callback is called from the user-space through the sysfs > attribute. So, it is not called in the same time that the > mt_post_parse function. This is just to avoid a user setting this > quirk once the device is up and running leading to a potential oops. Yes, but the quirk _is_ user modifiable. Hence, the problem lies in equating the user-modifiable quirk with the branch control of the program. > > An index into the the struct actually passed in mt_report() feels safer. > > again, you need to store "field" and "usage->usage_index". Agree, it > would be safer but it will take more space... :) If you think the code change is not only correct, but also moves the whole code base in a good direction, by all means. > >> @@ -750,11 +765,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; > > > > Since MT_QUIRK_CONTACT_CNT_ACCURATE is a quirk, modifiable by the > > user, it should probably not validate num_expected in the code. Better > > use the contact count index or something equivalent for that. > > As when the user changes the quirk, we validate it, this is not required. True, barring the comments above. 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
On Mon, Feb 4, 2013 at 12:42 PM, Henrik Rydberg <rydberg@euromail.se> wrote: > Hi Benjamin, > >> > Why not an index here? >> >> Just because an index is not sufficient. You need two things: an index >> within the field, and the actual field (a pointer to a struct >> hid_field). Each .value is local to a field, and even if in most of >> the case, the contact count is alone in its field, it would mean to >> take the risk that a new device does not follow this logic. > > The field value is passed to process_mt_event() in a fairly > straight-forward fashion, I was imagining that behavior could be > copied somehow. > >> So the actual pointer to the contact count value seemed to be the >> shortest way to do it. But it can be easily changed. > > Keeping a pointer into the core structure creates unwanted > dependencies to the scope of that value, making an eventual core > refactoring even harder, not to mention trickier to debug. So even > though it looks neat in the code, it pushes the problem forward. > >> >> @@ -251,6 +257,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; >> >> + >> > >> > Why override the overrider here? >> >> This callback is called from the user-space through the sysfs >> attribute. So, it is not called in the same time that the >> mt_post_parse function. This is just to avoid a user setting this >> quirk once the device is up and running leading to a potential oops. > > Yes, but the quirk _is_ user modifiable. Hence, the problem lies in > equating the user-modifiable quirk with the branch control of the > program. I'm not sure I understood what you meant there. The quirk is indeed user modifiable, but through the callback mt_set_quirks() only. If the HID field Contact Count is not present, this quirk should not be allowed to be present, thus the two removals of the quirk in met_set_quirks() and mt_post_parse(). As there are no other entry points, I'm quite confuse to understand where the pitfall is. > >> > An index into the the struct actually passed in mt_report() feels safer. >> >> again, you need to store "field" and "usage->usage_index". Agree, it >> would be safer but it will take more space... :) > > If you think the code change is not only correct, but also moves the > whole code base in a good direction, by all means. Ok, will do. After a deeper look at it, I can even have two int indexes (and no pointers): one for the field and one other for the value within the field. Cheers, Benjamin > >> >> @@ -750,11 +765,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; >> > >> > Since MT_QUIRK_CONTACT_CNT_ACCURATE is a quirk, modifiable by the >> > user, it should probably not validate num_expected in the code. Better >> > use the contact count index or something equivalent for that. >> >> As when the user changes the quirk, we validate it, this is not required. > > True, barring the comments above. > > 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
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 092c09b..c9b8fe5 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c @@ -54,6 +54,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 +84,7 @@ struct mt_device { struct mt_class mtclass; /* our mt device class */ struct mt_fields *fields; /* temporary placeholder for storing the multitouch fields */ + __s32 *contactcount; /* contact count value in the report */ unsigned last_field_index; /* last field index of the report */ unsigned last_slot_field; /* the last field of a slot */ unsigned mt_report_id; /* the report ID of the multitouch device */ @@ -112,6 +114,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 @@ -171,6 +174,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 @@ -251,6 +257,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; } @@ -461,6 +470,7 @@ 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->value + usage->usage_index; td->last_field_index = field->index; return 1; case HID_DG_CONTACTMAX: @@ -525,6 +535,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; @@ -635,12 +649,6 @@ static void mt_process_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 */ @@ -676,6 +684,13 @@ static void mt_report(struct hid_device *hid, struct hid_report *report) if (!(hid->claimed & HID_CLAIMED_INPUT)) return; + /* + * Includes multi-packet support where subsequent + * packets are sent with zero contactcount. + */ + if (td->contactcount && *td->contactcount) + td->num_expected = *td->contactcount; + for (r = 0; r < report->maxfield; r++) { field = report->field[r]; count = field->report_count; @@ -750,11 +765,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) @@ -1087,6 +1106,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,
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@gmail.com> --- drivers/hid/hid-ids.h | 3 +++ drivers/hid/hid-multitouch.c | 36 ++++++++++++++++++++++++++++++------ 2 files changed, 33 insertions(+), 6 deletions(-)