input: alps-fix the issue alps cs19 trackstick do not work.

Commit Message

XiaoXiao Liu May 27, 2019, 9:44 a.m. UTC

The alps devices which detected to use the ALPS_PROTO_V8 procotol contains
ALPS touchpad and ALPS trackstick.The ALPS_PROTO_V8 procotol do not
support the trackstick device process by default.

When the trackstick was detected to use ALPS_PROTO_V8 procotol,
the v8 process_packet method alps_process_packet_ss4_v2 will reject to
report the data when the device using ALPS_PROTO_V8 procotol is not set
the ALPS_DUALPOINT flag.

The alps cs19 trackstick is detected to use the ALPS_PROTO_V8 procotol
but without ALPS_DUALPOINT flag, the alps driver will not report the
input data. so the trackstick will not work.

solution: when the alps cs19 device detected, set the device
ALPS_DUALPOINT flag,then the input data will be processed.

Signed-off-by: XiaoXiao Liu <[e-mail]>
---
 drivers/input/mouse/alps.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

Comments

Pali Rohár May 27, 2019, 10:09 a.m. UTC | #1

Hi!

On Monday 27 May 2019 05:44:22 XiaoXiao Liu wrote:
> The alps devices which detected to use the ALPS_PROTO_V8 procotol contains
> ALPS touchpad and ALPS trackstick.The ALPS_PROTO_V8 procotol do not
> support the trackstick device process by default.

Normally PS/2 device handled by alps.c is touchpad and in some cases
touchpad sends also trackstick data in that one PS/2 channel.

Does it mean that your device (reported to kernel) sends only trackstick
packets and not touchpad?

> When the trackstick was detected to use ALPS_PROTO_V8 procotol,
> the v8 process_packet method alps_process_packet_ss4_v2 will reject to
> report the data when the device using ALPS_PROTO_V8 procotol is not set
> the ALPS_DUALPOINT flag.
> 
> The alps cs19 trackstick is detected to use the ALPS_PROTO_V8 procotol
> but without ALPS_DUALPOINT flag, the alps driver will not report the
> input data. so the trackstick will not work.
> 
> solution: when the alps cs19 device detected, set the device
> ALPS_DUALPOINT flag,then the input data will be processed.
> 
> Signed-off-by: XiaoXiao Liu <[e-mail]>
> ---
>  drivers/input/mouse/alps.c | 25 +++++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 0a6f7ca883e7..a54677cf7474 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
>   */
> @@ -220,6 +220,23 @@ static bool alps_is_valid_first_byte(struct alps_data *priv,
>  	return (data & priv->mask0) == priv->byte0;
>  }
>  
> +static int alps_check_cs19_trackstick(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 && param[1] & 0x20) {

I guess that you want parenthesis around (param[1] & 0x20). And also
describe what that 0x20 constant means.

> +		psmouse_warn(psmouse, "It is alps cs19 trackstick");

It is not a warning.

> +		return 0;
> +	}
> +	return -1;
> +}
> +
>  static void alps_report_buttons(struct input_dev *dev1, struct input_dev *dev2,
>  				int left, int right, int middle)
>  {
> @@ -2568,8 +2585,12 @@ static int alps_update_dual_info_ss4_v2(unsigned char otp[][4],
>  			alps_exit_command_mode(psmouse);
>  			ps2_command(ps2dev, NULL, PSMOUSE_CMD_ENABLE);
>  
> -			if (reg_val == 0x0C || reg_val == 0x1D)
> +			if (reg_val == 0x0C || reg_val == 0x1D) {

Hm... why your device does not match these constants?

> +				is_dual = true;
> +			} else if (alps_check_cs19_trackstick(psmouse) == 0) {
> +				//For support Thinkpad CS19 TrackStick
>  				is_dual = true;
> +			}
>  		}
>  	}
>

Xiaoxiao Liu May 28, 2019, 1:37 a.m. UTC | #2

Add Saito-san.

Hi Hui,
Does it mean that your device (reported to kernel) sends only trackstick packets and not touchpad?
   	-> Yes.
I guess that you want parenthesis around (param[1] & 0x20). And also describe what that 0x20 constant means.
It is not a warning.
	-> Yes, it should be (param[1] & 0x20). 
	-> 0x20 is used for detect which type device is. I will correct it.

Hm... why your device does not match these constants?
	->I am not clear what the alps_command_mode_read_reg(psmouse, 0xD7) used for.
              -> But I know our device did not meet the condition if (reg_val == 0x0C || reg_val == 0x1D) from the                running result.

Xiaoxiao Liu
[e-mail]
[e-mail]
-----邮件原件-----
发件人: Pali Rohár <[e-mail]> 
发送时间: Monday, May 27, 2019 6:09 PM
收件人: XiaoXiao Liu <[e-mail]>
抄送: [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; 曹 曉建 Xiaojian Cao <[e-mail]>; [e-mail]; 劉 曉曉 Xiaoxiao Liu <[e-mail]>
主题: Re: [PATCH] input: alps-fix the issue alps cs19 trackstick do not work.

Hi!

On Monday 27 May 2019 05:44:22 XiaoXiao Liu wrote:
> The alps devices which detected to use the ALPS_PROTO_V8 procotol 
> contains ALPS touchpad and ALPS trackstick.The ALPS_PROTO_V8 procotol 
> do not support the trackstick device process by default.

Normally PS/2 device handled by alps.c is touchpad and in some cases touchpad sends also trackstick data in that one PS/2 channel.

Does it mean that your device (reported to kernel) sends only trackstick packets and not touchpad?

> When the trackstick was detected to use ALPS_PROTO_V8 procotol, the v8 
> process_packet method alps_process_packet_ss4_v2 will reject to report 
> the data when the device using ALPS_PROTO_V8 procotol is not set the 
> ALPS_DUALPOINT flag.
> 
> The alps cs19 trackstick is detected to use the ALPS_PROTO_V8 procotol 
> but without ALPS_DUALPOINT flag, the alps driver will not report the 
> input data. so the trackstick will not work.
> 
> solution: when the alps cs19 device detected, set the device 
> ALPS_DUALPOINT flag,then the input data will be processed.
> 
> Signed-off-by: XiaoXiao Liu <[e-mail]>
> ---
>  drivers/input/mouse/alps.c | 25 +++++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c 
> index 0a6f7ca883e7..a54677cf7474 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
>   */
> @@ -220,6 +220,23 @@ static bool alps_is_valid_first_byte(struct alps_data *priv,
>  	return (data & priv->mask0) == priv->byte0;  }
>  
> +static int alps_check_cs19_trackstick(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 && param[1] & 0x20) {

I guess that you want parenthesis around (param[1] & 0x20). And also describe what that 0x20 constant means.

> +		psmouse_warn(psmouse, "It is alps cs19 trackstick");

It is not a warning.

> +		return 0;
> +	}
> +	return -1;
> +}
> +
>  static void alps_report_buttons(struct input_dev *dev1, struct input_dev *dev2,
>  				int left, int right, int middle)
>  {
> @@ -2568,8 +2585,12 @@ static int alps_update_dual_info_ss4_v2(unsigned char otp[][4],
>  			alps_exit_command_mode(psmouse);
>  			ps2_command(ps2dev, NULL, PSMOUSE_CMD_ENABLE);
>  
> -			if (reg_val == 0x0C || reg_val == 0x1D)
> +			if (reg_val == 0x0C || reg_val == 0x1D) {

Hm... why your device does not match these constants?

> +				is_dual = true;
> +			} else if (alps_check_cs19_trackstick(psmouse) == 0) {
> +				//For support Thinkpad CS19 TrackStick
>  				is_dual = true;
> +			}
>  		}
>  	}
>  

