* [PATCH v3 1/6] Input: edt-ft5x06: work around first register access error
2020-01-08 11:10 [PATCH v3 0/6] EDT-FT5x06 improvements Marco Felsch
@ 2020-01-08 11:10 ` Marco Felsch
2020-01-10 1:06 ` Dmitry Torokhov
2020-01-08 11:10 ` [PATCH v3 2/6] Input: edt-ft5x06 - alphabetical include reorder Marco Felsch
` (4 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Marco Felsch @ 2020-01-08 11:10 UTC (permalink / raw)
To: robh+dt, andriy.shevchenko, dmitry.torokhov, bparrot,
andy.shevchenko, simon.budig, hdegoede, fcooper, mripard,
alexandre.belloni, shawnguo
Cc: devicetree, kernel, linux-input, Philipp Zabel
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>
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v3 1/6] Input: edt-ft5x06: work around first register access error
2020-01-08 11:10 ` [PATCH v3 1/6] Input: edt-ft5x06: work around first register access error Marco Felsch
@ 2020-01-10 1:06 ` Dmitry Torokhov
0 siblings, 0 replies; 17+ messages in thread
From: Dmitry Torokhov @ 2020-01-10 1:06 UTC (permalink / raw)
To: Marco Felsch
Cc: robh+dt, andriy.shevchenko, bparrot, andy.shevchenko,
simon.budig, hdegoede, fcooper, mripard, alexandre.belloni,
shawnguo, devicetree, kernel, linux-input, Philipp Zabel
On Wed, Jan 08, 2020 at 12:10:45PM +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>
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Applied, thank you.
> ---
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 2/6] Input: edt-ft5x06 - alphabetical include reorder
2020-01-08 11:10 [PATCH v3 0/6] EDT-FT5x06 improvements Marco Felsch
2020-01-08 11:10 ` [PATCH v3 1/6] Input: edt-ft5x06: work around first register access error Marco Felsch
@ 2020-01-08 11:10 ` Marco Felsch
2020-01-10 1:06 ` Dmitry Torokhov
2020-01-08 11:10 ` [PATCH v3 3/6] dt-bindings: Input: edt-ft5x06 - document wakeup-source capability Marco Felsch
` (3 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Marco Felsch @ 2020-01-08 11:10 UTC (permalink / raw)
To: robh+dt, andriy.shevchenko, dmitry.torokhov, bparrot,
andy.shevchenko, simon.budig, hdegoede, fcooper, mripard,
alexandre.belloni, shawnguo
Cc: devicetree, kernel, linux-input
It seems that the include order is historical increased and no one takes
care of it. Fix this to align it with the common rule to be in a
alphabetical order.
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/input/touchscreen/edt-ft5x06.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index b87b1e074f62..e1b31fd525e2 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -13,22 +13,23 @@
* http://www.glyn.com/Products/Displays
*/
-#include <linux/module.h>
-#include <linux/ratelimit.h>
-#include <linux/irq.h>
-#include <linux/interrupt.h>
-#include <linux/input.h>
-#include <linux/i2c.h>
-#include <linux/kernel.h>
-#include <linux/uaccess.h>
-#include <linux/delay.h>
#include <linux/debugfs.h>
-#include <linux/slab.h>
+#include <linux/delay.h>
#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/input.h>
#include <linux/input/mt.h>
#include <linux/input/touchscreen.h>
-#include <asm/unaligned.h>
+#include <linux/irq.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/ratelimit.h>
#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+
+#include <asm/unaligned.h>
#define WORK_REGISTER_THRESHOLD 0x00
#define WORK_REGISTER_REPORT_RATE 0x08
--
2.20.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v3 2/6] Input: edt-ft5x06 - alphabetical include reorder
2020-01-08 11:10 ` [PATCH v3 2/6] Input: edt-ft5x06 - alphabetical include reorder Marco Felsch
@ 2020-01-10 1:06 ` Dmitry Torokhov
0 siblings, 0 replies; 17+ messages in thread
From: Dmitry Torokhov @ 2020-01-10 1:06 UTC (permalink / raw)
To: Marco Felsch
Cc: robh+dt, andriy.shevchenko, bparrot, andy.shevchenko,
simon.budig, hdegoede, fcooper, mripard, alexandre.belloni,
shawnguo, devicetree, kernel, linux-input
On Wed, Jan 08, 2020 at 12:10:46PM +0100, Marco Felsch wrote:
> It seems that the include order is historical increased and no one takes
> care of it. Fix this to align it with the common rule to be in a
> alphabetical order.
>
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Applied, thank you.
> ---
> drivers/input/touchscreen/edt-ft5x06.c | 23 ++++++++++++-----------
> 1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index b87b1e074f62..e1b31fd525e2 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -13,22 +13,23 @@
> * http://www.glyn.com/Products/Displays
> */
>
> -#include <linux/module.h>
> -#include <linux/ratelimit.h>
> -#include <linux/irq.h>
> -#include <linux/interrupt.h>
> -#include <linux/input.h>
> -#include <linux/i2c.h>
> -#include <linux/kernel.h>
> -#include <linux/uaccess.h>
> -#include <linux/delay.h>
> #include <linux/debugfs.h>
> -#include <linux/slab.h>
> +#include <linux/delay.h>
> #include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/input.h>
> #include <linux/input/mt.h>
> #include <linux/input/touchscreen.h>
> -#include <asm/unaligned.h>
> +#include <linux/irq.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/ratelimit.h>
> #include <linux/regulator/consumer.h>
> +#include <linux/slab.h>
> +#include <linux/uaccess.h>
> +
> +#include <asm/unaligned.h>
>
> #define WORK_REGISTER_THRESHOLD 0x00
> #define WORK_REGISTER_REPORT_RATE 0x08
> --
> 2.20.1
>
--
Dmitry
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 3/6] dt-bindings: Input: edt-ft5x06 - document wakeup-source capability
2020-01-08 11:10 [PATCH v3 0/6] EDT-FT5x06 improvements Marco Felsch
2020-01-08 11:10 ` [PATCH v3 1/6] Input: edt-ft5x06: work around first register access error Marco Felsch
2020-01-08 11:10 ` [PATCH v3 2/6] Input: edt-ft5x06 - alphabetical include reorder Marco Felsch
@ 2020-01-08 11:10 ` Marco Felsch
2020-01-10 1:07 ` Dmitry Torokhov
2020-01-08 11:10 ` [PATCH v3 4/6] Input: edt-ft5x06 - make wakeup-source switchable Marco Felsch
` (2 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Marco Felsch @ 2020-01-08 11:10 UTC (permalink / raw)
To: robh+dt, andriy.shevchenko, dmitry.torokhov, bparrot,
andy.shevchenko, simon.budig, hdegoede, fcooper, mripard,
alexandre.belloni, shawnguo
Cc: devicetree, kernel, linux-input, Rob Herring
The current driver behaviour was to enable the wakeup-source everytime.
After discussion [1] we decided to change that behaviour so the device
will act as wakeup-source only if the "wakeup-source" dt-property is
present.
The patch adds the binding documentation to enable the wakeup-source
capability.
[1] https://patchwork.kernel.org/patch/11149037/
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
v2:
- make use of common dt-property wakeup-source
- adapt commit message
---
.../devicetree/bindings/input/touchscreen/edt-ft5x06.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
index 0f6950073d6f..0e57315e9cbd 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
@@ -36,6 +36,8 @@ Optional properties:
- pinctrl-0: a phandle pointing to the pin settings for the
control gpios
+ - wakeup-source: If present the device will act as wakeup-source
+
- threshold: allows setting the "click"-threshold in the range
from 0 to 80.
--
2.20.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v3 3/6] dt-bindings: Input: edt-ft5x06 - document wakeup-source capability
2020-01-08 11:10 ` [PATCH v3 3/6] dt-bindings: Input: edt-ft5x06 - document wakeup-source capability Marco Felsch
@ 2020-01-10 1:07 ` Dmitry Torokhov
0 siblings, 0 replies; 17+ messages in thread
From: Dmitry Torokhov @ 2020-01-10 1:07 UTC (permalink / raw)
To: Marco Felsch
Cc: robh+dt, andriy.shevchenko, bparrot, andy.shevchenko,
simon.budig, hdegoede, fcooper, mripard, alexandre.belloni,
shawnguo, devicetree, kernel, linux-input, Rob Herring
On Wed, Jan 08, 2020 at 12:10:47PM +0100, Marco Felsch wrote:
> The current driver behaviour was to enable the wakeup-source everytime.
> After discussion [1] we decided to change that behaviour so the device
> will act as wakeup-source only if the "wakeup-source" dt-property is
> present.
>
> The patch adds the binding documentation to enable the wakeup-source
> capability.
>
> [1] https://patchwork.kernel.org/patch/11149037/
>
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
Applied, thank you.
> ---
> v2:
> - make use of common dt-property wakeup-source
> - adapt commit message
> ---
> .../devicetree/bindings/input/touchscreen/edt-ft5x06.txt | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> index 0f6950073d6f..0e57315e9cbd 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> @@ -36,6 +36,8 @@ Optional properties:
> - pinctrl-0: a phandle pointing to the pin settings for the
> control gpios
>
> + - wakeup-source: If present the device will act as wakeup-source
> +
> - threshold: allows setting the "click"-threshold in the range
> from 0 to 80.
>
> --
> 2.20.1
>
--
Dmitry
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 4/6] Input: edt-ft5x06 - make wakeup-source switchable
2020-01-08 11:10 [PATCH v3 0/6] EDT-FT5x06 improvements Marco Felsch
` (2 preceding siblings ...)
2020-01-08 11:10 ` [PATCH v3 3/6] dt-bindings: Input: edt-ft5x06 - document wakeup-source capability Marco Felsch
@ 2020-01-08 11:10 ` Marco Felsch
2020-01-10 1:07 ` Dmitry Torokhov
2020-01-08 11:10 ` [PATCH v3 5/6] Input: edt-ft5x06 - use pm core to enable/disable the wake irq Marco Felsch
2020-01-08 11:10 ` [PATCH v3 6/6] Input: edt-ft5x06 - improve power management operations Marco Felsch
5 siblings, 1 reply; 17+ messages in thread
From: Marco Felsch @ 2020-01-08 11:10 UTC (permalink / raw)
To: robh+dt, andriy.shevchenko, dmitry.torokhov, bparrot,
andy.shevchenko, simon.budig, hdegoede, fcooper, mripard,
alexandre.belloni, shawnguo
Cc: devicetree, kernel, linux-input
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>
Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v3:
- make use of i2c-core wakeup-source handling
v2:
- make use of common wakeup-source property
- adapt commit message
---
drivers/input/touchscreen/edt-ft5x06.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index e1b31fd525e2..c781952c3409 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -1208,7 +1208,6 @@ 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);
dev_dbg(&client->dev,
"EDT FT5x06 initialized: IRQ %d, WAKE pin %d, Reset pin %d.\n",
--
2.20.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v3 4/6] Input: edt-ft5x06 - make wakeup-source switchable
2020-01-08 11:10 ` [PATCH v3 4/6] Input: edt-ft5x06 - make wakeup-source switchable Marco Felsch
@ 2020-01-10 1:07 ` Dmitry Torokhov
0 siblings, 0 replies; 17+ messages in thread
From: Dmitry Torokhov @ 2020-01-10 1:07 UTC (permalink / raw)
To: Marco Felsch
Cc: robh+dt, andriy.shevchenko, bparrot, andy.shevchenko,
simon.budig, hdegoede, fcooper, mripard, alexandre.belloni,
shawnguo, devicetree, kernel, linux-input
On Wed, Jan 08, 2020 at 12:10:48PM +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>
> Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Applied, thank you.
> ---
> v3:
> - make use of i2c-core wakeup-source handling
>
> v2:
> - make use of common wakeup-source property
> - adapt commit message
> ---
> drivers/input/touchscreen/edt-ft5x06.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index e1b31fd525e2..c781952c3409 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -1208,7 +1208,6 @@ 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);
>
> dev_dbg(&client->dev,
> "EDT FT5x06 initialized: IRQ %d, WAKE pin %d, Reset pin %d.\n",
> --
> 2.20.1
>
--
Dmitry
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 5/6] Input: edt-ft5x06 - use pm core to enable/disable the wake irq
2020-01-08 11:10 [PATCH v3 0/6] EDT-FT5x06 improvements Marco Felsch
` (3 preceding siblings ...)
2020-01-08 11:10 ` [PATCH v3 4/6] Input: edt-ft5x06 - make wakeup-source switchable Marco Felsch
@ 2020-01-08 11:10 ` Marco Felsch
2020-01-10 1:07 ` Dmitry Torokhov
2020-01-08 11:10 ` [PATCH v3 6/6] Input: edt-ft5x06 - improve power management operations Marco Felsch
5 siblings, 1 reply; 17+ messages in thread
From: Marco Felsch @ 2020-01-08 11:10 UTC (permalink / raw)
To: robh+dt, andriy.shevchenko, dmitry.torokhov, bparrot,
andy.shevchenko, simon.budig, hdegoede, fcooper, mripard,
alexandre.belloni, shawnguo
Cc: devicetree, kernel, linux-input
We do not have to handle the wake-irq within the driver because the pm
core can handle this for us. The only use case for the suspend/resume
callbacks was to handle the wake-irq so we can remove the callbacks.
Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
v3:
- new patch to drop enable/disable_irq_wake() calls
---
drivers/input/touchscreen/edt-ft5x06.c | 24 ------------------------
1 file changed, 24 deletions(-)
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index c781952c3409..d2587724c52a 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -1227,29 +1227,6 @@ static int edt_ft5x06_ts_remove(struct i2c_client *client)
return 0;
}
-static int __maybe_unused edt_ft5x06_ts_suspend(struct device *dev)
-{
- struct i2c_client *client = to_i2c_client(dev);
-
- if (device_may_wakeup(dev))
- enable_irq_wake(client->irq);
-
- return 0;
-}
-
-static int __maybe_unused edt_ft5x06_ts_resume(struct device *dev)
-{
- struct i2c_client *client = to_i2c_client(dev);
-
- if (device_may_wakeup(dev))
- disable_irq_wake(client->irq);
-
- return 0;
-}
-
-static SIMPLE_DEV_PM_OPS(edt_ft5x06_ts_pm_ops,
- edt_ft5x06_ts_suspend, edt_ft5x06_ts_resume);
-
static const struct edt_i2c_chip_data edt_ft5x06_data = {
.max_support_points = 5,
};
@@ -1288,7 +1265,6 @@ static struct i2c_driver edt_ft5x06_ts_driver = {
.driver = {
.name = "edt_ft5x06",
.of_match_table = edt_ft5x06_of_match,
- .pm = &edt_ft5x06_ts_pm_ops,
},
.id_table = edt_ft5x06_ts_id,
.probe = edt_ft5x06_ts_probe,
--
2.20.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v3 5/6] Input: edt-ft5x06 - use pm core to enable/disable the wake irq
2020-01-08 11:10 ` [PATCH v3 5/6] Input: edt-ft5x06 - use pm core to enable/disable the wake irq Marco Felsch
@ 2020-01-10 1:07 ` Dmitry Torokhov
0 siblings, 0 replies; 17+ messages in thread
From: Dmitry Torokhov @ 2020-01-10 1:07 UTC (permalink / raw)
To: Marco Felsch
Cc: robh+dt, andriy.shevchenko, bparrot, andy.shevchenko,
simon.budig, hdegoede, fcooper, mripard, alexandre.belloni,
shawnguo, devicetree, kernel, linux-input
On Wed, Jan 08, 2020 at 12:10:49PM +0100, Marco Felsch wrote:
> We do not have to handle the wake-irq within the driver because the pm
> core can handle this for us. The only use case for the suspend/resume
> callbacks was to handle the wake-irq so we can remove the callbacks.
>
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
Applied, thank you.
> ---
> v3:
> - new patch to drop enable/disable_irq_wake() calls
> ---
> drivers/input/touchscreen/edt-ft5x06.c | 24 ------------------------
> 1 file changed, 24 deletions(-)
>
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index c781952c3409..d2587724c52a 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -1227,29 +1227,6 @@ static int edt_ft5x06_ts_remove(struct i2c_client *client)
> return 0;
> }
>
> -static int __maybe_unused edt_ft5x06_ts_suspend(struct device *dev)
> -{
> - struct i2c_client *client = to_i2c_client(dev);
> -
> - if (device_may_wakeup(dev))
> - enable_irq_wake(client->irq);
> -
> - return 0;
> -}
> -
> -static int __maybe_unused edt_ft5x06_ts_resume(struct device *dev)
> -{
> - struct i2c_client *client = to_i2c_client(dev);
> -
> - if (device_may_wakeup(dev))
> - disable_irq_wake(client->irq);
> -
> - return 0;
> -}
> -
> -static SIMPLE_DEV_PM_OPS(edt_ft5x06_ts_pm_ops,
> - edt_ft5x06_ts_suspend, edt_ft5x06_ts_resume);
> -
> static const struct edt_i2c_chip_data edt_ft5x06_data = {
> .max_support_points = 5,
> };
> @@ -1288,7 +1265,6 @@ static struct i2c_driver edt_ft5x06_ts_driver = {
> .driver = {
> .name = "edt_ft5x06",
> .of_match_table = edt_ft5x06_of_match,
> - .pm = &edt_ft5x06_ts_pm_ops,
> },
> .id_table = edt_ft5x06_ts_id,
> .probe = edt_ft5x06_ts_probe,
> --
> 2.20.1
>
--
Dmitry
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v3 6/6] Input: edt-ft5x06 - improve power management operations
2020-01-08 11:10 [PATCH v3 0/6] EDT-FT5x06 improvements Marco Felsch
` (4 preceding siblings ...)
2020-01-08 11:10 ` [PATCH v3 5/6] Input: edt-ft5x06 - use pm core to enable/disable the wake irq Marco Felsch
@ 2020-01-08 11:10 ` Marco Felsch
2020-01-10 1:09 ` Dmitry Torokhov
5 siblings, 1 reply; 17+ messages in thread
From: Marco Felsch @ 2020-01-08 11:10 UTC (permalink / raw)
To: robh+dt, andriy.shevchenko, dmitry.torokhov, bparrot,
andy.shevchenko, simon.budig, hdegoede, fcooper, mripard,
alexandre.belloni, shawnguo
Cc: devicetree, kernel, linux-input
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>
Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v3:
- drop enable/disable_irq_wake()
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 | 65 ++++++++++++++++++++++++++
1 file changed, 65 insertions(+)
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index d2587724c52a..4fd97758cbdd 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -38,6 +38,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
@@ -53,6 +56,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
@@ -1227,6 +1231,66 @@ static int edt_ft5x06_ts_remove(struct i2c_client *client)
return 0;
}
+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))
+ return 0;
+
+ /*
+ * Hibernate mode requires reset or wake signal to recover to active
+ * state.
+ */
+ if (!tsdata->wake_gpio && !tsdata->reset_gpio)
+ return 0;
+
+ ret = edt_ft5x06_register_write(tsdata, PMOD_REGISTER_OPMODE,
+ PMOD_REGISTER_HIBERNATE);
+ if (ret)
+ dev_warn(dev, "Failed to set hibernate mode\n");
+
+ ret = regulator_disable(tsdata->vcc);
+ if (ret)
+ dev_warn(dev, "Failed to disable vcc\n");
+
+ return 0;
+}
+
+static int __maybe_unused edt_ft5x06_ts_resume(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))
+ return 0;
+
+ ret = regulator_enable(tsdata->vcc);
+ if (ret)
+ dev_warn(dev, "Failed to enable vcc\n");
+
+ /* 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);
+ }
+
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(edt_ft5x06_ts_pm_ops,
+ edt_ft5x06_ts_suspend, edt_ft5x06_ts_resume);
+
static const struct edt_i2c_chip_data edt_ft5x06_data = {
.max_support_points = 5,
};
@@ -1265,6 +1329,7 @@ static struct i2c_driver edt_ft5x06_ts_driver = {
.driver = {
.name = "edt_ft5x06",
.of_match_table = edt_ft5x06_of_match,
+ .pm = &edt_ft5x06_ts_pm_ops,
},
.id_table = edt_ft5x06_ts_id,
.probe = edt_ft5x06_ts_probe,
--
2.20.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v3 6/6] Input: edt-ft5x06 - improve power management operations
2020-01-08 11:10 ` [PATCH v3 6/6] Input: edt-ft5x06 - improve power management operations Marco Felsch
@ 2020-01-10 1:09 ` Dmitry Torokhov
2020-01-10 7:16 ` Marco Felsch
0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Torokhov @ 2020-01-10 1:09 UTC (permalink / raw)
To: Marco Felsch
Cc: robh+dt, andriy.shevchenko, bparrot, andy.shevchenko,
simon.budig, hdegoede, fcooper, mripard, alexandre.belloni,
shawnguo, devicetree, kernel, linux-input
Hi Marco,
On Wed, Jan 08, 2020 at 12:10:50PM +0100, Marco Felsch wrote:
> +static int __maybe_unused edt_ft5x06_ts_resume(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))
> + return 0;
> +
> + ret = regulator_enable(tsdata->vcc);
> + if (ret)
> + dev_warn(dev, "Failed to enable vcc\n");
I wonder if we should not return error here instead of continuing. If
device is not powered up properly we'll have hard time communicating
with it.
The same is for suspend: maybe we should abort if we can't switch off
regulator or write to the device.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v3 6/6] Input: edt-ft5x06 - improve power management operations
2020-01-10 1:09 ` Dmitry Torokhov
@ 2020-01-10 7:16 ` Marco Felsch
2020-01-10 7:18 ` Marco Felsch
0 siblings, 1 reply; 17+ messages in thread
From: Marco Felsch @ 2020-01-10 7:16 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: robh+dt, andriy.shevchenko, bparrot, andy.shevchenko,
simon.budig, hdegoede, fcooper, mripard, alexandre.belloni,
shawnguo, devicetree, kernel, linux-input
Hi Dmitry,
On 20-01-09 17:09, Dmitry Torokhov wrote:
> Hi Marco,
>
> On Wed, Jan 08, 2020 at 12:10:50PM +0100, Marco Felsch wrote:
> > +static int __maybe_unused edt_ft5x06_ts_resume(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))
> > + return 0;
> > +
> > + ret = regulator_enable(tsdata->vcc);
> > + if (ret)
> > + dev_warn(dev, "Failed to enable vcc\n");
>
> I wonder if we should not return error here instead of continuing. If
> device is not powered up properly we'll have hard time communicating
> with it.
That's a reasonable point.
> The same is for suspend: maybe we should abort if we can't switch off
> regulator or write to the device.
I have no strong opinion about that case but IMHO it's okay to go further
if we can't switch it off. Instead we should print a warning.
Regards,
Marco
> Thanks.
>
> --
> Dmitry
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v3 6/6] Input: edt-ft5x06 - improve power management operations
2020-01-10 7:16 ` Marco Felsch
@ 2020-01-10 7:18 ` Marco Felsch
2020-01-16 13:32 ` Marco Felsch
0 siblings, 1 reply; 17+ messages in thread
From: Marco Felsch @ 2020-01-10 7:18 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: devicetree, alexandre.belloni, robh+dt, bparrot, hdegoede,
andy.shevchenko, simon.budig, mripard, kernel, linux-input,
andriy.shevchenko, shawnguo, fcooper
On 20-01-10 08:16, Marco Felsch wrote:
> Hi Dmitry,
>
> On 20-01-09 17:09, Dmitry Torokhov wrote:
> > Hi Marco,
> >
> > On Wed, Jan 08, 2020 at 12:10:50PM +0100, Marco Felsch wrote:
> > > +static int __maybe_unused edt_ft5x06_ts_resume(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))
> > > + return 0;
> > > +
> > > + ret = regulator_enable(tsdata->vcc);
> > > + if (ret)
> > > + dev_warn(dev, "Failed to enable vcc\n");
> >
> > I wonder if we should not return error here instead of continuing. If
> > device is not powered up properly we'll have hard time communicating
> > with it.
>
> That's a reasonable point.
>
> > The same is for suspend: maybe we should abort if we can't switch off
> > regulator or write to the device.
>
> I have no strong opinion about that case but IMHO it's okay to go further
> if we can't switch it off. Instead we should print a warning.
I just noticed that we do that already.. So the suspend case should be
okay.
> Regards,
> Marco
>
> > Thanks.
> >
> > --
> > Dmitry
> >
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v3 6/6] Input: edt-ft5x06 - improve power management operations
2020-01-10 7:18 ` Marco Felsch
@ 2020-01-16 13:32 ` Marco Felsch
2020-01-22 6:00 ` Dmitry Torokhov
0 siblings, 1 reply; 17+ messages in thread
From: Marco Felsch @ 2020-01-16 13:32 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: devicetree, alexandre.belloni, simon.budig, mripard, bparrot,
hdegoede, andy.shevchenko, robh+dt, kernel, linux-input,
andriy.shevchenko, shawnguo, fcooper
Hi Dmitry,
On 20-01-10 08:18, Marco Felsch wrote:
> On 20-01-10 08:16, Marco Felsch wrote:
> > Hi Dmitry,
> >
> > On 20-01-09 17:09, Dmitry Torokhov wrote:
> > > Hi Marco,
> > >
> > > On Wed, Jan 08, 2020 at 12:10:50PM +0100, Marco Felsch wrote:
> > > > +static int __maybe_unused edt_ft5x06_ts_resume(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))
> > > > + return 0;
> > > > +
> > > > + ret = regulator_enable(tsdata->vcc);
> > > > + if (ret)
> > > > + dev_warn(dev, "Failed to enable vcc\n");
> > >
> > > I wonder if we should not return error here instead of continuing. If
> > > device is not powered up properly we'll have hard time communicating
> > > with it.
> >
> > That's a reasonable point.
> >
> > > The same is for suspend: maybe we should abort if we can't switch off
> > > regulator or write to the device.
> >
> > I have no strong opinion about that case but IMHO it's okay to go further
> > if we can't switch it off. Instead we should print a warning.
>
> I just noticed that we do that already.. So the suspend case should be
> okay.
Is it okay to check the return val for the resume case only? I want to
prepare a v4 of this patch to get this done.
Regards,
Marco
>
> > Regards,
> > Marco
> >
> > > Thanks.
> > >
> > > --
> > > Dmitry
> > >
>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v3 6/6] Input: edt-ft5x06 - improve power management operations
2020-01-16 13:32 ` Marco Felsch
@ 2020-01-22 6:00 ` Dmitry Torokhov
0 siblings, 0 replies; 17+ messages in thread
From: Dmitry Torokhov @ 2020-01-22 6:00 UTC (permalink / raw)
To: Marco Felsch
Cc: devicetree, alexandre.belloni, simon.budig, mripard, bparrot,
hdegoede, andy.shevchenko, robh+dt, kernel, linux-input,
andriy.shevchenko, shawnguo, fcooper
Hi Marco,
On Thu, Jan 16, 2020 at 02:32:19PM +0100, Marco Felsch wrote:
> Hi Dmitry,
>
> On 20-01-10 08:18, Marco Felsch wrote:
> > On 20-01-10 08:16, Marco Felsch wrote:
> > > Hi Dmitry,
> > >
> > > On 20-01-09 17:09, Dmitry Torokhov wrote:
> > > > Hi Marco,
> > > >
> > > > On Wed, Jan 08, 2020 at 12:10:50PM +0100, Marco Felsch wrote:
> > > > > +static int __maybe_unused edt_ft5x06_ts_resume(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))
> > > > > + return 0;
> > > > > +
> > > > > + ret = regulator_enable(tsdata->vcc);
> > > > > + if (ret)
> > > > > + dev_warn(dev, "Failed to enable vcc\n");
> > > >
> > > > I wonder if we should not return error here instead of continuing. If
> > > > device is not powered up properly we'll have hard time communicating
> > > > with it.
> > >
> > > That's a reasonable point.
> > >
> > > > The same is for suspend: maybe we should abort if we can't switch off
> > > > regulator or write to the device.
> > >
> > > I have no strong opinion about that case but IMHO it's okay to go further
> > > if we can't switch it off. Instead we should print a warning.
> >
> > I just noticed that we do that already.. So the suspend case should be
> > okay.
>
>
> Is it okay to check the return val for the resume case only? I want to
> prepare a v4 of this patch to get this done.
OK, I now remember my issues with power management in this driver. It
supports factory mode vs operational/normal mode, and updating register
settings at runtime. If you want to cut power off at suspend, then you
need to make sure you restore the mode and register settings at resume
time, not simply revert to normal mode.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 17+ messages in thread