Bug 195215 - Touchpad touches occasionally not recognized after 4.10.7 upgrade
Summary: Touchpad touches occasionally not recognized after 4.10.7 upgrade
Status: RESOLVED CODE_FIX
Alias: None
Product: Drivers
Classification: Unclassified
Component: Input Devices (show other bugs)
Hardware: x86-64 Linux
: P1 normal
Assignee: drivers_input-devices
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-04-01 21:17 UTC by dandels
Modified: 2017-09-05 22:13 UTC (History)
3 users (show)

See Also:
Kernel Version: 4.10.8
Subsystem:
Regression: Yes
Bisected commit-id:


Attachments

Description dandels 2017-04-01 21:17:04 UTC
After upgrading to Linux 4.10.7 on my Dell Latitude e5470, every now and then moving my finger on the touchpad has no effect. The issue is also present in Linux 4.10.8. Downgrading to Linux 4.10.6 solves the issue. 

I haven't been able to reproduce the bug reliably due to the nature of the issue. Touching the touchpad and moving the cursor might work 20 times in a row before requiring one or more tries to move it once again, but it varies a lot.
Comment 1 Dmitry Torokhov 2017-04-12 15:58:04 UTC
There were 2 commits to ALPS driver between 4.10.6 and 4.10.7. Could you try reverting one and then another to see which one is faulty?