--
Pali Rohár
[e-mail]

Pali Rohár May 28, 2019, 7:18 a.m. UTC | #3

On Tuesday 28 May 2019 01:37:14 Xiaoxiao Liu wrote:
> Add Saito-san.
> 
> Hi Hui,
> Does it mean that your device (reported to kernel) sends only trackstick packets and not touchpad?
>    	-> Yes.

Ok, I think this answers all questions.

So your patch is not correct as it registers "fake" touchpad device even
there is no touchpad at all.

You should fix your patch to not register touchpad input device, in your
case it should register only trackstick device. I suggest to add some
flag which would indicate such device (e.g. ALPS_ONLY_TRACKSTICK).

Also currently kernel exports following names when device has both
trackstick and touchpad: "DualPoint Stick" and "DualPoint TouchPad".
And it exports name "GlidePoint" for touchpad-only device. So to be
consistent you need to also modify this code for trackstick-only device.

Xiaoxiao Liu May 28, 2019, 7:55 a.m. UTC | #4

Add Kawase-san.

-----邮件原件-----
发件人: Pali Rohár <[e-mail]> 
发送时间: Tuesday, May 28, 2019 3:18 PM
收件人: 劉 曉曉 Xiaoxiao Liu <[e-mail]>
抄送: XiaoXiao Liu <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; 曹 曉建 Xiaojian Cao <[e-mail]>; [e-mail]; 斉藤 直樹 Naoki Saito <[e-mail]>
主题: Re: 答复: [PATCH] input: alps-fix the issue alps cs19 trackstick do not work.

On Tuesday 28 May 2019 01:37:14 Xiaoxiao Liu wrote:
> Add Saito-san.
> 
> Hi Hui,
> Does it mean that your device (reported to kernel) sends only trackstick packets and not touchpad?
>    	-> Yes.

Ok, I think this answers all questions.

So your patch is not correct as it registers "fake" touchpad device even there is no touchpad at all.

You should fix your patch to not register touchpad input device, in your case it should register only trackstick device. I suggest to add some flag which would indicate such device (e.g. ALPS_ONLY_TRACKSTICK).

Also currently kernel exports following names when device has both trackstick and touchpad: "DualPoint Stick" and "DualPoint TouchPad".
And it exports name "GlidePoint" for touchpad-only device. So to be consistent you need to also modify this code for trackstick-only device.

--
Pali Rohár
[e-mail]

Xiaoxiao Liu June 10, 2019, 9:20 a.m. UTC | #5

Hi Pali,

We register our CS19 device as ALPS_ONLY_TRACKSTICK device.
And let the V8 protocol function support the process of ALPS_ONLY_TRACKSTICK device. 

I want to confirm if this solution OK?

Xiaoxiao.Liu
-----邮件原件-----
发件人: 劉 曉曉 Xiaoxiao Liu 
发送时间: Tuesday, May 28, 2019 3:55 PM
收件人: Pali Rohár <[e-mail]>
抄送: XiaoXiao Liu <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; 曹 曉建 Xiaojian Cao <[e-mail]>; [e-mail]; 斉藤 直樹 Naoki Saito <[e-mail]>; 川瀬 英夫 Hideo Kawase <[e-mail]>
主题: 答复: 答复: [PATCH] input: alps-fix the issue alps cs19 trackstick do not work.

Add Kawase-san.

-----邮件原件-----
发件人: Pali Rohár <[e-mail]> 
发送时间: Tuesday, May 28, 2019 3:18 PM
收件人: 劉 曉曉 Xiaoxiao Liu <[e-mail]>
抄送: XiaoXiao Liu <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; 曹 曉建 Xiaojian Cao <[e-mail]>; [e-mail]; 斉藤 直樹 Naoki Saito <[e-mail]>
主题: Re: 答复: [PATCH] input: alps-fix the issue alps cs19 trackstick do not work.

On Tuesday 28 May 2019 01:37:14 Xiaoxiao Liu wrote:
> Add Saito-san.
> 
> Hi Hui,
> Does it mean that your device (reported to kernel) sends only trackstick packets and not touchpad?
>    	-> Yes.

Ok, I think this answers all questions.

So your patch is not correct as it registers "fake" touchpad device even there is no touchpad at all.

You should fix your patch to not register touchpad input device, in your case it should register only trackstick device. I suggest to add some flag which would indicate such device (e.g. ALPS_ONLY_TRACKSTICK).

Also currently kernel exports following names when device has both trackstick and touchpad: "DualPoint Stick" and "DualPoint TouchPad".
And it exports name "GlidePoint" for touchpad-only device. So to be consistent you need to also modify this code for trackstick-only device.

--
Pali Rohár
[e-mail]

Xiaoxiao Liu June 10, 2019, 10:03 a.m. UTC | #6

++ Saito-san.

-----邮件原件-----
发件人: 劉 曉曉 Xiaoxiao Liu 
发送时间: Monday, June 10, 2019 5:20 PM
收件人: Pali Rohár <[e-mail]>
抄送: XiaoXiao Liu <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; 曹 曉建 Xiaojian Cao <[e-mail]>; [e-mail]; 斉藤 直樹 Naoki Saito <[e-mail]>; 川瀬 英夫 Hideo Kawase <[e-mail]>
主题: 答复: 答复: [PATCH] input: alps-fix the issue alps cs19 trackstick do not work.

Hi Pali,

We register our CS19 device as ALPS_ONLY_TRACKSTICK device.
And let the V8 protocol function support the process of ALPS_ONLY_TRACKSTICK device. 

I want to confirm if this solution OK?

Xiaoxiao.Liu
-----邮件原件-----
发件人: 劉 曉曉 Xiaoxiao Liu 
发送时间: Tuesday, May 28, 2019 3:55 PM
收件人: Pali Rohár <[e-mail]>
抄送: XiaoXiao Liu <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; 曹 曉建 Xiaojian Cao <[e-mail]>; [e-mail]; 斉藤 直樹 Naoki Saito <[e-mail]>; 川瀬 英夫 Hideo Kawase <[e-mail]>
主题: 答复: 答复: [PATCH] input: alps-fix the issue alps cs19 trackstick do not work.

