[PATCH RFC v2 0/7] fix reset line polarity for Goodix touchscreen controllers

[PATCH RFC v2 0/7] fix reset line polarity for Goodix touchscreen controllers

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@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 is marked as RFC because it breaks DT compatibility and I cannot test ACPI
support. 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?

I'm all ears if there's a better way to handle this. We could document this in
the DT binding but this kinda breaks the promise we make that the DT is not
bound to the driver implementation.

Thanks,
Quentin

To: Bastien Nocera <hadess@hadess.net>
To: Hans de Goede <hdegoede@redhat.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Rob Herring <robh+dt@kernel.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
To: Shawn Guo <shawnguo@kernel.org>
To: Sascha Hauer <s.hauer@pengutronix.de>
To: Pengutronix Kernel Team <kernel@pengutronix.de>
To: Fabio Estevam <festevam@gmail.com>
To: NXP Linux Team <linux-imx@nxp.com>
To: Chen-Yu Tsai <wens@csie.org>
To: Jernej Skrabec <jernej.skrabec@gmail.com>
To: Samuel Holland <samuel@sholland.org>
To: Andy Gross <agross@kernel.org>
To: Bjorn Andersson <andersson@kernel.org>
To: Konrad Dybcio <konrad.dybcio@somainline.org>
To: Heiko Stuebner <heiko@sntech.de>
Cc: linux-input@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-sunxi@lists.linux.dev
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-rockchip@lists.infradead.org
Signed-off-by: Quentin Schulz <quentin.schulz@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 (7):
      Input: goodix - fix reset polarity
      ARM: dts: imx: fix touchscreen reset GPIO polarity
      ARM: dts: sunxi: fix touchscreen reset GPIO polarity
      arm64: dts: allwinner: fix touchscreen reset GPIO polarity
      arm64: dts: imx: fix touchscreen reset GPIO polarity
      arm64: dts: qcom: 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 +-
 arch/arm64/boot/dts/freescale/imx8mm-prt8mm.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                 | 45 +++++++++++++++++-----
 14 files changed, 48 insertions(+), 23 deletions(-)
---
base-commit: 84368d882b9688bfac77ce48d33b1e20a4e4a787
change-id: 20221103-upstream-goodix-reset-aa1c65994f57

Best regards,
-- 
Quentin Schulz <quentin.schulz@theobroma-systems.com>

[PATCH RFC v2 0/7] fix reset line polarity for Goodix touchscreen controllers

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@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 is marked as RFC because it breaks DT compatibility and I cannot test ACPI
support. 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?

I'm all ears if there's a better way to handle this. We could document this in
the DT binding but this kinda breaks the promise we make that the DT is not
bound to the driver implementation.

Thanks,
Quentin

To: Bastien Nocera <hadess@hadess.net>
To: Hans de Goede <hdegoede@redhat.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Rob Herring <robh+dt@kernel.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
To: Shawn Guo <shawnguo@kernel.org>
To: Sascha Hauer <s.hauer@pengutronix.de>
To: Pengutronix Kernel Team <kernel@pengutronix.de>
To: Fabio Estevam <festevam@gmail.com>
To: NXP Linux Team <linux-imx@nxp.com>
To: Chen-Yu Tsai <wens@csie.org>
To: Jernej Skrabec <jernej.skrabec@gmail.com>
To: Samuel Holland <samuel@sholland.org>
To: Andy Gross <agross@kernel.org>
To: Bjorn Andersson <andersson@kernel.org>
To: Konrad Dybcio <konrad.dybcio@somainline.org>
To: Heiko Stuebner <heiko@sntech.de>
Cc: linux-input@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-sunxi@lists.linux.dev
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-rockchip@lists.infradead.org
Signed-off-by: Quentin Schulz <quentin.schulz@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 (7):
      Input: goodix - fix reset polarity
      ARM: dts: imx: fix touchscreen reset GPIO polarity
      ARM: dts: sunxi: fix touchscreen reset GPIO polarity
      arm64: dts: allwinner: fix touchscreen reset GPIO polarity
      arm64: dts: imx: fix touchscreen reset GPIO polarity
      arm64: dts: qcom: 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 +-
 arch/arm64/boot/dts/freescale/imx8mm-prt8mm.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                 | 45 +++++++++++++++++-----
 14 files changed, 48 insertions(+), 23 deletions(-)
---
base-commit: 84368d882b9688bfac77ce48d33b1e20a4e4a787
change-id: 20221103-upstream-goodix-reset-aa1c65994f57

Best regards,
-- 
Quentin Schulz <quentin.schulz@theobroma-systems.com>

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH RFC v2 0/7] fix reset line polarity for Goodix touchscreen controllers

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@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 is marked as RFC because it breaks DT compatibility and I cannot test ACPI
support. 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?

I'm all ears if there's a better way to handle this. We could document this in
the DT binding but this kinda breaks the promise we make that the DT is not
bound to the driver implementation.

Thanks,
Quentin

To: Bastien Nocera <hadess@hadess.net>
To: Hans de Goede <hdegoede@redhat.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Rob Herring <robh+dt@kernel.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
To: Shawn Guo <shawnguo@kernel.org>
To: Sascha Hauer <s.hauer@pengutronix.de>
To: Pengutronix Kernel Team <kernel@pengutronix.de>
To: Fabio Estevam <festevam@gmail.com>
To: NXP Linux Team <linux-imx@nxp.com>
To: Chen-Yu Tsai <wens@csie.org>
To: Jernej Skrabec <jernej.skrabec@gmail.com>
To: Samuel Holland <samuel@sholland.org>
To: Andy Gross <agross@kernel.org>
To: Bjorn Andersson <andersson@kernel.org>
To: Konrad Dybcio <konrad.dybcio@somainline.org>
To: Heiko Stuebner <heiko@sntech.de>
Cc: linux-input@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-sunxi@lists.linux.dev
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-rockchip@lists.infradead.org
Signed-off-by: Quentin Schulz <quentin.schulz@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 (7):
      Input: goodix - fix reset polarity
      ARM: dts: imx: fix touchscreen reset GPIO polarity
      ARM: dts: sunxi: fix touchscreen reset GPIO polarity
      arm64: dts: allwinner: fix touchscreen reset GPIO polarity
      arm64: dts: imx: fix touchscreen reset GPIO polarity
      arm64: dts: qcom: 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 +-
 arch/arm64/boot/dts/freescale/imx8mm-prt8mm.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                 | 45 +++++++++++++++++-----
 14 files changed, 48 insertions(+), 23 deletions(-)
---
base-commit: 84368d882b9688bfac77ce48d33b1e20a4e4a787
change-id: 20221103-upstream-goodix-reset-aa1c65994f57

Best regards,
-- 
Quentin Schulz <quentin.schulz@theobroma-systems.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH RFC v2 1/7] Input: goodix - fix reset polarity

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

The reset line is asserted for selecting the I2C target address and then
deasserted.

This inverted logic works because the boards using this touchscreen
controller also invert the polarity of their reset GPIO.

Instead of depending on this double-inversion of meaning, let's *assert*
the line.

Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
Changes in v2:
- implemented ACPI support as suggested by Hans,
- added comment on how to read gpiod_request_output and the GPIO DT polarity,

 drivers/input/touchscreen/goodix.c | 45 +++++++++++++++++++++++++++++---------
 1 file changed, 35 insertions(+), 10 deletions(-)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index a33cc7950cf5b..da10cbb6f8264 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -742,8 +742,25 @@ int goodix_reset_no_int_sync(struct goodix_ts_data *ts)
 {
 	int error;
 
-	/* begin select I2C slave addr */
-	error = gpiod_direction_output(ts->gpiod_rst, 0);
+	/*
+	 * begin select I2C slave addr by activating/asserting RESET.
+	 *
+	 * The value passed to gpiod_direction_output is decorrelated from the
+	 * actual physical state of the line. The 1 value here is just to
+	 * specify the *assertion* of the line, its meaning being dependent on
+	 * the HW design of the system.
+	 *
+	 * DT-based systems need to specify the GPIO level in which the reset is
+	 * active. Since the touchscreen controller is in reset when its RESET
+	 * line is low, it is the level of the GPIO that results in the RESET
+	 * pin on the touchscreen controller side being low. In a HW design in
+	 * which the GPIO is directly connected to the touchscreen controller
+	 * RESET pin, this would be GPIO_ACTIVE_LOW.
+	 *
+	 * ACPI systems do not have the ability to specify the level of the GPIO
+	 * and they are therefore all assumed active low.
+	 */
+	error = gpiod_direction_output(ts->gpiod_rst, 1);
 	if (error)
 		goto error;
 
@@ -756,7 +773,8 @@ int goodix_reset_no_int_sync(struct goodix_ts_data *ts)
 
 	usleep_range(100, 2000);		/* T3: > 100us */
 
-	error = gpiod_direction_output(ts->gpiod_rst, 1);
+	/* Disable/de-assert RESET */
+	error = gpiod_direction_output(ts->gpiod_rst, 0);
 	if (error)
 		goto error;
 
@@ -797,23 +815,30 @@ static int goodix_reset(struct goodix_ts_data *ts)
 }
 
 #ifdef ACPI_GPIO_SUPPORT
-static const struct acpi_gpio_params first_gpio = { 0, 0, false };
-static const struct acpi_gpio_params second_gpio = { 1, 0, false };
+static const struct acpi_gpio_params int_first_gpio = { 0, 0, false };
+static const struct acpi_gpio_params int_second_gpio = { 1, 0, false };
+
+/*
+ * The controller is in reset when the RESET GPIO is output low, so
+ * set acpi_gpio_params.active_low appropriately.
+ */
+static const struct acpi_gpio_params rst_first_gpio = { 0, 0, true };
+static const struct acpi_gpio_params rst_second_gpio = { 1, 0, true };
 
 static const struct acpi_gpio_mapping acpi_goodix_int_first_gpios[] = {
-	{ GOODIX_GPIO_INT_NAME "-gpios", &first_gpio, 1 },
-	{ GOODIX_GPIO_RST_NAME "-gpios", &second_gpio, 1 },
+	{ GOODIX_GPIO_INT_NAME "-gpios", &int_first_gpio, 1 },
+	{ GOODIX_GPIO_RST_NAME "-gpios", &rst_second_gpio, 1 },
 	{ },
 };
 
 static const struct acpi_gpio_mapping acpi_goodix_int_last_gpios[] = {
-	{ GOODIX_GPIO_RST_NAME "-gpios", &first_gpio, 1 },
-	{ GOODIX_GPIO_INT_NAME "-gpios", &second_gpio, 1 },
+	{ GOODIX_GPIO_RST_NAME "-gpios", &rst_first_gpio, 1 },
+	{ GOODIX_GPIO_INT_NAME "-gpios", &int_second_gpio, 1 },
 	{ },
 };
 
 static const struct acpi_gpio_mapping acpi_goodix_reset_only_gpios[] = {
-	{ GOODIX_GPIO_RST_NAME "-gpios", &first_gpio, 1 },
+	{ GOODIX_GPIO_RST_NAME "-gpios", &rst_first_gpio, 1 },
 	{ },
 };
 

-- 
b4 0.10.1

[PATCH RFC v2 1/7] Input: goodix - fix reset polarity

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

The reset line is asserted for selecting the I2C target address and then
deasserted.

This inverted logic works because the boards using this touchscreen
controller also invert the polarity of their reset GPIO.

Instead of depending on this double-inversion of meaning, let's *assert*
the line.

Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
Changes in v2:
- implemented ACPI support as suggested by Hans,
- added comment on how to read gpiod_request_output and the GPIO DT polarity,

 drivers/input/touchscreen/goodix.c | 45 +++++++++++++++++++++++++++++---------
 1 file changed, 35 insertions(+), 10 deletions(-)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index a33cc7950cf5b..da10cbb6f8264 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -742,8 +742,25 @@ int goodix_reset_no_int_sync(struct goodix_ts_data *ts)
 {
 	int error;
 
-	/* begin select I2C slave addr */
-	error = gpiod_direction_output(ts->gpiod_rst, 0);
+	/*
+	 * begin select I2C slave addr by activating/asserting RESET.
+	 *
+	 * The value passed to gpiod_direction_output is decorrelated from the
+	 * actual physical state of the line. The 1 value here is just to
+	 * specify the *assertion* of the line, its meaning being dependent on
+	 * the HW design of the system.
+	 *
+	 * DT-based systems need to specify the GPIO level in which the reset is
+	 * active. Since the touchscreen controller is in reset when its RESET
+	 * line is low, it is the level of the GPIO that results in the RESET
+	 * pin on the touchscreen controller side being low. In a HW design in
+	 * which the GPIO is directly connected to the touchscreen controller
+	 * RESET pin, this would be GPIO_ACTIVE_LOW.
+	 *
+	 * ACPI systems do not have the ability to specify the level of the GPIO
+	 * and they are therefore all assumed active low.
+	 */
+	error = gpiod_direction_output(ts->gpiod_rst, 1);
 	if (error)
 		goto error;
 
@@ -756,7 +773,8 @@ int goodix_reset_no_int_sync(struct goodix_ts_data *ts)
 
 	usleep_range(100, 2000);		/* T3: > 100us */
 
-	error = gpiod_direction_output(ts->gpiod_rst, 1);
+	/* Disable/de-assert RESET */
+	error = gpiod_direction_output(ts->gpiod_rst, 0);
 	if (error)
 		goto error;
 
@@ -797,23 +815,30 @@ static int goodix_reset(struct goodix_ts_data *ts)
 }
 
 #ifdef ACPI_GPIO_SUPPORT
-static const struct acpi_gpio_params first_gpio = { 0, 0, false };
-static const struct acpi_gpio_params second_gpio = { 1, 0, false };
+static const struct acpi_gpio_params int_first_gpio = { 0, 0, false };
+static const struct acpi_gpio_params int_second_gpio = { 1, 0, false };
+
+/*
+ * The controller is in reset when the RESET GPIO is output low, so
+ * set acpi_gpio_params.active_low appropriately.
+ */
+static const struct acpi_gpio_params rst_first_gpio = { 0, 0, true };
+static const struct acpi_gpio_params rst_second_gpio = { 1, 0, true };
 
 static const struct acpi_gpio_mapping acpi_goodix_int_first_gpios[] = {
-	{ GOODIX_GPIO_INT_NAME "-gpios", &first_gpio, 1 },
-	{ GOODIX_GPIO_RST_NAME "-gpios", &second_gpio, 1 },
+	{ GOODIX_GPIO_INT_NAME "-gpios", &int_first_gpio, 1 },
+	{ GOODIX_GPIO_RST_NAME "-gpios", &rst_second_gpio, 1 },
 	{ },
 };
 
 static const struct acpi_gpio_mapping acpi_goodix_int_last_gpios[] = {
-	{ GOODIX_GPIO_RST_NAME "-gpios", &first_gpio, 1 },
-	{ GOODIX_GPIO_INT_NAME "-gpios", &second_gpio, 1 },
+	{ GOODIX_GPIO_RST_NAME "-gpios", &rst_first_gpio, 1 },
+	{ GOODIX_GPIO_INT_NAME "-gpios", &int_second_gpio, 1 },
 	{ },
 };
 
 static const struct acpi_gpio_mapping acpi_goodix_reset_only_gpios[] = {
-	{ GOODIX_GPIO_RST_NAME "-gpios", &first_gpio, 1 },
+	{ GOODIX_GPIO_RST_NAME "-gpios", &rst_first_gpio, 1 },
 	{ },
 };
 

