[PATCH v7 0/4] input: touchscreen: Add goodix GT5553 CTP support

[PATCH v7 0/4] input: touchscreen: Add goodix GT5553 CTP support

[Jagan Teki]

This is v7 patchset for supporting goodix GT5553 CTP. Here is the
previous version[1]

Changes for v5:
- rebase on linux-next
Changes for v5:
- document bindings for required regulators, which are need during
  power-on sequence
- enable, disable required regulators as described in power-on sequence
  using normal regulator calls
- update the proper commi messages
Changes for v4:
- document AVDD22, DVDD12, VDDIO as optional properties
- use regulator bulk calls, for get, enable and disable functionalities
Changes for v4:
- devm_add_action_or_reset for disabling regulator
Changes for v3:
- add cover-letter
- s/ADVV28/AVDD28 on commit head
- fix few typo
Changes for v2:
- Rename vcc-supply with AVDD28-supply
- disable regulator in remove
- fix to setup regulator in probe code
- add chipdata
- drop example node in dt-bindings

[1] https://patchwork.kernel.org/cover/10819645/

Jagan Teki (4):
  dt-bindings: input: touchscreen: goodix: Document regulator properties
  Input: goodix - Add regulators suppot
  dt-bindings: input: touchscreen: goodix: Add GT5663 compatible
  Input: goodix - Add GT5663 CTP support

 .../bindings/input/touchscreen/goodix.txt     |  3 +
 drivers/input/touchscreen/goodix.c            | 60 +++++++++++++++++++
 2 files changed, 63 insertions(+)

-- 
2.18.0.321.gffc6fa0e3

[PATCH v7 1/4] dt-bindings: input: touchscreen: goodix: Document regulator properties

[Jagan Teki]

Goodix CTP controllers support analog, digital and gpio regulator
supplies on relevant controller pin configurations.

Out of which AVDD28 and VDDIO regulators are required in few goodix CTP
chips during power-on sequence.

AVDD22, DVDD12 regulators have no relevant functionality described from
datasheet [1].

So, document both AVDD28, VDDIO regulators into optional properties since
few of the goodix chip do work without these regulator power-on sequence.

[1] GT5663 Datasheet_English_20151106_Rev.01

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/input/touchscreen/goodix.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
index 8cf0b4d38a7e..255673250bbd 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
@@ -19,6 +19,8 @@ Optional properties:
  - irq-gpios		: GPIO pin used for IRQ. The driver uses the
 			  interrupt gpio pin as output to reset the device.
  - reset-gpios		: GPIO pin used for reset
+ - AVDD28-supply	: Analog power supply regulator on AVDD28 pin
+ - VDDIO-supply		: GPIO power supply regulator on VDDIO pin
  - touchscreen-inverted-x
  - touchscreen-inverted-y
  - touchscreen-size-x
-- 
2.18.0.321.gffc6fa0e3

[PATCH v7 2/4] Input: goodix - Add regulators suppot

[Jagan Teki]

Goodix CTP controllers require AVDD28, VDDIO regulators for power-on
sequence.

The delay between these regualtor operations as per Power-on Timing
from datasheet[1] is 0 (T1 >= 0 usec).

So, enable and disable these regulators in proper order using normal
regulator functions without any delay in between.

