2019-05-20 00:13:06

by Aaron Ma

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

Adding 2 new touchpad PNPIDs to enable middle button support.

Cc: [email protected]
Signed-off-by: Aaron Ma <[email protected]>
---
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



2019-05-20 00:31:55

by Aaron Ma

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

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: [email protected]
Signed-off-by: Aaron Ma <[email protected]>
---
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


2019-05-21 05:10:21

by Dmitry Torokhov

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

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: [email protected]
> Signed-off-by: Aaron Ma <[email protected]>
> ---
> 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

2019-05-21 05:12:49

by Dmitry Torokhov

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

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: [email protected]
> Signed-off-by: Aaron Ma <[email protected]>
> ---
> 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

2019-05-21 06:45:34

by Benjamin Tissoires

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

Hi,

On Tue, May 21, 2019 at 7:11 AM Dmitry Torokhov
<[email protected]> 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: [email protected]
> > Signed-off-by: Aaron Ma <[email protected]>
> > ---
> > 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

2019-05-21 06:50:39

by Benjamin Tissoires

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

On Tue, May 21, 2019 at 7:09 AM Dmitry Torokhov
<[email protected]> 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: [email protected]
> > Signed-off-by: Aaron Ma <[email protected]>
> > ---
> > 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

2019-05-23 05:04:09

by Aaron Ma

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



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