[SRU][B/OEM-B/D/OEM-OSP1-B/E][PATCH v2 0/2] input/mouse: alps trackpoint-only device doesn't work

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU][B/OEM-B/D/OEM-OSP1-B/E][PATCH v2 0/2] input/mouse: alps trackpoint-only device doesn't work

Hui Wang
BugLink: https://bugs.launchpad.net/bugs/1836752

In the V2:
I added one more patch (0002) to fix the problem of condition checking
of 0x20~0x2f (apw found this problem in the v1)

Since the OEM-OSP1-B already merged 0001, it only needs to merge 0002
this time, for the rest of kernel version, need to merge 0001 and 0002.

[Impact]
On one of the latest Lenovo laptops, the trackpoint doesn't work, we
consulted apls guys, they said this trackpoint is different from others,
it is a trackpoint-only device, so the current alps.c doesn't support it
well, while the trackpoint.c can drive it well.

[Fix]
Check if it is trakcpoint-only device, if it yes, let trackpoint.c
handle it.

[Test Case]
apply this patch and boot with the kernel, the trackpoint can work
well.

[Regression Risk]
Low. the way of detect trackpoint-only alps device is provided by
alps. so it is pretty safe. and I have tested the patch on two
lenovo machines with alsp touchpad+trackpoint device, they worked
well.


Hui Wang (2):
  Input: alps - don't handle ALPS cs19 trackpoint-only device
  Input: alps - fix a mismatch between a condition check and its comment

 drivers/input/mouse/alps.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][B/OEM-B/D/OEM-OSP1-B/E][PATCH v2 1/2] Input: alps - don't handle ALPS cs19 trackpoint-only device

Hui Wang
BugLink: https://bugs.launchpad.net/bugs/1836752

On a latest Lenovo laptop, the trackpoint and 3 buttons below it
don't work at all, when we move the trackpoint or press those 3
buttons, the kernel will print out:
"Rejected trackstick packet from non DualPoint device"

This device is identified as an alps touchpad but the packet has
trackpoint format, so the alps.c drops the packet and prints out
the message above.

According to XiaoXiao's explanation, this device is named cs19 and
is trackpoint-only device, its firmware is only for trackpoint, it
is independent of touchpad and is a device completely different from
DualPoint ones.

To drive this device with mininal changes to the existing driver, we
just let the alps driver not handle this device, then the trackpoint.c
will be the driver of this device if the trackpoint driver is enabled.
(if not, this device will fallback to a bare PS/2 device)

With the trackpoint.c, this trackpoint and 3 buttons all work well,
they have all features that the trackpoint should have, like
scrolling-screen, drag-and-drop and frame-selection.

Signed-off-by: XiaoXiao Liu <[hidden email]>
Signed-off-by: Hui Wang <[hidden email]>
Reviewed-by: Pali Rohár <[hidden email]>
Cc: [hidden email]
Signed-off-by: Dmitry Torokhov <[hidden email]>
(cherry picked from commit 7e4935ccc3236751e5fe4bd6846f86e46bb2e427
git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git)
Signed-off-by: Hui Wang <[hidden email]>
---
 drivers/input/mouse/alps.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 0a6f7ca883e7..11a4363c3b60 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -24,6 +24,7 @@
 
 #include "psmouse.h"
 #include "alps.h"
