Message-ID: <CAO-hwJLuzAccZbLSCvyP0JnkCW8fgswrm8RJfMaVKjtyF5Yg_A@mail.gmail.com>
Date:   Wed, 13 Jan 2021 16:08:41 +0100
From:   Benjamin Tissoires <benjamin.tissoires@...hat.com>
To:     Doug Anderson <dianders@...omium.org>
Cc:     Jiri Kosina <jkosina@...e.cz>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Hans de Goede <hdegoede@...hat.com>,
        "open list:HID CORE LAYER" <linux-input@...r.kernel.org>,
        Kai-Heng Feng <kai.heng.feng@...onical.com>,
        Rob Herring <robh+dt@...nel.org>,
        Stephen Boyd <swboyd@...omium.org>,
        Andrea Borgia <andrea@...gia.bo.it>,
        Anson Huang <Anson.Huang@....com>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Daniel Playfair Cal <daniel.playfair.cal@...il.com>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Guido Günther <agx@...xcpu.org>,
        Jiri Kosina <jikos@...nel.org>, Li Yang <leoyang.li@....com>,
        Masahiro Yamada <masahiroy@...nel.org>,
        Max Krummenacher <max.oss.09@...il.com>,
        Michael Walle <michael@...le.cc>,
        Pavel Balan <admin@...ma.net>, Shawn Guo <shawnguo@...nel.org>,
        Vinod Koul <vkoul@...nel.org>, Will Deacon <will@...nel.org>,
        Xiaofei Tan <tanxiaofei@...wei.com>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v8 0/4] HID: i2c-hid: Reorganize to allow supporting goodix,gt7375p

On Fri, Jan 8, 2021 at 6:52 PM Benjamin Tissoires
<benjamin.tissoires@...hat.com> wrote:
>
> Hi Doug,
>
> On Wed, Jan 6, 2021 at 2:35 AM Doug Anderson <dianders@...omium.org> wrote:
> >
> > Benjamin,
> >
> > On Fri, Dec 11, 2020 at 2:24 PM Douglas Anderson <dianders@...omium.org> wrote:
> > >
> > > The goal of this series is to support the Goodix GT7375P touchscreen.
> > > This touchscreen is special because it has power sequencing
> > > requirements that necessitate driving a reset GPIO.
> > >
> > > To do this, we totally rejigger the way i2c-hid is organized so that
> > > it's easier to jam the Goodix support in there.
> > >
> > > This series was:
> > > - Tested on a device that uses normal i2c-hid.
> > > - Tested on a device that has a Goodix i2c-hid device.
> > > - Tested on an ACPI device, but an earlier version of the series.
> > >
> > > I believe the plan is for Benjamin to land the whole series.  Will
> > > said this about the arm64 defconfig change (and provided his Ack):
> > > > ...there are a few things I really care about
> > > > in defconfig (e.g. things like page size!), generally speaking we don't
> > > > need to Ack everything that changes in there.
> > > >
> > > > That said, might be worth checking whether arm-soc have any defconfig
> > > > changes queued in -next so you don't end up with conflicts.
> > >
> > > Changes in v8:
> > > - Mark suspend/resume as static as per patches robot.
> > >
> > > Changes in v7:
> > > - Rebase atop commit afdd34c5fa40 ("HID: i2c-hid: show the error ...")
> > >
> > > Changes in v6:
> > > - ACPI probe function should have been "static"
> > > - Don't export suspend/resume, just export dev_pm_ops from core.
> > > - Fixed crash in ACPI module (missing init of "client")
> > > - No need for regulator include in the core.
> > > - Removed i2c_device_id table from ACPI module.
> > > - Suspend/resume are no longer exported from the core.
> > >
> > > Changes in v5:
> > > - Add shutdown_tail op and use it in ACPI.
> > > - Added mention of i2c-hid in the yaml itself as per Rob.
> > > - Adjusted subject as per Rob.
> > > - i2chid_subclass_data => i2chid_ops.
> > > - power_up_device => power_up (same with power_down).
> > > - subclass => ops.
> > >
> > > Changes in v4:
> > > - ("arm64: defconfig: Update config names for i2c-hid rejigger") new for v4.
> > > - Fully rejigger so ACPI and OF are full subclasses.
> > > - Totally redid based on the new subclass system.
> > >
> > > Changes in v3:
> > > - Fixed compatible in example.
> > > - Removed Benjamin as a maintainer.
> > > - Rework to use subclassing.
> > > - Updated description.
> > >
> > > Changes in v2:
> > > - ("dt-bindings: HID: i2c-hid: Introduce bindings for the Goodix GT7375P") new in v2.
> > > - Get timings based on the compatible string.
> > > - Use a separate compatible string for this new touchscreen.
> > >
> > > Douglas Anderson (4):
> > >   HID: i2c-hid: Reorganize so ACPI and OF are separate modules
> > >   arm64: defconfig: Update config names for i2c-hid rejigger
> > >   dt-bindings: input: HID: i2c-hid: Introduce bindings for the Goodix
> > >     GT7375P
> > >   HID: i2c-hid: Introduce goodix-i2c-hid using i2c-hid core
> >
> > I think this series is ready to land.  The "defconfig" has a trivial
> > conflict with commit 74b87103b3d0 ("arm64: defconfig: Enable HID
> > multitouch") against linuxnext, but it's so simple that hopefully
> > folks will be OK with that when it lands.
> >
> > Please let me know if there's anything else you need me to do.  :-)
> >
>
> I wanted to apply the series yesterday, but for these kinds of changes
> I like giving it a spin on actual hardware. Turns out that my XPS-13
> can not boot to v5.11-rc2, which makes testing the new branch slightly
> more difficult.
>
> I'll give it a spin next week, but I think I should be able to land it for 5.12.
>
> Regarding the defconfig conflict, no worries, we can handle it with
> Stephen and Linus.
>

After 2 full kernel bisects (I messed up the first because I am an
idiot and inverted good and bad after the first reboot), I found my
culprit, and I was able to test the series today.

The series works fine regarding enumeration and removing of devices,
but it prevents my system from being suspended. If I rmmod
i2c-hid-acpi, suspend works fine, but if it is present, it immediately
comes back, which makes me think that something must be wrong.

I also just reverted the series and confirmed that suspend/resume now
works, meaning that patch 1/4 needs to be checked.

Cheers,
Benjamin

Powered by blists - more mailing lists