Add Kawase-san.

-----邮件原件-----
发件人: Pali Rohár <[e-mail]> 
发送时间: Tuesday, May 28, 2019 3:18 PM
收件人: 劉 曉曉 Xiaoxiao Liu <[e-mail]>
抄送: XiaoXiao Liu <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; 曹 曉建 Xiaojian Cao <[e-mail]>; [e-mail]; 斉藤 直樹 Naoki Saito <[e-mail]>
主题: Re: 答复: [PATCH] input: alps-fix the issue alps cs19 trackstick do not work.

On Tuesday 28 May 2019 01:37:14 Xiaoxiao Liu wrote:
> Add Saito-san.
> 
> Hi Hui,
> Does it mean that your device (reported to kernel) sends only trackstick packets and not touchpad?
>    	-> Yes.

Ok, I think this answers all questions.

So your patch is not correct as it registers "fake" touchpad device even there is no touchpad at all.

You should fix your patch to not register touchpad input device, in your case it should register only trackstick device. I suggest to add some flag which would indicate such device (e.g. ALPS_ONLY_TRACKSTICK).

Also currently kernel exports following names when device has both trackstick and touchpad: "DualPoint Stick" and "DualPoint TouchPad".
And it exports name "GlidePoint" for touchpad-only device. So to be consistent you need to also modify this code for trackstick-only device.

--
Pali Rohár
[e-mail]

Pali Rohár June 10, 2019, 10:43 a.m. UTC | #7

On Monday 10 June 2019 10:03:51 Xiaoxiao Liu wrote:
> Hi Pali,

Hi!

> We register our CS19 device as ALPS_ONLY_TRACKSTICK device.
> And let the V8 protocol function support the process of ALPS_ONLY_TRACKSTICK device. 
> 
> I want to confirm if this solution OK?

Yes, it is fine. Just make sure that touchapad input device is not
registered when this ALPS_ONLY_TRACKSTICK flag is set. So userspace
would not see any fake/unavailable touchpad input device.

> Xiaoxiao.Liu

Xiaoxiao Liu June 11, 2019, 3:05 a.m. UTC | #8

Hi Pali,

I discussed with our FW team about this problem.
We think the V8 method means a touchpad feature  and does not fit the CS19 trackpoint device. 
CS19 TrackPoint needn't  use any Absolute (Raw) mode and is usually use standard mouse data.
CS19 TrackPoint device is a completely different device with DualPoint device of Dell/HP.
CS19 TrackPoint device is independent  of Touchpad. (Touchpad is connecting by I2C, TrackPoint is directly connecting with PS2 port.)
And it has completely another FW. 

So we think it is better to use the mouse mode for CS19 trackpoint.

Best Regards
Shona
-----邮件原件-----
发件人: Pali Rohár <[e-mail]> 
发送时间: Monday, June 10, 2019 6:43 PM
收件人: 劉 曉曉 Xiaoxiao Liu <[e-mail]>
抄送: XiaoXiao Liu <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; 曹 曉建 Xiaojian Cao <[e-mail]>; [e-mail]; 斉藤 直樹 Naoki Saito <[e-mail]>; 川瀬 英夫 Hideo Kawase <[e-mail]>
主题: Re: 答复: 答复: [PATCH] input: alps-fix the issue alps cs19 trackstick do not work.

On Monday 10 June 2019 10:03:51 Xiaoxiao Liu wrote:
> Hi Pali,

Hi!

> We register our CS19 device as ALPS_ONLY_TRACKSTICK device.
> And let the V8 protocol function support the process of ALPS_ONLY_TRACKSTICK device. 
> 
> I want to confirm if this solution OK?

Yes, it is fine. Just make sure that touchapad input device is not registered when this ALPS_ONLY_TRACKSTICK flag is set. So userspace would not see any fake/unavailable touchpad input device.

> Xiaoxiao.Liu

--
Pali Rohár
[e-mail]

Hui Wang June 11, 2019, 3:23 a.m. UTC | #9

On 2019/6/11 上午11:05, Xiaoxiao Liu wrote:
> Hi Pali,
>
> I discussed with our FW team about this problem.
> We think the V8 method means a touchpad feature  and does not fit the CS19 trackpoint device.
> CS19 TrackPoint needn't  use any Absolute (Raw) mode and is usually use standard mouse data.
> CS19 TrackPoint device is a completely different device with DualPoint device of Dell/HP.
> CS19 TrackPoint device is independent  of Touchpad. (Touchpad is connecting by I2C, TrackPoint is directly connecting with PS2 port.)
> And it has completely another FW.
>
> So we think it is better to use the mouse mode for CS19 trackpoint.

Maybe here is some mis-understanding,  the mouse mode here doesn't mean 
we use psmouse-base.c for cs19 (bare ps/2 mouse), we plan to use 
trackpoint.c to drive this HW, so this trackpoint has all features a 
trackpoint should have.

Regards,

Hui.

>
> Best Regards
> Shona
> -----邮件原件-----
> 发件人: Pali Rohár <[e-mail]>
> 发送时间: Monday, June 10, 2019 6:43 PM
> 收件人: 劉 曉曉 Xiaoxiao Liu <[e-mail]>
> 抄送: XiaoXiao Liu <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; [e-mail]; [e-mail]; 曹 曉建 Xiaojian Cao <[e-mail]>; [e-mail]; 斉藤 直樹 Naoki Saito <[e-mail]>; 川瀬 英夫 Hideo Kawase <[e-mail]>
> 主题: Re: 答复: 答复: [PATCH] input: alps-fix the issue alps cs19 trackstick do not work.
>
> On Monday 10 June 2019 10:03:51 Xiaoxiao Liu wrote:
>> Hi Pali,
> Hi!
>
>> We register our CS19 device as ALPS_ONLY_TRACKSTICK device.
>> And let the V8 protocol function support the process of ALPS_ONLY_TRACKSTICK device.
>>
>> I want to confirm if this solution OK?
> Yes, it is fine. Just make sure that touchapad input device is not registered when this ALPS_ONLY_TRACKSTICK flag is set. So userspace would not see any fake/unavailable touchpad input device.
>
>> Xiaoxiao.Liu
> --
> Pali Rohár
> [e-mail]

Hui Wang June 11, 2019, 4:32 a.m. UTC | #10