[1] GT5663 Datasheet_English_20151106_Rev.01

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

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index f57d82220a88..de5b80a08f41 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,8 @@ struct goodix_ts_data {
 	struct touchscreen_properties prop;
 	unsigned int max_touch_num;
 	unsigned int int_trigger_type;
+	struct regulator *avdd28;
+	struct regulator *vddio;
 	struct gpio_desc *gpiod_int;
 	struct gpio_desc *gpiod_rst;
 	u16 id;
@@ -532,6 +535,24 @@ static int goodix_get_gpio_config(struct goodix_ts_data *ts)
 		return -EINVAL;
 	dev = &ts->client->dev;
 
+	ts->avdd28 = devm_regulator_get(dev, "AVDD28");
+	if (IS_ERR(ts->avdd28)) {
+		error = PTR_ERR(ts->avdd28);
+		if (error != -EPROBE_DEFER)
+			dev_err(dev,
+				"Failed to get AVDD28 regulator: %d\n", error);
+		return error;
+	}
+
+	ts->vddio = devm_regulator_get(dev, "VDDIO");
+	if (IS_ERR(ts->vddio)) {
+		error = PTR_ERR(ts->vddio);
+		if (error != -EPROBE_DEFER)
+			dev_err(dev,
+				"Failed to get VDDIO regulator: %d\n", error);
+		return error;
+	}
+
 	/* Get the interrupt GPIO pin number */
 	gpiod = devm_gpiod_get_optional(dev, GOODIX_GPIO_INT_NAME, GPIOD_IN);
 	if (IS_ERR(gpiod)) {
@@ -764,6 +785,17 @@ static void goodix_config_cb(const struct firmware *cfg, void *ctx)
 	complete_all(&ts->firmware_loading_complete);
 }
 
+static void goodix_disable_regulator(void *arg)
+{
+	struct goodix_ts_data *ts = arg;
+
+	if (!IS_ERR(ts->vddio))
+		regulator_disable(ts->vddio);
+
+	if (!IS_ERR(ts->avdd28))
+		regulator_disable(ts->avdd28);
+}
+
 static int goodix_ts_probe(struct i2c_client *client,
 			   const struct i2c_device_id *id)
 {
@@ -789,6 +821,32 @@ static int goodix_ts_probe(struct i2c_client *client,
 	if (error)
 		return error;
 
+	error = devm_add_action_or_reset(&client->dev,
+					 goodix_disable_regulator, ts);
+	if (error)
+		return error;
+
+	/* power the controller */
+	if (!IS_ERR(ts->avdd28)) {
+		error = regulator_enable(ts->avdd28);
+		if (error) {
+			dev_err(&client->dev,
+				"Failed to enable AVDD28 regulator: %d\n",
+				error);
+			return error;
+		}
+	}
+
+	if (!IS_ERR(ts->vddio)) {
+		error = regulator_enable(ts->vddio);
+		if (error) {
+			dev_err(&client->dev,
+				"Failed to enable VDDIO regulator: %d\n",
+				error);
+			return error;
+		}
+	}
+
 	if (ts->gpiod_int && ts->gpiod_rst) {
 		/* reset the controller */
 		error = goodix_reset(ts);
-- 
2.18.0.321.gffc6fa0e3

[PATCH v7 3/4] dt-bindings: input: touchscreen: goodix: Add GT5663 compatible

[Jagan Teki]

GT5663 is capacitive touch controller with customized smart
wakeup gestures, it support chipdata which is similar to
existing GT1151 and require AVDD28 supply for some boards.

Document the compatible for the same.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 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 255673250bbd..fc03ea4cf5ab 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,gt5688"
 				 or "goodix,gt911"
 				 or "goodix,gt9110"
-- 
2.18.0.321.gffc6fa0e3

[PATCH v7 4/4] Input: goodix - Add GT5663 CTP support

[Jagan Teki]

GT5663 is capacitive touch controller with customized smart
wakeup gestures.

Add support for it by adding compatible and supported chip data.

The chip data on GT5663 is similar to GT1151, like
- config data register has 0x8050 address
- config data register max len is 240
- config data checksum has 16-bit

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

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index de5b80a08f41..c558b091749c 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -219,6 +219,7 @@ static const struct goodix_chip_data *goodix_get_chip_data(u16 id)
 {
 	switch (id) {
 	case 1151:
+	case 5663:
 	case 5688:
 		return &gt1x_chip_data;
 
@@ -1003,6 +1004,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,gt5688" },
 	{ .compatible = "goodix,gt911" },
 	{ .compatible = "goodix,gt9110" },
-- 
2.18.0.321.gffc6fa0e3

Re: [PATCH v7 0/4] input: touchscreen: Add goodix GT5553 CTP support

[Jagan Teki]

On Thu, Mar 21, 2019 at 1:51 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> This is v7 patchset for supporting goodix GT5553 CTP. Here is the
> previous version[1]
>
> Changes for v5:
> - rebase on linux-next
> Changes for v5:
> - document bindings for required regulators, which are need during
>   power-on sequence
> - enable, disable required regulators as described in power-on sequence
>   using normal regulator calls
> - update the proper commi messages
> Changes for v4:
> - document AVDD22, DVDD12, VDDIO as optional properties
> - use regulator bulk calls, for get, enable and disable functionalities
> Changes for v4:
> - devm_add_action_or_reset for disabling regulator
> Changes for v3:
> - add cover-letter
> - s/ADVV28/AVDD28 on commit head
> - fix few typo
> Changes for v2:
> - Rename vcc-supply with AVDD28-supply
> - disable regulator in remove
> - fix to setup regulator in probe code
> - add chipdata
> - drop example node in dt-bindings
>
> [1] https://patchwork.kernel.org/cover/10819645/
>
> Jagan Teki (4):
>   dt-bindings: input: touchscreen: goodix: Document regulator properties
>   Input: goodix - Add regulators suppot
>   dt-bindings: input: touchscreen: goodix: Add GT5663 compatible
>   Input: goodix - Add GT5663 CTP support

Ping?

Re: [PATCH v7 2/4] Input: goodix - Add regulators suppot

[Dmitry Torokhov]

Hi Jagan,

On Thu, Mar 21, 2019 at 01:51:02PM +0530, Jagan Teki wrote:
> Goodix CTP controllers require AVDD28, VDDIO regulators for power-on
> sequence.
> 
> The delay between these regualtor operations as per Power-on Timing
> from datasheet[1] is 0 (T1 >= 0 usec).
> 
> So, enable and disable these regulators in proper order using normal
> regulator functions without any delay in between.
> 
> [1] GT5663 Datasheet_English_20151106_Rev.01
> 
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  drivers/input/touchscreen/goodix.c | 58 ++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
> 
> diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
> index f57d82220a88..de5b80a08f41 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,8 @@ struct goodix_ts_data {
>  	struct touchscreen_properties prop;
>  	unsigned int max_touch_num;
>  	unsigned int int_trigger_type;
> +	struct regulator *avdd28;
> +	struct regulator *vddio;
>  	struct gpio_desc *gpiod_int;
>  	struct gpio_desc *gpiod_rst;
>  	u16 id;
> @@ -532,6 +535,24 @@ static int goodix_get_gpio_config(struct goodix_ts_data *ts)
>  		return -EINVAL;
>  	dev = &ts->client->dev;
>  
> +	ts->avdd28 = devm_regulator_get(dev, "AVDD28");
> +	if (IS_ERR(ts->avdd28)) {
> +		error = PTR_ERR(ts->avdd28);
> +		if (error != -EPROBE_DEFER)
> +			dev_err(dev,
> +				"Failed to get AVDD28 regulator: %d\n", error);
> +		return error;
> +	}
> +
> +	ts->vddio = devm_regulator_get(dev, "VDDIO");
> +	if (IS_ERR(ts->vddio)) {
> +		error = PTR_ERR(ts->vddio);
> +		if (error != -EPROBE_DEFER)
> +			dev_err(dev,
> +				"Failed to get VDDIO regulator: %d\n", error);
> +		return error;
> +	}
> +
>  	/* Get the interrupt GPIO pin number */
>  	gpiod = devm_gpiod_get_optional(dev, GOODIX_GPIO_INT_NAME, GPIOD_IN);
>  	if (IS_ERR(gpiod)) {
> @@ -764,6 +785,17 @@ static void goodix_config_cb(const struct firmware *cfg, void *ctx)
>  	complete_all(&ts->firmware_loading_complete);
>  }
>  
> +static void goodix_disable_regulator(void *arg)
> +{
> +	struct goodix_ts_data *ts = arg;
> +
> +	if (!IS_ERR(ts->vddio))
> +		regulator_disable(ts->vddio);

We error out of goodix_get_gpio_config() and abort probe() if
devm_regulator_get() fails, so there is no need to check for errors
here.

> +
> +	if (!IS_ERR(ts->avdd28))
> +		regulator_disable(ts->avdd28);
> +}
> +
>  static int goodix_ts_probe(struct i2c_client *client,
>  			   const struct i2c_device_id *id)
>  {
> @@ -789,6 +821,32 @@ static int goodix_ts_probe(struct i2c_client *client,
>  	if (error)
>  		return error;
>  
> +	error = devm_add_action_or_reset(&client->dev,
> +					 goodix_disable_regulator, ts);
> +	if (error)
> +		return error;

We need to do this after enabling regulators, otherwise there is a risk
of unbalanced disable.

I adjusted and applied, thank you.

-- 
Dmitry

Re: [PATCH v7 4/4] Input: goodix - Add GT5663 CTP support

[Dmitry Torokhov]

On Thu, Mar 21, 2019 at 01:51:04PM +0530, Jagan Teki wrote:
> GT5663 is capacitive touch controller with customized smart
> wakeup gestures.
> 
> Add support for it by adding compatible and supported chip data.
> 
> The chip data on GT5663 is similar to GT1151, like
> - config data register has 0x8050 address
> - config data register max len is 240
> - config data checksum has 16-bit
> 
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>

Applied, thank you.

> ---
>  drivers/input/touchscreen/goodix.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
> index de5b80a08f41..c558b091749c 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -219,6 +219,7 @@ static const struct goodix_chip_data *goodix_get_chip_data(u16 id)
>  {
>  	switch (id) {
>  	case 1151:
> +	case 5663:
>  	case 5688:
>  		return &gt1x_chip_data;
>  
> @@ -1003,6 +1004,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,gt5688" },
>  	{ .compatible = "goodix,gt911" },
>  	{ .compatible = "goodix,gt9110" },
> -- 
> 2.18.0.321.gffc6fa0e3
> 

-- 
Dmitry