[PATCH v3 0/9] fix reset line polarity for Goodix touchscreen controllers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Quentin Schulz <quentin.schulz@xxxxxxxxxxxxxxxxxxxxx>

The Goodix touchscreen controller has a reset line active low. It happens to
also be used to configure its i2c address at runtime. If the reset line is
incorrectly asserted, the address will be wrongly configured. This cost me a few
hours, trying to figure out why the touchscreen wouldn't work.

The driver is "asserting" this reset GPIO by setting its output to 0, probably
to reflect the physical state of the line. However, this relies on the fact that
the Device Tree node setting the reset line polarity to active high, which is
incorrect since the reset is active low in hardware.

To fix this inconsistency, the polarity is inverted to not confuse the user
about the reset line polarity. This obviously requires to fix the DT since most
users had the "incorrect" value in there, it needs to be inverted.
Note that the v2 highlighted that I was not the only one that got confused since
PRT8MM board has the "correct" HW representation for this line in DT (which does
not match what the driver was expecting).

This is marked as RFC because I can neither test ACPI support nor boards I don't
own. Please test on the boards you have that are impacted by this patchset and
give your Tested-By.
Do we also make this patch series only one patchset since the DT patches depend
on the driver patch and vice-versa? In which tree would this go?

Thanks,
Quentin

To: Bastien Nocera <hadess@xxxxxxxxxx>
To: Hans de Goede <hdegoede@xxxxxxxxxx>
To: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
To: Rob Herring <robh+dt@xxxxxxxxxx>
To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@xxxxxxxxxx>
To: Shawn Guo <shawnguo@xxxxxxxxxx>
To: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
To: Pengutronix Kernel Team <kernel@xxxxxxxxxxxxxx>
To: Fabio Estevam <festevam@xxxxxxxxx>
To: NXP Linux Team <linux-imx@xxxxxxx>
To: Chen-Yu Tsai <wens@xxxxxxxx>
To: Jernej Skrabec <jernej.skrabec@xxxxxxxxx>
To: Samuel Holland <samuel@xxxxxxxxxxxx>
To: Andy Gross <agross@xxxxxxxxxx>
To: Bjorn Andersson <andersson@xxxxxxxxxx>
To: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxx>
To: Heiko Stuebner <heiko@xxxxxxxxx>
To: David Jander <david@xxxxxxxxxxx>
To: Angus Ainslie <angus@xxxxxxxx>
To: Peter Geis <pgwipeout@xxxxxxxxx>
To: Michael Riesch <michael.riesch@xxxxxxxxxxxxxx>
To: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxx>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxxx>
To: Guido Günther <agx@xxxxxxxxxxx>
To: Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx>
To: Ondrej Jirman <megous@xxxxxxxxxx>
To: Icenowy Zheng <icenowy@xxxxxxx>
To: Aleksei Mamlin <mamlinav@xxxxxxxxx>
To: Lukasz Majewski <lukma@xxxxxxx>
To: Frieder Schrempf <frieder.schrempf@xxxxxxxxxx>
Cc: linux-input@xxxxxxxxxxxxxxx
Cc: linux-kernel@xxxxxxxxxxxxxxx
Cc: devicetree@xxxxxxxxxxxxxxx
Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
Cc: linux-sunxi@xxxxxxxxxxxxxxx
Cc: linux-arm-msm@xxxxxxxxxxxxxxx
Cc: linux-rockchip@xxxxxxxxxxxxxxxxxxx
Signed-off-by: Quentin Schulz <quentin.schulz@xxxxxxxxxxxxxxxxxxxxx>
---
Changes in v3:
- Cc'ing people who contributed to DTS of impacted boards,
- removed PRT8MM DTS change since it's been reported the polarity is actually
  correct (goes through an inverter), keeping the appropriate folks in Cc though
  since it'd be a good idea to check this patch series anyways,
- added ACPI_GPIO_QUIRK_NO_IO_RESTRICTION to acpi_gpio_mapping quirks to make
  gpiolib-acpi core respect GPIOD_ASIS flag in gpiod_get,
- checked schematics of:
  - pinephone: https://files.pine64.org/doc/PinePhone/PinePhone%20v1.2%20Released%20Schematic.pdf
  - pinetab: https://files.pine64.org/doc/PineTab/PineTab%20Schematic%20v1.2-20191125.pdf
  - px30 evb: https://opensource.rock-chips.com/images/d/db/Px30_mini_evb_v10_20180528.pdf
  - rockpro64: https://files.pine64.org/doc/rockpro64/rockpro64_v21-SCH.pdf
  - librem5 devkit: https://source.puri.sm/Librem5/dvk-mx8m-bsb/blob/master/dvk-mx8m-bsb.pdf

  All seems to be directly connected to the GPIO on the SoC side, without an
  inverter on the line.
- Link to v2: https://lore.kernel.org/r/20221103-upstream-goodix-reset-v2-0-2c38fb03a300@xxxxxxxxxxxxxxxxxxxxx

Changes in v2:
- implemented ACPI support as suggested by Hans,
- removed Qcom SC7180 Trogdor-based devices changes as they are not using this Goodix driver,
- added comment on how to read gpiod_request_output and the GPIO DT polarity,
- Link to v1: https://lore.kernel.org/r/20221103-upstream-goodix-reset-v1-0-87b49ae589f1@xxxxxxxxxxxxxxxxxxxxx

---
Quentin Schulz (9):
      Input: goodix - add macro for gpio mapping
      Input: goodix - make gpiod_get honor GPIOD_ASIS
      Input: goodix - fix reset polarity
      ARM: dts: imx: fix touchscreen reset GPIO polarity
      ARM: dts: sunxi: fix touchscreen reset GPIO polarity on Wexler TAB7200 tablet
      arm64: dts: allwinner: fix touchscreen reset GPIO polarity
      arm64: dts: librem5: fix touchscreen reset GPIO polarity
      arm64: dts: qcom: msm8998-fxtec: fix touchscreen reset GPIO polarity
      arm64: dts: rockchip: fix touchscreen reset GPIO polarity

 arch/arm/boot/dts/imx6q-kp.dtsi                    |  2 +-
 arch/arm/boot/dts/imx6ul-kontron-bl-43.dts         |  2 +-
 arch/arm/boot/dts/sun7i-a20-wexler-tab7200.dts     |  2 +-
 .../dts/allwinner/sun50i-a64-amarula-relic.dts     |  2 +-
 .../allwinner/sun50i-a64-oceanic-5205-5inmfd.dts   |  2 +-
 .../boot/dts/allwinner/sun50i-a64-pinephone.dtsi   |  2 +-
 .../boot/dts/allwinner/sun50i-a64-pinetab.dts      |  2 +-
 .../boot/dts/freescale/imx8mq-librem5-devkit.dts   |  2 +-
 arch/arm64/boot/dts/qcom/msm8998-fxtec-pro1.dts    |  2 +-
 arch/arm64/boot/dts/rockchip/px30-evb.dts          |  2 +-
 arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi |  2 +-
 arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts   |  2 +-
 drivers/input/touchscreen/goodix.c                 | 54 ++++++++++++++++++----
 13 files changed, 56 insertions(+), 22 deletions(-)
---
base-commit: 76dcd734eca23168cb008912c0f69ff408905235
change-id: 20221103-upstream-goodix-reset-aa1c65994f57

Best regards,
-- 
Quentin Schulz <quentin.schulz@xxxxxxxxxxxxxxxxxxxxx>



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux