On Sun, 22 Aug 2021 at 22:37:51 -0600, Thomas Frohwein wrote:
> On Sun, Aug 22, 2021 at 09:50:15PM -0500, joshua stein wrote:
> > On a particular laptop with a touchpad behind ihidev, dwiic would 
> > report a timeout every time it had to fetch touch data:
> > 
> >     dwiic0: timed out reading remaining 2
> > 
> > On re-reading the i2c HID spec, the size supplied by wMaxInputLength 
> > is already supposed to account for the size and report id bytes so 
> > we shouldn't be adding them after the fact.  Otherwise ihidev would 
> > ask for more data than can be available and, on this laptop anyway, 
> > dwiic would have to wait for this transaction to timeout and fail.
> > 
> > This fix matches how the Linux i2c-hid driver operates.  I've tested 
> > this on 3 laptops with touchpads and touchscreens and it doesn't 
> > cause any regressions here while fixing the touchpad on one of them.  
> > I'd appreciate tests on other laptops to make sure it doesn't break 
> > anything and perhaps fixes your issue if you've also seen constant 
> > dwiic timeouts.
> > 
> 
> I thought I had the same problem on my new Asus Expertbook B9400CEA.
> During boot, while reordering libraries and later it would print:
> 
> dwiic2: timed out reading remaining 16
> 
> This is accompanied by generalized slowness of the system - boot is
> noticeably slow, xenodm takes about 20 seconds to get to the login
> window, and glxgears(1) slows down visibly with any mouse movement.

That is because dwiic has to use polling while being invoked by 
ihidev's interrupt handler or else it panics in tsleep.  It has to 
wait longer than normal using delay(), so it blocks other kernel 
processing.

As for why the timeouts are happening in the first place, it could 
be a similar issue with the max report size being bogus.  Does this 
diff change anything?


diff --git sys/dev/i2c/ihidev.c sys/dev/i2c/ihidev.c
index 92778c679ad..0baba616329 100644
--- sys/dev/i2c/ihidev.c
+++ sys/dev/i2c/ihidev.c
@@ -152,7 +152,7 @@ ihidev_attach(struct device *parent, struct device *self, 
void *aux)
        }
 
        /* find largest report size and allocate memory for input buffer */
-       sc->sc_isize = letoh16(sc->hid_desc.wMaxInputLength);
+       sc->sc_isize = 4;
        for (repid = 0; repid < sc->sc_nrepid; repid++) {
                repsz = hid_report_size(sc->sc_report, sc->sc_reportlen,
                    hid_input, repid);
@@ -643,7 +643,7 @@ ihidev_intr(void *arg)
 
        iic_acquire_bus(sc->sc_tag, I2C_F_POLL);
        res = iic_exec(sc->sc_tag, I2C_OP_READ_WITH_STOP, sc->sc_addr, NULL, 0,
-           sc->sc_ibuf, letoh16(sc->hid_desc.wMaxInputLength), I2C_F_POLL);
+           sc->sc_ibuf, sc->sc_isize, I2C_F_POLL);
        iic_release_bus(sc->sc_tag, I2C_F_POLL);
 
        /*

Reply via email to