linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Input: elantench - Fix the firmware misreport coordinates for trackpoint occasionally.
@ 2021-07-29  1:03 Phoenix Huang
  0 siblings, 0 replies; 9+ messages in thread
From: Phoenix Huang @ 2021-07-29  1:03 UTC (permalink / raw)
  To: linux-kernel, linux-input, dmitry.torokhov
  Cc: jingle.wu, josh.chen, dave.wang, 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] Input: elantench - Fix the firmware misreport coordinates for trackpoint occasionally.
  2021-11-08  6:14         ` Yufei Du
@ 2021-11-10  6:24           ` Dmitry Torokhov
  0 siblings, 0 replies; 9+ messages in thread
From: Dmitry Torokhov @ 2021-11-10  6:24 UTC (permalink / raw)
  To: Yufei Du
  Cc: linux-kernel, linux-input, jingle.wu, josh.chen, dave.wang, phoenix

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Input: elantench - Fix the firmware misreport coordinates for trackpoint occasionally.
  2021-11-08  6:06       ` Dmitry Torokhov
@ 2021-11-08  6:14         ` Yufei Du
  2021-11-10  6:24           ` Dmitry Torokhov
  0 siblings, 1 reply; 9+ messages in thread
From: Yufei Du @ 2021-11-08  6:14 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-kernel, linux-input, jingle.wu, josh.chen, dave.wang, phoenix

Hi Dmitry,

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

Sure. Thanks!

Best,
Yufei

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Input: elantench - Fix the firmware misreport coordinates for trackpoint occasionally.
  2021-11-07 20:02     ` Yufei Du
@ 2021-11-08  6:06       ` Dmitry Torokhov
  2021-11-08  6:14         ` Yufei Du
  0 siblings, 1 reply; 9+ messages in thread
From: Dmitry Torokhov @ 2021-11-08  6:06 UTC (permalink / raw)
  To: Yufei Du
  Cc: linux-kernel, linux-input, jingle.wu, josh.chen, dave.wang, phoenix

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Input: elantench - Fix the firmware misreport coordinates for trackpoint occasionally.
  2021-08-30 20:23   ` Dmitry Torokhov
  2021-09-10  2:18     ` phoenix
@ 2021-11-07 20:02     ` Yufei Du
  2021-11-08  6:06       ` Dmitry Torokhov
  1 sibling, 1 reply; 9+ messages in thread
From: Yufei Du @ 2021-11-07 20:02 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-kernel, linux-input, jingle.wu, josh.chen, dave.wang, phoenix

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] Input: elantench - Fix the firmware misreport coordinates for trackpoint occasionally.
  2021-08-30 20:23   ` Dmitry Torokhov
@ 2021-09-10  2:18     ` phoenix
  2021-11-07 20:02     ` Yufei Du
  1 sibling, 0 replies; 9+ messages in thread
From: phoenix @ 2021-09-10  2:18 UTC (permalink / raw)
  To: 'Dmitry Torokhov'
  Cc: linux-kernel, linux-input, jingle.wu, josh.chen, dave.wang

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Input: elantench - Fix the firmware misreport coordinates for trackpoint occasionally.
  2021-08-30 12:19 ` phoenix
@ 2021-08-30 20:23   ` Dmitry Torokhov
  2021-09-10  2:18     ` phoenix
  2021-11-07 20:02     ` Yufei Du
  0 siblings, 2 replies; 9+ messages in thread
From: Dmitry Torokhov @ 2021-08-30 20:23 UTC (permalink / raw)
  To: phoenix; +Cc: linux-kernel, linux-input, jingle.wu, josh.chen, dave.wang

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] Input: elantench - Fix the firmware misreport coordinates for trackpoint occasionally.
  2021-07-29  1:09 Phoenix Huang
@ 2021-08-30 12:19 ` phoenix
  2021-08-30 20:23   ` Dmitry Torokhov
  0 siblings, 1 reply; 9+ messages in thread
From: phoenix @ 2021-08-30 12:19 UTC (permalink / raw)
  To: linux-kernel, linux-input, dmitry.torokhov
  Cc: jingle.wu, josh.chen, dave.wang

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH] Input: elantench - Fix the firmware misreport coordinates for trackpoint occasionally.
@ 2021-07-29  1:09 Phoenix Huang
  2021-08-30 12:19 ` phoenix
  0 siblings, 1 reply; 9+ messages in thread
From: Phoenix Huang @ 2021-07-29  1:09 UTC (permalink / raw)
  To: linux-kernel, linux-input, dmitry.torokhov
  Cc: jingle.wu, josh.chen, dave.wang, 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-11-10  6:24 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-29  1:03 [PATCH] Input: elantench - Fix the firmware misreport coordinates for trackpoint occasionally Phoenix Huang
2021-07-29  1:09 Phoenix Huang
2021-08-30 12:19 ` phoenix
2021-08-30 20:23   ` Dmitry Torokhov
2021-09-10  2:18     ` phoenix
2021-11-07 20:02     ` Yufei Du
2021-11-08  6:06       ` Dmitry Torokhov
2021-11-08  6:14         ` Yufei Du
2021-11-10  6:24           ` Dmitry Torokhov

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).