+#include "trackpoint.h"
 
 /*
  * Definitions for ALPS version 3 and 4 command mode protocol
@@ -2864,6 +2865,23 @@ static const struct alps_protocol_info *alps_match_table(unsigned char *e7,
  return NULL;
 }
 
+static bool alps_is_cs19_trackpoint(struct psmouse *psmouse)
+{
+ u8 param[2] = { 0 };
+
+ if (ps2_command(&psmouse->ps2dev,
+ param, MAKE_PS2_CMD(0, 2, TP_READ_ID)))
+ return false;
+
+ /*
+ * param[0] contains the trackpoint device variant_id while
+ * param[1] contains the firmware_id. So far all alps
+ * trackpoint-only devices have their variant_ids equal
+ * TP_VARIANT_ALPS and their firmware_ids are in 0x20~0x2f range.
+ */
+ return param[0] == TP_VARIANT_ALPS && (param[1] & 0x20);
+}
+
 static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
 {
  const struct alps_protocol_info *protocol;
@@ -3164,6 +3182,20 @@ int alps_detect(struct psmouse *psmouse, bool set_properties)
  if (error)
  return error;
 
+ /*
+ * ALPS cs19 is a trackpoint-only device, and uses different
+ * protocol than DualPoint ones, so we return -EINVAL here and let
+ * trackpoint.c drive this device. If the trackpoint driver is not
+ * enabled, the device will fall back to a bare PS/2 mouse.
+ * If ps2_command() fails here, we depend on the immediately
+ * followed psmouse_reset() to reset the device to normal state.
+ */
+ if (alps_is_cs19_trackpoint(psmouse)) {
+ psmouse_dbg(psmouse,
+    "ALPS CS19 trackpoint-only device detected, ignoring\n");
+ return -EINVAL;
+ }
+
  /*
  * Reset the device to make sure it is fully operational:
  * on some laptops, like certain Dell Latitudes, we may
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][B/OEM-B/D/OEM-OSP1-B/E][PATCH v2 2/2] Input: alps - fix a mismatch between a condition check and its comment

Hui Wang
In reply to this post by Hui Wang
BugLink: https://bugs.launchpad.net/bugs/1836752

In the function alps_is_cs19_trackpoint(), we check if the param[1] is
in the 0x20~0x2f range, but the code we wrote for this checking is not
correct:
(param[1] & 0x20) does not mean param[1] is in the range of 0x20~0x2f,
it also means the param[1] is in the range of 0x30~0x3f, 0x60~0x6f...

Now fix it with a new condition checking ((param[1] & 0xf0) == 0x20).

Fixes: 7e4935ccc323 ("Input: alps - don't handle ALPS cs19 trackpoint-only device")
Cc: [hidden email]
Signed-off-by: Hui Wang <[hidden email]>
Signed-off-by: Dmitry Torokhov <[hidden email]>
(cherry picked from commit 771a081e44a9baa1991ef011cc453ef425591740
git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git)
Signed-off-by: Hui Wang <[hidden email]>
---
 drivers/input/mouse/alps.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 11a4363c3b60..dd80ff6cc427 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -2879,7 +2879,7 @@ static bool alps_is_cs19_trackpoint(struct psmouse *psmouse)
  * trackpoint-only devices have their variant_ids equal
  * TP_VARIANT_ALPS and their firmware_ids are in 0x20~0x2f range.
  */
- return param[0] == TP_VARIANT_ALPS && (param[1] & 0x20);
+ return param[0] == TP_VARIANT_ALPS && ((param[1] & 0xf0) == 0x20);
 }
 
 static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK/cmt / APPLIED[E]: [SRU][B/OEM-B/D/OEM-OSP1-B/E][PATCH v2 0/2] input/mouse: alps trackpoint-only device doesn't work

Seth Forshee
In reply to this post by Hui Wang
On Fri, Jul 19, 2019 at 10:39:15PM +0800, Hui Wang wrote:

> BugLink: https://bugs.launchpad.net/bugs/1836752
>
> In the V2:
> I added one more patch (0002) to fix the problem of condition checking
> of 0x20~0x2f (apw found this problem in the v1)
>
> Since the OEM-OSP1-B already merged 0001, it only needs to merge 0002
> this time, for the rest of kernel version, need to merge 0001 and 0002.
>
> [Impact]
> On one of the latest Lenovo laptops, the trackpoint doesn't work, we
> consulted apls guys, they said this trackpoint is different from others,
> it is a trackpoint-only device, so the current alps.c doesn't support it
> well, while the trackpoint.c can drive it well.
>
> [Fix]
> Check if it is trakcpoint-only device, if it yes, let trackpoint.c
> handle it.
>
> [Test Case]
> apply this patch and boot with the kernel, the trackpoint can work
> well.
>
> [Regression Risk]
> Low. the way of detect trackpoint-only alps device is provided by
> alps. so it is pretty safe. and I have tested the patch on two
> lenovo machines with alsp touchpad+trackpoint device, they worked
> well.

