Bug 195215
Summary: | Touchpad touches occasionally not recognized after 4.10.7 upgrade | ||
---|---|---|---|
Product: | Drivers | Reporter: | dandels (dandybot) |
Component: | Input Devices | Assignee: | drivers_input-devices |
Status: | RESOLVED CODE_FIX | ||
Severity: | normal | CC: | dmitry.torokhov, linux-kernel, masaki.ota |
Priority: | P1 | ||
Hardware: | x86-64 | ||
OS: | Linux | ||
Kernel Version: | 4.10.8 | Subsystem: | |
Regression: | Yes | Bisected commit-id: |
Description
dandels
2017-04-01 21:17: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) For reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/commit/?id=b87dd1d7dacc2f0d300fa6d5e3065948db3095fc https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/commit/?id=0186e6a4e501d39f5f90dd7e5887bc668aef06c4 Dell Latitude e5470 has two models that are StickPointer + Touchpad or only Touchpad. Which model do you use? (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. (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. 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? 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. 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; } To clarify, here's a git diff of the changes I made: https://ptpb.pw/Trbh 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; /* }*/ Seems to work fine with these changes. 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; } 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. 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. /* 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? 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. 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 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 Works for me on 4.12.10, so I suppose I can resolve this bug. Thanks a lot for fixing this. |