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.

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.torok...@gmail.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. If this is intended then this should probably be in its own
patch.

> -             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 ?

> +                     } else {
> +                             priv->press = false;
> +                     }
> +
> +                     hw->left = priv->report_press;
> +
> +             } else if (SYN_CAP_CLICKPAD(priv->ext_cap_0c)) {
>                       /*
>                        * Clickpad's button is transmitted as middle button,
>                        * however, since it is primary button, we will report
> @@ -651,21 +701,6 @@ static int synaptics_parse_hw_state(const unsigned char 
> buf[],
>                       hw->down = ((buf[0] ^ buf[3]) & 0x02) ? 1 : 0;
>               }
>  
> -             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];
> -
>               if (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) &&
>                   ((buf[0] ^ buf[3]) & 0x02)) {
>                       switch (SYN_CAP_MULTI_BUTTON_NO(priv->ext_cap) & ~0x01) 
> {
> @@ -1642,11 +1677,24 @@ static const struct dmi_system_id __initconst 
> cr48_dmi_table[] = {
>       { }
>  };
>  
> +static const struct dmi_system_id forcepad_dmi_table[] __initconst = {
> +#if defined(CONFIG_DMI) && defined(CONFIG_X86)
> +     {
> +             .matches = {
> +                     DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
> +                     DMI_MATCH(DMI_PRODUCT_NAME, "HP EliteBook Folio 1040 
> G1"),
> +             },
> +     },
> +#endif
> +     { }
> +};
> +
>  void __init synaptics_module_init(void)
>  {
>       impaired_toshiba_kbc = dmi_check_system(toshiba_dmi_table);
>       broken_olpc_ec = dmi_check_system(olpc_dmi_table);
>       cr48_profile_sensor = dmi_check_system(cr48_dmi_table);
> +     is_forcepad = dmi_check_system(forcepad_dmi_table);
>  }
>  
>  static int __synaptics_init(struct psmouse *psmouse, bool absolute_mode)
> diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
> index e594af0..adc80c9 100644
> --- a/drivers/input/mouse/synaptics.h
> +++ b/drivers/input/mouse/synaptics.h
> @@ -177,6 +177,11 @@ struct synaptics_data {
>        */
>       struct synaptics_hw_state agm;
>       bool agm_pending;                       /* new AGM packet received */
> +
> +     /* ForcePad handling */
> +     unsigned long                           press_start;
> +     bool                                    press;
> +     bool                                    report_press;
>  };
>  
>  void synaptics_module_init(void);
> 

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to