On 2019/6/11 上午11:23, Hui Wang wrote:
> On 2019/6/11 上午11:05, Xiaoxiao Liu wrote:
>> Hi Pali,
>>
>> I discussed with our FW team about this problem.
>> We think the V8 method means a touchpad feature  and does not fit the 
>> CS19 trackpoint device.
>> CS19 TrackPoint needn't  use any Absolute (Raw) mode and is usually 
>> use standard mouse data.
>> CS19 TrackPoint device is a completely different device with 
>> DualPoint device of Dell/HP.
>> CS19 TrackPoint device is independent  of Touchpad. (Touchpad is 
>> connecting by I2C, TrackPoint is directly connecting with PS2 port.)
>> And it has completely another FW.
>>
>> So we think it is better to use the mouse mode for CS19 trackpoint.
>
> Maybe here is some mis-understanding,  the mouse mode here doesn't 
> mean we use psmouse-base.c for cs19 (bare ps/2 mouse), we plan to use 
> trackpoint.c to drive this HW, so this trackpoint has all features a 
> trackpoint should have.
>
And I sent a patch one month ago to let the the trackpoint.c to drive 
this HW: https://www.spinics.net/lists/linux-input/msg61341.html, maybe 
that patch is reference.
> Regards,
>
> Hui.
>
>>
>> Best Regards
>> Shona
>> -----邮件原件-----
>> 发件人: Pali Rohár <[e-mail]>
>> 发送时间: Monday, June 10, 2019 6:43 PM
>> 收件人: 劉 曉曉 Xiaoxiao Liu <[e-mail]>
>> 抄送: XiaoXiao Liu <[e-mail]>; 
>> [e-mail]; [e-mail]; 
>> [e-mail]; [e-mail]; 
>> [e-mail]; 曹 曉建 Xiaojian Cao 
>> <[e-mail]>; [e-mail]; 斉藤 直樹 Naoki Saito 
>> <[e-mail]>; 川瀬 英夫 Hideo Kawase 
>> <[e-mail]>
>> 主题: Re: 答复: 答复: [PATCH] input: alps-fix the issue alps cs19 
>> trackstick do not work.
>>
>> On Monday 10 June 2019 10:03:51 Xiaoxiao Liu wrote:
>>> Hi Pali,
>> Hi!
>>
>>> We register our CS19 device as ALPS_ONLY_TRACKSTICK device.
>>> And let the V8 protocol function support the process of 
>>> ALPS_ONLY_TRACKSTICK device.
>>>
>>> I want to confirm if this solution OK?
>> Yes, it is fine. Just make sure that touchapad input device is not 
>> registered when this ALPS_ONLY_TRACKSTICK flag is set. So userspace 
>> would not see any fake/unavailable touchpad input device.
>>
>>> Xiaoxiao.Liu
>> -- 
>> Pali Rohár
>> [e-mail]

Pali Rohár June 11, 2019, 7:23 a.m. UTC | #11

On Tuesday 11 June 2019 12:32:33 Hui Wang wrote:
> On 2019/6/11 上午11:23, Hui Wang wrote:
> > On 2019/6/11 上午11:05, Xiaoxiao Liu wrote:
> > > Hi Pali,
> > > 
> > > I discussed with our FW team about this problem.
> > > We think the V8 method means a touchpad feature  and does not fit
> > > the CS19 trackpoint device.
> > > CS19 TrackPoint needn't  use any Absolute (Raw) mode and is usually
> > > use standard mouse data.
> > > CS19 TrackPoint device is a completely different device with
> > > DualPoint device of Dell/HP.
> > > CS19 TrackPoint device is independent  of Touchpad. (Touchpad is
> > > connecting by I2C, TrackPoint is directly connecting with PS2 port.)
> > > And it has completely another FW.
> > > 
> > > So we think it is better to use the mouse mode for CS19 trackpoint.
> > 
> > Maybe here is some mis-understanding,  the mouse mode here doesn't mean
> > we use psmouse-base.c for cs19 (bare ps/2 mouse), we plan to use
> > trackpoint.c to drive this HW, so this trackpoint has all features a
> > trackpoint should have.
> > 
> And I sent a patch one month ago to let the the trackpoint.c to drive this
> HW: https://www.spinics.net/lists/linux-input/msg61341.html, maybe that
> patch is reference.

So instead of creating blacklist, you should check for TP_VARIANT_ALPS
in alps.c and disallow its usage.

Or maybe better, move trackpoint.c detect code before alsp.c detect code
in psmouse-base. And no changes in alps.c are needed.

> > Regards,
> > 
> > Hui.
> > 
> > > 
> > > Best Regards
> > > Shona
> > > -----邮件原件-----
> > > 发件人: Pali Rohár <[e-mail]>
> > > 发送时间: Monday, June 10, 2019 6:43 PM
> > > 收件人: 劉 曉曉 Xiaoxiao Liu <[e-mail]>
> > > 抄送: XiaoXiao Liu <[e-mail]>;
> > > [e-mail]; [e-mail];
> > > [e-mail]; [e-mail];
> > > [e-mail]; 曹 曉建 Xiaojian Cao
> > > <[e-mail]>; [e-mail]; 斉藤 直樹 Naoki
> > > Saito <[e-mail]>; 川瀬 英夫 Hideo Kawase
> > > <[e-mail]>
> > > 主题: Re: 答复: 答复: [PATCH] input: alps-fix the issue alps cs19
> > > trackstick do not work.
> > > 
> > > On Monday 10 June 2019 10:03:51 Xiaoxiao Liu wrote:
> > > > Hi Pali,
> > > Hi!
> > > 
> > > > We register our CS19 device as ALPS_ONLY_TRACKSTICK device.
> > > > And let the V8 protocol function support the process of
> > > > ALPS_ONLY_TRACKSTICK device.
> > > > 
> > > > I want to confirm if this solution OK?
> > > Yes, it is fine. Just make sure that touchapad input device is not
> > > registered when this ALPS_ONLY_TRACKSTICK flag is set. So userspace
> > > would not see any fake/unavailable touchpad input device.
> > > 
> > > > Xiaoxiao.Liu
> > > -- 
> > > Pali Rohár
> > > [e-mail]

Dmitry Torokhov June 11, 2019, 5:07 p.m. UTC | #12

