[PATCH] Input: ALPS - add support for 73 03 28 devices (Thinkpad L570)

[PATCH] Input: ALPS - add support for 73 03 28 devices (Thinkpad L570)

[Dennis Wassenberg]

The Lenovo Thinkpad L570 uses V8 protocol.
Add 0x73 0x03 0x28 devices to use V8 protovol which makes
trackstick and mouse buttons work with Lenovo Thinkpad L570.

Signed-off-by: Dennis Wassenberg <dennis.wassenberg@secunet.com>
---
 drivers/input/mouse/alps.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index dbe57da..5523d4e 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -136,6 +136,8 @@
 	{ { 0x73, 0x02, 0x0a }, { ALPS_PROTO_V2, 0xf8, 0xf8, 0 } },
 	{ { 0x73, 0x02, 0x14 }, { ALPS_PROTO_V2, 0xf8, 0xf8, ALPS_FW_BK_2 } },			/* Ahtec Laptop */
 	{ { 0x73, 0x02, 0x50 }, { ALPS_PROTO_V2, 0xcf, 0xcf, ALPS_FOUR_BUTTONS } },		/* Dell Vostro 1400 */
+	{ { 0x73, 0x03, 0x28 }, { ALPS_PROTO_V8, 0x18, 0x18,
+		ALPS_DUALPOINT | ALPS_DUALPOINT_WITH_PRESSURE | ALPS_BUTTONPAD } },		/* Lenovo L570 */
 };
 
 static const struct alps_protocol_info alps_v3_protocol_data = {
-- 
1.9.1

Re: [PATCH] Input: ALPS - add support for 73 03 28 devices (Thinkpad L570)

[Pali Rohár]

On Friday 23 March 2018 15:23:55 Dennis Wassenberg wrote:
> The Lenovo Thinkpad L570 uses V8 protocol.
> Add 0x73 0x03 0x28 devices to use V8 protovol which makes
> trackstick and mouse buttons work with Lenovo Thinkpad L570.
> 
> Signed-off-by: Dennis Wassenberg <dennis.wassenberg@secunet.com>
> ---
>  drivers/input/mouse/alps.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index dbe57da..5523d4e 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -136,6 +136,8 @@
>  	{ { 0x73, 0x02, 0x0a }, { ALPS_PROTO_V2, 0xf8, 0xf8, 0 } },
>  	{ { 0x73, 0x02, 0x14 }, { ALPS_PROTO_V2, 0xf8, 0xf8, ALPS_FW_BK_2 } },			/* Ahtec Laptop */
>  	{ { 0x73, 0x02, 0x50 }, { ALPS_PROTO_V2, 0xcf, 0xcf, ALPS_FOUR_BUTTONS } },		/* Dell Vostro 1400 */
> +	{ { 0x73, 0x03, 0x28 }, { ALPS_PROTO_V8, 0x18, 0x18,
> +		ALPS_DUALPOINT | ALPS_DUALPOINT_WITH_PRESSURE | ALPS_BUTTONPAD } },		/* Lenovo L570 */
>  };
>  
>  static const struct alps_protocol_info alps_v3_protocol_data = {

Hi! alps_model_data table is used for fixed identification of v1 and v2
protocols. Why you need to add there v8 protocol which autodetection is
already done in alps_identify() function? There is already code:

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

Which matches above your E7 detection 0x73, 0x03, 0x28.

Also you patch matches basically all v8 device and therefore has
potential to break proper v8 autodetection for other v8 devices...

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

Re: [PATCH] Input: ALPS - add support for 73 03 28 devices (Thinkpad L570)

[Dennis Wassenberg]

Hi,

oh ok, understood. Thanks for this hint.

So maybe there is something wrong with the alps_update_dual_info_ss4_v2
function or the reporting of the hardware.

alps_update_dual_info_ss4_v2 detects the ThinkPad L570 as ss4plus device
but not as dualpoint device. This means that the ALPS_DUALPOINT and the
ALPS_DUALPOINT_WITH_PRESSURE flag will not be set which results in a non
function trackstick and hardware mouse buttons. Each time I touch the
trackstick I get the message: "alps: Rejected trackstick packet from non
DualPoint device".

The value of otp[0][0] inside alps_update_dual_info_ss4_v2 is 0xCE. Are
there any ideas why it is not detected as dualpoint device?

Thank you & best regards,

Dennis

On 23.03.2018 15:33, Pali Rohár wrote:
> On Friday 23 March 2018 15:23:55 Dennis Wassenberg wrote:
>> The Lenovo Thinkpad L570 uses V8 protocol.
>> Add 0x73 0x03 0x28 devices to use V8 protovol which makes
>> trackstick and mouse buttons work with Lenovo Thinkpad L570.
>>
>> Signed-off-by: Dennis Wassenberg <dennis.wassenberg@secunet.com>
>> ---
>>  drivers/input/mouse/alps.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
>> index dbe57da..5523d4e 100644
>> --- a/drivers/input/mouse/alps.c
>> +++ b/drivers/input/mouse/alps.c
>> @@ -136,6 +136,8 @@
>>  	{ { 0x73, 0x02, 0x0a }, { ALPS_PROTO_V2, 0xf8, 0xf8, 0 } },
>>  	{ { 0x73, 0x02, 0x14 }, { ALPS_PROTO_V2, 0xf8, 0xf8, ALPS_FW_BK_2 } },			/* Ahtec Laptop */
>>  	{ { 0x73, 0x02, 0x50 }, { ALPS_PROTO_V2, 0xcf, 0xcf, ALPS_FOUR_BUTTONS } },		/* Dell Vostro 1400 */
>> +	{ { 0x73, 0x03, 0x28 }, { ALPS_PROTO_V8, 0x18, 0x18,
>> +		ALPS_DUALPOINT | ALPS_DUALPOINT_WITH_PRESSURE | ALPS_BUTTONPAD } },		/* Lenovo L570 */
>>  };
>>  
>>  static const struct alps_protocol_info alps_v3_protocol_data = {
> 
> Hi! alps_model_data table is used for fixed identification of v1 and v2
> protocols. Why you need to add there v8 protocol which autodetection is
> already done in alps_identify() function? There is already code:
> 
> 		} else if (e7[0] == 0x73 && e7[1] == 0x03 &&
> 			   (e7[2] == 0x14 || e7[2] == 0x28)) {
> 			protocol = &alps_v8_protocol_data;
> 
> Which matches above your E7 detection 0x73, 0x03, 0x28.
> 
> Also you patch matches basically all v8 device and therefore has
> potential to break proper v8 autodetection for other v8 devices...
> 

RE: [PATCH] Input: ALPS - add support for 73 03 28 devices (Thinkpad L570)

[Masaki Ota]

Hi, Dennis

I know your issue, and I added the solution for Thinkpad L/E system last year.
BTW, Pali also knows about it.

  On Wednesday 29 November 2017 17:33:58 Masaki Ota wrote:
From: Masaki Ota <masaki.ota@jp.alps.com
- The issue is that Thinkpad L570 TrackStick does not work. Because the main interface of Thinkpad L570 device is SMBus, so ALPS overlooked PS2 interface Firmware setting of TrackStick. The detail is that TrackStick otp bit is disabled.
- Add the code that checks 0xD7 address value. This value is device number information, so we can identify the device by checking this value.
- If we check 0xD7 value, we need to enable Command mode and after check the value we need to disable Command mode, then we have to enable the device(0xF4 command).
- Thinkpad L570 device number is 0x0C or 0x1D. If it is TRUE, enable ALPS_DUALPOINT flag.

Signed-off-by: Masaki Ota <masaki.ota@jp.alps.com
---
 drivers/input/mouse/alps.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/input/mouse/alps.c 
b/drivers/input/mouse/alps.c index 850b00e3ad8e..6f092bdd9fc5
100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -2541,13 +2541,31 @@ static int 
alps_update_btn_info_ss4_v2(unsigned char otp[][4],  }
 
 static int alps_update_dual_info_ss4_v2(unsigned char otp[][4],
-				       struct alps_data *priv)
+				       struct alps_data *priv,
+					struct psmouse *psmouse)
 {

 	bool is_dual = false;
+	int reg_val = 0;
+	struct ps2dev *ps2dev = &psmouse-ps2dev;
 
-	if (IS_SS4PLUS_DEV(priv-dev_id))
+	if (IS_SS4PLUS_DEV(priv-dev_id)) {
 		is_dual = (otp[0][0]  4) & 0x01;
 
+		if (!is_dual) {
+			/* For support TrackStick of Thinkpad L/E series */
+			if (alps_exit_command_mode(psmouse) == 0 &&
+				alps_enter_command_mode(psmouse) == 0) {
+				reg_val = alps_command_mode_read_reg(psmouse,
+									0xD7);
+			}
+			alps_exit_command_mode(psmouse);
+			ps2_command(ps2dev, NULL, PSMOUSE_CMD_ENABLE);
+
+			if (reg_val == 0x0C || reg_val == 0x1D)
+				is_dual = true;
+		}
+	}
+
 	if (is_dual)
 		priv-flags |= ALPS_DUALPOINT |
 					ALPS_DUALPOINT_WITH_PRESSURE; @@ -2570,7 +2588,7 @@ static 
int alps_set_defaults_ss4_v2(struct psmouse *psmouse,
 
 	alps_update_btn_info_ss4_v2(otp, priv);
 
-	alps_update_dual_info_ss4_v2(otp, priv);
+	alps_update_dual_info_ss4_v2(otp, priv, psmouse);

 	return 0;
 }
Best Regards,
Masaki Ota
-----Original Message-----
From: Dennis Wassenberg [mailto:dennis.wassenberg@secunet.com] 
Sent: Tuesday, March 27, 2018 10:56 PM
To: Pali Rohár <pali.rohar@gmail.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>; 太田 真喜 Masaki Ota <masaki.ota@jp.alps.com>; Takashi Iwai <tiwai@suse.de>; Kees Cook <keescook@chromium.org>; Nir Perry <nirperry@gmail.com>; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Input: ALPS - add support for 73 03 28 devices (Thinkpad L570)

Hi,

oh ok, understood. Thanks for this hint.

So maybe there is something wrong with the alps_update_dual_info_ss4_v2 function or the reporting of the hardware.

alps_update_dual_info_ss4_v2 detects the ThinkPad L570 as ss4plus device but not as dualpoint device. This means that the ALPS_DUALPOINT and the ALPS_DUALPOINT_WITH_PRESSURE flag will not be set which results in a non function trackstick and hardware mouse buttons. Each time I touch the trackstick I get the message: "alps: Rejected trackstick packet from non DualPoint device".

The value of otp[0][0] inside alps_update_dual_info_ss4_v2 is 0xCE. Are there any ideas why it is not detected as dualpoint device?

Thank you & best regards,

Dennis

On 23.03.2018 15:33, Pali Rohár wrote:
> On Friday 23 March 2018 15:23:55 Dennis Wassenberg wrote:
>> The Lenovo Thinkpad L570 uses V8 protocol.
>> Add 0x73 0x03 0x28 devices to use V8 protovol which makes trackstick 
>> and mouse buttons work with Lenovo Thinkpad L570.
>>
>> Signed-off-by: Dennis Wassenberg <dennis.wassenberg@secunet.com>
>> ---
>>  drivers/input/mouse/alps.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c 
>> index dbe57da..5523d4e 100644
>> --- a/drivers/input/mouse/alps.c
>> +++ b/drivers/input/mouse/alps.c
>> @@ -136,6 +136,8 @@
>>  	{ { 0x73, 0x02, 0x0a }, { ALPS_PROTO_V2, 0xf8, 0xf8, 0 } },
>>  	{ { 0x73, 0x02, 0x14 }, { ALPS_PROTO_V2, 0xf8, 0xf8, ALPS_FW_BK_2 } },			/* Ahtec Laptop */
>>  	{ { 0x73, 0x02, 0x50 }, { ALPS_PROTO_V2, 0xcf, 0xcf, ALPS_FOUR_BUTTONS } },		/* Dell Vostro 1400 */
>> +	{ { 0x73, 0x03, 0x28 }, { ALPS_PROTO_V8, 0x18, 0x18,
>> +		ALPS_DUALPOINT | ALPS_DUALPOINT_WITH_PRESSURE | ALPS_BUTTONPAD } },		/* Lenovo L570 */
>>  };
>>  
>>  static const struct alps_protocol_info alps_v3_protocol_data = {
> 
> Hi! alps_model_data table is used for fixed identification of v1 and 
> v2 protocols. Why you need to add there v8 protocol which 
> autodetection is already done in alps_identify() function? There is already code:
> 
> 		} else if (e7[0] == 0x73 && e7[1] == 0x03 &&
> 			   (e7[2] == 0x14 || e7[2] == 0x28)) {
> 			protocol = &alps_v8_protocol_data;
> 
> Which matches above your E7 detection 0x73, 0x03, 0x28.
> 
> Also you patch matches basically all v8 device and therefore has 
> potential to break proper v8 autodetection for other v8 devices...
> 

Re: [PATCH] Input: ALPS - add support for 73 03 28 devices (Thinkpad L570)

[Pali Rohár]

Ah, this si again same issue... It was already discussed here:

https://patchwork.kernel.org/patch/10081557/

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