[PATCH v3 0/2] Input: ilitek_ts_i2c - Fix spurious input events

[PATCH v3 0/2] Input: ilitek_ts_i2c - Fix spurious input events

[Francesco Dolcini]

From: Francesco Dolcini <francesco.dolcini@toradex.com>

A couple of fixes to prevent spurious events when the data buffer is not the
expected one.

v3:
 - added reviewed-by and take over series from emanuele

v2:
 - initial series, sent by mistake as v2 instead of v1

Emanuele Ghidoli (2):
  Input: ilitek_ts_i2c - avoid wrong input subsystem sync
  Input: ilitek_ts_i2c - add report id message validation

 drivers/input/touchscreen/ilitek_ts_i2c.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

-- 
2.25.1

[PATCH v3 1/2] Input: ilitek_ts_i2c - avoid wrong input subsystem sync

[Francesco Dolcini]

From: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>

For different reasons i2c transaction may fail or
report id message content may be wrong.
Avoid sync the input subsystem if message cannot be parsed.
An input subsystem sync without points is interpreted as
"nothing is touching the screen" while normally this is not the case.

Fixes: 42370681bd46 ("Input: Add support for ILITEK Lego Series")
Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
Signed-off-by: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>
Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v3: added reviewed by
---
 drivers/input/touchscreen/ilitek_ts_i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/ilitek_ts_i2c.c b/drivers/input/touchscreen/ilitek_ts_i2c.c
index fc4e39b6651a..250133f0d68f 100644
--- a/drivers/input/touchscreen/ilitek_ts_i2c.c
+++ b/drivers/input/touchscreen/ilitek_ts_i2c.c
@@ -203,9 +203,9 @@ static int ilitek_process_and_report_v6(struct ilitek_ts_data *ts)
 		ilitek_touch_down(ts, id, x, y);
 	}
 
-err_sync_frame:
 	input_mt_sync_frame(input);
 	input_sync(input);
+err_sync_frame:
 	return error;
 }
 
-- 
2.25.1

Re: [PATCH v3 1/2] Input: ilitek_ts_i2c - avoid wrong input subsystem sync

[Dmitry Torokhov]

Hi Francesco,

On Fri, Dec 22, 2023 at 07:31:13PM +0100, Francesco Dolcini wrote:
> From: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>
> 
> For different reasons i2c transaction may fail or
> report id message content may be wrong.
> Avoid sync the input subsystem if message cannot be parsed.
> An input subsystem sync without points is interpreted as
> "nothing is touching the screen" while normally this is not the case.
> 
> Fixes: 42370681bd46 ("Input: Add support for ILITEK Lego Series")
> Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> Signed-off-by: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>

If you are adding your sign-off you do not need to have your
reviewed-by, it is assumed that you reviewed the code you are passing
on.

> ---
> v3: added reviewed by
> ---
>  drivers/input/touchscreen/ilitek_ts_i2c.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/touchscreen/ilitek_ts_i2c.c b/drivers/input/touchscreen/ilitek_ts_i2c.c
> index fc4e39b6651a..250133f0d68f 100644
> --- a/drivers/input/touchscreen/ilitek_ts_i2c.c
> +++ b/drivers/input/touchscreen/ilitek_ts_i2c.c
> @@ -203,9 +203,9 @@ static int ilitek_process_and_report_v6(struct ilitek_ts_data *ts)
>  		ilitek_touch_down(ts, id, x, y);
>  	}
>  
> -err_sync_frame:
>  	input_mt_sync_frame(input);
>  	input_sync(input);
> +err_sync_frame:

You are not syncing the frame, so that label is wrong. If you do not
want to release the contacts just change "goto err_sync_frame" to
direct returns and remove the label.

Thanks.

-- 
Dmitry

[PATCH v3 2/2] Input: ilitek_ts_i2c - add report id message validation

[Francesco Dolcini]

From: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>

Ilitek touch IC driver answer to plain i2c read request, after it has
generated an interrupt request, with a report id message starting
with an identifier and a series of points.
If a request is sent unsolicited by an interrupt request the answer
do not contain this identifier.
Add a test to discard these messages, making the driver robust to
spurious interrupt requests.

