RE: [PATCH 1/2] input: egalax_ts: add system wakeup support

Hi, Dmitry

Anson Huang
Best Regards!

-----Original Message-----
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Sent: Thursday, September 6, 2018 1:27 AM
To: Anson Huang <redacted>
Cc: robh+dt@kernel.org; mark.rutland@arm.com; Marco Antonio Franchi
[off-list ref]; Fabio Estevam [off-list ref];
linux-input@vger.kernel.org; devicetree@vger.kernel.org;
linux-kernel@vger.kernel.org; dl-linux-imx [off-list ref]
Subject: Re: [PATCH 1/2] input: egalax_ts: add system wakeup support

Hi Anson,

On Wed, Sep 05, 2018 at 05:26:46PM +0800, Anson Huang wrote:

quoted

This patch adds wakeup function support for egalax touch screen, if
"wakeup-source" is added to device tree's egalax touch screen node,
the wakeup function will be enabled, and egalax touch screen will be
able to wakeup system from suspend.

Signed-off-by: Anson Huang <redacted>
---
 drivers/input/touchscreen/egalax_ts.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/input/touchscreen/egalax_ts.c

b/drivers/input/touchscreen/egalax_ts.c
index 80e69bb..74984ed 100644

--- a/drivers/input/touchscreen/egalax_ts.c
+++ b/drivers/input/touchscreen/egalax_ts.c

@@ -164,6 +164,7 @@ static int egalax_firmware_version(struct

i2c_client *client)  static int egalax_ts_probe(struct i2c_client *client,
 			   const struct i2c_device_id *id)  {
+	struct device_node *np = client->dev.of_node;
 	struct egalax_ts *ts;
 	struct input_dev *input_dev;
 	int error;

@@ -224,6 +225,9 @@ static int egalax_ts_probe(struct i2c_client *client,
 	if (error)
 		return error;

+	if (of_property_read_bool(np, "wakeup-source"))
+		device_init_wakeup(&client->dev, true);

This is done in i2c core, there is no need to do it in the driver.

 
Correct, I removed it in V2 patch.

quoted

+
 	return 0;
 }

@@ -241,6 +245,9 @@ static int __maybe_unused

egalax_ts_suspend(struct device *dev)

quoted

 	struct i2c_client *client = to_i2c_client(dev);
 	int ret;

+	if (device_may_wakeup(dev))
+		return enable_irq_wake(client->irq);
+
 	ret = i2c_master_send(client, suspend_cmd, MAX_I2C_DATA_LEN);
 	return ret > 0 ? 0 : ret;
 }

@@ -249,6 +256,9 @@ static int __maybe_unused egalax_ts_resume(struct

device *dev)  {
 	struct i2c_client *client = to_i2c_client(dev);

+	if (device_may_wakeup(dev))
+		return 0;

This will end up with unbalanced enable_irq_wake() from suspend.

Ah, it is a mistake, I fix it in V2 patch, please help review it.

And for the other patch about adding "wakeup-source" into dt-binding, I think it may
be no need now, since it is already included in i2c's dt-binding, right?

Thanks.

Anson.

quoted

+
 	return egalax_wake_up_device(client);  }

--
2.7.4

Thanks.

--
Dmitry