Hi Quentin, On 12/5/22 14:40, Quentin Schulz wrote: > 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. 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@xxxxxxxxxx> Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx> 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@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,