-- 
b4 0.10.1

[PATCH RFC v2 1/7] Input: goodix - fix reset polarity

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

The reset line is asserted for selecting the I2C target address and then
deasserted.

This inverted logic works because the boards using this touchscreen
controller also invert the polarity of their reset GPIO.

Instead of depending on this double-inversion of meaning, let's *assert*
the line.

Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
Changes in v2:
- implemented ACPI support as suggested by Hans,
- added comment on how to read gpiod_request_output and the GPIO DT polarity,

 drivers/input/touchscreen/goodix.c | 45 +++++++++++++++++++++++++++++---------
 1 file changed, 35 insertions(+), 10 deletions(-)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index a33cc7950cf5b..da10cbb6f8264 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -742,8 +742,25 @@ int goodix_reset_no_int_sync(struct goodix_ts_data *ts)
 {
 	int error;
 
-	/* begin select I2C slave addr */
-	error = gpiod_direction_output(ts->gpiod_rst, 0);
+	/*
+	 * begin select I2C slave addr by activating/asserting RESET.
+	 *
+	 * The value passed to gpiod_direction_output is decorrelated from the
+	 * actual physical state of the line. The 1 value here is just to
+	 * specify the *assertion* of the line, its meaning being dependent on
+	 * the HW design of the system.
+	 *
+	 * DT-based systems need to specify the GPIO level in which the reset is
+	 * active. Since the touchscreen controller is in reset when its RESET
+	 * line is low, it is the level of the GPIO that results in the RESET
+	 * pin on the touchscreen controller side being low. In a HW design in
+	 * which the GPIO is directly connected to the touchscreen controller
+	 * RESET pin, this would be GPIO_ACTIVE_LOW.
+	 *
+	 * ACPI systems do not have the ability to specify the level of the GPIO
+	 * and they are therefore all assumed active low.
+	 */
+	error = gpiod_direction_output(ts->gpiod_rst, 1);
 	if (error)
 		goto error;
 
@@ -756,7 +773,8 @@ int goodix_reset_no_int_sync(struct goodix_ts_data *ts)
 
 	usleep_range(100, 2000);		/* T3: > 100us */
 
-	error = gpiod_direction_output(ts->gpiod_rst, 1);
+	/* Disable/de-assert RESET */
+	error = gpiod_direction_output(ts->gpiod_rst, 0);
 	if (error)
 		goto error;
 
@@ -797,23 +815,30 @@ static int goodix_reset(struct goodix_ts_data *ts)
 }
 
 #ifdef ACPI_GPIO_SUPPORT
-static const struct acpi_gpio_params first_gpio = { 0, 0, false };
-static const struct acpi_gpio_params second_gpio = { 1, 0, false };
+static const struct acpi_gpio_params int_first_gpio = { 0, 0, false };
+static const struct acpi_gpio_params int_second_gpio = { 1, 0, false };
+
+/*
+ * The controller is in reset when the RESET GPIO is output low, so
+ * set acpi_gpio_params.active_low appropriately.
+ */
+static const struct acpi_gpio_params rst_first_gpio = { 0, 0, true };
+static const struct acpi_gpio_params rst_second_gpio = { 1, 0, true };
 
 static const struct acpi_gpio_mapping acpi_goodix_int_first_gpios[] = {
-	{ GOODIX_GPIO_INT_NAME "-gpios", &first_gpio, 1 },
-	{ GOODIX_GPIO_RST_NAME "-gpios", &second_gpio, 1 },
+	{ GOODIX_GPIO_INT_NAME "-gpios", &int_first_gpio, 1 },
+	{ GOODIX_GPIO_RST_NAME "-gpios", &rst_second_gpio, 1 },
 	{ },
 };
 
 static const struct acpi_gpio_mapping acpi_goodix_int_last_gpios[] = {
-	{ GOODIX_GPIO_RST_NAME "-gpios", &first_gpio, 1 },
-	{ GOODIX_GPIO_INT_NAME "-gpios", &second_gpio, 1 },
+	{ GOODIX_GPIO_RST_NAME "-gpios", &rst_first_gpio, 1 },
+	{ GOODIX_GPIO_INT_NAME "-gpios", &int_second_gpio, 1 },
 	{ },
 };
 
 static const struct acpi_gpio_mapping acpi_goodix_reset_only_gpios[] = {
-	{ GOODIX_GPIO_RST_NAME "-gpios", &first_gpio, 1 },
+	{ GOODIX_GPIO_RST_NAME "-gpios", &rst_first_gpio, 1 },
 	{ },
 };
 

-- 
b4 0.10.1

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH RFC v2 1/7] Input: goodix - fix reset polarity

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

The reset line is asserted for selecting the I2C target address and then
deasserted.

This inverted logic works because the boards using this touchscreen
controller also invert the polarity of their reset GPIO.

Instead of depending on this double-inversion of meaning, let's *assert*
the line.

Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
Changes in v2:
- implemented ACPI support as suggested by Hans,
- added comment on how to read gpiod_request_output and the GPIO DT polarity,

 drivers/input/touchscreen/goodix.c | 45 +++++++++++++++++++++++++++++---------
 1 file changed, 35 insertions(+), 10 deletions(-)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index a33cc7950cf5b..da10cbb6f8264 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -742,8 +742,25 @@ int goodix_reset_no_int_sync(struct goodix_ts_data *ts)
 {
 	int error;
 
-	/* begin select I2C slave addr */
-	error = gpiod_direction_output(ts->gpiod_rst, 0);
+	/*
+	 * begin select I2C slave addr by activating/asserting RESET.
+	 *
+	 * The value passed to gpiod_direction_output is decorrelated from the
+	 * actual physical state of the line. The 1 value here is just to
+	 * specify the *assertion* of the line, its meaning being dependent on
+	 * the HW design of the system.
+	 *
+	 * DT-based systems need to specify the GPIO level in which the reset is
+	 * active. Since the touchscreen controller is in reset when its RESET
+	 * line is low, it is the level of the GPIO that results in the RESET
+	 * pin on the touchscreen controller side being low. In a HW design in
+	 * which the GPIO is directly connected to the touchscreen controller
+	 * RESET pin, this would be GPIO_ACTIVE_LOW.
+	 *
+	 * ACPI systems do not have the ability to specify the level of the GPIO
+	 * and they are therefore all assumed active low.
+	 */
+	error = gpiod_direction_output(ts->gpiod_rst, 1);
 	if (error)
 		goto error;
 
@@ -756,7 +773,8 @@ int goodix_reset_no_int_sync(struct goodix_ts_data *ts)
 
 	usleep_range(100, 2000);		/* T3: > 100us */
 
-	error = gpiod_direction_output(ts->gpiod_rst, 1);
+	/* Disable/de-assert RESET */
+	error = gpiod_direction_output(ts->gpiod_rst, 0);
 	if (error)
 		goto error;
 
@@ -797,23 +815,30 @@ static int goodix_reset(struct goodix_ts_data *ts)
 }
 
 #ifdef ACPI_GPIO_SUPPORT
-static const struct acpi_gpio_params first_gpio = { 0, 0, false };
-static const struct acpi_gpio_params second_gpio = { 1, 0, false };
+static const struct acpi_gpio_params int_first_gpio = { 0, 0, false };
+static const struct acpi_gpio_params int_second_gpio = { 1, 0, false };
+
+/*
+ * The controller is in reset when the RESET GPIO is output low, so
+ * set acpi_gpio_params.active_low appropriately.
+ */
+static const struct acpi_gpio_params rst_first_gpio = { 0, 0, true };
+static const struct acpi_gpio_params rst_second_gpio = { 1, 0, true };
 
 static const struct acpi_gpio_mapping acpi_goodix_int_first_gpios[] = {
-	{ GOODIX_GPIO_INT_NAME "-gpios", &first_gpio, 1 },
-	{ GOODIX_GPIO_RST_NAME "-gpios", &second_gpio, 1 },
+	{ GOODIX_GPIO_INT_NAME "-gpios", &int_first_gpio, 1 },
+	{ GOODIX_GPIO_RST_NAME "-gpios", &rst_second_gpio, 1 },
 	{ },
 };
 
 static const struct acpi_gpio_mapping acpi_goodix_int_last_gpios[] = {
-	{ GOODIX_GPIO_RST_NAME "-gpios", &first_gpio, 1 },
-	{ GOODIX_GPIO_INT_NAME "-gpios", &second_gpio, 1 },
+	{ GOODIX_GPIO_RST_NAME "-gpios", &rst_first_gpio, 1 },
+	{ GOODIX_GPIO_INT_NAME "-gpios", &int_second_gpio, 1 },
 	{ },
 };
 
 static const struct acpi_gpio_mapping acpi_goodix_reset_only_gpios[] = {
-	{ GOODIX_GPIO_RST_NAME "-gpios", &first_gpio, 1 },
+	{ GOODIX_GPIO_RST_NAME "-gpios", &rst_first_gpio, 1 },
 	{ },
 };
 

-- 
b4 0.10.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH RFC v2 2/7] ARM: dts: imx: fix touchscreen reset GPIO polarity

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

The reset line is active low for the Goodix touchscreen controller so
let's fix the polarity in the Device Tree node.

Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
 arch/arm/boot/dts/imx6q-kp.dtsi            | 2 +-
 arch/arm/boot/dts/imx6ul-kontron-bl-43.dts | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/imx6q-kp.dtsi b/arch/arm/boot/dts/imx6q-kp.dtsi
index 1ade0bff681d6..dae14aaf803a8 100644
--- a/arch/arm/boot/dts/imx6q-kp.dtsi
+++ b/arch/arm/boot/dts/imx6q-kp.dtsi
@@ -188,7 +188,7 @@ touchscreen@5d {
 		interrupt-parent = <&gpio1>;
 		interrupts = <9 IRQ_TYPE_EDGE_FALLING>;
 		irq-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
-		reset-gpios = <&gpio5 2 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio5 2 GPIO_ACTIVE_LOW>;
 	};
 
 	ds1307: rtc@32 {
diff --git a/arch/arm/boot/dts/imx6ul-kontron-bl-43.dts b/arch/arm/boot/dts/imx6ul-kontron-bl-43.dts
index 0c643706a158b..767ef5da76136 100644
--- a/arch/arm/boot/dts/imx6ul-kontron-bl-43.dts
+++ b/arch/arm/boot/dts/imx6ul-kontron-bl-43.dts
@@ -29,7 +29,7 @@ touchscreen@5d {
 		pinctrl-0 = <&pinctrl_cap_touch>;
 		interrupt-parent = <&gpio5>;
 		interrupts = <6 IRQ_TYPE_LEVEL_LOW>;
-		reset-gpios = <&gpio5 8 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio5 8 GPIO_ACTIVE_LOW>;
 		irq-gpios = <&gpio5 6 GPIO_ACTIVE_HIGH>;
 	};
 };

-- 
b4 0.10.1

[PATCH RFC v2 2/7] ARM: dts: imx: fix touchscreen reset GPIO polarity

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

The reset line is active low for the Goodix touchscreen controller so
let's fix the polarity in the Device Tree node.

Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
 arch/arm/boot/dts/imx6q-kp.dtsi            | 2 +-
 arch/arm/boot/dts/imx6ul-kontron-bl-43.dts | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/imx6q-kp.dtsi b/arch/arm/boot/dts/imx6q-kp.dtsi
index 1ade0bff681d6..dae14aaf803a8 100644
--- a/arch/arm/boot/dts/imx6q-kp.dtsi
+++ b/arch/arm/boot/dts/imx6q-kp.dtsi
@@ -188,7 +188,7 @@ touchscreen@5d {
 		interrupt-parent = <&gpio1>;
 		interrupts = <9 IRQ_TYPE_EDGE_FALLING>;
 		irq-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
-		reset-gpios = <&gpio5 2 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio5 2 GPIO_ACTIVE_LOW>;
 	};
 
 	ds1307: rtc@32 {
diff --git a/arch/arm/boot/dts/imx6ul-kontron-bl-43.dts b/arch/arm/boot/dts/imx6ul-kontron-bl-43.dts
index 0c643706a158b..767ef5da76136 100644
--- a/arch/arm/boot/dts/imx6ul-kontron-bl-43.dts
+++ b/arch/arm/boot/dts/imx6ul-kontron-bl-43.dts
@@ -29,7 +29,7 @@ touchscreen@5d {
 		pinctrl-0 = <&pinctrl_cap_touch>;
 		interrupt-parent = <&gpio5>;
 		interrupts = <6 IRQ_TYPE_LEVEL_LOW>;
-		reset-gpios = <&gpio5 8 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio5 8 GPIO_ACTIVE_LOW>;
 		irq-gpios = <&gpio5 6 GPIO_ACTIVE_HIGH>;
 	};
 };

-- 
b4 0.10.1

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH RFC v2 2/7] ARM: dts: imx: fix touchscreen reset GPIO polarity

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

The reset line is active low for the Goodix touchscreen controller so
let's fix the polarity in the Device Tree node.

Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
 arch/arm/boot/dts/imx6q-kp.dtsi            | 2 +-
 arch/arm/boot/dts/imx6ul-kontron-bl-43.dts | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/imx6q-kp.dtsi b/arch/arm/boot/dts/imx6q-kp.dtsi
index 1ade0bff681d6..dae14aaf803a8 100644
--- a/arch/arm/boot/dts/imx6q-kp.dtsi
+++ b/arch/arm/boot/dts/imx6q-kp.dtsi
@@ -188,7 +188,7 @@ touchscreen@5d {
 		interrupt-parent = <&gpio1>;
 		interrupts = <9 IRQ_TYPE_EDGE_FALLING>;
 		irq-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
-		reset-gpios = <&gpio5 2 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio5 2 GPIO_ACTIVE_LOW>;
 	};
 
 	ds1307: rtc@32 {
diff --git a/arch/arm/boot/dts/imx6ul-kontron-bl-43.dts b/arch/arm/boot/dts/imx6ul-kontron-bl-43.dts
index 0c643706a158b..767ef5da76136 100644
--- a/arch/arm/boot/dts/imx6ul-kontron-bl-43.dts
+++ b/arch/arm/boot/dts/imx6ul-kontron-bl-43.dts
@@ -29,7 +29,7 @@ touchscreen@5d {
 		pinctrl-0 = <&pinctrl_cap_touch>;
 		interrupt-parent = <&gpio5>;
 		interrupts = <6 IRQ_TYPE_LEVEL_LOW>;
-		reset-gpios = <&gpio5 8 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio5 8 GPIO_ACTIVE_LOW>;
 		irq-gpios = <&gpio5 6 GPIO_ACTIVE_HIGH>;
 	};
 };

