Some fixes for Goodix Berlin touchscreen driver

PATCH Applied Luca Weiss 3 participants 8 messages Jan 3, 2025 – Feb 25, 2025

Subsystems (4)

Stable branch HID Input drivers Goodix touchscreen

lore

Share & tools

lore

Luca Weiss

author

2025-01-03 09:21 UTC #0 lore raw

cover Some fixes for Goodix Berlin touchscreen driver

One is a simple comment fix, and the second one fixes a discrepancy between dt-bindings and driver, aligning the driver to match dt-bindings.

Signed-off-by Luca Weiss <[email protected]>

---
Luca Weiss (2):
Input: goodix-berlin - fix comment referencing wrong regulator
Input: goodix-berlin - fix vddio regulator references

 drivers/input/touchscreen/goodix_berlin_core.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

---
base-commit: 8155b4ef3466f0e289e8fcc9e6e62f3f4dceeac2
change-id: 20250103-goodix-berlin-fixes-0f776d90caa7

Best regards,

Signature · 1 line

Luca Weiss <[email protected]>

Luca Weiss

author

2025-01-03 09:21 UTC #1 lore raw

PATCH 1/2 Input: goodix-berlin - fix comment referencing wrong regulator

In the statement above AVDD gets enabled, and not IOVDD, so fix this copy-paste mistake.

Fixes 44362279bdd4 ("Input: add core support for Goodix Berlin Touchscreen IC")

Reported-by Jens Reidel <[email protected]>

Signed-off-by Luca Weiss <[email protected]>

---
 drivers/input/touchscreen/goodix_berlin_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/input/touchscreen/goodix_berlin_core.c +1 -1

index 3fc03cf0ca23fdbe36028a6228030d3ffb8d3a09..e273fb8edc6b92bcbad0fd35223a841d7da7d671 100644
@@ -263,7 +263,7 @@ static int goodix_berlin_power_on(struct goodix_berlin_core *cd)
 		goto err_iovdd_disable;
 	}
 
-	/* Vendor waits 15ms for IOVDD to settle */
+	/* Vendor waits 15ms for AVDD to settle */
 	usleep_range(15000, 15100);
 
 	gpiod_set_value_cansleep(cd->reset_gpio, 0);

Signature · 1 line

2.47.1

Neil Armstrong

Reviewed-by on v1 1/2

2025-01-03 09:26 UTC #2 lore raw

Re: [PATCH 1/2] Input: goodix-berlin - fix comment referencing wrong regulator

Quoted 16 lines · Luca Weiss

In the statement above AVDD gets enabled, and not IOVDD, so fix this copy-paste mistake.
Fixes 44362279bdd4 ("Input: add core support for Goodix Berlin Touchscreen IC")
Reported-by Jens Reidel <[email protected]>
Signed-off-by Luca Weiss <[email protected]>
---
  drivers/input/touchscreen/goodix_berlin_core.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
drivers/input/touchscreen/goodix_berlin_core.c +0 -0
index 3fc03cf0ca23fdbe36028a6228030d3ffb8d3a09..e273fb8edc6b92bcbad0fd35223a841d7da7d671 100644

drivers/input/touchscreen/goodix_berlin_core.c static int goodix_berlin_power_on(struct goodix_berlin_core *cd) · line 269

Quoted 8 lines

drivers/input/touchscreen/goodix_berlin_core.c +1 -1

@@ -263,7 +263,7 @@ static int goodix_berlin_power_on(struct goodix_berlin_core *cd)
  		goto err_iovdd_disable;
  	}
 
-	/* Vendor waits 15ms for IOVDD to settle */
+	/* Vendor waits 15ms for AVDD to settle */
  	usleep_range(15000, 15100);
 
  	gpiod_set_value_cansleep(cd->reset_gpio, 0);

Reviewed-by Neil Armstrong <[email protected]>

Dmitry Torokhov

maintainer Applied

2025-02-25 06:29 UTC #3 lore raw

Re: [PATCH 1/2] Input: goodix-berlin - fix comment referencing wrong regulator

In reply to Luca Weiss

Quoted 6 lines · Luca Weiss

In the statement above AVDD gets enabled, and not IOVDD, so fix this copy-paste mistake.
Fixes 44362279bdd4 ("Input: add core support for Goodix Berlin Touchscreen IC")
Reported-by Jens Reidel <[email protected]>
Signed-off-by Luca Weiss <[email protected]>