Fixes: 42370681bd46 ("Input: Add support for ILITEK Lego Series")
Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
Signed-off-by: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>
Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v3: added reviewed by
---
 drivers/input/touchscreen/ilitek_ts_i2c.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/input/touchscreen/ilitek_ts_i2c.c b/drivers/input/touchscreen/ilitek_ts_i2c.c
index 250133f0d68f..557362490244 100644
--- a/drivers/input/touchscreen/ilitek_ts_i2c.c
+++ b/drivers/input/touchscreen/ilitek_ts_i2c.c
@@ -37,6 +37,8 @@
 #define ILITEK_TP_CMD_GET_MCU_VER			0x61
 #define ILITEK_TP_CMD_GET_IC_MODE			0xC0
 
+#define ILITEK_TP_I2C_REPORT_ID				0x48
+
 #define REPORT_COUNT_ADDRESS				61
 #define ILITEK_SUPPORT_MAX_POINT			40
 
@@ -163,6 +165,11 @@ static int ilitek_process_and_report_v6(struct ilitek_ts_data *ts)
 		goto err_sync_frame;
 	}
 
+	if (buf[0] != ILITEK_TP_I2C_REPORT_ID) {
+		dev_err(dev, "get touch info failed. Wrong id: 0x%02X\n", buf[0]);
+		goto err_sync_frame;
+	}
+
 	report_max_point = buf[REPORT_COUNT_ADDRESS];
 	if (report_max_point > ts->max_tp) {
 		dev_err(dev, "FW report max point:%d > panel info. max:%d\n",
-- 
2.25.1

Re: [PATCH v3 2/2] Input: ilitek_ts_i2c - add report id message validation

[Dmitry Torokhov]

On Fri, Dec 22, 2023 at 07:31:14PM +0100, Francesco Dolcini wrote:
> From: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>
> 
> Ilitek touch IC driver answer to plain i2c read request, after it has
> generated an interrupt request, with a report id message starting
> with an identifier and a series of points.
> If a request is sent unsolicited by an interrupt request the answer
> do not contain this identifier.
> Add a test to discard these messages, making the driver robust to
> spurious interrupt requests.
> 
> Fixes: 42370681bd46 ("Input: Add support for ILITEK Lego Series")
> Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> Signed-off-by: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> ---
> v3: added reviewed by
> ---
>  drivers/input/touchscreen/ilitek_ts_i2c.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/input/touchscreen/ilitek_ts_i2c.c b/drivers/input/touchscreen/ilitek_ts_i2c.c
> index 250133f0d68f..557362490244 100644
> --- a/drivers/input/touchscreen/ilitek_ts_i2c.c
> +++ b/drivers/input/touchscreen/ilitek_ts_i2c.c
> @@ -37,6 +37,8 @@
>  #define ILITEK_TP_CMD_GET_MCU_VER			0x61
>  #define ILITEK_TP_CMD_GET_IC_MODE			0xC0
>  
> +#define ILITEK_TP_I2C_REPORT_ID				0x48
> +
>  #define REPORT_COUNT_ADDRESS				61
>  #define ILITEK_SUPPORT_MAX_POINT			40
>  
> @@ -163,6 +165,11 @@ static int ilitek_process_and_report_v6(struct ilitek_ts_data *ts)
>  		goto err_sync_frame;
>  	}
>  
> +	if (buf[0] != ILITEK_TP_I2C_REPORT_ID) {
> +		dev_err(dev, "get touch info failed. Wrong id: 0x%02X\n", buf[0]);
> +		goto err_sync_frame;

Please rebase and use direct return once you fix the previous patch.

Thanks.

-- 
Dmitry

Re: [PATCH v3 0/2] Input: ilitek_ts_i2c - Fix spurious input events

[Francesco Dolcini]

Hello Dmitry,

On Fri, Dec 22, 2023 at 07:31:12PM +0100, Francesco Dolcini wrote:
> A couple of fixes to prevent spurious events when the data buffer is not the
> expected one.

Any chance you can merge this series or provide feedback on it? This is
open since 10 months now and I would need some help on moving it forward.

Thanks,
Francesco