Re: [PATCH] Input: Change BTN_TOOL_FINGER falg when HOVER event trigger

['Dmitry Torokhov']

On Wed, Mar 30, 2016 at 05:49:12PM +0800, DusonLin wrote:
> Hello Dmitry,
> 
> Please check my response below, thanks.
> 
> 
> -Original Message-
> From: 'Dmitry Torokhov' [mailto:dmitry.torok...@gmail.com] 
> Sent: Wednesday, March 30, 2016 10:31 AM
> To: DusonLin
> Cc: Linux-kernel@vger.kernel.org; linux-in...@vger.kernel.org; 'Charles 
> Mooney'; 'jeff'; 'Phoenix'
> Subject: Re: [PATCH] Input: Change BTN_TOOL_FINGER falg when HOVER event 
> trigger
> 
> Hi Duson,
> 
> On Wed, Mar 30, 2016 at 10:02:02AM +0800, DusonLin wrote:
> > Only ABS_DISTANCE is not enough for upper OS  to distiguish hover 
> > event be triggered from object form faraway to and close touchpad 
> > surface or from object prepare to leave the touchpad surface. We add 
> > BNT_TOOL_FINGER to help it.
> > 
> >  Object_at_faraway   object_at_hover_area  
> > object_touch_touchpad
> >   BTN_TOUCH 0  01
> > BTN_TOOL_FINGER 0  11
> >ABS_DISTANCE 0  10
> > 
> > Signed-off by: Duson Lin <duson...@emc.com.tw>
> > ---
> > drivers/input/mouse/elan_i2c_core.c |   14 +-
> > 1 file changed, 9 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/input/mouse/elan_i2c_core.c 
> > b/drivers/input/mouse/elan_i2c_core.c
> > index 2f58985..9392a8c 100644
> > --- a/drivers/input/mouse/elan_i2c_core.c
> > +++ b/drivers/input/mouse/elan_i2c_core.c
> > @@ -4,7 +4,7 @@
> >   * Copyright (c) 2013 ELAN Microelectronics Corp.
> >   *
> >   * Author: 林政維 (Duson Lin) <duson...@emc.com.tw>
> > - * Version: 1.6.0
> > + * Version: 1.6.1
> >   *
> >   * Based on cyapa driver:
> >   * copyright (c) 2011-2012 Cypress Semiconductor, Inc.
> > @@ -845,23 +845,27 @@ static void elan_report_absolute(struct 
> > elan_tp_data *data, u8 *packet) {
> >   struct input_dev *input = data->input;
> >   u8 *finger_data = [ETP_FINGER_DATA_OFFSET];
> > -   int i;
> > +  int i, valid_count = 0;
> 
> This appears all whitespace-damaged.
> [duson] will do.
> 
> >   u8 tp_info = packet[ETP_TOUCH_INFO_OFFSET];
> >   u8 hover_info = packet[ETP_HOVER_INFO_OFFSET];
> >   bool contact_valid, hover_event;
> > 
> > -   hover_event = hover_info & 0x40;
> > +  hover_event = (hover_info & 0x40);
> 
> We do not need parentheses here.
> [duson] will do.
> 
> >   for (i = 0; i < ETP_MAX_FINGERS; i++) {
> >   contact_valid = tp_info & (1U << (3 + i));
> >   elan_report_contact(data, i, contact_valid, 
> > finger_data);
> > 
> > -   if (contact_valid)
> > +  if (contact_valid) {
> >   finger_data += ETP_FINGER_DATA_LEN;
> > +  valid_count++;
> > +  }
> >   }
> > 
> >input_report_key(input, BTN_LEFT, tp_info & 0x01);
> > +  input_report_key(input, BTN_TOOL_FINGER,
> > +  ((hover_event != 0) | (valid_count > 0)));
> 
> You do not want to use "bitwise or" here.
> 
> >   input_report_abs(input, ABS_DISTANCE, hover_event != 0);
> > -   input_mt_report_pointer_emulation(input, true);
> > +  input_mt_report_pointer_emulation(input, false);
> 
> I wonder if we should teach input_mt_report_pointer_emulation() to handle 
> hover properly.
> [duson] But, it looks like the hover is not standard function and not all of 
> touchpad support this function, 
>Even though, we still teach inpu_mt_report_pointer_emulation() to 
> handle this function?

Yes, something like this:

If device supports ABS_DISTANCE and does not support ABS_MT_DISTANCE and
ABS_DISTANCE != 0 is reported in current frame and there are no other
contacts then report BTN_TOOL_FINGER.

Thanks.

-- 
Dmitry

Re: [PATCH] Input: Change BTN_TOOL_FINGER falg when HOVER event trigger

['Dmitry Torokhov']

On Wed, Mar 30, 2016 at 05:49:12PM +0800, DusonLin wrote:
> Hello Dmitry,
> 
> Please check my response below, thanks.
> 
> 
> -Original Message-
> From: 'Dmitry Torokhov' [mailto:dmitry.torok...@gmail.com] 
> Sent: Wednesday, March 30, 2016 10:31 AM
> To: DusonLin
> Cc: Linux-kernel@vger.kernel.org; linux-in...@vger.kernel.org; 'Charles 
> Mooney'; 'jeff'; 'Phoenix'
> Subject: Re: [PATCH] Input: Change BTN_TOOL_FINGER falg when HOVER event 
> trigger
> 
> Hi Duson,
> 
> On Wed, Mar 30, 2016 at 10:02:02AM +0800, DusonLin wrote:
> > Only ABS_DISTANCE is not enough for upper OS  to distiguish hover 
> > event be triggered from object form faraway to and close touchpad 
> > surface or from object prepare to leave the touchpad surface. We add 
> > BNT_TOOL_FINGER to help it.
> > 
> >  Object_at_faraway   object_at_hover_area  
> > object_touch_touchpad
> >   BTN_TOUCH 0  01
> > BTN_TOOL_FINGER 0  11
> >ABS_DISTANCE 0  10
> > 
> > Signed-off by: Duson Lin 
> > ---
> > drivers/input/mouse/elan_i2c_core.c |   14 +-
> > 1 file changed, 9 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/input/mouse/elan_i2c_core.c 
> > b/drivers/input/mouse/elan_i2c_core.c
> > index 2f58985..9392a8c 100644
> > --- a/drivers/input/mouse/elan_i2c_core.c
> > +++ b/drivers/input/mouse/elan_i2c_core.c
> > @@ -4,7 +4,7 @@
> >   * Copyright (c) 2013 ELAN Microelectronics Corp.
> >   *
> >   * Author: 林政維 (Duson Lin) 
> > - * Version: 1.6.0
> > + * Version: 1.6.1
> >   *
> >   * Based on cyapa driver:
> >   * copyright (c) 2011-2012 Cypress Semiconductor, Inc.
> > @@ -845,23 +845,27 @@ static void elan_report_absolute(struct 
> > elan_tp_data *data, u8 *packet) {
> >   struct input_dev *input = data->input;
> >   u8 *finger_data = [ETP_FINGER_DATA_OFFSET];
> > -   int i;
> > +  int i, valid_count = 0;
> 
> This appears all whitespace-damaged.
> [duson] will do.
> 
> >   u8 tp_info = packet[ETP_TOUCH_INFO_OFFSET];
> >   u8 hover_info = packet[ETP_HOVER_INFO_OFFSET];
> >   bool contact_valid, hover_event;
> > 
> > -   hover_event = hover_info & 0x40;
> > +  hover_event = (hover_info & 0x40);
> 
> We do not need parentheses here.
> [duson] will do.
> 
> >   for (i = 0; i < ETP_MAX_FINGERS; i++) {
> >   contact_valid = tp_info & (1U << (3 + i));
> >   elan_report_contact(data, i, contact_valid, 
> > finger_data);
> > 
> > -   if (contact_valid)
> > +  if (contact_valid) {
> >   finger_data += ETP_FINGER_DATA_LEN;
> > +  valid_count++;
> > +  }
> >   }
> > 
> >input_report_key(input, BTN_LEFT, tp_info & 0x01);
> > +  input_report_key(input, BTN_TOOL_FINGER,
> > +  ((hover_event != 0) | (valid_count > 0)));
> 
> You do not want to use "bitwise or" here.
> 
> >   input_report_abs(input, ABS_DISTANCE, hover_event != 0);
> > -   input_mt_report_pointer_emulation(input, true);
> > +  input_mt_report_pointer_emulation(input, false);
> 
> I wonder if we should teach input_mt_report_pointer_emulation() to handle 
> hover properly.
> [duson] But, it looks like the hover is not standard function and not all of 
> touchpad support this function, 
>Even though, we still teach inpu_mt_report_pointer_emulation() to 
> handle this function?

Yes, something like this:

If device supports ABS_DISTANCE and does not support ABS_MT_DISTANCE and
ABS_DISTANCE != 0 is reported in current frame and there are no other
contacts then report BTN_TOOL_FINGER.

Thanks.

-- 
Dmitry

RE: [PATCH] Input: Change BTN_TOOL_FINGER falg when HOVER event trigger

[DusonLin]

Hello Dmitry,

Please check my response below, thanks.


-Original Message-
From: 'Dmitry Torokhov' [mailto:dmitry.torok...@gmail.com] 
Sent: Wednesday, March 30, 2016 10:31 AM
To: DusonLin
Cc: Linux-kernel@vger.kernel.org; linux-in...@vger.kernel.org; 'Charles 
Mooney'; 'jeff'; 'Phoenix'
Subject: Re: [PATCH] Input: Change BTN_TOOL_FINGER falg when HOVER event trigger

Hi Duson,

On Wed, Mar 30, 2016 at 10:02:02AM +0800, DusonLin wrote:
> Only ABS_DISTANCE is not enough for upper OS  to distiguish hover 
> event be triggered from object form faraway to and close touchpad 
> surface or from object prepare to leave the touchpad surface. We add 
> BNT_TOOL_FINGER to help it.
> 
>  Object_at_faraway   object_at_hover_area  
> object_touch_touchpad
>   BTN_TOUCH 0  01
> BTN_TOOL_FINGER 0  11
>ABS_DISTANCE 0  10
> 
> Signed-off by: Duson Lin <duson...@emc.com.tw>
> ---
> drivers/input/mouse/elan_i2c_core.c |   14 +-
> 1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/input/mouse/elan_i2c_core.c 
> b/drivers/input/mouse/elan_i2c_core.c
> index 2f58985..9392a8c 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -4,7 +4,7 @@
>   * Copyright (c) 2013 ELAN Microelectronics Corp.
>   *
>   * Author: 林政維 (Duson Lin) <duson...@emc.com.tw>
> - * Version: 1.6.0
> + * Version: 1.6.1
>   *
>   * Based on cyapa driver:
>   * copyright (c) 2011-2012 Cypress Semiconductor, Inc.
> @@ -845,23 +845,27 @@ static void elan_report_absolute(struct 
> elan_tp_data *data, u8 *packet) {
>   struct input_dev *input = data->input;
>   u8 *finger_data = [ETP_FINGER_DATA_OFFSET];
> -   int i;
> +  int i, valid_count = 0;

This appears all whitespace-damaged.
[duson] will do.

>   u8 tp_info = packet[ETP_TOUCH_INFO_OFFSET];
>   u8 hover_info = packet[ETP_HOVER_INFO_OFFSET];
>   bool contact_valid, hover_event;
> 
> -   hover_event = hover_info & 0x40;
> +  hover_event = (hover_info & 0x40);

We do not need parentheses here.
[duson] will do.

>   for (i = 0; i < ETP_MAX_FINGERS; i++) {
>   contact_valid = tp_info & (1U << (3 + i));
>   elan_report_contact(data, i, contact_valid, 
> finger_data);
> 
> -   if (contact_valid)
> +  if (contact_valid) {
>   finger_data += ETP_FINGER_DATA_LEN;
> +  valid_count++;
> +  }
>   }
> 
>input_report_key(input, BTN_LEFT, tp_info & 0x01);
> +  input_report_key(input, BTN_TOOL_FINGER,
> +  ((hover_event != 0) | (valid_count > 0)));

You do not want to use "bitwise or" here.

>   input_report_abs(input, ABS_DISTANCE, hover_event != 0);
> -   input_mt_report_pointer_emulation(input, true);
> +  input_mt_report_pointer_emulation(input, false);

I wonder if we should teach input_mt_report_pointer_emulation() to handle hover 
properly.
[duson] But, it looks like the hover is not standard function and not all of 
touchpad support this function, 
   Even though, we still teach inpu_mt_report_pointer_emulation() to handle 
this function?

Thanks.

--
Dmitry

RE: [PATCH] Input: Change BTN_TOOL_FINGER falg when HOVER event trigger

[DusonLin]

Hello Dmitry,

Please check my response below, thanks.


-Original Message-
From: 'Dmitry Torokhov' [mailto:dmitry.torok...@gmail.com] 
Sent: Wednesday, March 30, 2016 10:31 AM
To: DusonLin
Cc: Linux-kernel@vger.kernel.org; linux-in...@vger.kernel.org; 'Charles 
Mooney'; 'jeff'; 'Phoenix'
Subject: Re: [PATCH] Input: Change BTN_TOOL_FINGER falg when HOVER event trigger

Hi Duson,

On Wed, Mar 30, 2016 at 10:02:02AM +0800, DusonLin wrote:
> Only ABS_DISTANCE is not enough for upper OS  to distiguish hover 
> event be triggered from object form faraway to and close touchpad 
> surface or from object prepare to leave the touchpad surface. We add 
> BNT_TOOL_FINGER to help it.
> 
>  Object_at_faraway   object_at_hover_area  
> object_touch_touchpad
>   BTN_TOUCH 0  01
> BTN_TOOL_FINGER 0  11
>ABS_DISTANCE 0  10
> 
> Signed-off by: Duson Lin 
> ---
> drivers/input/mouse/elan_i2c_core.c |   14 +-
> 1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/input/mouse/elan_i2c_core.c 
> b/drivers/input/mouse/elan_i2c_core.c
> index 2f58985..9392a8c 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -4,7 +4,7 @@
>   * Copyright (c) 2013 ELAN Microelectronics Corp.
>   *
>   * Author: 林政維 (Duson Lin) 
> - * Version: 1.6.0
> + * Version: 1.6.1
>   *
>   * Based on cyapa driver:
>   * copyright (c) 2011-2012 Cypress Semiconductor, Inc.
> @@ -845,23 +845,27 @@ static void elan_report_absolute(struct 
> elan_tp_data *data, u8 *packet) {
>   struct input_dev *input = data->input;
>   u8 *finger_data = [ETP_FINGER_DATA_OFFSET];
> -   int i;
> +  int i, valid_count = 0;

This appears all whitespace-damaged.
[duson] will do.

>   u8 tp_info = packet[ETP_TOUCH_INFO_OFFSET];
>   u8 hover_info = packet[ETP_HOVER_INFO_OFFSET];
>   bool contact_valid, hover_event;
> 
> -   hover_event = hover_info & 0x40;
> +  hover_event = (hover_info & 0x40);

We do not need parentheses here.
[duson] will do.

>   for (i = 0; i < ETP_MAX_FINGERS; i++) {
>   contact_valid = tp_info & (1U << (3 + i));
>   elan_report_contact(data, i, contact_valid, 
> finger_data);
> 
> -   if (contact_valid)
> +  if (contact_valid) {
>   finger_data += ETP_FINGER_DATA_LEN;
> +  valid_count++;
> +  }
>   }
> 
>input_report_key(input, BTN_LEFT, tp_info & 0x01);
> +  input_report_key(input, BTN_TOOL_FINGER,
> +  ((hover_event != 0) | (valid_count > 0)));

You do not want to use "bitwise or" here.

>   input_report_abs(input, ABS_DISTANCE, hover_event != 0);
> -   input_mt_report_pointer_emulation(input, true);
> +  input_mt_report_pointer_emulation(input, false);

I wonder if we should teach input_mt_report_pointer_emulation() to handle hover 
properly.
[duson] But, it looks like the hover is not standard function and not all of 
touchpad support this function, 
   Even though, we still teach inpu_mt_report_pointer_emulation() to handle 
this function?

Thanks.

--
Dmitry

Re: [PATCH] Input: Change BTN_TOOL_FINGER falg when HOVER event trigger

['Dmitry Torokhov']

Hi Duson,

On Wed, Mar 30, 2016 at 10:02:02AM +0800, DusonLin wrote:
> Only ABS_DISTANCE is not enough for upper OS  to distiguish hover event
> be triggered from object form faraway to and close touchpad surface or from
> object prepare to leave the touchpad surface. We add BNT_TOOL_FINGER to help
> it.
> 
>  Object_at_faraway   object_at_hover_area  
> object_touch_touchpad
>   BTN_TOUCH 0  01
> BTN_TOOL_FINGER 0  11
>ABS_DISTANCE 0  10
> 
> Signed-off by: Duson Lin 
> ---
> drivers/input/mouse/elan_i2c_core.c |   14 +-
> 1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/input/mouse/elan_i2c_core.c 
> b/drivers/input/mouse/elan_i2c_core.c
> index 2f58985..9392a8c 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -4,7 +4,7 @@
>   * Copyright (c) 2013 ELAN Microelectronics Corp.
>   *
>   * Author: 林政維 (Duson Lin) 
> - * Version: 1.6.0
> + * Version: 1.6.1
>   *
>   * Based on cyapa driver:
>   * copyright (c) 2011-2012 Cypress Semiconductor, Inc.
> @@ -845,23 +845,27 @@ static void elan_report_absolute(struct elan_tp_data 
> *data, u8 *packet)
> {
>   struct input_dev *input = data->input;
>   u8 *finger_data = [ETP_FINGER_DATA_OFFSET];
> -   int i;
> +  int i, valid_count = 0;

This appears all whitespace-damaged.

>   u8 tp_info = packet[ETP_TOUCH_INFO_OFFSET];
>   u8 hover_info = packet[ETP_HOVER_INFO_OFFSET];
>   bool contact_valid, hover_event;
> 
> -   hover_event = hover_info & 0x40;
> +  hover_event = (hover_info & 0x40);

We do not need parentheses here.

>   for (i = 0; i < ETP_MAX_FINGERS; i++) {
>   contact_valid = tp_info & (1U << (3 + i));
>   elan_report_contact(data, i, contact_valid, finger_data);
> 
> -   if (contact_valid)
> +  if (contact_valid) {
>   finger_data += ETP_FINGER_DATA_LEN;
> +  valid_count++;
> +  }
>   }
> 
>input_report_key(input, BTN_LEFT, tp_info & 0x01);
> +  input_report_key(input, BTN_TOOL_FINGER,
> +  ((hover_event != 0) | (valid_count > 0)));

You do not want to use "bitwise or" here.

>   input_report_abs(input, ABS_DISTANCE, hover_event != 0);
> -   input_mt_report_pointer_emulation(input, true);
> +  input_mt_report_pointer_emulation(input, false);

I wonder if we should teach input_mt_report_pointer_emulation() to
handle hover properly.

Thanks.

-- 
Dmitry

Re: [PATCH] Input: Change BTN_TOOL_FINGER falg when HOVER event trigger

['Dmitry Torokhov']

Hi Duson,

On Wed, Mar 30, 2016 at 10:02:02AM +0800, DusonLin wrote:
> Only ABS_DISTANCE is not enough for upper OS  to distiguish hover event
> be triggered from object form faraway to and close touchpad surface or from
> object prepare to leave the touchpad surface. We add BNT_TOOL_FINGER to help
> it.
> 
>  Object_at_faraway   object_at_hover_area  
> object_touch_touchpad
>   BTN_TOUCH 0  01
> BTN_TOOL_FINGER 0  11
>ABS_DISTANCE 0  10
> 
> Signed-off by: Duson Lin 
> ---
> drivers/input/mouse/elan_i2c_core.c |   14 +-
> 1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/input/mouse/elan_i2c_core.c 
> b/drivers/input/mouse/elan_i2c_core.c
> index 2f58985..9392a8c 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -4,7 +4,7 @@
>   * Copyright (c) 2013 ELAN Microelectronics Corp.
>   *
>   * Author: 林政維 (Duson Lin) 
> - * Version: 1.6.0
> + * Version: 1.6.1
>   *
>   * Based on cyapa driver:
>   * copyright (c) 2011-2012 Cypress Semiconductor, Inc.
> @@ -845,23 +845,27 @@ static void elan_report_absolute(struct elan_tp_data 
> *data, u8 *packet)
> {
>   struct input_dev *input = data->input;
>   u8 *finger_data = [ETP_FINGER_DATA_OFFSET];
> -   int i;
> +  int i, valid_count = 0;

This appears all whitespace-damaged.

>   u8 tp_info = packet[ETP_TOUCH_INFO_OFFSET];
>   u8 hover_info = packet[ETP_HOVER_INFO_OFFSET];
>   bool contact_valid, hover_event;
> 
> -   hover_event = hover_info & 0x40;
> +  hover_event = (hover_info & 0x40);

We do not need parentheses here.

>   for (i = 0; i < ETP_MAX_FINGERS; i++) {
>   contact_valid = tp_info & (1U << (3 + i));
>   elan_report_contact(data, i, contact_valid, finger_data);
> 
> -   if (contact_valid)
> +  if (contact_valid) {
>   finger_data += ETP_FINGER_DATA_LEN;
> +  valid_count++;
> +  }
>   }
> 
>input_report_key(input, BTN_LEFT, tp_info & 0x01);
> +  input_report_key(input, BTN_TOOL_FINGER,
> +  ((hover_event != 0) | (valid_count > 0)));

You do not want to use "bitwise or" here.

>   input_report_abs(input, ABS_DISTANCE, hover_event != 0);
> -   input_mt_report_pointer_emulation(input, true);
> +  input_mt_report_pointer_emulation(input, false);

I wonder if we should teach input_mt_report_pointer_emulation() to
handle hover properly.

Thanks.

-- 
Dmitry