On Tue, Jun 11, 2019 at 09:23:33AM +0200, Pali Rohár wrote:
> On Tuesday 11 June 2019 12:32:33 Hui Wang wrote:
> > On 2019/6/11 上午11:23, Hui Wang wrote:
> > > On 2019/6/11 上午11:05, Xiaoxiao Liu wrote:
> > > > Hi Pali,
> > > > 
> > > > I discussed with our FW team about this problem.
> > > > We think the V8 method means a touchpad feature  and does not fit
> > > > the CS19 trackpoint device.
> > > > CS19 TrackPoint needn't  use any Absolute (Raw) mode and is usually
> > > > use standard mouse data.
> > > > CS19 TrackPoint device is a completely different device with
> > > > DualPoint device of Dell/HP.
> > > > CS19 TrackPoint device is independent  of Touchpad. (Touchpad is
> > > > connecting by I2C, TrackPoint is directly connecting with PS2 port.)
> > > > And it has completely another FW.
> > > > 
> > > > So we think it is better to use the mouse mode for CS19 trackpoint.
> > > 
> > > Maybe here is some mis-understanding,  the mouse mode here doesn't mean
> > > we use psmouse-base.c for cs19 (bare ps/2 mouse), we plan to use
> > > trackpoint.c to drive this HW, so this trackpoint has all features a
> > > trackpoint should have.
> > > 
> > And I sent a patch one month ago to let the the trackpoint.c to drive this
> > HW: https://www.spinics.net/lists/linux-input/msg61341.html, maybe that
> > patch is reference.
> 
> So instead of creating blacklist, you should check for TP_VARIANT_ALPS
> in alps.c and disallow its usage.
> 
> Or maybe better, move trackpoint.c detect code before alsp.c detect code
> in psmouse-base. And no changes in alps.c are needed.

I'd be very cautions of moving around the protocol detection. It is very
fragile, so if we can detect trackpoint-only case in alps.c and skip on
to trackpoint I would prefer it.

Thanks.

Pali Rohár June 11, 2019, 5:17 p.m. UTC | #13

On Tuesday 11 June 2019 10:07:07 [e-mail] wrote:
> On Tue, Jun 11, 2019 at 09:23:33AM +0200, Pali Rohár wrote:
> > On Tuesday 11 June 2019 12:32:33 Hui Wang wrote:
> > > On 2019/6/11 上午11:23, Hui Wang wrote:
> > > > On 2019/6/11 上午11:05, Xiaoxiao Liu wrote:
> > > > > Hi Pali,
> > > > > 
> > > > > I discussed with our FW team about this problem.
> > > > > We think the V8 method means a touchpad feature  and does not fit
> > > > > the CS19 trackpoint device.
> > > > > CS19 TrackPoint needn't  use any Absolute (Raw) mode and is usually
> > > > > use standard mouse data.
> > > > > CS19 TrackPoint device is a completely different device with
> > > > > DualPoint device of Dell/HP.
> > > > > CS19 TrackPoint device is independent  of Touchpad. (Touchpad is
> > > > > connecting by I2C, TrackPoint is directly connecting with PS2 port.)
> > > > > And it has completely another FW.
> > > > > 
> > > > > So we think it is better to use the mouse mode for CS19 trackpoint.
> > > > 
> > > > Maybe here is some mis-understanding,  the mouse mode here doesn't mean
> > > > we use psmouse-base.c for cs19 (bare ps/2 mouse), we plan to use
> > > > trackpoint.c to drive this HW, so this trackpoint has all features a
> > > > trackpoint should have.
> > > > 
> > > And I sent a patch one month ago to let the the trackpoint.c to drive this
> > > HW: https://www.spinics.net/lists/linux-input/msg61341.html, maybe that
> > > patch is reference.
> > 
> > So instead of creating blacklist, you should check for TP_VARIANT_ALPS
> > in alps.c and disallow its usage.
> > 
> > Or maybe better, move trackpoint.c detect code before alsp.c detect code
> > in psmouse-base. And no changes in alps.c are needed.
> 
> I'd be very cautions of moving around the protocol detection. It is very
> fragile, so if we can detect trackpoint-only case in alps.c and skip on
> to trackpoint I would prefer it.

The main problem is that proposed trackpoint-only check in alps.c is
basically what trackpoint.c is doing for checking if device is
trackpoint (via function trackpoint_start_protocol()).

So I'm not sure now what is the best solution...

Dmitry Torokhov June 11, 2019, 5:32 p.m. UTC | #14

On Tue, Jun 11, 2019 at 07:17:07PM +0200, Pali Rohár wrote:
> On Tuesday 11 June 2019 10:07:07 [e-mail] wrote:
> > On Tue, Jun 11, 2019 at 09:23:33AM +0200, Pali Rohár wrote:
> > > On Tuesday 11 June 2019 12:32:33 Hui Wang wrote:
> > > > On 2019/6/11 上午11:23, Hui Wang wrote:
> > > > > On 2019/6/11 上午11:05, Xiaoxiao Liu wrote:
> > > > > > Hi Pali,
> > > > > > 
> > > > > > I discussed with our FW team about this problem.
> > > > > > We think the V8 method means a touchpad feature  and does not fit
> > > > > > the CS19 trackpoint device.
> > > > > > CS19 TrackPoint needn't  use any Absolute (Raw) mode and is usually
> > > > > > use standard mouse data.
> > > > > > CS19 TrackPoint device is a completely different device with
> > > > > > DualPoint device of Dell/HP.
> > > > > > CS19 TrackPoint device is independent  of Touchpad. (Touchpad is
> > > > > > connecting by I2C, TrackPoint is directly connecting with PS2 port.)
> > > > > > And it has completely another FW.
> > > > > > 
> > > > > > So we think it is better to use the mouse mode for CS19 trackpoint.
> > > > > 
> > > > > Maybe here is some mis-understanding,  the mouse mode here doesn't mean
> > > > > we use psmouse-base.c for cs19 (bare ps/2 mouse), we plan to use
> > > > > trackpoint.c to drive this HW, so this trackpoint has all features a
> > > > > trackpoint should have.
> > > > > 
> > > > And I sent a patch one month ago to let the the trackpoint.c to drive this
> > > > HW: https://www.spinics.net/lists/linux-input/msg61341.html, maybe that
> > > > patch is reference.
> > > 
> > > So instead of creating blacklist, you should check for TP_VARIANT_ALPS
> > > in alps.c and disallow its usage.
> > > 
> > > Or maybe better, move trackpoint.c detect code before alsp.c detect code
> > > in psmouse-base. And no changes in alps.c are needed.
> > 
> > I'd be very cautions of moving around the protocol detection. It is very
> > fragile, so if we can detect trackpoint-only case in alps.c and skip on
> > to trackpoint I would prefer it.
> 
> The main problem is that proposed trackpoint-only check in alps.c is
> basically what trackpoint.c is doing for checking if device is
> trackpoint (via function trackpoint_start_protocol()).
> 
> So I'm not sure now what is the best solution...

Unfortunately currently trackpoint is being probed only after we tried
Elan, Genius, and Logitech PS2++ protocols, and I am not sure if moving
trackpoint around will disturb them or not.

I do not think there is much code duplication by pulling limited version
of trackpoint detection code into alps.c and then have it bail out when
it sees trackpoint-only device so trackpoint.c can handle it properly.

Thanks.

Pali Rohár June 11, 2019, 5:38 p.m. UTC | #15

