Index | Thread | Search

From:
Mark Kettenis <mark.kettenis@xs4all.nl>
Subject:
Re: sys/ihidev: replace HONOR's touchpad quirk
To:
Kirill A. Korinsky <kirill@korins.ky>
Cc:
tech@openbsd.org
Date:
Wed, 27 May 2026 12:53:46 +0200

Download raw body.

Thread
> 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?

> 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);
> 
> 
> -- 
> wbr, Kirill
> 
>