Hi Jagan, On Tue, Feb 12, 2019 at 04:54:37PM +0530, Jagan Teki wrote: > Goodix CTP controllers have AVDD28 pin connected to voltage > regulator which may not be turned on by default, like for GT5663. > > Add support for such ctp used boards by adding voltage regulator > handling code to goodix ctp driver. > > Signed-off-by: Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx> > --- > drivers/input/touchscreen/goodix.c | 44 ++++++++++++++++++++++++++---- > 1 file changed, 39 insertions(+), 5 deletions(-) > > diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c > index f2d9c2c41885..e92b90be1ac2 100644 > --- a/drivers/input/touchscreen/goodix.c > +++ b/drivers/input/touchscreen/goodix.c > @@ -27,6 +27,7 @@ > #include <linux/delay.h> > #include <linux/irq.h> > #include <linux/interrupt.h> > +#include <linux/regulator/consumer.h> > #include <linux/slab.h> > #include <linux/acpi.h> > #include <linux/of.h> > @@ -47,6 +48,7 @@ struct goodix_ts_data { > struct touchscreen_properties prop; > unsigned int max_touch_num; > unsigned int int_trigger_type; > + struct regulator *avdd28; > struct gpio_desc *gpiod_int; > struct gpio_desc *gpiod_rst; > u16 id; > @@ -761,6 +763,13 @@ static void goodix_config_cb(const struct firmware *cfg, void *ctx) > complete_all(&ts->firmware_loading_complete); > } > > +static void goodix_disable_regulator(void *arg) > +{ > + struct goodix_ts_data *ts = arg; > + > + regulator_disable(ts->avdd28); > +} > + > static int goodix_ts_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > @@ -786,25 +795,46 @@ static int goodix_ts_probe(struct i2c_client *client, > if (error) > return error; > > + ts->avdd28 = devm_regulator_get(&client->dev, "AVDD28"); > + if (IS_ERR(ts->avdd28)) { > + error = PTR_ERR(ts->avdd28); > + if (error != -EPROBE_DEFER) > + dev_err(&client->dev, > + "Failed to get AVDD28 regulator: %d\n", error); > + return error; > + } > + > + /* power the controller */ > + error = regulator_enable(ts->avdd28); > + if (error) { > + dev_err(&client->dev, "Controller fail to enable AVDD28\n"); > + return error; > + } > + > + error = devm_add_action_or_reset(&client->dev, > + goodix_disable_regulator, ts); > + if (error) > + return error; > + > if (ts->gpiod_int && ts->gpiod_rst) { > /* reset the controller */ > error = goodix_reset(ts); > if (error) { > dev_err(&client->dev, "Controller reset failed.\n"); > - return error; > + goto error; Why are you changing this? You added goodix_disable_regulator() as custom devm action, so it will fire if probe() returns with error. Thanks. -- Dmitry