[PATCH 1/9] dt-bindings: gpio-axp209: Add AXP803 GPIOs compatible (w/ AXP813 fallback)

[PATCH 1/9] dt-bindings: gpio-axp209: Add AXP803 GPIOs compatible (w/ AXP813 fallback)

[Jagan Teki]

AXP803 PMIC has two GPIO's which is similar to the one in
AXP813 PMIC.

Add a compatible string for it with AXP813 fallback compatible string, in
this case the AXP813 driver can be used.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 Documentation/devicetree/bindings/gpio/gpio-axp209.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/gpio/gpio-axp209.txt b/Documentation/devicetree/bindings/gpio/gpio-axp209.txt
index fc42b2caa06d..c8c5d20856c9 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-axp209.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-axp209.txt
@@ -12,6 +12,7 @@ Required properties:
 - compatible: Should be one of:
 	- "x-powers,axp209-gpio"
 	- "x-powers,axp813-gpio"
+	- "x-powers,axp803-gpio", "x-powers,axp813-gpio"
 - #gpio-cells: Should be two. The first cell is the pin number and the
   second is the GPIO flags.
 - gpio-controller: Marks the device node as a GPIO controller.
-- 
2.18.0.321.gffc6fa0e3

[PATCH 2/9] mfd: axp20x: Add GPIO/pinctrl support for AXP803

[Jagan Teki]

Add GPIO/pinctrl support for AXP803.

This will reuse the similar compatible to that of AXP813,
since both PMIC GPIO's are same.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/mfd/axp20x.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 0be511dd93d0..bc705ba49c3f 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -731,8 +731,12 @@ static const struct mfd_cell axp803_cells[] = {
 		.name			= "axp221-pek",
 		.num_resources		= ARRAY_SIZE(axp803_pek_resources),
 		.resources		= axp803_pek_resources,
+	}, {
+		.name			= "axp20x-regulator",
+	}, {
+		.name			= "axp20x-gpio",
+		.of_compatible		= "x-powers,axp813-gpio",
 	},
