* [PATCH v2 0/5] EDT-FT5x06 improvements
@ 2019-11-27 12:09 Marco Felsch
2019-11-27 12:09 ` [PATCH v2 1/5] Input: edt-ft5x06: work around first register access error Marco Felsch
` (6 more replies)
0 siblings, 7 replies; 21+ messages in thread
From: Marco Felsch @ 2019-11-27 12:09 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
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.
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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 1/5] Input: edt-ft5x06: work around first register access error
2019-11-27 12:09 [PATCH v2 0/5] EDT-FT5x06 improvements Marco Felsch
@ 2019-11-27 12:09 ` Marco Felsch
2019-12-02 17:58 ` Dmitry Torokhov
2019-11-27 12:09 ` [PATCH v2 2/5] Input: edt-ft5x06 - alphabetical include reorder Marco Felsch
` (5 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: Marco Felsch @ 2019-11-27 12:09 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>
---
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] 21+ messages in thread
* [PATCH v2 2/5] Input: edt-ft5x06 - alphabetical include reorder
2019-11-27 12:09 [PATCH v2 0/5] EDT-FT5x06 improvements Marco Felsch
2019-11-27 12:09 ` [PATCH v2 1/5] Input: edt-ft5x06: work around first register access error Marco Felsch
@ 2019-11-27 12:09 ` Marco Felsch
2019-11-27 12:09 ` [PATCH v2 3/5] dt-bindings: Input: edt-ft5x06 - document wakeup-source capability Marco Felsch
` (4 subsequent siblings)
6 siblings, 0 replies; 21+ messages in thread
From: Marco Felsch @ 2019-11-27 12:09 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>
---
v2:
- move asm include behind linux includes
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] 21+ messages in thread
* [PATCH v2 3/5] dt-bindings: Input: edt-ft5x06 - document wakeup-source capability
2019-11-27 12:09 [PATCH v2 0/5] EDT-FT5x06 improvements Marco Felsch
2019-11-27 12:09 ` [PATCH v2 1/5] Input: edt-ft5x06: work around first register access error Marco Felsch
2019-11-27 12:09 ` [PATCH v2 2/5] Input: edt-ft5x06 - alphabetical include reorder Marco Felsch
@ 2019-11-27 12:09 ` Marco Felsch
2019-12-05 21:15 ` Rob Herring
2019-11-27 12:09 ` [PATCH v2 4/5] Input: edt-ft5x06 - make wakeup-source switchable Marco Felsch
` (3 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: Marco Felsch @ 2019-11-27 12:09 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
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>
---
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] 21+ messages in thread
* [PATCH v2 4/5] Input: edt-ft5x06 - make wakeup-source switchable
2019-11-27 12:09 [PATCH v2 0/5] EDT-FT5x06 improvements Marco Felsch
` (2 preceding siblings ...)
2019-11-27 12:09 ` [PATCH v2 3/5] dt-bindings: Input: edt-ft5x06 - document wakeup-source capability Marco Felsch
@ 2019-11-27 12:09 ` Marco Felsch
2019-12-02 18:00 ` Dmitry Torokhov
2019-11-27 12:09 ` [PATCH v2 5/5] Input: edt-ft5x06 - improve power management operations Marco Felsch
` (2 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: Marco Felsch @ 2019-11-27 12:09 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>
---
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);
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] 21+ messages in thread
* [PATCH v2 5/5] Input: edt-ft5x06 - improve power management operations
2019-11-27 12:09 [PATCH v2 0/5] EDT-FT5x06 improvements Marco Felsch
` (3 preceding siblings ...)
2019-11-27 12:09 ` [PATCH v2 4/5] Input: edt-ft5x06 - make wakeup-source switchable Marco Felsch
@ 2019-11-27 12:09 ` Marco Felsch
2019-11-27 12:59 ` Andy Shevchenko
2019-12-02 18:04 ` Dmitry Torokhov
2019-11-27 17:10 ` [PATCH v2 0/5] EDT-FT5x06 improvements Andy Shevchenko
2019-11-27 17:14 ` Andy Shevchenko
6 siblings, 2 replies; 21+ messages in thread
From: Marco Felsch @ 2019-11-27 12:09 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>
---
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);
+ 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;
}
@@ -1245,9 +1269,30 @@ static int __maybe_unused edt_ft5x06_ts_suspend(struct device *dev)
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))
+ if (device_may_wakeup(dev)) {
disable_irq_wake(client->irq);
+ 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;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 5/5] Input: edt-ft5x06 - improve power management operations
2019-11-27 12:09 ` [PATCH v2 5/5] Input: edt-ft5x06 - improve power management operations Marco Felsch
@ 2019-11-27 12:59 ` Andy Shevchenko
2019-11-27 13:06 ` Marco Felsch
2019-12-02 18:04 ` Dmitry Torokhov
1 sibling, 1 reply; 21+ messages in thread
From: Andy Shevchenko @ 2019-11-27 12:59 UTC (permalink / raw)
To: Marco Felsch
Cc: robh+dt, dmitry.torokhov, bparrot, simon.budig, hdegoede,
fcooper, mripard, alexandre.belloni, shawnguo, devicetree,
kernel, linux-input
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);
...
}
?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 5/5] Input: edt-ft5x06 - improve power management operations
2019-11-27 12:59 ` Andy Shevchenko
@ 2019-11-27 13:06 ` Marco Felsch
2019-11-27 14:32 ` Andy Shevchenko
0 siblings, 1 reply; 21+ messages in thread
From: Marco Felsch @ 2019-11-27 13:06 UTC (permalink / raw)
To: Andy Shevchenko
Cc: robh+dt, dmitry.torokhov, bparrot, simon.budig, hdegoede,
fcooper, mripard, alexandre.belloni, shawnguo, devicetree,
kernel, linux-input
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
>
>
>
--
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] 21+ messages in thread
* Re: [PATCH v2 5/5] Input: edt-ft5x06 - improve power management operations
2019-11-27 13:06 ` Marco Felsch
@ 2019-11-27 14:32 ` Andy Shevchenko
2019-11-27 17:25 ` Marco Felsch
0 siblings, 1 reply; 21+ messages in thread
From: Andy Shevchenko @ 2019-11-27 14:32 UTC (permalink / raw)
To: Marco Felsch
Cc: robh+dt, dmitry.torokhov, bparrot, simon.budig, hdegoede,
fcooper, mripard, alexandre.belloni, shawnguo, devicetree,
kernel, linux-input
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);
...
}
?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 0/5] EDT-FT5x06 improvements
2019-11-27 12:09 [PATCH v2 0/5] EDT-FT5x06 improvements Marco Felsch
` (4 preceding siblings ...)
2019-11-27 12:09 ` [PATCH v2 5/5] Input: edt-ft5x06 - improve power management operations Marco Felsch
@ 2019-11-27 17:10 ` Andy Shevchenko
2019-11-27 17:14 ` Andy Shevchenko
6 siblings, 0 replies; 21+ messages in thread
From: Andy Shevchenko @ 2019-11-27 17:10 UTC (permalink / raw)
To: Marco Felsch
Cc: robh+dt, dmitry.torokhov, bparrot, simon.budig, hdegoede,
fcooper, mripard, alexandre.belloni, shawnguo, devicetree,
kernel, linux-input
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
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 0/5] EDT-FT5x06 improvements
2019-11-27 12:09 [PATCH v2 0/5] EDT-FT5x06 improvements Marco Felsch
` (5 preceding siblings ...)
2019-11-27 17:10 ` [PATCH v2 0/5] EDT-FT5x06 improvements Andy Shevchenko
@ 2019-11-27 17:14 ` Andy Shevchenko
2019-11-27 17:25 ` Marco Felsch
6 siblings, 1 reply; 21+ messages in thread
From: Andy Shevchenko @ 2019-11-27 17:14 UTC (permalink / raw)
To: Marco Felsch
Cc: robh+dt, dmitry.torokhov, bparrot, simon.budig, hdegoede,
fcooper, mripard, alexandre.belloni, shawnguo, devicetree,
kernel, linux-input
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
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 5/5] Input: edt-ft5x06 - improve power management operations
2019-11-27 14:32 ` Andy Shevchenko
@ 2019-11-27 17:25 ` Marco Felsch
2019-11-27 18:06 ` Andy Shevchenko
0 siblings, 1 reply; 21+ messages in thread
From: Marco Felsch @ 2019-11-27 17:25 UTC (permalink / raw)
To: Andy Shevchenko
Cc: robh+dt, dmitry.torokhov, bparrot, simon.budig, hdegoede,
fcooper, mripard, alexandre.belloni, shawnguo, devicetree,
kernel, linux-input
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
>
>
>
--
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] 21+ messages in thread
* Re: [PATCH v2 0/5] EDT-FT5x06 improvements
2019-11-27 17:14 ` Andy Shevchenko
@ 2019-11-27 17:25 ` Marco Felsch
0 siblings, 0 replies; 21+ messages in thread
From: Marco Felsch @ 2019-11-27 17:25 UTC (permalink / raw)
To: Andy Shevchenko
Cc: robh+dt, dmitry.torokhov, bparrot, simon.budig, hdegoede,
fcooper, mripard, alexandre.belloni, shawnguo, devicetree,
kernel, linux-input
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
>
>
>
--
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] 21+ messages in thread
* Re: [PATCH v2 5/5] Input: edt-ft5x06 - improve power management operations
2019-11-27 17:25 ` Marco Felsch
@ 2019-11-27 18:06 ` Andy Shevchenko
0 siblings, 0 replies; 21+ messages in thread
From: Andy Shevchenko @ 2019-11-27 18:06 UTC (permalink / raw)
To: Marco Felsch
Cc: robh+dt, dmitry.torokhov, bparrot, simon.budig, hdegoede,
fcooper, mripard, alexandre.belloni, shawnguo, devicetree,
kernel, linux-input
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.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/5] Input: edt-ft5x06: work around first register access error
2019-11-27 12:09 ` [PATCH v2 1/5] Input: edt-ft5x06: work around first register access error Marco Felsch
@ 2019-12-02 17:58 ` Dmitry Torokhov
2019-12-03 8:15 ` Marco Felsch
0 siblings, 1 reply; 21+ messages in thread
From: Dmitry Torokhov @ 2019-12-02 17:58 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, 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
>
--
Dmitry
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 4/5] Input: edt-ft5x06 - make wakeup-source switchable
2019-11-27 12:09 ` [PATCH v2 4/5] Input: edt-ft5x06 - make wakeup-source switchable Marco Felsch
@ 2019-12-02 18:00 ` Dmitry Torokhov
2019-12-10 8:36 ` Marco Felsch
0 siblings, 1 reply; 21+ messages in thread
From: Dmitry Torokhov @ 2019-12-02 18:00 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, 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.
--
Dmitry
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 5/5] Input: edt-ft5x06 - improve power management operations
2019-11-27 12:09 ` [PATCH v2 5/5] Input: edt-ft5x06 - improve power management operations Marco Felsch
2019-11-27 12:59 ` Andy Shevchenko
@ 2019-12-02 18:04 ` Dmitry Torokhov
2019-12-10 8:43 ` Marco Felsch
1 sibling, 1 reply; 21+ messages in thread
From: Dmitry Torokhov @ 2019-12-02 18:04 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, 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.
--
Dmitry
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/5] Input: edt-ft5x06: work around first register access error
2019-12-02 17:58 ` Dmitry Torokhov
@ 2019-12-03 8:15 ` Marco Felsch
0 siblings, 0 replies; 21+ messages in thread
From: Marco Felsch @ 2019-12-03 8:15 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, Philipp Zabel
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
>
--
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] 21+ messages in thread
* Re: [PATCH v2 3/5] dt-bindings: Input: edt-ft5x06 - document wakeup-source capability
2019-11-27 12:09 ` [PATCH v2 3/5] dt-bindings: Input: edt-ft5x06 - document wakeup-source capability Marco Felsch
@ 2019-12-05 21:15 ` Rob Herring
0 siblings, 0 replies; 21+ messages in thread
From: Rob Herring @ 2019-12-05 21:15 UTC (permalink / raw)
To: Marco Felsch
Cc: robh+dt, andriy.shevchenko, dmitry.torokhov, bparrot,
andy.shevchenko, simon.budig, hdegoede, fcooper, mripard,
alexandre.belloni, shawnguo, devicetree, kernel, linux-input
On Wed, 27 Nov 2019 13:09:46 +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>
> ---
> 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(+)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 4/5] Input: edt-ft5x06 - make wakeup-source switchable
2019-12-02 18:00 ` Dmitry Torokhov
@ 2019-12-10 8:36 ` Marco Felsch
0 siblings, 0 replies; 21+ messages in thread
From: Marco Felsch @ 2019-12-10 8:36 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
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
>
--
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] 21+ messages in thread
* Re: [PATCH v2 5/5] Input: edt-ft5x06 - improve power management operations
2019-12-02 18:04 ` Dmitry Torokhov
@ 2019-12-10 8:43 ` Marco Felsch
0 siblings, 0 replies; 21+ messages in thread
From: Marco Felsch @ 2019-12-10 8:43 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
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
>
--
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] 21+ messages in thread
end of thread, other threads:[~2019-12-10 8:43 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-27 12:09 [PATCH v2 0/5] EDT-FT5x06 improvements Marco Felsch
2019-11-27 12:09 ` [PATCH v2 1/5] Input: edt-ft5x06: work around first register access error Marco Felsch
2019-12-02 17:58 ` Dmitry Torokhov
2019-12-03 8:15 ` Marco Felsch
2019-11-27 12:09 ` [PATCH v2 2/5] Input: edt-ft5x06 - alphabetical include reorder Marco Felsch
2019-11-27 12:09 ` [PATCH v2 3/5] dt-bindings: Input: edt-ft5x06 - document wakeup-source capability Marco Felsch
2019-12-05 21:15 ` Rob Herring
2019-11-27 12:09 ` [PATCH v2 4/5] Input: edt-ft5x06 - make wakeup-source switchable Marco Felsch
2019-12-02 18:00 ` Dmitry Torokhov
2019-12-10 8:36 ` Marco Felsch
2019-11-27 12:09 ` [PATCH v2 5/5] Input: edt-ft5x06 - improve power management operations Marco Felsch
2019-11-27 12:59 ` Andy Shevchenko
2019-11-27 13:06 ` Marco Felsch
2019-11-27 14:32 ` Andy Shevchenko
2019-11-27 17:25 ` Marco Felsch
2019-11-27 18:06 ` Andy Shevchenko
2019-12-02 18:04 ` Dmitry Torokhov
2019-12-10 8:43 ` Marco Felsch
2019-11-27 17:10 ` [PATCH v2 0/5] EDT-FT5x06 improvements Andy Shevchenko
2019-11-27 17:14 ` Andy Shevchenko
2019-11-27 17:25 ` Marco Felsch
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).