Index | Thread | Search

From:
Tobias Heider <tobias.heider@stusta.de>
Subject:
ihidev: be less aggressive with reset, fix T14s
To:
tech@openbsd.org
Cc:
deraadt@openbsd.org, kettenis@openbsd.org
Date:
Sat, 12 Oct 2024 14:53:08 +0200

Download raw body.

Thread
This diff fixes the ihidev keyboard on the Lenovo T14s Gen6.
It turns out that this device doesn't like to receive multiple reset
commands at all, after looking at other drivers it seems like we are
the only ones that reset so aggressively.

The diff replaces all but one occurrences of ihidev_reset() with the
new ihidev_poweron() which only sends a power on command instead of
always doing both.

It would probably make sense to test this extensively on a range of hardware,
Linux has a quirk to add explicit resets but that seems to only be needed for
very few devices (ALPS 0x44e, possibly more).
If break with this we might want to add a quirk to still reset them.

diff 502b52cbdf550ec88ed0fcdac323f77049e93d3d 56627129dafdc9ab5465cdd1d777aa09e63460c1
commit - 502b52cbdf550ec88ed0fcdac323f77049e93d3d
commit + 56627129dafdc9ab5465cdd1d777aa09e63460c1
blob - 4e65a4bf350b37ad5567a0c8d6535bd4e3c9cdbe
blob + f6b8cc7b3436dc6c9bff99c817d4e5e2a45c3f44
--- sys/dev/i2c/ihidev.c
+++ sys/dev/i2c/ihidev.c
@@ -67,6 +67,7 @@ int	ihidev_activate(struct device *, int);
 
 int	ihidev_hid_command(struct ihidev_softc *, int, void *);
 int	ihidev_intr(void *);
+int	ihidev_poweron(struct ihidev_softc *);
 int	ihidev_reset(struct ihidev_softc *);
 int	ihidev_hid_desc_parse(struct ihidev_softc *);
 
@@ -248,7 +249,7 @@ ihidev_activate(struct device *self, int act)
 			    sc->sc_dev.dv_xname);
 		break;
 	case DVACT_WAKEUP:
-		ihidev_reset(sc);
+		ihidev_poweron(sc);
 		sc->sc_dying = 0;
 		if (sc->sc_poll && timeout_initialized(&sc->sc_timer))
 			timeout_add(&sc->sc_timer, 2000);
@@ -525,7 +526,7 @@ ihidev_hid_command(struct ihidev_softc *sc, int hidcmd
 }
 
 int
-ihidev_reset(struct ihidev_softc *sc)
+ihidev_poweron(struct ihidev_softc *sc)
 {
 	DPRINTF(("%s: resetting\n", sc->sc_dev.dv_xname));
 
@@ -536,6 +537,16 @@ ihidev_reset(struct ihidev_softc *sc)
 
 	ihidev_sleep(sc, 100);
 
+	return 0;
+}
+
+
+int
+ihidev_reset(struct ihidev_softc *sc)
+{
+	if (ihidev_poweron(sc))
+		return (1);
+
 	if (ihidev_hid_command(sc, I2C_HID_CMD_RESET, 0)) {
 		printf("%s: failed to reset hardware\n", sc->sc_dev.dv_xname);
 
@@ -784,7 +795,7 @@ ihidev_open(struct ihidev *scd)
 		return (0);
 
 	/* power on */
-	ihidev_reset(sc);
+	ihidev_poweron(sc);
 
 	if (sc->sc_poll) {
 		if (!timeout_initialized(&sc->sc_timer))