Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1578875 > unrolled thread

[PATCH 2/3] Input: tsc2004/5 - fix regulator handling

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/3] Input: tsc2004/5 - fix regulator handling Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-02-11 01:10 +0100
    Re: [PATCH 2/3] Input: tsc2004/5 - fix regulator handling Sebastian Reichel <sre@kernel.org> - 2017-02-11 18:10 +0100

#1578875 — [PATCH 2/3] Input: tsc2004/5 - fix regulator handling

In case of an optional regulator missing regulator core will return
ERR_PTR(-ENOENT) and not NULL, so the check for missing regulator is
incorrect. Also, the regulator is not optional, it may simply be missing
from platform decsription, so let's use devm_regulator_get() and rely on
regulator core to give us dummy supply when real one is not available.

Fixes: d257f2980feb ("Input: tsc2005 - convert to gpiod")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

Sebastian, I am wondering, what regulator this is. If it is IO VDD,
then I think we activate it too late (i.e. we are truing to shut off
the controller before we turn the regulator on. If it is sensor VDD,
then we probably need to mention it, and also add IO VVD supply as
well.

 drivers/input/touchscreen/tsc200x-core.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/input/touchscreen/tsc200x-core.c b/drivers/input/touchscreen/tsc200x-core.c
index b7059ed8872e..1c14a38e3748 100644
--- a/drivers/input/touchscreen/tsc200x-core.c
+++ b/drivers/input/touchscreen/tsc200x-core.c
@@ -527,10 +527,10 @@ int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id,
 		return error;
 	}
 
-	ts->vio = devm_regulator_get_optional(dev, "vio");
+	ts->vio = devm_regulator_get(dev, "vio");
 	if (IS_ERR(ts->vio)) {
 		error = PTR_ERR(ts->vio);
-		dev_err(dev, "vio regulator missing (%d)", error);
+		dev_err(dev, "error acquiring vio regulator: %d", error);
 		return error;
 	}
 
@@ -587,12 +587,9 @@ int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id,
 		return error;
 	}
 
-	/* enable regulator for DT */
-	if (ts->vio) {
-		error = regulator_enable(ts->vio);
-		if (error)
-			return error;
-	}
+	error = regulator_enable(ts->vio);
+	if (error)
+		return error;
 
 	dev_set_drvdata(dev, ts);
 	error = sysfs_create_group(&dev->kobj, &tsc200x_attr_group);
@@ -615,8 +612,7 @@ int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id,
 err_remove_sysfs:
 	sysfs_remove_group(&dev->kobj, &tsc200x_attr_group);
 disable_regulator:
-	if (ts->vio)
-		regulator_disable(ts->vio);
+	regulator_disable(ts->vio);
 	return error;
 }
 EXPORT_SYMBOL_GPL(tsc200x_probe);
@@ -627,8 +623,7 @@ int tsc200x_remove(struct device *dev)
 
 	sysfs_remove_group(&dev->kobj, &tsc200x_attr_group);
 
-	if (ts->vio)
-		regulator_disable(ts->vio);
+	regulator_disable(ts->vio);
 
 	return 0;
 }
-- 
2.11.0.483.g087da7b7c-goog

[toc] | [next] | [standalone]

#1579033

[Multipart message — attachments visible in raw view] — view raw

Hi Dmitry,

On Fri, Feb 10, 2017 at 04:06:22PM -0800, Dmitry Torokhov wrote:
> In case of an optional regulator missing regulator core will return
> ERR_PTR(-ENOENT) and not NULL, so the check for missing regulator is
> incorrect. Also, the regulator is not optional, it may simply be missing
> from platform decsription, so let's use devm_regulator_get() and rely on
> regulator core to give us dummy supply when real one is not available.
>
> Fixes: d257f2980feb ("Input: tsc2005 - convert to gpiod")
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Acked-By: Sebastian Reichel <sre@kernel.org>

> ---
> 
> Sebastian, I am wondering, what regulator this is.

On N900 the same regultor is connected to I/OVDD & SNSVDD.

> If it is IO VDD, then I think we activate it too late (i.e. we are
> truing to shut off the controller before we turn the regulator on.

Yes, it should be moved.

> If it is sensor VDD, then we probably need to mention it, and also
> add IO VVD supply as well.

-- Sebastian

> 
>  drivers/input/touchscreen/tsc200x-core.c | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/tsc200x-core.c b/drivers/input/touchscreen/tsc200x-core.c
> index b7059ed8872e..1c14a38e3748 100644
> --- a/drivers/input/touchscreen/tsc200x-core.c
> +++ b/drivers/input/touchscreen/tsc200x-core.c
> @@ -527,10 +527,10 @@ int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id,
>  		return error;
>  	}
>  
> -	ts->vio = devm_regulator_get_optional(dev, "vio");
> +	ts->vio = devm_regulator_get(dev, "vio");
>  	if (IS_ERR(ts->vio)) {
>  		error = PTR_ERR(ts->vio);
> -		dev_err(dev, "vio regulator missing (%d)", error);
> +		dev_err(dev, "error acquiring vio regulator: %d", error);
>  		return error;
>  	}
>  
> @@ -587,12 +587,9 @@ int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id,
>  		return error;
>  	}
>  
> -	/* enable regulator for DT */
> -	if (ts->vio) {
> -		error = regulator_enable(ts->vio);
> -		if (error)
> -			return error;
> -	}
> +	error = regulator_enable(ts->vio);
> +	if (error)
> +		return error;
>  
>  	dev_set_drvdata(dev, ts);
>  	error = sysfs_create_group(&dev->kobj, &tsc200x_attr_group);
> @@ -615,8 +612,7 @@ int tsc200x_probe(struct device *dev, int irq, const struct input_id *tsc_id,
>  err_remove_sysfs:
>  	sysfs_remove_group(&dev->kobj, &tsc200x_attr_group);
>  disable_regulator:
> -	if (ts->vio)
> -		regulator_disable(ts->vio);
> +	regulator_disable(ts->vio);
>  	return error;
>  }
>  EXPORT_SYMBOL_GPL(tsc200x_probe);
> @@ -627,8 +623,7 @@ int tsc200x_remove(struct device *dev)
>  
>  	sysfs_remove_group(&dev->kobj, &tsc200x_attr_group);
>  
> -	if (ts->vio)
> -		regulator_disable(ts->vio);
> +	regulator_disable(ts->vio);
>  
>  	return 0;
>  }
> -- 
> 2.11.0.483.g087da7b7c-goog
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web