[PATCH 1/3 v2] Input: atmel_mxt_ts - Fix up inverted RESET handler

[PATCH 1/3 v2] Input: atmel_mxt_ts - Fix up inverted RESET handler

[Linus Walleij]

This driver uses GPIO descriptors to drive the touchscreen
RESET line. In the existing device trees this has in
conflict with intution been flagged as GPIO_ACTIVE_HIGH
and the driver then applies the reverse action by
driving the line low (setting to 0) to enter
reset state and driving the line high (setting to 1) to
get out of reset state.

The correct way to handle active low GPIO lines is to
provide the GPIO_ACTIVE_LOW in the device tree (thus
properly describing the hardware) and letting the GPIO
framework invert the assertion (driving high) to a low
level and vice versa.

This is considered a bug since the device trees are
incorrectly mis-specifying the line as active high.

Fix the driver and all device trees specifying a reset
line.

Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-tegra@vger.kernel.org
Cc: linux-samsung-soc@vger.kernel.org
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- New patch fixing this confusion before adding the
  new YAML bindings.
- CC some misc maintainers and mailing lists that should
  be aware that we do this change.
---
 arch/arm/boot/dts/imx53-ppd.dts                 | 2 +-
 arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts    | 2 +-
 arch/arm/boot/dts/imx6q-apalis-eval.dts         | 2 +-
 arch/arm/boot/dts/imx6q-apalis-ixora-v1.1.dts   | 2 +-
 arch/arm/boot/dts/imx6q-apalis-ixora.dts        | 2 +-
 arch/arm/boot/dts/imx7-colibri-aster.dtsi       | 2 +-
 arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi     | 2 +-
 arch/arm/boot/dts/motorola-mapphone-common.dtsi | 2 +-
 arch/arm/boot/dts/s5pv210-aries.dtsi            | 2 +-
 arch/arm/boot/dts/tegra20-acer-a500-picasso.dts | 2 +-
 drivers/input/touchscreen/atmel_mxt_ts.c        | 6 ++++--
 11 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/arch/arm/boot/dts/imx53-ppd.dts b/arch/arm/boot/dts/imx53-ppd.dts
