Re: Input: mouse: Trackpoint middle button not recognized on Thinkpad E470

Re: Input: mouse: Trackpoint middle button not recognized on Thinkpad E470

[Oscar Campos]

"linux-input@vger.kernel.org" <linux-input@vger.kernel.org>,
aduggan@synaptics.com, Peter Hutterer <peter.hutterer@who-t.net>
Bcc:
Subject: Re: Input: mouse: Trackpoint middle button not recognized on
 Thinkpad E470
Reply-To:
In-Reply-To: <20170531191305.u4hkksajll4q7o5l@beacon.debie>

Hi guys.

I was a bit confused when I received this email as I did not knwow about
the previous thread, I did took a look at the mailing list as well as
the most up to date commits in the input branch to see if someone was
already working in a patch for this but I missed this one, sorry about
that.

I used 0x33 as it is described as one of the four valid responses from
the device (being them 0x22, 0x23, 0x32 and 0x33) the byte contains a
pair of nibbles, the most significant bits indicates how many hardware
buttons are present in the device while the lower ones indicates the
number of buttons that must be presented to the host. Looks like the
Linux kernel just takes care of the lower so 0x23 could had been used as
well but I think 0x33 that indicates three hardware buttons present and
all of them should be exposed was more appropiate.

I can confirm that I wrote the code (after all is just two lines of
code) I do not really care which patch goes into the mainline as soon as
one of them does, I got a Lenovo E570 a couple of months ago and I've been
manually patching my Manjaro kernel since then, it would be really nice
to don't have to do it anymore.

Regards.
Oscar

On Wed, May 31, 2017 at 09:13:05PM +0200, ulrik.debie-os@e2big.org wrote:
> Hi Julian, Oscar,
>
> Peter, you are right and it was even for Lenovo !
> Oscar, I reviewed the exact same patch proposed by Julian one month, as
> you can read below.
>
> Since he was missing some formalities in the way of submission I pointed
> him to submitting-patches.rst .
>
> I was kind of surprised at first that the value was 0x33 instead of 0x3
> until I read the documentation pointers Julian provided.
>
> I really would like this patch to become part of the linux kernel and
> I hope that Julian will go through the formalities for patch submission.
>
> Or alternatively, Oscar, can you confirm that you fully wrote the patch
> yourself and just happened to have the same diff as Julian.
>
> Dmitry, what do you suggest as the best way forward?
>
> Thanks,
> Kind regards,
> Ulrik
>
>
>
>
>
>
>
> On Fri, Apr 28, 2017 at 11:05:32PM +0200, ulrik.debie-os@e2big.org wrote:
> > Date:   Fri, 28 Apr 2017 23:05:32 +0200
> > From: ulrik.debie-os@e2big.org
> > To: Julian Exner <jexner@techfak.uni-bielefeld.de>
> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>,
> >  "linux-input@vger.kernel.org" <linux-input@vger.kernel.org>,
> >  aduggan@synaptics.com
> > Subject: Re: Input: mouse: Trackpoint middle button not recognized on
> >  Thinkpad E470
> > X-Mailing-List: linux-input@vger.kernel.org
> >
> > Hi Julian,
> >
> > See below ..
> >
> > On Wed, Apr 26, 2017 at 11:18:44PM +0200, Julian Exner wrote:
> > > On 26/04/17 00:10, Dmitry Torokhov wrote:
> > > > Hmm.. I do not recall seeing real trackpoints with only 2 buttons. Let's
> > > > try defaulting to 3 and see what happens.
> > >
> > > This patch assumes three buttons when the trackpoint_read command failed.
> > > This is just a quick fix. trackpoint_read may fail for other reasons than
> > > the extended-button-data command not being available, e.g. errors in the ps2
> > > communication. A more narrow solution should check if a resend request was
> > > received, resend, and if the resend failed assume three buttons.
> >
> > > diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
> > > index 354d47e..70c3a48 100644
> > > --- a/drivers/input/mouse/trackpoint.c
> > > +++ b/drivers/input/mouse/trackpoint.c
> > > @@ -380,8 +380,8 @@ int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
> > >  		return 0;
> > >
> > >  	if (trackpoint_read(&psmouse->ps2dev, TP_EXT_BTN, &button_info)) {
> > > -		psmouse_warn(psmouse, "failed to get extended button data\n");
> > > -		button_info = 0;
> > > +		psmouse_warn(psmouse, "failed to get extended button data, assuming 3 buttons are present\n");
> > > +		button_info = 0x33;
> > >  	}
> > >
> > >  	psmouse->private = kzalloc(sizeof(struct trackpoint_data), GFP_KERNEL);
> >
> > Good, this patch is confirmed to make the trackpoint middle button work on
> > Thinkpad e570 when applied to vanilla 4.10 kernel.
> >
> > Now in order to submit the patch for inclusion to the linux kernel,
> > and before I'll grant a Reviewed-by: tag, it would be best when you
> > follow the guidelines in Documentation/SubmittingPatches
> > (recently moved to : Documentation/process/submitting-patches.rst )
> >
> > Attention points there are:
> > 2) Describe your changes
> > 6) No MIME
> > 11) Sign your work (Signed-off: line)
> > 14) The canonical patch format
> >
> > Thanks,
> > Kind regards,
> > Ulrik
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-input" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Input: mouse: Trackpoint middle button not recognized on Thinkpad E470

