Download raw body.
sys/ihidev: replace HONOR's touchpad quirk
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 <mark.kettenis@xs4all.nl> wrote:
> >
> > > Date: Wed, 27 May 2026 02:03:30 +0200
> > > From: Kirill A. Korinsky <kirill@korins.ky>
> > >
> > > 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);
sys/ihidev: replace HONOR's touchpad quirk