diff mbox series

[3/6] Input: edt-ft5x06 - add support to disable the wakeup-source

Message ID 20190917155808.27818-4-m.felsch@pengutronix.de (mailing list archive)
State Superseded
Headers show
Series EDT-FT5x06 improvements | expand

Commit Message

Marco Felsch Sept. 17, 2019, 3:58 p.m. UTC
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.

Add a firmware property to address this. The common 'wakeup-source'
property can't be used for that because the driver must be backward
compatible with old firmwares which may assume the default on
wakeup-source behaviour. So we need to go the other way by explicit
disable the wakeup-source capability.

[1] https://www.newhavendisplay.com/appnotes/datasheets/touchpanel/ \
    FT5x26.pdf

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/input/touchscreen/edt-ft5x06.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Andy Shevchenko Sept. 17, 2019, 4:32 p.m. UTC | #1
On Tue, Sep 17, 2019 at 05:58:05PM +0200, 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.
> 
> Add a firmware property to address this. The common 'wakeup-source'
> property can't be used for that because the driver must be backward
> compatible with old firmwares which may assume the default on
> wakeup-source behaviour. So we need to go the other way by explicit
> disable the wakeup-source capability.
> 

> [1] https://www.newhavendisplay.com/appnotes/datasheets/touchpanel/ \
>     FT5x26.pdf

Please, don't split URLs

>  	int error;
>  	char fw_version[EDT_NAME_LEN];

> +	bool en_wakeup;

Perhaps wakeup_en?
Marco Felsch Sept. 17, 2019, 4:46 p.m. UTC | #2
On 19-09-17 19:32, Andy Shevchenko wrote:
> On Tue, Sep 17, 2019 at 05:58:05PM +0200, 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.
> > 
> > Add a firmware property to address this. The common 'wakeup-source'
> > property can't be used for that because the driver must be backward
> > compatible with old firmwares which may assume the default on
> > wakeup-source behaviour. So we need to go the other way by explicit
> > disable the wakeup-source capability.
> > 
> 
> > [1] https://www.newhavendisplay.com/appnotes/datasheets/touchpanel/ \
> >     FT5x26.pdf
> 
> Please, don't split URLs

Hm.. then checkpatch complains.. If you prefer it, I can change it in
the v2.

> >  	int error;
> >  	char fw_version[EDT_NAME_LEN];
> 
> > +	bool en_wakeup;
> 
> Perhaps wakeup_en?

I have no strong opinion about that..

Regards,
  Marco


> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 
>
Dmitry Torokhov Sept. 17, 2019, 5:02 p.m. UTC | #3
On Tue, Sep 17, 2019 at 06:46:39PM +0200, Marco Felsch wrote:
> On 19-09-17 19:32, Andy Shevchenko wrote:
> > On Tue, Sep 17, 2019 at 05:58:05PM +0200, 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.
> > > 
> > > Add a firmware property to address this. The common 'wakeup-source'
> > > property can't be used for that because the driver must be backward
> > > compatible with old firmwares which may assume the default on
> > > wakeup-source behaviour. So we need to go the other way by explicit
> > > disable the wakeup-source capability.
> > > 
> > 
> > > [1] https://www.newhavendisplay.com/appnotes/datasheets/touchpanel/ \
> > >     FT5x26.pdf
> > 
> > Please, don't split URLs
> 
> Hm.. then checkpatch complains.. If you prefer it, I can change it in
> the v2.

Checkpatch complains about valid things and it complains about insane
things. In this case simply ignore it.

Thanks.
diff mbox series

Patch

diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index 9e328a82b765..aafb6f4e34d7 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -25,6 +25,7 @@ 
 #include <linux/irq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/property.h>
 #include <linux/ratelimit.h>
 #include <linux/slab.h>
 #include <linux/uaccess.h>
@@ -1047,6 +1048,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");
 
@@ -1084,6 +1086,13 @@  static int edt_ft5x06_ts_probe(struct i2c_client *client,
 		return error;
 	}
 
+	/*
+	 * We need to go this way to keep backward compatibility with old DT's
+	 * which may assume the default-on mechanism.
+	 */
+	en_wakeup = !device_property_present(&client->dev,
+					     "edt,disable-wakeup-source");
+
 	if (tsdata->wake_gpio) {
 		usleep_range(5000, 6000);
 		gpiod_set_value_cansleep(tsdata->wake_gpio, 1);
@@ -1172,7 +1181,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);
 
 	dev_dbg(&client->dev,
 		"EDT FT5x06 initialized: IRQ %d, WAKE pin %d, Reset pin %d.\n",