Index | Thread | Search

From:
Thomas Dettbarn <dettus@dettus.net>
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

Download raw body.

Thread
  • Thomas Dettbarn:

    imt(4) Patch To Increase Number Of Usable Devices

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 <dettus () dettus - net>

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;