Patches from a maintainer tree are sauce patches. With the commit
description fixed to reflect this:

Acked-by: Seth Forshee <[hidden email]>

Applied to eoan/master-next, with the subject changed to add "UBUNTU:
SAUCE:", thanks!

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK/Cmnt: [SRU][B/D][PATCH v2 0/2] input/mouse: alps trackpoint-only device doesn't work

Stefan Bader-2
In reply to this post by Hui Wang
On 19.07.19 16:39, Hui Wang wrote:

> BugLink: https://bugs.launchpad.net/bugs/1836752
>
> In the V2:
> I added one more patch (0002) to fix the problem of condition checking
> of 0x20~0x2f (apw found this problem in the v1)
>
> Since the OEM-OSP1-B already merged 0001, it only needs to merge 0002
> this time, for the rest of kernel version, need to merge 0001 and 0002.
>
> [Impact]
> On one of the latest Lenovo laptops, the trackpoint doesn't work, we
> consulted apls guys, they said this trackpoint is different from others,
> it is a trackpoint-only device, so the current alps.c doesn't support it
> well, while the trackpoint.c can drive it well.
>
> [Fix]
> Check if it is trakcpoint-only device, if it yes, let trackpoint.c
> handle it.
>
> [Test Case]
> apply this patch and boot with the kernel, the trackpoint can work
> well.
>
> [Regression Risk]
> Low. the way of detect trackpoint-only alps device is provided by
> alps. so it is pretty safe. and I have tested the patch on two
> lenovo machines with alsp touchpad+trackpoint device, they worked
> well.
>
>
> Hui Wang (2):
commit 7e4935ccc3236751e5fe4bd6846f86e46bb2e427
>   Input: alps - don't handle ALPS cs19 trackpoint-only device
commit 771a081e44a9baa1991ef011cc453ef425591740
>   Input: alps - fix a mismatch between a condition check and its comment
>
Patches are now upstream, so with updated sha1 reference:


Acked-by: Stefan Bader <[hidden email]>
>  drivers/input/mouse/alps.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

APPLIED(B,D)/cmt: [SRU][B/OEM-B/D/OEM-OSP1-B/E][PATCH v2 0/2] input/mouse: alps trackpoint-only device doesn't work

Khaled Elmously
In reply to this post by Hui Wang
Applied to Disco only, after updating the SHA references as Stefan said. Bug wasn't targeted for D so I targeted it.

Both patches were already present in Bionic via stable-updates, so marking the thread as applied to B as well.



On 2019-07-19 22:39:15 , Hui Wang wrote:

> BugLink: https://bugs.launchpad.net/bugs/1836752
>
> In the V2:
> I added one more patch (0002) to fix the problem of condition checking
> of 0x20~0x2f (apw found this problem in the v1)
>
> Since the OEM-OSP1-B already merged 0001, it only needs to merge 0002
> this time, for the rest of kernel version, need to merge 0001 and 0002.
>
> [Impact]
> On one of the latest Lenovo laptops, the trackpoint doesn't work, we
> consulted apls guys, they said this trackpoint is different from others,
> it is a trackpoint-only device, so the current alps.c doesn't support it
> well, while the trackpoint.c can drive it well.
>
> [Fix]
> Check if it is trakcpoint-only device, if it yes, let trackpoint.c
> handle it.
>
> [Test Case]
> apply this patch and boot with the kernel, the trackpoint can work
> well.
>
> [Regression Risk]
> Low. the way of detect trackpoint-only alps device is provided by
> alps. so it is pretty safe. and I have tested the patch on two
> lenovo machines with alsp touchpad+trackpoint device, they worked
> well.
>
>
> Hui Wang (2):
>   Input: alps - don't handle ALPS cs19 trackpoint-only device
>   Input: alps - fix a mismatch between a condition check and its comment
>
>  drivers/input/mouse/alps.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> --
> 2.17.1
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team