From: Kirill A. Korinsky Subject: Re: sys/ihidev: replace HONOR's touchpad quirk To: Mark Kettenis , Marcus Glocker Cc: OpenBSD tech Date: Tue, 23 Jun 2026 12:26:58 +0200 On Wed, 27 May 2026 12:53:46 +0200, Mark Kettenis wrote: > > > Date: Wed, 27 May 2026 02:03:30 +0200 > > From: Kirill A. Korinsky > > > > deraadt@ suggested to send the first byte to that device. > > > > It actually works and I started to dig around... > > > > Seems that the touchpad ignores the first command and it is a kind wakeup > > call. Replacing it to just touch an address works the same way. > > > > It was observed that sending anything to the touchpad wakes it up but it > > ignores the command that was sent. If it was GET_REPORT, when read reads > > series of 0xff. > > > > Sending anything to oher bus does not wake it up, but just touch it's > > address works. > > > > It smells like missed "wakeup" and I had checked linux sources in few > > places, here and where, but I haven't found anything similar to such wakeup > > call. > > > > So, here a very narrow quirk replacment for affected device. > > > > Thoughts? Ok? > > This doesn't make a lot of sense to me. What the Linux does is this > extra POWER_ON command that we've implemented. Why doesn't that "wake > up" the device? Is it in the wrong place? > Sorry for long delay. I had run a lot of expirements and checking. So, I have more or less full picture about that touchpad. The first things: Linux doesn't really care about GET_REPORT response. If it doesn't read sane response (0xff 0xff ...), it uses default values. We do not. We giveup on device. A lot of my expirements ends with two possibel direction which makes that touchpad stable to work. 1. A trivial hack by send the second GET_REPORT and ignore the first one. Small changes inside ihidev only. 2. We need to "touch" address before send that GET_REPORT somehow. I've used I2C_ADDR_ONLY for that and added it via cmdlen == 0 && buflen == 0. Ugly by implementation, but I don't see how to make it cleaner. But semantically it is minimal code which I needed to make it works. I not sure which is better, but after a month of background thinking about it I'd like to use the first one, with GET_REPORT as minimalized one. And yes, I haven't found any trace of similar hack anywhere. I attached both approach as two diff file. -- wbr, Kirill Index: sys/dev/i2c/ihidev.c =================================================================== RCS file: /home/cvs/src/sys/dev/i2c/ihidev.c,v diff -u -p -r1.43 ihidev.c --- sys/dev/i2c/ihidev.c 23 May 2026 11:10:57 -0000 1.43 +++ sys/dev/i2c/ihidev.c 23 May 2026 13:57:30 -0000 @@ -76,7 +76,7 @@ int ihidev_maxrepid(void *buf, int len); int ihidev_print(void *aux, const char *pnp); int ihidev_submatch(struct device *parent, void *cf, void *aux); -#define IHIDEV_QUIRK_RE_POWER_ON 0x1 +#define IHIDEV_QUIRK_RETRY_GET_REPORT 0x1 const struct ihidev_quirks { uint16_t ihq_vid; @@ -84,7 +84,7 @@ const struct ihidev_quirks { int ihq_quirks; } ihidev_devs[] = { /* HONOR MagicBook Art 14 Touchpad (QTEC0002) */ - { 0x35cc, 0x0104, IHIDEV_QUIRK_RE_POWER_ON }, + { 0x35cc, 0x0104, IHIDEV_QUIRK_RETRY_GET_REPORT }, }; const struct cfattach ihidev_ca = { @@ -368,6 +368,7 @@ ihidev_hid_command(struct ihidev_softc * int dataoff = 4; int report_id = rreq->id; int report_len = rreq->len + 2 + 1; + int attempt, ntries; int d; uint8_t *tmprep; @@ -403,8 +404,22 @@ ihidev_hid_command(struct ihidev_softc * tmprep = malloc(report_len, M_DEVBUF, M_WAITOK | M_ZERO); /* type 3 id 8: 22 00 38 02 23 00 */ - res = iic_exec(sc->sc_tag, I2C_OP_READ_WITH_STOP, sc->sc_addr, - &cmd, cmdlen, tmprep, report_len, 0); + ntries = (sc->sc_quirks & IHIDEV_QUIRK_RETRY_GET_REPORT) ? + 5 : 1; + for (attempt = 0; attempt < ntries; attempt++) { + memset(tmprep, 0, report_len); + res = iic_exec(sc->sc_tag, I2C_OP_READ_WITH_STOP, + sc->sc_addr, &cmd, cmdlen, tmprep, report_len, 0); + + d = tmprep[0] | tmprep[1] << 8; + if (res == 0 && + d == report_len && + tmprep[2] == rreq->id) + break; + + if (attempt + 1 < ntries) + ihidev_sleep(sc, 100); + } d = tmprep[0] | tmprep[1] << 8; if (d != report_len) @@ -653,23 +668,6 @@ ihidev_hid_desc_parse(struct ihidev_soft printf("%s: failed fetching HID report\n", sc->sc_dev.dv_xname); return (1); - } - - if (sc->sc_quirks & IHIDEV_QUIRK_RE_POWER_ON) { - if (ihidev_poweron(sc)) - return (1); - - /* - * 7.2.8 states that a device shall not respond back - * after receiving the power on command, and must ensure - * that it transitions to power on state in less than 1 - * second. The ihidev_poweron function uses a shorter - * sleep, sufficient for the ON-RESET sequence. Here, - * however, it sleeps for the full second to accommodate - * cold boot scenarios on affected devices. - */ - - ihidev_sleep(sc, 1000); } return (0); Index: sys/dev/fdt/qciic_fdt.c =================================================================== RCS file: /home/cvs/src/sys/dev/fdt/qciic_fdt.c,v diff -u -p -r1.4 qciic_fdt.c --- sys/dev/fdt/qciic_fdt.c 24 May 2026 10:36:01 -0000 1.4 +++ sys/dev/fdt/qciic_fdt.c 26 May 2026 23:30:50 -0000 @@ -38,6 +38,7 @@ #define GENI_M_CMD0 0x600 #define GENI_M_CMD0_OPCODE_I2C_WRITE (0x1 << 27) #define GENI_M_CMD0_OPCODE_I2C_READ (0x2 << 27) +#define GENI_M_CMD0_OPCODE_I2C_ADDR_ONLY (0x4 << 27) #define GENI_M_CMD0_SLV_ADDR_SHIFT 9 #define GENI_M_CMD0_STOP_STRETCH (1 << 2) #define GENI_M_IRQ_STATUS 0x610 @@ -222,6 +223,15 @@ qciic_fdt_exec(void *cookie, i2c_op_t op if (buflen == 0 && I2C_OP_STOP_P(op)) m_param &= ~GENI_M_CMD0_STOP_STRETCH; + + if (cmdlen == 0 && buflen == 0 && I2C_OP_WRITE_P(op)) { + stat = HREAD4(sc, GENI_M_IRQ_STATUS); + HWRITE4(sc, GENI_M_IRQ_CLEAR, stat); + m_cmd = GENI_M_CMD0_OPCODE_I2C_ADDR_ONLY | m_param; + HWRITE4(sc, GENI_M_CMD0, m_cmd); + + return qciic_fdt_wait(sc, GENI_M_IRQ_CMD_DONE); + } if (cmdlen > 0) { stat = HREAD4(sc, GENI_M_IRQ_STATUS); Index: sys/dev/i2c/ihidev.c =================================================================== RCS file: /home/cvs/src/sys/dev/i2c/ihidev.c,v diff -u -p -r1.43 ihidev.c --- sys/dev/i2c/ihidev.c 23 May 2026 11:10:57 -0000 1.43 +++ sys/dev/i2c/ihidev.c 26 May 2026 23:36:08 -0000 @@ -76,7 +76,7 @@ int ihidev_maxrepid(void *buf, int len); int ihidev_print(void *aux, const char *pnp); int ihidev_submatch(struct device *parent, void *cf, void *aux); -#define IHIDEV_QUIRK_RE_POWER_ON 0x1 +#define IHIDEV_QUIRK_WAKEUP 0x1 const struct ihidev_quirks { uint16_t ihq_vid; @@ -84,7 +84,7 @@ const struct ihidev_quirks { int ihq_quirks; } ihidev_devs[] = { /* HONOR MagicBook Art 14 Touchpad (QTEC0002) */ - { 0x35cc, 0x0104, IHIDEV_QUIRK_RE_POWER_ON }, + { 0x35cc, 0x0104, IHIDEV_QUIRK_WAKEUP }, }; const struct cfattach ihidev_ca = { @@ -402,6 +402,16 @@ ihidev_hid_command(struct ihidev_softc * */ tmprep = malloc(report_len, M_DEVBUF, M_WAITOK | M_ZERO); + if (sc->sc_quirks & IHIDEV_QUIRK_WAKEUP) { + /* + * Some devices, for example HONOR's Touchpad (QTEC0002) + * ignores the first command and returns all 0xff bytes + * unless it was "woke up" by an addressed transaction. + */ + iic_exec(sc->sc_tag, I2C_OP_WRITE_WITH_STOP, sc->sc_addr, + NULL, 0, NULL, 0, 0); + } + /* type 3 id 8: 22 00 38 02 23 00 */ res = iic_exec(sc->sc_tag, I2C_OP_READ_WITH_STOP, sc->sc_addr, &cmd, cmdlen, tmprep, report_len, 0); @@ -653,23 +663,6 @@ ihidev_hid_desc_parse(struct ihidev_soft printf("%s: failed fetching HID report\n", sc->sc_dev.dv_xname); return (1); - } - - if (sc->sc_quirks & IHIDEV_QUIRK_RE_POWER_ON) { - if (ihidev_poweron(sc)) - return (1); - - /* - * 7.2.8 states that a device shall not respond back - * after receiving the power on command, and must ensure - * that it transitions to power on state in less than 1 - * second. The ihidev_poweron function uses a shorter - * sleep, sufficient for the ON-RESET sequence. Here, - * however, it sleeps for the full second to accommodate - * cold boot scenarios on affected devices. - */ - - ihidev_sleep(sc, 1000); } return (0);