[PATCH] Input: synaptics - add support for ForcePads

[PATCH] Input: synaptics - add support for ForcePads

[Dmitry Torokhov]

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.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@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;
 
-		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;
+				}
+			} 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);
-- 
2.1.0.rc2.206.gedb03e5


-- 
Dmitry

Re: [PATCH] Input: synaptics - add support for ForcePads

[Hans de Goede]

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.torokhov@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

Re: [PATCH] Input: synaptics - add support for ForcePads

[Dmitry Torokhov]

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@dtor-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 :)

> 
> 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@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.

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.

-- 
Dmitry

Re: [PATCH] Input: synaptics - add support for ForcePads

[Hans de Goede]

Hi,

On 09/09/2014 07:06 PM, 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@dtor-glaptop:~$ cat /sys/bus/pnp/drivers/i8042\ aux/00\:07/id
> SYN300d
> SYN0100
> SYN0002
> PNP0f13

Hmm, those look pretty generic. Maybe the 300d is specific.

> I think if we see generalities we can switch over later. I hope
> Chris/Andrew will come with a capability bit though :)
>
>>
>> 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@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.
>
> 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.

Ok.

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

What I mean is that this will not work if the finger is held perfectly still,
at least AFAIK no more packets will be send in that case, but maybe that is
me thinking too much in userspace terms where no evdev events will be
generated in that case. If the touchpad will continue repeating events with
exact the same location + pressure, then there is nothing to worry about
and you can ignore my request for a comment.

Regards,

Hans

Re: [PATCH] Input: synaptics - add support for ForcePads

[Andrew Duggan]

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@dtor-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@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.
> 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.
>

Re: [PATCH] Input: synaptics - add support for ForcePads

[Andrew Duggan]

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@dtor-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@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.
> 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.
>

Re: [PATCH] Input: synaptics - add support for ForcePads

[Dmitry Torokhov]

On Tue, Sep 09, 2014 at 03:07:21PM -0700, Andrew Duggan wrote:
> 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@dtor-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.

Awesome, thanks!

Then I guess the patch below is what we'll be needing for these devices.

Thanks.

-- 
Dmitry

Input: synaptics - add support for ForcePads

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

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.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/mouse/synaptics.c |   68 ++++++++++++++++++++++++++++++---------
 drivers/input/mouse/synaptics.h |   11 ++++++
 2 files changed, 63 insertions(+), 16 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index e8573c6..fd23181 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -629,10 +629,61 @@ 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;
 
-		if (SYN_CAP_CLICKPAD(priv->ext_cap_0c)) {
+		if (SYN_CAP_FORCEPAD(priv->ext_cap_0c)) {
+			/*
+			 * 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 pressure stays long
+				 * enough, we'll start reporting primary
+				 * button. We rely on the device continuing
+				 * sending data even if finger does not
+				 * move.
+				 */
+				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;
+				}
+			} 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 +702,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) {
diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
index e594af0..fb2e076 100644
--- a/drivers/input/mouse/synaptics.h
+++ b/drivers/input/mouse/synaptics.h
@@ -78,6 +78,11 @@
  * 2	0x08	image sensor		image sensor tracks 5 fingers, but only
  *					reports 2.
  * 2	0x20	report min		query 0x0f gives min coord reported
+ * 2	0x80	forcepad		forcepad is a variant of clickpad that
+ *					does not have physical buttons but rather
+ *					uses pressure above certain threshold to
+ *					report primary clicks. Forcepads also have
+ *					clickpad bit set.
  */
 #define SYN_CAP_CLICKPAD(ex0c)		((ex0c) & 0x100000) /* 1-button ClickPad */
 #define SYN_CAP_CLICKPAD2BTN(ex0c)	((ex0c) & 0x000100) /* 2-button ClickPad */
@@ -86,6 +91,7 @@
 #define SYN_CAP_ADV_GESTURE(ex0c)	((ex0c) & 0x080000)
 #define SYN_CAP_REDUCED_FILTERING(ex0c)	((ex0c) & 0x000400)
 #define SYN_CAP_IMAGE_SENSOR(ex0c)	((ex0c) & 0x000800)
+#define SYN_CAP_FORCEPAD(ex0c)		((ex0c) & 0x008000)
 
 /* synaptics modes query bits */
 #define SYN_MODE_ABSOLUTE(m)		((m) & (1 << 7))
@@ -177,6 +183,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);

Re: [PATCH] Input: synaptics - add support for ForcePads

[Hans de Goede]

Hi,