index f7dcdf96e5c0..8f4a63ea912e 100644
--- a/arch/arm/boot/dts/imx53-ppd.dts
+++ b/arch/arm/boot/dts/imx53-ppd.dts
@@ -589,7 +589,7 @@ &i2c2 {
 
 	touchscreen@4b {
 		compatible = "atmel,maxtouch";
-		reset-gpio = <&gpio5 19 GPIO_ACTIVE_HIGH>;
+		reset-gpio = <&gpio5 19 GPIO_ACTIVE_LOW>;
 		reg = <0x4b>;
 		interrupt-parent = <&gpio5>;
 		interrupts = <4 IRQ_TYPE_LEVEL_LOW>;
diff --git a/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts b/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts
index 65359aece950..7da74e6f46d9 100644
--- a/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts
+++ b/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts
@@ -143,7 +143,7 @@ touchscreen@4a {
 		reg = <0x4a>;
 		interrupt-parent = <&gpio1>;
 		interrupts = <9 IRQ_TYPE_EDGE_FALLING>;		/* SODIMM 28 */
-		reset-gpios = <&gpio2 10 GPIO_ACTIVE_HIGH>;	/* SODIMM 30 */
+		reset-gpios = <&gpio2 10 GPIO_ACTIVE_LOW>;	/* SODIMM 30 */
 		status = "disabled";
 	};
 
diff --git a/arch/arm/boot/dts/imx6q-apalis-eval.dts b/arch/arm/boot/dts/imx6q-apalis-eval.dts
index fab83abb6466..a0683b4aeca1 100644
--- a/arch/arm/boot/dts/imx6q-apalis-eval.dts
+++ b/arch/arm/boot/dts/imx6q-apalis-eval.dts
@@ -140,7 +140,7 @@ touchscreen@4a {
 		reg = <0x4a>;
 		interrupt-parent = <&gpio6>;
 		interrupts = <10 IRQ_TYPE_EDGE_FALLING>;
-		reset-gpios = <&gpio6 9 GPIO_ACTIVE_HIGH>; /* SODIMM 13 */
+		reset-gpios = <&gpio6 9 GPIO_ACTIVE_LOW>; /* SODIMM 13 */
 		status = "disabled";
 	};
 
diff --git a/arch/arm/boot/dts/imx6q-apalis-ixora-v1.1.dts b/arch/arm/boot/dts/imx6q-apalis-ixora-v1.1.dts
index 1614b1ae501d..86e84781cf5d 100644
--- a/arch/arm/boot/dts/imx6q-apalis-ixora-v1.1.dts
+++ b/arch/arm/boot/dts/imx6q-apalis-ixora-v1.1.dts
@@ -145,7 +145,7 @@ touchscreen@4a {
 		reg = <0x4a>;
 		interrupt-parent = <&gpio6>;
 		interrupts = <10 IRQ_TYPE_EDGE_FALLING>;
-		reset-gpios = <&gpio6 9 GPIO_ACTIVE_HIGH>; /* SODIMM 13 */
+		reset-gpios = <&gpio6 9 GPIO_ACTIVE_LOW>; /* SODIMM 13 */
 		status = "disabled";
 	};
 
diff --git a/arch/arm/boot/dts/imx6q-apalis-ixora.dts b/arch/arm/boot/dts/imx6q-apalis-ixora.dts
index fa9f98dd15ac..62e72773e53b 100644
--- a/arch/arm/boot/dts/imx6q-apalis-ixora.dts
+++ b/arch/arm/boot/dts/imx6q-apalis-ixora.dts
@@ -144,7 +144,7 @@ touchscreen@4a {
 		reg = <0x4a>;
 		interrupt-parent = <&gpio6>;
 		interrupts = <10 IRQ_TYPE_EDGE_FALLING>;
-		reset-gpios = <&gpio6 9 GPIO_ACTIVE_HIGH>; /* SODIMM 13 */
+		reset-gpios = <&gpio6 9 GPIO_ACTIVE_LOW>; /* SODIMM 13 */
 		status = "disabled";
 	};
 
diff --git a/arch/arm/boot/dts/imx7-colibri-aster.dtsi b/arch/arm/boot/dts/imx7-colibri-aster.dtsi
index 9fa701bec2ec..139188eb9f40 100644
--- a/arch/arm/boot/dts/imx7-colibri-aster.dtsi
+++ b/arch/arm/boot/dts/imx7-colibri-aster.dtsi
@@ -99,7 +99,7 @@ touchscreen@4a {
 		reg = <0x4a>;
 		interrupt-parent = <&gpio2>;
 		interrupts = <15 IRQ_TYPE_EDGE_FALLING>;	/* SODIMM 107 */
-		reset-gpios = <&gpio2 28 GPIO_ACTIVE_HIGH>;	/* SODIMM 106 */
+		reset-gpios = <&gpio2 28 GPIO_ACTIVE_LOW>;	/* SODIMM 106 */
 	};
 
 	/* M41T0M6 real time clock on carrier board */
diff --git a/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi b/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
index 97601375f264..3caf450735d7 100644
--- a/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
+++ b/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
@@ -124,7 +124,7 @@ touchscreen@4a {
 		reg = <0x4a>;
 		interrupt-parent = <&gpio1>;
 		interrupts = <9 IRQ_TYPE_EDGE_FALLING>;		/* SODIMM 28 */
-		reset-gpios = <&gpio1 10 GPIO_ACTIVE_HIGH>;	/* SODIMM 30 */
+		reset-gpios = <&gpio1 10 GPIO_ACTIVE_LOW>;	/* SODIMM 30 */
 		status = "disabled";
 	};
 
diff --git a/arch/arm/boot/dts/motorola-mapphone-common.dtsi b/arch/arm/boot/dts/motorola-mapphone-common.dtsi
index d5ded4f794df..5f8f77cfbe59 100644
--- a/arch/arm/boot/dts/motorola-mapphone-common.dtsi
+++ b/arch/arm/boot/dts/motorola-mapphone-common.dtsi
@@ -430,7 +430,7 @@ touchscreen@4a {
 		pinctrl-names = "default";
 		pinctrl-0 = <&touchscreen_pins>;
 
-		reset-gpios = <&gpio6 13 GPIO_ACTIVE_HIGH>; /* gpio173 */
+		reset-gpios = <&gpio6 13 GPIO_ACTIVE_LOW>; /* gpio173 */
 
 		/* gpio_183 with sys_nirq2 pad as wakeup */
 		interrupts-extended = <&gpio6 23 IRQ_TYPE_LEVEL_LOW>,
diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi b/arch/arm/boot/dts/s5pv210-aries.dtsi
index bd4450dbdcb6..4da33d0f2748 100644
--- a/arch/arm/boot/dts/s5pv210-aries.dtsi
+++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
@@ -632,7 +632,7 @@ touchscreen@4a {
 		interrupts = <5 IRQ_TYPE_EDGE_FALLING>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&ts_irq>;
-		reset-gpios = <&gpj1 3 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpj1 3 GPIO_ACTIVE_LOW>;
 	};
 };
 
diff --git a/arch/arm/boot/dts/tegra20-acer-a500-picasso.dts b/arch/arm/boot/dts/tegra20-acer-a500-picasso.dts
index a0b829738e8f..10794a870776 100644
--- a/arch/arm/boot/dts/tegra20-acer-a500-picasso.dts
+++ b/arch/arm/boot/dts/tegra20-acer-a500-picasso.dts
@@ -446,7 +446,7 @@ touchscreen@4c {
 			interrupt-parent = <&gpio>;
 			interrupts = <TEGRA_GPIO(V, 6) IRQ_TYPE_LEVEL_LOW>;
 
-			reset-gpios = <&gpio TEGRA_GPIO(Q, 7) GPIO_ACTIVE_HIGH>;
+			reset-gpios = <&gpio TEGRA_GPIO(Q, 7) GPIO_ACTIVE_LOW>;
 
 			avdd-supply = <&vdd_3v3_sys>;
 			vdd-supply  = <&vdd_3v3_sys>;
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 98f17fa3a892..ef7915400c9f 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -3134,8 +3134,9 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	if (error)
 		return error;
 
+	/* Request the RESET line as asserted so we go into reset */
 	data->reset_gpio = devm_gpiod_get_optional(&client->dev,
-						   "reset", GPIOD_OUT_LOW);
+						   "reset", GPIOD_OUT_HIGH);
 	if (IS_ERR(data->reset_gpio)) {
 		error = PTR_ERR(data->reset_gpio);
 		dev_err(&client->dev, "Failed to get reset gpio: %d\n", error);
@@ -3153,8 +3154,9 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	disable_irq(client->irq);
 
 	if (data->reset_gpio) {
+		/* Wait a while and then de-assert the RESET GPIO line */
 		msleep(MXT_RESET_GPIO_TIME);
-		gpiod_set_value(data->reset_gpio, 1);
+		gpiod_set_value(data->reset_gpio, 0);
 		msleep(MXT_RESET_INVALID_CHG);
 	}
 
-- 
2.26.2

[PATCH 2/3 v2] Input: atmel_mxt_ts - Convert bindings to YAML and extend

[Linus Walleij]

This converts the Armel MXT touchscreen bindings to YAML
format and extends them with the following two properties:

- vdda-supply: the optional analog supply voltage
- vdd-supply: the optional digital supply voltage

I also explained about the reset-gpios property that this
better be flagged as active high (0) despite actually
being active low, because all current device trees and
drivers assume that this is the case and will actively
drive the line low to assert RESET.

Tested the schema with all in-tree users and they verify
fine.

Cc: Nick Dyer <nick@shmanahar.org>
Cc: Stephan Gerhold <stephan@gerhold.net>
Cc: devicetree@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- State that the GPIO lines shall be flagged as active low.
  We will fix all users.
---
 .../bindings/input/atmel,maxtouch.txt         | 41 ----------
 .../bindings/input/atmel,maxtouch.yaml        | 81 +++++++++++++++++++
 MAINTAINERS                                   |  2 +-
 3 files changed, 82 insertions(+), 42 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/input/atmel,maxtouch.txt
 create mode 100644 Documentation/devicetree/bindings/input/atmel,maxtouch.yaml

diff --git a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt b/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
deleted file mode 100644
index c88919480d37..000000000000
--- a/Documentation/devicetree/bindings/input/atmel,maxtouch.txt
+++ /dev/null
@@ -1,41 +0,0 @@
-Atmel maXTouch touchscreen/touchpad
-
-Required properties:
-- compatible:
-    atmel,maxtouch
-
-    The following compatibles have been used in various products but are
-    deprecated:
-	atmel,qt602240_ts
-	atmel,atmel_mxt_ts
-	atmel,atmel_mxt_tp
-	atmel,mXT224
-
-- reg: The I2C address of the device
-
-- interrupts: The sink for the touchpad's IRQ output
-    See ../interrupt-controller/interrupts.txt
-
-Optional properties for main touchpad device:
-
-- linux,gpio-keymap: When enabled, the SPT_GPIOPWN_T19 object sends messages
-    on GPIO bit changes. An array of up to 8 entries can be provided
-    indicating the Linux keycode mapped to each bit of the status byte,
-    starting at the LSB. Linux keycodes are defined in
-    <dt-bindings/input/input.h>.
-
-    Note: the numbering of the GPIOs and the bit they start at varies between
-    maXTouch devices. You must either refer to the documentation, or
-    experiment to determine which bit corresponds to which input. Use
-    KEY_RESERVED for unused padding values.
-
-- reset-gpios: GPIO specifier for the touchscreen's reset pin (active low)
-
-Example:
-
-	touch@4b {
-		compatible = "atmel,maxtouch";
-		reg = <0x4b>;
-		interrupt-parent = <&gpio>;
-		interrupts = <TEGRA_GPIO(W, 3) IRQ_TYPE_LEVEL_LOW>;
-	};
diff --git a/Documentation/devicetree/bindings/input/atmel,maxtouch.yaml b/Documentation/devicetree/bindings/input/atmel,maxtouch.yaml
new file mode 100644
index 000000000000..8c6418f76e94
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/atmel,maxtouch.yaml
@@ -0,0 +1,81 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/atmel,maxtouch.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Atmel maXTouch touchscreen/touchpad
+
+maintainers:
+  - Nick Dyer <nick@shmanahar.org>
+  - Linus Walleij <linus.walleij@linaro.org>
+
+description: |
+  Atmel maXTouch touchscreen or touchpads such as the mXT244
+  and similar devices.
+
+properties:
+  compatible:
+    const: atmel,maxtouch
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  vdda-supply:
+    description:
+      Optional regulator for the AVDD analog voltage.
+
+  vdd-supply:
+    description:
+      Optional regulator for the VDD digital voltage.
+
+  reset-gpios:
+    maxItems: 1
+    description:
+      Optional GPIO specifier for the touchscreen's reset pin
+      (active low). The line must be flagged with
+      GPIO_ACTIVE_LOW.
+
+  linux,gpio-keymap:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description: |
+      When enabled, the SPT_GPIOPWN_T19 object sends messages
+      on GPIO bit changes. An array of up to 8 entries can be provided
+      indicating the Linux keycode mapped to each bit of the status byte,
+      starting at the LSB. Linux keycodes are defined in
+      <dt-bindings/input/input.h>.
+
+      Note: the numbering of the GPIOs and the bit they start at varies
+      between maXTouch devices. You must either refer to the documentation,
+      or experiment to determine which bit corresponds to which input. Use
+      KEY_RESERVED for unused padding values.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/gpio/gpio.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      touchscreen@4a {
+        compatible = "atmel,maxtouch";
+        reg = <0x4a>;
+        interrupt-parent = <&gpio>;
+        interrupts = <26 IRQ_TYPE_EDGE_FALLING>;
+        reset-gpios = <&gpio 27 GPIO_ACTIVE_LOW>;
+        vdda-supply = <&ab8500_ldo_aux2_reg>;
+        vdd-supply = <&ab8500_ldo_aux5_reg>;
+      };
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index e73636b75f29..b4b46fcb82db 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2966,7 +2966,7 @@ ATMEL MAXTOUCH DRIVER
 M:	Nick Dyer <nick@shmanahar.org>
 S:	Maintained
 T:	git git://github.com/ndyer/linux.git
-F:	Documentation/devicetree/bindings/input/atmel,maxtouch.txt
+F:	Documentation/devicetree/bindings/input/atmel,maxtouch.yaml
 F:	drivers/input/touchscreen/atmel_mxt_ts.c
 
 ATMEL WIRELESS DRIVER
-- 
2.26.2

[PATCH 3/3 v2] Input: atmel_mxt_ts - Support regulator supplies

[Linus Walleij]

This adds the code for the Atmel touchscreens such as
mXT224 to obtain power regulators for the supply voltages
AVDD and VDD. On mobile phones such as Samsung GT-I8190
(Golden) this is needed to explicitly bring power online.

We just enable the regulators at probe() and disable
them at remove() or in the errorpath for now.

As regulators are naturally stubbed if not available,
this should have no impact on existing systems.

Cc: Nick Dyer <nick@shmanahar.org>
Cc: Stephan Gerhold <stephan@gerhold.net>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Resend with the other changes.
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 37 +++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index ef7915400c9f..e34984388791 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -24,6 +24,7 @@
 #include <linux/of.h>
 #include <linux/property.h>
 #include <linux/slab.h>
+#include <linux/regulator/consumer.h>
 #include <linux/gpio/consumer.h>
 #include <asm/unaligned.h>
 #include <media/v4l2-device.h>
@@ -309,6 +310,7 @@ struct mxt_data {
 	u8 multitouch;
 	struct t7_config t7_cfg;
 	struct mxt_dbg dbg;
+	struct regulator_bulk_data regulators[2];
 	struct gpio_desc *reset_gpio;
 	bool use_retrigen_workaround;
 
@@ -3134,6 +3136,21 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	if (error)
 		return error;
 
+	/*
+	 * VDDA is the analog voltage supply 2.57..3.47 V
+	 * VDD  is the digital voltage supply 1.71..3.47 V
+	 */
+	data->regulators[0].supply = "vdda";
+	data->regulators[1].supply = "vdd";
+	error = devm_regulator_bulk_get(&client->dev, ARRAY_SIZE(data->regulators),
+					data->regulators);
+	if (error) {
+		if (error != -EPROBE_DEFER)
+			dev_err(&client->dev, "Failed to get regulators %d\n",
+				error);
+		return error;
+	}
+
 	/* Request the RESET line as asserted so we go into reset */
 	data->reset_gpio = devm_gpiod_get_optional(&client->dev,
 						   "reset", GPIOD_OUT_HIGH);
@@ -3153,6 +3170,19 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
 
 	disable_irq(client->irq);
 
+	error = regulator_bulk_enable(ARRAY_SIZE(data->regulators),
+				      data->regulators);
+	if (error) {
+		dev_err(&client->dev, "failed to enable regulators: %d\n",
+			error);
+		return error;
+	}
+	/*
+	 * The device takes 40ms to come up after power-on according
+	 * to the mXT224 datasheet, page 13.
+	 */
+	msleep(MXT_BACKUP_TIME);
+
 	if (data->reset_gpio) {
 		/* Wait a while and then de-assert the RESET GPIO line */
 		msleep(MXT_RESET_GPIO_TIME);
@@ -3162,7 +3192,7 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
 
 	error = mxt_initialize(data);
 	if (error)
-		return error;
+		goto err_disable_regulators;
 
 	error = sysfs_create_group(&client->dev.kobj, &mxt_attr_group);
 	if (error) {
@@ -3176,6 +3206,9 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
 err_free_object:
 	mxt_free_input_device(data);
 	mxt_free_object_table(data);
+err_disable_regulators:
+	regulator_bulk_disable(ARRAY_SIZE(data->regulators),
+			       data->regulators);
 	return error;
 }
 
@@ -3187,6 +3220,8 @@ static int mxt_remove(struct i2c_client *client)
 	sysfs_remove_group(&client->dev.kobj, &mxt_attr_group);
 	mxt_free_input_device(data);
 	mxt_free_object_table(data);
+	regulator_bulk_disable(ARRAY_SIZE(data->regulators),
+			       data->regulators);
 
 	return 0;
 }
-- 
2.26.2

Re: [PATCH 1/3 v2] Input: atmel_mxt_ts - Fix up inverted RESET handler

[Philippe Schenker]

On Wed, 2020-11-04 at 16:30 +0100, Linus Walleij wrote:
> This driver uses GPIO descriptors to drive the touchscreen
> RESET line. In the existing device trees this has in
> conflict with intution been flagged as GPIO_ACTIVE_HIGH
> and the driver then applies the reverse action by
> driving the line low (setting to 0) to enter
> reset state and driving the line high (setting to 1) to
> get out of reset state.
> 
> The correct way to handle active low GPIO lines is to
> provide the GPIO_ACTIVE_LOW in the device tree (thus
> properly describing the hardware) and letting the GPIO
> framework invert the assertion (driving high) to a low
> level and vice versa.
> 
> This is considered a bug since the device trees are
> incorrectly mis-specifying the line as active high.
> 
> Fix the driver and all device trees specifying a reset
> line.
> 
> Cc: Krzysztof Kozlowski <krzk@kernel.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-tegra@vger.kernel.org
> Cc: linux-samsung-soc@vger.kernel.org
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Thanks for fixing this!

Reviewed-by: Philippe Schenker <philippe.schenker@toradex.com>

> ---
> ChangeLog v1->v2:
> - New patch fixing this confusion before adding the
>   new YAML bindings.
> - CC some misc maintainers and mailing lists that should
>   be aware that we do this change.
> ---
>  arch/arm/boot/dts/imx53-ppd.dts                 | 2 +-
>  arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts    | 2 +-
>  arch/arm/boot/dts/imx6q-apalis-eval.dts         | 2 +-
>  arch/arm/boot/dts/imx6q-apalis-ixora-v1.1.dts   | 2 +-
>  arch/arm/boot/dts/imx6q-apalis-ixora.dts        | 2 +-
>  arch/arm/boot/dts/imx7-colibri-aster.dtsi       | 2 +-
>  arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi     | 2 +-
>  arch/arm/boot/dts/motorola-mapphone-common.dtsi | 2 +-
>  arch/arm/boot/dts/s5pv210-aries.dtsi            | 2 +-
>  arch/arm/boot/dts/tegra20-acer-a500-picasso.dts | 2 +-
>  drivers/input/touchscreen/atmel_mxt_ts.c        | 6 ++++--
>  11 files changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx53-ppd.dts
> b/arch/arm/boot/dts/imx53-ppd.dts
> index f7dcdf96e5c0..8f4a63ea912e 100644
> --- a/arch/arm/boot/dts/imx53-ppd.dts
> +++ b/arch/arm/boot/dts/imx53-ppd.dts
> @@ -589,7 +589,7 @@ &i2c2 {
>  
>  	touchscreen@4b {
>  		compatible = "atmel,maxtouch";
> -		reset-gpio = <&gpio5 19 GPIO_ACTIVE_HIGH>;
> +		reset-gpio = <&gpio5 19 GPIO_ACTIVE_LOW>;
>  		reg = <0x4b>;
>  		interrupt-parent = <&gpio5>;
>  		interrupts = <4 IRQ_TYPE_LEVEL_LOW>;
> diff --git a/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts
> b/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts
> index 65359aece950..7da74e6f46d9 100644
> --- a/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts
> +++ b/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts
> @@ -143,7 +143,7 @@ touchscreen@4a {
>  		reg = <0x4a>;
>  		interrupt-parent = <&gpio1>;
>  		interrupts = <9 IRQ_TYPE_EDGE_FALLING>;		/*
> SODIMM 28 */
> -		reset-gpios = <&gpio2 10 GPIO_ACTIVE_HIGH>;	/*
> SODIMM 30 */
> +		reset-gpios = <&gpio2 10 GPIO_ACTIVE_LOW>;	/*
> SODIMM 30 */
>  		status = "disabled";
>  	};
>  
> diff --git a/arch/arm/boot/dts/imx6q-apalis-eval.dts
> b/arch/arm/boot/dts/imx6q-apalis-eval.dts
> index fab83abb6466..a0683b4aeca1 100644
> --- a/arch/arm/boot/dts/imx6q-apalis-eval.dts
> +++ b/arch/arm/boot/dts/imx6q-apalis-eval.dts
> @@ -140,7 +140,7 @@ touchscreen@4a {
>  		reg = <0x4a>;
>  		interrupt-parent = <&gpio6>;
>  		interrupts = <10 IRQ_TYPE_EDGE_FALLING>;
> -		reset-gpios = <&gpio6 9 GPIO_ACTIVE_HIGH>; /* SODIMM 13
> */
> +		reset-gpios = <&gpio6 9 GPIO_ACTIVE_LOW>; /* SODIMM 13
> */
>  		status = "disabled";
>  	};
>  
> diff --git a/arch/arm/boot/dts/imx6q-apalis-ixora-v1.1.dts
> b/arch/arm/boot/dts/imx6q-apalis-ixora-v1.1.dts
> index 1614b1ae501d..86e84781cf5d 100644
> --- a/arch/arm/boot/dts/imx6q-apalis-ixora-v1.1.dts
> +++ b/arch/arm/boot/dts/imx6q-apalis-ixora-v1.1.dts
> @@ -145,7 +145,7 @@ touchscreen@4a {
>  		reg = <0x4a>;
>  		interrupt-parent = <&gpio6>;
>  		interrupts = <10 IRQ_TYPE_EDGE_FALLING>;
> -		reset-gpios = <&gpio6 9 GPIO_ACTIVE_HIGH>; /* SODIMM 13
> */
> +		reset-gpios = <&gpio6 9 GPIO_ACTIVE_LOW>; /* SODIMM 13
> */
>  		status = "disabled";
>  	};
>  
> diff --git a/arch/arm/boot/dts/imx6q-apalis-ixora.dts
> b/arch/arm/boot/dts/imx6q-apalis-ixora.dts
> index fa9f98dd15ac..62e72773e53b 100644
> --- a/arch/arm/boot/dts/imx6q-apalis-ixora.dts
> +++ b/arch/arm/boot/dts/imx6q-apalis-ixora.dts
> @@ -144,7 +144,7 @@ touchscreen@4a {
>  		reg = <0x4a>;
>  		interrupt-parent = <&gpio6>;
>  		interrupts = <10 IRQ_TYPE_EDGE_FALLING>;
> -		reset-gpios = <&gpio6 9 GPIO_ACTIVE_HIGH>; /* SODIMM 13
> */
> +		reset-gpios = <&gpio6 9 GPIO_ACTIVE_LOW>; /* SODIMM 13
> */
>  		status = "disabled";
>  	};
>  
> diff --git a/arch/arm/boot/dts/imx7-colibri-aster.dtsi
> b/arch/arm/boot/dts/imx7-colibri-aster.dtsi
> index 9fa701bec2ec..139188eb9f40 100644
> --- a/arch/arm/boot/dts/imx7-colibri-aster.dtsi
> +++ b/arch/arm/boot/dts/imx7-colibri-aster.dtsi
> @@ -99,7 +99,7 @@ touchscreen@4a {
>  		reg = <0x4a>;
>  		interrupt-parent = <&gpio2>;
>  		interrupts = <15 IRQ_TYPE_EDGE_FALLING>;	/* SODIMM 107
> */
> -		reset-gpios = <&gpio2 28 GPIO_ACTIVE_HIGH>;	/*
> SODIMM 106 */
> +		reset-gpios = <&gpio2 28 GPIO_ACTIVE_LOW>;	/*
> SODIMM 106 */
>  	};
>  
>  	/* M41T0M6 real time clock on carrier board */
> diff --git a/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
> b/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
> index 97601375f264..3caf450735d7 100644
> --- a/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
> +++ b/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
> @@ -124,7 +124,7 @@ touchscreen@4a {
>  		reg = <0x4a>;
>  		interrupt-parent = <&gpio1>;
>  		interrupts = <9 IRQ_TYPE_EDGE_FALLING>;		/*
> SODIMM 28 */
> -		reset-gpios = <&gpio1 10 GPIO_ACTIVE_HIGH>;	/*
> SODIMM 30 */
> +		reset-gpios = <&gpio1 10 GPIO_ACTIVE_LOW>;	/*
> SODIMM 30 */
>  		status = "disabled";
>  	};
>  
> diff --git a/arch/arm/boot/dts/motorola-mapphone-common.dtsi
> b/arch/arm/boot/dts/motorola-mapphone-common.dtsi
> index d5ded4f794df..5f8f77cfbe59 100644
> --- a/arch/arm/boot/dts/motorola-mapphone-common.dtsi
> +++ b/arch/arm/boot/dts/motorola-mapphone-common.dtsi
> @@ -430,7 +430,7 @@ touchscreen@4a {
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&touchscreen_pins>;
>  
> -		reset-gpios = <&gpio6 13 GPIO_ACTIVE_HIGH>; /* gpio173
> */
> +		reset-gpios = <&gpio6 13 GPIO_ACTIVE_LOW>; /* gpio173 */
>  
>  		/* gpio_183 with sys_nirq2 pad as wakeup */
>  		interrupts-extended = <&gpio6 23 IRQ_TYPE_LEVEL_LOW>,
> diff --git a/arch/arm/boot/dts/s5pv210-aries.dtsi
> b/arch/arm/boot/dts/s5pv210-aries.dtsi
> index bd4450dbdcb6..4da33d0f2748 100644
> --- a/arch/arm/boot/dts/s5pv210-aries.dtsi
> +++ b/arch/arm/boot/dts/s5pv210-aries.dtsi
> @@ -632,7 +632,7 @@ touchscreen@4a {
>  		interrupts = <5 IRQ_TYPE_EDGE_FALLING>;
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&ts_irq>;
> -		reset-gpios = <&gpj1 3 GPIO_ACTIVE_HIGH>;
> +		reset-gpios = <&gpj1 3 GPIO_ACTIVE_LOW>;
>  	};
>  };
>  
> diff --git a/arch/arm/boot/dts/tegra20-acer-a500-picasso.dts
> b/arch/arm/boot/dts/tegra20-acer-a500-picasso.dts
> index a0b829738e8f..10794a870776 100644
> --- a/arch/arm/boot/dts/tegra20-acer-a500-picasso.dts
> +++ b/arch/arm/boot/dts/tegra20-acer-a500-picasso.dts
> @@ -446,7 +446,7 @@ touchscreen@4c {
>  			interrupt-parent = <&gpio>;
>  			interrupts = <TEGRA_GPIO(V, 6)
> IRQ_TYPE_LEVEL_LOW>;
>  
> -			reset-gpios = <&gpio TEGRA_GPIO(Q, 7)
> GPIO_ACTIVE_HIGH>;
> +			reset-gpios = <&gpio TEGRA_GPIO(Q, 7)
> GPIO_ACTIVE_LOW>;
>  
>  			avdd-supply = <&vdd_3v3_sys>;
>  			vdd-supply  = <&vdd_3v3_sys>;
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c
> b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 98f17fa3a892..ef7915400c9f 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -3134,8 +3134,9 @@ static int mxt_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
>  	if (error)
>  		return error;
>  
> +	/* Request the RESET line as asserted so we go into reset */
>  	data->reset_gpio = devm_gpiod_get_optional(&client->dev,
> -						   "reset",
> GPIOD_OUT_LOW);
> +						   "reset",
> GPIOD_OUT_HIGH);
>  	if (IS_ERR(data->reset_gpio)) {
>  		error = PTR_ERR(data->reset_gpio);
>  		dev_err(&client->dev, "Failed to get reset gpio: %d\n",
> error);
> @@ -3153,8 +3154,9 @@ static int mxt_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
>  	disable_irq(client->irq);
>  
>  	if (data->reset_gpio) {
> +		/* Wait a while and then de-assert the RESET GPIO line
> */
>  		msleep(MXT_RESET_GPIO_TIME);
> -		gpiod_set_value(data->reset_gpio, 1);
> +		gpiod_set_value(data->reset_gpio, 0);
>  		msleep(MXT_RESET_INVALID_CHG);
>  	}
>  

Re: [PATCH 1/3 v2] Input: atmel_mxt_ts - Fix up inverted RESET handler

[Krzysztof Kozlowski]

On Wed, Nov 04, 2020 at 04:30:30PM +0100, Linus Walleij wrote:
> This driver uses GPIO descriptors to drive the touchscreen
> RESET line. In the existing device trees this has in
> conflict with intution been flagged as GPIO_ACTIVE_HIGH
> and the driver then applies the reverse action by
> driving the line low (setting to 0) to enter
> reset state and driving the line high (setting to 1) to
> get out of reset state.
> 
> The correct way to handle active low GPIO lines is to
> provide the GPIO_ACTIVE_LOW in the device tree (thus
> properly describing the hardware) and letting the GPIO
> framework invert the assertion (driving high) to a low
> level and vice versa.
> 
> This is considered a bug since the device trees are
> incorrectly mis-specifying the line as active high.
> 
> Fix the driver and all device trees specifying a reset
> line.
> 
> Cc: Krzysztof Kozlowski <krzk@kernel.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-tegra@vger.kernel.org
> Cc: linux-samsung-soc@vger.kernel.org
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - New patch fixing this confusion before adding the
>   new YAML bindings.
> - CC some misc maintainers and mailing lists that should
>   be aware that we do this change.

Acked-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

Re: [PATCH 2/3 v2] Input: atmel_mxt_ts - Convert bindings to YAML and extend

[Rob Herring]

On Wed, 04 Nov 2020 16:30:31 +0100, Linus Walleij wrote:
> This converts the Armel MXT touchscreen bindings to YAML
> format and extends them with the following two properties:
> 
> - vdda-supply: the optional analog supply voltage
> - vdd-supply: the optional digital supply voltage
> 
> I also explained about the reset-gpios property that this
> better be flagged as active high (0) despite actually
> being active low, because all current device trees and
> drivers assume that this is the case and will actively
> drive the line low to assert RESET.
> 
> Tested the schema with all in-tree users and they verify
> fine.
> 
> Cc: Nick Dyer <nick@shmanahar.org>
> Cc: Stephan Gerhold <stephan@gerhold.net>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - State that the GPIO lines shall be flagged as active low.
>   We will fix all users.
> ---
>  .../bindings/input/atmel,maxtouch.txt         | 41 ----------
>  .../bindings/input/atmel,maxtouch.yaml        | 81 +++++++++++++++++++
>  MAINTAINERS                                   |  2 +-
>  3 files changed, 82 insertions(+), 42 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/input/atmel,maxtouch.txt
>  create mode 100644 Documentation/devicetree/bindings/input/atmel,maxtouch.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH 1/3 v2] Input: atmel_mxt_ts - Fix up inverted RESET handler

[Dmitry Torokhov]

On Wed, Nov 04, 2020 at 04:30:30PM +0100, Linus Walleij wrote:
> This driver uses GPIO descriptors to drive the touchscreen
> RESET line. In the existing device trees this has in
> conflict with intution been flagged as GPIO_ACTIVE_HIGH
> and the driver then applies the reverse action by
> driving the line low (setting to 0) to enter
> reset state and driving the line high (setting to 1) to
> get out of reset state.
> 
> The correct way to handle active low GPIO lines is to
> provide the GPIO_ACTIVE_LOW in the device tree (thus
> properly describing the hardware) and letting the GPIO
> framework invert the assertion (driving high) to a low
> level and vice versa.
> 
> This is considered a bug since the device trees are
> incorrectly mis-specifying the line as active high.
> 
> Fix the driver and all device trees specifying a reset
> line.
> 
> Cc: Krzysztof Kozlowski <krzk@kernel.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-tegra@vger.kernel.org
> Cc: linux-samsung-soc@vger.kernel.org
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Applied, thank you.

-- 
Dmitry

Re: [PATCH 2/3 v2] Input: atmel_mxt_ts - Convert bindings to YAML and extend

[Dmitry Torokhov]

On Wed, Nov 04, 2020 at 04:30:31PM +0100, Linus Walleij wrote:
> This converts the Armel MXT touchscreen bindings to YAML
> format and extends them with the following two properties:
> 
> - vdda-supply: the optional analog supply voltage
> - vdd-supply: the optional digital supply voltage
> 
> I also explained about the reset-gpios property that this
> better be flagged as active high (0) despite actually
> being active low, because all current device trees and
> drivers assume that this is the case and will actively
> drive the line low to assert RESET.
> 
> Tested the schema with all in-tree users and they verify
> fine.
> 
> Cc: Nick Dyer <nick@shmanahar.org>
> Cc: Stephan Gerhold <stephan@gerhold.net>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Applied, thank you.

-- 
Dmitry

Re: [PATCH 3/3 v2] Input: atmel_mxt_ts - Support regulator supplies

[Dmitry Torokhov]

On Wed, Nov 04, 2020 at 04:30:32PM +0100, Linus Walleij wrote:
> This adds the code for the Atmel touchscreens such as
> mXT224 to obtain power regulators for the supply voltages
> AVDD and VDD. On mobile phones such as Samsung GT-I8190
> (Golden) this is needed to explicitly bring power online.
> 
> We just enable the regulators at probe() and disable
> them at remove() or in the errorpath for now.
> 
> As regulators are naturally stubbed if not available,
> this should have no impact on existing systems.
> 
> Cc: Nick Dyer <nick@shmanahar.org>
> Cc: Stephan Gerhold <stephan@gerhold.net>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Applied, thank you.

-- 
Dmitry