Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 29 Nov 2017 09:16:34 +0100
From:   Pali =?utf-8?B?Um9ow6Fy?= <pali.rohar@gmail.com>
To:     Masaki Ota <012nexus@gmail.com>
Cc:     dmitry.torokhov@gmail.com, benjamin.tissoires@redhat.com,
        aaron.ma@canonical.com, jaak@ristioja.ee, masaki.ota@jp.alps.com,
        linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Support TrackStick of Thinkpad L570
Message-ID: <20171129081634.agzw2f2pqbby4iln@pali>
References: <20171129070105.5540-1-masaki.ota@jp.alps.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20171129070105.5540-1-masaki.ota@jp.alps.com>
User-Agent: NeoMutt/20170113 (1.7.2)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wednesday 29 November 2017 16:01:05 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..dd2c9e9f4830 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 L570 device */

Hi! Jonathan Liu wrote that this patch is needed also for ThinkPad E570p
laptop. So it would be better to add comment that Thinkpad L570 is not
the only one affected and fixing is_dual is needed also for others.
(Probably all Thinkpads which has same generation with this touchpad.)
Such information could prevent in future to wrongly refactor this hook
code.

> +			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;
>  }

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

From 1585382940679968842@xxx Wed Nov 29 07:03:12 +0000 2017
X-GM-THRID: 1585382940679968842
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread