From: Marcus Glocker Subject: Re: sys/ihidev: replace HONOR's touchpad quirk To: "Kirill A. Korinsky" Cc: tech@openbsd.org Date: Sat, 4 Jul 2026 15:51:14 +0200 On Tue, Jun 23, 2026 at 12:26:58PM +0200, Kirill A. Korinsky wrote: > 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. I wouldn't over complicate this and go with this diff. It's isolated in ihidev(4), shouldn't impact working devices, and seems to fix your issue. If somebody comes up with a better solution later on, so be it. ok mglocker@ > 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);