[PATCH] Input: elantench - Fix the firmware misreport coordinates for trackpoint occasionally.

[PATCH] Input: elantench - Fix the firmware misreport coordinates for trackpoint occasionally.

[Phoenix Huang]

Signed-off-by: Phoenix Huang <phoenix@emc.com.tw>
---
 drivers/input/mouse/elantech.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 2d0bc029619f..07e1098f2d31 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -517,6 +517,17 @@ static void elantech_report_trackpoint(struct psmouse *psmouse,
 	case 0x16008020U:
 	case 0x26800010U:
 	case 0x36808000U:
+	
+		/* This firmware misreport coordinates for trackpoint occasionally.
+		* So we discard these packets by pattern to prevent cursor jumps.
+		*/
+		if (packet[4] == 0x80 || packet[5] == 0x80 ||
+		    packet[1] >> 7 == packet[4] >> 7 ||
+		    packet[2] >> 7 == packet[5] >> 7) {
+		    	elantech_debug("discarding packet [%6ph]\n", packet);
+			break;
+
+		}
 		x = packet[4] - (int)((packet[1]^0x80) << 1);
 		y = (int)((packet[2]^0x80) << 1) - packet[5];
 
-- 
2.25.1

RE: [PATCH] Input: elantench - Fix the firmware misreport coordinates for trackpoint occasionally.

[phoenix]

Hi Dmitry,

Would you review this patch, thanks

Best regards,
Phoenix Huang

-----Original Message-----
From: Phoenix Huang [mailto:phoenix@emc.com.tw] 
Sent: Thursday, July 29, 2021 9:10 AM
To: linux-kernel@vger.kernel.org; linux-input@vger.kernel.org;
dmitry.torokhov@gmail.com
Cc: jingle.wu@emc.com.tw; josh.chen@emc.com.tw; dave.wang@emc.com.tw;
Phoenix Huang <phoenix@emc.com.tw>
Subject: [PATCH] Input: elantench - Fix the firmware misreport coordinates
for trackpoint occasionally.

Signed-off-by: Phoenix Huang <phoenix@emc.com.tw>
---
 drivers/input/mouse/elantech.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 2d0bc029619f..07e1098f2d31 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -517,6 +517,17 @@ static void elantech_report_trackpoint(struct psmouse
*psmouse,
 	case 0x16008020U:
 	case 0x26800010U:
 	case 0x36808000U:
+	
+		/* This firmware misreport coordinates for trackpoint
occasionally.
+		* So we discard these packets by pattern to prevent cursor
jumps.
+		*/
+		if (packet[4] == 0x80 || packet[5] == 0x80 ||
+		    packet[1] >> 7 == packet[4] >> 7 ||
+		    packet[2] >> 7 == packet[5] >> 7) {
+		    	elantech_debug("discarding packet [%6ph]\n",
packet);
+			break;
+
+		}
 		x = packet[4] - (int)((packet[1]^0x80) << 1);
 		y = (int)((packet[2]^0x80) << 1) - packet[5];
 
--
2.25.1

Re: [PATCH] Input: elantench - Fix the firmware misreport coordinates for trackpoint occasionally.

[Dmitry Torokhov]

Hi Phoenix,

On Mon, Aug 30, 2021 at 08:19:48PM +0800, phoenix wrote:
> Hi Dmitry,
> 
> Would you review this patch, thanks
> 
> Best regards,
> Phoenix Huang
> 
> -----Original Message-----
> From: Phoenix Huang [mailto:phoenix@emc.com.tw] 
> Sent: Thursday, July 29, 2021 9:10 AM
> To: linux-kernel@vger.kernel.org; linux-input@vger.kernel.org;
> dmitry.torokhov@gmail.com
> Cc: jingle.wu@emc.com.tw; josh.chen@emc.com.tw; dave.wang@emc.com.tw;
> Phoenix Huang <phoenix@emc.com.tw>
> Subject: [PATCH] Input: elantench - Fix the firmware misreport coordinates
> for trackpoint occasionally.
> 
> Signed-off-by: Phoenix Huang <phoenix@emc.com.tw>
> ---
>  drivers/input/mouse/elantech.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index 2d0bc029619f..07e1098f2d31 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -517,6 +517,17 @@ static void elantech_report_trackpoint(struct psmouse
> *psmouse,
>  	case 0x16008020U:
>  	case 0x26800010U:
>  	case 0x36808000U:
> +	
> +		/* This firmware misreport coordinates for trackpoint
> occasionally.
> +		* So we discard these packets by pattern to prevent cursor
> jumps.
> +		*/
> +		if (packet[4] == 0x80 || packet[5] == 0x80 ||
> +		    packet[1] >> 7 == packet[4] >> 7 ||

I think this will reject X coordinates in range [0, 127]. Is this really
what is needed? What kind of patterns are you observing when firmware
misreports coordinates?

Thanks.

-- 
Dmitry

RE: [PATCH] Input: elantench - Fix the firmware misreport coordinates for trackpoint occasionally.

[phoenix]

Hi Dmity,

Sorry for late reply.
> +		if (packet[4] == 0x80 || packet[5] == 0x80 ||
> +		    packet[1] >> 7 == packet[4] >> 7 ||
"I think this will reject X coordinates in range [0, 127]. Is this really
what is needed? What kind of patterns are you observing when firmware
misreports coordinates?"
this will reject X in range [128, 256] and [-128, -256]. This patch prevent
cursor suddenly jump to screen edge or corner.
The patterns we observing as below:
1. x (packet[4]) or y (packet[5]) is 0x80, ex: [xx xx xx xx 80 80]
2. ~x sign bit is same as MSB(most significant bit) of x, ex: [30 00 00 06
00 00] 
3. ~y sign bit is same as MSB of y, ex: [30 00 00 06 fa 00]

Best regards,
Phoenix Huang
-----Original Message-----
From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com] 
Sent: Tuesday, August 31, 2021 4:24 AM
To: phoenix <phoenix@emc.com.tw>
Cc: linux-kernel@vger.kernel.org; linux-input@vger.kernel.org;
jingle.wu@emc.com.tw; josh.chen@emc.com.tw; dave.wang@emc.com.tw
Subject: Re: [PATCH] Input: elantench - Fix the firmware misreport
coordinates for trackpoint occasionally.

Hi Phoenix,

On Mon, Aug 30, 2021 at 08:19:48PM +0800, phoenix wrote:
> Hi Dmitry,
> 
> Would you review this patch, thanks
> 
> Best regards,
> Phoenix Huang
> 
> -----Original Message-----
> From: Phoenix Huang [mailto:phoenix@emc.com.tw]
> Sent: Thursday, July 29, 2021 9:10 AM
> To: linux-kernel@vger.kernel.org; linux-input@vger.kernel.org; 
> dmitry.torokhov@gmail.com
> Cc: jingle.wu@emc.com.tw; josh.chen@emc.com.tw; dave.wang@emc.com.tw; 
> Phoenix Huang <phoenix@emc.com.tw>
> Subject: [PATCH] Input: elantench - Fix the firmware misreport 
> coordinates for trackpoint occasionally.
> 
> Signed-off-by: Phoenix Huang <phoenix@emc.com.tw>
> ---
>  drivers/input/mouse/elantech.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/input/mouse/elantech.c 
> b/drivers/input/mouse/elantech.c index 2d0bc029619f..07e1098f2d31 
> 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -517,6 +517,17 @@ static void elantech_report_trackpoint(struct 
> psmouse *psmouse,
>  	case 0x16008020U:
>  	case 0x26800010U:
>  	case 0x36808000U:
> +	
> +		/* This firmware misreport coordinates for trackpoint
> occasionally.
> +		* So we discard these packets by pattern to prevent cursor
> jumps.
> +		*/
> +		if (packet[4] == 0x80 || packet[5] == 0x80 ||
> +		    packet[1] >> 7 == packet[4] >> 7 ||

I think this will reject X coordinates in range [0, 127]. Is this really
what is needed? What kind of patterns are you observing when firmware
misreports coordinates?

Thanks.

--
Dmitry

Re: [PATCH] Input: elantench - Fix the firmware misreport coordinates for trackpoint occasionally.

[Yufei Du]

Hi Dmitry,

I am a user currently suffering from the firmware bug that causes my trackpoint 
to be unusable on my Lenovo X13 Gen 1 AMD. I tested this patch in the latest 
5.15 mainline kernel tree and can possibly answer your questions regarding the 
patch.

 > I think this will reject X coordinates in range [0, 127]. Is this really
 > what is needed?

As Phoenix explained in his reply, the patch will reject X and Y coordinates 
larger than 127 or smaller than -127. This is because the most significant bit 
of packet[1] (for X) and packet[2] (for Y) are inverse of the sign bit. Although 
I do not have the formal design specification of the device, I tried pushing the 
trackpoint as hard as I could, and the X and Y coordinates never go beyond 
+/-127 normally, so I believe that this patch would not cause any functionality 
issue.

 > What kind of patterns are you observing when firmware
 > misreports coordinates?

Here is a sample log I observed (with elantech.debug set to 2 for more debugging 
messages):
psmouse serio1: elantech: PS/2 packet [10 00 80 26 f9 0a]
psmouse serio1: elantech: PS/2 packet [10 00 80 26 00 80]
psmouse serio1: elantech: PS/2 packet [10 00 80 00 00 00]
psmouse serio1: Touchpad at isa0060/serio1/input0 lost sync at byte 6
psmouse serio1: elantech: PS/2 packet [10 00 80 26 f8 0b]
psmouse serio1: Touchpad at isa0060/serio1/input0 - driver resynced.

The first packet is a valid packet. The second packet is the incorrect packet 
that this patch prevents. The third packet is a corrupted packet that psmouse 
detects (as shown in the "lost sync" message), and the fourth packet is a valid 
packet again. Without the patch, the second packet would cause the cursor to 
jump to the edge of the screen.

So far, all my logs of this issue show the same pattern of a jumpy incorrect 
packet followed by a corrupted packet that causes psmouse to lose sync. There 
are more logs and discussions in the BugZilla entry here: 
https://bugzilla.kernel.org/show_bug.cgi?id=209167

Hope my explanation helped a bit.

Thanks!

Best,
Yufei Du

Re: [PATCH] Input: elantench - Fix the firmware misreport coordinates for trackpoint occasionally.

[Dmitry Torokhov]

Hi Yufei,

On Sun, Nov 07, 2021 at 03:02:27PM -0500, Yufei Du wrote:
> Hi Dmitry,
> 
> I am a user currently suffering from the firmware bug that causes my
> trackpoint to be unusable on my Lenovo X13 Gen 1 AMD. I tested this patch in
> the latest 5.15 mainline kernel tree and can possibly answer your questions
> regarding the patch.
> 
> > I think this will reject X coordinates in range [0, 127]. Is this really
> > what is needed?
> 
> As Phoenix explained in his reply, the patch will reject X and Y coordinates
> larger than 127 or smaller than -127. This is because the most significant
> bit of packet[1] (for X) and packet[2] (for Y) are inverse of the sign bit.
> Although I do not have the formal design specification of the device, I
> tried pushing the trackpoint as hard as I could, and the X and Y coordinates
> never go beyond +/-127 normally, so I believe that this patch would not
> cause any functionality issue.
> 
> > What kind of patterns are you observing when firmware
> > misreports coordinates?
> 
> Here is a sample log I observed (with elantech.debug set to 2 for more
> debugging messages):
> psmouse serio1: elantech: PS/2 packet [10 00 80 26 f9 0a]
> psmouse serio1: elantech: PS/2 packet [10 00 80 26 00 80]
> psmouse serio1: elantech: PS/2 packet [10 00 80 00 00 00]
> psmouse serio1: Touchpad at isa0060/serio1/input0 lost sync at byte 6
> psmouse serio1: elantech: PS/2 packet [10 00 80 26 f8 0b]
> psmouse serio1: Touchpad at isa0060/serio1/input0 - driver resynced.
> 
> The first packet is a valid packet. The second packet is the incorrect
> packet that this patch prevents. The third packet is a corrupted packet that
> psmouse detects (as shown in the "lost sync" message), and the fourth packet
> is a valid packet again. Without the patch, the second packet would cause
> the cursor to jump to the edge of the screen.
> 
> So far, all my logs of this issue show the same pattern of a jumpy incorrect
> packet followed by a corrupted packet that causes psmouse to lose sync.
> There are more logs and discussions in the BugZilla entry here:
> https://bugzilla.kernel.org/show_bug.cgi?id=209167
> 
> Hope my explanation helped a bit.

Sorry I lost track of this patch. Can I add your Tested-by to it before
applying?

Thanks.

-- 
Dmitry

Re: [PATCH] Input: elantench - Fix the firmware misreport coordinates for trackpoint occasionally.

[Yufei Du]

Hi Dmitry,

 > Sorry I lost track of this patch. Can I add your Tested-by to it before
 > applying?

Sure. Thanks!

Best,
Yufei

Re: [PATCH] Input: elantench - Fix the firmware misreport coordinates for trackpoint occasionally.

[Dmitry Torokhov]

On Mon, Nov 08, 2021 at 01:14:44AM -0500, Yufei Du wrote:
> Hi Dmitry,
> 
> > Sorry I lost track of this patch. Can I add your Tested-by to it before
> > applying?
> 
> Sure. Thanks!

It is applied now, thank you everyone.

-- 
Dmitry

[PATCH] Input: elantench - Fix the firmware misreport coordinates for trackpoint occasionally.

[Phoenix Huang]

Signed-off-by: Phoenix Huang <phoenix@emc.com.tw>
---
 drivers/input/mouse/elantech.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 2d0bc029619f..a31c2f5b7cf0 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -517,6 +517,17 @@ static void elantech_report_trackpoint(struct psmouse *psmouse,
 	case 0x16008020U:
 	case 0x26800010U:
 	case 0x36808000U:
+	
+		/* This firmware misreport coordinates for trackpoint occasionally.
+		* So we discard these packets by pattern to prevent cursor jumps.
+		*/
+		if (packet[4] == 0x80 || packet[5] == 0x80 ||
+		    packet[1] >> 7 == packet[4] >> 7 ||
+		    packet[2] >> 7 == packet[5] >> 7) {
+		    	elantech_debug("discarding packet [%6ph]\n", packet);
+			break;
+
+		}
 		x = packet[4] - (int)((packet[1]^0x80) << 1);
 		y = (int)((packet[2]^0x80) << 1) - packet[5];
 
@@ -1678,7 +1689,7 @@ static int elantech_query_info(struct psmouse *psmouse,
 		return -EINVAL;
 	}
 	psmouse_info(psmouse,
-		     "assuming hardware version %d (with firmware version 0x%02x%02x%02x)\n",
+		     "(7/19) assuming hardware version %d (with firmware version 0x%02x%02x%02x)\n",
 		     info->hw_version, param[0], param[1], param[2]);
 
 	if (info->send_cmd(psmouse, ETP_CAPABILITIES_QUERY,
-- 
2.25.1