On Tuesday 11 June 2019 10:32:28 [e-mail] wrote:
> On Tue, Jun 11, 2019 at 07:17:07PM +0200, Pali Rohár wrote:
> > On Tuesday 11 June 2019 10:07:07 [e-mail] wrote:
> > > On Tue, Jun 11, 2019 at 09:23:33AM +0200, Pali Rohár wrote:
> > > > On Tuesday 11 June 2019 12:32:33 Hui Wang wrote:
> > > > > On 2019/6/11 上午11:23, Hui Wang wrote:
> > > > > > On 2019/6/11 上午11:05, Xiaoxiao Liu wrote:
> > > > > > > Hi Pali,
> > > > > > > 
> > > > > > > I discussed with our FW team about this problem.
> > > > > > > We think the V8 method means a touchpad feature  and does not fit
> > > > > > > the CS19 trackpoint device.
> > > > > > > CS19 TrackPoint needn't  use any Absolute (Raw) mode and is usually
> > > > > > > use standard mouse data.
> > > > > > > CS19 TrackPoint device is a completely different device with
> > > > > > > DualPoint device of Dell/HP.
> > > > > > > CS19 TrackPoint device is independent  of Touchpad. (Touchpad is
> > > > > > > connecting by I2C, TrackPoint is directly connecting with PS2 port.)
> > > > > > > And it has completely another FW.
> > > > > > > 
> > > > > > > So we think it is better to use the mouse mode for CS19 trackpoint.
> > > > > > 
> > > > > > Maybe here is some mis-understanding,  the mouse mode here doesn't mean
> > > > > > we use psmouse-base.c for cs19 (bare ps/2 mouse), we plan to use
> > > > > > trackpoint.c to drive this HW, so this trackpoint has all features a
> > > > > > trackpoint should have.
> > > > > > 
> > > > > And I sent a patch one month ago to let the the trackpoint.c to drive this
> > > > > HW: https://www.spinics.net/lists/linux-input/msg61341.html, maybe that
> > > > > patch is reference.
> > > > 
> > > > So instead of creating blacklist, you should check for TP_VARIANT_ALPS
> > > > in alps.c and disallow its usage.
> > > > 
> > > > Or maybe better, move trackpoint.c detect code before alsp.c detect code
> > > > in psmouse-base. And no changes in alps.c are needed.
> > > 
> > > I'd be very cautions of moving around the protocol detection. It is very
> > > fragile, so if we can detect trackpoint-only case in alps.c and skip on
> > > to trackpoint I would prefer it.
> > 
> > The main problem is that proposed trackpoint-only check in alps.c is
> > basically what trackpoint.c is doing for checking if device is
> > trackpoint (via function trackpoint_start_protocol()).
> > 
> > So I'm not sure now what is the best solution...
> 
> Unfortunately currently trackpoint is being probed only after we tried
> Elan, Genius, and Logitech PS2++ protocols, and I am not sure if moving
> trackpoint around will disturb them or not.
> 
> I do not think there is much code duplication by pulling limited version
> of trackpoint detection code into alps.c and then have it bail out when
> it sees trackpoint-only device so trackpoint.c can handle it properly.

Ok. Seems that it is the best solution.

The last question is, should be use ALPS or Trackpoint protocol? Because
it looks like that device can be configured to one or other.

What are pros and cons of these two protocols?

Dmitry Torokhov June 11, 2019, 5:45 p.m. UTC | #16

On Tue, Jun 11, 2019 at 07:38:56PM +0200, Pali Rohár wrote:
> On Tuesday 11 June 2019 10:32:28 [e-mail] wrote:
> > On Tue, Jun 11, 2019 at 07:17:07PM +0200, Pali Rohár wrote:
> > > On Tuesday 11 June 2019 10:07:07 [e-mail] wrote:
> > > > On Tue, Jun 11, 2019 at 09:23:33AM +0200, Pali Rohár wrote:
> > > > > On Tuesday 11 June 2019 12:32:33 Hui Wang wrote:
> > > > > > On 2019/6/11 上午11:23, Hui Wang wrote:
> > > > > > > On 2019/6/11 上午11:05, Xiaoxiao Liu wrote:
> > > > > > > > Hi Pali,
> > > > > > > > 
> > > > > > > > I discussed with our FW team about this problem.
> > > > > > > > We think the V8 method means a touchpad feature  and does not fit
> > > > > > > > the CS19 trackpoint device.
> > > > > > > > CS19 TrackPoint needn't  use any Absolute (Raw) mode and is usually
> > > > > > > > use standard mouse data.
> > > > > > > > CS19 TrackPoint device is a completely different device with
> > > > > > > > DualPoint device of Dell/HP.
> > > > > > > > CS19 TrackPoint device is independent  of Touchpad. (Touchpad is
> > > > > > > > connecting by I2C, TrackPoint is directly connecting with PS2 port.)
> > > > > > > > And it has completely another FW.
> > > > > > > > 
> > > > > > > > So we think it is better to use the mouse mode for CS19 trackpoint.
> > > > > > > 
> > > > > > > Maybe here is some mis-understanding,  the mouse mode here doesn't mean
> > > > > > > we use psmouse-base.c for cs19 (bare ps/2 mouse), we plan to use
> > > > > > > trackpoint.c to drive this HW, so this trackpoint has all features a
> > > > > > > trackpoint should have.
> > > > > > > 
> > > > > > And I sent a patch one month ago to let the the trackpoint.c to drive this
> > > > > > HW: https://www.spinics.net/lists/linux-input/msg61341.html, maybe that
> > > > > > patch is reference.
> > > > > 
> > > > > So instead of creating blacklist, you should check for TP_VARIANT_ALPS
> > > > > in alps.c and disallow its usage.
> > > > > 
> > > > > Or maybe better, move trackpoint.c detect code before alsp.c detect code
> > > > > in psmouse-base. And no changes in alps.c are needed.
> > > > 
> > > > I'd be very cautions of moving around the protocol detection. It is very
> > > > fragile, so if we can detect trackpoint-only case in alps.c and skip on
> > > > to trackpoint I would prefer it.
> > > 
> > > The main problem is that proposed trackpoint-only check in alps.c is
> > > basically what trackpoint.c is doing for checking if device is
> > > trackpoint (via function trackpoint_start_protocol()).
> > > 
> > > So I'm not sure now what is the best solution...
> > 
> > Unfortunately currently trackpoint is being probed only after we tried
> > Elan, Genius, and Logitech PS2++ protocols, and I am not sure if moving
> > trackpoint around will disturb them or not.
> > 
> > I do not think there is much code duplication by pulling limited version
> > of trackpoint detection code into alps.c and then have it bail out when
> > it sees trackpoint-only device so trackpoint.c can handle it properly.
> 
> Ok. Seems that it is the best solution.
> 
> The last question is, should be use ALPS or Trackpoint protocol? Because
> it looks like that device can be configured to one or other.
> 
> What are pros and cons of these two protocols?

