mbox series

[v4,0/4] HID: i2c-hid: Reorganize to allow supporting goodix,gt7375p

Message ID 20201104012929.3850691-1-dianders@chromium.org
Headers show
Series HID: i2c-hid: Reorganize to allow supporting goodix,gt7375p | expand

Message

Douglas Anderson Nov. 4, 2020, 1:29 a.m. UTC
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 pretended to have a Goodix i2c-hid device on
  it.  I don't have a device with GT7375P setup yet and the person who
  has been testing remotely for me hasn't tested this exact series.  I
  think it should still work, though.
- NOT tested on any ACPI devices (just compile tested).

There are probably still small nits, but hopefully we're getting
closer to something people like.

Changes in v4:
- Fully rejigger so ACPI and OF are full subclasses.
- ("arm64: defconfig: Update config names for i2c-hid rejigger") new for v4.
- Totally redid based on the new subclass system.

Changes in v3:
- Rework to use subclassing.
- Removed Benjamin as a maintainer.
- Fixed compatible in example.
- Updated description.
- Rework to use subclassing.

Changes in v2:
- Use a separate compatible string for this new touchscreen.
- Get timings based on the compatible string.
- ("dt-bindings: HID: i2c-hid: Introduce bindings for the Goodix GT7375P") new in v2.

Douglas Anderson (4):
  HID: i2c-hid: Reorganize so ACPI and OF are subclasses
  arm64: defconfig: Update config names for i2c-hid rejigger
  dt-bindings: HID: i2c-hid: Introduce bindings for the Goodix GT7375P
  HID: i2c-hid: Introduce goodix-i2c-hid as a subclass of i2c-hid

 .../bindings/input/goodix,gt7375p.yaml        |  63 +++++
 arch/arm64/configs/defconfig                  |   3 +-
 drivers/hid/Makefile                          |   2 +-
 drivers/hid/i2c-hid/Kconfig                   |  47 +++-
 drivers/hid/i2c-hid/Makefile                  |   6 +-
 drivers/hid/i2c-hid/i2c-hid-acpi.c            | 167 ++++++++++++
 drivers/hid/i2c-hid/i2c-hid-core.c            | 253 +++---------------
 drivers/hid/i2c-hid/i2c-hid-of-goodix.c       | 120 +++++++++
 drivers/hid/i2c-hid/i2c-hid-of.c              | 149 +++++++++++
 drivers/hid/i2c-hid/i2c-hid.h                 |  24 ++
 include/linux/platform_data/i2c-hid.h         |  41 ---
 11 files changed, 607 insertions(+), 268 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
 create mode 100644 drivers/hid/i2c-hid/i2c-hid-acpi.c
 create mode 100644 drivers/hid/i2c-hid/i2c-hid-of-goodix.c
 create mode 100644 drivers/hid/i2c-hid/i2c-hid-of.c
 delete mode 100644 include/linux/platform_data/i2c-hid.h

Comments

Douglas Anderson Nov. 4, 2020, 4:10 p.m. UTC | #1
Hi,

On Wed, Nov 4, 2020 at 4:09 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 11/4/20 2:29 AM, Douglas Anderson wrote:
> > Goodix i2c-hid touchscreens are mostly i2c-hid compliant but have some
> > special power sequencing requirements, including the need to drive a
> > reset line during the sequencing.
> >
> > Let's use the new ability of i2c-hid to have subclasses for power
> > sequencing to support the first Goodix i2c-hid touchscreen: GT7375P
> >
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > ---
> >
> > Changes in v4:
> > - Totally redid based on the new subclass system.
>
> Again just my 2 cents, but I'm not sure if this should be an
> entirely separate driver, or just something added to the
> generic drivers/hid/i2c-hid/i2c-hid-of.c code.
>
> I have no real preference either way, just asking the
> question to make sure both options are considered.

Yeah, I thought about it.  ...but at the moment I'm not convinced it's
really any cleaner and I think there's very little shared code.  In
the goodix case, I don't want to specify the extra regulator or the
timings or even the hid descriptor address.  In the non-goodix case I
don't want the goodix properties.  It also sounded as if Benjamin's
preferred solutions involved having two separate files.  I'll wait for
Benjamin's feedback here, though, and if he wants me to combine them
then I'll give it a shot for v5.

-Doug

-Doug