[PATCH V3 1/2] input: egalax_ts: free irq resource before request the line as GPIO

[PATCH V3 1/2] input: egalax_ts: free irq resource before request the line as GPIO

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

If GPIO is connected to an IRQ then it should not request it as
GPIO function only when free its IRQ resource.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/input/touchscreen/egalax_ts.c | 46 ++++++++++++++++++++++-----
 1 file changed, 38 insertions(+), 8 deletions(-)

diff --git a/drivers/input/touchscreen/egalax_ts.c b/drivers/input/touchscreen/egalax_ts.c
index 83ac8c128192..d3dc6d14bb78 100644
--- a/drivers/input/touchscreen/egalax_ts.c
+++ b/drivers/input/touchscreen/egalax_ts.c
@@ -116,6 +116,26 @@ static irqreturn_t egalax_ts_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static int egalax_irq_request(struct egalax_ts *ts)
+{
+	int ret;
+	struct i2c_client *client = ts->client;
+
+	ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
+					egalax_ts_interrupt,
+					IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+					"egalax_ts", ts);
+	if (ret < 0)
+		dev_err(&client->dev, "Failed to register interrupt\n");
+
+	return ret;
+}
+
+static void egalax_free_irq(struct egalax_ts *ts)
+{
+	devm_free_irq(&ts->client->dev, ts->client->irq, ts);
+}
+
 /* wake up controller by an falling edge of interrupt gpio.  */
 static int egalax_wake_up_device(struct i2c_client *client)
 {
@@ -211,19 +231,16 @@ static int egalax_ts_probe(struct i2c_client *client,
 			     ABS_MT_POSITION_Y, 0, EGALAX_MAX_Y, 0, 0);
 	input_mt_init_slots(input_dev, MAX_SUPPORT_POINTS, 0);
 
-	error = devm_request_threaded_irq(&client->dev, client->irq, NULL,
-					  egalax_ts_interrupt,
-					  IRQF_TRIGGER_LOW | IRQF_ONESHOT,
-					  "egalax_ts", ts);
-	if (error < 0) {
-		dev_err(&client->dev, "Failed to register interrupt\n");
+	error = egalax_irq_request(ts);
+	if (error)
 		return error;
-	}
 
 	error = input_register_device(ts->input_dev);
 	if (error)
 		return error;
 
+	i2c_set_clientdata(client, ts);
+
 	return 0;
 }
 
@@ -251,11 +268,24 @@ static int __maybe_unused egalax_ts_suspend(struct device *dev)
 static int __maybe_unused egalax_ts_resume(struct device *dev)
 {
 	struct i2c_client *client = to_i2c_client(dev);
+	struct egalax_ts *ts = i2c_get_clientdata(client);
+	int error;
 
 	if (device_may_wakeup(dev))
 		return disable_irq_wake(client->irq);
 
-	return egalax_wake_up_device(client);
+	/* Free IRQ as IRQ pin is used as output in the resume sequence */
+	egalax_free_irq(ts);
+
+	error = egalax_wake_up_device(client);
+	if (error) {
+		dev_err(&client->dev, "Failed to wake up the controller\n");
+		return error;
+	}
+
+	error = egalax_irq_request(ts);
+
+	return error;
 }
 
 static SIMPLE_DEV_PM_OPS(egalax_ts_pm_ops, egalax_ts_suspend, egalax_ts_resume);
-- 
2.17.1

[PATCH V3 2/2] input: egalax_ts: fix the get_firmware_command

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

According to the User Guide, the get firmware command is
{ 0x03, 0x03, 0xa, 0x01, 'D' }, ASCII value of 'D' is 0x44.

This patch fix that.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/input/touchscreen/egalax_ts.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/egalax_ts.c b/drivers/input/touchscreen/egalax_ts.c
index d3dc6d14bb78..1da6ddb9b4ee 100644
--- a/drivers/input/touchscreen/egalax_ts.c
+++ b/drivers/input/touchscreen/egalax_ts.c
@@ -171,7 +171,7 @@ static int egalax_wake_up_device(struct i2c_client *client)
 
 static int egalax_firmware_version(struct i2c_client *client)
 {
-	static const u8 cmd[MAX_I2C_DATA_LEN] = { 0x03, 0x03, 0xa, 0x01, 0x41 };
+	static const u8 cmd[MAX_I2C_DATA_LEN] = { 0x03, 0x03, 0xa, 0x01, 0x44 };
 	int ret;
 
 	ret = i2c_master_send(client, cmd, MAX_I2C_DATA_LEN);
-- 
2.17.1

Re: [PATCH V3 2/2] input: egalax_ts: fix the get_firmware_command

[Dmitry Torokhov]

Hi Haibo,

On Wed, Apr 15, 2020 at 04:01:03PM +0800, haibo.chen@nxp.com wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> According to the User Guide, the get firmware command is
> { 0x03, 0x03, 0xa, 0x01, 'D' }, ASCII value of 'D' is 0x44.
> 
> This patch fix that.

You are absolutely right that 0x03 0x03 0x0a 0x01 0x44 is the proper
sequence for the "get firmware version" command, however, despite the function
name, we are not fetching firmware here, but rather try to check if
device operates normally via the "check active" command. So if anything
we should rename the function to egalax_check_active(). We should also
try reading the data sent back by the device and verify that it is what
we expect.

And if you indeed want to retrieve firmware version and controller type,
that should be separate functions.

> 
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> ---
>  drivers/input/touchscreen/egalax_ts.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/touchscreen/egalax_ts.c b/drivers/input/touchscreen/egalax_ts.c
> index d3dc6d14bb78..1da6ddb9b4ee 100644
> --- a/drivers/input/touchscreen/egalax_ts.c
> +++ b/drivers/input/touchscreen/egalax_ts.c
> @@ -171,7 +171,7 @@ static int egalax_wake_up_device(struct i2c_client *client)
>  
>  static int egalax_firmware_version(struct i2c_client *client)
>  {
> -	static const u8 cmd[MAX_I2C_DATA_LEN] = { 0x03, 0x03, 0xa, 0x01, 0x41 };
> +	static const u8 cmd[MAX_I2C_DATA_LEN] = { 0x03, 0x03, 0xa, 0x01, 0x44 };
>  	int ret;
>  
>  	ret = i2c_master_send(client, cmd, MAX_I2C_DATA_LEN);
> -- 
> 2.17.1
> 

Thanks.

-- 
Dmitry