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: [email protected]
> Signed-off-by: Benjamin Tissoires <[email protected]>
> ---
> 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
On Mon, Feb 2, 2015 at 4:46 PM, Dmitry Torokhov
<[email protected]> 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: [email protected]
>> Signed-off-by: Benjamin Tissoires <[email protected]>
>> ---
>> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Feb 02 2015 or thereabouts, Benjamin Tissoires wrote:
> On Mon, Feb 2, 2015 at 4:46 PM, Dmitry Torokhov
> <[email protected]> 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
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
>> <[email protected]> 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
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
>> <[email protected]> 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