| Message ID | 20200325133334.19346-14-jiada_wang@mentor.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | atmel_mxt_ts misc | expand |
25.03.2020 16:32, Jiada Wang пишет: > From: Nick Dyer <nick.dyer@itdev.co.uk> > > Allow the driver to optionally manage enabling/disable power to the touch > controller itself. If the regulators are not present then use the deep > sleep power mode instead. > > For a correct power on sequence, it is required that we have control over > the RESET line. ... > + data->reg_vdd = devm_regulator_get(dev, "vdd"); > + if (IS_ERR(data->reg_vdd)) { > + error = PTR_ERR(data->reg_vdd); > + dev_err(dev, "Error %d getting vdd regulator\n", error); > + goto fail; > + } > + > + data->reg_avdd = devm_regulator_get(dev, "avdd"); > + if (IS_ERR(data->reg_avdd)) { > + error = PTR_ERR(data->reg_avdd); > + dev_err(dev, "Error %d getting avdd regulator\n", error); > + goto fail_release; > + } Hello Jiada, The new regulator properties should be documented in the device-tree binding.
Hi Dmitry On 2020/03/26 4:05, Dmitry Osipenko wrote: > 25.03.2020 16:32, Jiada Wang пишет: >> From: Nick Dyer <nick.dyer@itdev.co.uk> >> >> Allow the driver to optionally manage enabling/disable power to the touch >> controller itself. If the regulators are not present then use the deep >> sleep power mode instead. >> >> For a correct power on sequence, it is required that we have control over >> the RESET line. > > ... >> + data->reg_vdd = devm_regulator_get(dev, "vdd"); >> + if (IS_ERR(data->reg_vdd)) { >> + error = PTR_ERR(data->reg_vdd); >> + dev_err(dev, "Error %d getting vdd regulator\n", error); >> + goto fail; >> + } >> + >> + data->reg_avdd = devm_regulator_get(dev, "avdd"); >> + if (IS_ERR(data->reg_avdd)) { >> + error = PTR_ERR(data->reg_avdd); >> + dev_err(dev, "Error %d getting avdd regulator\n", error); >> + goto fail_release; >> + } > > Hello Jiada, > > The new regulator properties should be documented in the device-tree > binding. > I will document new regulator properties in a separate commit in v10 patch-set Thanks, Jiada
27.03.2020 14:09, Wang, Jiada пишет: > Hi Dmitry > > On 2020/03/26 4:05, Dmitry Osipenko wrote: >> 25.03.2020 16:32, Jiada Wang пишет: >>> From: Nick Dyer <nick.dyer@itdev.co.uk> >>> >>> Allow the driver to optionally manage enabling/disable power to the >>> touch >>> controller itself. If the regulators are not present then use the deep >>> sleep power mode instead. >>> >>> For a correct power on sequence, it is required that we have control >>> over >>> the RESET line. >> >> ... >>> + data->reg_vdd = devm_regulator_get(dev, "vdd"); >>> + if (IS_ERR(data->reg_vdd)) { >>> + error = PTR_ERR(data->reg_vdd); >>> + dev_err(dev, "Error %d getting vdd regulator\n", error); >>> + goto fail; >>> + } >>> + >>> + data->reg_avdd = devm_regulator_get(dev, "avdd"); >>> + if (IS_ERR(data->reg_avdd)) { >>> + error = PTR_ERR(data->reg_avdd); >>> + dev_err(dev, "Error %d getting avdd regulator\n", error); >>> + goto fail_release; >>> + } >> >> Hello Jiada, >> >> The new regulator properties should be documented in the device-tree >> binding. >> > I will document new regulator properties in a separate commit in > v10 patch-set Please make sure that all patches are added in a correct order and that they at least compile. This patch doesn't compile: CC drivers/input/touchscreen/atmel_mxt_ts.o drivers/input/touchscreen/atmel_mxt_ts.c:34:10: fatal error: dt-bindings/input/atmel_mxt_ts.h: No such file or directory 34 | #include <dt-bindings/input/atmel_mxt_ts.h>
Hi Dmitry On 2020/03/30 3:54, Dmitry Osipenko wrote: > 27.03.2020 14:09, Wang, Jiada пишет: >> Hi Dmitry >> >> On 2020/03/26 4:05, Dmitry Osipenko wrote: >>> 25.03.2020 16:32, Jiada Wang пишет: >>>> From: Nick Dyer <nick.dyer@itdev.co.uk> >>>> >>>> Allow the driver to optionally manage enabling/disable power to the >>>> touch >>>> controller itself. If the regulators are not present then use the deep >>>> sleep power mode instead. >>>> >>>> For a correct power on sequence, it is required that we have control >>>> over >>>> the RESET line. >>> >>> ... >>>> + data->reg_vdd = devm_regulator_get(dev, "vdd"); >>>> + if (IS_ERR(data->reg_vdd)) { >>>> + error = PTR_ERR(data->reg_vdd); >>>> + dev_err(dev, "Error %d getting vdd regulator\n", error); >>>> + goto fail; >>>> + } >>>> + >>>> + data->reg_avdd = devm_regulator_get(dev, "avdd"); >>>> + if (IS_ERR(data->reg_avdd)) { >>>> + error = PTR_ERR(data->reg_avdd); >>>> + dev_err(dev, "Error %d getting avdd regulator\n", error); >>>> + goto fail_release; >>>> + } >>> >>> Hello Jiada, >>> >>> The new regulator properties should be documented in the device-tree >>> binding. >>> >> I will document new regulator properties in a separate commit in >> v10 patch-set > > Please make sure that all patches are added in a correct order and that > they at least compile. This patch doesn't compile: > Sure, I will take care of this issue in next version Thanks, Jiada > CC drivers/input/touchscreen/atmel_mxt_ts.o > drivers/input/touchscreen/atmel_mxt_ts.c:34:10: fatal error: > dt-bindings/input/atmel_mxt_ts.h: No such file or directory > 34 | #include <dt-bindings/input/atmel_mxt_ts.h> >
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c index 9aafed92db9c..ef8baf64659e 100644 --- a/drivers/input/touchscreen/atmel_mxt_ts.c +++ b/drivers/input/touchscreen/atmel_mxt_ts.c @@ -26,10 +26,12 @@ #include <linux/slab.h> #include <linux/gpio/consumer.h> #include <asm/unaligned.h> +#include <linux/regulator/consumer.h> #include <media/v4l2-device.h> #include <media/v4l2-ioctl.h> #include <media/videobuf2-v4l2.h> #include <media/videobuf2-vmalloc.h> +#include <dt-bindings/input/atmel_mxt_ts.h> /* Firmware files */ #define MXT_FW_NAME "maxtouch.fw" @@ -215,6 +217,9 @@ enum t100_type { #define MXT_CRC_TIMEOUT 1000 /* msec */ #define MXT_FW_RESET_TIME 3000 /* msec */ #define MXT_FW_CHG_TIMEOUT 300 /* msec */ +#define MXT_REGULATOR_DELAY 150 /* msec */ +#define MXT_CHG_DELAY 100 /* msec */ +#define MXT_POWERON_DELAY 150 /* msec */ /* Command to unlock bootloader */ #define MXT_UNLOCK_CMD_MSB 0xaa @@ -275,11 +280,6 @@ enum v4l_dbg_inputs { MXT_V4L_INPUT_MAX, }; -enum mxt_suspend_mode { - MXT_SUSPEND_DEEP_SLEEP = 0, - MXT_SUSPEND_T9_CTRL = 1, -}; - /* Config update context */ struct mxt_cfg { u8 *raw; @@ -333,6 +333,8 @@ struct mxt_data { u8 stylus_aux_pressure; u8 stylus_aux_peak; bool use_retrigen_workaround; + struct regulator *reg_vdd; + struct regulator *reg_avdd; /* Cached parameters from object table */ u16 T5_address; @@ -2073,6 +2075,94 @@ static int mxt_read_info_block(struct mxt_data *data) return error; } +static void mxt_regulator_enable(struct mxt_data *data) +{ + int error; + + if (!data->reg_vdd || !data->reg_avdd) + return; + + gpiod_set_value(data->reset_gpio, 0); + + error = regulator_enable(data->reg_vdd); + if (error) + return; + + error = regulator_enable(data->reg_avdd); + if (error) { + regulator_disable(data->reg_vdd); + return; + } + + /* + * According to maXTouch power sequencing specification, RESET line + * must be kept low until some time after regulators come up to + * voltage + */ + msleep(MXT_REGULATOR_DELAY); + gpiod_set_value(data->reset_gpio, 1); + msleep(MXT_CHG_DELAY); + +retry_wait: + reinit_completion(&data->bl_completion); + data->in_bootloader = true; + error = mxt_wait_for_completion(data, &data->bl_completion, + MXT_POWERON_DELAY); + if (error == -EINTR) + goto retry_wait; + + data->in_bootloader = false; +} + +static void mxt_regulator_disable(struct mxt_data *data) +{ + if (!data->reg_vdd || !data->reg_avdd) + return; + + if (regulator_is_enabled(data->reg_vdd)) + regulator_disable(data->reg_vdd); + if (regulator_is_enabled(data->reg_avdd)) + regulator_disable(data->reg_avdd); +} + +static int mxt_probe_regulators(struct mxt_data *data) +{ + struct device *dev = &data->client->dev; + int error; + + /* Must have reset GPIO to use regulator support */ + if (!data->reset_gpio) { + error = -EINVAL; + goto fail; + } + + data->reg_vdd = devm_regulator_get(dev, "vdd"); + if (IS_ERR(data->reg_vdd)) { + error = PTR_ERR(data->reg_vdd); + dev_err(dev, "Error %d getting vdd regulator\n", error); + goto fail; + } + + data->reg_avdd = devm_regulator_get(dev, "avdd"); + if (IS_ERR(data->reg_avdd)) { + error = PTR_ERR(data->reg_avdd); + dev_err(dev, "Error %d getting avdd regulator\n", error); + goto fail_release; + } + + mxt_regulator_enable(data); + + dev_dbg(dev, "Initialised regulators\n"); + return 0; + +fail_release: + regulator_put(data->reg_vdd); +fail: + data->reg_vdd = NULL; + data->reg_avdd = NULL; + return error; +} + static int mxt_read_t9_resolution(struct mxt_data *data) { struct i2c_client *client = data->client; @@ -3139,7 +3229,12 @@ static int mxt_load_fw(struct device *dev, const char *fn) goto release_firmware; if (data->suspended) { - enable_irq(data->irq); + if (data->suspend_mode == MXT_SUSPEND_REGULATOR) + mxt_regulator_enable(data); + + if (data->suspend_mode == MXT_SUSPEND_DEEP_SLEEP) + enable_irq(data->irq); + data->suspended = false; } @@ -3345,6 +3440,11 @@ static void mxt_start(struct mxt_data *data) MXT_TOUCH_MULTI_T9, MXT_T9_CTRL, 0x83); break; + case MXT_SUSPEND_REGULATOR: + enable_irq(data->irq); + mxt_regulator_enable(data); + break; + case MXT_SUSPEND_DEEP_SLEEP: default: /* @@ -3377,6 +3477,12 @@ static void mxt_stop(struct mxt_data *data) MXT_TOUCH_MULTI_T9, MXT_T9_CTRL, 0); break; + case MXT_SUSPEND_REGULATOR: + disable_irq(data->irq); + mxt_regulator_disable(data); + mxt_reset_slots(data); + break; + case MXT_SUSPEND_DEEP_SLEEP: default: disable_irq(data->irq); @@ -3469,6 +3575,8 @@ static int mxt_parse_device_properties(struct mxt_data *data) data->t15_num_keys = n_keys; } + device_property_read_u32(dev, "atmel,suspend-mode", + &data->suspend_mode); return 0; } @@ -3555,14 +3663,18 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id) return error; } - disable_irq(client->irq); - - if (data->reset_gpio) { + if (data->suspend_mode == MXT_SUSPEND_REGULATOR) { + error = mxt_probe_regulators(data); + if (error) + return error; + } else if (data->reset_gpio) { msleep(MXT_RESET_GPIO_TIME); gpiod_set_value(data->reset_gpio, 1); msleep(MXT_RESET_INVALID_CHG); } + disable_irq(data->irq); + error = mxt_initialize(data); if (error) return error;