Index | Thread | Search

From:
Marcus Glocker <marcus@nazgul.ch>
Subject:
Re: Fix boot on HONOR MagicBook Art 14 Snapdragon
To:
"Kirill A. Korinsky" <kirill@korins.ky>
Cc:
tech@openbsd.org
Date:
Mon, 23 Dec 2024 17:14:57 +0100

Download raw body.

Thread
On Mon, Dec 23, 2024 at 11:45:17AM GMT, Kirill A. Korinsky wrote:

> Greetings,
> 
> thanks for review!
> 
> On Mon, 23 Dec 2024 10:42:59 +0100,
> Marcus Glocker <marcus@nazgul.ch> wrote:
> > 
> > On Sun, Dec 22, 2024 at 10:23:30PM GMT, Kirill A. Korinsky wrote:
> > 
> > > After mglocker@ commit to qcgpio.c(v 1.12) with updated mapping, touchpad
> > > started worked.
> > > 
> > > The only quirks that I need to boot it is skip reset before get HID report.
> > 
> > If skipping the reset entirely fixes your touch-pad, I'm fine to
> > introduce the quirk.  Though, a few re-marks inline ...
> >
> 
> After some future diging with it, I had discovered that ihidev_poweron(sc)
> after I2C_HID_REPORT_DESCR allows system to read this as multitouch touchpad.
> 
> With only IHIDEV_QUIRK_SKIP_RESET it reads as:
> 
>         ihidev1 at iic3 addr 0x5d gpio 896, vendor 0x35cc product 0x104, QTEC0002
>         ihidev1: 14 report ids
>         imt0 at ihidev1ihidev1: failed fetching report
>         imt0: failed getting capability report
>         ims0 at ihidev1 reportid 1: 3 buttons, Z dir
>         wsmouse0 at ims0 mux 0
>         hid at ihidev1 reportid 5 not configured
>         hid at ihidev1 reportid 6 not configured
>         hid at ihidev1 reportid 7 not configured
>         icc1 at ihidev1 reportid 8: 768 usages, 20 keys, array
>         wskbd2 at icc1 mux 1
>         hid at ihidev1 reportid 14 not configured
> 
> and with IHIDEV_QUIRK_SKIP_RESET | IHIDEV_QUIRK_RE_POWER_ON as:
> 
>         ihidev1 at iic3 addr 0x5d gpio 896, vendor 0x35cc product 0x104, QTEC0002
>         ihidev1: 14 report ids
>         imt0 at ihidev1: touchpad, 5 contacts
>         wsmouse0 at imt0 mux 0
>         ims0 at ihidev1 reportid 1: 3 buttons, Z dir
>         wsmouse1 at ims0 mux 0
>         hid at ihidev1 reportid 5 not configured
>         hid at ihidev1 reportid 6 not configured
>         hid at ihidev1 reportid 7 not configured
>         icc1 at ihidev1 reportid 8: 768 usages, 20 keys, array
>         wskbd2 at icc1 mux 1
>         hid at ihidev1 reportid 14 not configured

OK, good, that is progress, but ...
 
> > > +
> > > +	if (sc->sc_quirks & IHIDEV_QUIRK_SKIP_RESET)
> > > +		goto skip_rest;
> > 
> > I think "skip_reset" would be more clear.
> >
> 
> Yay, what a typo! Thanks to catching it.
> 
> Here a diff which address all remarks and introduced the second quirk.

... one ask;  Is it really required to add two quirks for that?
The consequence of skipping the reset is that we are also skipping the
power on, because it's contained in ihidev_reset().

Would it also work for the device if we do the power on at the same
place where we skip the reset, so before the I2C_HID_REPORT_DESCR
command?  See what I mean with the following diff.


Index: sys/dev/i2c/ihidev.c
===================================================================
RCS file: /cvs/src/sys/dev/i2c/ihidev.c,v
diff -u -p -u -p -r1.33 ihidev.c
--- sys/dev/i2c/ihidev.c	18 Oct 2024 12:53:49 -0000	1.33
+++ sys/dev/i2c/ihidev.c	23 Dec 2024 16:04:22 -0000
@@ -61,6 +61,7 @@ static int I2C_HID_POWER_ON	= 0x0;
 static int I2C_HID_POWER_OFF	= 0x1;
 
 int	ihidev_match(struct device *, void *, void *);
+int	ihidev_quirks(struct ihidev_softc *);
 void	ihidev_attach(struct device *, struct device *, void *);
 int	ihidev_detach(struct device *, int);
 int	ihidev_activate(struct device *, int);
@@ -75,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_SKIP_RESET		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_SKIP_RESET },
+};
+
 const struct cfattach ihidev_ca = {
 	sizeof(struct ihidev_softc),
 	ihidev_match,
@@ -98,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)
 {
@@ -572,6 +603,8 @@ ihidev_hid_desc_parse(struct ihidev_soft
 {
 	int retries = 3;
 
+	sc->sc_quirks = ihidev_quirks(sc);
+
 	/* must be v01.00 */
 	if (letoh16(sc->hid_desc.bcdVersion) != 0x0100) {
 		printf("%s: bad HID descriptor bcdVersion (0x%x)\n",
@@ -597,6 +630,12 @@ ihidev_hid_desc_parse(struct ihidev_soft
 		return (1);
 	}
 
+	if (sc->sc_quirks & IHIDEV_QUIRK_SKIP_RESET) {
+		if (ihidev_poweron(sc))
+			return (1);
+		goto skip_reset;
+	}
+
 	while (retries-- > 0) {
 		if (ihidev_reset(sc)) {
 			if (retries == 0)
@@ -608,6 +647,7 @@ ihidev_hid_desc_parse(struct ihidev_soft
 			break;
 	}
 
+skip_reset:
 	sc->sc_reportlen = letoh16(sc->hid_desc.wReportDescLength);
 	sc->sc_report = malloc(sc->sc_reportlen, M_DEVBUF, M_WAITOK | M_ZERO);
 
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	23 Dec 2024 16:04:22 -0000
@@ -93,6 +93,8 @@ struct ihidev_softc {
 	int		sc_fastpoll;
 	struct timeout	sc_timer;
 	int		sc_dying;
+
+	int		sc_quirks;
 };
 
 struct ihidev {