-- 
b4 0.10.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH RFC v2 3/7] ARM: dts: sunxi: fix touchscreen reset GPIO polarity

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

The reset line is active low for the Goodix touchscreen controller so
let's fix the polarity in the Device Tree node.

Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
 arch/arm/boot/dts/sun7i-a20-wexler-tab7200.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/sun7i-a20-wexler-tab7200.dts b/arch/arm/boot/dts/sun7i-a20-wexler-tab7200.dts
index fef02fcbbdf82..8c79ecdb40608 100644
--- a/arch/arm/boot/dts/sun7i-a20-wexler-tab7200.dts
+++ b/arch/arm/boot/dts/sun7i-a20-wexler-tab7200.dts
@@ -114,7 +114,7 @@ gt911: touchscreen@5d {
 		interrupt-parent = <&pio>;
 		interrupts = <7 21 IRQ_TYPE_EDGE_FALLING>; /* EINT21 (PH21) */
 		irq-gpios = <&pio 7 21 GPIO_ACTIVE_HIGH>; /* INT (PH21) */
-		reset-gpios = <&pio 1 13 GPIO_ACTIVE_HIGH>; /* RST (PB13) */
+		reset-gpios = <&pio 1 13 GPIO_ACTIVE_LOW>; /* RST (PB13) */
 		touchscreen-swapped-x-y;
 	};
 };

-- 
b4 0.10.1

[PATCH RFC v2 3/7] ARM: dts: sunxi: fix touchscreen reset GPIO polarity

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

The reset line is active low for the Goodix touchscreen controller so
let's fix the polarity in the Device Tree node.

Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
 arch/arm/boot/dts/sun7i-a20-wexler-tab7200.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/sun7i-a20-wexler-tab7200.dts b/arch/arm/boot/dts/sun7i-a20-wexler-tab7200.dts
index fef02fcbbdf82..8c79ecdb40608 100644
--- a/arch/arm/boot/dts/sun7i-a20-wexler-tab7200.dts
+++ b/arch/arm/boot/dts/sun7i-a20-wexler-tab7200.dts
@@ -114,7 +114,7 @@ gt911: touchscreen@5d {
 		interrupt-parent = <&pio>;
 		interrupts = <7 21 IRQ_TYPE_EDGE_FALLING>; /* EINT21 (PH21) */
 		irq-gpios = <&pio 7 21 GPIO_ACTIVE_HIGH>; /* INT (PH21) */
-		reset-gpios = <&pio 1 13 GPIO_ACTIVE_HIGH>; /* RST (PB13) */
+		reset-gpios = <&pio 1 13 GPIO_ACTIVE_LOW>; /* RST (PB13) */
 		touchscreen-swapped-x-y;
 	};
 };

-- 
b4 0.10.1

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH RFC v2 3/7] ARM: dts: sunxi: fix touchscreen reset GPIO polarity

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

The reset line is active low for the Goodix touchscreen controller so
let's fix the polarity in the Device Tree node.

Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
 arch/arm/boot/dts/sun7i-a20-wexler-tab7200.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/sun7i-a20-wexler-tab7200.dts b/arch/arm/boot/dts/sun7i-a20-wexler-tab7200.dts
index fef02fcbbdf82..8c79ecdb40608 100644
--- a/arch/arm/boot/dts/sun7i-a20-wexler-tab7200.dts
+++ b/arch/arm/boot/dts/sun7i-a20-wexler-tab7200.dts
@@ -114,7 +114,7 @@ gt911: touchscreen@5d {
 		interrupt-parent = <&pio>;
 		interrupts = <7 21 IRQ_TYPE_EDGE_FALLING>; /* EINT21 (PH21) */
 		irq-gpios = <&pio 7 21 GPIO_ACTIVE_HIGH>; /* INT (PH21) */
-		reset-gpios = <&pio 1 13 GPIO_ACTIVE_HIGH>; /* RST (PB13) */
+		reset-gpios = <&pio 1 13 GPIO_ACTIVE_LOW>; /* RST (PB13) */
 		touchscreen-swapped-x-y;
 	};
 };

-- 
b4 0.10.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH RFC v2 4/7] arm64: dts: allwinner: fix touchscreen reset GPIO polarity

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

The reset line is active low for the Goodix touchscreen controller so
let's fix the polarity in the Device Tree node.

Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts       | 2 +-
 arch/arm64/boot/dts/allwinner/sun50i-a64-oceanic-5205-5inmfd.dts | 2 +-
 arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi          | 2 +-
 arch/arm64/boot/dts/allwinner/sun50i-a64-pinetab.dts             | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
index 8233582f62881..5fd581037d987 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
@@ -122,7 +122,7 @@ touchscreen@5d {
 		interrupt-parent = <&pio>;
 		interrupts = <7 4 IRQ_TYPE_EDGE_FALLING>;
 		irq-gpios = <&pio 7 4 GPIO_ACTIVE_HIGH>;	/* CTP-INT: PH4 */
-		reset-gpios = <&pio 7 8 GPIO_ACTIVE_HIGH>;	/* CTP-RST: PH8 */
+		reset-gpios = <&pio 7 8 GPIO_ACTIVE_LOW>;	/* CTP-RST: PH8 */
 		touchscreen-inverted-x;
 		touchscreen-inverted-y;
 	};
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-oceanic-5205-5inmfd.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-oceanic-5205-5inmfd.dts
index 577f9e1d08a14..990f042f5a5b1 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-oceanic-5205-5inmfd.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-oceanic-5205-5inmfd.dts
@@ -45,7 +45,7 @@ touchscreen@5d {
 		interrupt-parent = <&pio>;
 		interrupts = <7 4 IRQ_TYPE_EDGE_FALLING>;
 		irq-gpios = <&pio 7 4 GPIO_ACTIVE_HIGH>;	/* CTP-INT: PH4 */
-		reset-gpios = <&pio 7 11 GPIO_ACTIVE_HIGH>;	/* CTP-RST: PH11 */
+		reset-gpios = <&pio 7 11 GPIO_ACTIVE_LOW>;	/* CTP-RST: PH11 */
 		touchscreen-inverted-x;
 		touchscreen-inverted-y;
 	};
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
index 87847116ab6d9..97359cc7f13e2 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
@@ -167,7 +167,7 @@ touchscreen@5d {
 		interrupt-parent = <&pio>;
 		interrupts = <7 4 IRQ_TYPE_LEVEL_HIGH>; /* PH4 */
 		irq-gpios = <&pio 7 4 GPIO_ACTIVE_HIGH>; /* PH4 */
-		reset-gpios = <&pio 7 11 GPIO_ACTIVE_HIGH>; /* PH11 */
+		reset-gpios = <&pio 7 11 GPIO_ACTIVE_LOW>; /* PH11 */
 		AVDD28-supply = <&reg_ldo_io0>;
 		VDDIO-supply = <&reg_ldo_io0>;
 		touchscreen-size-x = <720>;
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinetab.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinetab.dts
index 0a5607f73049e..c0eccc753e3f5 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinetab.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinetab.dts
@@ -189,7 +189,7 @@ touchscreen@5d {
 		interrupt-parent = <&pio>;
 		interrupts = <7 4 IRQ_TYPE_LEVEL_HIGH>; /* PH4 */
 		irq-gpios = <&pio 7 4 GPIO_ACTIVE_HIGH>; /* PH4 */
-		reset-gpios = <&pio 7 8 GPIO_ACTIVE_HIGH>; /* PH8 */
+		reset-gpios = <&pio 7 8 GPIO_ACTIVE_LOW>; /* PH8 */
 		AVDD28-supply = <&reg_ldo_io1>;
 	};
 };

-- 
b4 0.10.1

[PATCH RFC v2 4/7] arm64: dts: allwinner: fix touchscreen reset GPIO polarity

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

The reset line is active low for the Goodix touchscreen controller so
let's fix the polarity in the Device Tree node.

Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts       | 2 +-
 arch/arm64/boot/dts/allwinner/sun50i-a64-oceanic-5205-5inmfd.dts | 2 +-
 arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi          | 2 +-
 arch/arm64/boot/dts/allwinner/sun50i-a64-pinetab.dts             | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
index 8233582f62881..5fd581037d987 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
@@ -122,7 +122,7 @@ touchscreen@5d {
 		interrupt-parent = <&pio>;
 		interrupts = <7 4 IRQ_TYPE_EDGE_FALLING>;
 		irq-gpios = <&pio 7 4 GPIO_ACTIVE_HIGH>;	/* CTP-INT: PH4 */
-		reset-gpios = <&pio 7 8 GPIO_ACTIVE_HIGH>;	/* CTP-RST: PH8 */
+		reset-gpios = <&pio 7 8 GPIO_ACTIVE_LOW>;	/* CTP-RST: PH8 */
 		touchscreen-inverted-x;
 		touchscreen-inverted-y;
 	};
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-oceanic-5205-5inmfd.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-oceanic-5205-5inmfd.dts
index 577f9e1d08a14..990f042f5a5b1 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-oceanic-5205-5inmfd.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-oceanic-5205-5inmfd.dts
@@ -45,7 +45,7 @@ touchscreen@5d {
 		interrupt-parent = <&pio>;
 		interrupts = <7 4 IRQ_TYPE_EDGE_FALLING>;
 		irq-gpios = <&pio 7 4 GPIO_ACTIVE_HIGH>;	/* CTP-INT: PH4 */
-		reset-gpios = <&pio 7 11 GPIO_ACTIVE_HIGH>;	/* CTP-RST: PH11 */
+		reset-gpios = <&pio 7 11 GPIO_ACTIVE_LOW>;	/* CTP-RST: PH11 */
 		touchscreen-inverted-x;
 		touchscreen-inverted-y;
 	};
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
index 87847116ab6d9..97359cc7f13e2 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
@@ -167,7 +167,7 @@ touchscreen@5d {
 		interrupt-parent = <&pio>;
 		interrupts = <7 4 IRQ_TYPE_LEVEL_HIGH>; /* PH4 */
 		irq-gpios = <&pio 7 4 GPIO_ACTIVE_HIGH>; /* PH4 */
-		reset-gpios = <&pio 7 11 GPIO_ACTIVE_HIGH>; /* PH11 */
+		reset-gpios = <&pio 7 11 GPIO_ACTIVE_LOW>; /* PH11 */
 		AVDD28-supply = <&reg_ldo_io0>;
 		VDDIO-supply = <&reg_ldo_io0>;
 		touchscreen-size-x = <720>;
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinetab.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinetab.dts
index 0a5607f73049e..c0eccc753e3f5 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinetab.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinetab.dts
@@ -189,7 +189,7 @@ touchscreen@5d {
 		interrupt-parent = <&pio>;
 		interrupts = <7 4 IRQ_TYPE_LEVEL_HIGH>; /* PH4 */
 		irq-gpios = <&pio 7 4 GPIO_ACTIVE_HIGH>; /* PH4 */
-		reset-gpios = <&pio 7 8 GPIO_ACTIVE_HIGH>; /* PH8 */
+		reset-gpios = <&pio 7 8 GPIO_ACTIVE_LOW>; /* PH8 */
 		AVDD28-supply = <&reg_ldo_io1>;
 	};
 };

-- 
b4 0.10.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH RFC v2 4/7] arm64: dts: allwinner: fix touchscreen reset GPIO polarity

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

The reset line is active low for the Goodix touchscreen controller so
let's fix the polarity in the Device Tree node.

Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts       | 2 +-
 arch/arm64/boot/dts/allwinner/sun50i-a64-oceanic-5205-5inmfd.dts | 2 +-
 arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi          | 2 +-
 arch/arm64/boot/dts/allwinner/sun50i-a64-pinetab.dts             | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
index 8233582f62881..5fd581037d987 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
@@ -122,7 +122,7 @@ touchscreen@5d {
 		interrupt-parent = <&pio>;
 		interrupts = <7 4 IRQ_TYPE_EDGE_FALLING>;
 		irq-gpios = <&pio 7 4 GPIO_ACTIVE_HIGH>;	/* CTP-INT: PH4 */
-		reset-gpios = <&pio 7 8 GPIO_ACTIVE_HIGH>;	/* CTP-RST: PH8 */
+		reset-gpios = <&pio 7 8 GPIO_ACTIVE_LOW>;	/* CTP-RST: PH8 */
 		touchscreen-inverted-x;
 		touchscreen-inverted-y;
 	};
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-oceanic-5205-5inmfd.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-oceanic-5205-5inmfd.dts
index 577f9e1d08a14..990f042f5a5b1 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-oceanic-5205-5inmfd.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-oceanic-5205-5inmfd.dts
@@ -45,7 +45,7 @@ touchscreen@5d {
 		interrupt-parent = <&pio>;
 		interrupts = <7 4 IRQ_TYPE_EDGE_FALLING>;
 		irq-gpios = <&pio 7 4 GPIO_ACTIVE_HIGH>;	/* CTP-INT: PH4 */
-		reset-gpios = <&pio 7 11 GPIO_ACTIVE_HIGH>;	/* CTP-RST: PH11 */
+		reset-gpios = <&pio 7 11 GPIO_ACTIVE_LOW>;	/* CTP-RST: PH11 */
 		touchscreen-inverted-x;
 		touchscreen-inverted-y;
 	};
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
index 87847116ab6d9..97359cc7f13e2 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi
@@ -167,7 +167,7 @@ touchscreen@5d {
 		interrupt-parent = <&pio>;
 		interrupts = <7 4 IRQ_TYPE_LEVEL_HIGH>; /* PH4 */
 		irq-gpios = <&pio 7 4 GPIO_ACTIVE_HIGH>; /* PH4 */
-		reset-gpios = <&pio 7 11 GPIO_ACTIVE_HIGH>; /* PH11 */
+		reset-gpios = <&pio 7 11 GPIO_ACTIVE_LOW>; /* PH11 */
 		AVDD28-supply = <&reg_ldo_io0>;
 		VDDIO-supply = <&reg_ldo_io0>;
 		touchscreen-size-x = <720>;
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinetab.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinetab.dts
index 0a5607f73049e..c0eccc753e3f5 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pinetab.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pinetab.dts
@@ -189,7 +189,7 @@ touchscreen@5d {
 		interrupt-parent = <&pio>;
 		interrupts = <7 4 IRQ_TYPE_LEVEL_HIGH>; /* PH4 */
 		irq-gpios = <&pio 7 4 GPIO_ACTIVE_HIGH>; /* PH4 */
-		reset-gpios = <&pio 7 8 GPIO_ACTIVE_HIGH>; /* PH8 */
+		reset-gpios = <&pio 7 8 GPIO_ACTIVE_LOW>; /* PH8 */
 		AVDD28-supply = <&reg_ldo_io1>;
 	};
 };

-- 
b4 0.10.1

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH RFC v2 5/7] arm64: dts: imx: fix touchscreen reset GPIO polarity

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

The reset line is active low for the Goodix touchscreen controller so
let's fix the polarity in the Device Tree node.

Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
 arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts         | 2 +-
 arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