-	{	.name			= "axp20x-regulator" },
 };
 
 static const struct mfd_cell axp806_self_working_cells[] = {
-- 
2.18.0.321.gffc6fa0e3

[PATCH 3/9] arm64: allwinner: dts: axp803: Add GPIO DT node

[Jagan Teki]

Add GPIO/pinctrl node for AXP803 PMIC.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 arch/arm64/boot/dts/allwinner/axp803.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/axp803.dtsi b/arch/arm64/boot/dts/allwinner/axp803.dtsi
index e5eae8bafc42..c6b95980d476 100644
--- a/arch/arm64/boot/dts/allwinner/axp803.dtsi
+++ b/arch/arm64/boot/dts/allwinner/axp803.dtsi
@@ -49,6 +49,13 @@
 	interrupt-controller;
 	#interrupt-cells = <1>;
 
+	axp_gpio: axp-gpio {
+		compatible = "x-powers,axp803-gpio",
+			     "x-powers,axp813-gpio";
+		gpio-controller;
+		#gpio-cells = <2>;
+	};
+
 	regulators {
 		/* Default work frequency for buck regulators */
 		x-powers,dcdc-freq = <3000>;
-- 
2.18.0.321.gffc6fa0e3

[PATCH 4/9] arm64: allwinner: dts: axp803: Set GPIO0/1 pinmux for LD0s

[Jagan Teki]

GPIO0, GPIO1 can be used as LDO like ldo_io0, ldo_io1 in
AXP803.

So, attach the LDO GPIO pins along with pinctrl properties.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 arch/arm64/boot/dts/allwinner/axp803.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/axp803.dtsi b/arch/arm64/boot/dts/allwinner/axp803.dtsi
index c6b95980d476..b0d6b3c3cf38 100644
--- a/arch/arm64/boot/dts/allwinner/axp803.dtsi
+++ b/arch/arm64/boot/dts/allwinner/axp803.dtsi
@@ -54,6 +54,16 @@
 			     "x-powers,axp813-gpio";
 		gpio-controller;
 		#gpio-cells = <2>;
+
+		gpio0_ldo: gpio0-ldo {
+			pins = "GPIO0";
+			function = "ldo";
+		};
+
+		gpio1_ldo: gpio1-ldo {
+			pins = "GPIO1";
+			function = "ldo";
+		};
 	};
 
 	regulators {
@@ -137,12 +147,16 @@
 		};
 
 		reg_ldo_io0: ldo-io0 {
+			pinctrl-names = "default";
+			pinctrl-0 = <&gpio0_ldo>;
 			regulator-name = "ldo-io0";
 			status = "disabled";
 		};
 
 		reg_ldo_io1: ldo-io1 {
 			regulator-name = "ldo-io1";
+			pinctrl-names = "default";
+			pinctrl-0 = <&gpio1_ldo>;
 			status = "disabled";
 		};
 
-- 
2.18.0.321.gffc6fa0e3

[PATCH 5/9] dt-bindings: input: touchscreen: goodix: Document vcc-supply property

[Jagan Teki]

vcc-supply property is need for some Goodix CTP controller like GT5663
where 3.3V external pull-up regulator connected via controller VCC pin.

So, document the same as optional property.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 Documentation/devicetree/bindings/input/touchscreen/goodix.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
index f7e95c52f3c7..604766e347ce 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
@@ -23,6 +23,7 @@ Optional properties:
  - touchscreen-inverted-y  : Y axis is inverted (boolean)
  - touchscreen-swapped-x-y : X and Y axis are swapped (boolean)
                              (swapping is done after inverting the axis)
+ - vcc-supply		: 3v3 regulator phandle for controller VCC pin
 
 Example:
 
-- 
2.18.0.321.gffc6fa0e3

[PATCH 6/9] Input: goodix - Add vcc-supply regulator support

[Jagan Teki]

vcc-supply property is need for some Goodix CTP controller like GT5663
where 3.3V external pull-up regulator connected via controller VCC pin.

So, enable the regulator for those it attached the vcc-supply.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/input/touchscreen/goodix.c | 39 ++++++++++++++++++++++++++----
 1 file changed, 34 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index f2d9c2c41885..7adcf1491609 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -27,6 +27,7 @@
 #include <linux/delay.h>
 #include <linux/irq.h>
 #include <linux/interrupt.h>
+#include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 #include <linux/acpi.h>
 #include <linux/of.h>
@@ -47,6 +48,7 @@ struct goodix_ts_data {
 	struct touchscreen_properties prop;
 	unsigned int max_touch_num;
 	unsigned int int_trigger_type;
+	struct regulator *vcc;
 	struct gpio_desc *gpiod_int;
 	struct gpio_desc *gpiod_rst;
 	u16 id;
@@ -58,6 +60,7 @@ struct goodix_ts_data {
 
 #define GOODIX_GPIO_INT_NAME		"irq"
 #define GOODIX_GPIO_RST_NAME		"reset"
+#define GOODIX_VCC_CTP_NAME		"vcc"
 
 #define GOODIX_MAX_HEIGHT		4096
 #define GOODIX_MAX_WIDTH		4096
@@ -525,12 +528,24 @@ static int goodix_get_gpio_config(struct goodix_ts_data *ts)
 {
 	int error;
 	struct device *dev;
+	struct regulator *regulator;
 	struct gpio_desc *gpiod;
 
 	if (!ts->client)
 		return -EINVAL;
 	dev = &ts->client->dev;
 
+	regulator = devm_regulator_get(dev, GOODIX_VCC_CTP_NAME);
+	if (IS_ERR(regulator)) {
+		error = PTR_ERR(regulator);
+		if (error != -EPROBE_DEFER)
+			dev_err(dev, "Failed to get vcc regulator: %d\n",
+				error);
+		return error;
+	}
+
+	ts->vcc = regulator;
+
 	/* Get the interrupt GPIO pin number */
 	gpiod = devm_gpiod_get_optional(dev, GOODIX_GPIO_INT_NAME, GPIOD_IN);
 	if (IS_ERR(gpiod)) {
@@ -786,25 +801,34 @@ static int goodix_ts_probe(struct i2c_client *client,
 	if (error)
 		return error;
 
+	/* power the controller */
+	if (ts->vcc) {
+		error = regulator_enable(ts->vcc);
+		if (error) {
+			dev_err(&client->dev, "Controller fail to enable vcc\n");
+			return error;
+		}
+	}
+
 	if (ts->gpiod_int && ts->gpiod_rst) {
 		/* reset the controller */
 		error = goodix_reset(ts);
 		if (error) {
 			dev_err(&client->dev, "Controller reset failed.\n");
-			return error;
+			goto error;
 		}
 	}
 
 	error = goodix_i2c_test(client);
 	if (error) {
 		dev_err(&client->dev, "I2C communication failure: %d\n", error);
-		return error;
+		goto error;
 	}
 
 	error = goodix_read_version(ts);
 	if (error) {
 		dev_err(&client->dev, "Read version failed.\n");
-		return error;
+		goto error;
 	}
 
 	ts->chip = goodix_get_chip_data(ts->id);
@@ -823,17 +847,22 @@ static int goodix_ts_probe(struct i2c_client *client,
 			dev_err(&client->dev,
 				"Failed to invoke firmware loader: %d\n",
 				error);
-			return error;
+			goto error;
 		}
 
 		return 0;
 	} else {
 		error = goodix_configure_dev(ts);
 		if (error)
-			return error;
+			goto error;
 	}
 
 	return 0;
+
+error:
+	if (ts->vcc)
+		regulator_disable(ts->vcc);
+	return error;
 }
 
 static int goodix_ts_remove(struct i2c_client *client)
-- 
2.18.0.321.gffc6fa0e3

[PATCH 7/9] dt-bindings: input: touchscreen: goodix: Add GT5663 compatible

[Jagan Teki]

GT5663 is capacitive touch controller with customized smart wakeup gestures,
the existing goodix driver will work by phandle vcc-supply regulator.

So, document compatible and example node for the same.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 .../bindings/input/touchscreen/goodix.txt           | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
index 604766e347ce..1898d3dde8e1 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
@@ -3,6 +3,7 @@ Device tree bindings for Goodix GT9xx series touchscreen controller
 Required properties:
 
  - compatible		: Should be "goodix,gt1151"
+				 or "goodix,gt5663"
 				 or "goodix,gt911"
 				 or "goodix,gt9110"
 				 or "goodix,gt912"
@@ -42,3 +43,15 @@ Example:
 
 		/* ... */
 	};
+
+	touchscreen@5d {
+		compatible = "goodix,gt5663";
+		reg = <0x5d>;
+		vcc-supply = <&reg_ldo_io0>;
+		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 */
+		touchscreen-inverted-x;
+		touchscreen-inverted-y;
+	};
-- 
2.18.0.321.gffc6fa0e3

[PATCH 8/9] Input: goodix - Add GT5663 CTP support

[Jagan Teki]

GT5663 is capacitive touch controller with customized smart wakeup gestures,
the existing goodix driver will work by phandle vcc-supply regulator.

So, add compatible for the same.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/input/touchscreen/goodix.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index 7adcf1491609..0c09512bcdea 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -971,6 +971,7 @@ MODULE_DEVICE_TABLE(acpi, goodix_acpi_match);
 #ifdef CONFIG_OF
 static const struct of_device_id goodix_of_match[] = {
 	{ .compatible = "goodix,gt1151" },
+	{ .compatible = "goodix,gt5663" },
 	{ .compatible = "goodix,gt911" },
 	{ .compatible = "goodix,gt9110" },
 	{ .compatible = "goodix,gt912" },
-- 
2.18.0.321.gffc6fa0e3

[PATCH 9/9] arm64: dts: allwinner: a64-amarula-relic: Add GT5663 CTP node

[Jagan Teki]

Add Goodix GT5663 capacitive touch controller node on
Amarula A64-Relic board.

The CTP connected to board with,
- SDA, SCK from i2c1
- GPIO-LD0 as vcc supply
- PH4 gpio as interrupt pin
- PH8 gpio as reset pin
- X and Y axis are inverted

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 .../allwinner/sun50i-a64-amarula-relic.dts    | 29 +++++++++++++++++++
 1 file changed, 29 insertions(+)

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 9ac6d773188b..913fcdff828e 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
@@ -88,6 +88,28 @@
 	status = "okay";
 };
 
+&i2c1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c1_pins>;
+	status = "okay";
+
+	touchscreen@5d {
+		compatible = "goodix,gt5663";
+		reg = <0x5d>;
+		vcc-supply = <&reg_ldo_io0>;			/* VCC-CTP: GPIO0-LDO */
+		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 */
+		touchscreen-inverted-x;
+		touchscreen-inverted-y;
+	};
+};
+
+&i2c1_pins {
+	bias-pull-up;
+};
+
 &mmc1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&mmc1_pins>;
@@ -251,6 +273,13 @@
 	regulator-name = "vdd-cpus";
 };
 
+&reg_ldo_io0 {
+	regulator-min-microvolt = <3300000>;
+	regulator-max-microvolt = <3300000>;
+	regulator-name = "vcc-ctp";
+	status = "okay";
+};
+
 &reg_rtc_ldo {
 	regulator-name = "vcc-rtc";
 };
-- 
2.18.0.321.gffc6fa0e3

Re: [PATCH 1/9] dt-bindings: gpio-axp209: Add AXP803 GPIOs compatible (w/ AXP813 fallback)

[Chen-Yu Tsai]

On Mon, Dec 3, 2018 at 6:16 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> AXP803 PMIC has two GPIO's which is similar to the one in
> AXP813 PMIC.
>
> Add a compatible string for it with AXP813 fallback compatible string, in
> this case the AXP813 driver can be used.
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>

First of all, please write a cover letter.

Second, the GPIO stuff is covered already by Oskari Lemmela's
"AXP8x3 AC and battery power supply support" series. The device
tree patches from that series were merged last Friday. The driver
patches are pending. That already covers patches 1~3 and half of
patch 4 of your series.

ChenYu

Re: [PATCH 5/9] dt-bindings: input: touchscreen: goodix: Document vcc-supply property

[Chen-Yu Tsai]

On Mon, Dec 3, 2018 at 6:16 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> vcc-supply property is need for some Goodix CTP controller like GT5663
> where 3.3V external pull-up regulator connected via controller VCC pin.

"External pull-up regulator" sounds fishy. Chips have power supply
pins, either combined, or separate rails for analog, digital, and I/O.
For Goodix chips these are AVDD28, AVDD22, DVDD12, amd VDDIO. The name
and description you provide match none of these.

If a regulator is needed for pull-up resistors on the I2C bus, this
should be referenced in either the I2C bus master node, or if the I2C
bus goes through a pin controller, in the pin controller node.

Putting the reference in the slave device node is wrong, since it
doesn't actually use it. It becomes even clearer when you have multiple
slaves on the same bus, and you reference the same regulator in all
of them.

Or, as a last resort, you could mark it as always-on with a TODO note.

ChenYu


> So, document the same as optional property.
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  Documentation/devicetree/bindings/input/touchscreen/goodix.txt | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> index f7e95c52f3c7..604766e347ce 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> @@ -23,6 +23,7 @@ Optional properties:
>   - touchscreen-inverted-y  : Y axis is inverted (boolean)
>   - touchscreen-swapped-x-y : X and Y axis are swapped (boolean)
>                               (swapping is done after inverting the axis)
> + - vcc-supply          : 3v3 regulator phandle for controller VCC pin
>
>  Example:
>
> --
> 2.18.0.321.gffc6fa0e3
>

Re: [PATCH 6/9] Input: goodix - Add vcc-supply regulator support

[Dmitry Torokhov]

On Mon, Dec 03, 2018 at 03:45:44PM +0530, Jagan Teki wrote:
> vcc-supply property is need for some Goodix CTP controller like GT5663
> where 3.3V external pull-up regulator connected via controller VCC pin.
> 
> So, enable the regulator for those it attached the vcc-supply.
> 
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  drivers/input/touchscreen/goodix.c | 39 ++++++++++++++++++++++++++----
>  1 file changed, 34 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
> index f2d9c2c41885..7adcf1491609 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -27,6 +27,7 @@
>  #include <linux/delay.h>
>  #include <linux/irq.h>
>  #include <linux/interrupt.h>
> +#include <linux/regulator/consumer.h>
>  #include <linux/slab.h>
>  #include <linux/acpi.h>
>  #include <linux/of.h>
> @@ -47,6 +48,7 @@ struct goodix_ts_data {
>  	struct touchscreen_properties prop;
>  	unsigned int max_touch_num;
>  	unsigned int int_trigger_type;
> +	struct regulator *vcc;
>  	struct gpio_desc *gpiod_int;
>  	struct gpio_desc *gpiod_rst;
>  	u16 id;
> @@ -58,6 +60,7 @@ struct goodix_ts_data {
>  
>  #define GOODIX_GPIO_INT_NAME		"irq"
>  #define GOODIX_GPIO_RST_NAME		"reset"
> +#define GOODIX_VCC_CTP_NAME		"vcc"
>  
>  #define GOODIX_MAX_HEIGHT		4096
>  #define GOODIX_MAX_WIDTH		4096
> @@ -525,12 +528,24 @@ static int goodix_get_gpio_config(struct goodix_ts_data *ts)
>  {
>  	int error;
>  	struct device *dev;
> +	struct regulator *regulator;
>  	struct gpio_desc *gpiod;
>  
>  	if (!ts->client)
>  		return -EINVAL;
>  	dev = &ts->client->dev;
>  
> +	regulator = devm_regulator_get(dev, GOODIX_VCC_CTP_NAME);
> +	if (IS_ERR(regulator)) {
> +		error = PTR_ERR(regulator);
> +		if (error != -EPROBE_DEFER)
> +			dev_err(dev, "Failed to get vcc regulator: %d\n",
> +				error);
> +		return error;
> +	}
> +
> +	ts->vcc = regulator;
> +
>  	/* Get the interrupt GPIO pin number */
>  	gpiod = devm_gpiod_get_optional(dev, GOODIX_GPIO_INT_NAME, GPIOD_IN);
>  	if (IS_ERR(gpiod)) {
> @@ -786,25 +801,34 @@ static int goodix_ts_probe(struct i2c_client *client,
>  	if (error)
>  		return error;
>  
> +	/* power the controller */
> +	if (ts->vcc) {

ts->vcc is never NULL.

> +		error = regulator_enable(ts->vcc);
> +		if (error) {
> +			dev_err(&client->dev, "Controller fail to enable vcc\n");
> +			return error;
> +		}
> +	}
> +
>  	if (ts->gpiod_int && ts->gpiod_rst) {
>  		/* reset the controller */
>  		error = goodix_reset(ts);
>  		if (error) {
>  			dev_err(&client->dev, "Controller reset failed.\n");
> -			return error;
> +			goto error;
>  		}
>  	}
>  
>  	error = goodix_i2c_test(client);
>  	if (error) {
>  		dev_err(&client->dev, "I2C communication failure: %d\n", error);
> -		return error;
> +		goto error;
>  	}
>  
>  	error = goodix_read_version(ts);
>  	if (error) {
>  		dev_err(&client->dev, "Read version failed.\n");
> -		return error;
> +		goto error;
>  	}
>  
>  	ts->chip = goodix_get_chip_data(ts->id);
> @@ -823,17 +847,22 @@ static int goodix_ts_probe(struct i2c_client *client,
>  			dev_err(&client->dev,
>  				"Failed to invoke firmware loader: %d\n",
>  				error);
> -			return error;
> +			goto error;
>  		}
>  
>  		return 0;
>  	} else {
>  		error = goodix_configure_dev(ts);
>  		if (error)
> -			return error;
> +			goto error;
>  	}
>  
>  	return 0;
> +
> +error:
> +	if (ts->vcc)
> +		regulator_disable(ts->vcc);

Same here.

> +	return error;
>  }
>  
>  static int goodix_ts_remove(struct i2c_client *client)

You need to disable regulator on removal as well.

Thanks.

-- 
Dmitry

Re: [PATCH 5/9] dt-bindings: input: touchscreen: goodix: Document vcc-supply property

[Jagan Teki]

On Mon, Dec 3, 2018 at 4:11 PM Chen-Yu Tsai <wens@csie.org> wrote:
>
> On Mon, Dec 3, 2018 at 6:16 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > vcc-supply property is need for some Goodix CTP controller like GT5663
> > where 3.3V external pull-up regulator connected via controller VCC pin.
>
> "External pull-up regulator" sounds fishy. Chips have power supply
> pins, either combined, or separate rails for analog, digital, and I/O.
> For Goodix chips these are AVDD28, AVDD22, DVDD12, amd VDDIO. The name
> and description you provide match none of these.

We have vcc pin in the connector to supply vcc-ctp, but indeed it is
AVDD28 supply as per the reference schematic design[1] Page 23.  Host
interface has AVDD pin which is connected to ADVDD28 on goodix chip,
so I can name this as avdd28-supply. is that OK?

[1] https://www.crystalfontz.com/controllers/GOODIX/GT5663/459/

Re: [PATCH 5/9] dt-bindings: input: touchscreen: goodix: Document vcc-supply property

[Chen-Yu Tsai]

On Thu, Dec 6, 2018 at 3:56 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> On Mon, Dec 3, 2018 at 4:11 PM Chen-Yu Tsai <wens@csie.org> wrote:
> >
> > On Mon, Dec 3, 2018 at 6:16 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> > >
> > > vcc-supply property is need for some Goodix CTP controller like GT5663
> > > where 3.3V external pull-up regulator connected via controller VCC pin.
> >
> > "External pull-up regulator" sounds fishy. Chips have power supply
> > pins, either combined, or separate rails for analog, digital, and I/O.
> > For Goodix chips these are AVDD28, AVDD22, DVDD12, amd VDDIO. The name
> > and description you provide match none of these.
>
> We have vcc pin in the connector to supply vcc-ctp, but indeed it is
> AVDD28 supply as per the reference schematic design[1] Page 23.  Host
> interface has AVDD pin which is connected to ADVDD28 on goodix chip,
> so I can name this as avdd28-supply. is that OK?

Yeah. That's better. It's always best to be precise with hardware
descriptions. If you simplify it, the reason should be given in
detail. And it should be a good one.

ChenYu

Re: [PATCH 7/9] dt-bindings: input: touchscreen: goodix: Add GT5663 compatible

[Rob Herring]

On Mon, Dec 03, 2018 at 03:45:45PM +0530, Jagan Teki wrote:
> GT5663 is capacitive touch controller with customized smart wakeup gestures,
> the existing goodix driver will work by phandle vcc-supply regulator.
> 
> So, document compatible and example node for the same.
> 
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  .../bindings/input/touchscreen/goodix.txt           | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> index 604766e347ce..1898d3dde8e1 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> @@ -3,6 +3,7 @@ Device tree bindings for Goodix GT9xx series touchscreen controller
>  Required properties:
>  
>   - compatible		: Should be "goodix,gt1151"
> +				 or "goodix,gt5663"
>  				 or "goodix,gt911"
>  				 or "goodix,gt9110"
>  				 or "goodix,gt912"
> @@ -42,3 +43,15 @@ Example:
>  
>  		/* ... */
>  	};
> +
> +	touchscreen@5d {
> +		compatible = "goodix,gt5663";

Just a new compatible doesn't warrant a new example.

> +		reg = <0x5d>;
> +		vcc-supply = <&reg_ldo_io0>;
> +		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 */
> +		touchscreen-inverted-x;
> +		touchscreen-inverted-y;
> +	};
> -- 
> 2.18.0.321.gffc6fa0e3
>