[PATCH 0/2] Some fixes for Goodix Berlin touchscreen driver

[PATCH 0/2] Some fixes for Goodix Berlin touchscreen driver

[Luca Weiss]

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 <luca.weiss@fairphone.com>
---
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,
-- 
Luca Weiss <luca.weiss@fairphone.com>

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

[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 <adrian@travitia.xyz>
Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 drivers/input/touchscreen/goodix_berlin_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/goodix_berlin_core.c b/drivers/input/touchscreen/goodix_berlin_core.c
index 3fc03cf0ca23fdbe36028a6228030d3ffb8d3a09..e273fb8edc6b92bcbad0fd35223a841d7da7d671 100644
--- a/drivers/input/touchscreen/goodix_berlin_core.c
+++ b/drivers/input/touchscreen/goodix_berlin_core.c
@@ -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);

-- 
2.47.1

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

[Neil Armstrong]

On 03/01/2025 10:21, Luca Weiss wrote:
> 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 <adrian@travitia.xyz>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>   drivers/input/touchscreen/goodix_berlin_core.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/touchscreen/goodix_berlin_core.c b/drivers/input/touchscreen/goodix_berlin_core.c
> index 3fc03cf0ca23fdbe36028a6228030d3ffb8d3a09..e273fb8edc6b92bcbad0fd35223a841d7da7d671 100644
> --- a/drivers/input/touchscreen/goodix_berlin_core.c
> +++ b/drivers/input/touchscreen/goodix_berlin_core.c
> @@ -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 <neil.armstrong@linaro.org>

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

[Dmitry Torokhov]

On Fri, Jan 03, 2025 at 10:21:35AM +0100, Luca Weiss wrote:
> 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 <adrian@travitia.xyz>
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>

Applied, thank you.

-- 
Dmitry

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

[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: stable@vger.kernel.org
Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 drivers/input/touchscreen/goodix_berlin_core.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/input/touchscreen/goodix_berlin_core.c b/drivers/input/touchscreen/goodix_berlin_core.c
index e273fb8edc6b92bcbad0fd35223a841d7da7d671..7f8cfdd106fae03a6b197582bca4eb61f80182c6 100644
--- a/drivers/input/touchscreen/goodix_berlin_core.c
+++ b/drivers/input/touchscreen/goodix_berlin_core.c
@@ -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) {

-- 
2.47.1

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

[Neil Armstrong]

Hi,

On 03/01/2025 10:21, Luca Weiss wrote:
> 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/20240805155806.16203-1-danila@jiaxyga.com/

But it's still valid:

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

> 
> Fixes: 44362279bdd4 ("Input: add core support for Goodix Berlin Touchscreen IC")
> Cc: stable@vger.kernel.org
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>   drivers/input/touchscreen/goodix_berlin_core.c | 24 ++++++++++++------------
>   1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/goodix_berlin_core.c b/drivers/input/touchscreen/goodix_berlin_core.c
> index e273fb8edc6b92bcbad0fd35223a841d7da7d671..7f8cfdd106fae03a6b197582bca4eb61f80182c6 100644
> --- a/drivers/input/touchscreen/goodix_berlin_core.c
> +++ b/drivers/input/touchscreen/goodix_berlin_core.c
> @@ -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) {
> 

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

[Luca Weiss]

On Fri Jan 3, 2025 at 10:27 AM CET, Neil Armstrong wrote:
> Hi,
>
> On 03/01/2025 10:21, Luca Weiss wrote:
> > 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/20240805155806.16203-1-danila@jiaxyga.com/

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

>
> But it's still valid:
>
> Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Thanks!

Regards
Luca

>
> > 
> > Fixes: 44362279bdd4 ("Input: add core support for Goodix Berlin Touchscreen IC")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > ---
> >   drivers/input/touchscreen/goodix_berlin_core.c | 24 ++++++++++++------------
> >   1 file changed, 12 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/input/touchscreen/goodix_berlin_core.c b/drivers/input/touchscreen/goodix_berlin_core.c
> > index e273fb8edc6b92bcbad0fd35223a841d7da7d671..7f8cfdd106fae03a6b197582bca4eb61f80182c6 100644
> > --- a/drivers/input/touchscreen/goodix_berlin_core.c
> > +++ b/drivers/input/touchscreen/goodix_berlin_core.c
> > @@ -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) {
> > 

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

[Dmitry Torokhov]

On Fri, Jan 03, 2025 at 10:21:36AM +0100, Luca Weiss wrote:
> 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: stable@vger.kernel.org
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>

Applied, thank you.

-- 
Dmitry