From: Thomas Dettbarn Subject: imt(4) Patch To Increase Number Of Usable Devices To: tech@openbsd.org, jcs@jcs.org, dettus@dettus.net Date: Wed, 21 May 2025 17:04:54 +0200 Hello. Below this email, you can find a patch to the imt(4) module in the OpenBSD kernel; for your consideration. Hopefully, it meets your standards and turns out to be useable on as many machines as possible. Thus far, it passes only the ItWorksOnMyMachine(tm) test. (Albeit with flying colours!) A little bit of background: Since my Touchpad was not supported in OpenBSD 7.7, I took the liberty of tracking down the problem. The root issue was the parser of HID Reports Descriptors, which became confused by some constructs. Thus, I made the parser slightly more robust when it encounters said descriptors. In addition to this, I also improved upon the report parser for multitouch devices. I can give you more details, if you are interested. As of now, I can think of two problems with this patch: 1: Its style(9) needs to be checked. 2: It has not been tested in a lot of computers. I need help with those. Thomas xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx Patch by Thomas Dettbarn The original code does not respect Logical Collections inside Application collections from the report descriptor. Instead, it is searching for the LOWEST report ID inside an application collection with the correct usage code (000D/0005). The report descriptor, which was causing issues, looked like the one below. It ended with a vendor specific section, that defined a vendor specific report 06, even though the input report was defined as report 0x54. What this patch does is to restrict the reports which can be found to the ones that are ACTUALLY providing X and Y coordinates. (Device: ihidev0 at iic0 addr 0x15 gpio 171, vendor 0x4f3 product 0x3195, ELAN0676) usage page 000D/0005 (Digitizer/Touch Pad ) collection 01 (Application) report ID 0x54 usage 0009/0001 (Button/Button 1) 1 Bit padding 3 Bit usage 000D/0054 (Digitizer/Contact Count) 4 Bit usage 000D/0056 (Digitizer/Scan Count) 16 Bit usage 000D/0022 (Digitizer/Finger) collection 02 (Logical) usage 000D/0047 (Digitizer/Touch Valid) 1 Bit usage 000D/0042 (Digitizer/Tip Switch) 1 Bit padding 2 Bit usage 000D/0051 (Digitizer/Contact Identifier) 4 Bit usage 0001/0030 (Generic Desktop/X) 16 Bit usage 0001/0031 (Generic Desktop/Y) 16 Bit collection end collection 02 (Logical) usage 000D/0047 (Digitizer/Touch Valid) 1 Bit usage 000D/0042 (Digitizer/Tip Switch) 1 Bit padding 2 Bit usage 000D/0051 (Digitizer/Contact Identifier) 4 Bit usage 0001/0030 (Generic Desktop/X) 16 Bit usage 0001/0031 (Generic Desktop/Y) 16 Bit collection end collection 02 (Logical) usage 000D/0047 (Digitizer/Touch Valid) 1 Bit usage 000D/0042 (Digitizer/Tip Switch) 1 Bit padding 2 Bit usage 000D/0051 (Digitizer/Contact Identifier) 4 Bit usage 0001/0030 (Generic Desktop/X) 16 Bit usage 0001/0031 (Generic Desktop/Y) 16 Bit collection end collection 02 (Logical) usage 000D/0047 (Digitizer/Touch Valid) 1 Bit usage 000D/0042 (Digitizer/Tip Switch) 1 Bit padding 2 Bit usage 000D/0051 (Digitizer/Contact Identifier) 4 Bit usage 0001/0030 (Generic Desktop/X) 16 Bit usage 0001/0031 (Generic Desktop/Y) 16 Bit collection end collection 02 (Logical) usage 000D/0047 (Digitizer/Touch Valid) 1 Bit usage 000D/0042 (Digitizer/Tip Switch) 1 Bit padding 2 Bit usage 000D/0051 (Digitizer/Contact Identifier) 4 Bit usage 0001/0030 (Generic Desktop/X) 16 Bit usage 0001/0031 (Generic Desktop/Y) 16 Bit collection end report ID 02 .... Capabilities report ID 07 .... Latency Mode report ID 06 usage page FF00/00C5 (Vendor specific) 2048 Bit collection end diff --git dev/hid/hidmt.c dev/hid/hidmt.c index 62b500a4f44..27948b38f6f 100644 --- dev/hid/hidmt.c +++ dev/hid/hidmt.c @@ -175,45 +175,46 @@ hidmt_setup(struct device *self, struct hidmt *mt, void *desc, int dlen) if (h.report_ID != mt->sc_rep_input) continue; - if (h.kind != hid_input) - continue; - - switch (h.usage) { - /* contact level usages */ - case HID_USAGE2(HUP_GENERIC_DESKTOP, HUG_X): - if (h.logical_maximum - h.logical_minimum) { - mt->sc_minx = h.logical_minimum; - mt->sc_maxx = h.logical_maximum; - mt->sc_resx = hidmt_get_resolution(&h); - } - break; - case HID_USAGE2(HUP_GENERIC_DESKTOP, HUG_Y): - if (h.logical_maximum - h.logical_minimum) { - mt->sc_miny = h.logical_minimum; - mt->sc_maxy = h.logical_maximum; - mt->sc_resy = hidmt_get_resolution(&h); - } - break; - case HID_USAGE2(HUP_DIGITIZERS, HUD_TIP_SWITCH): - case HID_USAGE2(HUP_DIGITIZERS, HUD_CONFIDENCE): - case HID_USAGE2(HUP_DIGITIZERS, HUD_WIDTH): - case HID_USAGE2(HUP_DIGITIZERS, HUD_HEIGHT): - case HID_USAGE2(HUP_DIGITIZERS, HUD_CONTACTID): - - /* report level usages */ - case HID_USAGE2(HUP_DIGITIZERS, HUD_CONTACTCOUNT): - case HID_USAGE2(HUP_BUTTON, 0x01): - case HID_USAGE2(HUP_BUTTON, 0x02): - case HID_USAGE2(HUP_BUTTON, 0x03): - break; - default: - continue; - } input = malloc(sizeof(*input), M_DEVBUF, M_NOWAIT | M_ZERO); memcpy(&input->loc, &h.loc, sizeof(struct hid_location)); - input->usage = h.usage; + input->kind = h.kind; + + if (h.kind == hid_input) { + switch (h.usage) { + /* contact level usages */ + case HID_USAGE2(HUP_GENERIC_DESKTOP, HUG_X): + if (h.logical_maximum - h.logical_minimum) { + mt->sc_minx = h.logical_minimum; + mt->sc_maxx = h.logical_maximum; + mt->sc_resx = hidmt_get_resolution(&h); + } + break; + case HID_USAGE2(HUP_GENERIC_DESKTOP, HUG_Y): + if (h.logical_maximum - h.logical_minimum) { + mt->sc_miny = h.logical_minimum; + mt->sc_maxy = h.logical_maximum; + mt->sc_resy = hidmt_get_resolution(&h); + } + break; + case HID_USAGE2(HUP_DIGITIZERS, HUD_TIP_SWITCH): + case HID_USAGE2(HUP_DIGITIZERS, HUD_CONFIDENCE): + case HID_USAGE2(HUP_DIGITIZERS, HUD_WIDTH): + case HID_USAGE2(HUP_DIGITIZERS, HUD_HEIGHT): + case HID_USAGE2(HUP_DIGITIZERS, HUD_CONTACTID): + + /* report level usages */ + case HID_USAGE2(HUP_DIGITIZERS, HUD_CONTACTCOUNT): + case HID_USAGE2(HUP_BUTTON, 0x01): + case HID_USAGE2(HUP_BUTTON, 0x02): + case HID_USAGE2(HUP_BUTTON, 0x03): + break; + default: + continue; + } + input->usage = h.usage; + } SIMPLEQ_INSERT_TAIL(&mt->sc_inputs, input, entry); } hid_end_parse(hd); @@ -300,6 +301,14 @@ hidmt_input(struct hidmt *mt, uint8_t *data, u_int len) struct hidmt_contact hc; int32_t d, firstu = 0; int contactcount = 0, seencontacts = 0, tips = 0, buttons = 0, i, s, z; + /* There are two strategies for handling multiple inputs. + * The first one strategy = 0, is plan A: There are logical + * collections inside the report. strategy = 1 is plan B: + * Simply search for repeating usages. + */ + int collection_cnt; + int endcollection_cnt; + int strategy = 1; // enable plan B. if (len != mt->sc_rep_input_size) { DPRINTF(("%s: %s: length %d not %d, ignoring\n", @@ -317,10 +326,19 @@ hidmt_input(struct hidmt *mt, uint8_t *data, u_int len) * report with contactid=0 but contactids are zero-based, find * contactcount first. */ + collection_cnt = 0; + endcollection_cnt = 0; SIMPLEQ_FOREACH(hi, &mt->sc_inputs, entry) { - if (hi->usage == HID_USAGE2(HUP_DIGITIZERS, HUD_CONTACTCOUNT)) + if (hi->kind == hid_collection) + collection_cnt++; + if (hi->kind == hid_endcollection) + endcollection_cnt++; + if (hi->kind == hid_input && hi->usage == HID_USAGE2(HUP_DIGITIZERS, HUD_CONTACTCOUNT)) contactcount = hid_get_udata(data, len, &hi->loc); } + if (collection_cnt == endcollection_cnt && collection_cnt >= contactcount) { + strategy = 0; // plan A is possible + } if (contactcount) mt->sc_cur_contactcount = contactcount; @@ -358,66 +376,69 @@ hidmt_input(struct hidmt *mt, uint8_t *data, u_int len) */ bzero(&hc, sizeof(struct hidmt_contact)); SIMPLEQ_FOREACH(hi, &mt->sc_inputs, entry) { - d = hid_get_udata(data, len, &hi->loc); - - if (firstu && hi->usage == firstu) { + if ((!strategy && hi->kind == hid_endcollection) || + (strategy && firstu && hi->usage == firstu) + ) { if (seencontacts < contactcount) { hc.seen = 1; i = wsmouse_id_to_slot( - mt->sc_wsmousedev, hc.contactid); + mt->sc_wsmousedev, hc.contactid); if (i >= 0) memcpy(&mt->sc_contacts[i], &hc, - sizeof(struct hidmt_contact)); + sizeof(struct hidmt_contact)); seencontacts++; } bzero(&hc, sizeof(struct hidmt_contact)); } - else if (!firstu) - firstu = hi->usage; - - switch (hi->usage) { - case HID_USAGE2(HUP_GENERIC_DESKTOP, HUG_X): - hc.x = d; - break; - case HID_USAGE2(HUP_GENERIC_DESKTOP, HUG_Y): - if (mt->sc_flags & HIDMT_REVY) - hc.y = mt->sc_maxy - d; - else - hc.y = d; - break; - case HID_USAGE2(HUP_DIGITIZERS, HUD_TIP_SWITCH): - hc.tip = d; - if (d) - tips++; - break; - case HID_USAGE2(HUP_DIGITIZERS, HUD_CONFIDENCE): - hc.confidence = d; - break; - case HID_USAGE2(HUP_DIGITIZERS, HUD_WIDTH): - hc.width = d; - break; - case HID_USAGE2(HUP_DIGITIZERS, HUD_HEIGHT): - hc.height = d; - break; - case HID_USAGE2(HUP_DIGITIZERS, HUD_CONTACTID): - hc.contactid = d; - break; - - /* these will only appear once per report */ - case HID_USAGE2(HUP_DIGITIZERS, HUD_CONTACTCOUNT): - if (d) - contactcount = d; - break; - case HID_USAGE2(HUP_BUTTON, 0x01): - case HID_USAGE2(HUP_BUTTON, 0x02): - if (d != 0) - buttons |= 1; - break; - case HID_USAGE2(HUP_BUTTON, 0x03): - if (d != 0) - buttons |= 1 << 2; - break; + else if (hi->kind == hid_input) { + d = hid_get_udata(data, len, &hi->loc); + if (!firstu) + firstu = hi->usage; + + switch (hi->usage) { + case HID_USAGE2(HUP_GENERIC_DESKTOP, HUG_X): + hc.x = d; + break; + case HID_USAGE2(HUP_GENERIC_DESKTOP, HUG_Y): + if (mt->sc_flags & HIDMT_REVY) + hc.y = mt->sc_maxy - d; + else + hc.y = d; + break; + case HID_USAGE2(HUP_DIGITIZERS, HUD_TIP_SWITCH): + hc.tip = d; + if (d) + tips++; + break; + case HID_USAGE2(HUP_DIGITIZERS, HUD_CONFIDENCE): + hc.confidence = d; + break; + case HID_USAGE2(HUP_DIGITIZERS, HUD_WIDTH): + hc.width = d; + break; + case HID_USAGE2(HUP_DIGITIZERS, HUD_HEIGHT): + hc.height = d; + break; + case HID_USAGE2(HUP_DIGITIZERS, HUD_CONTACTID): + hc.contactid = d; + break; + + /* these will only appear once per report */ + case HID_USAGE2(HUP_DIGITIZERS, HUD_CONTACTCOUNT): + if (d) + contactcount = d; + break; + case HID_USAGE2(HUP_BUTTON, 0x01): + case HID_USAGE2(HUP_BUTTON, 0x02): + if (d != 0) + buttons |= 1; + break; + case HID_USAGE2(HUP_BUTTON, 0x03): + if (d != 0) + buttons |= 1 << 2; + break; + } } } if (seencontacts < contactcount) { diff --git dev/hid/hidmtvar.h dev/hid/hidmtvar.h index dabe5867b7e..dd4560b70ff 100644 --- dev/hid/hidmtvar.h +++ dev/hid/hidmtvar.h @@ -16,6 +16,7 @@ */ struct hidmt_data { + enum hid_kind kind; int32_t usage; struct hid_location loc; SIMPLEQ_ENTRY(hidmt_data) entry; diff --git dev/i2c/imt.c dev/i2c/imt.c index 5dcdbed57e9..875e9120e4e 100644 --- dev/i2c/imt.c +++ dev/i2c/imt.c @@ -116,12 +116,18 @@ imt_find_winptp_reports(struct ihidev_softc *parent, void *desc, int size, continue; if (hid_is_collection(desc, size, repid, - HID_USAGE2(HUP_DIGITIZERS, HUD_TOUCHPAD))) { + HID_USAGE2(HUP_DIGITIZERS, HUD_TOUCHPAD)) && + hid_locate(desc, size, HID_USAGE2(HUP_GENERIC_DESKTOP, + HUG_X), repid, hid_input, NULL, NULL) && + hid_locate(desc, size, HID_USAGE2(HUP_GENERIC_DESKTOP, + HUG_Y), repid, hid_input, NULL, NULL)) { input = 1; if (sc != NULL && sc->sc_rep_input == -1) sc->sc_rep_input = repid; } else if (hid_is_collection(desc, size, repid, - HID_USAGE2(HUP_DIGITIZERS, HUD_CONFIG))) { + HID_USAGE2(HUP_DIGITIZERS, HUD_CONFIG)) && + hid_locate(desc, size, HID_USAGE2(HUP_DIGITIZERS, + HUD_INPUT_MODE), repid, hid_feature, NULL, NULL)) { conf = 1; if (sc != NULL && sc->sc_rep_config == -1) sc->sc_rep_config = repid;