[ulrik.debie-os]

Hi,

On Wed, Jun 07, 2017 at 09:22:09AM +1000, Peter Hutterer wrote:
> 
> On Tue, Jun 06, 2017 at 08:09:50PM +0200, Julian Exner wrote:
> > Hi all,
> > 
> > sorry for neglecting this thread for so long. And thanks Ulrik for pointing
> > me to the formalities.
> > 
> > > I was kind of surprised at first that the value was 0x33 instead of 0x3
> > > until I read the documentation pointers Julian provided.
> > 
> > It seems like some kind of softswitch or masking for the individual buttons.
> > One of the 4-bit nibbles indicates the number of available buttons in
> > hardware, the other one the number of activated buttons.
> > 
> > > I really would like this patch to become part of the linux kernel and
> > > I hope that Julian will go through the formalities for patch submission.
> > 
> > I'm willing to do so, but I'm very short on time currently. Additionally, I
> > checked on a Thinkpad forum and was told that there are Thinkpads from the
> > 90s with two buttons (e.g. ThinkPad 760 and those before ThinkPad
> > 390/600/770). So I'm a bit afraid that simply setting the default to three
> > buttons may break something for these models and a more elaborate patch may
> > be necessary.
> 
> how many of these do you expect to still be alive *and* in need of a new
> kernel? wikipedia says the 760 was produced 1995 to 1998 which would make
> the newest of them 19 years old now. I'd rather have a hwdb quirk for those
> in userspace, because disabling event codes is trivial.

And next to that, question is if it actually would be using this specific
driver AND would fail to read out the amount of buttons via the extended 
button request.

Up until commit from Mon Nov 16 22:12:21 2009

315eb996d550 Input: psmouse - rework setting of BTN_MIDDLE capability

it was default middle button for all trackpoints; after that the
middle button was disabled when < 3 buttons reported or the readout
of the buttons failed.

Ulrik

Re: Input: mouse: Trackpoint middle button not recognized on Thinkpad E470

[Peter Hutterer]

On Tue, Jun 06, 2017 at 08:09:50PM +0200, Julian Exner wrote:
> Hi all,
> 
> sorry for neglecting this thread for so long. And thanks Ulrik for pointing
> me to the formalities.
> 
> > I was kind of surprised at first that the value was 0x33 instead of 0x3
> > until I read the documentation pointers Julian provided.
> 
> It seems like some kind of softswitch or masking for the individual buttons.
> One of the 4-bit nibbles indicates the number of available buttons in
> hardware, the other one the number of activated buttons.
> 
> > I really would like this patch to become part of the linux kernel and
> > I hope that Julian will go through the formalities for patch submission.
> 
> I'm willing to do so, but I'm very short on time currently. Additionally, I
> checked on a Thinkpad forum and was told that there are Thinkpads from the
> 90s with two buttons (e.g. ThinkPad 760 and those before ThinkPad
> 390/600/770). So I'm a bit afraid that simply setting the default to three
> buttons may break something for these models and a more elaborate patch may
> be necessary.

how many of these do you expect to still be alive *and* in need of a new
kernel? wikipedia says the 760 was produced 1995 to 1998 which would make
the newest of them 19 years old now. I'd rather have a hwdb quirk for those
in userspace, because disabling event codes is trivial.

Cheers,
   Peter

Re: Input: mouse: Trackpoint middle button not recognized on Thinkpad E470

[Julian Exner]

Hi all,

sorry for neglecting this thread for so long. And thanks Ulrik for 
pointing me to the formalities.

> I was kind of surprised at first that the value was 0x33 instead of 0x3
> until I read the documentation pointers Julian provided.

It seems like some kind of softswitch or masking for the individual 
buttons. One of the 4-bit nibbles indicates the number of available 
buttons in hardware, the other one the number of activated buttons.

