| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 9 Sep 2014 15:07:21 -0700
From: Andrew Duggan <aduggan@...aptics.com>
To: Dmitry Torokhov <dmitry.torokhov@...il.com>,
Hans de Goede <hdegoede@...hat.com>
CC: <linux-input@...r.kernel.org>,
Benjamin Tissoires <benjamin.tissoires@...hat.com>,
Christopher Heiny <cheiny@...aptics.com>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] Input: synaptics - add support for ForcePads
On 09/09/2014 10:06 AM, Dmitry Torokhov wrote:
> On Tue, Sep 09, 2014 at 09:44:17AM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 09/08/2014 06:55 PM, Dmitry Torokhov wrote:
>>> ForcePads are found on HP EliteBook 1040 laptops. They lack any kind of
>>> physical buttons, instead they generate primary button click when user
>>> presses somewhat hard on the surface of the touchpad. Unfortunately they
>>> also report primary button click whenever there are 2 or more contacts
>>> on the pad, messing up all multi-finger gestures (2-finger scrolling,
>>> multi-finger tapping, etc). To cope with this behavior we introduce a
>>> delay (currently 50 msecs) in reporting primary press in case more
>>> contacts appear.
>>>
>>> For now we are using DMI matching to detect ForcePads, hopefully we'll
>>> be able to figure a better way down the road.
>> What about using the pnp-id, in my experience with the recent lenovo
>> laptops that tends to be more reliable.
> Not sure. So far I only know of HP 1040 having it. FWIW:
>
> dtor@...r-glaptop:~$ cat /sys/bus/pnp/drivers/i8042\ aux/00\:07/id
> SYN300d
> SYN0100
> SYN0002
> PNP0f13
>
> I think if we see generalities we can switch over later. I hope
> Chris/Andrew will come with a capability bit though :)
The ForcePad capabilities bit is 1 << 15.
>
>> Christopher, Andrew (added to the CC), can one of you tell us if there
>> is a capability bit to detect this, and if not can you perhaps provide
>> a list of pnp-ids of devices which behave like this ?
>>
>>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
>>> ---
>>> drivers/input/mouse/synaptics.c | 80 ++++++++++++++++++++++++++++++++---------
>>> drivers/input/mouse/synaptics.h | 5 +++
>>> 2 files changed, 69 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
>>> index e8573c6..5de1bb6 100644
>>> --- a/drivers/input/mouse/synaptics.c
>>> +++ b/drivers/input/mouse/synaptics.c
>>> @@ -618,6 +618,8 @@ static void synaptics_parse_agm(const unsigned char buf[],
>>> priv->agm_pending = true;
>>> }
>>>
>>> +static bool is_forcepad;
>>> +
>>> static int synaptics_parse_hw_state(const unsigned char buf[],
>>> struct synaptics_data *priv,
>>> struct synaptics_hw_state *hw)
>>> @@ -629,10 +631,58 @@ static int synaptics_parse_hw_state(const unsigned char buf[],
>>> ((buf[0] & 0x04) >> 1) |
>>> ((buf[3] & 0x04) >> 2));
>>>
>>> + if ((SYN_CAP_ADV_GESTURE(priv->ext_cap_0c) ||
>>> + SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) &&
>>> + hw->w == 2) {
>>> + synaptics_parse_agm(buf, priv, hw);
>>> + return 1;
>>> + }
>>> +
>>> + hw->x = (((buf[3] & 0x10) << 8) |
>>> + ((buf[1] & 0x0f) << 8) |
>>> + buf[4]);
>>> + hw->y = (((buf[3] & 0x20) << 7) |
>>> + ((buf[1] & 0xf0) << 4) |
>>> + buf[5]);
>>> + hw->z = buf[2];
>>> +
>>> hw->left = (buf[0] & 0x01) ? 1 : 0;
>>> hw->right = (buf[0] & 0x02) ? 1 : 0;
>>>
>> Moving this up means that on clickpads the button will no longer
>> be checked + set for hw->w == 2 packets. That seems like an unintended
>> side effect.
> I guess we can call it a side effect, but it is fine - we parse AGM and
> stash the data, then when 2nd packet comes we parse and act upon it. In
> the old code we'd parse x, y, z, in AGM etc. and then have it
> overwritten with the data from the 2nd packet, here we do not bother.
>
>> If this is intended then this should probably be in its own
>> patch.
> It does not make sense on its own as ForcePad support needs to know
> values of Z and W when deciding how to handle primary click, otherwise
> the old code was just fine.
>
>>> - if (SYN_CAP_CLICKPAD(priv->ext_cap_0c)) {
>>> + if (is_forcepad) {
>>> + /* XXX is there a proper capability bit for this? */
>>> + /*
>>> + * ForcePads, like Clickpads, use middle button
>>> + * bits to report primary button clicks.
>>> + * Unfortunately they report primary button not only
>>> + * when user presses on the pad above certain threshold,
>>> + * but also when there are more than one finger on the
>>> + * touchpad, which interferes with out multi-finger
>>> + * gestures.
>>> + */
>>> + if (hw->z == 0) {
>>> + /* No contacts */
>>> + priv->press = priv->report_press = false;
>>> + } else if (hw->w >= 4 && ((buf[0] ^ buf[3]) & 0x01)) {
>>> + /*
>>> + * Single-finger touch with pressure above
>>> + * the threshold.
>>> + */
>>> + if (!priv->press) {
>>> + priv->press_start = jiffies;
>>> + priv->press = true;
>>> + } else if (time_after(jiffies,
>>> + priv->press_start +
>>> + msecs_to_jiffies(50))) {
>>> + priv->report_press = true;
>>> + }
>> You're not setting a timer here, instead relying on there to be further events,
>> that is probably ok, but maybe put a comment to that extent here ?
> I can, any suggestion on what you want me to say? "If single point
> contact held pressure long enough start reporting primary click"?
>
> Thanks.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists