* [PATCH 0/4] New Lenovos 2015 touchpads: party time!
@ 2015-01-28 20:10 Benjamin Tissoires
2015-01-28 20:10 ` [PATCH 1/4] Input - synaptics: fix middle button on Lenovo 2015 products Benjamin Tissoires
` (4 more replies)
0 siblings, 5 replies; 15+ messages in thread
From: Benjamin Tissoires @ 2015-01-28 20:10 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Hans de Goede, Peter Hutterer, linux-input, linux-kernel
Hi Dmitry,
Lenovo released their new models, and believe it or not, some kernel
work is required to support them!
We learned the lesson and we will try to maximize the effort in user space
through udev rules instead of polluting the kernel with exceptions.
Patches 1 & 2 are general fixes for synaptics that have been unnoticed
until this new models. They are marked as stable.
Patch 3 is a generic way to fix the PnpId list that was hardcoded in the
kernel. This one is also marked as stable so that distributions will not
have the top software buttons on devices that do not require it.
Patch 4 is a nice to have. It is not mandatory, so not CC-ed to stable,
it just reduced the PnpId list.
The good thing is that the reported min/max are correct, so the problems
with the first generation of the t440 touchpad have been fixed now (and
synaptics assured us that they will keep checking the quality of the PS/2
handling of their touchpads).
One last thing, I still did not manage to talk to the device over SMBus,
so we are still stuck with the PS/2 protocol and its limitations for this
series.
Cheers,
Benjamin
Benjamin Tissoires (4):
Input - synaptics: fix middle button on Lenovo 2015 products
Input - synaptics: do not release extra buttons once they are pressed
Input - synaptics: remove TOPBUTTONPAD property for Lenovos 2015
Input - synaptics: Remove X1 Carbon 3rd gen from the topbuttonpad list
drivers/input/mouse/synaptics.c | 20 ++++++++++++++------
drivers/input/mouse/synaptics.h | 3 +++
2 files changed, 17 insertions(+), 6 deletions(-)
--
2.1.0
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 1/4] Input - synaptics: fix middle button on Lenovo 2015 products 2015-01-28 20:10 [PATCH 0/4] New Lenovos 2015 touchpads: party time! Benjamin Tissoires @ 2015-01-28 20:10 ` Benjamin Tissoires 2015-02-02 22:54 ` Dmitry Torokhov 2015-01-28 20:10 ` [PATCH 2/4] Input - synaptics: do not release extra buttons once they are pressed Benjamin Tissoires ` (3 subsequent siblings) 4 siblings, 1 reply; 15+ messages in thread From: Benjamin Tissoires @ 2015-01-28 20:10 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Hans de Goede, Peter Hutterer, linux-input, linux-kernel On the X1 Carbon 3rd gen (with a 2015 broadwell cpu), the physical middle button of the trackstick (attached to the touchpad serio device, of course) seems to get lost. Actually, the touchpads reports 3 extra buttons, which falls in the switch below to the '2' case. Let's handle the case of odd numbers also, so that the middle button finds its way back. Cc: stable@vger.kernel.org Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> --- drivers/input/mouse/synaptics.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c index f89de89..0d12664 100644 --- a/drivers/input/mouse/synaptics.c +++ b/drivers/input/mouse/synaptics.c @@ -689,7 +689,7 @@ static int synaptics_parse_hw_state(const unsigned char buf[], if (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) && ((buf[0] ^ buf[3]) & 0x02)) { - switch (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) & ~0x01) { + switch (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap)) { default: /* * if nExtBtn is greater than 8 it should be @@ -698,15 +698,19 @@ static int synaptics_parse_hw_state(const unsigned char buf[], break; case 8: hw->ext_buttons |= ((buf[5] & 0x08)) ? 0x80 : 0; + case 7: hw->ext_buttons |= ((buf[4] & 0x08)) ? 0x40 : 0; case 6: hw->ext_buttons |= ((buf[5] & 0x04)) ? 0x20 : 0; + case 5: hw->ext_buttons |= ((buf[4] & 0x04)) ? 0x10 : 0; case 4: hw->ext_buttons |= ((buf[5] & 0x02)) ? 0x08 : 0; + case 3: hw->ext_buttons |= ((buf[4] & 0x02)) ? 0x04 : 0; case 2: hw->ext_buttons |= ((buf[5] & 0x01)) ? 0x02 : 0; + case 1: hw->ext_buttons |= ((buf[4] & 0x01)) ? 0x01 : 0; } } -- 2.1.0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] Input - synaptics: fix middle button on Lenovo 2015 products 2015-01-28 20:10 ` [PATCH 1/4] Input - synaptics: fix middle button on Lenovo 2015 products Benjamin Tissoires @ 2015-02-02 22:54 ` Dmitry Torokhov 2015-02-04 19:43 ` Benjamin Tissoires 0 siblings, 1 reply; 15+ messages in thread From: Dmitry Torokhov @ 2015-02-02 22:54 UTC (permalink / raw) To: Benjamin Tissoires Cc: Hans de Goede, Peter Hutterer, linux-input, linux-kernel On Wed, Jan 28, 2015 at 03:10:04PM -0500, Benjamin Tissoires wrote: > On the X1 Carbon 3rd gen (with a 2015 broadwell cpu), the physical middle > button of the trackstick (attached to the touchpad serio device, of course) > seems to get lost. > > Actually, the touchpads reports 3 extra buttons, which falls in the switch > below to the '2' case. Let's handle the case of odd numbers also, so that > the middle button finds its way back. > > Cc: stable@vger.kernel.org > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > --- > drivers/input/mouse/synaptics.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c > index f89de89..0d12664 100644 > --- a/drivers/input/mouse/synaptics.c > +++ b/drivers/input/mouse/synaptics.c > @@ -689,7 +689,7 @@ static int synaptics_parse_hw_state(const unsigned char buf[], > > if (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) && > ((buf[0] ^ buf[3]) & 0x02)) { > - switch (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) & ~0x01) { > + switch (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap)) { > default: > /* > * if nExtBtn is greater than 8 it should be > @@ -698,15 +698,19 @@ static int synaptics_parse_hw_state(const unsigned char buf[], > break; > case 8: > hw->ext_buttons |= ((buf[5] & 0x08)) ? 0x80 : 0; > + case 7: > hw->ext_buttons |= ((buf[4] & 0x08)) ? 0x40 : 0; > case 6: > hw->ext_buttons |= ((buf[5] & 0x04)) ? 0x20 : 0; > + case 5: > hw->ext_buttons |= ((buf[4] & 0x04)) ? 0x10 : 0; > case 4: > hw->ext_buttons |= ((buf[5] & 0x02)) ? 0x08 : 0; > + case 3: > hw->ext_buttons |= ((buf[4] & 0x02)) ? 0x04 : 0; > case 2: > hw->ext_buttons |= ((buf[5] & 0x01)) ? 0x02 : 0; > + case 1: > hw->ext_buttons |= ((buf[4] & 0x01)) ? 0x01 : 0; > } > } Hmm, that switch has gotten unwieldy. Maybe we can do someting like this: diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c index a039cb7..1250a5d 100644 --- a/drivers/input/mouse/synaptics.c +++ b/drivers/input/mouse/synaptics.c @@ -608,6 +608,18 @@ static void synaptics_parse_agm(const unsigned char buf[], } } +static void synaptics_parse_ext_buttons(const unsigned char buf[], + struct synaptics_data *priv, + struct synaptics_hw_state *hw) +{ + unsigned int ext_bits = + (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) + 1) >> 1; + unsigned int ext_mask = GENMASK(ext_bits - 1, 0); + + hw->ext_buttons = buf[4] & ext_mask; + hw->ext_buttons |= (buf[4] & ext_mask) << ext_bits; +} + static int synaptics_parse_hw_state(const unsigned char buf[], struct synaptics_data *priv, struct synaptics_hw_state *hw) @@ -692,28 +704,9 @@ static int synaptics_parse_hw_state(const unsigned char buf[], hw->down = ((buf[0] ^ buf[3]) & 0x02) ? 1 : 0; } - if (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) && + if (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) > 0 && ((buf[0] ^ buf[3]) & 0x02)) { - switch (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) & ~0x01) { - default: - /* - * if nExtBtn is greater than 8 it should be - * considered invalid and treated as 0 - */ - break; - case 8: - hw->ext_buttons |= ((buf[5] & 0x08)) ? 0x80 : 0; - hw->ext_buttons |= ((buf[4] & 0x08)) ? 0x40 : 0; - case 6: - hw->ext_buttons |= ((buf[5] & 0x04)) ? 0x20 : 0; - hw->ext_buttons |= ((buf[4] & 0x04)) ? 0x10 : 0; - case 4: - hw->ext_buttons |= ((buf[5] & 0x02)) ? 0x08 : 0; - hw->ext_buttons |= ((buf[4] & 0x02)) ? 0x04 : 0; - case 2: - hw->ext_buttons |= ((buf[5] & 0x01)) ? 0x02 : 0; - hw->ext_buttons |= ((buf[4] & 0x01)) ? 0x01 : 0; - } + synaptics_parse_ext_buttons(buf, priv, hw); } } else { hw->x = (((buf[1] & 0x1f) << 8) | buf[2]); @@ -780,6 +773,7 @@ static void synaptics_report_buttons(struct psmouse *psmouse, { struct input_dev *dev = psmouse->dev; struct synaptics_data *priv = psmouse->private; + int ext_bits = (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) + 1) >> 1; int i; input_report_key(dev, BTN_LEFT, hw->left); @@ -793,8 +787,12 @@ static void synaptics_report_buttons(struct psmouse *psmouse, input_report_key(dev, BTN_BACK, hw->down); } - for (i = 0; i < SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap); i++) - input_report_key(dev, BTN_0 + i, hw->ext_buttons & (1 << i)); + for (i = 0; i < ext_bits; i++) { + input_report_key(dev, BTN_0 + 2 * i, + hw->ext_buttons & (1 << i)); + input_report_key(dev, BTN_1 + 2 * i, + hw->ext_buttons & (1 << (i + ext_bits))); + } } static void synaptics_report_mt_data(struct psmouse *psmouse, Thanks. -- Dmitry ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] Input - synaptics: fix middle button on Lenovo 2015 products 2015-02-02 22:54 ` Dmitry Torokhov @ 2015-02-04 19:43 ` Benjamin Tissoires 2015-02-04 19:53 ` Dmitry Torokhov 0 siblings, 1 reply; 15+ messages in thread From: Benjamin Tissoires @ 2015-02-04 19:43 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Hans de Goede, Peter Hutterer, linux-input, linux-kernel On Feb 02 2015 or thereabouts, Dmitry Torokhov wrote: > On Wed, Jan 28, 2015 at 03:10:04PM -0500, Benjamin Tissoires wrote: > > On the X1 Carbon 3rd gen (with a 2015 broadwell cpu), the physical middle > > button of the trackstick (attached to the touchpad serio device, of course) > > seems to get lost. > > > > Actually, the touchpads reports 3 extra buttons, which falls in the switch > > below to the '2' case. Let's handle the case of odd numbers also, so that > > the middle button finds its way back. > > > > Cc: stable@vger.kernel.org > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > --- > > drivers/input/mouse/synaptics.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c > > index f89de89..0d12664 100644 > > --- a/drivers/input/mouse/synaptics.c > > +++ b/drivers/input/mouse/synaptics.c > > @@ -689,7 +689,7 @@ static int synaptics_parse_hw_state(const unsigned char buf[], > > > > if (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) && > > ((buf[0] ^ buf[3]) & 0x02)) { > > - switch (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) & ~0x01) { > > + switch (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap)) { > > default: > > /* > > * if nExtBtn is greater than 8 it should be > > @@ -698,15 +698,19 @@ static int synaptics_parse_hw_state(const unsigned char buf[], > > break; > > case 8: > > hw->ext_buttons |= ((buf[5] & 0x08)) ? 0x80 : 0; > > + case 7: > > hw->ext_buttons |= ((buf[4] & 0x08)) ? 0x40 : 0; > > case 6: > > hw->ext_buttons |= ((buf[5] & 0x04)) ? 0x20 : 0; > > + case 5: > > hw->ext_buttons |= ((buf[4] & 0x04)) ? 0x10 : 0; > > case 4: > > hw->ext_buttons |= ((buf[5] & 0x02)) ? 0x08 : 0; > > + case 3: > > hw->ext_buttons |= ((buf[4] & 0x02)) ? 0x04 : 0; > > case 2: > > hw->ext_buttons |= ((buf[5] & 0x01)) ? 0x02 : 0; > > + case 1: > > hw->ext_buttons |= ((buf[4] & 0x01)) ? 0x01 : 0; > > } > > } > > Hmm, that switch has gotten unwieldy. Maybe we can do someting like > this: Minus a typo, it works too. Though I must say even if the switch is ugly, it's a no brainer to understand what is this about. Here, it requires a little bit more skills :) > > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c > index a039cb7..1250a5d 100644 > --- a/drivers/input/mouse/synaptics.c > +++ b/drivers/input/mouse/synaptics.c > @@ -608,6 +608,18 @@ static void synaptics_parse_agm(const unsigned char buf[], > } > } > > +static void synaptics_parse_ext_buttons(const unsigned char buf[], > + struct synaptics_data *priv, > + struct synaptics_hw_state *hw) > +{ > + unsigned int ext_bits = > + (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) + 1) >> 1; > + unsigned int ext_mask = GENMASK(ext_bits - 1, 0); > + > + hw->ext_buttons = buf[4] & ext_mask; > + hw->ext_buttons |= (buf[4] & ext_mask) << ext_bits; typo: this should read (buf[5] & ext_mask) << ext_bits; (not buf[4]) You can add my Tested-by on this one when applying. Cheers, Benjamin > +} > + > static int synaptics_parse_hw_state(const unsigned char buf[], > struct synaptics_data *priv, > struct synaptics_hw_state *hw) > @@ -692,28 +704,9 @@ static int synaptics_parse_hw_state(const unsigned char buf[], > hw->down = ((buf[0] ^ buf[3]) & 0x02) ? 1 : 0; > } > > - if (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) && > + if (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) > 0 && > ((buf[0] ^ buf[3]) & 0x02)) { > - switch (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) & ~0x01) { > - default: > - /* > - * if nExtBtn is greater than 8 it should be > - * considered invalid and treated as 0 > - */ > - break; > - case 8: > - hw->ext_buttons |= ((buf[5] & 0x08)) ? 0x80 : 0; > - hw->ext_buttons |= ((buf[4] & 0x08)) ? 0x40 : 0; > - case 6: > - hw->ext_buttons |= ((buf[5] & 0x04)) ? 0x20 : 0; > - hw->ext_buttons |= ((buf[4] & 0x04)) ? 0x10 : 0; > - case 4: > - hw->ext_buttons |= ((buf[5] & 0x02)) ? 0x08 : 0; > - hw->ext_buttons |= ((buf[4] & 0x02)) ? 0x04 : 0; > - case 2: > - hw->ext_buttons |= ((buf[5] & 0x01)) ? 0x02 : 0; > - hw->ext_buttons |= ((buf[4] & 0x01)) ? 0x01 : 0; > - } > + synaptics_parse_ext_buttons(buf, priv, hw); > } > } else { > hw->x = (((buf[1] & 0x1f) << 8) | buf[2]); > @@ -780,6 +773,7 @@ static void synaptics_report_buttons(struct psmouse *psmouse, > { > struct input_dev *dev = psmouse->dev; > struct synaptics_data *priv = psmouse->private; > + int ext_bits = (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) + 1) >> 1; > int i; > > input_report_key(dev, BTN_LEFT, hw->left); > @@ -793,8 +787,12 @@ static void synaptics_report_buttons(struct psmouse *psmouse, > input_report_key(dev, BTN_BACK, hw->down); > } > > - for (i = 0; i < SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap); i++) > - input_report_key(dev, BTN_0 + i, hw->ext_buttons & (1 << i)); > + for (i = 0; i < ext_bits; i++) { > + input_report_key(dev, BTN_0 + 2 * i, > + hw->ext_buttons & (1 << i)); > + input_report_key(dev, BTN_1 + 2 * i, > + hw->ext_buttons & (1 << (i + ext_bits))); > + } > } > > static void synaptics_report_mt_data(struct psmouse *psmouse, > > Thanks. > > -- > Dmitry ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/4] Input - synaptics: fix middle button on Lenovo 2015 products 2015-02-04 19:43 ` Benjamin Tissoires @ 2015-02-04 19:53 ` Dmitry Torokhov 0 siblings, 0 replies; 15+ messages in thread From: Dmitry Torokhov @ 2015-02-04 19:53 UTC (permalink / raw) To: Benjamin Tissoires Cc: Hans de Goede, Peter Hutterer, linux-input, linux-kernel On Wed, Feb 04, 2015 at 02:43:32PM -0500, Benjamin Tissoires wrote: > On Feb 02 2015 or thereabouts, Dmitry Torokhov wrote: > > On Wed, Jan 28, 2015 at 03:10:04PM -0500, Benjamin Tissoires wrote: > > > On the X1 Carbon 3rd gen (with a 2015 broadwell cpu), the physical middle > > > button of the trackstick (attached to the touchpad serio device, of course) > > > seems to get lost. > > > > > > Actually, the touchpads reports 3 extra buttons, which falls in the switch > > > below to the '2' case. Let's handle the case of odd numbers also, so that > > > the middle button finds its way back. > > > > > > Cc: stable@vger.kernel.org > > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > > --- > > > drivers/input/mouse/synaptics.c | 6 +++++- > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c > > > index f89de89..0d12664 100644 > > > --- a/drivers/input/mouse/synaptics.c > > > +++ b/drivers/input/mouse/synaptics.c > > > @@ -689,7 +689,7 @@ static int synaptics_parse_hw_state(const unsigned char buf[], > > > > > > if (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) && > > > ((buf[0] ^ buf[3]) & 0x02)) { > > > - switch (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) & ~0x01) { > > > + switch (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap)) { > > > default: > > > /* > > > * if nExtBtn is greater than 8 it should be > > > @@ -698,15 +698,19 @@ static int synaptics_parse_hw_state(const unsigned char buf[], > > > break; > > > case 8: > > > hw->ext_buttons |= ((buf[5] & 0x08)) ? 0x80 : 0; > > > + case 7: > > > hw->ext_buttons |= ((buf[4] & 0x08)) ? 0x40 : 0; > > > case 6: > > > hw->ext_buttons |= ((buf[5] & 0x04)) ? 0x20 : 0; > > > + case 5: > > > hw->ext_buttons |= ((buf[4] & 0x04)) ? 0x10 : 0; > > > case 4: > > > hw->ext_buttons |= ((buf[5] & 0x02)) ? 0x08 : 0; > > > + case 3: > > > hw->ext_buttons |= ((buf[4] & 0x02)) ? 0x04 : 0; > > > case 2: > > > hw->ext_buttons |= ((buf[5] & 0x01)) ? 0x02 : 0; > > > + case 1: > > > hw->ext_buttons |= ((buf[4] & 0x01)) ? 0x01 : 0; > > > } > > > } > > > > Hmm, that switch has gotten unwieldy. Maybe we can do someting like > > this: > > Minus a typo, it works too. > > Though I must say even if the switch is ugly, it's a no brainer to > understand what is this about. Here, it requires a little bit more > skills :) Yeah, it really wasn't about making code that much clearer but rather about number of operations we need to make to figure button state. Switch plus bazillion of ternary operations results in many many branches, the new version does not have any. > > > > > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c > > index a039cb7..1250a5d 100644 > > --- a/drivers/input/mouse/synaptics.c > > +++ b/drivers/input/mouse/synaptics.c > > @@ -608,6 +608,18 @@ static void synaptics_parse_agm(const unsigned char buf[], > > } > > } > > > > +static void synaptics_parse_ext_buttons(const unsigned char buf[], > > + struct synaptics_data *priv, > > + struct synaptics_hw_state *hw) > > +{ > > + unsigned int ext_bits = > > + (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) + 1) >> 1; > > + unsigned int ext_mask = GENMASK(ext_bits - 1, 0); > > + > > + hw->ext_buttons = buf[4] & ext_mask; > > + hw->ext_buttons |= (buf[4] & ext_mask) << ext_bits; > > typo: this should read (buf[5] & ext_mask) << ext_bits; (not buf[4]) Ah, yes, indeed! Thanks for spotting this. > > You can add my Tested-by on this one when applying. Great, I'll queue it then. > > Cheers, > Benjamin > > > +} > > + > > static int synaptics_parse_hw_state(const unsigned char buf[], > > struct synaptics_data *priv, > > struct synaptics_hw_state *hw) > > @@ -692,28 +704,9 @@ static int synaptics_parse_hw_state(const unsigned char buf[], > > hw->down = ((buf[0] ^ buf[3]) & 0x02) ? 1 : 0; > > } > > > > - if (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) && > > + if (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) > 0 && > > ((buf[0] ^ buf[3]) & 0x02)) { > > - switch (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) & ~0x01) { > > - default: > > - /* > > - * if nExtBtn is greater than 8 it should be > > - * considered invalid and treated as 0 > > - */ > > - break; > > - case 8: > > - hw->ext_buttons |= ((buf[5] & 0x08)) ? 0x80 : 0; > > - hw->ext_buttons |= ((buf[4] & 0x08)) ? 0x40 : 0; > > - case 6: > > - hw->ext_buttons |= ((buf[5] & 0x04)) ? 0x20 : 0; > > - hw->ext_buttons |= ((buf[4] & 0x04)) ? 0x10 : 0; > > - case 4: > > - hw->ext_buttons |= ((buf[5] & 0x02)) ? 0x08 : 0; > > - hw->ext_buttons |= ((buf[4] & 0x02)) ? 0x04 : 0; > > - case 2: > > - hw->ext_buttons |= ((buf[5] & 0x01)) ? 0x02 : 0; > > - hw->ext_buttons |= ((buf[4] & 0x01)) ? 0x01 : 0; > > - } > > + synaptics_parse_ext_buttons(buf, priv, hw); > > } > > } else { > > hw->x = (((buf[1] & 0x1f) << 8) | buf[2]); > > @@ -780,6 +773,7 @@ static void synaptics_report_buttons(struct psmouse *psmouse, > > { > > struct input_dev *dev = psmouse->dev; > > struct synaptics_data *priv = psmouse->private; > > + int ext_bits = (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) + 1) >> 1; > > int i; > > > > input_report_key(dev, BTN_LEFT, hw->left); > > @@ -793,8 +787,12 @@ static void synaptics_report_buttons(struct psmouse *psmouse, > > input_report_key(dev, BTN_BACK, hw->down); > > } > > > > - for (i = 0; i < SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap); i++) > > - input_report_key(dev, BTN_0 + i, hw->ext_buttons & (1 << i)); > > + for (i = 0; i < ext_bits; i++) { > > + input_report_key(dev, BTN_0 + 2 * i, > > + hw->ext_buttons & (1 << i)); > > + input_report_key(dev, BTN_1 + 2 * i, > > + hw->ext_buttons & (1 << (i + ext_bits))); > > + } > > } > > > > static void synaptics_report_mt_data(struct psmouse *psmouse, > > > > Thanks. > > > > -- > > Dmitry -- Dmitry ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/4] Input - synaptics: do not release extra buttons once they are pressed 2015-01-28 20:10 [PATCH 0/4] New Lenovos 2015 touchpads: party time! Benjamin Tissoires 2015-01-28 20:10 ` [PATCH 1/4] Input - synaptics: fix middle button on Lenovo 2015 products Benjamin Tissoires @ 2015-01-28 20:10 ` Benjamin Tissoires 2015-02-02 21:46 ` Dmitry Torokhov 2015-01-28 20:10 ` [PATCH 3/4] Input - synaptics: remove TOPBUTTONPAD property for Lenovos 2015 Benjamin Tissoires ` (2 subsequent siblings) 4 siblings, 1 reply; 15+ messages in thread From: Benjamin Tissoires @ 2015-01-28 20:10 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Hans de Goede, Peter Hutterer, linux-input, linux-kernel The current code releases the extra buttons right after they are pressed. As soon as a new serio report comes in, the hw state is reset to 0 and so the buttons are released. Check for the report type before acting on the current extra buttons state. Cc: stable@vger.kernel.org Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> --- drivers/input/mouse/synaptics.c | 10 +++++++--- drivers/input/mouse/synaptics.h | 3 +++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c index 0d12664..d58863b 100644 --- a/drivers/input/mouse/synaptics.c +++ b/drivers/input/mouse/synaptics.c @@ -688,7 +688,7 @@ static int synaptics_parse_hw_state(const unsigned char buf[], } if (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) && - ((buf[0] ^ buf[3]) & 0x02)) { + SYN_REPORT_EXT_BUTTONS(buf)) { switch (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap)) { default: /* @@ -792,8 +792,12 @@ static void synaptics_report_buttons(struct psmouse *psmouse, input_report_key(dev, BTN_BACK, hw->down); } - for (i = 0; i < SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap); i++) - input_report_key(dev, BTN_0 + i, hw->ext_buttons & (1 << i)); + if (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) && + SYN_REPORT_EXT_BUTTONS(psmouse->packet)) { + for (i = 0; i < SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap); i++) + input_report_key(dev, BTN_0 + i, + hw->ext_buttons & (1 << i)); + } } static void synaptics_report_mt_data(struct psmouse *psmouse, diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h index 6faf9bb..0b2b711 100644 --- a/drivers/input/mouse/synaptics.h +++ b/drivers/input/mouse/synaptics.h @@ -115,6 +115,9 @@ #define SYN_NEWABS_RELAXED 2 #define SYN_OLDABS 3 +/* synaptics extended button packet */ +#define SYN_REPORT_EXT_BUTTONS(buf) (((buf)[0] ^ (buf)[3]) & 0x02) + /* amount to fuzz position data when touchpad reports reduced filtering */ #define SYN_REDUCED_FILTER_FUZZ 8 -- 2.1.0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] Input - synaptics: do not release extra buttons once they are pressed 2015-01-28 20:10 ` [PATCH 2/4] Input - synaptics: do not release extra buttons once they are pressed Benjamin Tissoires @ 2015-02-02 21:46 ` Dmitry Torokhov 2015-02-02 22:14 ` Benjamin Tissoires 0 siblings, 1 reply; 15+ messages in thread From: Dmitry Torokhov @ 2015-02-02 21:46 UTC (permalink / raw) To: Benjamin Tissoires Cc: Hans de Goede, Peter Hutterer, linux-input, linux-kernel On Wed, Jan 28, 2015 at 03:10:05PM -0500, Benjamin Tissoires wrote: > The current code releases the extra buttons right after they are pressed. > As soon as a new serio report comes in, the hw state is reset to 0 > and so the buttons are released. > > Check for the report type before acting on the current extra buttons > state. No: "If Ext is 0 (if bit 1 of bytes 1 and 4 are the same) then there are no external buttons being pressed (or all external buttons have been released)." - Synaptics PS/2 TouchPad Interfacing Guide PN: 511-000275-01 Rev. B Thanks. > > Cc: stable@vger.kernel.org > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > --- > drivers/input/mouse/synaptics.c | 10 +++++++--- > drivers/input/mouse/synaptics.h | 3 +++ > 2 files changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c > index 0d12664..d58863b 100644 > --- a/drivers/input/mouse/synaptics.c > +++ b/drivers/input/mouse/synaptics.c > @@ -688,7 +688,7 @@ static int synaptics_parse_hw_state(const unsigned char buf[], > } > > if (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) && > - ((buf[0] ^ buf[3]) & 0x02)) { > + SYN_REPORT_EXT_BUTTONS(buf)) { > switch (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap)) { > default: > /* > @@ -792,8 +792,12 @@ static void synaptics_report_buttons(struct psmouse *psmouse, > input_report_key(dev, BTN_BACK, hw->down); > } > > - for (i = 0; i < SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap); i++) > - input_report_key(dev, BTN_0 + i, hw->ext_buttons & (1 << i)); > + if (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) && > + SYN_REPORT_EXT_BUTTONS(psmouse->packet)) { > + for (i = 0; i < SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap); i++) > + input_report_key(dev, BTN_0 + i, > + hw->ext_buttons & (1 << i)); > + } > } > > static void synaptics_report_mt_data(struct psmouse *psmouse, > diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h > index 6faf9bb..0b2b711 100644 > --- a/drivers/input/mouse/synaptics.h > +++ b/drivers/input/mouse/synaptics.h > @@ -115,6 +115,9 @@ > #define SYN_NEWABS_RELAXED 2 > #define SYN_OLDABS 3 > > +/* synaptics extended button packet */ > +#define SYN_REPORT_EXT_BUTTONS(buf) (((buf)[0] ^ (buf)[3]) & 0x02) > + > /* amount to fuzz position data when touchpad reports reduced filtering */ > #define SYN_REDUCED_FILTER_FUZZ 8 > > -- > 2.1.0 > -- Dmitry ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] Input - synaptics: do not release extra buttons once they are pressed 2015-02-02 21:46 ` Dmitry Torokhov @ 2015-02-02 22:14 ` Benjamin Tissoires 2015-02-04 21:29 ` Benjamin Tissoires 0 siblings, 1 reply; 15+ messages in thread From: Benjamin Tissoires @ 2015-02-02 22:14 UTC (permalink / raw) To: Dmitry Torokhov, Andrew Duggan Cc: Benjamin Tissoires, Hans de Goede, Peter Hutterer, linux-input, linux-kernel On Mon, Feb 2, 2015 at 4:46 PM, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > On Wed, Jan 28, 2015 at 03:10:05PM -0500, Benjamin Tissoires wrote: >> The current code releases the extra buttons right after they are pressed. >> As soon as a new serio report comes in, the hw state is reset to 0 >> and so the buttons are released. >> >> Check for the report type before acting on the current extra buttons >> state. > > No: > > "If Ext is 0 (if bit 1 of bytes 1 and 4 are the same) then there are no > external buttons being pressed (or all external buttons have been > released)." - Synaptics PS/2 TouchPad Interfacing Guide PN: > 511-000275-01 Rev. B > Hmm, indeed, the current code follows the spec. Yeah! a new firmware bug! So when I dumped the ps/2 reports, I clearly saw the release report being sent with the Ext bit to 1, and other reports were in between with no information (because no fingers were on the touchpad). I may have access to the hardware tomorrow if the snow has been removed from the roads, but tonight, I won't be able to test anything :( Cheers, Benjamin > >> >> Cc: stable@vger.kernel.org >> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> >> --- >> drivers/input/mouse/synaptics.c | 10 +++++++--- >> drivers/input/mouse/synaptics.h | 3 +++ >> 2 files changed, 10 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c >> index 0d12664..d58863b 100644 >> --- a/drivers/input/mouse/synaptics.c >> +++ b/drivers/input/mouse/synaptics.c >> @@ -688,7 +688,7 @@ static int synaptics_parse_hw_state(const unsigned char buf[], >> } >> >> if (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) && >> - ((buf[0] ^ buf[3]) & 0x02)) { >> + SYN_REPORT_EXT_BUTTONS(buf)) { >> switch (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap)) { >> default: >> /* >> @@ -792,8 +792,12 @@ static void synaptics_report_buttons(struct psmouse *psmouse, >> input_report_key(dev, BTN_BACK, hw->down); >> } >> >> - for (i = 0; i < SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap); i++) >> - input_report_key(dev, BTN_0 + i, hw->ext_buttons & (1 << i)); >> + if (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) && >> + SYN_REPORT_EXT_BUTTONS(psmouse->packet)) { >> + for (i = 0; i < SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap); i++) >> + input_report_key(dev, BTN_0 + i, >> + hw->ext_buttons & (1 << i)); >> + } >> } >> >> static void synaptics_report_mt_data(struct psmouse *psmouse, >> diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h >> index 6faf9bb..0b2b711 100644 >> --- a/drivers/input/mouse/synaptics.h >> +++ b/drivers/input/mouse/synaptics.h >> @@ -115,6 +115,9 @@ >> #define SYN_NEWABS_RELAXED 2 >> #define SYN_OLDABS 3 >> >> +/* synaptics extended button packet */ >> +#define SYN_REPORT_EXT_BUTTONS(buf) (((buf)[0] ^ (buf)[3]) & 0x02) >> + >> /* amount to fuzz position data when touchpad reports reduced filtering */ >> #define SYN_REDUCED_FILTER_FUZZ 8 >> >> -- >> 2.1.0 >> > > -- > Dmitry > -- > 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] Input - synaptics: do not release extra buttons once they are pressed 2015-02-02 22:14 ` Benjamin Tissoires @ 2015-02-04 21:29 ` Benjamin Tissoires 2015-02-05 1:53 ` Andrew Duggan 2015-02-06 1:54 ` Andrew Duggan 0 siblings, 2 replies; 15+ messages in thread From: Benjamin Tissoires @ 2015-02-04 21:29 UTC (permalink / raw) To: Dmitry Torokhov, Andrew Duggan Cc: Hans de Goede, Peter Hutterer, linux-input, linux-kernel On Feb 02 2015 or thereabouts, Benjamin Tissoires wrote: > On Mon, Feb 2, 2015 at 4:46 PM, Dmitry Torokhov > <dmitry.torokhov@gmail.com> wrote: > > On Wed, Jan 28, 2015 at 03:10:05PM -0500, Benjamin Tissoires wrote: > >> The current code releases the extra buttons right after they are pressed. > >> As soon as a new serio report comes in, the hw state is reset to 0 > >> and so the buttons are released. > >> > >> Check for the report type before acting on the current extra buttons > >> state. > > > > No: > > > > "If Ext is 0 (if bit 1 of bytes 1 and 4 are the same) then there are no > > external buttons being pressed (or all external buttons have been > > released)." - Synaptics PS/2 TouchPad Interfacing Guide PN: > > 511-000275-01 Rev. B > > > > Hmm, indeed, the current code follows the spec. Yeah! a new firmware bug! > > So when I dumped the ps/2 reports, I clearly saw the release report > being sent with the Ext bit to 1, and other reports were in between > with no information (because no fingers were on the touchpad). > OK, so with the hardware, here are the results: After each incoming event (being a button or touch), when there is no more information to send (i.e. touch released or button pressed or released), I receive 80 times the following buffer: 80 00 00 c0 00 00 The number 80 seems quite consistant, though I got once 96. Not sure that this repeated buffer might be of interest however. When a finger is touched on the sensor, I receive the following: b0 ba 35 c0 8d ed <- first finger down on the sensor .... b0 ba 3b c0 6f d0 b0 ba 3b c2 6d d0 <- button 1 pressed b0 ba 3b c0 6f d0 .... b0 ba 3b c0 6f d0 b0 ba 3c c2 6c d0 <- button 1 released b0 ba 3c c0 6f d0 .... 90 ba 26 c0 6f d0 80 00 00 c0 00 00 <- first finger released from the sensor 80 00 00 c0 00 00 <- repeated 80 times So here, either the spec is wrong, either the Synaptics with FW 8.1 in the Lenovos are not following it. But I clearly see that the extended buttons are reported only when the Ext bit is 1. Andrew, could you help us determine which way to go? Ideally, could you point out at a firmware version where we could be sure that the spec has not been followed so we can add a quirk in the driver? Cheers, Benjamin ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] Input - synaptics: do not release extra buttons once they are pressed 2015-02-04 21:29 ` Benjamin Tissoires @ 2015-02-05 1:53 ` Andrew Duggan 2015-02-06 1:54 ` Andrew Duggan 1 sibling, 0 replies; 15+ messages in thread From: Andrew Duggan @ 2015-02-05 1:53 UTC (permalink / raw) To: Benjamin Tissoires, Dmitry Torokhov Cc: Hans de Goede, Peter Hutterer, linux-input, linux-kernel On 02/04/2015 01:29 PM, Benjamin Tissoires wrote: > On Feb 02 2015 or thereabouts, Benjamin Tissoires wrote: >> On Mon, Feb 2, 2015 at 4:46 PM, Dmitry Torokhov >> <dmitry.torokhov@gmail.com> wrote: >>> On Wed, Jan 28, 2015 at 03:10:05PM -0500, Benjamin Tissoires wrote: >>>> The current code releases the extra buttons right after they are pressed. >>>> As soon as a new serio report comes in, the hw state is reset to 0 >>>> and so the buttons are released. >>>> >>>> Check for the report type before acting on the current extra buttons >>>> state. >>> No: >>> >>> "If Ext is 0 (if bit 1 of bytes 1 and 4 are the same) then there are no >>> external buttons being pressed (or all external buttons have been >>> released)." - Synaptics PS/2 TouchPad Interfacing Guide PN: >>> 511-000275-01 Rev. B >>> >> Hmm, indeed, the current code follows the spec. Yeah! a new firmware bug! >> >> So when I dumped the ps/2 reports, I clearly saw the release report >> being sent with the Ext bit to 1, and other reports were in between >> with no information (because no fingers were on the touchpad). >> > OK, so with the hardware, here are the results: > > After each incoming event (being a button or touch), when there is no > more information to send (i.e. touch released or button pressed or > released), I receive 80 times the following buffer: > 80 00 00 c0 00 00 Yeah, this is normal. The firmware reports this for 1 second after a finger lift. The last paragraph of 3.2 mentions this behavior. > The number 80 seems quite consistant, though I got once 96. > Not sure that this repeated buffer might be of interest however. > > When a finger is touched on the sensor, I receive the following: > > b0 ba 35 c0 8d ed <- first finger down on the sensor > .... > b0 ba 3b c0 6f d0 > b0 ba 3b c2 6d d0 <- button 1 pressed > b0 ba 3b c0 6f d0 > .... > b0 ba 3b c0 6f d0 > b0 ba 3c c2 6c d0 <- button 1 released > b0 ba 3c c0 6f d0 > .... > 90 ba 26 c0 6f d0 > 80 00 00 c0 00 00 <- first finger released from the sensor > 80 00 00 c0 00 00 <- repeated 80 times > > So here, either the spec is wrong, either the Synaptics with FW 8.1 in > the Lenovos are not following it. But I clearly see that the extended > buttons are reported only when the Ext bit is 1. I will have to clarify with those who are more familiar with PS/2. The spec might not be up to date since it looks like it was last updated in 2011. Andrew > > Andrew, could you help us determine which way to go? > Ideally, could you point out at a firmware version where we could be > sure that the spec has not been followed so we can add a quirk in the > driver? > > Cheers, > Benjamin ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/4] Input - synaptics: do not release extra buttons once they are pressed 2015-02-04 21:29 ` Benjamin Tissoires 2015-02-05 1:53 ` Andrew Duggan @ 2015-02-06 1:54 ` Andrew Duggan 1 sibling, 0 replies; 15+ messages in thread From: Andrew Duggan @ 2015-02-06 1:54 UTC (permalink / raw) To: Benjamin Tissoires, Dmitry Torokhov Cc: Hans de Goede, Peter Hutterer, linux-input, linux-kernel On 02/04/2015 01:29 PM, Benjamin Tissoires wrote: > On Feb 02 2015 or thereabouts, Benjamin Tissoires wrote: >> On Mon, Feb 2, 2015 at 4:46 PM, Dmitry Torokhov >> <dmitry.torokhov@gmail.com> wrote: >>> On Wed, Jan 28, 2015 at 03:10:05PM -0500, Benjamin Tissoires wrote: >>>> The current code releases the extra buttons right after they are pressed. >>>> As soon as a new serio report comes in, the hw state is reset to 0 >>>> and so the buttons are released. >>>> >>>> Check for the report type before acting on the current extra buttons >>>> state. >>> No: >>> >>> "If Ext is 0 (if bit 1 of bytes 1 and 4 are the same) then there are no >>> external buttons being pressed (or all external buttons have been >>> released)." - Synaptics PS/2 TouchPad Interfacing Guide PN: >>> 511-000275-01 Rev. B >>> >> Hmm, indeed, the current code follows the spec. Yeah! a new firmware bug! >> >> So when I dumped the ps/2 reports, I clearly saw the release report >> being sent with the Ext bit to 1, and other reports were in between >> with no information (because no fingers were on the touchpad). >> > OK, so with the hardware, here are the results: > > After each incoming event (being a button or touch), when there is no > more information to send (i.e. touch released or button pressed or > released), I receive 80 times the following buffer: > 80 00 00 c0 00 00 > > The number 80 seems quite consistant, though I got once 96. > Not sure that this repeated buffer might be of interest however. > > When a finger is touched on the sensor, I receive the following: > > b0 ba 35 c0 8d ed <- first finger down on the sensor > .... > b0 ba 3b c0 6f d0 > b0 ba 3b c2 6d d0 <- button 1 pressed > b0 ba 3b c0 6f d0 > .... > b0 ba 3b c0 6f d0 > b0 ba 3c c2 6c d0 <- button 1 released > b0 ba 3c c0 6f d0 > .... > 90 ba 26 c0 6f d0 > 80 00 00 c0 00 00 <- first finger released from the sensor > 80 00 00 c0 00 00 <- repeated 80 times > > So here, either the spec is wrong, either the Synaptics with FW 8.1 in > the Lenovos are not following it. But I clearly see that the extended > buttons are reported only when the Ext bit is 1. > > Andrew, could you help us determine which way to go? > Ideally, could you point out at a firmware version where we could be > sure that the spec has not been followed so we can add a quirk in the > driver? I got confirmation that this is a firmware bug and that it has probably been there since 2011. This bug is definitely in 8.1 and probably several versions before it. But, we haven't shipped anything with extended buttons in a long time, so applying the quirk to firmware revision 8.1 should be adequate. When we fix the bug we will bump the minor version. Andrew > Cheers, > Benjamin ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/4] Input - synaptics: remove TOPBUTTONPAD property for Lenovos 2015 2015-01-28 20:10 [PATCH 0/4] New Lenovos 2015 touchpads: party time! Benjamin Tissoires 2015-01-28 20:10 ` [PATCH 1/4] Input - synaptics: fix middle button on Lenovo 2015 products Benjamin Tissoires 2015-01-28 20:10 ` [PATCH 2/4] Input - synaptics: do not release extra buttons once they are pressed Benjamin Tissoires @ 2015-01-28 20:10 ` Benjamin Tissoires 2015-02-02 21:59 ` Dmitry Torokhov 2015-01-28 20:10 ` [PATCH 4/4] Input - synaptics: Remove X1 Carbon 3rd gen from the topbuttonpad list Benjamin Tissoires 2015-01-29 7:52 ` [PATCH 0/4] New Lenovos 2015 touchpads: party time! Hans de Goede 4 siblings, 1 reply; 15+ messages in thread From: Benjamin Tissoires @ 2015-01-28 20:10 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Hans de Goede, Peter Hutterer, linux-input, linux-kernel The 2015 series of the Lenovo thinkpads added back the hardware buttons on top of the touchpad for the trackstick. Unfortunately, they are wired to the touchpad, and not the trackstick. Thus, they are seen as extra buttons from the kernel point of view. This leads to a problem in user space because extra buttons on synaptics devices used to be used as scroll up/down buttons. So in the end, the experience for the user is scroll events for buttons left and right when using the trackstick. Yay! Still, this user space problem will be fixed in the user space, and the kernel will only handle the TOPBUTTONPAD property - which was not a good idea to introduce in the kernel in the first place, I concede. Furthermore, Lenovo kept using the PNPIDs that are supposed to be "5 buttons" touchpads, so the new laptops also have the INPUT_PROP_TOPBUTTONPAD. Yay again! Insteead of manually removing each of the new ones, or hoping that we know all the current ones, we can consider that the PNPIDs list that were given contains touchpads that have the trackstick buttons, either physically wired to them, or emulated with the top software button property. Thanks to the extra buttons capability, we can reliably detect the physical buttons from the software ones, and so we can remove the TOPBUTTONPAD property even if it was declared as such. Cc: stable@vger.kernel.org Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> --- drivers/input/mouse/synaptics.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c index d58863b..4f05c9f 100644 --- a/drivers/input/mouse/synaptics.c +++ b/drivers/input/mouse/synaptics.c @@ -1129,7 +1129,8 @@ static void set_input_params(struct psmouse *psmouse, if (SYN_CAP_CLICKPAD(priv->ext_cap_0c)) { __set_bit(INPUT_PROP_BUTTONPAD, dev->propbit); - if (psmouse_matches_pnp_id(psmouse, topbuttonpad_pnp_ids)) + if (psmouse_matches_pnp_id(psmouse, topbuttonpad_pnp_ids) && + !test_bit(BTN_0, dev->keybit)) __set_bit(INPUT_PROP_TOPBUTTONPAD, dev->propbit); /* Clickpads report only left button */ __clear_bit(BTN_RIGHT, dev->keybit); -- 2.1.0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/4] Input - synaptics: remove TOPBUTTONPAD property for Lenovos 2015 2015-01-28 20:10 ` [PATCH 3/4] Input - synaptics: remove TOPBUTTONPAD property for Lenovos 2015 Benjamin Tissoires @ 2015-02-02 21:59 ` Dmitry Torokhov 0 siblings, 0 replies; 15+ messages in thread From: Dmitry Torokhov @ 2015-02-02 21:59 UTC (permalink / raw) To: Benjamin Tissoires Cc: Hans de Goede, Peter Hutterer, linux-input, linux-kernel On Wed, Jan 28, 2015 at 03:10:06PM -0500, Benjamin Tissoires wrote: > The 2015 series of the Lenovo thinkpads added back the hardware buttons > on top of the touchpad for the trackstick. > > Unfortunately, they are wired to the touchpad, and not the trackstick. > Thus, they are seen as extra buttons from the kernel point of view. > > This leads to a problem in user space because extra buttons on synaptics > devices used to be used as scroll up/down buttons. So in the end, the > experience for the user is scroll events for buttons left and right > when using the trackstick. Yay! > Still, this user space problem will be fixed in the user space, and > the kernel will only handle the TOPBUTTONPAD property - which was not > a good idea to introduce in the kernel in the first place, I concede. > > Furthermore, Lenovo kept using the PNPIDs that are supposed to be > "5 buttons" touchpads, so the new laptops also have the > INPUT_PROP_TOPBUTTONPAD. Yay again! > > Insteead of manually removing each of the new ones, or hoping that we > know all the current ones, we can consider that the PNPIDs list that > were given contains touchpads that have the trackstick buttons, either > physically wired to them, or emulated with the top software button > property. > > Thanks to the extra buttons capability, we can reliably detect the > physical buttons from the software ones, and so we can remove the > TOPBUTTONPAD property even if it was declared as such. > > Cc: stable@vger.kernel.org > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > --- > drivers/input/mouse/synaptics.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c > index d58863b..4f05c9f 100644 > --- a/drivers/input/mouse/synaptics.c > +++ b/drivers/input/mouse/synaptics.c > @@ -1129,7 +1129,8 @@ static void set_input_params(struct psmouse *psmouse, > > if (SYN_CAP_CLICKPAD(priv->ext_cap_0c)) { > __set_bit(INPUT_PROP_BUTTONPAD, dev->propbit); > - if (psmouse_matches_pnp_id(psmouse, topbuttonpad_pnp_ids)) > + if (psmouse_matches_pnp_id(psmouse, topbuttonpad_pnp_ids) && > + !test_bit(BTN_0, dev->keybit)) I do not really like this kind of heuristic, but all topbuttonpad_pnp_ids are currently Lenovos so I guess it's OK and I'll apply the patch. Regarding the tracksick/touchpad buttons I would not mind if we'd mix them in into pass-through port data - kernel's task is to make sense of hardware for userspace and in this case raw picture does not seem to make much sense... Thanks. > __set_bit(INPUT_PROP_TOPBUTTONPAD, dev->propbit); > /* Clickpads report only left button */ > __clear_bit(BTN_RIGHT, dev->keybit); > -- > 2.1.0 > -- Dmitry ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/4] Input - synaptics: Remove X1 Carbon 3rd gen from the topbuttonpad list 2015-01-28 20:10 [PATCH 0/4] New Lenovos 2015 touchpads: party time! Benjamin Tissoires ` (2 preceding siblings ...) 2015-01-28 20:10 ` [PATCH 3/4] Input - synaptics: remove TOPBUTTONPAD property for Lenovos 2015 Benjamin Tissoires @ 2015-01-28 20:10 ` Benjamin Tissoires 2015-01-29 7:52 ` [PATCH 0/4] New Lenovos 2015 touchpads: party time! Hans de Goede 4 siblings, 0 replies; 15+ messages in thread From: Benjamin Tissoires @ 2015-01-28 20:10 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Hans de Goede, Peter Hutterer, linux-input, linux-kernel Lenovo decided to switch back to physical buttons for the trackstick on their latest series. The PNPId list was provided before they reverted back to physical buttons, so it contains the new models too. We can know from the touchpad capabilities that the touchpad has physical buttons, so removing the ids from the list is not mandatory. It is still nicer to remove the wrong ids, so start by removing the X1 Carbon 3rd gen, with the PNPId of LEN0048. Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.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 4f05c9f..b2944ee 100644 --- a/drivers/input/mouse/synaptics.c +++ b/drivers/input/mouse/synaptics.c @@ -173,7 +173,6 @@ static const char * const topbuttonpad_pnp_ids[] = { "LEN0045", "LEN0046", "LEN0047", - "LEN0048", "LEN0049", "LEN2000", "LEN2001", /* Edge E431 */ -- 2.1.0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 0/4] New Lenovos 2015 touchpads: party time! 2015-01-28 20:10 [PATCH 0/4] New Lenovos 2015 touchpads: party time! Benjamin Tissoires ` (3 preceding siblings ...) 2015-01-28 20:10 ` [PATCH 4/4] Input - synaptics: Remove X1 Carbon 3rd gen from the topbuttonpad list Benjamin Tissoires @ 2015-01-29 7:52 ` Hans de Goede 4 siblings, 0 replies; 15+ messages in thread From: Hans de Goede @ 2015-01-29 7:52 UTC (permalink / raw) To: Benjamin Tissoires, Dmitry Torokhov Cc: Peter Hutterer, linux-input, linux-kernel Hi Benjamin & et al, On 28-01-15 21:10, Benjamin Tissoires wrote: > Hi Dmitry, > > Lenovo released their new models, and believe it or not, some kernel > work is required to support them! > > We learned the lesson and we will try to maximize the effort in user space > through udev rules instead of polluting the kernel with exceptions. > > Patches 1 & 2 are general fixes for synaptics that have been unnoticed > until this new models. They are marked as stable. > > Patch 3 is a generic way to fix the PnpId list that was hardcoded in the > kernel. This one is also marked as stable so that distributions will not > have the top software buttons on devices that do not require it. > > Patch 4 is a nice to have. It is not mandatory, so not CC-ed to stable, > it just reduced the PnpId list. > > The good thing is that the reported min/max are correct, so the problems > with the first generation of the t440 touchpad have been fixed now (and > synaptics assured us that they will keep checking the quality of the PS/2 > handling of their touchpads). > > One last thing, I still did not manage to talk to the device over SMBus, > so we are still stuck with the PS/2 protocol and its limitations for this > series. Series looks good to me and is: Acked-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2015-02-06 1:57 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-01-28 20:10 [PATCH 0/4] New Lenovos 2015 touchpads: party time! Benjamin Tissoires 2015-01-28 20:10 ` [PATCH 1/4] Input - synaptics: fix middle button on Lenovo 2015 products Benjamin Tissoires 2015-02-02 22:54 ` Dmitry Torokhov 2015-02-04 19:43 ` Benjamin Tissoires 2015-02-04 19:53 ` Dmitry Torokhov 2015-01-28 20:10 ` [PATCH 2/4] Input - synaptics: do not release extra buttons once they are pressed Benjamin Tissoires 2015-02-02 21:46 ` Dmitry Torokhov 2015-02-02 22:14 ` Benjamin Tissoires 2015-02-04 21:29 ` Benjamin Tissoires 2015-02-05 1:53 ` Andrew Duggan 2015-02-06 1:54 ` Andrew Duggan 2015-01-28 20:10 ` [PATCH 3/4] Input - synaptics: remove TOPBUTTONPAD property for Lenovos 2015 Benjamin Tissoires 2015-02-02 21:59 ` Dmitry Torokhov 2015-01-28 20:10 ` [PATCH 4/4] Input - synaptics: Remove X1 Carbon 3rd gen from the topbuttonpad list Benjamin Tissoires 2015-01-29 7:52 ` [PATCH 0/4] New Lenovos 2015 touchpads: party time! Hans de Goede
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).