> I really would like this patch to become part of the linux kernel and
> I hope that Julian will go through the formalities for patch submission.

I'm willing to do so, but I'm very short on time currently. 
Additionally, I checked on a Thinkpad forum and was told that there are 
Thinkpads from the 90s with two buttons (e.g. ThinkPad 760 and those 
before ThinkPad 390/600/770). So I'm a bit afraid that simply setting 
the default to three buttons may break something for these models and a 
more elaborate patch may be necessary.


Kind regards,
Julian

Re: Input: mouse: Trackpoint middle button not recognized on Thinkpad E470

[ulrik.debie-os]

Hi Julian, Oscar,

Peter, you are right and it was even for Lenovo !
Oscar, I reviewed the exact same patch proposed by Julian one month, as
you can read below.

Since he was missing some formalities in the way of submission I pointed
him to submitting-patches.rst .

I was kind of surprised at first that the value was 0x33 instead of 0x3
until I read the documentation pointers Julian provided. 

I really would like this patch to become part of the linux kernel and 
I hope that Julian will go through the formalities for patch submission.

Or alternatively, Oscar, can you confirm that you fully wrote the patch
yourself and just happened to have the same diff as Julian.

Dmitry, what do you suggest as the best way forward?

Thanks,
Kind regards,
Ulrik







On Fri, Apr 28, 2017 at 11:05:32PM +0200, ulrik.debie-os@e2big.org wrote:
> Date:   Fri, 28 Apr 2017 23:05:32 +0200
> From: ulrik.debie-os@e2big.org
> To: Julian Exner <jexner@techfak.uni-bielefeld.de>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>,
>  "linux-input@vger.kernel.org" <linux-input@vger.kernel.org>,
>  aduggan@synaptics.com
> Subject: Re: Input: mouse: Trackpoint middle button not recognized on
>  Thinkpad E470
> X-Mailing-List: linux-input@vger.kernel.org
> 
> Hi Julian,
> 
> See below ..
> 
> On Wed, Apr 26, 2017 at 11:18:44PM +0200, Julian Exner wrote:
> > On 26/04/17 00:10, Dmitry Torokhov wrote:
> > > Hmm.. I do not recall seeing real trackpoints with only 2 buttons. Let's
> > > try defaulting to 3 and see what happens.
> > 
> > This patch assumes three buttons when the trackpoint_read command failed.
> > This is just a quick fix. trackpoint_read may fail for other reasons than
> > the extended-button-data command not being available, e.g. errors in the ps2
> > communication. A more narrow solution should check if a resend request was
> > received, resend, and if the resend failed assume three buttons.
> 
> > diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
> > index 354d47e..70c3a48 100644
> > --- a/drivers/input/mouse/trackpoint.c
> > +++ b/drivers/input/mouse/trackpoint.c
> > @@ -380,8 +380,8 @@ int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
> >  		return 0;
> >  
> >  	if (trackpoint_read(&psmouse->ps2dev, TP_EXT_BTN, &button_info)) {
> > -		psmouse_warn(psmouse, "failed to get extended button data\n");
> > -		button_info = 0;
> > +		psmouse_warn(psmouse, "failed to get extended button data, assuming 3 buttons are present\n");
> > +		button_info = 0x33;
> >  	}
> >  
> >  	psmouse->private = kzalloc(sizeof(struct trackpoint_data), GFP_KERNEL);
> 
> Good, this patch is confirmed to make the trackpoint middle button work on
> Thinkpad e570 when applied to vanilla 4.10 kernel.
> 
> Now in order to submit the patch for inclusion to the linux kernel,
> and before I'll grant a Reviewed-by: tag, it would be best when you
> follow the guidelines in Documentation/SubmittingPatches  
> (recently moved to : Documentation/process/submitting-patches.rst )
> 
> Attention points there are: 
> 2) Describe your changes 
> 6) No MIME 
> 11) Sign your work (Signed-off: line) 
> 14) The canonical patch format
> 
> Thanks,
> Kind regards,
> Ulrik
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Input: mouse: Trackpoint middle button not recognized on Thinkpad E470

[ulrik.debie-os]

Hi Julian,

See below ..

On Wed, Apr 26, 2017 at 11:18:44PM +0200, Julian Exner wrote:
> On 26/04/17 00:10, Dmitry Torokhov wrote:
> > Hmm.. I do not recall seeing real trackpoints with only 2 buttons. Let's
> > try defaulting to 3 and see what happens.
> 
> This patch assumes three buttons when the trackpoint_read command failed.
> This is just a quick fix. trackpoint_read may fail for other reasons than
> the extended-button-data command not being available, e.g. errors in the ps2
> communication. A more narrow solution should check if a resend request was
> received, resend, and if the resend failed assume three buttons.

> diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
> index 354d47e..70c3a48 100644
> --- a/drivers/input/mouse/trackpoint.c
> +++ b/drivers/input/mouse/trackpoint.c
> @@ -380,8 +380,8 @@ int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
>  		return 0;
>  
>  	if (trackpoint_read(&psmouse->ps2dev, TP_EXT_BTN, &button_info)) {
> -		psmouse_warn(psmouse, "failed to get extended button data\n");
> -		button_info = 0;
> +		psmouse_warn(psmouse, "failed to get extended button data, assuming 3 buttons are present\n");
> +		button_info = 0x33;
>  	}
>  
>  	psmouse->private = kzalloc(sizeof(struct trackpoint_data), GFP_KERNEL);

Good, this patch is confirmed to make the trackpoint middle button work on
Thinkpad e570 when applied to vanilla 4.10 kernel.

Now in order to submit the patch for inclusion to the linux kernel,
and before I'll grant a Reviewed-by: tag, it would be best when you
follow the guidelines in Documentation/SubmittingPatches  
(recently moved to : Documentation/process/submitting-patches.rst )

Attention points there are: 
2) Describe your changes 
6) No MIME 
11) Sign your work (Signed-off: line) 
14) The canonical patch format

Thanks,
Kind regards,
Ulrik

Re: Input: mouse: Trackpoint middle button not recognized on Thinkpad E470

[Julian Exner]

