Index | Thread | Search

From:
Thomas Dettbarn <dettus@dettus.net>
Subject:
imt(4): Offering a patch
To:
tech@openbsd.org
Date:
Fri, 16 May 2025 17:28:10 +0200

Download raw body.

Thread
Hello.

My touchpad was giving me trouble on OpenBSD 7.7, so I deciced
to obsess over^H^H^H^H^H^H^H^H^H^H^Hfix it.

I was able to narrow my problem down to a HID report descriptor, which
was slightly more feature-rich than what OpenBSD was able to handle.(*)

I came up with a small patch, and took the liberty of attaching it to 
this email.

It is not yet a "perfect" solution, but it already extends the number of 
touchpad
devices your Operating system can use. Therefore, I would like to offer 
it for your
consideration.

(For an easier understanding of what it is trying to achieve, I added 
slightly long
comments, feel free to remove them. )

It would be amazing if it could become part of 7.8, if it meets your 
standards, that is.


Thomas


(*) Thus far, the parser for HID Report Descriptors in imt(4) is 
ignoring logical collections.(**)
(**) A proper solution would be to extend the parser, but I wanted to 
keep the patch
small for now. (Since it is my first time...) So it is more of a fix 
than a solution, but not a
hotfix, hence the name.
--- dev/i2c/imt.c.orig	Fri May 16 16:10:42 2025
+++ dev/i2c/imt.c	Fri May 16 16:50:52 2025
@@ -100,6 +100,77 @@
 imt_find_winptp_reports(struct ihidev_softc *parent, void *desc, int size,
     struct imt_softc *sc)
 {
+	/*
+	 * 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
+	 *     
+	 */
 	int repid;
 	int input = 0, conf = 0, cap = 0;
 
@@ -116,12 +187,18 @@
 			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_feature, NULL, NULL) &&
+		    hid_locate(desc, size, HID_USAGE2(HUP_GENERIC_DESKTOP,
+		      HUG_Y), repid, hid_feature, 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;
--- dev/hid/hidmt.c.orig	Fri May 16 16:15:18 2025
+++ dev/hid/hidmt.c	Fri May 16 16:52:49 2025
@@ -364,60 +364,141 @@
 			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;
+		else if (!firstu) {
+			/*
+			 * TODO: So, what is actually happening here is that this loop is not respecting logical 
+			 * collections from the report descriptor.
+			 * Instead, it is searching for the usage which is repeating.
+			 *
+			 * The report descriptor, which was causing issues, looked like the one below.
+			 * The first usage, "firstu" was 00090001, but that one was never repeated.
+			 *
+			 * (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
+			 *     
+			 */
+			switch (hi->usage) {
+				case HID_USAGE2(HUP_GENERIC_DESKTOP, HUG_X):
+				case HID_USAGE2(HUP_GENERIC_DESKTOP, HUG_Y):
+				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):
+//				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):
+					firstu = hi->usage;
+					break;
+			}
+		}
 
 		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;
+			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;
+				/* 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) {