Hi!

On Wednesday 22 May 2019 02:53:18 Xiaoxiao Liu wrote:

Hi Pali,

Why it does not report input data?
--> The alps devices which detected to use the ALPS_PROTO_V8  contains ALPS touchpad and ALPS track point. 
     But the ALPS_PROTO_V8 do not support the track point device process. 
     When the track point was detected to use ALPS_PROTO_V8 ,the v8 process_packet method  alps_process_packet_ss4_v2 will reject to report the data because the v8 device is      not set the ALPS_DUALPOINT flag. 
     You can refer below code.

Ok, and cannot you set ALPS_DUALPOINT flag based on that
alps_check_is_trackpoint() result and then update
alps_process_packet_ss4_v3() code to supports also
V8 trackpoint packets?

	/* Report trackstick */
	if (alps_get_pkt_id_ss4_v2(packet) == SS4_PACKET_ID_STICK) {
		if (!(priv->flags & ALPS_DUALPOINT)) {
			psmouse_warn(psmouse,
				     "Rejected trackstick packet from non DualPoint device");
			return;
		}
		...
		return;
	}

why is for non-ALPS trackpoints used ALPS driver?
--> the non-Alps track point is the ALPS touchpad, it is right to use the ALPS driver for ALPS touchpad.

Ok, now I got it.

The v8 protocol condition may modified  as below, it will be more easier to understand.

	 if (e7[0] == 0x73 && e7[1] == 0x03 && (e7[2] == 0x14 || e7[2] == 0x28) &&  (alps_check_is_trackpoint(psmouse) != 0)  ) {
 		protocol = &alps_v8_protocol_data;
	}

Yes, this would be more easier to understand.

Anyway, more information should be in commit message.

Best Regards
XiaoXiao Liu
sliuuxiaonxiao@gmail.com
xiaoxiao.liu-1@cn.alps.com

-----邮件原件-----
发件人: Pali Rohár [off-list ref] 
发送时间: Tuesday, May 21, 2019 5:46 PM
收件人: Hui Wang [off-list ref]
抄送: 劉 曉曉 Xiaoxiao Liu [off-list ref]; XiaoXiao Liu [off-list ref]; dmitry.torokhov@gmail.com; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; 曹 曉建 Xiaojian Cao [off-list ref]; zhangfp1@lenovo.com
主题: Re: 答复: [PATCH] input: alps-fix the issue the special alps trackpoint do not work.

Hello!

On Tuesday 21 May 2019 10:26:47 Hui Wang wrote:

quoted

Tested-by: Hui Wang <redacted>

On 2019/5/21 上午9:07, Xiaoxiao Liu wrote:

quoted

Add Pali Rohár.

-----邮件原件-----
发件人: XiaoXiao Liu [off-list ref]
发送时间: Monday, May 20, 2019 7:02 PM
收件人: dmitry.torokhov@gmail.com
抄送: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; 
hui.wang@canonical.com; 曹 曉建 Xiaojian Cao 
[off-list ref]; zhangfp1@lenovo.com; 劉 曉曉 Xiaoxiao Liu 
[off-list ref]; XiaoXiao Liu 
[off-list ref]
主题: [PATCH] input: alps-fix the issue the special alps trackpoint do not work.

when the alps trackpoint is detected and using the 
alps_v8_protocol_data procotol, the alps driver will not report the input data.

Why it does not report input data?

quoted

quoted

solution: use standard mouse driver instead of alps driver when the specail trackpoint was detected.

This looks like an (undocumented) hack or workaround. Not a solution.

quoted

quoted

Signed-off-by: XiaoXiao Liu <redacted>
---
  drivers/input/mouse/alps.c | 23 ++++++++++++++++++++++-
  1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c 
index 0a6f7ca883e7..516ae1d0eb17 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c

@@ -24,7 +24,7 @@
  #include "psmouse.h"
  #include "alps.h"
-
+#include "trackpoint.h"
  /*
   * Definitions for ALPS version 3 and 4 command mode protocol
   */

@@ -2864,6 +2864,22 @@ static const struct alps_protocol_info *alps_match_table(unsigned char *e7,
  	return NULL;
  }
+int alps_check_is_trackpoint(struct psmouse *psmouse) {
+	u8 param[2] = { 0 };
+	int error;
+
+	error = ps2_command(&psmouse->ps2dev,
+			    param, MAKE_PS2_CMD(0, 2, TP_READ_ID));
+	if (error)
+		return error;
+
+	if (param[0] == TP_VARIANT_ALPS)
+		return 0;
+	psmouse_warn(psmouse, "It is not alps trackpoint.\n");
+	return -ENODEV;
+}

So, this function returns 0 when detected ALPS trackpoint and -ENODEV when not.

quoted

quoted

+
  static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)  {
  	const struct alps_protocol_info *protocol; @@ -2912,6 +2928,11 @@ static int alps_identify(struct psmouse *psmouse, struct alps_data *priv)
  			protocol = &alps_v3_protocol_data;
  		} else if (e7[0] == 0x73 && e7[1] == 0x03 &&
  			   (e7[2] == 0x14 || e7[2] == 0x28)) {
+				if (alps_check_is_trackpoint(psmouse) == 0) {
+					psmouse_warn(psmouse,
+					"It is alps trackpoint, use the standard mouse driver.\n");
+					return -EINVAL;

And here I'm lost. If we have *not* detected ALPS trackpoint then if block is not called which means that ALPS driver is used.

So why is for non-ALPS trackpoints used ALPS driver? This does not seem like a correct...

And when we have detected ALPS trackpoint (return value 0) then standard mouse driver is used and returned -EINVAL. This seems strange too.

So either this code is wrong or there are missing more details for explaining this strange logic. But still this looks like a hack not a proper fix/solution.

quoted

quoted

+				}
  			protocol = &alps_v8_protocol_data;
  		} else if (e7[0] == 0x73 && e7[1] == 0x03 && e7[2] == 0xc8) {
  			protocol = &alps_v9_protocol_data;
--
2.20.1

--
Pali Rohár
pali.rohar@gmail.com

-- 
Pali Rohár
pali.rohar@gmail.com