| Message ID | 20240417090527.15357-2-eichest@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add a property to turn off the max touch controller if not used | expand |
Hi Stefan, On Wed, Apr 17, 2024 at 11:05:24AM +0200, Stefan Eichenberger wrote: > From: Stefan Eichenberger <stefan.eichenberger@toradex.com> > > Add a separate function for power off and power on instead of calling > regulator_bulk_enable and regulator_bulk_disable directly. > > Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com> > --- > drivers/input/touchscreen/atmel_mxt_ts.c | 59 +++++++++++++++--------- > 1 file changed, 37 insertions(+), 22 deletions(-) > > diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c > index 542a31448c8f..52867ce3b9b6 100644 > --- a/drivers/input/touchscreen/atmel_mxt_ts.c > +++ b/drivers/input/touchscreen/atmel_mxt_ts.c > @@ -1307,6 +1307,38 @@ static int mxt_soft_reset(struct mxt_data *data) > return 0; > } > > +static int mxt_power_on(struct mxt_data *data) > +{ > + int error; > + > + error = regulator_bulk_enable(ARRAY_SIZE(data->regulators), > + data->regulators); > + if (error) { > + dev_err(&data->client->dev, "failed to enable regulators: %d\n", > + error); > + return error; > + } > + > + msleep(MXT_BACKUP_TIME); > + > + if (data->reset_gpio) { > + /* Wait a while and then de-assert the RESET GPIO line */ > + msleep(MXT_RESET_GPIO_TIME); > + gpiod_set_value(data->reset_gpio, 0); > + msleep(MXT_RESET_INVALID_CHG); > + } > + > + return 0; > +} > + > +static void mxt_power_off(struct mxt_data *data) > +{ > + if (data->reset_gpio) > + gpiod_set_value(data->reset_gpio, 1); > + > + regulator_bulk_disable(ARRAY_SIZE(data->regulators), data->regulators); > +} > + > static void mxt_update_crc(struct mxt_data *data, u8 cmd, u8 value) > { > /* > @@ -3305,25 +3337,9 @@ static int mxt_probe(struct i2c_client *client) > return error; > } > > - error = regulator_bulk_enable(ARRAY_SIZE(data->regulators), > - data->regulators); > - if (error) { > - dev_err(&client->dev, "failed to enable regulators: %d\n", > - error); > + error = mxt_power_on(data); > + if (error) > return error; > - } > - /* > - * The device takes 40ms to come up after power-on according > - * to the mXT224 datasheet, page 13. > - */ > - msleep(MXT_BACKUP_TIME); > - > - if (data->reset_gpio) { > - /* Wait a while and then de-assert the RESET GPIO line */ > - msleep(MXT_RESET_GPIO_TIME); > - gpiod_set_value(data->reset_gpio, 0); > - msleep(MXT_RESET_INVALID_CHG); > - } > > /* > * Controllers like mXT1386 have a dedicated WAKE line that could be > @@ -3361,8 +3377,8 @@ static int mxt_probe(struct i2c_client *client) > mxt_free_input_device(data); > mxt_free_object_table(data); > err_disable_regulators: > - regulator_bulk_disable(ARRAY_SIZE(data->regulators), > - data->regulators); > + mxt_power_off(data); > + > return error; > } > > @@ -3374,8 +3390,7 @@ static void mxt_remove(struct i2c_client *client) > sysfs_remove_group(&client->dev.kobj, &mxt_attr_group); > mxt_free_input_device(data); > mxt_free_object_table(data); > - regulator_bulk_disable(ARRAY_SIZE(data->regulators), > - data->regulators); > + mxt_power_off(data); This change means that on unbind we will leave with GPIO line asserted. Won't this potentially cause some current leakage? Why do we need to have reset asserted here? Thanks.
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c index 542a31448c8f..52867ce3b9b6 100644 --- a/drivers/input/touchscreen/atmel_mxt_ts.c +++ b/drivers/input/touchscreen/atmel_mxt_ts.c @@ -1307,6 +1307,38 @@ static int mxt_soft_reset(struct mxt_data *data) return 0; } +static int mxt_power_on(struct mxt_data *data) +{ + int error; + + error = regulator_bulk_enable(ARRAY_SIZE(data->regulators), + data->regulators); + if (error) { + dev_err(&data->client->dev, "failed to enable regulators: %d\n", + error); + return error; + } + + msleep(MXT_BACKUP_TIME); + + if (data->reset_gpio) { + /* Wait a while and then de-assert the RESET GPIO line */ + msleep(MXT_RESET_GPIO_TIME); + gpiod_set_value(data->reset_gpio, 0); + msleep(MXT_RESET_INVALID_CHG); + } + + return 0; +} + +static void mxt_power_off(struct mxt_data *data) +{ + if (data->reset_gpio) + gpiod_set_value(data->reset_gpio, 1); + + regulator_bulk_disable(ARRAY_SIZE(data->regulators), data->regulators); +} + static void mxt_update_crc(struct mxt_data *data, u8 cmd, u8 value) { /* @@ -3305,25 +3337,9 @@ static int mxt_probe(struct i2c_client *client) return error; } - error = regulator_bulk_enable(ARRAY_SIZE(data->regulators), - data->regulators); - if (error) { - dev_err(&client->dev, "failed to enable regulators: %d\n", - error); + error = mxt_power_on(data); + if (error) return error; - } - /* - * The device takes 40ms to come up after power-on according - * to the mXT224 datasheet, page 13. - */ - msleep(MXT_BACKUP_TIME); - - if (data->reset_gpio) { - /* Wait a while and then de-assert the RESET GPIO line */ - msleep(MXT_RESET_GPIO_TIME); - gpiod_set_value(data->reset_gpio, 0); - msleep(MXT_RESET_INVALID_CHG); - } /* * Controllers like mXT1386 have a dedicated WAKE line that could be @@ -3361,8 +3377,8 @@ static int mxt_probe(struct i2c_client *client) mxt_free_input_device(data); mxt_free_object_table(data); err_disable_regulators: - regulator_bulk_disable(ARRAY_SIZE(data->regulators), - data->regulators); + mxt_power_off(data); + return error; } @@ -3374,8 +3390,7 @@ static void mxt_remove(struct i2c_client *client) sysfs_remove_group(&client->dev.kobj, &mxt_attr_group); mxt_free_input_device(data); mxt_free_object_table(data); - regulator_bulk_disable(ARRAY_SIZE(data->regulators), - data->regulators); + mxt_power_off(data); } static int mxt_suspend(struct device *dev)