Download raw body.
imt(4) Patch To Increase Number Of Usable Devices
Hello.
In accordance with slide 16 of the wonderful document
https://www.openbsd.org/papers/eurobsdcon2017-device-drivers.pdf
this is me, following up on my own thread after one week. During said week,
a new corner case came to mind, so the patch below is an improvement upon the
first improvment.
There are still two things with which I need help:
1. Testing on more than one computer
2. Reformatting the code in style(9)
Thomas
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
Patch 2 by Thomas Dettbarn <dettus () dettus - net>
(git diff against c6ac02a2b3a608d4bafeca03d190519141521fad)
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 sys/dev/hid/hidmt.c sys/dev/hid/hidmt.c
index 62b500a4f44..b393b40e9e5 100644
--- sys/dev/hid/hidmt.c
+++ sys/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,15 @@ 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 usagey_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 +327,22 @@ 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;
+ usagey_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_GENERIC_DESKTOP, HUG_Y))
+ usagey_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 >= usagey_cnt && collection_cnt >= contactcount) {
+ strategy = 0; // plan A is possible
+ }
if (contactcount)
mt->sc_cur_contactcount = contactcount;
@@ -358,66 +380,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 sys/dev/hid/hidmtvar.h sys/dev/hid/hidmtvar.h
index dabe5867b7e..dd4560b70ff 100644
--- sys/dev/hid/hidmtvar.h
+++ sys/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 sys/dev/i2c/imt.c sys/dev/i2c/imt.c
index 5dcdbed57e9..875e9120e4e 100644
--- sys/dev/i2c/imt.c
+++ sys/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;
imt(4) Patch To Increase Number Of Usable Devices