[PATCH v3 0/9] fix reset line polarity for Goodix touchscreen controllers
Hans de Goede
hdegoede at redhat.com
Mon Dec 5 14:19:17 PST 2022
Hi Quentin,
On 12/5/22 14:40, Quentin Schulz wrote:
> From: Quentin Schulz <quentin.schulz at theobroma-systems.com>
>
> 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.
I have tested this on a x86/ACPI device where we actually need to reset
the controller at boot to get it to work and things still work fine there
after this series.
I've also reviewd patches 1-3 and they look good to me to.
So for patches 1-3 you may add my:
Tested-by: Hans de Goede <hdegoede at redhat.com>
Reviewed-by: Hans de Goede <hdegoede at redhat.com>
Regards,
Hans
> 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 at hadess.net>
> To: Hans de Goede <hdegoede at redhat.com>
> To: Dmitry Torokhov <dmitry.torokhov at gmail.com>
> To: Rob Herring <robh+dt at kernel.org>
> To: Krzysztof Kozlowski <krzysztof.kozlowski+dt at linaro.org>
> To: Shawn Guo <shawnguo at kernel.org>
> To: Sascha Hauer <s.hauer at pengutronix.de>
> To: Pengutronix Kernel Team <kernel at pengutronix.de>
> To: Fabio Estevam <festevam at gmail.com>
> To: NXP Linux Team <linux-imx at nxp.com>
> To: Chen-Yu Tsai <wens at csie.org>
> To: Jernej Skrabec <jernej.skrabec at gmail.com>
> To: Samuel Holland <samuel at sholland.org>
> To: Andy Gross <agross at kernel.org>
> To: Bjorn Andersson <andersson at kernel.org>
> To: Konrad Dybcio <konrad.dybcio at somainline.org>
> To: Heiko Stuebner <heiko at sntech.de>
> To: David Jander <david at protonic.nl>
> To: Angus Ainslie <angus at akkea.ca>
> To: Peter Geis <pgwipeout at gmail.com>
> To: Michael Riesch <michael.riesch at wolfvision.net>
> To: Konrad Dybcio <konrad.dybcio at somainline.org>
> To: AngeloGioacchino Del Regno <angelogioacchino.delregno at somainline.org>
> To: Guido Günther <agx at sigxcpu.org>
> To: Jagan Teki <jagan at amarulasolutions.com>
> To: Ondrej Jirman <megous at megous.com>
> To: Icenowy Zheng <icenowy at aosc.io>
> To: Aleksei Mamlin <mamlinav at gmail.com>
> To: Lukasz Majewski <lukma at denx.de>
> To: Frieder Schrempf <frieder.schrempf at kontron.de>
> Cc: linux-input at vger.kernel.org
> Cc: linux-kernel at vger.kernel.org
> Cc: devicetree at vger.kernel.org
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-sunxi at lists.linux.dev
> Cc: linux-arm-msm at vger.kernel.org
> Cc: linux-rockchip at lists.infradead.org
> Signed-off-by: Quentin Schulz <quentin.schulz at theobroma-systems.com>
> ---
> 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@theobroma-systems.com
>
> 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@theobroma-systems.com
>
> ---
> 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,
More information about the Linux-rockchip
mailing list