b87dd1d7dacc Input: ALPS - fix trackstick button handling on V8 devices
0186e6a4e501 Input: ALPS - fix V8+ protocol handling (73 03 28)
Comment 3 Masaki Ota 2017-04-13 02:58:37 UTC
Dell Latitude e5470 has two models that are StickPointer + Touchpad or only Touchpad.
Which model do you use?
Comment 4 dandels 2017-04-13 07:57:13 UTC
(In reply to Masaki Ota from comment #3)
> Dell Latitude e5470 has two models that are StickPointer + Touchpad or only
> Touchpad.
> Which model do you use?

The stickpointer + touchpad variant.
Comment 5 dandels 2017-04-13 11:46:07 UTC
(In reply to Dmitry Torokhov from comment #1)
> There were 2 commits to ALPS driver between 4.10.6 and 4.10.7. Could you try
> reverting one and then another to see which one is faulty?
> 
> b87dd1d7dacc Input: ALPS - fix trackstick button handling on V8 devices
> 0186e6a4e501 Input: ALPS - fix V8+ protocol handling (73 03 28)

I tested by editing Arch's Linux PKGBUILD to use version 4.10.7 and reverse patching both commits in turn.  

Reverting "Input: ALPS - fix V8+ protocol handling (73 03 28)" fixes the issue. I found two finger scrolling failed often enough to be reproducible, and the issue is no longer present after reverting the commit.
Comment 6 Masaki Ota 2017-04-14 00:27:05 UTC
I couldn't reproduce this issue.
Actually I have a real Touchpad, but don't have a real machine.
Can you check where the issue happens in the source code?
Comment 7 dandels 2017-04-28 17:20:07 UTC
Based on my rather limited understanding of C, I changed "#define IS_SS4PLUS(_b)" in alps.h to "#define IS_SS4PLUS_DEV(_b) (0)" and "#define IS_SS4PLUS_DEV(_b) (1)", then compared results. 

When it's set to 1, the same faulty behavior occurs. 

When it's set to 0 the touchpad works as expected but the stickpointer and related buttons are disabled, with dmesg reporting "psmouse serio1: alps: Rejected trackstick packet from non DualPoint device". Everything appears to work correctly when I also comment out the "if (is_dual)"-check in "alps_update_dual_info_ss4_v2" on line 2525 of alps.c. 

Is this enough to answer the question of where the issue occurs in the source? I'll gladly test any proposed fixes.
Comment 8 Masaki Ota 2017-05-09 09:47:49 UTC
Thank you for checking the code.
You mean that if delete "if (is_dual)", the issue is fixed, right?
Does "priv->flags" value have both of ALPS_DUALPOINT and ALPS_DUALPOINT_WITH_PRESSURE ?

If the following code that has a brackets, the issue is fixed? 

	if (is_dual) {
		priv->flags |= ALPS_DUALPOINT |
					ALPS_DUALPOINT_WITH_PRESSURE;
}
Comment 9 dandels 2017-05-09 18:37:44 UTC
To clarify, here's a git diff of the changes I made: https://ptpb.pw/Trbh
Comment 10 Masaki Ota 2017-05-10 00:18:28 UTC
OK, can you try to comment out the following part, and test it? (Don't change #define IS_SS4PLUS_DEV(_b) (1))

alps_update_device_area_ss4_v2()
{
...
...
/*	if (IS_SS4PLUS_DEV(priv->dev_id)) {
		num_x_electrode =
			SS4PLUS_NUMSENSOR_XOFFSET + (otp[0][2] & 0x0F);
		num_y_electrode =
			SS4PLUS_NUMSENSOR_YOFFSET + ((otp[0][2] >> 4) & 0x0F);

		priv->x_max =
			(num_x_electrode - 1) * SS4PLUS_COUNT_PER_ELECTRODE;
		priv->y_max =
			(num_y_electrode - 1) * SS4PLUS_COUNT_PER_ELECTRODE;

		x_pitch = (otp[0][1] & 0x0F) + SS4PLUS_MIN_PITCH_MM;
		y_pitch = ((otp[0][1] >> 4) & 0x0F) + SS4PLUS_MIN_PITCH_MM;

	} else {
*/		num_x_electrode =
			SS4_NUMSENSOR_XOFFSET + (otp[1][0] & 0x0F);
		num_y_electrode =
			SS4_NUMSENSOR_YOFFSET + ((otp[1][0] >> 4) & 0x0F);

		priv->x_max =
			(num_x_electrode - 1) * SS4_COUNT_PER_ELECTRODE;
		priv->y_max =
			(num_y_electrode - 1) * SS4_COUNT_PER_ELECTRODE;

		x_pitch = ((otp[1][2] >> 2) & 0x07) + SS4_MIN_PITCH_MM;
		y_pitch = ((otp[1][2] >> 5) & 0x07) + SS4_MIN_PITCH_MM;
/*	}*/
Comment 11 dandels 2017-05-10 05:44:05 UTC
Seems to work fine with these changes.
Comment 12 Masaki Ota 2017-05-10 06:25:28 UTC
Thank you for testing.
Finally, I have found the problem in current source code.
I believe this is the root cause.

Can you try to test the following code?

alps_set_defaults_ss4_v2()
{
...
...
/*	if (alps_get_otp_values_ss4_v2(psmouse, 0, &otp[0][0]) ||
	    alps_get_otp_values_ss4_v2(psmouse, 1, &otp[1][0]))
		return -1;
*/

if (alps_get_otp_values_ss4_v2(psmouse, 1, &otp[1][0]) ||
	    alps_get_otp_values_ss4_v2(psmouse, 0, &otp[0][0]))
		return -1;
}
Comment 13 dandels 2017-05-10 15:26:41 UTC
I tried it can't tell the difference between previous broken behavior. 

If it's of any help, something that I only noticed after filing the bug: when the touchpad "doesn't work", the sensitivity is a lot higher than normally.
Comment 14 Masaki Ota 2017-05-11 09:47:34 UTC
OK, but current codes don't change the device sensitivity and any other parameters.
It's weird that the sensitivity is high.
Now I have no idea, if my comment 12 does not fix this issue.
Comment 15 Paul Donohue 2017-05-11 21:09:00 UTC
/* if (alps_get_otp_values_ss4_v2(psmouse, 0, &otp[0][0]) ||
       alps_get_otp_values_ss4_v2(psmouse, 1, &otp[1][0]))
*/
   if (alps_get_otp_values_ss4_v2(psmouse, 1, &otp[1][0]) ||
       alps_get_otp_values_ss4_v2(psmouse, 0, &otp[0][0]))

Is there a copy/paste error in this code in comment 12?  The commented and uncommented if statements are effectively identical - the only change is the order of the conditions, which wouldn't make any difference in the behavior: if(A || B) vs if(B || A)
Are you sure this change should affect the behavior somehow?
Comment 16 Masaki Ota 2017-05-11 23:50:02 UTC
Yes, this is the special commands for reading the OTP(Device information).
To read otp[0][X] page, it sends "EA EA E9" commands, but I found the problem that one front of this command is "EA".
So, it means that it sends "EA EA EA E9". In these commands, we cannot get correct otp[0][X] value.

On the other hands, to read otp[1][X] page, it sends "F0 F0 E9". 
This page can get correct otp[1][X] value, even if we send "EA F0 F0 E9" commands.

I debugged it and it works correctly.
So, I'm not sure that it has to do with the original issue, but anyway my comment 12 should apply to the mainstream codes.
Comment 17 Paul Donohue 2017-08-05 18:44:20 UTC
Fix has been developed, but has not been merged yet.

For reference, see these threads:
https://www.spinics.net/lists/linux-input/msg51636.html
https://www.spinics.net/lists/linux-input/msg52352.html
Comment 18 Paul Donohue 2017-08-28 21:57:03 UTC
Fix is merged in kernel 4.13-rc7 : https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4a646580f793d19717f7e034c8d473b509c27d49

It is also queued for kernels 4.9.46 and 4.12.10
Comment 19 dandels 2017-09-05 22:13:18 UTC
Works for me on 4.12.10, so I suppose I can resolve this bug. Thanks a lot for fixing this.

Note You need to log in before you can comment on or make changes to this bug.