From: Tobias Heider <tobias.heider@stusta.de>
Subject: Re: sys/ihidev: quirk for Honor MagicBook's touchpad
To: Mark Kettenis <mark.kettenis@xs4all.nl>
Cc: tech@openbsd.org
Date: Tue, 7 Jan 2025 12:02:54 +0100

On Tue, Jan 07, 2025 at 11:49:07AM GMT, Mark Kettenis wrote:
> > Date: Tue, 7 Jan 2025 01:29:54 +0100
> > From: Tobias Heider <tobias.heider@stusta.de>
> > 
> > On Mon, Jan 06, 2025 at 03:27:07AM GMT, Kirill A. Korinsky wrote:
> > > tech@,
> > > 
> > > Here a quirk which is required for Honor MagicBook's touchpad to be read as
> > > multioutch. This touchpad needs re-power command with sleep after report
> > > description is read. Without waitng quite enough, almost the seconds, or
> > > re-power command it reads only as:
> > > 
> > >         imt0 at ihidev1ihidev1: failed fetching report
> > >         ims0 at ihidev1 reportid 1: 3 buttons, Z dir
> > > 
> > > but seems only on the cold boot. Otherwise with 1 seconds sleep and re-power
> > > command, or on warm boot, it reads as:
> > > 
> > >         imt0 at ihidev1: touchpad, 5 contacts
> > >         ims0 at ihidev1 reportid 1: 3 buttons, Z dir
> > > 
> > > Ok?
> > 
> > Tested on the T14s. I have a similar bug where I get "imt0: invalid max X/Y 0/0",
> > I was hoping this might fix that one too but looks like it doesn't.
> 
> I'm seeing that messages on more and more machines.  I think it is the
> result of misparsing/misinterpreting the HID descriptor for these
> devices.  Seems fairly harmless though, so I haven't looked into what
> exactly is going wrong here.

Harmless yes but I think it breaks a few multi-touch features like scrolling 

> 
> > It also didn't make it worse though.
> > 
> > ok tobhe@
> > 
> > > 
> > > Index: sys/dev/i2c/ihidev.c
> > > ===================================================================
> > > RCS file: /cvs/src/sys/dev/i2c/ihidev.c,v
> > > diff -u -p -u -p -r1.35 ihidev.c
> > > --- sys/dev/i2c/ihidev.c	6 Jan 2025 02:13:55 -0000	1.35
> > > +++ sys/dev/i2c/ihidev.c	6 Jan 2025 02:18:27 -0000
> > > @@ -76,6 +76,17 @@ 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
> > > +
> > > +const struct ihidev_quirks {
> > > +	uint16_t		ihq_vid;
> > > +	uint16_t		ihq_pid;
> > > +	int			ihq_quirks;
> > > +} ihidev_devs[] = {
> > > +	/* HONOR MagicBook Art 14 Touchpad (QTEC0002) */
> > > +	{ 0x35cc, 0x0104, IHIDEV_QUIRK_RE_POWER_ON },
> > > +};
> > > +
> > >  const struct cfattach ihidev_ca = {
> > >  	sizeof(struct ihidev_softc),
> > >  	ihidev_match,
> > > @@ -99,6 +110,25 @@ ihidev_match(struct device *parent, void
> > >  	return (0);
> > >  }
> > >  
> > > +int
> > > +ihidev_quirks(struct ihidev_softc *sc)
> > > +{
> > > +	const struct ihidev_quirks	*q;
> > > +	uint16_t			 vid, pid;
> > > +	int 				 i, nent;
> > > +
> > > +	nent = nitems(ihidev_devs);
> > > +
> > > +	vid = letoh16(sc->hid_desc.wVendorID);
> > > +	pid = letoh16(sc->hid_desc.wProductID);
> > > +
> > > +	for (i = 0, q = ihidev_devs; i < nent; i++, q++)
> > > +		if (vid == q->ihq_vid && pid == q->ihq_pid)
> > > +			return (q->ihq_quirks);
> > > +
> > > +	return (0);
> > > +}
> > > +
> > >  void
> > >  ihidev_attach(struct device *parent, struct device *self, void *aux)
> > >  {
> > > @@ -637,6 +667,23 @@ 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/i2c/ihidev.h
> > > ===================================================================
> > > RCS file: /cvs/src/sys/dev/i2c/ihidev.h,v
> > > diff -u -p -u -p -r1.9 ihidev.h
> > > --- sys/dev/i2c/ihidev.h	3 Sep 2022 15:48:16 -0000	1.9
> > > +++ sys/dev/i2c/ihidev.h	6 Jan 2025 02:18:27 -0000
> > > @@ -93,6 +93,8 @@ struct ihidev_softc {
> > >  	int		sc_fastpoll;
> > >  	struct timeout	sc_timer;
> > >  	int		sc_dying;
> > > +
> > > +	int		sc_quirks;
> > >  };
> > >  
> > >  struct ihidev {
> > > 
> > > 
> > > -- 
> > > wbr, Kirill
> > > 
> > 
> >