Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220118072628.1617172-1-dmitry.torokhov@gmail.com> <nycvar.YFH.7.76.2202021456020.11721@cbobk.fhfr.pm>
In-Reply-To: <nycvar.YFH.7.76.2202021456020.11721@cbobk.fhfr.pm>
From:   Benjamin Tissoires <benjamin.tissoires@redhat.com>
Date:   Wed, 2 Feb 2022 18:59:56 +0100
Message-ID: <CAO-hwJJ08vfMTEhU03VEef8Ejx=Ts+akUwGMKTUGqMWwOK3QoA@mail.gmail.com>
Subject: Re: [PATCH 00/12] i2c-hid: fixes for unnumbered reports and other improvements
To:     Jiri Kosina <jikos@kernel.org>
Cc:     Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        Angela Czubak <acz@semihalf.com>,
        "open list:HID CORE LAYER" <linux-input@vger.kernel.org>,
        lkml <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Feb 2, 2022 at 2:56 PM Jiri Kosina <jikos@kernel.org> wrote:
>
> On Mon, 17 Jan 2022, Dmitry Torokhov wrote:
>
> > Hi,
> >
> > This series came about after I reviewed Angela's patch that fixed issues
> > with incorrect handling of high-numbered reports (15 and above) in
> > i2c-hid driver:
> >
> > - it appears to me that the driver did not handle unnumbered reports
> >   correctly as the kernel internally expects unnumbered reports to be
> >   still prepended with report number 0, but i2c_hid_get_raw_report() and
> >   i2c_hid_output_raw_report() only expected report ID to be present for
> >   numbered reports.
> >
> > - the driver tried to consolidate both command handling and sending
> >   output reports in __i2c_hid_command() but the rules for different
> >   commands vary significantly and because of that the logic was hard to
> >   follow and it bled out from __i2c_hid_command() to callers. I decided
> >   to introduce a few simple helpers and have the data encoding for
> >   individual commands done at the call site. I believe this made it
> >   easier to validate the rules and the logic and allowed to remove
> >   special handling for the HID descriptor retrieval, among other things.
> >
> > - the driver does too many copying of data; encoding the data for
> >   commands at the call site allowed to only copy data once into the
> >   transfer buffers.
> >
> > I tested this on a couple of Chromebooks with i2c-hid touchscreens, but
> > it would be great if other folks tried it out as well.
>
> Benjamin,
>
> is this something you could feed into your testing machinery as well? (not
> sure whether it's handling i2c-hid specifics).

Not really. I mean I have a sample touchpad connected on an USB to I2C
bridge that I included in the automated tests (though it may be
failing in the latest release because I need to update the patch that
enables it), but it would probably not catch all the corner cases.

I can add this series to my local tree and test on my XPS with both
and Elan touchpad and a Wacom touchscreen/panel. It will still add a
few more tests :)

I'll try to report that tomorrow now that I think I fixed my tablet series.

Cheers,
Benjamin

>
> Thanks!
>
> --
> Jiri Kosina
> SUSE Labs
>