diff mbox series

[v5] Input: alps - Don't handle ALPS cs19 trackpoint-only device

Message ID 20190619063756.9714-1-hui.wang@canonical.com (mailing list archive)
State Superseded
Headers show
Series [v5] Input: alps - Don't handle ALPS cs19 trackpoint-only device | expand

Commit Message

Hui Wang June 19, 2019, 6:37 a.m. UTC
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 <sliuuxiaonxiao@gmail.com>
Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
In the v5:
change the commit header to add "fallback to a bare PS/2 device if
trackpont driver is not enabled".
add comment for checking param[0] and param[1]
add psmouse_dbg and psmouse_warn respectively for PS2_TRACKPOINT is
enabled or not enabled.

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

Comments

Pali Rohár June 19, 2019, 7:29 a.m. UTC | #1
On Wednesday 19 June 2019 14:37:56 Hui Wang wrote:
> 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 <sliuuxiaonxiao@gmail.com>
> Signed-off-by: Hui Wang <hui.wang@canonical.com>

Looks good, you can add my:

Reviewed-by: Pali Rohár <pali.rohar@gmail.com>

Thanks!

> ---
> In the v5:
> change the commit header to add "fallback to a bare PS/2 device if
> trackpont driver is not enabled".
> add comment for checking param[0] and param[1]
> add psmouse_dbg and psmouse_warn respectively for PS2_TRACKPOINT is
> enabled or not enabled.
> 
>  drivers/input/mouse/alps.c | 41 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 0a6f7ca883e7..536b8e531169 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,34 @@ 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 for all alps trackpoint-only
> +	 * devices, their variant_ids equal TP_VARIANT_ALPS and their
> +	 * firmware_ids are 0x20~0x2f, so here we check param[0] as well as
> +	 * param[1] to detect an ALPS trackpoint-only device.
> +	 */
> +	if ((param[0] == TP_VARIANT_ALPS) && (param[1] & 0x20)) {
> +		if (IS_ENABLED(MOUSE_PS2_TRACKPOINT))
> +			psmouse_dbg(psmouse,
> +				    "ALPS CS19 trackpoint-only device detected, not using ALPS touchpad driver\n");
> +		else
> +			psmouse_warn(psmouse,
> +				     "ALPS CS19 trackpoint-only device detected but MOUSE_PS2_TRACKPOINT not enabled, fallback to bare PS/2 mouse\n");
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
>  static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
>  {
>  	const struct alps_protocol_info *protocol;
> @@ -3164,6 +3193,18 @@ int alps_detect(struct psmouse *psmouse, bool set_properties)
>  	if (error)
>  		return error;
>  
> +	/*
> +	 * ALPS cs19 is a trackpoint-only device, it is completely independent
> +	 * of touchpad. So it is a device different from DualPoint ones, if it
> +	 * is identified as a cs19 trackpoint device, we return -EINVAL here and
> +	 * let trackpoint.c to drive this device, if the trackpoint driver is
> +	 * not enabled, the device will fallback to a bare PS/2 mouse.
> +	 * If ps2_command() fails here, we depend on the immediate followed
> +	 * psmouse_reset() to reset the device to normal state.
> +	 */
> +	if (alps_is_cs19_trackpoint(psmouse))
> +		return -EINVAL;
> +
>  	/*
>  	 * Reset the device to make sure it is fully operational:
>  	 * on some laptops, like certain Dell Latitudes, we may
Hui Wang June 19, 2019, 8:11 a.m. UTC | #2
On 2019/6/19 下午3:29, Pali Rohár wrote:
> On Wednesday 19 June 2019 14:37:56 Hui Wang wrote:
>> 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 <sliuuxiaonxiao@gmail.com>
>> Signed-off-by: Hui Wang <hui.wang@canonical.com>
> Looks good, you can add my:
>
> Reviewed-by: Pali Rohár <pali.rohar@gmail.com>
>
> Thanks!
>
Thank you Pali.

And Dmitry, suppose this patch is ok to merge, do I need to send a new 
patch to add "Reviewed-by: Pali Rohár <pali.rohar@gmail.com>" or you 
will add it yourself.

Thanks,

Hui.

>> ---
>> In the v5:
>> change the commit header to add "fallback to a bare PS/2 device if
>
Hui Wang June 24, 2019, 3:43 a.m. UTC | #3
Hi Dmitry,

When you have time, could you please take a look at this patch?

Thanks,

Hui.

On 2019/6/19 下午4:11, Hui Wang wrote:
>
> On 2019/6/19 下午3:29, Pali Rohár wrote:
>> On Wednesday 19 June 2019 14:37:56 Hui Wang wrote:
>>> 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 <sliuuxiaonxiao@gmail.com>
>>> Signed-off-by: Hui Wang <hui.wang@canonical.com>
>> Looks good, you can add my:
>>
>> Reviewed-by: Pali Rohár <pali.rohar@gmail.com>
>>
>> Thanks!
>>
> Thank you Pali.
>
> And Dmitry, suppose this patch is ok to merge, do I need to send a new 
> patch to add "Reviewed-by: Pali Rohár <pali.rohar@gmail.com>" or you 
> will add it yourself.
>
> Thanks,
>
> Hui.
>
>>> ---
>>> In the v5:
>>> change the commit header to add "fallback to a bare PS/2 device if
>>
diff mbox series

Patch

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 0a6f7ca883e7..536b8e531169 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,34 @@  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 for all alps trackpoint-only
+	 * devices, their variant_ids equal TP_VARIANT_ALPS and their
+	 * firmware_ids are 0x20~0x2f, so here we check param[0] as well as
+	 * param[1] to detect an ALPS trackpoint-only device.
+	 */
+	if ((param[0] == TP_VARIANT_ALPS) && (param[1] & 0x20)) {
+		if (IS_ENABLED(MOUSE_PS2_TRACKPOINT))
+			psmouse_dbg(psmouse,
+				    "ALPS CS19 trackpoint-only device detected, not using ALPS touchpad driver\n");
+		else
+			psmouse_warn(psmouse,
+				     "ALPS CS19 trackpoint-only device detected but MOUSE_PS2_TRACKPOINT not enabled, fallback to bare PS/2 mouse\n");
+		return true;
+	}
+
+	return false;
+}
+
 static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
 {
 	const struct alps_protocol_info *protocol;
@@ -3164,6 +3193,18 @@  int alps_detect(struct psmouse *psmouse, bool set_properties)
 	if (error)
 		return error;
 
+	/*
+	 * ALPS cs19 is a trackpoint-only device, it is completely independent
+	 * of touchpad. So it is a device different from DualPoint ones, if it
+	 * is identified as a cs19 trackpoint device, we return -EINVAL here and
+	 * let trackpoint.c to drive this device, if the trackpoint driver is
+	 * not enabled, the device will fallback to a bare PS/2 mouse.
+	 * If ps2_command() fails here, we depend on the immediate followed
+	 * psmouse_reset() to reset the device to normal state.
+	 */
+	if (alps_is_cs19_trackpoint(psmouse))
+		return -EINVAL;
+
 	/*
 	 * Reset the device to make sure it is fully operational:
 	 * on some laptops, like certain Dell Latitudes, we may