As far as I know the device implements trackpoint protocol, although not
complete version. Several manufacturers started making trackponts once
IBM/Lenovo patents on the original one expired (I think).

The data stream is regular relative PS/2, bit it allows controlling
device behavior a bit, such as press-to-select option and device
sensitivity. IBM/Lenovo version has many more parameters.

Thanks.

Xiaoxiao Liu June 17, 2019, 1:29 a.m. UTC | #17

Hi Pali,

Since design architecture change of CS19, input device connection has been changed to below architecture, 
Touchpad has been moved to I2C connection.

  kernel/host  <--PS/2-->  EC  <--PS/2-->  external PS/2 mouse
           |                 |
           |                <--PS/2-->  trackstick
               |
                <--I2C-->  Touchpad

In the past TrackPoint does not show in the device list because of TrackPoint was hidden device of Touchpad.
But from CS19, TrackPoint is directly connecting with PS2 port, 
3 bytes packet does not need to take affect by other vendors Touchpad format. 
So alps.c is no need for CS19 device.

Best Regards
Shona
-----邮件原件-----
发件人: Pali Rohár <[e-mail]> 
发送时间: Wednesday, June 12, 2019 1:39 AM
收件人: [e-mail]
抄送: Hui Wang <[e-mail]>; 劉 曉曉 Xiaoxiao Liu <[e-mail]>; XiaoXiao Liu <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; 曹 曉建 Xiaojian Cao <[e-mail]>; [e-mail]; 斉藤 直樹 Naoki Saito <[e-mail]>; 川瀬 英夫 Hideo Kawase <[e-mail]>
主题: Re: 答复: 答复: 答复: [PATCH] input: alps-fix the issue alps cs19 trackstick do not work.

On Tuesday 11 June 2019 10:32:28 [e-mail] wrote:
> On Tue, Jun 11, 2019 at 07:17:07PM +0200, Pali Rohár wrote:
> > On Tuesday 11 June 2019 10:07:07 [e-mail] wrote:
> > > On Tue, Jun 11, 2019 at 09:23:33AM +0200, Pali Rohár wrote:
> > > > On Tuesday 11 June 2019 12:32:33 Hui Wang wrote:
> > > > > On 2019/6/11 上午11:23, Hui Wang wrote:
> > > > > > On 2019/6/11 上午11:05, Xiaoxiao Liu wrote:
> > > > > > > Hi Pali,
> > > > > > > 
> > > > > > > I discussed with our FW team about this problem.
> > > > > > > We think the V8 method means a touchpad feature  and does 
> > > > > > > not fit the CS19 trackpoint device.
> > > > > > > CS19 TrackPoint needn't  use any Absolute (Raw) mode and 
> > > > > > > is usually use standard mouse data.
> > > > > > > CS19 TrackPoint device is a completely different device 
> > > > > > > with DualPoint device of Dell/HP.
> > > > > > > CS19 TrackPoint device is independent  of Touchpad. 
> > > > > > > (Touchpad is connecting by I2C, TrackPoint is directly 
> > > > > > > connecting with PS2 port.) And it has completely another FW.
> > > > > > > 
> > > > > > > So we think it is better to use the mouse mode for CS19 trackpoint.
> > > > > > 
> > > > > > Maybe here is some mis-understanding,  the mouse mode here 
> > > > > > doesn't mean we use psmouse-base.c for cs19 (bare ps/2 
> > > > > > mouse), we plan to use trackpoint.c to drive this HW, so 
> > > > > > this trackpoint has all features a trackpoint should have.
> > > > > > 
> > > > > And I sent a patch one month ago to let the the trackpoint.c 
> > > > > to drive this
> > > > > HW: https://www.spinics.net/lists/linux-input/msg61341.html, 
> > > > > maybe that patch is reference.
> > > > 
> > > > So instead of creating blacklist, you should check for 
> > > > TP_VARIANT_ALPS in alps.c and disallow its usage.
> > > > 
> > > > Or maybe better, move trackpoint.c detect code before alsp.c 
> > > > detect code in psmouse-base. And no changes in alps.c are needed.
> > > 
> > > I'd be very cautions of moving around the protocol detection. It 
> > > is very fragile, so if we can detect trackpoint-only case in 
> > > alps.c and skip on to trackpoint I would prefer it.
> > 
> > The main problem is that proposed trackpoint-only check in alps.c is 
> > basically what trackpoint.c is doing for checking if device is 
> > trackpoint (via function trackpoint_start_protocol()).
> > 
> > So I'm not sure now what is the best solution...
> 
> Unfortunately currently trackpoint is being probed only after we tried 
> Elan, Genius, and Logitech PS2++ protocols, and I am not sure if 
> moving trackpoint around will disturb them or not.
> 
> I do not think there is much code duplication by pulling limited 
> version of trackpoint detection code into alps.c and then have it bail 
> out when it sees trackpoint-only device so trackpoint.c can handle it properly.

Ok. Seems that it is the best solution.

The last question is, should be use ALPS or Trackpoint protocol? Because it looks like that device can be configured to one or other.

What are pros and cons of these two protocols?

--
Pali Rohár
[e-mail]

Pali Rohár June 17, 2019, 7:49 a.m. UTC | #18

On Monday 17 June 2019 01:29:17 Xiaoxiao Liu wrote:
> Hi Pali,
> 
> Since design architecture change of CS19, input device connection has been changed to below architecture, 
> Touchpad has been moved to I2C connection.
> 
>   kernel/host  <--PS/2-->  EC  <--PS/2-->  external PS/2 mouse
>            |                 |
>            |                <--PS/2-->  trackstick
>                |
>                 <--I2C-->  Touchpad

Hi, thank you for explanation!

So in our case, ALPS device should not be put into passthrough mode as
there is no device after it.

> In the past TrackPoint does not show in the device list because of TrackPoint was hidden device of Touchpad.
> But from CS19, TrackPoint is directly connecting with PS2 port, 
> 3 bytes packet does not need to take affect by other vendors Touchpad format. 
> So alps.c is no need for CS19 device.

So if trackpoint.c driver is working fine with this configuration, it is
just needed to instruct alps.c to not take this device.

