linux-kernel - Re: [PATCH 2/3] Input: tsc2004/5

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 11 Feb 2017 18:02:48 +0100
From:   Sebastian Reichel <sre@...nel.org>
To:     Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc:     linux-input@...r.kernel.org,
        Pali Rohár <pali.rohar@...il.com>,
        Michael Welling <mwelling@...e.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] Input: tsc2004/5 - fix regulator handling

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@...il.com>

Acked-By: Sebastian Reichel <sre@...nel.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
> 

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)