[PATCH] Input: sentelic - filter on-pad clicks in absolute mode

[PATCH] Input: sentelic - filter on-pad clicks in absolute mode

[Oskari Saarenmaa]

On-pad clicks in absolute positioning single-finger mode are reported
without the PHY_BTN bit set, the on-pad clicks are handled by userspace
so the kernel shouldn't report them as real clicks.

Signed-off-by: Oskari Saarenmaa <os@ohmu.fi>
---
 Applies to v3.4-rc1

 drivers/input/mouse/sentelic.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/input/mouse/sentelic.c b/drivers/input/mouse/sentelic.c
index a977bfa..d4df8cd 100644
--- a/drivers/input/mouse/sentelic.c
+++ b/drivers/input/mouse/sentelic.c
@@ -741,6 +741,14 @@ static psmouse_ret_t fsp_process_byte(struct psmouse *psmouse)
 			}
 		} else {
 			/* SFAC packet */
+			if ((packet[0] & FSP_PB0_LBTN|FSP_PB0_PHY_BTN) ==
+				FSP_PB0_LBTN) {
+				/* On-pad click in SFAC mode should be handled
+				 * by userspace.  On-pad clicks in MFMC mode
+				 * are real clickpad clicks, and not ignored.
+				 */
+				packet[0] &= ~FSP_PB0_LBTN;
+			}
 
 			/* no multi-finger information */
 			ad->last_mt_fgr = 0;
-- 
1.7.7.6

Re: [PATCH] Input: sentelic - filter on-pad clicks in absolute mode

[Chase Douglas]

On 04/01/2012 11:30 AM, Oskari Saarenmaa wrote:
> On-pad clicks in absolute positioning single-finger mode are reported
> without the PHY_BTN bit set, the on-pad clicks are handled by userspace
> so the kernel shouldn't report them as real clicks.

What is the definition of an "on-pad" click?

> Signed-off-by: Oskari Saarenmaa <os@ohmu.fi>
> ---
>  Applies to v3.4-rc1
> 
>  drivers/input/mouse/sentelic.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/input/mouse/sentelic.c b/drivers/input/mouse/sentelic.c
> index a977bfa..d4df8cd 100644
> --- a/drivers/input/mouse/sentelic.c
> +++ b/drivers/input/mouse/sentelic.c
> @@ -741,6 +741,14 @@ static psmouse_ret_t fsp_process_byte(struct psmouse *psmouse)
>  			}
>  		} else {
>  			/* SFAC packet */
> +			if ((packet[0] & FSP_PB0_LBTN|FSP_PB0_PHY_BTN) ==
> +				FSP_PB0_LBTN) {
> +				/* On-pad click in SFAC mode should be handled
> +				 * by userspace.  On-pad clicks in MFMC mode
> +				 * are real clickpad clicks, and not ignored.
> +				 */
> +				packet[0] &= ~FSP_PB0_LBTN;
> +			}
>  
>  			/* no multi-finger information */
>  			ad->last_mt_fgr = 0;

Re: [PATCH] Input: sentelic - filter on-pad clicks in absolute mode

[Oskari Saarenmaa]

On 02/04/12 17:22, Chase Douglas wrote:
> On 04/01/2012 11:30 AM, Oskari Saarenmaa wrote:
>> On-pad clicks in absolute positioning single-finger mode are reported
>> without the PHY_BTN bit set, the on-pad clicks are handled by userspace
>> so the kernel shouldn't report them as real clicks.
>
> What is the definition of an "on-pad" click?

In this case it's a tap on the pad without actually clicking it.  The 
Asus UX21 Zenbook has a touchpad that can be tapped or clicked.
Single-finger taps on it are reported by the touchpad with FSP_PB0_LBTN 
flag set and FSP_PB0_PHY_BTN not set.

/ Oskari

Re: [PATCH] Input: sentelic - filter on-pad clicks in absolute mode

[Chase Douglas]

On 04/02/2012 07:57 AM, Oskari Saarenmaa wrote:
> On 02/04/12 17:22, Chase Douglas wrote:
>> On 04/01/2012 11:30 AM, Oskari Saarenmaa wrote:
>>> On-pad clicks in absolute positioning single-finger mode are reported
>>> without the PHY_BTN bit set, the on-pad clicks are handled by userspace
>>> so the kernel shouldn't report them as real clicks.
>>
>> What is the definition of an "on-pad" click?
> 
> In this case it's a tap on the pad without actually clicking it.  The 
> Asus UX21 Zenbook has a touchpad that can be tapped or clicked.
> Single-finger taps on it are reported by the touchpad with FSP_PB0_LBTN 
> flag set and FSP_PB0_PHY_BTN not set.

So if I understand correctly, the device currently emits a "physical"
left button click event when the clickpad is tapped. Your patch will
prevent this event from being sent to userspace, since userspace does
its own tap detection. Is that correct?

-- Chase

Re: [PATCH] Input: sentelic - filter on-pad clicks in absolute mode

[Oskari Saarenmaa]

On 02/04/12 18:05, Chase Douglas wrote:
> On 04/02/2012 07:57 AM, Oskari Saarenmaa wrote:
>> On 02/04/12 17:22, Chase Douglas wrote:
>>> On 04/01/2012 11:30 AM, Oskari Saarenmaa wrote:
>>>> On-pad clicks in absolute positioning single-finger mode are reported
>>>> without the PHY_BTN bit set, the on-pad clicks are handled by userspace
>>>> so the kernel shouldn't report them as real clicks.
>>>
>>> What is the definition of an "on-pad" click?
>>
>> In this case it's a tap on the pad without actually clicking it.  The
>> Asus UX21 Zenbook has a touchpad that can be tapped or clicked.
>> Single-finger taps on it are reported by the touchpad with FSP_PB0_LBTN
>> flag set and FSP_PB0_PHY_BTN not set.
>
> So if I understand correctly, the device currently emits a "physical"
> left button click event when the clickpad is tapped. Your patch will
> prevent this event from being sent to userspace, since userspace does
> its own tap detection. Is that correct?

Yes.  Without this patch there's no way to disable click-on-tap in 
userspace.

/ Oskari

Re: [PATCH] Input: sentelic - filter on-pad clicks in absolute mode

[Dmitry Torokhov]

On Mon, Apr 02, 2012 at 06:37:04PM +0300, Oskari Saarenmaa wrote:
> On 02/04/12 18:05, Chase Douglas wrote:
> >On 04/02/2012 07:57 AM, Oskari Saarenmaa wrote:
> >>On 02/04/12 17:22, Chase Douglas wrote:
> >>>On 04/01/2012 11:30 AM, Oskari Saarenmaa wrote:
> >>>>On-pad clicks in absolute positioning single-finger mode are reported
> >>>>without the PHY_BTN bit set, the on-pad clicks are handled by userspace
> >>>>so the kernel shouldn't report them as real clicks.
> >>>
> >>>What is the definition of an "on-pad" click?
> >>
> >>In this case it's a tap on the pad without actually clicking it.  The
> >>Asus UX21 Zenbook has a touchpad that can be tapped or clicked.
> >>Single-finger taps on it are reported by the touchpad with FSP_PB0_LBTN
> >>flag set and FSP_PB0_PHY_BTN not set.
> >
> >So if I understand correctly, the device currently emits a "physical"
> >left button click event when the clickpad is tapped. Your patch will
> >prevent this event from being sent to userspace, since userspace does
> >its own tap detection. Is that correct?
> 
> Yes.  Without this patch there's no way to disable click-on-tap in
> userspace.

Makes sense to me. Tai-hwa, any objections?

BTW, I think we should suppress creating vscroll, hscroll and probably
flags attributes when touchpad is in absolute mode since they are not
applicable there (via attribute group's visible() methos). I am always
takig patches ;)

Thanks.

-- 
Dmitry

Re: [PATCH] Input: sentelic - filter on-pad clicks in absolute mode

[Tai-hwa Liang]

On Mon, 2 Apr 2012, Dmitry Torokhov wrote:
> On Mon, Apr 02, 2012 at 06:37:04PM +0300, Oskari Saarenmaa wrote:
>> On 02/04/12 18:05, Chase Douglas wrote:
>>> On 04/02/2012 07:57 AM, Oskari Saarenmaa wrote:
>>>> On 02/04/12 17:22, Chase Douglas wrote:
>>>>> On 04/01/2012 11:30 AM, Oskari Saarenmaa wrote:
>>>>>> On-pad clicks in absolute positioning single-finger mode are reported
>>>>>> without the PHY_BTN bit set, the on-pad clicks are handled by userspace
>>>>>> so the kernel shouldn't report them as real clicks.
>>>>>
>>>>> What is the definition of an "on-pad" click?
>>>>
>>>> In this case it's a tap on the pad without actually clicking it.  The
>>>> Asus UX21 Zenbook has a touchpad that can be tapped or clicked.
>>>> Single-finger taps on it are reported by the touchpad with FSP_PB0_LBTN
>>>> flag set and FSP_PB0_PHY_BTN not set.
>>>
>>> So if I understand correctly, the device currently emits a "physical"
>>> left button click event when the clickpad is tapped. Your patch will
>>> prevent this event from being sent to userspace, since userspace does
>>> its own tap detection. Is that correct?
>>
>> Yes.  Without this patch there's no way to disable click-on-tap in
>> userspace.
>
> Makes sense to me. Tai-hwa, any objections?

   No objection.

 	Reviewed-by:	Tai-hwa Liang <avatar@sentelic.com>

> BTW, I think we should suppress creating vscroll, hscroll and probably
> flags attributes when touchpad is in absolute mode since they are not
> applicable there (via attribute group's visible() methos). I am always
> takig patches ;)

Re: [PATCH] Input: sentelic - filter on-pad clicks in absolute mode

[Chase Douglas]

On 04/02/2012 08:37 AM, Oskari Saarenmaa wrote:
> On 02/04/12 18:05, Chase Douglas wrote:
>> On 04/02/2012 07:57 AM, Oskari Saarenmaa wrote:
>>> On 02/04/12 17:22, Chase Douglas wrote:
>>>> On 04/01/2012 11:30 AM, Oskari Saarenmaa wrote:
>>>>> On-pad clicks in absolute positioning single-finger mode are reported
>>>>> without the PHY_BTN bit set, the on-pad clicks are handled by userspace
>>>>> so the kernel shouldn't report them as real clicks.
>>>>
>>>> What is the definition of an "on-pad" click?
>>>
>>> In this case it's a tap on the pad without actually clicking it.  The
>>> Asus UX21 Zenbook has a touchpad that can be tapped or clicked.
>>> Single-finger taps on it are reported by the touchpad with FSP_PB0_LBTN
>>> flag set and FSP_PB0_PHY_BTN not set.
>>
>> So if I understand correctly, the device currently emits a "physical"
>> left button click event when the clickpad is tapped. Your patch will
>> prevent this event from being sent to userspace, since userspace does
>> its own tap detection. Is that correct?
> 
> Yes.  Without this patch there's no way to disable click-on-tap in 
> userspace.

Ok. May I suggest improving the commit message to distinguish between
taps and clicks. This was the cause of my confusion. I would suggest
something like:

"""

Input: sentelic - filter taps in absolute mode

Taps in absolute positioning single-finger mode are currently reported
as physical clicks by the driver. This should be handled by userspace,
not the kernel.

When a tap occurs, the FSP_PB0_LBTN bit is set, but the FSP_PB0_PHY_BTN
is not. We use this to filter out physical clicks from taps.

"""

Otherwise, the code looks right and the desired result makes sense, so:

Reviewed-by: Chase Douglas <chase.douglas@canonical.com>

[PATCHv2] Input: sentelic - filter taps in absolute mode

[Oskari Saarenmaa]

Taps in absolute positioning single-finger mode are currently reported
as physical clicks by the driver. This should be handled by userspace,
not the kernel.

When a tap occurs, the FSP_PB0_LBTN bit is set, but the FSP_PB0_PHY_BTN
is not. We use this to filter out physical clicks from taps.

Signed-off-by: Oskari Saarenmaa <os@ohmu.fi>
Reviewed-by: Tai-hwa Liang <avatar@sentelic.com> 
Reviewed-by: Chase Douglas <chase.douglas@canonical.com>
Cc: Dmitry Torokhov <dtor@mail.ru>
---
 Added missing braces around the bitwise or operation, reworded the
 commit message and added reviewed-by and cc lines.

 drivers/input/mouse/sentelic.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/input/mouse/sentelic.c b/drivers/input/mouse/sentelic.c
index a977bfa..d4df8cd 100644
--- a/drivers/input/mouse/sentelic.c
+++ b/drivers/input/mouse/sentelic.c
@@ -741,6 +741,14 @@ static psmouse_ret_t fsp_process_byte(struct psmouse *psmouse)
 			}
 		} else {
 			/* SFAC packet */
+			if ((packet[0] & (FSP_PB0_LBTN|FSP_PB0_PHY_BTN)) ==
+				FSP_PB0_LBTN) {
+				/* On-pad click in SFAC mode should be handled
+				 * by userspace.  On-pad clicks in MFMC mode
+				 * are real clickpad clicks, and not ignored.
+				 */
+				packet[0] &= ~FSP_PB0_LBTN;
+			}
 
 			/* no multi-finger information */
 			ad->last_mt_fgr = 0;
-- 
1.7.7.6

Re: [PATCHv2] Input: sentelic - filter taps in absolute mode

[Dmitry Torokhov]

On Tue, Apr 03, 2012 at 09:59:09AM +0300, Oskari Saarenmaa wrote:
> Taps in absolute positioning single-finger mode are currently reported
> as physical clicks by the driver. This should be handled by userspace,
> not the kernel.
> 
> When a tap occurs, the FSP_PB0_LBTN bit is set, but the FSP_PB0_PHY_BTN
> is not. We use this to filter out physical clicks from taps.
> 
> Signed-off-by: Oskari Saarenmaa <os@ohmu.fi>
> Reviewed-by: Tai-hwa Liang <avatar@sentelic.com> 
> Reviewed-by: Chase Douglas <chase.douglas@canonical.com>
> Cc: Dmitry Torokhov <dtor@mail.ru>

Applied, thank you.

-- 
Dmitry