Re: [PATCH 3/3 v5] Input: cyttsp - Obtain regulators

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

Hi Linus,

On Mon, May 10, 2021 at 12:34:16AM +0200, Linus Walleij wrote:
> The CYTTSP TMA340 chips have two supplies: VCPIN and
> VDD for analog and digital voltage respectively.
> Add some minimal code to obtain and enable these
> regulators if need be.
> 
> Reviewed-by: Javier Martinez Canillas <javier@dowhile0.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v3->v5:
> - Rebase on v5.13-rc1
> ChangeLog v1->v3:
> - Collect Javier's reviewed-by.
> ---
>  drivers/input/touchscreen/cyttsp_core.c | 30 +++++++++++++++++++++++--
>  drivers/input/touchscreen/cyttsp_core.h |  2 ++
>  2 files changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/cyttsp_core.c b/drivers/input/touchscreen/cyttsp_core.c
> index 106dd4962785..5af4d034b36b 100644
> --- a/drivers/input/touchscreen/cyttsp_core.c
> +++ b/drivers/input/touchscreen/cyttsp_core.c
> @@ -22,6 +22,7 @@
>  #include <linux/slab.h>
>  #include <linux/property.h>
>  #include <linux/gpio/consumer.h>
> +#include <linux/regulator/consumer.h>
>  
>  #include "cyttsp_core.h"
>  
> @@ -628,6 +629,19 @@ struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
>  	ts->bus_ops = bus_ops;
>  	ts->irq = irq;
>  
> +	/*
> +	 * VCPIN is the analog voltage supply
> +	 * VDD is the digital voltage supply
> +	 */
> +	ts->regulators[0].supply = "vcpin";
> +	ts->regulators[1].supply = "vdd";
> +	error = devm_regulator_bulk_get(dev, ARRAY_SIZE(ts->regulators),
> +					ts->regulators);
> +	if (error) {
> +		dev_err(dev, "Failed to get regulators: %d\n", error);
> +		return ERR_PTR(error);
> +	}
> +
>  	ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
>  	if (IS_ERR(ts->reset_gpio)) {
>  		error = PTR_ERR(ts->reset_gpio);
> @@ -673,20 +687,32 @@ struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
>  		return ERR_PTR(error);
>  	}
>  
> +	error = regulator_bulk_enable(ARRAY_SIZE(ts->regulators),
> +				      ts->regulators);
> +	if (error) {
> +		dev_err(dev, "Cannot enable regulators: %d\n", error);
> +		return ERR_PTR(error);
> +	}

Please use devm_add_action_or_reset() to inject code disabling
regulators on error or remove() into devm sequence, otherwise we are
leaving regulators on.

Thanks.

-- 
Dmitry