index 9fbbbb556c0b3..df7e5ae9698e1 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
@@ -107,7 +107,7 @@ touchscreeen@5d {
 		interrupt-parent = <&gpio1>;
 		interrupts = <8 IRQ_TYPE_NONE>;
 		irq-gpios = <&gpio1 8 GPIO_ACTIVE_HIGH>;
-		reset-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;
 	};
 
 	temp-sense@70 {
diff --git a/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts b/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
index 6445c6b90b5bb..b038300812b1e 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
@@ -542,7 +542,7 @@ touchscreen@5d {
 		pinctrl-0 = <&pinctrl_ts>;
 		interrupt-parent = <&gpio3>;
 		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
-		reset-gpios = <&gpio1 5 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio1 5 GPIO_ACTIVE_LOW>;
 		irq-gpios = <&gpio3 0 GPIO_ACTIVE_HIGH>;
 		touchscreen-size-x = <720>;
 		touchscreen-size-y = <1440>;

-- 
b4 0.10.1

[PATCH RFC v2 5/7] arm64: dts: imx: fix touchscreen reset GPIO polarity

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

The reset line is active low for the Goodix touchscreen controller so
let's fix the polarity in the Device Tree node.

Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
 arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts         | 2 +-
 arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
index 9fbbbb556c0b3..df7e5ae9698e1 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
@@ -107,7 +107,7 @@ touchscreeen@5d {
 		interrupt-parent = <&gpio1>;
 		interrupts = <8 IRQ_TYPE_NONE>;
 		irq-gpios = <&gpio1 8 GPIO_ACTIVE_HIGH>;
-		reset-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;
 	};
 
 	temp-sense@70 {
diff --git a/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts b/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
index 6445c6b90b5bb..b038300812b1e 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
@@ -542,7 +542,7 @@ touchscreen@5d {
 		pinctrl-0 = <&pinctrl_ts>;
 		interrupt-parent = <&gpio3>;
 		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
-		reset-gpios = <&gpio1 5 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio1 5 GPIO_ACTIVE_LOW>;
 		irq-gpios = <&gpio3 0 GPIO_ACTIVE_HIGH>;
 		touchscreen-size-x = <720>;
 		touchscreen-size-y = <1440>;

-- 
b4 0.10.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH RFC v2 5/7] arm64: dts: imx: fix touchscreen reset GPIO polarity

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

The reset line is active low for the Goodix touchscreen controller so
let's fix the polarity in the Device Tree node.

Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
 arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts         | 2 +-
 arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
index 9fbbbb556c0b3..df7e5ae9698e1 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
@@ -107,7 +107,7 @@ touchscreeen@5d {
 		interrupt-parent = <&gpio1>;
 		interrupts = <8 IRQ_TYPE_NONE>;
 		irq-gpios = <&gpio1 8 GPIO_ACTIVE_HIGH>;
-		reset-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;
 	};
 
 	temp-sense@70 {
diff --git a/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts b/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
index 6445c6b90b5bb..b038300812b1e 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
@@ -542,7 +542,7 @@ touchscreen@5d {
 		pinctrl-0 = <&pinctrl_ts>;
 		interrupt-parent = <&gpio3>;
 		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
-		reset-gpios = <&gpio1 5 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio1 5 GPIO_ACTIVE_LOW>;
 		irq-gpios = <&gpio3 0 GPIO_ACTIVE_HIGH>;
 		touchscreen-size-x = <720>;
 		touchscreen-size-y = <1440>;

-- 
b4 0.10.1

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH RFC v2 5/7] arm64: dts: imx: fix touchscreen reset GPIO polarity

[Fabio Estevam]

[Adding Angus and David]

On Mon, Nov 21, 2022 at 3:12 PM Quentin Schulz <foss+kernel@0leil.net> wrote:
>
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>
> The reset line is active low for the Goodix touchscreen controller so
> let's fix the polarity in the Device Tree node.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> ---
>  arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts         | 2 +-
>  arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> index 9fbbbb556c0b3..df7e5ae9698e1 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> @@ -107,7 +107,7 @@ touchscreeen@5d {
>                 interrupt-parent = <&gpio1>;
>                 interrupts = <8 IRQ_TYPE_NONE>;
>                 irq-gpios = <&gpio1 8 GPIO_ACTIVE_HIGH>;
> -               reset-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
> +               reset-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;
>         };
>
>         temp-sense@70 {
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts b/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
> index 6445c6b90b5bb..b038300812b1e 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
> @@ -542,7 +542,7 @@ touchscreen@5d {
>                 pinctrl-0 = <&pinctrl_ts>;
>                 interrupt-parent = <&gpio3>;
>                 interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> -               reset-gpios = <&gpio1 5 GPIO_ACTIVE_HIGH>;
> +               reset-gpios = <&gpio1 5 GPIO_ACTIVE_LOW>;
>                 irq-gpios = <&gpio3 0 GPIO_ACTIVE_HIGH>;
>                 touchscreen-size-x = <720>;
>                 touchscreen-size-y = <1440>;
>
> --
> b4 0.10.1

Re: [PATCH RFC v2 5/7] arm64: dts: imx: fix touchscreen reset GPIO polarity

[Fabio Estevam]

[Adding Angus and David]

On Mon, Nov 21, 2022 at 3:12 PM Quentin Schulz <foss+kernel@0leil.net> wrote:
>
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>
> The reset line is active low for the Goodix touchscreen controller so
> let's fix the polarity in the Device Tree node.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> ---
>  arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts         | 2 +-
>  arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> index 9fbbbb556c0b3..df7e5ae9698e1 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> @@ -107,7 +107,7 @@ touchscreeen@5d {
>                 interrupt-parent = <&gpio1>;
>                 interrupts = <8 IRQ_TYPE_NONE>;
>                 irq-gpios = <&gpio1 8 GPIO_ACTIVE_HIGH>;
> -               reset-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
> +               reset-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;
>         };
>
>         temp-sense@70 {
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts b/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
> index 6445c6b90b5bb..b038300812b1e 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
> @@ -542,7 +542,7 @@ touchscreen@5d {
>                 pinctrl-0 = <&pinctrl_ts>;
>                 interrupt-parent = <&gpio3>;
>                 interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> -               reset-gpios = <&gpio1 5 GPIO_ACTIVE_HIGH>;
> +               reset-gpios = <&gpio1 5 GPIO_ACTIVE_LOW>;
>                 irq-gpios = <&gpio3 0 GPIO_ACTIVE_HIGH>;
>                 touchscreen-size-x = <720>;
>                 touchscreen-size-y = <1440>;
>
> --
> b4 0.10.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH RFC v2 5/7] arm64: dts: imx: fix touchscreen reset GPIO polarity

[Fabio Estevam]

[Adding Angus and David]

On Mon, Nov 21, 2022 at 3:12 PM Quentin Schulz <foss+kernel@0leil.net> wrote:
>
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>
> The reset line is active low for the Goodix touchscreen controller so
> let's fix the polarity in the Device Tree node.
>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> ---
>  arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts         | 2 +-
>  arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> index 9fbbbb556c0b3..df7e5ae9698e1 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> @@ -107,7 +107,7 @@ touchscreeen@5d {
>                 interrupt-parent = <&gpio1>;
>                 interrupts = <8 IRQ_TYPE_NONE>;
>                 irq-gpios = <&gpio1 8 GPIO_ACTIVE_HIGH>;
> -               reset-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
> +               reset-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;
>         };
>
>         temp-sense@70 {
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts b/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
> index 6445c6b90b5bb..b038300812b1e 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
> @@ -542,7 +542,7 @@ touchscreen@5d {
>                 pinctrl-0 = <&pinctrl_ts>;
>                 interrupt-parent = <&gpio3>;
>                 interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> -               reset-gpios = <&gpio1 5 GPIO_ACTIVE_HIGH>;
> +               reset-gpios = <&gpio1 5 GPIO_ACTIVE_LOW>;
>                 irq-gpios = <&gpio3 0 GPIO_ACTIVE_HIGH>;
>                 touchscreen-size-x = <720>;
>                 touchscreen-size-y = <1440>;
>
> --
> b4 0.10.1

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH RFC v2 5/7] arm64: dts: imx: fix touchscreen reset GPIO polarity

[David Jander]

On Mon, 21 Nov 2022 15:18:32 -0300
Fabio Estevam <festevam@gmail.com> wrote:

> [Adding Angus and David]

Thanks. This was apparently necessary ;-)

> On Mon, Nov 21, 2022 at 3:12 PM Quentin Schulz <foss+kernel@0leil.net> wrote:
> >
> > From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> >
> > The reset line is active low for the Goodix touchscreen controller so
> > let's fix the polarity in the Device Tree node.
> >
> > Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> > ---
> >  arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts         | 2 +-
> >  arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> > index 9fbbbb556c0b3..df7e5ae9698e1 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> > @@ -107,7 +107,7 @@ touchscreeen@5d {
> >                 interrupt-parent = <&gpio1>;
> >                 interrupts = <8 IRQ_TYPE_NONE>;
> >                 irq-gpios = <&gpio1 8 GPIO_ACTIVE_HIGH>;
> > -               reset-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
> > +               reset-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;

NACK!

The PRT8MM has an inverter in the reset line. The reason for that is that the
reset line needs to be inactive when the driving side is unpowered.
The DT was correct, this change will break it.

> >         };
> >
> >         temp-sense@70 {
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts b/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
> > index 6445c6b90b5bb..b038300812b1e 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
> > @@ -542,7 +542,7 @@ touchscreen@5d {
> >                 pinctrl-0 = <&pinctrl_ts>;
> >                 interrupt-parent = <&gpio3>;
> >                 interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> > -               reset-gpios = <&gpio1 5 GPIO_ACTIVE_HIGH>;
> > +               reset-gpios = <&gpio1 5 GPIO_ACTIVE_LOW>;
> >                 irq-gpios = <&gpio3 0 GPIO_ACTIVE_HIGH>;
> >                 touchscreen-size-x = <720>;
> >                 touchscreen-size-y = <1440>;
> >
> > --
> > b4 0.10.1  

Best regards,

-- 
David Jander
Protonic Holland.

Re: [PATCH RFC v2 5/7] arm64: dts: imx: fix touchscreen reset GPIO polarity

[David Jander]

On Mon, 21 Nov 2022 15:18:32 -0300
Fabio Estevam <festevam@gmail.com> wrote:

> [Adding Angus and David]

Thanks. This was apparently necessary ;-)

> On Mon, Nov 21, 2022 at 3:12 PM Quentin Schulz <foss+kernel@0leil.net> wrote:
> >
> > From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> >
> > The reset line is active low for the Goodix touchscreen controller so
> > let's fix the polarity in the Device Tree node.
> >
> > Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> > ---
> >  arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts         | 2 +-
> >  arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> > index 9fbbbb556c0b3..df7e5ae9698e1 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> > @@ -107,7 +107,7 @@ touchscreeen@5d {
> >                 interrupt-parent = <&gpio1>;
> >                 interrupts = <8 IRQ_TYPE_NONE>;
> >                 irq-gpios = <&gpio1 8 GPIO_ACTIVE_HIGH>;
> > -               reset-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
> > +               reset-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;

NACK!

The PRT8MM has an inverter in the reset line. The reason for that is that the
reset line needs to be inactive when the driving side is unpowered.
The DT was correct, this change will break it.

> >         };
> >
> >         temp-sense@70 {
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts b/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
> > index 6445c6b90b5bb..b038300812b1e 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
> > @@ -542,7 +542,7 @@ touchscreen@5d {
> >                 pinctrl-0 = <&pinctrl_ts>;
> >                 interrupt-parent = <&gpio3>;
> >                 interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> > -               reset-gpios = <&gpio1 5 GPIO_ACTIVE_HIGH>;
> > +               reset-gpios = <&gpio1 5 GPIO_ACTIVE_LOW>;
> >                 irq-gpios = <&gpio3 0 GPIO_ACTIVE_HIGH>;
> >                 touchscreen-size-x = <720>;
> >                 touchscreen-size-y = <1440>;
> >
> > --
> > b4 0.10.1  

Best regards,

-- 
David Jander
Protonic Holland.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH RFC v2 5/7] arm64: dts: imx: fix touchscreen reset GPIO polarity

[David Jander]

On Mon, 21 Nov 2022 15:18:32 -0300
Fabio Estevam <festevam@gmail.com> wrote:

> [Adding Angus and David]

Thanks. This was apparently necessary ;-)

> On Mon, Nov 21, 2022 at 3:12 PM Quentin Schulz <foss+kernel@0leil.net> wrote:
> >
> > From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> >
> > The reset line is active low for the Goodix touchscreen controller so
> > let's fix the polarity in the Device Tree node.
> >
> > Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> > ---
> >  arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts         | 2 +-
> >  arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> > index 9fbbbb556c0b3..df7e5ae9698e1 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> > @@ -107,7 +107,7 @@ touchscreeen@5d {
> >                 interrupt-parent = <&gpio1>;
> >                 interrupts = <8 IRQ_TYPE_NONE>;
> >                 irq-gpios = <&gpio1 8 GPIO_ACTIVE_HIGH>;
> > -               reset-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
> > +               reset-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;

NACK!

The PRT8MM has an inverter in the reset line. The reason for that is that the
reset line needs to be inactive when the driving side is unpowered.
The DT was correct, this change will break it.

> >         };
> >
> >         temp-sense@70 {
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts b/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
> > index 6445c6b90b5bb..b038300812b1e 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
> > @@ -542,7 +542,7 @@ touchscreen@5d {
> >                 pinctrl-0 = <&pinctrl_ts>;
> >                 interrupt-parent = <&gpio3>;
> >                 interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> > -               reset-gpios = <&gpio1 5 GPIO_ACTIVE_HIGH>;
> > +               reset-gpios = <&gpio1 5 GPIO_ACTIVE_LOW>;
> >                 irq-gpios = <&gpio3 0 GPIO_ACTIVE_HIGH>;
> >                 touchscreen-size-x = <720>;
> >                 touchscreen-size-y = <1440>;
> >
> > --
> > b4 0.10.1  

Best regards,

-- 
David Jander
Protonic Holland.

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH RFC v2 5/7] arm64: dts: imx: fix touchscreen reset GPIO polarity

[Quentin Schulz]

Hi David,

Thanks Fabio for the Cc.

On 11/22/22 08:18, David Jander wrote:
> On Mon, 21 Nov 2022 15:18:32 -0300
> Fabio Estevam <festevam@gmail.com> wrote:
> 
>> [Adding Angus and David]
> 
> Thanks. This was apparently necessary ;-)
> 
>> On Mon, Nov 21, 2022 at 3:12 PM Quentin Schulz <foss+kernel@0leil.net> wrote:
>>>
>>> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>>
>>> The reset line is active low for the Goodix touchscreen controller so
>>> let's fix the polarity in the Device Tree node.
>>>
>>> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>> ---
>>>   arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts         | 2 +-
>>>   arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts | 2 +-
>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>> index 9fbbbb556c0b3..df7e5ae9698e1 100644
>>> --- a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>> @@ -107,7 +107,7 @@ touchscreeen@5d {
>>>                  interrupt-parent = <&gpio1>;
>>>                  interrupts = <8 IRQ_TYPE_NONE>;
>>>                  irq-gpios = <&gpio1 8 GPIO_ACTIVE_HIGH>;
>>> -               reset-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
>>> +               reset-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;
> 
> NACK!
> 
> The PRT8MM has an inverter in the reset line. The reason for that is that the
> reset line needs to be inactive when the driving side is unpowered.
> The DT was correct, this change will break it.
> 

The DT was correct. The implementation in the driver is changed (the 
polarity is swapped) in this patch series, therefore the DT isn't 
correct anymore, hence this patch.

See 
https://lore.kernel.org/linux-input/20221103-upstream-goodix-reset-v2-0-2c38fb03a300@theobroma-systems.com/ 
for the whole patch series.

This DT patch alone is obviously incorrect, but the context around it 
matters. I could/should have made it all into one big patch, the 
question is then how this big tree-crossing patch would be merged into 
Linux (if there's consensus). We're not there yet.

For some additional background on the discussion that was had in the v1:
https://lore.kernel.org/all/267de96a-0129-a97d-9bf6-e1001b422a1a@theobroma-systems.com/
I messed up the Cc list in the v1, apologies for the missing context in 
the archived mails, I think one should be able to understand the 
important bits by reading the answers in-mail. There, Dmitry, Hans and I 
discussed the meaning of the active level of GPIOs/reset lines and I 
expressed the reasons for such a change (which are also listed in the 
cover letter of this patch series).

As stated in v1 cover letter, no implementation will satisfy every one. 
We either make the DT binding implementation specific (which is what it 
shouldn't be), or we swap the polarity in the Linux implementation and 
thus the DT but then break DT backward compatibility.

Cheers,
Quentin

Re: [PATCH RFC v2 5/7] arm64: dts: imx: fix touchscreen reset GPIO polarity

[Quentin Schulz]

Hi David,

Thanks Fabio for the Cc.

On 11/22/22 08:18, David Jander wrote:
> On Mon, 21 Nov 2022 15:18:32 -0300
> Fabio Estevam <festevam@gmail.com> wrote:
> 
>> [Adding Angus and David]
> 
> Thanks. This was apparently necessary ;-)
> 
>> On Mon, Nov 21, 2022 at 3:12 PM Quentin Schulz <foss+kernel@0leil.net> wrote:
>>>
>>> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>>
>>> The reset line is active low for the Goodix touchscreen controller so
>>> let's fix the polarity in the Device Tree node.
>>>
>>> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>> ---
>>>   arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts         | 2 +-
>>>   arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts | 2 +-
>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>> index 9fbbbb556c0b3..df7e5ae9698e1 100644
>>> --- a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>> @@ -107,7 +107,7 @@ touchscreeen@5d {
>>>                  interrupt-parent = <&gpio1>;
>>>                  interrupts = <8 IRQ_TYPE_NONE>;
>>>                  irq-gpios = <&gpio1 8 GPIO_ACTIVE_HIGH>;
>>> -               reset-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
>>> +               reset-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;
> 
> NACK!
> 
> The PRT8MM has an inverter in the reset line. The reason for that is that the
> reset line needs to be inactive when the driving side is unpowered.
> The DT was correct, this change will break it.
> 

The DT was correct. The implementation in the driver is changed (the 
polarity is swapped) in this patch series, therefore the DT isn't 
correct anymore, hence this patch.

See 
https://lore.kernel.org/linux-input/20221103-upstream-goodix-reset-v2-0-2c38fb03a300@theobroma-systems.com/ 
for the whole patch series.

This DT patch alone is obviously incorrect, but the context around it 
matters. I could/should have made it all into one big patch, the 
question is then how this big tree-crossing patch would be merged into 
Linux (if there's consensus). We're not there yet.

For some additional background on the discussion that was had in the v1:
https://lore.kernel.org/all/267de96a-0129-a97d-9bf6-e1001b422a1a@theobroma-systems.com/
I messed up the Cc list in the v1, apologies for the missing context in 
the archived mails, I think one should be able to understand the 
important bits by reading the answers in-mail. There, Dmitry, Hans and I 
discussed the meaning of the active level of GPIOs/reset lines and I 
expressed the reasons for such a change (which are also listed in the 
cover letter of this patch series).

As stated in v1 cover letter, no implementation will satisfy every one. 
We either make the DT binding implementation specific (which is what it 
shouldn't be), or we swap the polarity in the Linux implementation and 
thus the DT but then break DT backward compatibility.

Cheers,
Quentin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH RFC v2 5/7] arm64: dts: imx: fix touchscreen reset GPIO polarity

[Quentin Schulz]

Hi David,

Thanks Fabio for the Cc.

On 11/22/22 08:18, David Jander wrote:
> On Mon, 21 Nov 2022 15:18:32 -0300
> Fabio Estevam <festevam@gmail.com> wrote:
> 
>> [Adding Angus and David]
> 
> Thanks. This was apparently necessary ;-)
> 
>> On Mon, Nov 21, 2022 at 3:12 PM Quentin Schulz <foss+kernel@0leil.net> wrote:
>>>
>>> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>>
>>> The reset line is active low for the Goodix touchscreen controller so
>>> let's fix the polarity in the Device Tree node.
>>>
>>> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>> ---
>>>   arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts         | 2 +-
>>>   arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts | 2 +-
>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>> index 9fbbbb556c0b3..df7e5ae9698e1 100644
>>> --- a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>> @@ -107,7 +107,7 @@ touchscreeen@5d {
>>>                  interrupt-parent = <&gpio1>;
>>>                  interrupts = <8 IRQ_TYPE_NONE>;
>>>                  irq-gpios = <&gpio1 8 GPIO_ACTIVE_HIGH>;
>>> -               reset-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
>>> +               reset-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;
> 
> NACK!
> 
> The PRT8MM has an inverter in the reset line. The reason for that is that the
> reset line needs to be inactive when the driving side is unpowered.
> The DT was correct, this change will break it.
> 

The DT was correct. The implementation in the driver is changed (the 
polarity is swapped) in this patch series, therefore the DT isn't 
correct anymore, hence this patch.

See 
https://lore.kernel.org/linux-input/20221103-upstream-goodix-reset-v2-0-2c38fb03a300@theobroma-systems.com/ 
for the whole patch series.

This DT patch alone is obviously incorrect, but the context around it 
matters. I could/should have made it all into one big patch, the 
question is then how this big tree-crossing patch would be merged into 
Linux (if there's consensus). We're not there yet.

For some additional background on the discussion that was had in the v1:
https://lore.kernel.org/all/267de96a-0129-a97d-9bf6-e1001b422a1a@theobroma-systems.com/
I messed up the Cc list in the v1, apologies for the missing context in 
the archived mails, I think one should be able to understand the 
important bits by reading the answers in-mail. There, Dmitry, Hans and I 
discussed the meaning of the active level of GPIOs/reset lines and I 
expressed the reasons for such a change (which are also listed in the 
cover letter of this patch series).

As stated in v1 cover letter, no implementation will satisfy every one. 
We either make the DT binding implementation specific (which is what it 
shouldn't be), or we swap the polarity in the Linux implementation and 
thus the DT but then break DT backward compatibility.

Cheers,
Quentin

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH RFC v2 5/7] arm64: dts: imx: fix touchscreen reset GPIO polarity

[Robin Murphy]

On 2022-11-22 09:58, Quentin Schulz wrote:
> Hi David,
> 
> Thanks Fabio for the Cc.
> 
> On 11/22/22 08:18, David Jander wrote:
>> On Mon, 21 Nov 2022 15:18:32 -0300
>> Fabio Estevam <festevam@gmail.com> wrote:
>>
>>> [Adding Angus and David]
>>
>> Thanks. This was apparently necessary ;-)
>>
>>> On Mon, Nov 21, 2022 at 3:12 PM Quentin Schulz 
>>> <foss+kernel@0leil.net> wrote:
>>>>
>>>> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>>>
>>>> The reset line is active low for the Goodix touchscreen controller so
>>>> let's fix the polarity in the Device Tree node.
>>>>
>>>> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>>> ---
>>>>   arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts         | 2 +-
>>>>   arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts | 2 +-
>>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts 
>>>> b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>>> index 9fbbbb556c0b3..df7e5ae9698e1 100644
>>>> --- a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>>> @@ -107,7 +107,7 @@ touchscreeen@5d {
>>>>                  interrupt-parent = <&gpio1>;
>>>>                  interrupts = <8 IRQ_TYPE_NONE>;
>>>>                  irq-gpios = <&gpio1 8 GPIO_ACTIVE_HIGH>;
>>>> -               reset-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
>>>> +               reset-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;
>>
>> NACK!
>>
>> The PRT8MM has an inverter in the reset line. The reason for that is 
>> that the
>> reset line needs to be inactive when the driving side is unpowered.
>> The DT was correct, this change will break it.
>>
> 
> The DT was correct. The implementation in the driver is changed (the 
> polarity is swapped) in this patch series, therefore the DT isn't 
> correct anymore, hence this patch.

I'm not sure it's quite that simple... FWIW I'm using an add-on LCD 
module with a GT9271[1] (and I won't be the only one - Raspberry Pi and 
other SBC users using DT overlays or custom-built DTBs are a whole other 
can of worms here), where GPIO_ACTIVE_LOW is correctly specified per the 
schematics, thus "wrong" for the current driver behaviour, yet it *is* 
working OK as-is. I guess that's because /RSTB ends up driven low for 
long enough between the current "deassertion" by 
gpiod_direction_output(1) and gpiod_direction_input() allowing the 
external pull-up to take it high again.

Robin.

[1] 
https://www.friendlyelec.com/index.php?route=product/product&path=81&product_id=230

> 
> See 
> https://lore.kernel.org/linux-input/20221103-upstream-goodix-reset-v2-0-2c38fb03a300@theobroma-systems.com/ for the whole patch series.
> 
> This DT patch alone is obviously incorrect, but the context around it 
> matters. I could/should have made it all into one big patch, the 
> question is then how this big tree-crossing patch would be merged into 
> Linux (if there's consensus). We're not there yet.
> 
> For some additional background on the discussion that was had in the v1:
> https://lore.kernel.org/all/267de96a-0129-a97d-9bf6-e1001b422a1a@theobroma-systems.com/
> I messed up the Cc list in the v1, apologies for the missing context in 
> the archived mails, I think one should be able to understand the 
> important bits by reading the answers in-mail. There, Dmitry, Hans and I 
> discussed the meaning of the active level of GPIOs/reset lines and I 
> expressed the reasons for such a change (which are also listed in the 
> cover letter of this patch series).
> 
> As stated in v1 cover letter, no implementation will satisfy every one. 
> We either make the DT binding implementation specific (which is what it 
> shouldn't be), or we swap the polarity in the Linux implementation and 
> thus the DT but then break DT backward compatibility.
> 
> Cheers,
> Quentin
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH RFC v2 5/7] arm64: dts: imx: fix touchscreen reset GPIO polarity

[Robin Murphy]

On 2022-11-22 09:58, Quentin Schulz wrote:
> Hi David,
> 
> Thanks Fabio for the Cc.
> 
> On 11/22/22 08:18, David Jander wrote:
>> On Mon, 21 Nov 2022 15:18:32 -0300
>> Fabio Estevam <festevam@gmail.com> wrote:
>>
>>> [Adding Angus and David]
>>
>> Thanks. This was apparently necessary ;-)
>>
>>> On Mon, Nov 21, 2022 at 3:12 PM Quentin Schulz 
>>> <foss+kernel@0leil.net> wrote:
>>>>
>>>> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>>>
>>>> The reset line is active low for the Goodix touchscreen controller so
>>>> let's fix the polarity in the Device Tree node.
>>>>
>>>> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>>> ---
>>>>   arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts         | 2 +-
>>>>   arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts | 2 +-
>>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts 
>>>> b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>>> index 9fbbbb556c0b3..df7e5ae9698e1 100644
>>>> --- a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>>> @@ -107,7 +107,7 @@ touchscreeen@5d {
>>>>                  interrupt-parent = <&gpio1>;
>>>>                  interrupts = <8 IRQ_TYPE_NONE>;
>>>>                  irq-gpios = <&gpio1 8 GPIO_ACTIVE_HIGH>;
>>>> -               reset-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
>>>> +               reset-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;
>>
>> NACK!
>>
>> The PRT8MM has an inverter in the reset line. The reason for that is 
>> that the
>> reset line needs to be inactive when the driving side is unpowered.
>> The DT was correct, this change will break it.
>>
> 
> The DT was correct. The implementation in the driver is changed (the 
> polarity is swapped) in this patch series, therefore the DT isn't 
> correct anymore, hence this patch.

I'm not sure it's quite that simple... FWIW I'm using an add-on LCD 
module with a GT9271[1] (and I won't be the only one - Raspberry Pi and 
other SBC users using DT overlays or custom-built DTBs are a whole other 
can of worms here), where GPIO_ACTIVE_LOW is correctly specified per the 
schematics, thus "wrong" for the current driver behaviour, yet it *is* 
working OK as-is. I guess that's because /RSTB ends up driven low for 
long enough between the current "deassertion" by 
gpiod_direction_output(1) and gpiod_direction_input() allowing the 
external pull-up to take it high again.

Robin.

[1] 
https://www.friendlyelec.com/index.php?route=product/product&path=81&product_id=230

> 
> See 
> https://lore.kernel.org/linux-input/20221103-upstream-goodix-reset-v2-0-2c38fb03a300@theobroma-systems.com/ for the whole patch series.
> 
> This DT patch alone is obviously incorrect, but the context around it 
> matters. I could/should have made it all into one big patch, the 
> question is then how this big tree-crossing patch would be merged into 
> Linux (if there's consensus). We're not there yet.
> 
> For some additional background on the discussion that was had in the v1:
> https://lore.kernel.org/all/267de96a-0129-a97d-9bf6-e1001b422a1a@theobroma-systems.com/
> I messed up the Cc list in the v1, apologies for the missing context in 
> the archived mails, I think one should be able to understand the 
> important bits by reading the answers in-mail. There, Dmitry, Hans and I 
> discussed the meaning of the active level of GPIOs/reset lines and I 
> expressed the reasons for such a change (which are also listed in the 
> cover letter of this patch series).
> 
> As stated in v1 cover letter, no implementation will satisfy every one. 
> We either make the DT binding implementation specific (which is what it 
> shouldn't be), or we swap the polarity in the Linux implementation and 
> thus the DT but then break DT backward compatibility.
> 
> Cheers,
> Quentin
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH RFC v2 5/7] arm64: dts: imx: fix touchscreen reset GPIO polarity

[Robin Murphy]

On 2022-11-22 09:58, Quentin Schulz wrote:
> Hi David,
> 
> Thanks Fabio for the Cc.
> 
> On 11/22/22 08:18, David Jander wrote:
>> On Mon, 21 Nov 2022 15:18:32 -0300
>> Fabio Estevam <festevam@gmail.com> wrote:
>>
>>> [Adding Angus and David]
>>
>> Thanks. This was apparently necessary ;-)
>>
>>> On Mon, Nov 21, 2022 at 3:12 PM Quentin Schulz 
>>> <foss+kernel@0leil.net> wrote:
>>>>
>>>> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>>>
>>>> The reset line is active low for the Goodix touchscreen controller so
>>>> let's fix the polarity in the Device Tree node.
>>>>
>>>> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>>> ---
>>>>   arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts         | 2 +-
>>>>   arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts | 2 +-
>>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts 
>>>> b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>>> index 9fbbbb556c0b3..df7e5ae9698e1 100644
>>>> --- a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>>> @@ -107,7 +107,7 @@ touchscreeen@5d {
>>>>                  interrupt-parent = <&gpio1>;
>>>>                  interrupts = <8 IRQ_TYPE_NONE>;
>>>>                  irq-gpios = <&gpio1 8 GPIO_ACTIVE_HIGH>;
>>>> -               reset-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
>>>> +               reset-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;
>>
>> NACK!
>>
>> The PRT8MM has an inverter in the reset line. The reason for that is 
>> that the
>> reset line needs to be inactive when the driving side is unpowered.
>> The DT was correct, this change will break it.
>>
> 
> The DT was correct. The implementation in the driver is changed (the 
> polarity is swapped) in this patch series, therefore the DT isn't 
> correct anymore, hence this patch.

I'm not sure it's quite that simple... FWIW I'm using an add-on LCD 
module with a GT9271[1] (and I won't be the only one - Raspberry Pi and 
other SBC users using DT overlays or custom-built DTBs are a whole other 
can of worms here), where GPIO_ACTIVE_LOW is correctly specified per the 
schematics, thus "wrong" for the current driver behaviour, yet it *is* 
working OK as-is. I guess that's because /RSTB ends up driven low for 
long enough between the current "deassertion" by 
gpiod_direction_output(1) and gpiod_direction_input() allowing the 
external pull-up to take it high again.

Robin.

[1] 
https://www.friendlyelec.com/index.php?route=product/product&path=81&product_id=230

> 
> See 
> https://lore.kernel.org/linux-input/20221103-upstream-goodix-reset-v2-0-2c38fb03a300@theobroma-systems.com/ for the whole patch series.
> 
> This DT patch alone is obviously incorrect, but the context around it 
> matters. I could/should have made it all into one big patch, the 
> question is then how this big tree-crossing patch would be merged into 
> Linux (if there's consensus). We're not there yet.
> 
> For some additional background on the discussion that was had in the v1:
> https://lore.kernel.org/all/267de96a-0129-a97d-9bf6-e1001b422a1a@theobroma-systems.com/
> I messed up the Cc list in the v1, apologies for the missing context in 
> the archived mails, I think one should be able to understand the 
> important bits by reading the answers in-mail. There, Dmitry, Hans and I 
> discussed the meaning of the active level of GPIOs/reset lines and I 
> expressed the reasons for such a change (which are also listed in the 
> cover letter of this patch series).
> 
> As stated in v1 cover letter, no implementation will satisfy every one. 
> We either make the DT binding implementation specific (which is what it 
> shouldn't be), or we swap the polarity in the Linux implementation and 
> thus the DT but then break DT backward compatibility.
> 
> Cheers,
> Quentin
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH RFC v2 5/7] arm64: dts: imx: fix touchscreen reset GPIO polarity

[Quentin Schulz]

Hi Robin,

On 11/22/22 13:46, Robin Murphy wrote:
> On 2022-11-22 09:58, Quentin Schulz wrote:
>> Hi David,
>>
>> Thanks Fabio for the Cc.
>>
>> On 11/22/22 08:18, David Jander wrote:
>>> On Mon, 21 Nov 2022 15:18:32 -0300
>>> Fabio Estevam <festevam@gmail.com> wrote:
>>>
>>>> [Adding Angus and David]
>>>
>>> Thanks. This was apparently necessary ;-)
>>>
>>>> On Mon, Nov 21, 2022 at 3:12 PM Quentin Schulz 
>>>> <foss+kernel@0leil.net> wrote:
>>>>>
>>>>> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>>>>
>>>>> The reset line is active low for the Goodix touchscreen controller so
>>>>> let's fix the polarity in the Device Tree node.
>>>>>
>>>>> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>>>> ---
>>>>>   arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts         | 2 +-
>>>>>   arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts | 2 +-
>>>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts 
>>>>> b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>>>> index 9fbbbb556c0b3..df7e5ae9698e1 100644
>>>>> --- a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>>>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>>>> @@ -107,7 +107,7 @@ touchscreeen@5d {
>>>>>                  interrupt-parent = <&gpio1>;
>>>>>                  interrupts = <8 IRQ_TYPE_NONE>;
>>>>>                  irq-gpios = <&gpio1 8 GPIO_ACTIVE_HIGH>;
>>>>> -               reset-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
>>>>> +               reset-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;
>>>
>>> NACK!
>>>
>>> The PRT8MM has an inverter in the reset line. The reason for that is 
>>> that the
>>> reset line needs to be inactive when the driving side is unpowered.
>>> The DT was correct, this change will break it.
>>>
>>
>> The DT was correct. The implementation in the driver is changed (the 
>> polarity is swapped) in this patch series, therefore the DT isn't 
>> correct anymore, hence this patch.
> 
> I'm not sure it's quite that simple... FWIW I'm using an add-on LCD 
> module with a GT9271[1] (and I won't be the only one - Raspberry Pi and 
> other SBC users using DT overlays or custom-built DTBs are a whole other 
> can of worms here), where GPIO_ACTIVE_LOW is correctly specified per the 
> schematics, thus "wrong" for the current driver behaviour, yet it *is* 
> working OK as-is. I guess that's because /RSTB ends up driven low for 
> long enough between the current "deassertion" by 
> gpiod_direction_output(1) and gpiod_direction_input() allowing the 
> external pull-up to take it high again.
> 

Correct, that seems like it is the case for "current-wrong" GPIO level, 
provided you have a pull-up on the RESET line (as recommended in the 
datasheet).

c.f. 
https://github.com/hadess/gt9xx/blob/master/specifications/GT9271%20Datasheet.pdf

with the current driver, what I assume we should get is:
             ___________________
INT _______|                   |___________

     ____________           __________________
RST             |_________|

    ^
    L__ pull-up on RST so high by default
         ^
         L___ gpiod_direction_output(0) (deassert GPIO active-low, so high)
            ^
            L____ goodix_irq_direction_output
                 ^
                 L___ gpiod_direction_output(1) (assert GPIO active-low, 
so low)
                           ^
                           L____ gpiod_direction_input() (floating, 
pull-up on RST so high)

This works because of the pull-up on RST and that what matters is that 
the INT lane is configured 100µs before a rising edge on RST line (for 
at least 5ms). However, the init sequence is not properly followed and 
might get broken in the future since it is not something that we 
explicitly support.

With the proposed patch:
             ___________________
INT _______|                   |___________

     ____         __________________
RST     |_______|

    ^
    L__ pull-up on RST so high by default
         ^
         L___ gpiod_direction_output(1) (assert GPIO active-low, so low)
            ^
            L____ goodix_irq_direction_output
                 ^
                 L___ gpiod_direction_output(1) (deassert GPIO 
active-low, so high)
                           ^
                           L____ gpiod_direction_input() (floating, 
pull-up on RST so high)

This should work too and does not rely on some side effects/timings and 
should be future-proof.

As for OOT DTB/DTBO, nothing I can do unfortunately, that is correct.

The fact that we have both "interpretations" of the RESET GPIO level in 
Linux is worrying, albeit with OOT DTBs. A driver or subsystem rework 
might break boards in which case people should update their DTB as 
suggested here or we'll decide to support both interpretations, which 
ain't great either.

Cheers,
Quentin

Re: [PATCH RFC v2 5/7] arm64: dts: imx: fix touchscreen reset GPIO polarity

[Quentin Schulz]

Hi Robin,

On 11/22/22 13:46, Robin Murphy wrote:
> On 2022-11-22 09:58, Quentin Schulz wrote:
>> Hi David,
>>
>> Thanks Fabio for the Cc.
>>
>> On 11/22/22 08:18, David Jander wrote:
>>> On Mon, 21 Nov 2022 15:18:32 -0300
>>> Fabio Estevam <festevam@gmail.com> wrote:
>>>
>>>> [Adding Angus and David]
>>>
>>> Thanks. This was apparently necessary ;-)
>>>
>>>> On Mon, Nov 21, 2022 at 3:12 PM Quentin Schulz 
>>>> <foss+kernel@0leil.net> wrote:
>>>>>
>>>>> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>>>>
>>>>> The reset line is active low for the Goodix touchscreen controller so
>>>>> let's fix the polarity in the Device Tree node.
>>>>>
>>>>> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>>>> ---
>>>>>   arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts         | 2 +-
>>>>>   arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts | 2 +-
>>>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts 
>>>>> b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>>>> index 9fbbbb556c0b3..df7e5ae9698e1 100644
>>>>> --- a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>>>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>>>> @@ -107,7 +107,7 @@ touchscreeen@5d {
>>>>>                  interrupt-parent = <&gpio1>;
>>>>>                  interrupts = <8 IRQ_TYPE_NONE>;
>>>>>                  irq-gpios = <&gpio1 8 GPIO_ACTIVE_HIGH>;
>>>>> -               reset-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
>>>>> +               reset-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;
>>>
>>> NACK!
>>>
>>> The PRT8MM has an inverter in the reset line. The reason for that is 
>>> that the
>>> reset line needs to be inactive when the driving side is unpowered.
>>> The DT was correct, this change will break it.
>>>
>>
>> The DT was correct. The implementation in the driver is changed (the 
>> polarity is swapped) in this patch series, therefore the DT isn't 
>> correct anymore, hence this patch.
> 
> I'm not sure it's quite that simple... FWIW I'm using an add-on LCD 
> module with a GT9271[1] (and I won't be the only one - Raspberry Pi and 
> other SBC users using DT overlays or custom-built DTBs are a whole other 
> can of worms here), where GPIO_ACTIVE_LOW is correctly specified per the 
> schematics, thus "wrong" for the current driver behaviour, yet it *is* 
> working OK as-is. I guess that's because /RSTB ends up driven low for 
> long enough between the current "deassertion" by 
> gpiod_direction_output(1) and gpiod_direction_input() allowing the 
> external pull-up to take it high again.
> 

Correct, that seems like it is the case for "current-wrong" GPIO level, 
provided you have a pull-up on the RESET line (as recommended in the 
datasheet).

c.f. 
https://github.com/hadess/gt9xx/blob/master/specifications/GT9271%20Datasheet.pdf

with the current driver, what I assume we should get is:
             ___________________
INT _______|                   |___________

     ____________           __________________
RST             |_________|

    ^
    L__ pull-up on RST so high by default
         ^
         L___ gpiod_direction_output(0) (deassert GPIO active-low, so high)
            ^
            L____ goodix_irq_direction_output
                 ^
                 L___ gpiod_direction_output(1) (assert GPIO active-low, 
so low)
                           ^
                           L____ gpiod_direction_input() (floating, 
pull-up on RST so high)

This works because of the pull-up on RST and that what matters is that 
the INT lane is configured 100µs before a rising edge on RST line (for 
at least 5ms). However, the init sequence is not properly followed and 
might get broken in the future since it is not something that we 
explicitly support.

With the proposed patch:
             ___________________
INT _______|                   |___________

     ____         __________________
RST     |_______|

    ^
    L__ pull-up on RST so high by default
         ^
         L___ gpiod_direction_output(1) (assert GPIO active-low, so low)
            ^
            L____ goodix_irq_direction_output
                 ^
                 L___ gpiod_direction_output(1) (deassert GPIO 
active-low, so high)
                           ^
                           L____ gpiod_direction_input() (floating, 
pull-up on RST so high)

This should work too and does not rely on some side effects/timings and 
should be future-proof.

As for OOT DTB/DTBO, nothing I can do unfortunately, that is correct.

The fact that we have both "interpretations" of the RESET GPIO level in 
Linux is worrying, albeit with OOT DTBs. A driver or subsystem rework 
might break boards in which case people should update their DTB as 
suggested here or we'll decide to support both interpretations, which 
ain't great either.

Cheers,
Quentin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH RFC v2 5/7] arm64: dts: imx: fix touchscreen reset GPIO polarity

[Quentin Schulz]

Hi Robin,

On 11/22/22 13:46, Robin Murphy wrote:
> On 2022-11-22 09:58, Quentin Schulz wrote:
>> Hi David,
>>
>> Thanks Fabio for the Cc.
>>
>> On 11/22/22 08:18, David Jander wrote:
>>> On Mon, 21 Nov 2022 15:18:32 -0300
>>> Fabio Estevam <festevam@gmail.com> wrote:
>>>
>>>> [Adding Angus and David]
>>>
>>> Thanks. This was apparently necessary ;-)
>>>
>>>> On Mon, Nov 21, 2022 at 3:12 PM Quentin Schulz 
>>>> <foss+kernel@0leil.net> wrote:
>>>>>
>>>>> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>>>>
>>>>> The reset line is active low for the Goodix touchscreen controller so
>>>>> let's fix the polarity in the Device Tree node.
>>>>>
>>>>> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>>>> ---
>>>>>   arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts         | 2 +-
>>>>>   arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts | 2 +-
>>>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts 
>>>>> b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>>>> index 9fbbbb556c0b3..df7e5ae9698e1 100644
>>>>> --- a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>>>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
>>>>> @@ -107,7 +107,7 @@ touchscreeen@5d {
>>>>>                  interrupt-parent = <&gpio1>;
>>>>>                  interrupts = <8 IRQ_TYPE_NONE>;
>>>>>                  irq-gpios = <&gpio1 8 GPIO_ACTIVE_HIGH>;
>>>>> -               reset-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
>>>>> +               reset-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;
>>>
>>> NACK!
>>>
>>> The PRT8MM has an inverter in the reset line. The reason for that is 
>>> that the
>>> reset line needs to be inactive when the driving side is unpowered.
>>> The DT was correct, this change will break it.
>>>
>>
>> The DT was correct. The implementation in the driver is changed (the 
>> polarity is swapped) in this patch series, therefore the DT isn't 
>> correct anymore, hence this patch.
> 
> I'm not sure it's quite that simple... FWIW I'm using an add-on LCD 
> module with a GT9271[1] (and I won't be the only one - Raspberry Pi and 
> other SBC users using DT overlays or custom-built DTBs are a whole other 
> can of worms here), where GPIO_ACTIVE_LOW is correctly specified per the 
> schematics, thus "wrong" for the current driver behaviour, yet it *is* 
> working OK as-is. I guess that's because /RSTB ends up driven low for 
> long enough between the current "deassertion" by 
> gpiod_direction_output(1) and gpiod_direction_input() allowing the 
> external pull-up to take it high again.
> 

Correct, that seems like it is the case for "current-wrong" GPIO level, 
provided you have a pull-up on the RESET line (as recommended in the 
datasheet).

c.f. 
https://github.com/hadess/gt9xx/blob/master/specifications/GT9271%20Datasheet.pdf

with the current driver, what I assume we should get is:
             ___________________
INT _______|                   |___________

     ____________           __________________
RST             |_________|

    ^
    L__ pull-up on RST so high by default
         ^
         L___ gpiod_direction_output(0) (deassert GPIO active-low, so high)
            ^
            L____ goodix_irq_direction_output
                 ^
                 L___ gpiod_direction_output(1) (assert GPIO active-low, 
so low)
                           ^
                           L____ gpiod_direction_input() (floating, 
pull-up on RST so high)

This works because of the pull-up on RST and that what matters is that 
the INT lane is configured 100µs before a rising edge on RST line (for 
at least 5ms). However, the init sequence is not properly followed and 
might get broken in the future since it is not something that we 
explicitly support.

With the proposed patch:
             ___________________
INT _______|                   |___________

     ____         __________________
RST     |_______|

    ^
    L__ pull-up on RST so high by default
         ^
         L___ gpiod_direction_output(1) (assert GPIO active-low, so low)
            ^
            L____ goodix_irq_direction_output
                 ^
                 L___ gpiod_direction_output(1) (deassert GPIO 
active-low, so high)
                           ^
                           L____ gpiod_direction_input() (floating, 
pull-up on RST so high)

This should work too and does not rely on some side effects/timings and 
should be future-proof.

As for OOT DTB/DTBO, nothing I can do unfortunately, that is correct.

The fact that we have both "interpretations" of the RESET GPIO level in 
Linux is worrying, albeit with OOT DTBs. A driver or subsystem rework 
might break boards in which case people should update their DTB as 
suggested here or we'll decide to support both interpretations, which 
ain't great either.

Cheers,
Quentin

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH RFC v2 5/7] arm64: dts: imx: fix touchscreen reset GPIO polarity

[David Jander]

On Tue, 22 Nov 2022 17:22:45 +0100
Quentin Schulz <quentin.schulz@theobroma-systems.com> wrote:

> Hi Robin,
> 
> On 11/22/22 13:46, Robin Murphy wrote:
> > On 2022-11-22 09:58, Quentin Schulz wrote:  
> >> Hi David,
> >>
> >> Thanks Fabio for the Cc.
> >>
> >> On 11/22/22 08:18, David Jander wrote:  
> >>> On Mon, 21 Nov 2022 15:18:32 -0300
> >>> Fabio Estevam <festevam@gmail.com> wrote:
> >>>  
> >>>> [Adding Angus and David]  
> >>>
> >>> Thanks. This was apparently necessary ;-)
> >>>  
> >>>> On Mon, Nov 21, 2022 at 3:12 PM Quentin Schulz 
> >>>> <foss+kernel@0leil.net> wrote:  
> >>>>>
> >>>>> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> >>>>>
> >>>>> The reset line is active low for the Goodix touchscreen controller so
> >>>>> let's fix the polarity in the Device Tree node.
> >>>>>
> >>>>> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> >>>>> ---
> >>>>>   arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts         | 2 +-
> >>>>>   arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts | 2 +-
> >>>>>   2 files changed, 2 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts 
> >>>>> b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> >>>>> index 9fbbbb556c0b3..df7e5ae9698e1 100644
> >>>>> --- a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> >>>>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> >>>>> @@ -107,7 +107,7 @@ touchscreeen@5d {
> >>>>>                  interrupt-parent = <&gpio1>;
> >>>>>                  interrupts = <8 IRQ_TYPE_NONE>;
> >>>>>                  irq-gpios = <&gpio1 8 GPIO_ACTIVE_HIGH>;
> >>>>> -               reset-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
> >>>>> +               reset-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;  
> >>>
> >>> NACK!
> >>>
> >>> The PRT8MM has an inverter in the reset line. The reason for that is 
> >>> that the
> >>> reset line needs to be inactive when the driving side is unpowered.
> >>> The DT was correct, this change will break it.
> >>>  
> >>
> >> The DT was correct. The implementation in the driver is changed (the 
> >> polarity is swapped) in this patch series, therefore the DT isn't 
> >> correct anymore, hence this patch.  
> > 
> > I'm not sure it's quite that simple... FWIW I'm using an add-on LCD 
> > module with a GT9271[1] (and I won't be the only one - Raspberry Pi and 
> > other SBC users using DT overlays or custom-built DTBs are a whole other 
> > can of worms here), where GPIO_ACTIVE_LOW is correctly specified per the 
> > schematics, thus "wrong" for the current driver behaviour, yet it *is* 
> > working OK as-is. I guess that's because /RSTB ends up driven low for 
> > long enough between the current "deassertion" by 
> > gpiod_direction_output(1) and gpiod_direction_input() allowing the 
> > external pull-up to take it high again.
> >   
> 
> Correct, that seems like it is the case for "current-wrong" GPIO level, 
> provided you have a pull-up on the RESET line (as recommended in the 
> datasheet).
> 
> c.f. 
> https://github.com/hadess/gt9xx/blob/master/specifications/GT9271%20Datasheet.pdf
> 
> with the current driver, what I assume we should get is:
>              ___________________
> INT _______|                   |___________
> 
>      ____________           __________________
> RST             |_________|
> 
>     ^
>     L__ pull-up on RST so high by default
>          ^
>          L___ gpiod_direction_output(0) (deassert GPIO active-low, so high)
>             ^
>             L____ goodix_irq_direction_output
>                  ^
>                  L___ gpiod_direction_output(1) (assert GPIO active-low, 
> so low)
>                            ^
>                            L____ gpiod_direction_input() (floating, 
> pull-up on RST so high)
> 
> This works because of the pull-up on RST and that what matters is that 
> the INT lane is configured 100µs before a rising edge on RST line (for 
> at least 5ms). However, the init sequence is not properly followed and 
> might get broken in the future since it is not something that we 
> explicitly support.
> 
> With the proposed patch:
>              ___________________
> INT _______|                   |___________
> 
>      ____         __________________
> RST     |_______|
> 
>     ^
>     L__ pull-up on RST so high by default
>          ^
>          L___ gpiod_direction_output(1) (assert GPIO active-low, so low)
>             ^
>             L____ goodix_irq_direction_output
>                  ^
>                  L___ gpiod_direction_output(1) (deassert GPIO 
> active-low, so high)
>                            ^
>                            L____ gpiod_direction_input() (floating, 
> pull-up on RST so high)
> 
> This should work too and does not rely on some side effects/timings and 
> should be future-proof.
> 
> As for OOT DTB/DTBO, nothing I can do unfortunately, that is correct.
> 
> The fact that we have both "interpretations" of the RESET GPIO level in 
> Linux is worrying, albeit with OOT DTBs. A driver or subsystem rework 
> might break boards in which case people should update their DTB as 
> suggested here or we'll decide to support both interpretations, which 
> ain't great either.

Thanks for the detailed drawings. Yes, it seems this is an unfortunate case of
both polarities kind of working.

Our DT was correct, but the driver was wrong, even if it seemed to work
correctly, so this NACK still stands.

Our board has this GPIO pin wired to the gate of a MOSFET which acts as an
inverter and also isolates the signal from the GPIO port power rail. So from
the GPIO point of view, the signal is ACTIVE_HIGH. Please leave the DT like
this.

Thanks for fixing the driver.

Best regards,

-- 
David Jander
Protonic Holland.

Re: [PATCH RFC v2 5/7] arm64: dts: imx: fix touchscreen reset GPIO polarity

[David Jander]

On Tue, 22 Nov 2022 17:22:45 +0100
Quentin Schulz <quentin.schulz@theobroma-systems.com> wrote:

> Hi Robin,
> 
> On 11/22/22 13:46, Robin Murphy wrote:
> > On 2022-11-22 09:58, Quentin Schulz wrote:  
> >> Hi David,
> >>
> >> Thanks Fabio for the Cc.
> >>
> >> On 11/22/22 08:18, David Jander wrote:  
> >>> On Mon, 21 Nov 2022 15:18:32 -0300
> >>> Fabio Estevam <festevam@gmail.com> wrote:
> >>>  
> >>>> [Adding Angus and David]  
> >>>
> >>> Thanks. This was apparently necessary ;-)
> >>>  
> >>>> On Mon, Nov 21, 2022 at 3:12 PM Quentin Schulz 
> >>>> <foss+kernel@0leil.net> wrote:  
> >>>>>
> >>>>> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> >>>>>
> >>>>> The reset line is active low for the Goodix touchscreen controller so
> >>>>> let's fix the polarity in the Device Tree node.
> >>>>>
> >>>>> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> >>>>> ---
> >>>>>   arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts         | 2 +-
> >>>>>   arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts | 2 +-
> >>>>>   2 files changed, 2 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts 
> >>>>> b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> >>>>> index 9fbbbb556c0b3..df7e5ae9698e1 100644
> >>>>> --- a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> >>>>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> >>>>> @@ -107,7 +107,7 @@ touchscreeen@5d {
> >>>>>                  interrupt-parent = <&gpio1>;
> >>>>>                  interrupts = <8 IRQ_TYPE_NONE>;
> >>>>>                  irq-gpios = <&gpio1 8 GPIO_ACTIVE_HIGH>;
> >>>>> -               reset-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
> >>>>> +               reset-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;  
> >>>
> >>> NACK!
> >>>
> >>> The PRT8MM has an inverter in the reset line. The reason for that is 
> >>> that the
> >>> reset line needs to be inactive when the driving side is unpowered.
> >>> The DT was correct, this change will break it.
> >>>  
> >>
> >> The DT was correct. The implementation in the driver is changed (the 
> >> polarity is swapped) in this patch series, therefore the DT isn't 
> >> correct anymore, hence this patch.  
> > 
> > I'm not sure it's quite that simple... FWIW I'm using an add-on LCD 
> > module with a GT9271[1] (and I won't be the only one - Raspberry Pi and 
> > other SBC users using DT overlays or custom-built DTBs are a whole other 
> > can of worms here), where GPIO_ACTIVE_LOW is correctly specified per the 
> > schematics, thus "wrong" for the current driver behaviour, yet it *is* 
> > working OK as-is. I guess that's because /RSTB ends up driven low for 
> > long enough between the current "deassertion" by 
> > gpiod_direction_output(1) and gpiod_direction_input() allowing the 
> > external pull-up to take it high again.
> >   
> 
> Correct, that seems like it is the case for "current-wrong" GPIO level, 
> provided you have a pull-up on the RESET line (as recommended in the 
> datasheet).
> 
> c.f. 
> https://github.com/hadess/gt9xx/blob/master/specifications/GT9271%20Datasheet.pdf
> 
> with the current driver, what I assume we should get is:
>              ___________________
> INT _______|                   |___________
> 
>      ____________           __________________
> RST             |_________|
> 
>     ^
>     L__ pull-up on RST so high by default
>          ^
>          L___ gpiod_direction_output(0) (deassert GPIO active-low, so high)
>             ^
>             L____ goodix_irq_direction_output
>                  ^
>                  L___ gpiod_direction_output(1) (assert GPIO active-low, 
> so low)
>                            ^
>                            L____ gpiod_direction_input() (floating, 
> pull-up on RST so high)
> 
> This works because of the pull-up on RST and that what matters is that 
> the INT lane is configured 100µs before a rising edge on RST line (for 
> at least 5ms). However, the init sequence is not properly followed and 
> might get broken in the future since it is not something that we 
> explicitly support.
> 
> With the proposed patch:
>              ___________________
> INT _______|                   |___________
> 
>      ____         __________________
> RST     |_______|
> 
>     ^
>     L__ pull-up on RST so high by default
>          ^
>          L___ gpiod_direction_output(1) (assert GPIO active-low, so low)
>             ^
>             L____ goodix_irq_direction_output
>                  ^
>                  L___ gpiod_direction_output(1) (deassert GPIO 
> active-low, so high)
>                            ^
>                            L____ gpiod_direction_input() (floating, 
> pull-up on RST so high)
> 
> This should work too and does not rely on some side effects/timings and 
> should be future-proof.
> 
> As for OOT DTB/DTBO, nothing I can do unfortunately, that is correct.
> 
> The fact that we have both "interpretations" of the RESET GPIO level in 
> Linux is worrying, albeit with OOT DTBs. A driver or subsystem rework 
> might break boards in which case people should update their DTB as 
> suggested here or we'll decide to support both interpretations, which 
> ain't great either.

Thanks for the detailed drawings. Yes, it seems this is an unfortunate case of
both polarities kind of working.

Our DT was correct, but the driver was wrong, even if it seemed to work
correctly, so this NACK still stands.

Our board has this GPIO pin wired to the gate of a MOSFET which acts as an
inverter and also isolates the signal from the GPIO port power rail. So from
the GPIO point of view, the signal is ACTIVE_HIGH. Please leave the DT like
this.

Thanks for fixing the driver.

Best regards,

-- 
David Jander
Protonic Holland.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH RFC v2 5/7] arm64: dts: imx: fix touchscreen reset GPIO polarity

[David Jander]

On Tue, 22 Nov 2022 17:22:45 +0100
Quentin Schulz <quentin.schulz@theobroma-systems.com> wrote:

> Hi Robin,
> 
> On 11/22/22 13:46, Robin Murphy wrote:
> > On 2022-11-22 09:58, Quentin Schulz wrote:  
> >> Hi David,
> >>
> >> Thanks Fabio for the Cc.
> >>
> >> On 11/22/22 08:18, David Jander wrote:  
> >>> On Mon, 21 Nov 2022 15:18:32 -0300
> >>> Fabio Estevam <festevam@gmail.com> wrote:
> >>>  
> >>>> [Adding Angus and David]  
> >>>
> >>> Thanks. This was apparently necessary ;-)
> >>>  
> >>>> On Mon, Nov 21, 2022 at 3:12 PM Quentin Schulz 
> >>>> <foss+kernel@0leil.net> wrote:  
> >>>>>
> >>>>> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> >>>>>
> >>>>> The reset line is active low for the Goodix touchscreen controller so
> >>>>> let's fix the polarity in the Device Tree node.
> >>>>>
> >>>>> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> >>>>> ---
> >>>>>   arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts         | 2 +-
> >>>>>   arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts | 2 +-
> >>>>>   2 files changed, 2 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts 
> >>>>> b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> >>>>> index 9fbbbb556c0b3..df7e5ae9698e1 100644
> >>>>> --- a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> >>>>> +++ b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
> >>>>> @@ -107,7 +107,7 @@ touchscreeen@5d {
> >>>>>                  interrupt-parent = <&gpio1>;
> >>>>>                  interrupts = <8 IRQ_TYPE_NONE>;
> >>>>>                  irq-gpios = <&gpio1 8 GPIO_ACTIVE_HIGH>;
> >>>>> -               reset-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
> >>>>> +               reset-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;  
> >>>
> >>> NACK!
> >>>
> >>> The PRT8MM has an inverter in the reset line. The reason for that is 
> >>> that the
> >>> reset line needs to be inactive when the driving side is unpowered.
> >>> The DT was correct, this change will break it.
> >>>  
> >>
> >> The DT was correct. The implementation in the driver is changed (the 
> >> polarity is swapped) in this patch series, therefore the DT isn't 
> >> correct anymore, hence this patch.  
> > 
> > I'm not sure it's quite that simple... FWIW I'm using an add-on LCD 
> > module with a GT9271[1] (and I won't be the only one - Raspberry Pi and 
> > other SBC users using DT overlays or custom-built DTBs are a whole other 
> > can of worms here), where GPIO_ACTIVE_LOW is correctly specified per the 
> > schematics, thus "wrong" for the current driver behaviour, yet it *is* 
> > working OK as-is. I guess that's because /RSTB ends up driven low for 
> > long enough between the current "deassertion" by 
> > gpiod_direction_output(1) and gpiod_direction_input() allowing the 
> > external pull-up to take it high again.
> >   
> 
> Correct, that seems like it is the case for "current-wrong" GPIO level, 
> provided you have a pull-up on the RESET line (as recommended in the 
> datasheet).
> 
> c.f. 
> https://github.com/hadess/gt9xx/blob/master/specifications/GT9271%20Datasheet.pdf
> 
> with the current driver, what I assume we should get is:
>              ___________________
> INT _______|                   |___________
> 
>      ____________           __________________
> RST             |_________|
> 
>     ^
>     L__ pull-up on RST so high by default
>          ^
>          L___ gpiod_direction_output(0) (deassert GPIO active-low, so high)
>             ^
>             L____ goodix_irq_direction_output
>                  ^
>                  L___ gpiod_direction_output(1) (assert GPIO active-low, 
> so low)
>                            ^
>                            L____ gpiod_direction_input() (floating, 
> pull-up on RST so high)
> 
> This works because of the pull-up on RST and that what matters is that 
> the INT lane is configured 100µs before a rising edge on RST line (for 
> at least 5ms). However, the init sequence is not properly followed and 
> might get broken in the future since it is not something that we 
> explicitly support.
> 
> With the proposed patch:
>              ___________________
> INT _______|                   |___________
> 
>      ____         __________________
> RST     |_______|
> 
>     ^
>     L__ pull-up on RST so high by default
>          ^
>          L___ gpiod_direction_output(1) (assert GPIO active-low, so low)
>             ^
>             L____ goodix_irq_direction_output
>                  ^
>                  L___ gpiod_direction_output(1) (deassert GPIO 
> active-low, so high)
>                            ^
>                            L____ gpiod_direction_input() (floating, 
> pull-up on RST so high)
> 
> This should work too and does not rely on some side effects/timings and 
> should be future-proof.
> 
> As for OOT DTB/DTBO, nothing I can do unfortunately, that is correct.
> 
> The fact that we have both "interpretations" of the RESET GPIO level in 
> Linux is worrying, albeit with OOT DTBs. A driver or subsystem rework 
> might break boards in which case people should update their DTB as 
> suggested here or we'll decide to support both interpretations, which 
> ain't great either.

Thanks for the detailed drawings. Yes, it seems this is an unfortunate case of
both polarities kind of working.

Our DT was correct, but the driver was wrong, even if it seemed to work
correctly, so this NACK still stands.

Our board has this GPIO pin wired to the gate of a MOSFET which acts as an
inverter and also isolates the signal from the GPIO port power rail. So from
the GPIO point of view, the signal is ACTIVE_HIGH. Please leave the DT like
this.

Thanks for fixing the driver.

Best regards,

-- 
David Jander
Protonic Holland.


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH RFC v2 6/7] arm64: dts: qcom: fix touchscreen reset GPIO polarity

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

The reset line is active low for the Goodix touchscreen controller so
let's fix the polarity in the Device Tree node.

Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
Changes in v2:
- removed Qcom SC7180 Trogdor-based devices changes as they are not using this Goodix driver,

 arch/arm64/boot/dts/qcom/msm8998-fxtec-pro1.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8998-fxtec-pro1.dts b/arch/arm64/boot/dts/qcom/msm8998-fxtec-pro1.dts
index 429ba57e20f71..8f738cade2652 100644
--- a/arch/arm64/boot/dts/qcom/msm8998-fxtec-pro1.dts
+++ b/arch/arm64/boot/dts/qcom/msm8998-fxtec-pro1.dts
@@ -249,7 +249,7 @@ touchscreen@14 {
 		reg = <0x14>;
 		interrupt-parent = <&tlmm>;
 		interrupts = <125 IRQ_TYPE_LEVEL_LOW>;
-		reset-gpios = <&tlmm 89 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&tlmm 89 GPIO_ACTIVE_LOW>;
 		AVDD28-supply = <&vreg_l28_3p0>;
 		VDDIO-supply = <&ts_vio_vreg>;
 		pinctrl-names = "active";

-- 
b4 0.10.1

[PATCH RFC v2 6/7] arm64: dts: qcom: fix touchscreen reset GPIO polarity

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

The reset line is active low for the Goodix touchscreen controller so
let's fix the polarity in the Device Tree node.

Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
Changes in v2:
- removed Qcom SC7180 Trogdor-based devices changes as they are not using this Goodix driver,

 arch/arm64/boot/dts/qcom/msm8998-fxtec-pro1.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8998-fxtec-pro1.dts b/arch/arm64/boot/dts/qcom/msm8998-fxtec-pro1.dts
index 429ba57e20f71..8f738cade2652 100644
--- a/arch/arm64/boot/dts/qcom/msm8998-fxtec-pro1.dts
+++ b/arch/arm64/boot/dts/qcom/msm8998-fxtec-pro1.dts
@@ -249,7 +249,7 @@ touchscreen@14 {
 		reg = <0x14>;
 		interrupt-parent = <&tlmm>;
 		interrupts = <125 IRQ_TYPE_LEVEL_LOW>;
-		reset-gpios = <&tlmm 89 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&tlmm 89 GPIO_ACTIVE_LOW>;
 		AVDD28-supply = <&vreg_l28_3p0>;
 		VDDIO-supply = <&ts_vio_vreg>;
 		pinctrl-names = "active";

-- 
b4 0.10.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH RFC v2 6/7] arm64: dts: qcom: fix touchscreen reset GPIO polarity

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

The reset line is active low for the Goodix touchscreen controller so
let's fix the polarity in the Device Tree node.

Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
Changes in v2:
- removed Qcom SC7180 Trogdor-based devices changes as they are not using this Goodix driver,

 arch/arm64/boot/dts/qcom/msm8998-fxtec-pro1.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8998-fxtec-pro1.dts b/arch/arm64/boot/dts/qcom/msm8998-fxtec-pro1.dts
index 429ba57e20f71..8f738cade2652 100644
--- a/arch/arm64/boot/dts/qcom/msm8998-fxtec-pro1.dts
+++ b/arch/arm64/boot/dts/qcom/msm8998-fxtec-pro1.dts
@@ -249,7 +249,7 @@ touchscreen@14 {
 		reg = <0x14>;
 		interrupt-parent = <&tlmm>;
 		interrupts = <125 IRQ_TYPE_LEVEL_LOW>;
-		reset-gpios = <&tlmm 89 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&tlmm 89 GPIO_ACTIVE_LOW>;
 		AVDD28-supply = <&vreg_l28_3p0>;
 		VDDIO-supply = <&ts_vio_vreg>;
 		pinctrl-names = "active";

-- 
b4 0.10.1

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH RFC v2 6/7] arm64: dts: qcom: fix touchscreen reset GPIO polarity

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

The reset line is active low for the Goodix touchscreen controller so
let's fix the polarity in the Device Tree node.

Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
Changes in v2:
- removed Qcom SC7180 Trogdor-based devices changes as they are not using this Goodix driver,

 arch/arm64/boot/dts/qcom/msm8998-fxtec-pro1.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8998-fxtec-pro1.dts b/arch/arm64/boot/dts/qcom/msm8998-fxtec-pro1.dts
index 429ba57e20f71..8f738cade2652 100644
--- a/arch/arm64/boot/dts/qcom/msm8998-fxtec-pro1.dts
+++ b/arch/arm64/boot/dts/qcom/msm8998-fxtec-pro1.dts
@@ -249,7 +249,7 @@ touchscreen@14 {
 		reg = <0x14>;
 		interrupt-parent = <&tlmm>;
 		interrupts = <125 IRQ_TYPE_LEVEL_LOW>;
-		reset-gpios = <&tlmm 89 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&tlmm 89 GPIO_ACTIVE_LOW>;
 		AVDD28-supply = <&vreg_l28_3p0>;
 		VDDIO-supply = <&ts_vio_vreg>;
 		pinctrl-names = "active";

-- 
b4 0.10.1

[PATCH RFC v2 7/7] arm64: dts: rockchip: fix touchscreen reset GPIO polarity

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

The reset line is active low for the Goodix touchscreen controller so
let's fix the polarity in the Device Tree node.

Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
 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 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/px30-evb.dts b/arch/arm64/boot/dts/rockchip/px30-evb.dts
index 07008d84434c1..7ecd38566e7e4 100644
--- a/arch/arm64/boot/dts/rockchip/px30-evb.dts
+++ b/arch/arm64/boot/dts/rockchip/px30-evb.dts
@@ -420,7 +420,7 @@ touchscreen@14 {
 		interrupt-parent = <&gpio0>;
 		interrupts = <RK_PA5 IRQ_TYPE_LEVEL_LOW>;
 		irq-gpios = <&gpio0 RK_PA5 GPIO_ACTIVE_LOW>;
-		reset-gpios = <&gpio0 RK_PB4 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio0 RK_PB4 GPIO_ACTIVE_LOW>;
 		VDDIO-supply = <&vcc3v3_lcd>;
 	};
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
index 78157521e9449..e63491fb443be 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
@@ -588,7 +588,7 @@ touch: touchscreen@5d {
 		AVDD28-supply = <&vcc3v0_touch>;
 		VDDIO-supply = <&vcc3v0_touch>;
 		irq-gpios = <&gpio4 RK_PD5 GPIO_ACTIVE_HIGH>;
-		reset-gpios = <&gpio4 RK_PD6 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio4 RK_PD6 GPIO_ACTIVE_LOW>;
 		status = "disabled";
 	};
 };
diff --git a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts
index 674792567fa6e..234531aaa430a 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts
@@ -495,7 +495,7 @@ touchscreen0: goodix@14 {
 		irq-gpios = <&gpio0 RK_PB5 GPIO_ACTIVE_HIGH>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&touch_int &touch_rst>;
-		reset-gpios = <&gpio0 RK_PB6 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio0 RK_PB6 GPIO_ACTIVE_LOW>;
 		VDDIO-supply = <&vcc3v3_lcd0_n>;
 	};
 };

-- 
b4 0.10.1

[PATCH RFC v2 7/7] arm64: dts: rockchip: fix touchscreen reset GPIO polarity

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

The reset line is active low for the Goodix touchscreen controller so
let's fix the polarity in the Device Tree node.

Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
 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 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/px30-evb.dts b/arch/arm64/boot/dts/rockchip/px30-evb.dts
index 07008d84434c1..7ecd38566e7e4 100644
--- a/arch/arm64/boot/dts/rockchip/px30-evb.dts
+++ b/arch/arm64/boot/dts/rockchip/px30-evb.dts
@@ -420,7 +420,7 @@ touchscreen@14 {
 		interrupt-parent = <&gpio0>;
 		interrupts = <RK_PA5 IRQ_TYPE_LEVEL_LOW>;
 		irq-gpios = <&gpio0 RK_PA5 GPIO_ACTIVE_LOW>;
-		reset-gpios = <&gpio0 RK_PB4 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio0 RK_PB4 GPIO_ACTIVE_LOW>;
 		VDDIO-supply = <&vcc3v3_lcd>;
 	};
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
index 78157521e9449..e63491fb443be 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
@@ -588,7 +588,7 @@ touch: touchscreen@5d {
 		AVDD28-supply = <&vcc3v0_touch>;
 		VDDIO-supply = <&vcc3v0_touch>;
 		irq-gpios = <&gpio4 RK_PD5 GPIO_ACTIVE_HIGH>;
-		reset-gpios = <&gpio4 RK_PD6 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio4 RK_PD6 GPIO_ACTIVE_LOW>;
 		status = "disabled";
 	};
 };
diff --git a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts
index 674792567fa6e..234531aaa430a 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts
@@ -495,7 +495,7 @@ touchscreen0: goodix@14 {
 		irq-gpios = <&gpio0 RK_PB5 GPIO_ACTIVE_HIGH>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&touch_int &touch_rst>;
-		reset-gpios = <&gpio0 RK_PB6 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio0 RK_PB6 GPIO_ACTIVE_LOW>;
 		VDDIO-supply = <&vcc3v3_lcd0_n>;
 	};
 };

-- 
b4 0.10.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH RFC v2 7/7] arm64: dts: rockchip: fix touchscreen reset GPIO polarity

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

The reset line is active low for the Goodix touchscreen controller so
let's fix the polarity in the Device Tree node.

Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
 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 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/px30-evb.dts b/arch/arm64/boot/dts/rockchip/px30-evb.dts
index 07008d84434c1..7ecd38566e7e4 100644
--- a/arch/arm64/boot/dts/rockchip/px30-evb.dts
+++ b/arch/arm64/boot/dts/rockchip/px30-evb.dts
@@ -420,7 +420,7 @@ touchscreen@14 {
 		interrupt-parent = <&gpio0>;
 		interrupts = <RK_PA5 IRQ_TYPE_LEVEL_LOW>;
 		irq-gpios = <&gpio0 RK_PA5 GPIO_ACTIVE_LOW>;
-		reset-gpios = <&gpio0 RK_PB4 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio0 RK_PB4 GPIO_ACTIVE_LOW>;
 		VDDIO-supply = <&vcc3v3_lcd>;
 	};
 
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
index 78157521e9449..e63491fb443be 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
@@ -588,7 +588,7 @@ touch: touchscreen@5d {
 		AVDD28-supply = <&vcc3v0_touch>;
 		VDDIO-supply = <&vcc3v0_touch>;
 		irq-gpios = <&gpio4 RK_PD5 GPIO_ACTIVE_HIGH>;
-		reset-gpios = <&gpio4 RK_PD6 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio4 RK_PD6 GPIO_ACTIVE_LOW>;
 		status = "disabled";
 	};
 };
diff --git a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts
index 674792567fa6e..234531aaa430a 100644
--- a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts
@@ -495,7 +495,7 @@ touchscreen0: goodix@14 {
 		irq-gpios = <&gpio0 RK_PB5 GPIO_ACTIVE_HIGH>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&touch_int &touch_rst>;
-		reset-gpios = <&gpio0 RK_PB6 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio0 RK_PB6 GPIO_ACTIVE_LOW>;
 		VDDIO-supply = <&vcc3v3_lcd0_n>;
 	};
 };

-- 
b4 0.10.1

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip