From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: Re: sys/ihidev: prevent crash on interrupt storm
To: mpi@grenadille.net, tech@openbsd.org
Date: Wed, 25 Dec 2024 03:51:07 +0300

On Wed, Dec 25, 2024 at 01:30:09AM +0100, Kirill A. Korinsky wrote:
> On Wed, 25 Dec 2024 01:17:56 +0100,
> Kirill A. Korinsky <kirill@korins.ky> wrote:
> > 
> > So, the diff:
> > 

I confirm, my ELAN touchpad works as previous.

iic0 at dwiic0
ihidev0 at iic0 addr 0x15 gpio 83, vendor 0x4f3 product 0x3282, PNP0C50
ihidev0: 92 report ids
imt0 at ihidev0: clickpad, 5 contacts
wsmouse0 at imt0 mux 0
ims0 at ihidev0 reportid 1: 2 buttons
wsmouse1 at ims0 mux 0
hid at ihidev0 reportid 5 not configured
hid at ihidev0 reportid 6 not configured
hid at ihidev0 reportid 7 not configured
hid at ihidev0 reportid 11 not configured
hid at ihidev0 reportid 12 not configured
hid at ihidev0 reportid 13 not configured
ims1 at ihidev0 reportid 14: 0 buttons
wsmouse2 at ims1 mux 0
hid at ihidev0 reportid 92 not configured

> > Index: sys/dev/i2c/ihidev.c
> > ===================================================================
> > RCS file: /home/cvs/src/sys/dev/i2c/ihidev.c,v
> > diff -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	25 Dec 2024 00:13:30 -0000
> > @@ -55,6 +55,7 @@ enum {
> >  
> >  	/* pseudo commands */
> >  	I2C_HID_REPORT_DESCR	= 0x100,
> > +	I2C_HID_RESET_RESPONSE	= 0x101,
> >  };
> >  
> >  static int I2C_HID_POWER_ON	= 0x0;
> > @@ -137,6 +138,21 @@ ihidev_attach(struct device *parent, str
> >  	    letoh16(sc->hid_desc.wVendorID), letoh16(sc->hid_desc.wProductID),
> >  	    (char *)ia->ia_cookie);
> >  
> > +	if (ihidev_reset(sc))
> > +		return;
> > +
> > +	if (ihidev_hid_command(sc, I2C_HID_RESET_RESPONSE, 0)) {
> > +		printf("%s: unexpected reset response\n",
> > +		    sc->sc_dev.dv_xname);
> > +		return;
> > +	}
> > +
> > +	if (ihidev_hid_command(sc, I2C_HID_REPORT_DESCR, 0)) {
> > +		printf("%s: failed fetching HID report\n",
> > +		    sc->sc_dev.dv_xname);
> > +		return;
> > +	}
> > +
> >  	sc->sc_nrepid = ihidev_maxrepid(sc->sc_report, sc->sc_reportlen);
> >  	if (sc->sc_nrepid < 0)
> >  		return;
> > @@ -515,6 +531,28 @@ ihidev_hid_command(struct ihidev_softc *
> >  
> >  		break;
> >  	}
> > +	case I2C_HID_RESET_RESPONSE: {
> > +		int i;
> > +		uint8_t buf[2] = { 0xff, 0xff };
> > +
> > +		DPRINTF(("%s: HID command I2C_HID_RESET_RESPONSE\n",
> > +		    sc->sc_dev.dv_xname));
> > +
> > +		for (i = 0; i < 100; i++) {
> > +			res = iic_exec(sc->sc_tag, I2C_OP_READ_WITH_STOP, sc->sc_addr,
> > +			    NULL, 0, buf, sizeof(buf), 0);
> > +			DPRINTF(("%s: read attempt %d: 0x%x, 0x%x, res: %d\n",
> > +			    sc->sc_dev.dv_xname, i, buf[0], buf[1], res));
> > +			if (!res)
> > +				break;
> > +			ihidev_sleep(sc, 100);
> > +		}
> > +
> > +		if (!res)
> > +			res = (buf[0] != 0x00 || buf[1] != 0x00);
> > +
> > +		break;
> > +	}
> >  	default:
> >  		printf("%s: unknown command %d\n", sc->sc_dev.dv_xname,
> >  		    hidcmd);
> > @@ -528,8 +566,6 @@ ihidev_hid_command(struct ihidev_softc *
> >  int
> >  ihidev_poweron(struct ihidev_softc *sc)
> >  {
> > -	DPRINTF(("%s: resetting\n", sc->sc_dev.dv_xname));
> > -
> >  	if (ihidev_hid_command(sc, I2C_HID_CMD_SET_POWER, &I2C_HID_POWER_ON)) {
> >  		printf("%s: failed to power on\n", sc->sc_dev.dv_xname);
> >  		return (1);
> > @@ -547,6 +583,8 @@ ihidev_reset(struct ihidev_softc *sc)
> >  	if (ihidev_poweron(sc))
> >  		return (1);
> >  
> > +	DPRINTF(("%s: resetting\n", sc->sc_dev.dv_xname));
> > +
> >  	if (ihidev_hid_command(sc, I2C_HID_CMD_RESET, 0)) {
> >  		printf("%s: failed to reset hardware\n", sc->sc_dev.dv_xname);
> >  
> > @@ -570,8 +608,6 @@ ihidev_reset(struct ihidev_softc *sc)
> >  int
> >  ihidev_hid_desc_parse(struct ihidev_softc *sc)
> >  {
> > -	int retries = 3;
> > -
> >  	/* must be v01.00 */
> >  	if (letoh16(sc->hid_desc.bcdVersion) != 0x0100) {
> >  		printf("%s: bad HID descriptor bcdVersion (0x%x)\n",
> > @@ -597,25 +633,8 @@ ihidev_hid_desc_parse(struct ihidev_soft
> >  		return (1);
> >  	}
> >  
> > -	while (retries-- > 0) {
> > -		if (ihidev_reset(sc)) {
> > -			if (retries == 0)
> > -				return(1);
> > -
> > -			ihidev_sleep(sc, 10);
> > -		}
> > -		else
> > -			break;
> > -	}
> > -
> >  	sc->sc_reportlen = letoh16(sc->hid_desc.wReportDescLength);
> >  	sc->sc_report = malloc(sc->sc_reportlen, M_DEVBUF, M_WAITOK | M_ZERO);
> > -
> > -	if (ihidev_hid_command(sc, I2C_HID_REPORT_DESCR, 0)) {
> > -		printf("%s: failed fetching HID report\n",
> > -		    sc->sc_dev.dv_xname);
> > -		return (1);
> > -	}
> >  
> >  	return (0);
> >  }
> > 
> > 
> 
> BTW on Huwaei Matebook X 2020 it also works without any regression.
> 
> -- 
> wbr, Kirill