On 09/10/2014 12:57 AM, Dmitry Torokhov wrote:
> On Tue, Sep 09, 2014 at 03:07:21PM -0700, Andrew Duggan wrote:
>> 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@dtor-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.
> 
> Awesome, thanks!
> 
> Then I guess the patch below is what we'll be needing for these devices.

Yep, the patch looks good to me.

Regards,

Hans

Re: [PATCH] Input: synaptics - add support for ForcePads

[Dmitry Torokhov]

On Wed, Sep 10, 2014 at 01:07:02AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 09/10/2014 12:57 AM, Dmitry Torokhov wrote:
> > On Tue, Sep 09, 2014 at 03:07:21PM -0700, Andrew Duggan wrote:
> >> 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@dtor-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.
> > 
> > Awesome, thanks!
> > 
> > Then I guess the patch below is what we'll be needing for these devices.
> 
> Yep, the patch looks good to me.

Thanks, I'll put you down as reviewed-by then.

-- 
Dmitry

Re: [PATCH] Input: synaptics - add support for ForcePads

[Peter Hutterer]

On Tue, Sep 09, 2014 at 03:57:25PM -0700, Dmitry Torokhov wrote:
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> 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.

can we assign INPUT_PROP_FORCEPAD to this? or PRESSUREPAD or something if
we're worried about trademarks. This is something I'd like to know in
userspace, we can then ignore clicks completely and do the rest ourselves. 

Cheers,
   Peter

> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/mouse/synaptics.c |   68 ++++++++++++++++++++++++++++++---------
>  drivers/input/mouse/synaptics.h |   11 ++++++
>  2 files changed, 63 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index e8573c6..fd23181 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -629,10 +629,61 @@ 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;
>  
> -		if (SYN_CAP_CLICKPAD(priv->ext_cap_0c)) {
> +		if (SYN_CAP_FORCEPAD(priv->ext_cap_0c)) {
> +			/*
> +			 * 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 pressure stays long
> +				 * enough, we'll start reporting primary
> +				 * button. We rely on the device continuing
> +				 * sending data even if finger does not
> +				 * move.
> +				 */
> +				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;
> +				}
> +			} 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 +702,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) {
> diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
> index e594af0..fb2e076 100644
> --- a/drivers/input/mouse/synaptics.h
> +++ b/drivers/input/mouse/synaptics.h
> @@ -78,6 +78,11 @@
>   * 2	0x08	image sensor		image sensor tracks 5 fingers, but only
>   *					reports 2.
>   * 2	0x20	report min		query 0x0f gives min coord reported
> + * 2	0x80	forcepad		forcepad is a variant of clickpad that
> + *					does not have physical buttons but rather
> + *					uses pressure above certain threshold to
> + *					report primary clicks. Forcepads also have
> + *					clickpad bit set.
>   */
>  #define SYN_CAP_CLICKPAD(ex0c)		((ex0c) & 0x100000) /* 1-button ClickPad */
>  #define SYN_CAP_CLICKPAD2BTN(ex0c)	((ex0c) & 0x000100) /* 2-button ClickPad */
> @@ -86,6 +91,7 @@
>  #define SYN_CAP_ADV_GESTURE(ex0c)	((ex0c) & 0x080000)
>  #define SYN_CAP_REDUCED_FILTERING(ex0c)	((ex0c) & 0x000400)
>  #define SYN_CAP_IMAGE_SENSOR(ex0c)	((ex0c) & 0x000800)
> +#define SYN_CAP_FORCEPAD(ex0c)		((ex0c) & 0x008000)
>  
>  /* synaptics modes query bits */
>  #define SYN_MODE_ABSOLUTE(m)		((m) & (1 << 7))
> @@ -177,6 +183,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);
> --
> 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: [PATCH] Input: synaptics - add support for ForcePads

[Dmitry Torokhov]

On Thu, Oct 30, 2014 at 10:38:55AM +1000, Peter Hutterer wrote:
> On Tue, Sep 09, 2014 at 03:57:25PM -0700, Dmitry Torokhov wrote:
> > From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > 
> > 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.
> 
> can we assign INPUT_PROP_FORCEPAD to this? or PRESSUREPAD or something if
> we're worried about trademarks. This is something I'd like to know in
> userspace, we can then ignore clicks completely and do the rest ourselves. 

Hmm, I think we can, but I'd like to wait to see if there are more
models with such touchpads (I sure hope there isn't; lack of tactile
feedback on presses is not the best user experience so I hope this
initiative dies a quick death).

Thanks.

-- 
Dmitry