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

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

[Jagan Teki]

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

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/10816901/

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 v6 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>
---
 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 f7e95c52f3c7..b9b8f196dc90 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
@@ -23,6 +23,8 @@ 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)
+ - AVDD28-supply	: Analog power supply regulator on AVDD28 pin
+ - VDDIO-supply		: GPIO power supply regulator on VDDIO pin
 
 Example:
 
-- 
2.18.0.321.gffc6fa0e3

[PATCH v6 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 f2d9c2c41885..5f9e755c5bc7 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;
@@ -531,6 +534,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)) {
@@ -761,6 +782,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)
 {
@@ -786,6 +818,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 v6 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 b9b8f196dc90..1dcae643427f 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"
-- 
2.18.0.321.gffc6fa0e3

[PATCH v6 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 5f9e755c5bc7..405246d61701 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:
 		return &gt1x_chip_data;
 
 	case 911:
@@ -1000,6 +1001,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

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

[Rob Herring]

On Tue, 19 Feb 2019 15:46:26 +0530, Jagan Teki wrote:
> 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>
> ---
>  Documentation/devicetree/bindings/input/touchscreen/goodix.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 

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

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

[Jagan Teki]

Hi Dmitry,

On Tue, Feb 19, 2019 at 3:46 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> This is v6 patchset for supporting goodix GT5553 CTP. Here is the
> previous version[1]
>
> 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/10816901/
>
> 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(+)

Let me know if you have any further comments on regulator patch, fyi
Rob reviewed it already.

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

[Jagan Teki]

Hi Dmitry,

On Tue, Feb 26, 2019 at 11:43 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> Hi Dmitry,
>
> On Tue, Feb 19, 2019 at 3:46 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > This is v6 patchset for supporting goodix GT5553 CTP. Here is the
> > previous version[1]
> >
> > 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/10816901/
> >
> > 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(+)
>
> Let me know if you have any further comments on regulator patch, fyi
> Rob reviewed it already.

Would you comment on this? few dts changes are pending for this to support.

Jagan.

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

[Jagan Teki]

On Wed, Mar 6, 2019 at 10:47 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> Hi Dmitry,
>
> On Tue, Feb 26, 2019 at 11:43 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > Hi Dmitry,
> >
> > On Tue, Feb 19, 2019 at 3:46 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> > >
> > > This is v6 patchset for supporting goodix GT5553 CTP. Here is the
> > > previous version[1]
> > >
> > > 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/10816901/
> > >
> > > 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(+)
> >
> > Let me know if you have any further comments on regulator patch, fyi
> > Rob reviewed it already.
>
> Would you comment on this? few dts changes are pending for this to support.

Gentle remainder.

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

[Jagan Teki]

Hi Dmitry,

On Mon, Mar 11, 2019 at 7:08 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> On Wed, Mar 6, 2019 at 10:47 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > Hi Dmitry,
> >
> > On Tue, Feb 26, 2019 at 11:43 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
> > >
> > > Hi Dmitry,
> > >
> > > On Tue, Feb 19, 2019 at 3:46 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> > > >
> > > > This is v6 patchset for supporting goodix GT5553 CTP. Here is the
> > > > previous version[1]
> > > >
> > > > 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/10816901/
> > > >
> > > > 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(+)
> > >
> > > Let me know if you have any further comments on regulator patch, fyi
> > > Rob reviewed it already.
> >
> > Would you comment on this? few dts changes are pending for this to support.
>
> Gentle remainder.

It's been several weeks, any further comments?