Applied, thank you.

Signature · 1 line

Dmitry

Luca Weiss

author

2025-01-03 09:21 UTC #4 lore raw

PATCH 2/2 Input: goodix-berlin - fix vddio regulator references

In reply to Luca Weiss

As per dt-bindings the property is called vddio-supply, so use the correct name in the driver instead of iovdd. The datasheet also calls the supply 'VDDIO'.

Fixes 44362279bdd4 ("Input: add core support for Goodix Berlin Touchscreen IC")

Cc [email protected]

Signed-off-by Luca Weiss <[email protected]>

---
 drivers/input/touchscreen/goodix_berlin_core.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

drivers/input/touchscreen/goodix_berlin_core.c +12 ~~-12~~

index e273fb8edc6b92bcbad0fd35223a841d7da7d671..7f8cfdd106fae03a6b197582bca4eb61f80182c6 100644
@@ -165,7 +165,7 @@ struct goodix_berlin_core {
 	struct device *dev;
 	struct regmap *regmap;
 	struct regulator *avdd;
-	struct regulator *iovdd;
+	struct regulator *vddio;
 	struct gpio_desc *reset_gpio;
 	struct touchscreen_properties props;
 	struct goodix_berlin_fw_version fw_version;

@@ -248,19 +248,19 @@ static int goodix_berlin_power_on(struct goodix_berlin_core *cd)
 {
 	int error;
 
-	error = regulator_enable(cd->iovdd);
+	error = regulator_enable(cd->vddio);
 	if (error) {
-		dev_err(cd->dev, "Failed to enable iovdd: %d\n", error);
+		dev_err(cd->dev, "Failed to enable vddio: %d\n", error);
 		return error;
 	}
 
-	/* Vendor waits 3ms for IOVDD to settle */
+	/* Vendor waits 3ms for VDDIO to settle */
 	usleep_range(3000, 3100);
 
 	error = regulator_enable(cd->avdd);
 	if (error) {
 		dev_err(cd->dev, "Failed to enable avdd: %d\n", error);
-		goto err_iovdd_disable;
+		goto err_vddio_disable;
 	}
 
 	/* Vendor waits 15ms for AVDD to settle */

@@ -283,8 +283,8 @@ static int goodix_berlin_power_on(struct goodix_berlin_core *cd)
 err_dev_reset:
 	gpiod_set_value_cansleep(cd->reset_gpio, 1);
 	regulator_disable(cd->avdd);
-err_iovdd_disable:
-	regulator_disable(cd->iovdd);
+err_vddio_disable:
+	regulator_disable(cd->vddio);
 	return error;
 }

@@ -292,7 +292,7 @@ static void goodix_berlin_power_off(struct goodix_berlin_core *cd)
 {
 	gpiod_set_value_cansleep(cd->reset_gpio, 1);
 	regulator_disable(cd->avdd);
-	regulator_disable(cd->iovdd);
+	regulator_disable(cd->vddio);
 }
 
 static int goodix_berlin_read_version(struct goodix_berlin_core *cd)

@@ -744,10 +744,10 @@ int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
 		return dev_err_probe(dev, PTR_ERR(cd->avdd),
 				     "Failed to request avdd regulator\n");
 
-	cd->iovdd = devm_regulator_get(dev, "iovdd");
-	if (IS_ERR(cd->iovdd))
-		return dev_err_probe(dev, PTR_ERR(cd->iovdd),
-				     "Failed to request iovdd regulator\n");
+	cd->vddio = devm_regulator_get(dev, "vddio");
+	if (IS_ERR(cd->vddio))
+		return dev_err_probe(dev, PTR_ERR(cd->vddio),
+				     "Failed to request vddio regulator\n");
 
 	error = goodix_berlin_power_on(cd);
 	if (error) {

Signature · 1 line

2.47.1

Neil Armstrong

Reviewed-by on v1 2/2

2025-01-03 09:27 UTC #5 lore raw

Re: [PATCH 2/2] Input: goodix-berlin - fix vddio regulator references

Hi,

Quoted 3 lines · Luca Weiss

As per dt-bindings the property is called vddio-supply, so use the correct name in the driver instead of iovdd. The datasheet also calls the supply 'VDDIO'.

This is duplicate of https://lore.kernel.org/all/[email protected]/

But it's still valid:

Reviewed-by Neil Armstrong <[email protected]>

Quoted 81 lines

Fixes 44362279bdd4 ("Input: add core support for Goodix Berlin Touchscreen IC")

Cc [email protected]

Signed-off-by Luca Weiss <[email protected]>

---
  drivers/input/touchscreen/goodix_berlin_core.c | 24 ++++++++++++------------
  1 file changed, 12 insertions(+), 12 deletions(-)

drivers/input/touchscreen/goodix_berlin_core.c +12 ~~-12~~

index e273fb8edc6b92bcbad0fd35223a841d7da7d671..7f8cfdd106fae03a6b197582bca4eb61f80182c6 100644
@@ -165,7 +165,7 @@ struct goodix_berlin_core {
  	struct device *dev;
  	struct regmap *regmap;
  	struct regulator *avdd;
-	struct regulator *iovdd;
+	struct regulator *vddio;
  	struct gpio_desc *reset_gpio;
  	struct touchscreen_properties props;
  	struct goodix_berlin_fw_version fw_version;

@@ -248,19 +248,19 @@ static int goodix_berlin_power_on(struct goodix_berlin_core *cd)
  {
  	int error;
 
-	error = regulator_enable(cd->iovdd);
+	error = regulator_enable(cd->vddio);
  	if (error) {
-		dev_err(cd->dev, "Failed to enable iovdd: %d\n", error);
+		dev_err(cd->dev, "Failed to enable vddio: %d\n", error);
  		return error;
  	}
 
-	/* Vendor waits 3ms for IOVDD to settle */
+	/* Vendor waits 3ms for VDDIO to settle */
  	usleep_range(3000, 3100);
 
  	error = regulator_enable(cd->avdd);
  	if (error) {
  		dev_err(cd->dev, "Failed to enable avdd: %d\n", error);
-		goto err_iovdd_disable;
+		goto err_vddio_disable;
  	}
 
  	/* Vendor waits 15ms for AVDD to settle */

@@ -283,8 +283,8 @@ static int goodix_berlin_power_on(struct goodix_berlin_core *cd)
  err_dev_reset:
  	gpiod_set_value_cansleep(cd->reset_gpio, 1);
  	regulator_disable(cd->avdd);
-err_iovdd_disable:
-	regulator_disable(cd->iovdd);
+err_vddio_disable:
+	regulator_disable(cd->vddio);
  	return error;
  }

@@ -292,7 +292,7 @@ static void goodix_berlin_power_off(struct goodix_berlin_core *cd)
  {
  	gpiod_set_value_cansleep(cd->reset_gpio, 1);
  	regulator_disable(cd->avdd);
-	regulator_disable(cd->iovdd);
+	regulator_disable(cd->vddio);
  }
 
  static int goodix_berlin_read_version(struct goodix_berlin_core *cd)

@@ -744,10 +744,10 @@ int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
  		return dev_err_probe(dev, PTR_ERR(cd->avdd),
  				     "Failed to request avdd regulator\n");
 
-	cd->iovdd = devm_regulator_get(dev, "iovdd");
-	if (IS_ERR(cd->iovdd))
-		return dev_err_probe(dev, PTR_ERR(cd->iovdd),
-				     "Failed to request iovdd regulator\n");
+	cd->vddio = devm_regulator_get(dev, "vddio");
+	if (IS_ERR(cd->vddio))
+		return dev_err_probe(dev, PTR_ERR(cd->vddio),
+				     "Failed to request vddio regulator\n");
 
  	error = goodix_berlin_power_on(cd);
  	if (error) {

Luca Weiss

author

2025-01-03 09:52 UTC #6 lore raw

Re: [PATCH 2/2] Input: goodix-berlin - fix vddio regulator references

Quoted 8 lines · Neil Armstrong

Hi,
Quoted 3 lines · Luca Weiss
As per dt-bindings the property is called vddio-supply, so use the correct name in the driver instead of iovdd. The datasheet also calls the supply 'VDDIO'.
This is duplicate of https://lore.kernel.org/all/[email protected]/

Oh, any idea why it wasn't picked up since August?

Quoted 4 lines

But it's still valid:
Reviewed-by Neil Armstrong <[email protected]>

Thanks!

Regards
Luca

Quoted 82 lines

Fixes 44362279bdd4 ("Input: add core support for Goodix Berlin Touchscreen IC")

Cc [email protected]

Signed-off-by Luca Weiss <[email protected]>

---
  drivers/input/touchscreen/goodix_berlin_core.c | 24 ++++++++++++------------
  1 file changed, 12 insertions(+), 12 deletions(-)

drivers/input/touchscreen/goodix_berlin_core.c +12 ~~-12~~

index e273fb8edc6b92bcbad0fd35223a841d7da7d671..7f8cfdd106fae03a6b197582bca4eb61f80182c6 100644
@@ -165,7 +165,7 @@ struct goodix_berlin_core {
  	struct device *dev;
  	struct regmap *regmap;
  	struct regulator *avdd;
-	struct regulator *iovdd;
+	struct regulator *vddio;
  	struct gpio_desc *reset_gpio;
  	struct touchscreen_properties props;
  	struct goodix_berlin_fw_version fw_version;

@@ -248,19 +248,19 @@ static int goodix_berlin_power_on(struct goodix_berlin_core *cd)
  {
  	int error;
 
-	error = regulator_enable(cd->iovdd);
+	error = regulator_enable(cd->vddio);
  	if (error) {
-		dev_err(cd->dev, "Failed to enable iovdd: %d\n", error);
+		dev_err(cd->dev, "Failed to enable vddio: %d\n", error);
  		return error;
  	}
 
-	/* Vendor waits 3ms for IOVDD to settle */
+	/* Vendor waits 3ms for VDDIO to settle */
  	usleep_range(3000, 3100);
 
  	error = regulator_enable(cd->avdd);
  	if (error) {
  		dev_err(cd->dev, "Failed to enable avdd: %d\n", error);
-		goto err_iovdd_disable;
+		goto err_vddio_disable;
  	}
 
  	/* Vendor waits 15ms for AVDD to settle */

@@ -283,8 +283,8 @@ static int goodix_berlin_power_on(struct goodix_berlin_core *cd)
  err_dev_reset:
  	gpiod_set_value_cansleep(cd->reset_gpio, 1);
  	regulator_disable(cd->avdd);
-err_iovdd_disable:
-	regulator_disable(cd->iovdd);
+err_vddio_disable:
+	regulator_disable(cd->vddio);
  	return error;
  }

@@ -292,7 +292,7 @@ static void goodix_berlin_power_off(struct goodix_berlin_core *cd)
  {
  	gpiod_set_value_cansleep(cd->reset_gpio, 1);
  	regulator_disable(cd->avdd);
-	regulator_disable(cd->iovdd);
+	regulator_disable(cd->vddio);
  }
 
  static int goodix_berlin_read_version(struct goodix_berlin_core *cd)

@@ -744,10 +744,10 @@ int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
  		return dev_err_probe(dev, PTR_ERR(cd->avdd),
  				     "Failed to request avdd regulator\n");
 
-	cd->iovdd = devm_regulator_get(dev, "iovdd");
-	if (IS_ERR(cd->iovdd))
-		return dev_err_probe(dev, PTR_ERR(cd->iovdd),
-				     "Failed to request iovdd regulator\n");
+	cd->vddio = devm_regulator_get(dev, "vddio");
+	if (IS_ERR(cd->vddio))
+		return dev_err_probe(dev, PTR_ERR(cd->vddio),
+				     "Failed to request vddio regulator\n");
 
  	error = goodix_berlin_power_on(cd);
  	if (error) {

Dmitry Torokhov

maintainer Applied

2025-02-25 06:29 UTC #7 lore raw

Re: [PATCH 2/2] Input: goodix-berlin - fix vddio regulator references

In reply to Luca Weiss

Quoted 7 lines · Luca Weiss

As per dt-bindings the property is called vddio-supply, so use the correct name in the driver instead of iovdd. The datasheet also calls the supply 'VDDIO'.
Fixes 44362279bdd4 ("Input: add core support for Goodix Berlin Touchscreen IC")
Cc [email protected]
Signed-off-by Luca Weiss <[email protected]>

Applied, thank you.

Signature · 1 line

Dmitry