[-- Attachment #1: Type: text/plain, Size: 532 bytes --]

On 26/04/17 00:10, Dmitry Torokhov wrote:
> Hmm.. I do not recall seeing real trackpoints with only 2 buttons. Let's
> try defaulting to 3 and see what happens.

This patch assumes three buttons when the trackpoint_read command 
failed. This is just a quick fix. trackpoint_read may fail for other 
reasons than the extended-button-data command not being available, e.g. 
errors in the ps2 communication. A more narrow solution should check if 
a resend request was received, resend, and if the resend failed assume 
three buttons.

[-- Attachment #2: trackpoint-default-3-buttons.patch --]
[-- Type: text/x-patch, Size: 648 bytes --]

diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
index 354d47e..70c3a48 100644
--- a/drivers/input/mouse/trackpoint.c
+++ b/drivers/input/mouse/trackpoint.c
@@ -380,8 +380,8 @@ int trackpoint_detect(struct psmouse *psmouse, bool set_properties)
 		return 0;
 
 	if (trackpoint_read(&psmouse->ps2dev, TP_EXT_BTN, &button_info)) {
-		psmouse_warn(psmouse, "failed to get extended button data\n");
-		button_info = 0;
+		psmouse_warn(psmouse, "failed to get extended button data, assuming 3 buttons are present\n");
+		button_info = 0x33;
 	}
 
 	psmouse->private = kzalloc(sizeof(struct trackpoint_data), GFP_KERNEL);

Re: Input: mouse: Trackpoint middle button not recognized on Thinkpad E470

[Dmitry Torokhov]

On Tue, Apr 25, 2017 at 11:16:33PM +0200, ulrik.debie-os@e2big.org wrote:
> Hi Julian, Dmitry,
> 
> I have a similar behaviour for thinkpad e570 trackpoint. Manually forcing 
> button_info also made the middle button work.
> Julian, I planned to go the investigation road just when I happened to see
> your email. 
> 
> Dmitry, what do you suggest as a path forward when
> extended-button-data command fails ? Check for DMI and whitelist 3button
> for certain laptops which will be a growing list (2 laptops known already)? 
> Default to 3 buttons when extended-button-data command fails ?

Hmm.. I do not recall seeing real trackpoints with only 2 buttons. Let's
try defaulting to 3 and see what happens.

> 
> Julian, feel free to propose a patch, I'll review and test on E570.
> 
> Kind regards,
> Ulrik
> 
> 
> 
> On Tue, Apr 25, 2017 at 10:06:56PM +0200, Julian Exner wrote:
> > Date:   Tue, 25 Apr 2017 22:06:56 +0200
> > From: Julian Exner <jexner@techfak.uni-bielefeld.de>
> > To: "linux-input@vger.kernel.org" <linux-input@vger.kernel.org>
> > Cc: aduggan@synaptics.com
> > Subject: Re: Input: mouse: Trackpoint middle button not recognized on
> >  Thinkpad E470
> > X-Mailing-List: linux-input@vger.kernel.org
> > 
> > I checked the Trackpoint specification [1] and apparently 0xFE stands for
> > the Trackpoint controller asking to resend the command due to invalid input.
> > Resending the extended-button-data command results in 0xFC which stands for
> > another invalid input. But 0xFE does not only stand for a resend request but
> > might also mean that the issued command is not implemented in the ROM. [2]
> > 
> > So maybe when the extended-button-data command is not support, we can check
> > for certain Trackpoint or ROM ids to deduce if a middle button is present.
> > If you think this is the right approach I may craft a patch for this.
> > 
> > [1] http://blogs.epfl.ch/icenet/documents/Ykt3Eext.pdf
> > [2] http://blogs.epfl.ch/icenet/documents/Ykt3Eext.pdf#page=20
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-input" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Dmitry

Re: Input: mouse: Trackpoint middle button not recognized on Thinkpad E470

[ulrik.debie-os]

Hi Julian, Dmitry,

I have a similar behaviour for thinkpad e570 trackpoint. Manually forcing 
button_info also made the middle button work.
Julian, I planned to go the investigation road just when I happened to see
your email. 

Dmitry, what do you suggest as a path forward when
extended-button-data command fails ? Check for DMI and whitelist 3button
for certain laptops which will be a growing list (2 laptops known already)? 
Default to 3 buttons when extended-button-data command fails ?

Julian, feel free to propose a patch, I'll review and test on E570.

Kind regards,
Ulrik



On Tue, Apr 25, 2017 at 10:06:56PM +0200, Julian Exner wrote:
> Date:   Tue, 25 Apr 2017 22:06:56 +0200
> From: Julian Exner <jexner@techfak.uni-bielefeld.de>
> To: "linux-input@vger.kernel.org" <linux-input@vger.kernel.org>
> Cc: aduggan@synaptics.com
> Subject: Re: Input: mouse: Trackpoint middle button not recognized on
>  Thinkpad E470
> X-Mailing-List: linux-input@vger.kernel.org
> 
> I checked the Trackpoint specification [1] and apparently 0xFE stands for
> the Trackpoint controller asking to resend the command due to invalid input.
> Resending the extended-button-data command results in 0xFC which stands for
> another invalid input. But 0xFE does not only stand for a resend request but
> might also mean that the issued command is not implemented in the ROM. [2]
> 
> So maybe when the extended-button-data command is not support, we can check
> for certain Trackpoint or ROM ids to deduce if a middle button is present.
> If you think this is the right approach I may craft a patch for this.
> 
> [1] http://blogs.epfl.ch/icenet/documents/Ykt3Eext.pdf
> [2] http://blogs.epfl.ch/icenet/documents/Ykt3Eext.pdf#page=20
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Input: mouse: Trackpoint middle button not recognized on Thinkpad E470

[Julian Exner]

I checked the Trackpoint specification [1] and apparently 0xFE stands 
for the Trackpoint controller asking to resend the command due to 
invalid input. Resending the extended-button-data command results in 
0xFC which stands for another invalid input. But 0xFE does not only 
stand for a resend request but might also mean that the issued command 
is not implemented in the ROM. [2]

So maybe when the extended-button-data command is not support, we can 
check for certain Trackpoint or ROM ids to deduce if a middle button is 
present. If you think this is the right approach I may craft a patch for 
this.

[1] http://blogs.epfl.ch/icenet/documents/Ykt3Eext.pdf
[2] http://blogs.epfl.ch/icenet/documents/Ykt3Eext.pdf#page=20

Input: mouse: Trackpoint middle button not recognized on Thinkpad E470

[Julian Exner]

The middle trackpoint button on the Thinkpad E470 (20H2) is not 
recognized. The psmouse module warns about "failed to get extended 
button data".

I tried to dive a bit deeper into trackpoint.c and track down the issue:

- trackpoint_read for TP_EXT_BTN fails.
- This read consists of two ps2_commands, first TP_COMMAND and then the 
actual TP_EXT_BTN command
- The second ps2_command fails.
- In __ps2_command, it's the first ps2_sendbyte that fails.
- In ps2_sendbyte, serio_write returns 0. Waiting for the 
acknowledgement does not timeout. But ps2dev->nak is 0xfe.
- Therefore ps2_sendbyte fails.

 From there the whole stack unwinds, "failed to get extended button 
data" is produced and button_info is set to 0. And finally

__set_bit(BTN_MIDDLE, psmouse->dev->keybit);

is not called. Manually calling it, or setting button_info to 0x33, 
makes the middle button functional. Of course that's not a general solution.

Additionally, when loading the psmouse module with proto=imps the middle 
mouse button is detected as discussed here: 
https://bugs.freedesktop.org/show_bug.cgi?id=100694