[PATCH] Input: goodix-berlin - Fix VDDIO regulator name according to dt-bindings

[PATCH] Input: goodix-berlin - Fix VDDIO regulator name according to dt-bindings

[Danila Tikhonov]

The dt-bindings specify the regulator as "vddio" instead of "iovdd".

This patch fixes the regulator name from "iovdd" to "vddio" in the
driver code to align with the dt-bindings. Fixing the dt-bindings
would break ABI, hence the fix is made in the driver instead.

There are no users of this regulator сurrently.

Fixes: 44362279bdd4 ("Input: add core support for Goodix Berlin Touchscreen IC")
Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
---
 .../input/touchscreen/goodix_berlin_core.c    | 26 +++++++++----------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/input/touchscreen/goodix_berlin_core.c b/drivers/input/touchscreen/goodix_berlin_core.c
index 0bfca897ce5a..b5d6e6360fff 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,22 +248,22 @@ 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 IOVDD to settle */
+	/* Vendor waits 15ms for VDDIO to settle */
 	usleep_range(15000, 15100);
 
 	gpiod_set_value_cansleep(cd->reset_gpio, 0);
@@ -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.45.2

Re: [PATCH] Input: goodix-berlin - Fix VDDIO regulator name according to dt-bindings

[Dmitry Torokhov]

Hi Danila,

On Mon, Aug 05, 2024 at 06:58:06PM +0300, Danila Tikhonov wrote:
> The dt-bindings specify the regulator as "vddio" instead of "iovdd".
> 
> This patch fixes the regulator name from "iovdd" to "vddio" in the
> driver code to align with the dt-bindings. Fixing the dt-bindings
> would break ABI, hence the fix is made in the driver instead.
> 
> There are no users of this regulator сurrently.

If there are no users (and the binding is pretty new) we should consider
all options. Do you know the name of the supply in the datasheet?

Thanks.

-- 
Dmitry

Re: [PATCH] Input: goodix-berlin - Fix VDDIO regulator name according to dt-bindings

[neil.armstrong]

On 05/08/2024 17:58, Danila Tikhonov wrote:
> The dt-bindings specify the regulator as "vddio" instead of "iovdd".
> 
> This patch fixes the regulator name from "iovdd" to "vddio" in the
> driver code to align with the dt-bindings. Fixing the dt-bindings
> would break ABI, hence the fix is made in the driver instead.
> 
> There are no users of this regulator сurrently.
> 
> Fixes: 44362279bdd4 ("Input: add core support for Goodix Berlin Touchscreen IC")
> Signed-off-by: Danila Tikhonov <danila@jiaxyga.com>
> ---
>   .../input/touchscreen/goodix_berlin_core.c    | 26 +++++++++----------
>   1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/goodix_berlin_core.c b/drivers/input/touchscreen/goodix_berlin_core.c
> index 0bfca897ce5a..b5d6e6360fff 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,22 +248,22 @@ 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 IOVDD to settle */
> +	/* Vendor waits 15ms for VDDIO to settle */
>   	usleep_range(15000, 15100);
>   
>   	gpiod_set_value_cansleep(cd->reset_gpio, 0);
> @@ -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) {

My bad on this one...

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

Re: [PATCH] Input: goodix-berlin - Fix VDDIO regulator name according to dt-bindings

[neil.armstrong]

Hi,

On 06/09/2024 07:29, Dmitry Torokhov wrote:
> Hi Danila,
> 
> On Mon, Aug 05, 2024 at 06:58:06PM +0300, Danila Tikhonov wrote:
>> The dt-bindings specify the regulator as "vddio" instead of "iovdd".
>>
>> This patch fixes the regulator name from "iovdd" to "vddio" in the
>> driver code to align with the dt-bindings. Fixing the dt-bindings
>> would break ABI, hence the fix is made in the driver instead.
>>
>> There are no users of this regulator сurrently.
> 
> If there are no users (and the binding is pretty new) we should consider
> all options. Do you know the name of the supply in the datasheet?

The names comes from the downstream driver & bindings, but we don't
declare them on the Qualcomm platforms using the berlin touch ICs.

Perhaps someone from Goodix or someone with access to datasheet could confirm...

Anyway, this aligns with bindings so it's a correct patch.

Neil

> 
> Thanks.
>