[PATCH 1/2] Input: elantech - enable middle button support on 2 ThinkPads

[PATCH 1/2] Input: elantech - enable middle button support on 2 ThinkPads

[Aaron Ma]

Adding 2 new touchpad PNPIDs to enable middle button support.

Cc: stable@vger.kernel.org
Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
---
 drivers/input/mouse/elantech.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index a7f8b1614559..530142b5a115 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -1189,6 +1189,8 @@ static const char * const middle_button_pnp_ids[] = {
 	"LEN2132", /* ThinkPad P52 */
 	"LEN2133", /* ThinkPad P72 w/ NFC */
 	"LEN2134", /* ThinkPad P72 */
+	"LEN0407",
+	"LEN0408",
 	NULL
 };
 
-- 
2.17.1

[PATCH 2/2] Input: synaptics - remove X240 from the topbuttonpad list

[Aaron Ma]

Lenovo ThinkPad X240 does not have the top software button.
When this wrong ID in top button list, smbus mode will fail to probe,
so keep it working at PS2 mode.

Cc: stable@vger.kernel.org
Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
---
 drivers/input/mouse/synaptics.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index b6da0c1267e3..6ae7bc92476b 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -140,7 +140,6 @@ static const char * const topbuttonpad_pnp_ids[] = {
 	"LEN002E",
 	"LEN0033", /* Helix */
 	"LEN0034", /* T431s, L440, L540, T540, W540, X1 Carbon 2nd */
-	"LEN0035", /* X240 */
 	"LEN0036", /* T440 */
 	"LEN0037", /* X1 Carbon 2nd */
 	"LEN0038",
-- 
2.17.1

Re: [PATCH 2/2] Input: synaptics - remove X240 from the topbuttonpad list

[Dmitry Torokhov]

Hi Aaron,

On Sun, May 19, 2019 at 03:27:11PM +0800, Aaron Ma wrote:
> Lenovo ThinkPad X240 does not have the top software button.
> When this wrong ID in top button list, smbus mode will fail to probe,
> so keep it working at PS2 mode.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> ---
>  drivers/input/mouse/synaptics.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index b6da0c1267e3..6ae7bc92476b 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -140,7 +140,6 @@ static const char * const topbuttonpad_pnp_ids[] = {
>  	"LEN002E",
>  	"LEN0033", /* Helix */
>  	"LEN0034", /* T431s, L440, L540, T540, W540, X1 Carbon 2nd */
> -	"LEN0035", /* X240 */

According to the history this came from Synaptics through Hans, so I'd
like to make sure there are no several X240 versions floating around...

>  	"LEN0036", /* T440 */
>  	"LEN0037", /* X1 Carbon 2nd */
>  	"LEN0038",
> -- 
> 2.17.1
> 

Thanks.

-- 
Dmitry

Re: [PATCH 2/2] Input: synaptics - remove X240 from the topbuttonpad list

[Benjamin Tissoires]

On Tue, May 21, 2019 at 7:09 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Hi Aaron,
>
> On Sun, May 19, 2019 at 03:27:11PM +0800, Aaron Ma wrote:
> > Lenovo ThinkPad X240 does not have the top software button.
> > When this wrong ID in top button list, smbus mode will fail to probe,
> > so keep it working at PS2 mode.
> >
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> > ---
> >  drivers/input/mouse/synaptics.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> > index b6da0c1267e3..6ae7bc92476b 100644
> > --- a/drivers/input/mouse/synaptics.c
> > +++ b/drivers/input/mouse/synaptics.c
> > @@ -140,7 +140,6 @@ static const char * const topbuttonpad_pnp_ids[] = {
> >       "LEN002E",
> >       "LEN0033", /* Helix */
> >       "LEN0034", /* T431s, L440, L540, T540, W540, X1 Carbon 2nd */
> > -     "LEN0035", /* X240 */
>
> According to the history this came from Synaptics through Hans, so I'd
> like to make sure there are no several X240 versions floating around...

A quick google image search showed that the X240 had 2 versions: one
with the top software buttons, one without.

And this definitively rings a bell. I am sure we asked Lenovo and
Synaptics to change the PnPID when they would do such a change, but
they "forgot" during the *40 series refresh.
We have code in place to fix the reported ranges of the coordinates,
and we had to check against the board id (see min_max_pnpid_table[] in
synaptics.c).
Unfortunately, X240 (LEN0035) is not part of this table, so I don't
know which refresh of the board ID has implemented the non top
software buttons.

Cheers,
Benjamin


>
> >       "LEN0036", /* T440 */
> >       "LEN0037", /* X1 Carbon 2nd */
> >       "LEN0038",
> > --
> > 2.17.1
> >
>
> Thanks.
>
> --
> Dmitry

Re: [PATCH 2/2] Input: synaptics - remove X240 from the topbuttonpad list

[Aaron Ma]

On 5/21/19 2:49 PM, Benjamin Tissoires wrote:
> A quick google image search showed that the X240 had 2 versions: one
> with the top software buttons, one without.
> 
> And this definitively rings a bell. I am sure we asked Lenovo and
> Synaptics to change the PnPID when they would do such a change, but
> they "forgot" during the *40 series refresh.
> We have code in place to fix the reported ranges of the coordinates,
> and we had to check against the board id (see min_max_pnpid_table[] in
> synaptics.c).
> Unfortunately, X240 (LEN0035) is not part of this table, so I don't
> know which refresh of the board ID has implemented the non top
> software buttons.

After double confirm from Lenovo, looks like they mixed up with
touchpads on X240/X240s.

For now only one user reported this LEN0035 doesn't work on SMBus mode.
module parameter can be a workaround.
Maybe some touchpads with software top buttons are working well on
SMBus. Let's keep eyes on this issue for now.

Regards,
Aaron

> 
> Cheers,
> Benjamin

Re: [PATCH 1/2] Input: elantech - enable middle button support on 2 ThinkPads

[Dmitry Torokhov]

Hi Aaron,

On Sun, May 19, 2019 at 03:27:10PM +0800, Aaron Ma wrote:
> Adding 2 new touchpad PNPIDs to enable middle button support.

Could you add their names in the comments please?

> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> ---
>  drivers/input/mouse/elantech.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index a7f8b1614559..530142b5a115 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -1189,6 +1189,8 @@ static const char * const middle_button_pnp_ids[] = {
>  	"LEN2132", /* ThinkPad P52 */
>  	"LEN2133", /* ThinkPad P72 w/ NFC */
>  	"LEN2134", /* ThinkPad P72 */
> +	"LEN0407",
> +	"LEN0408",

These should come first - I'd like to keep the list sorted
alphabetically.

>  	NULL
>  };
>  
> -- 
> 2.17.1
> 

Thanks.

-- 
Dmitry

Re: [PATCH 1/2] Input: elantech - enable middle button support on 2 ThinkPads

[Benjamin Tissoires]

Hi,

On Tue, May 21, 2019 at 7:11 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Hi Aaron,
>
> On Sun, May 19, 2019 at 03:27:10PM +0800, Aaron Ma wrote:
> > Adding 2 new touchpad PNPIDs to enable middle button support.
>
> Could you add their names in the comments please?
>
> >
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> > ---
> >  drivers/input/mouse/elantech.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> > index a7f8b1614559..530142b5a115 100644
> > --- a/drivers/input/mouse/elantech.c
> > +++ b/drivers/input/mouse/elantech.c
> > @@ -1189,6 +1189,8 @@ static const char * const middle_button_pnp_ids[] = {
> >       "LEN2132", /* ThinkPad P52 */
> >       "LEN2133", /* ThinkPad P72 w/ NFC */
> >       "LEN2134", /* ThinkPad P72 */
> > +     "LEN0407",

AFAICT, this one is the Lenovo P53.
However, having a whitelist of new models is not going to scale IMO.

I was thinking at submitting a patch to enable middle button
unconditionally, but then I realized that libinput disables middle
click emulation on touchpads with an actual physical middle button.

And this morning I just realized that we might have a better chance at
this if we consider the new models to have the physical middle button.
I know LEN0407 attempts to bind over SMBus, so I would think we can
reduce the set of whitelist by just checking for
ETP_NEW_IC_SMBUS_HOST_NOTIFY (patch coming in).

Cheers,
Benjamin

> > +     "LEN0408",
>
> These should come first - I'd like to keep the list sorted
> alphabetically.
>
> >       NULL
> >  };
> >
> > --
> > 2.17.1
> >
>
> Thanks.
>
> --
> Dmitry