tech@,

i2c states that a device should response for RESET in less than 5
seconds, and if it doesn't reponse as 0x00 0x00 it shouldn't be used.

Also, a device shall not respond back after receiving POWER ON, and must
ensure that it transitions power on state in less than 1 second, not
100ms.

Without waiting the RESET response a HID report description from
touchpad at Honor Magicbook 14 Art Snapdragon is reading as as series of
0xFF bytes every time, and with not enough waiting after power on likes
1 out of 10 reboot.

Feedback and help to find the right fix instead of qurik from mvs@

Additionally tested on Huawe Matebook X 2020, no regression.

Feedback? Tests? Ok?

Index: sys/dev/i2c/ihidev.c
===================================================================
RCS file: /home/cvs/src/sys/dev/i2c/ihidev.c,v
diff -u -p -u -p -r1.34 ihidev.c
--- sys/dev/i2c/ihidev.c	2 Jan 2025 23:26:50 -0000	1.34
+++ sys/dev/i2c/ihidev.c	3 Jan 2025 00:24:12 -0000
@@ -55,6 +55,7 @@ enum {
 
 	/* pseudo commands */
 	I2C_HID_REPORT_DESCR	= 0x100,
+	I2C_HID_RESET_RESPONSE	= 0x101,
 };
 
 static int I2C_HID_POWER_ON	= 0x0;
@@ -516,6 +517,32 @@ ihidev_hid_command(struct ihidev_softc *
 
 		break;
 	}
+	case I2C_HID_RESET_RESPONSE: {
+		int i;
+		uint8_t buf[2] = { 0xff, 0xff };
+
+		DPRINTF(("%s: HID command I2C_HID_RESET_RESPONSE\n",
+		    sc->sc_dev.dv_xname));
+
+		/*
+		 * 7.2.1 states that a device should response for RESET
+		 * in less than 5 seconds. It uses poll instead of
+		 * tsleep because interrupts are blocked during autoconf.
+		 */
+		for (i = 0; i < 50; i++) {
+			ihidev_sleep(sc, 100);
+			res = iic_exec(sc->sc_tag, I2C_OP_READ_WITH_STOP, sc->sc_addr,
+			    NULL, 0, buf, sizeof(buf), 0);
+			DPRINTF(("%s: read attempt %d: 0x%x, 0x%x, res: %d\n",
+			    sc->sc_dev.dv_xname, i, buf[0], buf[1], res));
+			if (!res)
+				res = (buf[0] != 0x00 || buf[1] != 0x00);
+			if (!res)
+				break;
+		}
+
+		break;
+	}
 	default:
 		printf("%s: unknown command %d\n", sc->sc_dev.dv_xname,
 		    hidcmd);
@@ -529,14 +556,15 @@ ihidev_hid_command(struct ihidev_softc *
 int
 ihidev_poweron(struct ihidev_softc *sc)
 {
-	DPRINTF(("%s: resetting\n", sc->sc_dev.dv_xname));
-
 	if (ihidev_hid_command(sc, I2C_HID_CMD_SET_POWER, &I2C_HID_POWER_ON)) {
 		printf("%s: failed to power on\n", sc->sc_dev.dv_xname);
 		return (1);
 	}
 
-	ihidev_sleep(sc, 100);
+	/* 7.2.8 states that a device shall not respond back after
+	 * receiving the command, and must ensure that it transitions to
+	 * specified power state in less than 1 second */
+	ihidev_sleep(sc, 1000);
 
 	return 0;
 }
@@ -548,6 +576,8 @@ ihidev_reset(struct ihidev_softc *sc)
 	if (ihidev_poweron(sc))
 		return (1);
 
+	DPRINTF(("%s: resetting\n", sc->sc_dev.dv_xname));
+
 	if (ihidev_hid_command(sc, I2C_HID_CMD_RESET, 0)) {
 		printf("%s: failed to reset hardware\n", sc->sc_dev.dv_xname);
 
@@ -557,7 +587,11 @@ ihidev_reset(struct ihidev_softc *sc)
 		return (1);
 	}
 
-	ihidev_sleep(sc, 100);
+	if (ihidev_hid_command(sc, I2C_HID_RESET_RESPONSE, 0)) {
+		printf("%s: unexpected reset response\n",
+		    sc->sc_dev.dv_xname);
+		return (1);
+	}
 
 	return (0);
 }
@@ -571,8 +605,6 @@ ihidev_reset(struct ihidev_softc *sc)
 int
 ihidev_hid_desc_parse(struct ihidev_softc *sc)
 {
-	int retries = 3;
-
 	/* must be v01.00 */
 	if (letoh16(sc->hid_desc.bcdVersion) != 0x0100) {
 		printf("%s: bad HID descriptor bcdVersion (0x%x)\n",
@@ -598,16 +630,8 @@ ihidev_hid_desc_parse(struct ihidev_soft
 		return (1);
 	}
 
-	while (retries-- > 0) {
-		if (ihidev_reset(sc)) {
-			if (retries == 0)
-				return(1);
-
-			ihidev_sleep(sc, 10);
-		}
-		else
-			break;
-	}
+	if (ihidev_reset(sc))
+		return (1);
 
 	sc->sc_reportlen = letoh16(sc->hid_desc.wReportDescLength);
 	sc->sc_report = malloc(sc->sc_reportlen, M_DEVBUF, M_WAITOK | M_ZERO);

-- 
wbr, Kirill