> Best Regards
> Shona
> -----邮件原件-----
> 发件人: Pali Rohár <[e-mail]> 
> 发送时间: Wednesday, June 12, 2019 1:39 AM
> 收件人: [e-mail]
> 抄送: Hui Wang <[e-mail]>; 劉 曉曉 Xiaoxiao Liu <[e-mail]>; XiaoXiao Liu <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; 曹 曉建 Xiaojian Cao <[e-mail]>; [e-mail]; 斉藤 直樹 Naoki Saito <[e-mail]>; 川瀬 英夫 Hideo Kawase <[e-mail]>
> 主题: Re: 答复: 答复: 答复: [PATCH] input: alps-fix the issue alps cs19 trackstick do not work.
> 
> On Tuesday 11 June 2019 10:32:28 [e-mail] wrote:
> > On Tue, Jun 11, 2019 at 07:17:07PM +0200, Pali Rohár wrote:
> > > On Tuesday 11 June 2019 10:07:07 [e-mail] wrote:
> > > > On Tue, Jun 11, 2019 at 09:23:33AM +0200, Pali Rohár wrote:
> > > > > On Tuesday 11 June 2019 12:32:33 Hui Wang wrote:
> > > > > > On 2019/6/11 上午11:23, Hui Wang wrote:
> > > > > > > On 2019/6/11 上午11:05, Xiaoxiao Liu wrote:
> > > > > > > > Hi Pali,
> > > > > > > > 
> > > > > > > > I discussed with our FW team about this problem.
> > > > > > > > We think the V8 method means a touchpad feature  and does 
> > > > > > > > not fit the CS19 trackpoint device.
> > > > > > > > CS19 TrackPoint needn't  use any Absolute (Raw) mode and 
> > > > > > > > is usually use standard mouse data.
> > > > > > > > CS19 TrackPoint device is a completely different device 
> > > > > > > > with DualPoint device of Dell/HP.
> > > > > > > > CS19 TrackPoint device is independent  of Touchpad. 
> > > > > > > > (Touchpad is connecting by I2C, TrackPoint is directly 
> > > > > > > > connecting with PS2 port.) And it has completely another FW.
> > > > > > > > 
> > > > > > > > So we think it is better to use the mouse mode for CS19 trackpoint.
> > > > > > > 
> > > > > > > Maybe here is some mis-understanding,  the mouse mode here 
> > > > > > > doesn't mean we use psmouse-base.c for cs19 (bare ps/2 
> > > > > > > mouse), we plan to use trackpoint.c to drive this HW, so 
> > > > > > > this trackpoint has all features a trackpoint should have.
> > > > > > > 
> > > > > > And I sent a patch one month ago to let the the trackpoint.c 
> > > > > > to drive this
> > > > > > HW: https://www.spinics.net/lists/linux-input/msg61341.html, 
> > > > > > maybe that patch is reference.
> > > > > 
> > > > > So instead of creating blacklist, you should check for 
> > > > > TP_VARIANT_ALPS in alps.c and disallow its usage.
> > > > > 
> > > > > Or maybe better, move trackpoint.c detect code before alsp.c 
> > > > > detect code in psmouse-base. And no changes in alps.c are needed.
> > > > 
> > > > I'd be very cautions of moving around the protocol detection. It 
> > > > is very fragile, so if we can detect trackpoint-only case in 
> > > > alps.c and skip on to trackpoint I would prefer it.
> > > 
> > > The main problem is that proposed trackpoint-only check in alps.c is 
> > > basically what trackpoint.c is doing for checking if device is 
> > > trackpoint (via function trackpoint_start_protocol()).
> > > 
> > > So I'm not sure now what is the best solution...
> > 
> > Unfortunately currently trackpoint is being probed only after we tried 
> > Elan, Genius, and Logitech PS2++ protocols, and I am not sure if 
> > moving trackpoint around will disturb them or not.
> > 
> > I do not think there is much code duplication by pulling limited 
> > version of trackpoint detection code into alps.c and then have it bail 
> > out when it sees trackpoint-only device so trackpoint.c can handle it properly.
> 
> Ok. Seems that it is the best solution.
> 
> The last question is, should be use ALPS or Trackpoint protocol? Because it looks like that device can be configured to one or other.
> 
> What are pros and cons of these two protocols?
> 
> --
> Pali Rohár
> [e-mail]

Hui Wang June 18, 2019, 8:19 a.m. UTC | #19

On 2019/6/17 下午3:49, Pali Rohár wrote:
> On Monday 17 June 2019 01:29:17 Xiaoxiao Liu wrote:
>> Hi Pali,
>>
>> Since design architecture change of CS19, input device connection has been changed to below architecture,
>> Touchpad has been moved to I2C connection.
>>
>>    kernel/host  <--PS/2-->  EC  <--PS/2-->  external PS/2 mouse
>>             |                 |
>>             |                <--PS/2-->  trackstick
>>                 |
>>                  <--I2C-->  Touchpad
> Hi, thank you for explanation!
>
> So in our case, ALPS device should not be put into passthrough mode as
> there is no device after it.
>
>> In the past TrackPoint does not show in the device list because of TrackPoint was hidden device of Touchpad.
>> But from CS19, TrackPoint is directly connecting with PS2 port,
>> 3 bytes packet does not need to take affect by other vendors Touchpad format.
>> So alps.c is no need for CS19 device.
> So if trackpoint.c driver is working fine with this configuration, it is
> just needed to instruct alps.c to not take this device.

Hello Pali,

When you have time, could you take a look at the patch of v4? It is 
implemented according to our discussion.

Thanks,

Hui.


>> Best Regards
>> Shona
>> -----邮件原件-----
>> 发件人: Pali Rohár <[e-mail]>
>> 发送时间: Wednesday, June 12, 2019 1:39 AM
>> 收件人: [e-mail]
>> 抄送: Hui Wang <[e-mail]>; 劉 曉曉 Xiaoxiao Liu <[e-mail]>; XiaoXiao Liu <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; 曹 曉建 Xiaojian Cao <[e-mail]>; [e-mail]; 斉藤 直樹 Naoki Saito <[e-mail]>; 川瀬 英夫 Hideo Kawase <[e-mail]>
>

Patch

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 0a6f7ca883e7..a54677cf7474 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
  */
@@ -220,6 +220,23 @@  static bool alps_is_valid_first_byte(struct alps_data *priv,
 	return (data & priv->mask0) == priv->byte0;
 }
 
+static int alps_check_cs19_trackstick(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 && param[1] & 0x20) {
+		psmouse_warn(psmouse, "It is alps cs19 trackstick");
+		return 0;
+	}
+	return -1;
+}
+
 static void alps_report_buttons(struct input_dev *dev1, struct input_dev *dev2,
 				int left, int right, int middle)
 {
@@ -2568,8 +2585,12 @@  static int alps_update_dual_info_ss4_v2(unsigned char otp[][4],
 			alps_exit_command_mode(psmouse);
 			ps2_command(ps2dev, NULL, PSMOUSE_CMD_ENABLE);
 
-			if (reg_val == 0x0C || reg_val == 0x1D)
+			if (reg_val == 0x0C || reg_val == 0x1D) {
+				is_dual = true;
+			} else if (alps_check_cs19_trackstick(psmouse) == 0) {
+				//For support Thinkpad CS19 TrackStick
 				is_dual = true;
+			}
 		}
 	}