Message ID | 20191127120948.22251-1-m.felsch@pengutronix.de |
---|---|
Headers | show |
Series | EDT-FT5x06 improvements | expand |
On Wed, Nov 27, 2019 at 01:09:48PM +0100, Marco Felsch wrote: > It is possible to bring the device into a deep sleep state. To exit this > state the reset or wakeup pin must be toggeled as documented in [1]. > Because of the poor documentation I used the several downstream kernels > [2] and other applications notes [3] to indentify the related registers. > > Furthermore I added the support to disable the device completely. This is > the most effective power-saving mechanism. Disabling the device don't > change the suspend logic because the hibernate mode needs a hardware > reset anyway. > > [1] https://www.newhavendisplay.com/appnotes/datasheets/touchpanel/FT5x26.pdf > [2] https://github.com/linux-sunxi/linux-sunxi/blob/sunxi-3.4/drivers/input/touchscreen/ft5x_ts.c > https://github.com/Pablito2020/focaltech-touch-driver/blob/master/ft5336_driver.c > [3] https://www.newhavendisplay.com/appnotes/datasheets/touchpanel/FT5x16_registers.pdf > + /* Recover from hibernate mode if hardware supports it */ > + if (tsdata->wake_gpio) { > + gpiod_set_value_cansleep(tsdata->wake_gpio, 0); > + usleep_range(5000, 6000); > + gpiod_set_value_cansleep(tsdata->wake_gpio, 1); > + msleep(300); > + } else if (tsdata->reset_gpio) { > + gpiod_set_value_cansleep(tsdata->reset_gpio, 1); > + usleep_range(5000, 6000); > + gpiod_set_value_cansleep(tsdata->reset_gpio, 0); > + msleep(300); > + } Perhaps static void edt_ft5x06_ts_toggle_gpio(struct gpio_desc *gpiod) { ... } ...resume(...) { ... if (wake_gpio) ...toggle_gpio(wake_gpio); else if (reset_gpio) ...toggle_gpio(reset_gpio); ... } ?
Hi Andy, On 19-11-27 14:59, Andy Shevchenko wrote: > On Wed, Nov 27, 2019 at 01:09:48PM +0100, Marco Felsch wrote: > > It is possible to bring the device into a deep sleep state. To exit this > > state the reset or wakeup pin must be toggeled as documented in [1]. > > Because of the poor documentation I used the several downstream kernels > > [2] and other applications notes [3] to indentify the related registers. > > > > Furthermore I added the support to disable the device completely. This is > > the most effective power-saving mechanism. Disabling the device don't > > change the suspend logic because the hibernate mode needs a hardware > > reset anyway. > > > > [1] https://www.newhavendisplay.com/appnotes/datasheets/touchpanel/FT5x26.pdf > > [2] https://github.com/linux-sunxi/linux-sunxi/blob/sunxi-3.4/drivers/input/touchscreen/ft5x_ts.c > > https://github.com/Pablito2020/focaltech-touch-driver/blob/master/ft5336_driver.c > > [3] https://www.newhavendisplay.com/appnotes/datasheets/touchpanel/FT5x16_registers.pdf > > > > + /* Recover from hibernate mode if hardware supports it */ > > + if (tsdata->wake_gpio) { > > + gpiod_set_value_cansleep(tsdata->wake_gpio, 0); > > + usleep_range(5000, 6000); > > + gpiod_set_value_cansleep(tsdata->wake_gpio, 1); > > + msleep(300); > > + } else if (tsdata->reset_gpio) { > > + gpiod_set_value_cansleep(tsdata->reset_gpio, 1); > > + usleep_range(5000, 6000); > > + gpiod_set_value_cansleep(tsdata->reset_gpio, 0); > > + msleep(300); > > + } > > Perhaps > > static void edt_ft5x06_ts_toggle_gpio(struct gpio_desc *gpiod) > { > ... > } > > ...resume(...) > { > ... > if (wake_gpio) > ...toggle_gpio(wake_gpio); > else if (reset_gpio) > ...toggle_gpio(reset_gpio); > ... > } > > ? Thanks fpr your suggestion but we need to differentiate between reset and wake logic level. The wake-gpio keeps asserted while the reset is released. So the edt_ft5x06_ts_toggle_gpio() needs at least a 'is_reset' parameter but then the simplification is gone. Regards, Marco > -- > With Best Regards, > Andy Shevchenko > > >
On Wed, Nov 27, 2019 at 02:06:02PM +0100, Marco Felsch wrote: > On 19-11-27 14:59, Andy Shevchenko wrote: > > On Wed, Nov 27, 2019 at 01:09:48PM +0100, Marco Felsch wrote: > > Perhaps > > > > static void edt_ft5x06_ts_toggle_gpio(struct gpio_desc *gpiod) > > { > > ... > > } > > > > ...resume(...) > > { > > ... > > if (wake_gpio) > > ...toggle_gpio(wake_gpio); > > else if (reset_gpio) > > ...toggle_gpio(reset_gpio); > > ... > > } > > > > ? > > Thanks fpr your suggestion but we need to differentiate between reset > and wake logic level. The wake-gpio keeps asserted while the reset is > released. So the edt_ft5x06_ts_toggle_gpio() needs at least a 'is_reset' > parameter but then the simplification is gone. How about this: static void edt_ft5x06_ts_toggle_gpio(struct gpio_desc *gpiod, int value) { gpiod_...(..., !value); ... gpiod_...(..., value); ... } ...resume(...) { ... if (wake_gpio) ...toggle_gpio(wake_gpio, 1); else if (reset_gpio) ...toggle_gpio(reset_gpio, 0); ... } ?
On Wed, Nov 27, 2019 at 01:09:43PM +0100, Marco Felsch wrote: > Hi, > > this v2 address all comments made on [1]. See the patch based changelog > for further information. > > During the discussion we all agreed to use the common wakeup-src device > property but I added all users of this driver again. So they can provide > their ack-tag. It doesn't break my case (Adafruit 2.8" TFT + cap/ts). Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Regards, > Marco > > [1] https://patchwork.kernel.org/cover/11149039/ > > Marco Felsch (4): > Input: edt-ft5x06 - alphabetical include reorder > dt-bindings: Input: edt-ft5x06 - document wakeup-source capability > Input: edt-ft5x06 - make wakeup-source switchable > Input: edt-ft5x06 - improve power management operations > > Philipp Zabel (1): > Input: edt-ft5x06: work around first register access error > > .../bindings/input/touchscreen/edt-ft5x06.txt | 2 + > drivers/input/touchscreen/edt-ft5x06.c | 85 ++++++++++++++++--- > 2 files changed, 73 insertions(+), 14 deletions(-) > > -- > 2.20.1 >
On Wed, Nov 27, 2019 at 01:09:43PM +0100, Marco Felsch wrote: > Hi, > > this v2 address all comments made on [1]. See the patch based changelog > for further information. > > During the discussion we all agreed to use the common wakeup-src device > property but I added all users of this driver again. So they can provide > their ack-tag. > Independently on the decision on patch 5, FWIW, Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Regards, > Marco > > [1] https://patchwork.kernel.org/cover/11149039/ > > Marco Felsch (4): > Input: edt-ft5x06 - alphabetical include reorder > dt-bindings: Input: edt-ft5x06 - document wakeup-source capability > Input: edt-ft5x06 - make wakeup-source switchable > Input: edt-ft5x06 - improve power management operations > > Philipp Zabel (1): > Input: edt-ft5x06: work around first register access error > > .../bindings/input/touchscreen/edt-ft5x06.txt | 2 + > drivers/input/touchscreen/edt-ft5x06.c | 85 ++++++++++++++++--- > 2 files changed, 73 insertions(+), 14 deletions(-) > > -- > 2.20.1 >
Hi, On 19-11-27 16:32, Andy Shevchenko wrote: > On Wed, Nov 27, 2019 at 02:06:02PM +0100, Marco Felsch wrote: > > On 19-11-27 14:59, Andy Shevchenko wrote: > > > On Wed, Nov 27, 2019 at 01:09:48PM +0100, Marco Felsch wrote: > > > > Perhaps > > > > > > static void edt_ft5x06_ts_toggle_gpio(struct gpio_desc *gpiod) > > > { > > > ... > > > } > > > > > > ...resume(...) > > > { > > > ... > > > if (wake_gpio) > > > ...toggle_gpio(wake_gpio); > > > else if (reset_gpio) > > > ...toggle_gpio(reset_gpio); > > > ... > > > } > > > > > > ? > > > > Thanks fpr your suggestion but we need to differentiate between reset > > and wake logic level. The wake-gpio keeps asserted while the reset is > > released. So the edt_ft5x06_ts_toggle_gpio() needs at least a 'is_reset' > > parameter but then the simplification is gone. > > > How about this: > static void edt_ft5x06_ts_toggle_gpio(struct gpio_desc *gpiod, int value) > { > gpiod_...(..., !value); > ... > gpiod_...(..., value); > ... > } > > ...resume(...) > { > ... > if (wake_gpio) > ...toggle_gpio(wake_gpio, 1); > else if (reset_gpio) > ...toggle_gpio(reset_gpio, 0); > ... > } > > ? That's possible.. Don't see the improvement yet, but I can prepare a folllow-up patch if Dmitry wants. Regards, Marco > -- > With Best Regards, > Andy Shevchenko > > >
Hi Andy, On 19-11-27 19:14, Andy Shevchenko wrote: > On Wed, Nov 27, 2019 at 01:09:43PM +0100, Marco Felsch wrote: > > Hi, > > > > this v2 address all comments made on [1]. See the patch based changelog > > for further information. > > > > During the discussion we all agreed to use the common wakeup-src device > > property but I added all users of this driver again. So they can provide > > their ack-tag. > > > > Independently on the decision on patch 5, FWIW, > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Thanks for testing and the review =) Regards, Marco > > Regards, > > Marco > > > > [1] https://patchwork.kernel.org/cover/11149039/ > > > > Marco Felsch (4): > > Input: edt-ft5x06 - alphabetical include reorder > > dt-bindings: Input: edt-ft5x06 - document wakeup-source capability > > Input: edt-ft5x06 - make wakeup-source switchable > > Input: edt-ft5x06 - improve power management operations > > > > Philipp Zabel (1): > > Input: edt-ft5x06: work around first register access error > > > > .../bindings/input/touchscreen/edt-ft5x06.txt | 2 + > > drivers/input/touchscreen/edt-ft5x06.c | 85 ++++++++++++++++--- > > 2 files changed, 73 insertions(+), 14 deletions(-) > > > > -- > > 2.20.1 > > > > -- > With Best Regards, > Andy Shevchenko > > >
On Wed, Nov 27, 2019 at 06:25:22PM +0100, Marco Felsch wrote: > On 19-11-27 16:32, Andy Shevchenko wrote: > > On Wed, Nov 27, 2019 at 02:06:02PM +0100, Marco Felsch wrote: > > > On 19-11-27 14:59, Andy Shevchenko wrote: > > > > On Wed, Nov 27, 2019 at 01:09:48PM +0100, Marco Felsch wrote: > > static void edt_ft5x06_ts_toggle_gpio(struct gpio_desc *gpiod, int value) > > { > > gpiod_...(..., !value); > > ... > > gpiod_...(..., value); > > ... > > } > > > > ...resume(...) > > { > > ... > > if (wake_gpio) > > ...toggle_gpio(wake_gpio, 1); > > else if (reset_gpio) > > ...toggle_gpio(reset_gpio, 0); > > ... > > } > > > > ? > > That's possible.. Don't see the improvement yet, but I can prepare a > folllow-up patch if Dmitry wants. The improvement is to get rid of duplication of flow how you toggle the GPIO. But yes, up to Dmitry.
On Wed, Nov 27, 2019 at 01:09:44PM +0100, Marco Felsch wrote: > From: Philipp Zabel <p.zabel@pengutronix.de> > > The EP0700MLP1 returns bogus data on the first register read access > (reading the threshold parameter from register 0x00): > > edt_ft5x06 2-0038: crc error: 0xfc expected, got 0x40 > > It ignores writes until then. This patch adds a dummy read after which > the number of sensors and parameter read/writes work correctly. > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> Need your signed-off-by as well. > --- > drivers/input/touchscreen/edt-ft5x06.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c > index d61731c0037d..b87b1e074f62 100644 > --- a/drivers/input/touchscreen/edt-ft5x06.c > +++ b/drivers/input/touchscreen/edt-ft5x06.c > @@ -1050,6 +1050,7 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client, > { > const struct edt_i2c_chip_data *chip_data; > struct edt_ft5x06_ts_data *tsdata; > + u8 buf[2] = { 0xfc, 0x00 }; > struct input_dev *input; > unsigned long irq_flags; > int error; > @@ -1140,6 +1141,12 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client, > return error; > } > > + /* > + * Dummy read access. EP0700MLP1 returns bogus data on the first > + * register read access and ignores writes. > + */ > + edt_ft5x06_ts_readwrite(tsdata->client, 2, buf, 2, buf); > + > edt_ft5x06_ts_set_regs(tsdata); > edt_ft5x06_ts_get_defaults(&client->dev, tsdata); > edt_ft5x06_ts_get_parameters(tsdata); > -- > 2.20.1 >
On Wed, Nov 27, 2019 at 01:09:47PM +0100, Marco Felsch wrote: > Since day one the touch controller acts as wakeup-source. This seems to > be wrong since the device supports deep-sleep mechanism [1] which > requires a reset to leave it. Also some designs won't use the > touchscreen as wakeup-source. > > According discussion [2] we decided to break backward compatibility and > go the common way by using the 'wakeup-source' device-property. > > [1] https://www.newhavendisplay.com/appnotes/datasheets/touchpanel/FT5x26.pdf > [2] https://patchwork.kernel.org/patch/11149037/ > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > --- > v2: > - make use of common wakeup-source property > - adapt commit message > > drivers/input/touchscreen/edt-ft5x06.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c > index e1b31fd525e2..8d2ec7947f0e 100644 > --- a/drivers/input/touchscreen/edt-ft5x06.c > +++ b/drivers/input/touchscreen/edt-ft5x06.c > @@ -24,6 +24,7 @@ > #include <linux/irq.h> > #include <linux/kernel.h> > #include <linux/module.h> > +#include <linux/property.h> > #include <linux/ratelimit.h> > #include <linux/regulator/consumer.h> > #include <linux/slab.h> > @@ -1056,6 +1057,7 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client, > unsigned long irq_flags; > int error; > char fw_version[EDT_NAME_LEN]; > + bool en_wakeup; > > dev_dbg(&client->dev, "probing for EDT FT5x06 I2C\n"); > > @@ -1114,6 +1116,8 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client, > return error; > } > > + en_wakeup = device_property_present(&client->dev, "wakeup-source"); > + > if (tsdata->wake_gpio) { > usleep_range(5000, 6000); > gpiod_set_value_cansleep(tsdata->wake_gpio, 1); > @@ -1208,7 +1212,7 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client, > return error; > > edt_ft5x06_ts_prepare_debugfs(tsdata, dev_driver_string(&client->dev)); > - device_init_wakeup(&client->dev, 1); > + device_init_wakeup(&client->dev, en_wakeup); I2C core already marks device as wakeup source if I2C_CLIEMT_WAKE is set (and the flag is specified when, among other things, device has "wakeup-source" property). So the only thing that is needed is to remove device_init_wakeup(&client->dev, 1); line. Thanks.
On Wed, Nov 27, 2019 at 01:09:48PM +0100, Marco Felsch wrote: > It is possible to bring the device into a deep sleep state. To exit this > state the reset or wakeup pin must be toggeled as documented in [1]. > Because of the poor documentation I used the several downstream kernels > [2] and other applications notes [3] to indentify the related registers. > > Furthermore I added the support to disable the device completely. This is > the most effective power-saving mechanism. Disabling the device don't > change the suspend logic because the hibernate mode needs a hardware > reset anyway. > > [1] https://www.newhavendisplay.com/appnotes/datasheets/touchpanel/FT5x26.pdf > [2] https://github.com/linux-sunxi/linux-sunxi/blob/sunxi-3.4/drivers/input/touchscreen/ft5x_ts.c > https://github.com/Pablito2020/focaltech-touch-driver/blob/master/ft5336_driver.c > [3] https://www.newhavendisplay.com/appnotes/datasheets/touchpanel/FT5x16_registers.pdf > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > --- > v2: > - adapt commit message > - don't return errors during suspend/resume > - replace dev_err() by dev_warn() > - add support to disable the regulator too > > drivers/input/touchscreen/edt-ft5x06.c | 49 ++++++++++++++++++++++++-- > 1 file changed, 47 insertions(+), 2 deletions(-) > > diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c > index 8d2ec7947f0e..0bdd3440f684 100644 > --- a/drivers/input/touchscreen/edt-ft5x06.c > +++ b/drivers/input/touchscreen/edt-ft5x06.c > @@ -39,6 +39,9 @@ > #define WORK_REGISTER_NUM_X 0x33 > #define WORK_REGISTER_NUM_Y 0x34 > > +#define PMOD_REGISTER_ACTIVE 0x00 > +#define PMOD_REGISTER_HIBERNATE 0x03 > + > #define M09_REGISTER_THRESHOLD 0x80 > #define M09_REGISTER_GAIN 0x92 > #define M09_REGISTER_OFFSET 0x93 > @@ -54,6 +57,7 @@ > > #define WORK_REGISTER_OPMODE 0x3c > #define FACTORY_REGISTER_OPMODE 0x01 > +#define PMOD_REGISTER_OPMODE 0xa5 > > #define TOUCH_EVENT_DOWN 0x00 > #define TOUCH_EVENT_UP 0x01 > @@ -1235,9 +1239,29 @@ static int edt_ft5x06_ts_remove(struct i2c_client *client) > static int __maybe_unused edt_ft5x06_ts_suspend(struct device *dev) > { > struct i2c_client *client = to_i2c_client(dev); > + struct edt_ft5x06_ts_data *tsdata = i2c_get_clientdata(client); > + int ret; > > - if (device_may_wakeup(dev)) > + if (device_may_wakeup(dev)) { > enable_irq_wake(client->irq); Can we start with preliminary patch dropping calls to enable_irq_wake() and disable_irq_wake() as device/PM core will tale care of configuring wake irqs properly for us, since we are now allowing I2C core to mark the interrupt as wake IRQ. So we need to do: if (device_may_wakeup(dev)) return 0; <execute power down sequence> Thanks.
On 19-12-02 09:58, Dmitry Torokhov wrote: > On Wed, Nov 27, 2019 at 01:09:44PM +0100, Marco Felsch wrote: > > From: Philipp Zabel <p.zabel@pengutronix.de> > > > > The EP0700MLP1 returns bogus data on the first register read access > > (reading the threshold parameter from register 0x00): > > > > edt_ft5x06 2-0038: crc error: 0xfc expected, got 0x40 > > > > It ignores writes until then. This patch adds a dummy read after which > > the number of sensors and parameter read/writes work correctly. > > > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> > > Need your signed-off-by as well. Damn, you're right.. I fix this in the next version. > > > --- > > drivers/input/touchscreen/edt-ft5x06.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c > > index d61731c0037d..b87b1e074f62 100644 > > --- a/drivers/input/touchscreen/edt-ft5x06.c > > +++ b/drivers/input/touchscreen/edt-ft5x06.c > > @@ -1050,6 +1050,7 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client, > > { > > const struct edt_i2c_chip_data *chip_data; > > struct edt_ft5x06_ts_data *tsdata; > > + u8 buf[2] = { 0xfc, 0x00 }; > > struct input_dev *input; > > unsigned long irq_flags; > > int error; > > @@ -1140,6 +1141,12 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client, > > return error; > > } > > > > + /* > > + * Dummy read access. EP0700MLP1 returns bogus data on the first > > + * register read access and ignores writes. > > + */ > > + edt_ft5x06_ts_readwrite(tsdata->client, 2, buf, 2, buf); > > + > > edt_ft5x06_ts_set_regs(tsdata); > > edt_ft5x06_ts_get_defaults(&client->dev, tsdata); > > edt_ft5x06_ts_get_parameters(tsdata); > > -- > > 2.20.1 > > > > -- > Dmitry >
On 19-12-02 10:00, Dmitry Torokhov wrote: > On Wed, Nov 27, 2019 at 01:09:47PM +0100, Marco Felsch wrote: > > Since day one the touch controller acts as wakeup-source. This seems to > > be wrong since the device supports deep-sleep mechanism [1] which > > requires a reset to leave it. Also some designs won't use the > > touchscreen as wakeup-source. > > > > According discussion [2] we decided to break backward compatibility and > > go the common way by using the 'wakeup-source' device-property. > > > > [1] https://www.newhavendisplay.com/appnotes/datasheets/touchpanel/FT5x26.pdf > > [2] https://patchwork.kernel.org/patch/11149037/ > > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > --- > > v2: > > - make use of common wakeup-source property > > - adapt commit message > > > > drivers/input/touchscreen/edt-ft5x06.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c > > index e1b31fd525e2..8d2ec7947f0e 100644 > > --- a/drivers/input/touchscreen/edt-ft5x06.c > > +++ b/drivers/input/touchscreen/edt-ft5x06.c > > @@ -24,6 +24,7 @@ > > #include <linux/irq.h> > > #include <linux/kernel.h> > > #include <linux/module.h> > > +#include <linux/property.h> > > #include <linux/ratelimit.h> > > #include <linux/regulator/consumer.h> > > #include <linux/slab.h> > > @@ -1056,6 +1057,7 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client, > > unsigned long irq_flags; > > int error; > > char fw_version[EDT_NAME_LEN]; > > + bool en_wakeup; > > > > dev_dbg(&client->dev, "probing for EDT FT5x06 I2C\n"); > > > > @@ -1114,6 +1116,8 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client, > > return error; > > } > > > > + en_wakeup = device_property_present(&client->dev, "wakeup-source"); > > + > > if (tsdata->wake_gpio) { > > usleep_range(5000, 6000); > > gpiod_set_value_cansleep(tsdata->wake_gpio, 1); > > @@ -1208,7 +1212,7 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client, > > return error; > > > > edt_ft5x06_ts_prepare_debugfs(tsdata, dev_driver_string(&client->dev)); > > - device_init_wakeup(&client->dev, 1); > > + device_init_wakeup(&client->dev, en_wakeup); > > I2C core already marks device as wakeup source if I2C_CLIEMT_WAKE is set > (and the flag is specified when, among other things, device has > "wakeup-source" property). > > So the only thing that is needed is to remove > > device_init_wakeup(&client->dev, 1); > > line. You are right, thanks for covering that. Regards, Marco > > Thanks. > > -- > Dmitry >
On 19-12-02 10:04, Dmitry Torokhov wrote: > On Wed, Nov 27, 2019 at 01:09:48PM +0100, Marco Felsch wrote: > > It is possible to bring the device into a deep sleep state. To exit this > > state the reset or wakeup pin must be toggeled as documented in [1]. > > Because of the poor documentation I used the several downstream kernels > > [2] and other applications notes [3] to indentify the related registers. > > > > Furthermore I added the support to disable the device completely. This is > > the most effective power-saving mechanism. Disabling the device don't > > change the suspend logic because the hibernate mode needs a hardware > > reset anyway. > > > > [1] https://www.newhavendisplay.com/appnotes/datasheets/touchpanel/FT5x26.pdf > > [2] https://github.com/linux-sunxi/linux-sunxi/blob/sunxi-3.4/drivers/input/touchscreen/ft5x_ts.c > > https://github.com/Pablito2020/focaltech-touch-driver/blob/master/ft5336_driver.c > > [3] https://www.newhavendisplay.com/appnotes/datasheets/touchpanel/FT5x16_registers.pdf > > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > --- > > v2: > > - adapt commit message > > - don't return errors during suspend/resume > > - replace dev_err() by dev_warn() > > - add support to disable the regulator too > > > > drivers/input/touchscreen/edt-ft5x06.c | 49 ++++++++++++++++++++++++-- > > 1 file changed, 47 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c > > index 8d2ec7947f0e..0bdd3440f684 100644 > > --- a/drivers/input/touchscreen/edt-ft5x06.c > > +++ b/drivers/input/touchscreen/edt-ft5x06.c > > @@ -39,6 +39,9 @@ > > #define WORK_REGISTER_NUM_X 0x33 > > #define WORK_REGISTER_NUM_Y 0x34 > > > > +#define PMOD_REGISTER_ACTIVE 0x00 > > +#define PMOD_REGISTER_HIBERNATE 0x03 > > + > > #define M09_REGISTER_THRESHOLD 0x80 > > #define M09_REGISTER_GAIN 0x92 > > #define M09_REGISTER_OFFSET 0x93 > > @@ -54,6 +57,7 @@ > > > > #define WORK_REGISTER_OPMODE 0x3c > > #define FACTORY_REGISTER_OPMODE 0x01 > > +#define PMOD_REGISTER_OPMODE 0xa5 > > > > #define TOUCH_EVENT_DOWN 0x00 > > #define TOUCH_EVENT_UP 0x01 > > @@ -1235,9 +1239,29 @@ static int edt_ft5x06_ts_remove(struct i2c_client *client) > > static int __maybe_unused edt_ft5x06_ts_suspend(struct device *dev) > > { > > struct i2c_client *client = to_i2c_client(dev); > > + struct edt_ft5x06_ts_data *tsdata = i2c_get_clientdata(client); > > + int ret; > > > > - if (device_may_wakeup(dev)) > > + if (device_may_wakeup(dev)) { > > enable_irq_wake(client->irq); > > Can we start with preliminary patch dropping calls to enable_irq_wake() > and disable_irq_wake() as device/PM core will tale care of configuring > wake irqs properly for us, since we are now allowing I2C core to mark > the interrupt as wake IRQ. > > So we need to do: > > if (device_may_wakeup(dev)) > return 0; > > <execute power down sequence> > > Thanks. Of course, thanks for covering that.A Regards, Marco > > -- > Dmitry >