RE: [PATCH v2 08/10] Input: elan_i2c - export true width/height

2019-05-29 Thread



-Original Message-
From: Benjamin Tissoires [mailto:benjamin.tissoi...@redhat.com] 
Sent: Wednesday, May 29, 2019 3:17 PM
To: Sean O'Brien; Peter Hutterer
Cc: Harry Cutts; Dmitry Torokhov; 廖崇榮; Rob Herring; Aaron Ma; Hans de Goede; 
open list:HID CORE LAYER; lkml; devicet...@vger.kernel.org
Subject: Re: [PATCH v2 08/10] Input: elan_i2c - export true width/height

On Wed, May 29, 2019 at 2:12 AM Sean O'Brien  wrote:
>
> We do still use a maxed out major axis as a signal for a palm in the 
> touchscreen logic, but I'm not too concerned because if that axis is 
> maxed out, the contact should probably be treated as a palm anyway...
>
> I'm more concerned with this affecting our gesture detection for 
> touchpad. It looks like this change would cause all contacts to 
> reported as some percentage bigger than they are currently. Can you 
> give me an idea of how big that percentage is?

On the P52, I currently have:
[  +0.09] max:(3045,1731) drivers/input/mouse/elan_i2c_core.c:428
[  +0.03] traces: (24,14) drivers/input/mouse/elan_i2c_core.c:429

-> with the computation done in the kernel:
width_ratio: 126
height_ratio: 123

For my average finger, the reported traces are between 4 and 6:
With the ETP_FWIDTH_REDUCE:
Major between 144 to 216
Minor between 132 to 198

Without:
Major between 504 to 756
Minor between 492 to 738

So a rough augmentation of 350%

For the Synaptics devices (over SMBus), they send the raw value of the traces, 
so you will get a major/minor between 2 to 5. Max on these axes is 15, so we 
should get the same percentage of value comparing to the range.

Elan's vendor report contains such information, which indicate how many trace 
are touched by finger/palm
mk_x = (finger_data[3] & 0x0f); 
mk_y = (finger_data[3] >> 4);
Do we need to use mk_* for major/minor for keeping it consistent with other 
vendor?
But this modification will impact Chromebook's usability and Chrome test suite.



Which is why libinput has a database of which device reports which 
pressure/major/minor ranges as otherwise the values are just impossible to 
understand.

Cheers,
Benjamin



>
> On Tue, May 28, 2019 at 11:13 AM Harry Cutts  wrote:
> >
> > On Mon, 27 May 2019 at 18:21, Dmitry Torokhov  
> > wrote:
> > >
> > > Hi Benjamin, KT,
> > >
> > > On Mon, May 27, 2019 at 11:55:01AM +0800, 廖崇榮 wrote:
> > > > Hi
> > > >
> > > > -Original Message-
> > > > From: Benjamin Tissoires [mailto:benjamin.tissoi...@redhat.com]
> > > > Sent: Friday, May 24, 2019 5:37 PM
> > > > To: Dmitry Torokhov; KT Liao; Rob Herring; Aaron Ma; Hans de 
> > > > Goede
> > > > Cc: open list:HID CORE LAYER; lkml; devicet...@vger.kernel.org
> > > > Subject: Re: [PATCH v2 08/10] Input: elan_i2c - export true 
> > > > width/height
> > > >
> > > > On Tue, May 21, 2019 at 3:28 PM Benjamin Tissoires 
> > > >  wrote:
> > > > >
> > > > > The width/height is actually in the same unit than X and Y. So 
> > > > > we should not tamper the data, but just set the proper 
> > > > > resolution, so that userspace can correctly detect which touch is a 
> > > > > palm or a finger.
> > > > >
> > > > > Signed-off-by: Benjamin Tissoires 
> > > > > 
> > > > >
> > > > > --
> > > > >
> > > > > new in v2
> > > > > ---
> > > > >  drivers/input/mouse/elan_i2c_core.c | 11 ---
> > > > >  1 file changed, 4 insertions(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/drivers/input/mouse/elan_i2c_core.c
> > > > > b/drivers/input/mouse/elan_i2c_core.c
> > > > > index 7ff044c6cd11..6f4feedb7765 100644
> > > > > --- a/drivers/input/mouse/elan_i2c_core.c
> > > > > +++ b/drivers/input/mouse/elan_i2c_core.c
> > > > > @@ -45,7 +45,6 @@
> > > > >  #define DRIVER_NAME"elan_i2c"
> > > > >  #define ELAN_VENDOR_ID 0x04f3
> > > > >  #define ETP_MAX_PRESSURE   255
> > > > > -#define ETP_FWIDTH_REDUCE  90
> > > > >  #define ETP_FINGER_WIDTH   15
> > > > >  #define ETP_RETRY_COUNT3
> > > > >
> > > > > @@ -915,12 +914,8 @@ static void elan_report_contact(struct 
> > > > > elan_tp_data *data,
> > > > > return;
> > > > > }
> > > > >
> > &

RE: [PATCH v2 08/10] Input: elan_i2c - export true width/height

2019-05-26 Thread
Hi

-Original Message-
From: Benjamin Tissoires [mailto:benjamin.tissoi...@redhat.com] 
Sent: Friday, May 24, 2019 5:37 PM
To: Dmitry Torokhov; KT Liao; Rob Herring; Aaron Ma; Hans de Goede
Cc: open list:HID CORE LAYER; lkml; devicet...@vger.kernel.org
Subject: Re: [PATCH v2 08/10] Input: elan_i2c - export true width/height

On Tue, May 21, 2019 at 3:28 PM Benjamin Tissoires 
 wrote:
>
> The width/height is actually in the same unit than X and Y. So we 
> should not tamper the data, but just set the proper resolution, so 
> that userspace can correctly detect which touch is a palm or a finger.
>
> Signed-off-by: Benjamin Tissoires 
>
> --
>
> new in v2
> ---
>  drivers/input/mouse/elan_i2c_core.c | 11 ---
>  1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/input/mouse/elan_i2c_core.c 
> b/drivers/input/mouse/elan_i2c_core.c
> index 7ff044c6cd11..6f4feedb7765 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -45,7 +45,6 @@
>  #define DRIVER_NAME"elan_i2c"
>  #define ELAN_VENDOR_ID 0x04f3
>  #define ETP_MAX_PRESSURE   255
> -#define ETP_FWIDTH_REDUCE  90
>  #define ETP_FINGER_WIDTH   15
>  #define ETP_RETRY_COUNT3
>
> @@ -915,12 +914,8 @@ static void elan_report_contact(struct elan_tp_data 
> *data,
> return;
> }
>
> -   /*
> -* To avoid treating large finger as palm, let's reduce the
> -* width x and y per trace.
> -*/
> -   area_x = mk_x * (data->width_x - ETP_FWIDTH_REDUCE);
> -   area_y = mk_y * (data->width_y - ETP_FWIDTH_REDUCE);
> +   area_x = mk_x * data->width_x;
> +   area_y = mk_y * data->width_y;
>
> major = max(area_x, area_y);
> minor = min(area_x, area_y); @@ -1123,8 +1118,10 @@ 
> static int elan_setup_input_device(struct elan_tp_data *data)
>  ETP_MAX_PRESSURE, 0, 0);
> input_set_abs_params(input, ABS_MT_TOUCH_MAJOR, 0,
>  ETP_FINGER_WIDTH * max_width, 0, 0);
> +   input_abs_set_res(input, ABS_MT_TOUCH_MAJOR, data->x_res);
> input_set_abs_params(input, ABS_MT_TOUCH_MINOR, 0,
>  ETP_FINGER_WIDTH * min_width, 0, 0);
> +   input_abs_set_res(input, ABS_MT_TOUCH_MINOR, data->y_res);

I had a chat with Peter on Wednesday, and he mentioned that this is dangerous 
as Major/Minor are max/min of the width and height. And given that we might 
have 2 different resolutions, we would need to do some computation in the 
kernel to ensure the data is correct with respect to the resolution.

TL;DR: I don't think we should export the resolution there :(

KT, should I drop the patch entirely, or is there a strong argument for keeping 
the ETP_FWIDTH_REDUCE around?
I suggest you apply the patch, I have no idea why ETP_FWIDTH_REDUCE existed. 
Our FW team know nothing about ETP_FWIDTH_REDUCE ether.

The only side effect will happen on Chromebook because such computation have 
stayed in ChromeOS' kernel for four years.
Chrome's finger/palm threshold may be different from other Linux distribution.
We will discuss it with Google once the patch picked by chrome and cause 
something wrong.

Cheers,
Benjamin


>
> data->input = input;
>
> --
> 2.21.0
>



RE: [PATCH v2 09/10] Input: elan_i2c - correct the width/size base value

2019-05-24 Thread
Hi

-Original Message-
From: Benjamin Tissoires [mailto:benjamin.tissoi...@redhat.com] 
Sent: Friday, May 24, 2019 3:06 PM
To: 廖崇榮
Cc: Dmitry Torokhov; Rob Herring; Aaron Ma; Hans de Goede; open list:HID CORE 
LAYER; lkml; devicet...@vger.kernel.org
Subject: Re: [PATCH v2 09/10] Input: elan_i2c - correct the width/size base 
value

On Fri, May 24, 2019 at 5:13 AM 廖崇榮  wrote:
>
> Hi Benjamin,
>
> Thanks so much for all you do for Elan touchpad.
>
> For the width_*, I have a question for it.
> Our antenna sensors fully occupied the whole touchpad PCB.
>
> The Gap between 2 sensors are 7.5 mil (0.19mm).
> That's why we did not minus one trace.

So, with the P52 I have:
[  +0.09] max:(3045,1731) drivers/input/mouse/elan_i2c_core.c:428
[  +0.03] traces: (24,14) drivers/input/mouse/elan_i2c_core.c:429
[  +0.02] size:   (98,55) drivers/input/mouse/elan_i2c_core.c:430
[  +0.01] res:(31,31) drivers/input/mouse/elan_i2c_core.c:431

calculated size (max/res): 98 x 56 mm
true size, as measured: 101 x 60 mm

I list layout information of P52 touchpad as below.
Physical size : 99 x 58 mm
Active Area size : ~ 97 * 56 mm, (boarding is 1.008mm for each side)

Sensor layout:
X Pitch : 4.0286 mm
Y Pitch : 4.0147 mm

Which gives (without the minus 1):
width_x = max_x / x_traces = 3045 / 24 = 126.875 -> 3.9885 mm width_y = max_y / 
y_traces = 1731 / 14 = 123.643 -> 4.0927 mm

-> this gives a total size of the touchpad of: 96 x 57 mm (width_x *
24, width_y * 14)

With the minus 1:
width_x = max_x / x_traces = 3045 / 23 = 132.391 -> 4.2707 mm width_y = max_y / 
y_traces = 1731 / 14 = 133.154 -> 4.2953 mm

-> this gives a total size of the touchpad of: 102 x 60 mm (width_x *
24, width_y * 14)
and considering traces-1: 98 x 56 mm

Removing 1 to the number of traces gave a squarer values in rows and columns, 
and this is what is done in the PS/2 driver.
Also, going back to the size of the touchpad gives a better value when removing 
1 on the *traces.
So maybe when forwarding the properties we should remove one there in the PS/2 
driver?

Removing 1 trace may be better for some of previous touchpad. (depending on 
sensor pattern)
mk_* indicate the number of trace which is touched, and it's not a precise 
value.
I think the usability won't change too much whether removing one trace.
PS/2 have supported plenty of touchpad. It's better to remain the same.

Thanks
KT

Cheers,
Benjamin

>
>
> Thanks
> KT
> -Original Message-
> From: Benjamin Tissoires [mailto:benjamin.tissoi...@redhat.com]
> Sent: Tuesday, May 21, 2019 9:27 PM
> To: Dmitry Torokhov; KT Liao; Rob Herring; Aaron Ma; Hans de Goede
> Cc: linux-in...@vger.kernel.org; linux-kernel@vger.kernel.org; 
> devicet...@vger.kernel.org; Benjamin Tissoires
> Subject: [PATCH v2 09/10] Input: elan_i2c - correct the width/size 
> base value
>
> *_traces are the number of antennas. width_* is thus the space between 
> 2 antennas. Which means, we should subtract 1 to the number of 
> antennas to divide the touchpad by the number of holes between each antenna.
>
> Signed-off-by: Benjamin Tissoires 
>
> --
>
> new in v2
> ---
>  drivers/input/mouse/elan_i2c_core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/mouse/elan_i2c_core.c
> b/drivers/input/mouse/elan_i2c_core.c
> index 6f4feedb7765..3375eaa9a72e 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -398,8 +398,8 @@ static int elan_query_device_parameters(struct
> elan_tp_data *data)
> if (error)
> return error;
> }
> -   data->width_x = data->max_x / x_traces;
> -   data->width_y = data->max_y / y_traces;
> +   data->width_x = data->max_x / (x_traces - 1);
> +   data->width_y = data->max_y / (y_traces - 1);
>
> if (device_property_read_u32(&client->dev,
>  "touchscreen-x-mm", &x_mm) ||
> --
> 2.21.0
>



RE: [PATCH v2 09/10] Input: elan_i2c - correct the width/size base value

2019-05-23 Thread
Hi Benjamin,

Thanks so much for all you do for Elan touchpad.

For the width_*, I have a question for it.
Our antenna sensors fully occupied the whole touchpad PCB.

The Gap between 2 sensors are 7.5 mil (0.19mm).
That's why we did not minus one trace.


Thanks
KT
-Original Message-
From: Benjamin Tissoires [mailto:benjamin.tissoi...@redhat.com] 
Sent: Tuesday, May 21, 2019 9:27 PM
To: Dmitry Torokhov; KT Liao; Rob Herring; Aaron Ma; Hans de Goede
Cc: linux-in...@vger.kernel.org; linux-kernel@vger.kernel.org;
devicet...@vger.kernel.org; Benjamin Tissoires
Subject: [PATCH v2 09/10] Input: elan_i2c - correct the width/size base
value

*_traces are the number of antennas. width_* is thus the space between 2
antennas. Which means, we should subtract 1 to the number of antennas to
divide the touchpad by the number of holes between each antenna.

Signed-off-by: Benjamin Tissoires 

--

new in v2
---
 drivers/input/mouse/elan_i2c_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/mouse/elan_i2c_core.c
b/drivers/input/mouse/elan_i2c_core.c
index 6f4feedb7765..3375eaa9a72e 100644
--- a/drivers/input/mouse/elan_i2c_core.c
+++ b/drivers/input/mouse/elan_i2c_core.c
@@ -398,8 +398,8 @@ static int elan_query_device_parameters(struct
elan_tp_data *data)
if (error)
return error;
}
-   data->width_x = data->max_x / x_traces;
-   data->width_y = data->max_y / y_traces;
+   data->width_x = data->max_x / (x_traces - 1);
+   data->width_y = data->max_y / (y_traces - 1);
 
if (device_property_read_u32(&client->dev,
 "touchscreen-x-mm", &x_mm) ||
--
2.21.0



RE: [PATCH] ELAN touchpad i2c_hid bugs fix

2019-04-01 Thread
Hi Dmitry,

-Original Message-
From: Dmitry Torokhov [mailto:d...@chromium.org] 
Sent: Saturday, March 30, 2019 2:24 AM
To: Hans de Goede; 廖崇榮
Cc: Vladislav Dalechyn; Benjamin Tissoires; Jiri Kosina; 
kai.heng.f...@canonical.com; swb...@chromium.org; bige...@linutronix.de; open 
list:HID CORE LAYER; lkml; hotwater...@tutanota.com
Subject: Re: [PATCH] ELAN touchpad i2c_hid bugs fix

On Fri, Mar 29, 2019 at 5:18 AM Hans de Goede  wrote:
>
> Hi,
>
> On 3/25/19 5:56 PM, Dmitry Torokhov wrote:
> > Hi Hans,
> >
> > On Mon, Mar 25, 2019 at 9:38 AM Hans de Goede  wrote:
> >>
> >> Hi Dmitry,
> >>
> >> On 25-03-19 17:02, Dmitry Torokhov wrote:
> >>> Hi Vladislav,
> >>>
> >>> On Mon, Mar 25, 2019 at 5:57 AM Vladislav Dalechyn 
> >>>  wrote:
> >>>>
> >>>> From: Vladislav Dalechyn 
> >>>>
> >>>> Description: The ELAN1200:04F3:303E touchpad exposes several 
> >>>> issues, all caused by an error setting the correct IRQ_TRIGGER flag:
> >>>> - i2c_hid incoplete error flood in journalctl;
> >>>> - Five finger tap kill's module so you have to restart it;
> >>>> - Two finger scoll is working incorrect and sometimes even when 
> >>>> you raised one of two finger still thinks that you are scrolling.
> >>>>
> >>>> Fix all of these with a new quirk that corrects the trigger flag 
> >>>> announced by the ACPI tables. (edge-falling).
> >>>
> >>> I do not believe this is right solution. The driver makes liberal 
> >>> use of disable_irq() and enable_irq() which may lead to lost edges 
> >>> and touchpad stopping working altogether.
> >>>
> >>> Usually the "extra" report is caused by GPIO controller clearing 
> >>> interrupt condition at the wrong time (too early), or in unsafe or 
> >>> racy fashion. You need to look there instead of adding quirk to 
> >>> i2c-hid.
> >>
> >> The falling-edge solution was proposed by Elan themselves.
> >>
> >> Also if you look at: 
> >> https://bugzilla.redhat.com/show_bug.cgi?id=1543769
> >>
> >> And esp. the "cat /proc/interrupts" output there, then you will see 
> >> that the interrupt seems to be stuck at low level, which according 
> >> to the ACPI tables is its active level.
> >
> > So how does it generate a new edge if it is stuck at low?
> >
> > Is it bad touchpad firmware that does not deassert interrupt quickly 
> > enough?
>
> I do not believe it is not de-asserting it quick enough (I believe the 
> amount of interrups is too high for that.
>
> It seems to simply be low most of the time, or it is really really 
> slow with de-asserting.
>
> Vladislav can you check the output of /cat/interrupts on a kernel 
> without the patch and while *not* using the touchpad; and check if the 
> amount of touchpads-interrupts still keeps increasing in this case?
>
> Also I believe that you had contact with Elan about this and they told 
> you to change the interrupt type to falling-edge as work-around, 
> right?  Can you ask them why?

KT, do you know anything about using edge interrupts with your hid-over-i2c 
products?

Ya, I sent the patch to Vladislav for testing his 5 finger-tap issue.

The patch is created by Dell/Intel for debugging Elan PTP's incomplete report 
message.
Host tried to get report again while touchpad finish report transmission.

I excerpt from Dell's mail
===Begin=
We have worked with Intel for a while to revise Linux kernel driver code to 
prevent Touch I2C error message show up,
And Intel dig out there’re 2 issues to cause this i2c error on Linux kernel 
4.18.

Therefore Intel suggested to revised SW code into “trigger falling edge + 
pm_disabled + IRQF_SHARED | IRQF_COND_SUSPEND(i2c-designware-master.c)” 
and then we can get positive result that i2c error message disappeared when 
Touch Panel is working.

Since this issue only happened on ELAN touch pad + Intel GLK platform (Linux) 
and other Intel reference platform (Whisky lake) won’t, would you help check 
the solution (as patch attached)  and let us know if any question/concern for 
this modification?  
===End=

I don't know the root cause of 5-finger issue with level trigger so far.
I will check it once I get the touchpad with the same issue.

>From previous issue list, it seems that some touchpad's crush issue will be 
>fixed by edge trigger.

I have discussed with FW engineer, both level and edge trigger should be OK for 
our PTP(HID touchpad).
I summary 

RE: [PATCH] Input: elan_i2c - Add i2c_reset in sysfs for ELAN touchpad recovery

2019-03-28 Thread
Hi Dmitry,

-Original Message-
From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com] 
Sent: Wednesday, March 27, 2019 12:34 PM
To: KT Liao
Cc: linux-kernel@vger.kernel.org; linux-in...@vger.kernel.org;
ulrik.debie...@e2big.org; roger.whitta...@suse.com
Subject: Re: [PATCH] Input: elan_i2c - Add i2c_reset in sysfs for ELAN
touchpad recovery

Hi KT,

On Sat, Feb 02, 2019 at 03:54:56PM +0800, KT Liao wrote:
> Roger from SUSE reported the touchpad on Lenovo yoga2 crush sometimes.
> He found that rmmod/modprobe elan_i2c will recover the issue.
> He add the workaround on SUSE and solve the problem.
> Recently, the workaround fails in kernel 4.20 becasue IRQ mismatch.
> genirq: Flags mismatch irq 0. 2002 (ELAN0600:00) vs. 00015a00 
> (timer) I can't reproduce the issue in SUSE with the same kernel.
> And it's a 5 years old laptop, ELAN can't find the module for testing.
> Instead of IRQ debugging IRQ, I tried another approach.
> I added i2c_reset in sysfs to avoid IRQ requesting in probe.

How will users discover this flag? I do not think this is the best approach.
Can we detect that the touchpad firmware crashed from the kernel and reset
automatically?
Agree your better idea.
I will modify the driver for Roger's testing.
Will upstream after his testing.

Thanks.

> 
> Signed-off-by: KT Liao 
> Acked-by: Roger Whittaker 
> ---
>  drivers/input/mouse/elan_i2c_core.c | 25 +
>  1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/input/mouse/elan_i2c_core.c 
> b/drivers/input/mouse/elan_i2c_core.c
> index 2690a4b..388b1f0 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -670,6 +670,29 @@ static ssize_t calibrate_store(struct device *dev,
>   return retval ?: count;
>  }
>  
> +static ssize_t i2c_reset_store(struct device *dev,
> +struct device_attribute *attr,
> +const char *buf, size_t count) {
> + struct i2c_client *client = to_i2c_client(dev);
> + struct elan_tp_data *data = i2c_get_clientdata(client);
> + int retval;
> +
> + retval = mutex_lock_interruptible(&data->sysfs_mutex);
> + if (retval)
> + return retval;
> +
> + disable_irq(client->irq);
> +
> + retval = elan_initialize(data);
> + if (retval)
> + dev_err(dev, "failed to re-initialize touchpad: %d\n",
retval);
> +
> + enable_irq(client->irq);
> + mutex_unlock(&data->sysfs_mutex);
> + return retval ?: count;
> +}
> +
>  static ssize_t elan_sysfs_read_mode(struct device *dev,
>   struct device_attribute *attr,
>   char *buf)
> @@ -702,6 +725,7 @@ static DEVICE_ATTR(mode, S_IRUGO, 
> elan_sysfs_read_mode, NULL);  static DEVICE_ATTR(update_fw, S_IWUSR, 
> NULL, elan_sysfs_update_fw);
>  
>  static DEVICE_ATTR_WO(calibrate);
> +static DEVICE_ATTR_WO(i2c_reset);
>  
>  static struct attribute *elan_sysfs_entries[] = {
>   &dev_attr_product_id.attr,
> @@ -710,6 +734,7 @@ static struct attribute *elan_sysfs_entries[] = {
>   &dev_attr_iap_version.attr,
>   &dev_attr_fw_checksum.attr,
>   &dev_attr_calibrate.attr,
> + &dev_attr_i2c_reset.attr,
>   &dev_attr_mode.attr,
>   &dev_attr_update_fw.attr,
>   NULL,
> --
> 2.7.4
> 

-- 
Dmitry



RE: [PATCH 1/1] Input: elantech: Use SMBus based on bus info

2019-01-20 Thread



-Original Message-
From: Kai-Heng Feng [mailto:kai.heng.f...@canonical.com] 
Sent: Monday, January 21, 2019 12:28 PM
To: 廖崇榮
Cc: Benjamin Tissoires; Dmitry Torokhov; open list:HID CORE LAYER; lkml
Subject: Re: [PATCH 1/1] Input: elantech: Use SMBus based on bus info



> On Jan 18, 2019, at 17:29, 廖崇榮  wrote:
> 
> 
> 
> -Original Message-
> From: Kai Heng Feng [mailto:kai.heng.f...@canonical.com]
> Sent: Friday, January 18, 2019 12:15 AM
> To: Benjamin Tissoires
> Cc: Dmitry Torokhov; 廖崇榮; open list:HID CORE LAYER; lkml
> Subject: Re: [PATCH 1/1] Input: elantech: Use SMBus based on bus info
> 
> 
> 
>> On Jan 17, 2019, at 10:42 PM, Benjamin Tissoires
>  wrote:
>> 
>> Hi Kai-Heng,
>> 
>> On Thu, Jan 17, 2019 at 10:30 AM Kai-Heng Feng 
>>  wrote:
>>> 
>>> There are some new HP laptops with Elantech touchpad don't support 
>>> multitouch.
>>> 
>>> Both ETP_BUS_SMB_HST_NTFY_ONLY and ETP_BUS_PS2_SMB_HST_NTFY devices 
>>> can use SMBus to support 5 fingers touch, so we need to chech them too.
>>> 
>>> So use elantech_use_host_notify() to do a more thouroughly check.
>>> 
>>> Signed-off-by: Kai-Heng Feng 
>>> ---
>>> drivers/input/mouse/elantech.c | 58
>>> +-
>>> 1 file changed, 29 insertions(+), 29 deletions(-)
>>> 
>>> diff --git a/drivers/input/mouse/elantech.c 
>>> b/drivers/input/mouse/elantech.c index 9fe075c137dc..5bcf1c147eb1
>>> 100644
>>> --- a/drivers/input/mouse/elantech.c
>>> +++ b/drivers/input/mouse/elantech.c
>>> @@ -1799,6 +1799,34 @@ static int elantech_create_smbus(struct 
>>> psmouse
> *psmouse,
>>> leave_breadcrumbs); }
>>> 
>>> +static bool elantech_use_host_notify(struct psmouse *psmouse,
>>> +struct elantech_device_info
>>> +*info) {
>>> +   if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version))
>>> +   return true;
>>> +
>>> +   switch (info->bus) {
>>> +   case ETP_BUS_PS2_ONLY:
>>> +   /* expected case */
>>> +   break;
>>> +   case ETP_BUS_SMB_ALERT_ONLY:
>>> +   /* fall-through  */
>>> +   case ETP_BUS_PS2_SMB_ALERT:
>>> +   psmouse_dbg(psmouse, "Ignoring SMBus provider 
>>> + through
> alert protocol.\n");
>>> +   break;
>>> +   case ETP_BUS_SMB_HST_NTFY_ONLY:
>>> +   /* fall-through  */
>>> +   case ETP_BUS_PS2_SMB_HST_NTFY:
>>> +   return true;
>>> +   default:
>>> +   psmouse_dbg(psmouse,
>>> +   "Ignoring SMBus bus provider %d.\n",
>>> +   info->bus);
>>> +   }
>>> +
>>> +   return false;
>>> +}
>>> +
>>> /**
>>> * elantech_setup_smbus - called once the PS/2 devices are enumerated
>>> * and decides to instantiate a SMBus InterTouch device.
>>> @@ -1818,7 +1846,7 @@ static int elantech_setup_smbus(struct psmouse
> *psmouse,
>>>* i2c_blacklist_pnp_ids.
>>>* Old ICs are up to the user to decide.
>>>*/
>>> -   if (!ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version) ||
>>> +   if (!elantech_use_host_notify(psmouse, info) ||
>> 
>> That was my initial approach of the series, but I ended up being more 
>> conservative as this would flip all of the existing elantech SMBUS 
>> capable touchpads to use elan_i2c.
>> And I didn't want to deal with 4/5 year old laptops that suddenly broke.
>> 
>> So I wonder if you can restrict this default change to the recent 
>> laptops (let's say 2018+). Maybe by looking at their FW version or 
>> something else in the DMI?
> 
> It was KT who told me that I should use ETP_BUS_PS2_SMB_HST_NTFY.
> 
> As for date, KT still knows better than me.
> 
> KT,
> Can you name a year which is safe enough to enable SMBus?
> 
> I have discussed it internally. 
> The internal rule for FW's SMbus implementation is stable after 2018 
> If you meet some special case, please let me know.

Thanks for the info. I’ll use this for the V2 patch.

> 
> BTW, The SMbus supporting is requested by HP this time, and there are 
> plenty of HP laptop use old IC which doesn't meet " 
> ETP_NEW_IC_SMBUS_HOST_NOTIFY”.

One 

RE: [PATCH 1/1] Input: elantech: Use SMBus based on bus info

2019-01-18 Thread



-Original Message-
From: Kai Heng Feng [mailto:kai.heng.f...@canonical.com] 
Sent: Friday, January 18, 2019 12:15 AM
To: Benjamin Tissoires
Cc: Dmitry Torokhov; 廖崇榮; open list:HID CORE LAYER; lkml
Subject: Re: [PATCH 1/1] Input: elantech: Use SMBus based on bus info



> On Jan 17, 2019, at 10:42 PM, Benjamin Tissoires
 wrote:
> 
> Hi Kai-Heng,
> 
> On Thu, Jan 17, 2019 at 10:30 AM Kai-Heng Feng 
>  wrote:
>> 
>> There are some new HP laptops with Elantech touchpad don't support 
>> multitouch.
>> 
>> Both ETP_BUS_SMB_HST_NTFY_ONLY and ETP_BUS_PS2_SMB_HST_NTFY devices 
>> can use SMBus to support 5 fingers touch, so we need to chech them too.
>> 
>> So use elantech_use_host_notify() to do a more thouroughly check.
>> 
>> Signed-off-by: Kai-Heng Feng 
>> ---
>> drivers/input/mouse/elantech.c | 58 
>> +-
>> 1 file changed, 29 insertions(+), 29 deletions(-)
>> 
>> diff --git a/drivers/input/mouse/elantech.c 
>> b/drivers/input/mouse/elantech.c index 9fe075c137dc..5bcf1c147eb1 
>> 100644
>> --- a/drivers/input/mouse/elantech.c
>> +++ b/drivers/input/mouse/elantech.c
>> @@ -1799,6 +1799,34 @@ static int elantech_create_smbus(struct psmouse
*psmouse,
>>  leave_breadcrumbs); }
>> 
>> +static bool elantech_use_host_notify(struct psmouse *psmouse,
>> +struct elantech_device_info 
>> +*info) {
>> +   if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version))
>> +   return true;
>> +
>> +   switch (info->bus) {
>> +   case ETP_BUS_PS2_ONLY:
>> +   /* expected case */
>> +   break;
>> +   case ETP_BUS_SMB_ALERT_ONLY:
>> +   /* fall-through  */
>> +   case ETP_BUS_PS2_SMB_ALERT:
>> +   psmouse_dbg(psmouse, "Ignoring SMBus provider through
alert protocol.\n");
>> +   break;
>> +   case ETP_BUS_SMB_HST_NTFY_ONLY:
>> +   /* fall-through  */
>> +   case ETP_BUS_PS2_SMB_HST_NTFY:
>> +   return true;
>> +   default:
>> +   psmouse_dbg(psmouse,
>> +   "Ignoring SMBus bus provider %d.\n",
>> +   info->bus);
>> +   }
>> +
>> +   return false;
>> +}
>> +
>> /**
>>  * elantech_setup_smbus - called once the PS/2 devices are enumerated
>>  * and decides to instantiate a SMBus InterTouch device.
>> @@ -1818,7 +1846,7 @@ static int elantech_setup_smbus(struct psmouse
*psmouse,
>> * i2c_blacklist_pnp_ids.
>> * Old ICs are up to the user to decide.
>> */
>> -   if (!ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version) ||
>> +   if (!elantech_use_host_notify(psmouse, info) ||
> 
> That was my initial approach of the series, but I ended up being more 
> conservative as this would flip all of the existing elantech SMBUS 
> capable touchpads to use elan_i2c.
> And I didn't want to deal with 4/5 year old laptops that suddenly broke.
> 
> So I wonder if you can restrict this default change to the recent 
> laptops (let's say 2018+). Maybe by looking at their FW version or 
> something else in the DMI?

It was KT who told me that I should use ETP_BUS_PS2_SMB_HST_NTFY.

As for date, KT still knows better than me.

KT,
Can you name a year which is safe enough to enable SMBus?

I have discussed it internally. 
The internal rule for FW's SMbus implementation is stable after 2018
If you meet some special case, please let me know.

BTW, The SMbus supporting is requested by HP this time, and there are plenty
of HP laptop use old IC
which doesn't meet " ETP_NEW_IC_SMBUS_HOST_NOTIFY".

Elan touchpad works well in PS/2 for HP, because it don't support
TrackPoint.
You may let old HP platform work as PS/2 for safety.

Thanks
KT

Kai-Heng

> 
> Cheers,
> Benjamin
> 
>>psmouse_matches_pnp_id(psmouse,
i2c_blacklist_pnp_ids))
>>return -ENXIO;
>>}
>> @@ -1838,34 +1866,6 @@ static int elantech_setup_smbus(struct psmouse
*psmouse,
>>return 0;
>> }
>> 
>> -static bool elantech_use_host_notify(struct psmouse *psmouse,
>> -struct elantech_device_info *info)
>> -{
>> -   if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version))
>> -   return true;
>> -
>> -   switch (info->bus) {
>> -   case ETP_BUS_PS2_ONLY:

RE: [PATCH] Input: elantech - Fix V4 report decoding for module with middle key

2018-05-30 Thread
Hi Dmitry,

-Original Message-
From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com] 
Sent: Wednesday, May 30, 2018 2:05 AM
To: KT Liao
Cc: linux-kernel@vger.kernel.org; linux-in...@vger.kernel.org;
ulrik.debie...@e2big.org; phoe...@emc.com.tw; aaron...@canonical.com;
josh.c...@emc.com.tw
Subject: Re: [PATCH] Input: elantech - Fix V4 report decoding for module
with middle key

Hi KT,

On Mon, May 28, 2018 at 07:33:02PM +0800, KT Liao wrote:
> Some touchpad has middle key and it will be indicated in bit 2 of
packet[0].
> We need to fix V4 formation's byte mask to prevent error decoding.

Could you please let me know what devices this patch fixes? Are they
released or new hardware?


The primary target is Lenovo thinkpad P52 and it will be released in 6/M.

force_crc_enabled will fix the issue too because less bit-check in the
specific byte. 
I guess Fujitsu H730/H760 in elantech_dmi_force_crc_enabled may have the
same issue.
I leave them in DMI table because I am not sure of it.

Thanks
KT
> 
> Signed-off-by: KT Liao 
> ---
>  drivers/input/mouse/elantech.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/mouse/elantech.c 
> b/drivers/input/mouse/elantech.c index fb4d902..f39dc66 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -799,7 +799,7 @@ static int elantech_packet_check_v4(struct psmouse
*psmouse)
>   else if (ic_version == 7 && etd->info.samples[1] == 0x2A)
>   sanity_check = ((packet[3] & 0x1c) == 0x10);
>   else
> - sanity_check = ((packet[0] & 0x0c) == 0x04 &&
> + sanity_check = ((packet[0] & 0x08) == 0x00 &&
>   (packet[3] & 0x1c) == 0x10);
>  
>   if (!sanity_check)
> --
> 2.7.4
> 

Thanks.

-- 
Dmitry



RE: [PATCH v2 0/9] Input: support for latest Lenovo thinkpads (series 80)

2018-04-17 Thread
Hi Benjamin,

I agree this series.

Thanks
KT

-Original Message-
From: Benjamin Tissoires [mailto:benjamin.tissoi...@redhat.com] 
Sent: Tuesday, April 17, 2018 4:34 PM
To: 廖崇榮
Cc: Oliver Haessler; Benjamin Berg; Rob Herring; devicet...@vger.kernel.org; 
open list:HID CORE LAYER; lkml; Dmitry Torokhov; Aaron Ma
Subject: Re: [PATCH v2 0/9] Input: support for latest Lenovo thinkpads (series 
80)

Hi KT,

gentle ping :)

Could you ACK/NACK this series?

Dmitry, the first patch could go without KT's approval. Also I realized that 
Aaron submitted a similar patch for the X1 Carbon last
October: https://patchwork.kernel.org/patch/10008513/
So I think the first one should go ASAP now that the laptops are shipping from 
Lenovo.

Cheers,
Benjamin

On Mon, Apr 9, 2018 at 11:10 AM, Benjamin Tissoires 
 wrote:
> Hi Dmitry,
>
> Here is the v2 of the Lenovo 80 series.
> Changes from v1:
> - included patch to convert a function to static from build bot
> - use of device property instead of platform data (thus the new device tree
>   binding)
>
> BTW, KT, if you want to add your Signed-off-by on the patches, feel 
> free to do so. You were of tremendous help :)
>
> Cheers,
> Benjamin
>
> Benjamin Tissoires (9):
>   Input: synaptics - add Lenovo 80 series ids to SMBus
>   input: elan_i2c_smbus - fix corrupted stack
>   Input: elan_i2c - add trackstick report
>   Input: elantech - split device info into a separate structure
>   Input: elantech_query_info() can be static
>   Input: elantech - query the resolution in query_info
>   Input: elantech - add support for SMBus devices
>   Input: elantech - detect new ICs and setup Host Notify for them
>   input: psmouse-smbus: allow to control psmouse_deactivate
>
>  .../devicetree/bindings/input/elan_i2c.txt |   1 +
>  drivers/input/mouse/Kconfig|  12 +
>  drivers/input/mouse/elan_i2c_core.c|  90 +++-
>  drivers/input/mouse/elan_i2c_smbus.c   |  22 +-
>  drivers/input/mouse/elantech.c | 479 
> +++--
>  drivers/input/mouse/elantech.h |  69 ++-
>  drivers/input/mouse/psmouse-base.c |  21 +-
>  drivers/input/mouse/psmouse-smbus.c|  24 +-
>  drivers/input/mouse/psmouse.h  |   2 +
>  drivers/input/mouse/synaptics.c|   5 +-
>  10 files changed, 565 insertions(+), 160 deletions(-)
>
> --
> 2.14.3
>



RE: [PATCH 0/8] Input: support for latest Lenovo thinkpads (series 80)

2018-04-10 Thread
Hi Benjamin,

-Original Message-
From: Benjamin Tissoires [mailto:benjamin.tissoi...@redhat.com] 
Sent: Tuesday, April 10, 2018 3:35 PM
To: 廖崇榮
Cc: Dmitry Torokhov; Oliver Haessler; Benjamin Berg; open list:HID CORE LAYER; 
lkml
Subject: Re: [PATCH 0/8] Input: support for latest Lenovo thinkpads (series 80)

Hi KT,

On Tue, Apr 10, 2018 at 7:45 AM, 廖崇榮  wrote:
> Hi Benjamin,
>
> Thanks so much for your patch.
>
> I have tested them for Elan Gen5/Gen6(new) touchpad with SMbus/PS2.
> It works fine in my thinkpad so far but I find an issue today after 
> lid-close/open.
>
> I am not sure if you can see it in T480S , I "guess" it may be relative to 
> i2c_i801.
>
> The lid-close will enter deep sleep and cut touchpad power.
> I can see the resume flow after lid-open and SMbus-initial try to request 
> hello package but fail.
> Strangely, I can't see any SMbus host signal on LA scope after power-on.

That's weird. I do not see this, by either closing the lid or directly calling 
'systemctl suspend'.
On my system, i2c-i801 is also compiled as a module but psmouse is not 
(directly in vmlinuz).
[KT] : It's good to know your system doesn't meet this problem. 
 My SMbus laptop is an engineer sample in 2015, and PM tell me that NFC 
attaches to the same bus.
 I guess it's a single case because it's not a stable platform.

>
> I can't switch to SMbus after rmmod/modprobe psmouse because error happen in 
> elantech_create_smbus.

If the SMBus adapter is failing, it is somewhat expected. Reloading psmouse 
will force a re-trigger of the SMBus probe function, but if the underlying 
communication fails, there is no way for psmouse to know it failed, so the PS/2 
node will disappear. And the SMBus device will not be there.
[KT] : It switch to PS/2 interface after rmmod/modprobe psmouse.

> It will be recovered only if I rmmod/modprobe i2c_i801 first.

Just to be sure, what happens if you rmmod/modprobe elan_i2c instead of 
i2c_i801?
[KT] :I tried rmmod/modprobe elan_i2c first , but can't recover touchpad. 
No error printed in the dmesg. I will add more message to check it later.

And which kernel are you running? On a vanilla 4.16 + Dmitry's next branch I do 
not see such issues.
[KT]: I use 4.15, I may try 4.16 later.

>
> Do you have any idea about it?

If reloading elan_i2c doesn't fix the situation, it must be in i2c_i801. But 
this is weird that this happens on your platform but not on my t480s going into 
S3.
[KT] I think so. As I mention that I can't see bus signal on scope. That make 
me guess it's the bus adaptor issue.
It's an early-stage engineer system, power Issue will be expected.

Cheers,
Benjamin

>
> Thanks
> KT
> -----Original Message-
> From: Benjamin Tissoires [mailto:benjamin.tissoi...@redhat.com]
> Sent: Friday, April 06, 2018 2:51 PM
> To: Dmitry Torokhov
> Cc: 廖崇榮; Oliver Haessler; Benjamin Berg; open list:HID CORE LAYER; 
> lkml
> Subject: Re: [PATCH 0/8] Input: support for latest Lenovo thinkpads 
> (series 80)
>
> Hi Dmitry,
>
> On Fri, Apr 6, 2018 at 1:51 AM, Dmitry Torokhov  
> wrote:
>> Hi Benjamin,
>>
>> On Thu, Apr 05, 2018 at 03:25:29PM +0200, Benjamin Tissoires wrote:
>>> Hi Dmitry,
>>>
>>> well, this year, Lenovo gave us a surprise and decided to not use 
>>> the same touchpad/trackstick in all its model. And by default, the 
>>> support under Linux is less than ideal.
>>>
>>> Please find a series that should fix those issues. Compared to the 
>>> 60 series, there do not seem to e BIOS table issues this time, and 
>>> suspend/resume works fine thanks to your latest trackstick fixes.
>>>
>>> The T480s is a different beast, as it uses an Elan touchpad.
>>> I have been carrying the patches 3-6 for a while and tested previous 
>>> versions on various Elan PS/2 hardware without an issue as far as I 
>>> could tell. I was lacking tests from users with SMBus as all the 
>>> laptops I tried where puer PS/2.
>>>
>>> Anyway, it would be cool if you could have a look at the series.
>>
>> I am mostly happy with the series, but I would love to hear KT's take 
>> on it.
>
> thanks for the quick review.
> I worked closely with KT for this series. He helped me a lot for the tiny 
> firmware changes that were required. However, quoting his email from Tuesday:
> "There will be a spring vacation in Taiwan from tomorrow." I guess we won't 
> hear from him until the end of next week as we always have a backlog of 
> urgent things to do after holidays...
>
> Cheers,
> Benjamin
>



RE: [PATCH 0/8] Input: support for latest Lenovo thinkpads (series 80)

2018-04-09 Thread
Hi Benjamin,

Thanks so much for your patch.

I have tested them for Elan Gen5/Gen6(new) touchpad with SMbus/PS2.
It works fine in my thinkpad so far but I find an issue today after 
lid-close/open.

I am not sure if you can see it in T480S , I "guess" it may be relative to 
i2c_i801.

The lid-close will enter deep sleep and cut touchpad power. 
I can see the resume flow after lid-open and SMbus-initial try to request hello 
package but fail.
Strangely, I can't see any SMbus host signal on LA scope after power-on.

I can't switch to SMbus after rmmod/modprobe psmouse because error happen in 
elantech_create_smbus.
It will be recovered only if I rmmod/modprobe i2c_i801 first.

Do you have any idea about it?

Thanks
KT
-Original Message-
From: Benjamin Tissoires [mailto:benjamin.tissoi...@redhat.com] 
Sent: Friday, April 06, 2018 2:51 PM
To: Dmitry Torokhov
Cc: 廖崇榮; Oliver Haessler; Benjamin Berg; open list:HID CORE LAYER; lkml
Subject: Re: [PATCH 0/8] Input: support for latest Lenovo thinkpads (series 80)

Hi Dmitry,

On Fri, Apr 6, 2018 at 1:51 AM, Dmitry Torokhov  
wrote:
> Hi Benjamin,
>
> On Thu, Apr 05, 2018 at 03:25:29PM +0200, Benjamin Tissoires wrote:
>> Hi Dmitry,
>>
>> well, this year, Lenovo gave us a surprise and decided to not use the 
>> same touchpad/trackstick in all its model. And by default, the 
>> support under Linux is less than ideal.
>>
>> Please find a series that should fix those issues. Compared to the 60 
>> series, there do not seem to e BIOS table issues this time, and 
>> suspend/resume works fine thanks to your latest trackstick fixes.
>>
>> The T480s is a different beast, as it uses an Elan touchpad.
>> I have been carrying the patches 3-6 for a while and tested previous 
>> versions on various Elan PS/2 hardware without an issue as far as I 
>> could tell. I was lacking tests from users with SMBus as all the 
>> laptops I tried where puer PS/2.
>>
>> Anyway, it would be cool if you could have a look at the series.
>
> I am mostly happy with the series, but I would love to hear KT's take 
> on it.

thanks for the quick review.
I worked closely with KT for this series. He helped me a lot for the tiny 
firmware changes that were required. However, quoting his email from Tuesday:
"There will be a spring vacation in Taiwan from tomorrow." I guess we won't 
hear from him until the end of next week as we always have a backlog of urgent 
things to do after holidays...

Cheers,
Benjamin



RE: [PATCH] Input: elantech - support new touchpad IC and extend elan's touchapd command Origianl ic-body field is not sufficient for upcoming IC, Elan ps/2 driver extend the fomation to support futur

2017-08-07 Thread
Hi Ulrik,
Thanks your review, I add comment in below.

-Original Message-
From: ulrik.debie...@e2big.org [mailto:ulrik.debie...@e2big.org] 
Sent: Friday, August 04, 2017 4:52 AM
To: KT Liao
Cc: linux-kernel@vger.kernel.org; linux-in...@vger.kernel.org;
dmitry.torok...@gmail.com; phoe...@emc.com.tw
Subject: Re: [PATCH] Input: elantech - support new touchpad IC and extend
elan's touchapd command Origianl ic-body field is not sufficient for
upcoming IC, Elan ps/2 driver extend the fomation to support future IC.
Signed-off-by: KT Liao 

Hi,

Something went wrong with your subject line, it also includes the further
commit message lines and the signed off line ..

Origianl -> Original
fomation .. Do you mean information ?
[KT]:Fix typo and subject line in next patch
On Wed, Aug 02, 2017 at 07:11:41PM +0800, KT Liao wrote:
> 
> ---
>  drivers/input/mouse/elantech.c | 28   
> drivers/input/mouse/elantech.h |  3 +++
>  2 files changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/input/mouse/elantech.c 
> b/drivers/input/mouse/elantech.c index 65c9de3..14ab5ba 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -1398,6 +1398,10 @@ static bool elantech_is_signature_valid(const
unsigned char *param)
>   param[2] < 40)
>   return true;
>  
> + /* hw_version 0x0F does not need to check rate alose*/

Drop the word also:
[KT] : OK

 /* hw_version 0x0F does not need to check rate */

> + if ((param[0] & 0x0f) == 0x0f)
> + return true;
> +
>   for (i = 0; i < ARRAY_SIZE(rates); i++)
>   if (param[2] == rates[i])
>   return false;
> @@ -1576,7 +1580,7 @@ static const struct dmi_system_id 
> no_hw_res_dmi_table[] = {
>  /*
>   * determine hardware version and set some properties according to it.
>   */
> -static int elantech_set_properties(struct elantech_data *etd)
> +static int elantech_set_properties(struct elantech_data *etd, struct 
> +psmouse *psmouse)

Isn't the line too long for this one ?

>  {
>   /* This represents the version of IC body. */
>   int ver = (etd->fw_version & 0x0f) >> 16; @@ -1593,14 +1597,14 
> @@ static int elantech_set_properties(struct elantech_data *etd)
>   case 5:
>   etd->hw_version = 3;
>   break;
> - case 6 ... 14:
> + case 6 ... 15:
>   etd->hw_version = 4;
>   break;
>   default:
>   return -1;
>   }
>   }

Remove this tab on a line alone you added below ..
[KT]:OK

> -
> + 
>   /* decide which send_cmd we're gonna use early */
>   etd->send_cmd = etd->hw_version >= 3 ? elantech_send_cmd :
>  synaptics_send_cmd;
I would propose to place the lines below (up to the end of function
elantech_set_properties) in elantech_init instead of
elantech_set_properties. 
[KT]: OK, it's better to move my part to elantech_init and skip psmouse
parameter.


> @@ -1634,6 +1638,22 @@ static int elantech_set_properties(struct
elantech_data *etd)
>   /* Enable real hardware resolution on hw_version 3 ? */
>   etd->set_hw_resolution = !dmi_check_system(no_hw_res_dmi_table);
>  
> + /*if ver == 15 and info_pattern == 0x01, it is ELAN new pattern
> +  *which support a command for extend IC_body/FW_Version reading
> +  */
> + etd->info_pattern = etd->fw_version & 0xFF;
> + if (ver == 0x0F && etd->info_pattern == 0x01) {
I really appreciate the message that is given in the comment. 
Why would you store the lowest byte of fw_version once more as an int .. and
use it only once at exactly the next line ..

Alternatives I see are: 
1) Merge the two lines (so there is no info_pattern anymore)
2) Use a local variable info_pattern to store it intermediately
3) Have some macro that basically takes the lowest byte (I can't immediately
find a good example for this one)
[KT] I will use a local UCHAR info_pattern for it .

> + if (etd->send_cmd(psmouse, ETP_ICBODY_QUERY, etd->icbody)) {
> + psmouse_err(psmouse, "failed to query icbody
data\n");
> + return -1;
> + }
> + psmouse_info(psmouse,
> + "Elan ICBODY query result %02x, %02x,
%02x\n",
> + etd->icbody[0], etd->icbody[1],
etd->icbody[2]);
> + etd->fw_version &= 0x00;
> + etd->fw_version |= etd->icbody[2];

Brr, this throws away information. Is icbody2 really meant to replace bottom
byte of fw_version ? I see no benefit of doing this ! I would propose to
drop the above 2 lines that alter fw_version.
[KT] icbody2 will be the real fw version in upcoming HW, it's only useful
when we really need to check it.
>From vendor's view, original etd->fw_version is not fw-versoin. It contain
many information in i

RE: [PATCH] Input: elan_i2c - Prevent breaking of FW updating from unexpected signal

2017-05-24 Thread
Hi Dmitry

-Original Message-
From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com] 
Sent: Wednesday, May 24, 2017 5:14 AM
To: KT Liao
Cc: linux-kernel@vger.kernel.org; linux-in...@vger.kernel.org;
phoe...@emc.com.tw
Subject: Re: [PATCH] Input: elan_i2c - Prevent breaking of FW updating from
unexpected signal

Hi KT,

On Tue, May 23, 2017 at 10:32:02PM +0800, KT Liao wrote:
> Use wait_for_completion_timeout to prevent breaking of FW updating from
unexpected signal
> 
> Signed-off-by: KT Liao 
> ---
>  drivers/input/mouse/elan_i2c_i2c.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/mouse/elan_i2c_i2c.c
b/drivers/input/mouse/elan_i2c_i2c.c
> index 3be75c6e..34356c7 100644
> --- a/drivers/input/mouse/elan_i2c_i2c.c
> +++ b/drivers/input/mouse/elan_i2c_i2c.c
> @@ -619,7 +619,7 @@ static int elan_i2c_finish_fw_update(struct i2c_client
*client,
>  
>   error = elan_i2c_write_cmd(client, ETP_I2C_STAND_CMD,
ETP_I2C_RESET);
>   if (!error)
> - ret = wait_for_completion_interruptible_timeout(completion,
> + ret = wait_for_completion_timeout(completion,
>
msecs_to_jiffies(300));
>   disable_irq(client->irq);
>  
> -- 
> 2.7.4
> 

wait_for_completion_timeout(), unlike
wait_for_completion_interruptible_timeout() returns unsigned long, so
the code below checking if 'ret' is negative, is no longer needed. How
about the version of the patch below?
[KT] : Thanks, it's ok. We have tested it on some Chrome system.
-- 
Dmitry


Input: elan_i2c - ignore signals when finishing updating firmware

From: KT Liao 

Use wait_for_completion_timeout() instead of
wait_for_completion_interruptible_timeout() to avoid stray signals ruining
firmware update. Our timeout is only 300 msec so we are fine simply letting
it expire in case device misbehaves.

Signed-off-by: KT Liao 
Patchwork-Id: 9742857
Signed-off-by: Dmitry Torokhov 
---
 drivers/input/mouse/elan_i2c_i2c.c |   21 -
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/input/mouse/elan_i2c_i2c.c
b/drivers/input/mouse/elan_i2c_i2c.c
index 3be75c6e8090..278832047075 100644
--- a/drivers/input/mouse/elan_i2c_i2c.c
+++ b/drivers/input/mouse/elan_i2c_i2c.c
@@ -609,7 +609,6 @@ static int elan_i2c_finish_fw_update(struct i2c_client
*client,
 struct completion *completion)
 {
struct device *dev = &client->dev;
-   long ret;
int error;
int len;
u8 buffer[ETP_I2C_INF_LENGTH];
@@ -618,23 +617,19 @@ static int elan_i2c_finish_fw_update(struct i2c_client
*client,
enable_irq(client->irq);
 
error = elan_i2c_write_cmd(client, ETP_I2C_STAND_CMD,
ETP_I2C_RESET);
-   if (!error)
-   ret = wait_for_completion_interruptible_timeout(completion,
-   msecs_to_jiffies(300
));
-   disable_irq(client->irq);
-
if (error) {
dev_err(dev, "device reset failed: %d\n", error);
-   return error;
-   } else if (ret == 0) {
+   } else if (!wait_for_completion_timeout(completion,
+   msecs_to_jiffies(300))) {
dev_err(dev, "timeout waiting for device reset\n");
-   return -ETIMEDOUT;
-   } else if (ret < 0) {
-   error = ret;
-   dev_err(dev, "error waiting for device reset: %d\n", error);
-   return error;
+   error = -ETIMEDOUT;
}
 
+   disable_irq(client->irq);
+
+   if (error)
+   return error;
+
len = i2c_master_recv(client, buffer, ETP_I2C_INF_LENGTH);
if (len != ETP_I2C_INF_LENGTH) {
error = len < 0 ? len : -EIO;



RE: [PATCH] Input: elan_i2c - add ASUS EeeBook X205TA special touchpad fw

2017-03-07 Thread
Hi Matjaz,

-Original Message-
From: Matjaž Hegedič [mailto:matjaz.hege...@gmail.com] 
Sent: Tuesday, March 07, 2017 6:52 PM
To: 廖崇榮; 'Dmitry Torokhov'
Cc: linux-in...@vger.kernel.org; linux-kernel@vger.kernel.org; 黃世鵬 經理; 
miller_w...@emc.com.tw
Subject: Re: [PATCH] Input: elan_i2c - add ASUS EeeBook X205TA special touchpad 
fw

Hi Dmitry, KT!

On 2017-03-07 08:05, 廖崇榮 wrote:
> Hi Dmitry
>
> -Original Message-
> From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com]
> Sent: Tuesday, March 07, 2017 3:55 AM
> To: KT Liao
> Cc: linux-in...@vger.kernel.org; linux-kernel@vger.kernel.org; Matjaz 
> Hegedic
> Subject: Re: [PATCH] Input: elan_i2c - add ASUS EeeBook X205TA special 
> touchpad fw
>
> On Sun, Mar 05, 2017 at 03:13:02AM +0100, Matjaz Hegedic wrote:
>> EeeBook X205TA is yet another ASUS device with a special touchpad 
>> firmware that needs to be accounted for during initialization, or 
>> else the touchpad will go into an invalid state upon suspend/resume.
>> Adding the appropriate ic_type and product_id check fixes the problem.
>
> KT, does this look reasonable? Are there more ASUS models that need 
> such handling?
> [KT] : I just discuss it with FW team.
> We can't confirm it right now because it's an old product. And the 
> solution focus on power-on issue, not suspend/resume.
> I will let you know once we figure it out.
>
> Our FW has modified, the issue should not happen on new models.
>
> ThanksKT

As it is now, the touchpad will stop working upon resume, returning an invalid 
id (and requires a cumbersome workaround of reloading the module). As the 
touchpad FW is opaque to me, the only way I could resolve the bug was through 
trial-and-error. Including the touchpad in the 'special fw' resolves the bug 
and the touchpad resumes without issue.

Now, even if the function is indeed used to resolve a different issue on other 
ASUS touchpads, I would argue that this is the most pragmatic way of resolving 
the problem on X205TA, X205TAW and F205TA (and possibly also X200HA & X206HA, 
though I don't have those to test).

It shouldn't affect any other models or touchpad products.

Thanks!

I agree your opinion.
The special handle simply changes the sequence of commands and adds delay 
cycle. 
It is harmless to general Elan products but rescue control flaws in some FW.

Thanks  KT
>>
>> Signed-off-by: Matjaz Hegedic 
>> ---
>>  drivers/input/mouse/elan_i2c_core.c | 22 --
>>  1 file changed, 12 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/input/mouse/elan_i2c_core.c
>> b/drivers/input/mouse/elan_i2c_core.c
>> index 2c7d287..dde3ad7 100644
>> --- a/drivers/input/mouse/elan_i2c_core.c
>> +++ b/drivers/input/mouse/elan_i2c_core.c
>> @@ -218,17 +218,19 @@ static int elan_query_product(struct 
>> elan_tp_data *data)
>>
>>  static int elan_check_ASUS_special_fw(struct elan_tp_data *data)  {
>> -if (data->ic_type != 0x0E)
>> -return false;
>> -
>> -switch (data->product_id) {
>> -case 0x05 ... 0x07:
>> -case 0x09:
>> -case 0x13:
>> -return true;
>> -default:
>> -return false;
>> +if (data->ic_type == 0x0E) {
>> +switch (data->product_id) {
>> +case 0x05 ... 0x07:
>> +case 0x09:
>> +case 0x13:
>> +return true;
>> +}
>>  }
>> +/* ASUS EeeBook X205TA */
>> +else if (data->ic_type == 0x8 && data->product_id == 0x26)
>> +return true;
>> +
>> +return false;
>>  }
>>
>>  static int __elan_initialize(struct elan_tp_data *data)
>> --
>> 2.7.4
>>
>
> Thanks.
>



RE: [PATCH] Input: elan_i2c - add ASUS EeeBook X205TA special touchpad fw

2017-03-06 Thread
Hi Dmitry

-Original Message-
From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com] 
Sent: Tuesday, March 07, 2017 3:55 AM
To: KT Liao
Cc: linux-in...@vger.kernel.org; linux-kernel@vger.kernel.org; Matjaz
Hegedic
Subject: Re: [PATCH] Input: elan_i2c - add ASUS EeeBook X205TA special
touchpad fw

On Sun, Mar 05, 2017 at 03:13:02AM +0100, Matjaz Hegedic wrote:
> EeeBook X205TA is yet another ASUS device with a special touchpad 
> firmware that needs to be accounted for during initialization, or else 
> the touchpad will go into an invalid state upon suspend/resume.
> Adding the appropriate ic_type and product_id check fixes the problem.

KT, does this look reasonable? Are there more ASUS models that need such
handling?
[KT] : I just discuss it with FW team. 
We can't confirm it right now because it's an old product. And the solution
focus on power-on issue, not suspend/resume.
I will let you know once we figure it out.

Our FW has modified, the issue should not happen on new models.

Thanks  KT
> 
> Signed-off-by: Matjaz Hegedic 
> ---
>  drivers/input/mouse/elan_i2c_core.c | 22 --
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/input/mouse/elan_i2c_core.c 
> b/drivers/input/mouse/elan_i2c_core.c
> index 2c7d287..dde3ad7 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -218,17 +218,19 @@ static int elan_query_product(struct 
> elan_tp_data *data)
>  
>  static int elan_check_ASUS_special_fw(struct elan_tp_data *data)  {
> - if (data->ic_type != 0x0E)
> - return false;
> -
> - switch (data->product_id) {
> - case 0x05 ... 0x07:
> - case 0x09:
> - case 0x13:
> - return true;
> - default:
> - return false;
> + if (data->ic_type == 0x0E) {
> + switch (data->product_id) {
> + case 0x05 ... 0x07:
> + case 0x09:
> + case 0x13:
> + return true;
> + }
>   }
> + /* ASUS EeeBook X205TA */
> + else if (data->ic_type == 0x8 && data->product_id == 0x26)
> + return true;
> +
> + return false;
>  }
>  
>  static int __elan_initialize(struct elan_tp_data *data)
> --
> 2.7.4
> 

Thanks.

-- 
Dmitry



RE: [PATCH] Input: elantech - Add a special mode for a specific FW The touchapd which sample ver is 0x74 and hw_version is 0x03 have a fw bug which will cause crush sometimes, I add some work-around f

2016-12-04 Thread
Hi Ulrik, Dmitry,

Thanks your comment for the patch.
Please see my reply I the below.

-Original Message-
From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com] 
Sent: Sunday, December 04, 2016 2:48 AM
To: ulrik.debie...@e2big.org
Cc: KT Liao; linux-kernel@vger.kernel.org; linux-in...@vger.kernel.org;
phoe...@emc.com.tw
Subject: Re: [PATCH] Input: elantech - Add a special mode for a specific FW
The touchapd which sample ver is 0x74 and hw_version is 0x03 have a fw bug
which will cause crush sometimes, I add some work-around for it and our
customer ask us to upstream the patch Sig

On Fri, Dec 02, 2016 at 11:05:29PM +0100, ulrik.debie...@e2big.org wrote:
> Hi,
> 
> Thank you for the patch, see below my feedback on your patch.
> Can you provide the contents of fw_verison, capabilities and samples ?
> 
> It this fw bug present on multiple laptops ?
> 
>  
> 
> On Fri, Dec 02, 2016 at 01:59:17PM +0800, KT Liao wrote:
> > Date:   Fri,  2 Dec 2016 13:59:17 +0800
> > From: KT Liao 
> > To: linux-kernel@vger.kernel.org, linux-in...@vger.kernel.org,  
> > dmitry.torok...@gmail.com
> > Cc: phoe...@emc.com.tw, kt.l...@emc.com.tw
> > Subject: [PATCH] Input: elantech - Add a special mode for a specific 
> > FW The  touchapd which sample ver is 0x74 and hw_version is 0x03 
> > have a fw bug  which will cause crush sometimes, I add some 
> > work-around for it and our  customer ask us to upstream the patch 
> > Signed-off-by: KT Liao  
> 
> It seems that the newlines got lost when you used git-send-email. The 
> subject should be a oneliner, the remaining part should go to the mail
body.

I think KT forgets to add an empty line between patch subject and body when
committing to their tree.

> 
> > X-Mailer: git-send-email 2.7.4
> > X-Mailing-List: linux-in...@vger.kernel.org
> > 
> > ---
> >  drivers/input/mouse/elantech.c | 152 
> > +++--
> >  1 file changed, 131 insertions(+), 21 deletions(-)
> > 
> > diff --git a/drivers/input/mouse/elantech.c 
> > b/drivers/input/mouse/elantech.c index db7d1d6..acfe7f2 100644
> > --- a/drivers/input/mouse/elantech.c
> > +++ b/drivers/input/mouse/elantech.c
> > @@ -539,6 +539,30 @@ static void elantech_report_absolute_v3(struct
psmouse *psmouse,
> > input_sync(dev);
> >  }
> >  
> > +static psmouse_ret_t elantech_report_relative_v3(struct psmouse 
> > +*psmouse) {
> > +   struct input_dev *dev = psmouse->dev;
> > +   unsigned char *packet = psmouse->packet;
> > +   int rel_x = 0, rel_y = 0;
> > +
> > +   if (psmouse->pktcnt < psmouse->pktsize)
> > +   return PSMOUSE_GOOD_DATA;
> 
> This is a duplicate of elantech_process_byte and you skipped the 
> elantech_packet_dump feature of elantech_process_byte. I think it 
> would be better to let elantech_process_byte call this 
> elantech_report_relative_v3 just like all the other reportings.
> Is it required to also disable the elantech_packet_check_v3 ? 
> 
> Can you document the typical packet format for
> elantech_report_relative_v3 ? Something similar to
elantech_report_trackpoint ?
> 
> 
> > +
> > +   input_report_rel(dev, REL_WHEEL, -(signed char)packet[3]);
> > +
> > +   rel_x = (int) packet[1] - (int) ((packet[0] << 4) & 0x100);
> > +   rel_y = (int) ((packet[0] << 3) & 0x100) - (int) packet[2];
> > +
> > +   input_report_key(dev, BTN_LEFT,packet[0]   & 1);
> > +   input_report_key(dev, BTN_RIGHT,  (packet[0] >> 1) & 1);
> > +   input_report_rel(dev, REL_X, rel_x);
> > +   input_report_rel(dev, REL_Y, rel_y);
> > +
> > +   input_sync(dev);
> > +
> > +   return PSMOUSE_FULL_PACKET;
> > +}
> > +
> >  static void elantech_input_sync_v4(struct psmouse *psmouse)  {
> > struct input_dev *dev = psmouse->dev; @@ -696,14 +720,14 @@ static 
> > int elantech_packet_check_v1(struct psmouse *psmouse)
> >  
> >  static int elantech_debounce_check_v2(struct psmouse *psmouse)  {
> > -/*
> > - * When we encounter packet that matches this exactly, it means
the
> > - * hardware is in debounce status. Just ignore the whole
packet.
> > - */
> > -const u8 debounce_packet[] = { 0x84, 0xff, 0xff, 0x02, 0xff,
0xff };
> > -unsigned char *packet = psmouse->packet;
> > -
> > -return !memcmp(packet, debounce_packet,
sizeof(debounce_packet));
> > +   /*
> > +* When we encounter packet that matches this exactly, it means the
> > +* hardware is in debounce status. Just ignore the whole packet.
> > +*/
> > +   const u8 debounce_packet[] = { 0x84, 0xff, 0xff, 0x02, 0xff, 0xff };
> > +   unsigned char *packet = psmouse->packet;
> > +
> > +   return !memcmp(packet, debounce_packet, sizeof(debounce_packet));
> >  }
> 
> Confirmed, the lines of elantech_debounce_check_v2 do not start with 
> tab but spaces, but preferably this will be part of a separate commit.

Yes please.

> 
> >  
> >  static int elantech_packet_check_v2(struct psmouse *psmouse) @@ 
> > -995,6 +1019,29 @@ static int elantech_set_absolute_mode(struct psmouse
*psmous

RE: [PATCH] Input: elan_i2c - Add new ic and modify some functions for new IC infomation Signed-off-by: KT Liao

2016-11-18 Thread
Hi Dmitry

-Original Message-
From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com] 
Sent: Friday, November 18, 2016 1:15 AM
To: KT Liao
Cc: linux-kernel@vger.kernel.org; linux-in...@vger.kernel.org; 
phoe...@emc.com.tw; kt.l...@emc.com.tw
Subject: Re: [PATCH] Input: elan_i2c - Add new ic and modify some functions for 
new IC infomation Signed-off-by: KT Liao 

Hi KT,

On Thu, Nov 17, 2016 at 07:47:43PM +0800, KT Liao wrote:
> ---
>  drivers/input/mouse/elan_i2c.h   |  6 ++--
>  drivers/input/mouse/elan_i2c_core.c  | 46 ++
>  drivers/input/mouse/elan_i2c_i2c.c   | 63 
> ++--
>  drivers/input/mouse/elan_i2c_smbus.c | 11 +--
>  4 files changed, 99 insertions(+), 27 deletions(-)  mode change 
> 100644 => 100755 drivers/input/mouse/elan_i2c.h  mode change 100644 => 
> 100755 drivers/input/mouse/elan_i2c_core.c
>  mode change 100644 => 100755 drivers/input/mouse/elan_i2c_i2c.c
>  mode change 100644 => 100755 drivers/input/mouse/elan_i2c_smbus.c

Why are you changing mode on the files?
[KT]:Sorry, it's my fault to change file mode. I will fix it in next upstream.

> 
> diff --git a/drivers/input/mouse/elan_i2c.h 
> b/drivers/input/mouse/elan_i2c.h old mode 100644 new mode 100755 index 
> c0ec261..a90df14
> --- a/drivers/input/mouse/elan_i2c.h
> +++ b/drivers/input/mouse/elan_i2c.h
> @@ -56,9 +56,10 @@ struct elan_transport_ops {
>   int (*get_baseline_data)(struct i2c_client *client,
>bool max_baseliune, u8 *value);
>  
> - int (*get_version)(struct i2c_client *client, bool iap, u8 *version);
> + int (*get_version)(struct i2c_client *client,
> +bool iap, u8 *version, u8 pattern);
>   int (*get_sm_version)(struct i2c_client *client,
> -   u8* ic_type, u8 *version);
> +   u16 *ic_type, u8 *version, u8 pattern);
>   int (*get_checksum)(struct i2c_client *client, bool iap, u16 *csum);
>   int (*get_product_id)(struct i2c_client *client, u16 *id);
>  
> @@ -82,6 +83,7 @@ struct elan_transport_ops {
>   int (*get_report)(struct i2c_client *client, u8 *report);
>   int (*get_pressure_adjustment)(struct i2c_client *client,
>  int *adjustment);
> + int (*get_pattern)(struct i2c_client *client, u8 *pattern);
>  };
>  
>  extern const struct elan_transport_ops elan_smbus_ops, elan_i2c_ops; 
> diff --git a/drivers/input/mouse/elan_i2c_core.c 
> b/drivers/input/mouse/elan_i2c_core.c
> old mode 100644
> new mode 100755
> index d15b338..bb0c832
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -5,7 +5,7 @@
>   *
>   * Author: 林政維 (Duson Lin) 
>   * Author: KT Liao 
> - * Version: 1.6.2
> + * Version: 1.6.3
>   *
>   * Based on cyapa driver:
>   * copyright (c) 2011-2012 Cypress Semiconductor, Inc.
> @@ -41,7 +41,7 @@
>  #include "elan_i2c.h"
>  
>  #define DRIVER_NAME  "elan_i2c"
> -#define ELAN_DRIVER_VERSION  "1.6.2"
> +#define ELAN_DRIVER_VERSION  "1.6.3"
>  #define ELAN_VENDOR_ID   0x04f3
>  #define ETP_MAX_PRESSURE 255
>  #define ETP_FWIDTH_REDUCE90
> @@ -78,6 +78,7 @@ struct elan_tp_data {
>   unsigned intx_res;
>   unsigned inty_res;
>  
> + u8  pattern;
>   u16 product_id;
>   u8  fw_version;
>   u8  sm_version;
> @@ -85,7 +86,7 @@ struct elan_tp_data {
>   u16 fw_checksum;
>   int pressure_adjustment;
>   u8  mode;
> - u8  ic_type;
> + u16 ic_type;
>   u16 fw_validpage_count;
>   u16 fw_signature_address;
>  
> @@ -96,10 +97,10 @@ struct elan_tp_data {
>   boolbaseline_ready;
>  };
>  
> -static int elan_get_fwinfo(u8 iap_version, u16 *validpage_count,
> +static int elan_get_fwinfo(u16 ic_type, u16 *validpage_count,
>  u16 *signature_address)
>  {
> - switch (iap_version) {
> + switch (ic_type) {
>   case 0x00:
>   case 0x06:
>   case 0x08:
> @@ -119,6 +120,9 @@ static int elan_get_fwinfo(u8 iap_version, u16 
> *validpage_count,
>   case 0x0E:
>   *validpage_count = 640;
>   break;
> + case 0x10:
> + *validpage_count = 1024;
> + break;
>   default:
>   /* unknown ic type clear value */
>   *validpage_count = 0;
> @@ -204,12 +208,17 @@ static int elan_query_product(struct 
> elan_tp_data *data)  {
>   int error;
>  
> + error = data->ops->get_pattern(data->client, &data->pattern);
> + if (error)
> + return error;
> +
>   error = data->ops->get_product_id(data->client, &data->product_id);
>   if (error)
>   return error;

RE: [PATCH] Input: /input/mouse/elan_i2c_core.c Fix some Asus touchapod which casue TP no funciton sometimes, the patch detect some specific touchpad and run a special initialize

2016-07-12 Thread
Hi Dmitry,

-Original Message-
From: 'Dmitry Torokhov' [mailto:dmitry.torok...@gmail.com] 
Sent: Tuesday, July 12, 2016 1:03 AM
To: 廖崇榮
Cc: 'KT Liao'; linux-kernel@vger.kernel.org; linux-in...@vger.kernel.org; 
phoe...@emc.com.tw
Subject: Re: [PATCH] Input: /input/mouse/elan_i2c_core.c Fix some Asus 
touchapod which casue TP no funciton sometimes, the patch detect some specific 
touchpad and run a special initialize

On Mon, Jul 11, 2016 at 08:40:58PM +0800, 廖崇榮 wrote:
> > +
> > +   error = data->ops->get_sm_version(client, &data->ic_type,
> > + &data->sm_version);
> > +   if (error)
> > +   return false;
> 
> That means we'd be fetching product ID and IC type twice when initializing 
> the device. Can we come with a way to do it once?
> [KT]:Because the elan_query_device_info() is behind the elan_initialize(). 
> That's why I fetching product ID and IC type in the elan_initialize()
> I will discuss with FW team and then execute elan_query_device_info() 
> first to get product_id and ic_type. 

We might need to split fetching product ID and IC type form the rest of the 
device info.
[KT] : ok, I will extract "product ID" and "IC type" to another query-device 
function. and it won't change original flow too much.

Thanks.

--
Dmitry



RE: [PATCH] Input: /input/mouse/elan_i2c_core.c Fix some Asus touchapod which casue TP no funciton sometimes, the patch detect some specific touchpad and run a special initialize

2016-07-11 Thread
Hi Dmitry,

-Original Message-
From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com] 
Sent: Saturday, July 09, 2016 8:25 AM
To: KT Liao
Cc: linux-kernel@vger.kernel.org; linux-in...@vger.kernel.org; 
phoe...@emc.com.tw; kt.l...@emc.com.tw
Subject: Re: [PATCH] Input: /input/mouse/elan_i2c_core.c Fix some Asus 
touchapod which casue TP no funciton sometimes, the patch detect some specific 
touchpad and run a special initialize

Hi KT,

On Fri, Jul 08, 2016 at 08:12:09PM +0800, KT Liao wrote:
> Signed-off-by: KT Liao 

Please make sure you add a blan line between change subject and the rest of 
description. Then git send-email will compose your email properly instead of 
lumping everything into one long line subject.
[KT] :Sorry, I will fix it in the later upstream
> ---
>  drivers/input/mouse/elan_i2c_core.c | 81 
> +
>  1 file changed, 65 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/input/mouse/elan_i2c_core.c 
> b/drivers/input/mouse/elan_i2c_core.c
> index 2f58985..1c200fb 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -3,8 +3,8 @@
>   *
>   * Copyright (c) 2013 ELAN Microelectronics Corp.
>   *
> - * Author: 林政維 (Duson Lin) 
> - * Version: 1.6.0
> + * Author: KT Liao 
> + * Version: 1.6.2
>   *
>   * Based on cyapa driver:
>   * copyright (c) 2011-2012 Cypress Semiconductor, Inc.
> @@ -40,7 +40,7 @@
>  #include "elan_i2c.h"
>  
>  #define DRIVER_NAME  "elan_i2c"
> -#define ELAN_DRIVER_VERSION  "1.6.1"
> +#define ELAN_DRIVER_VERSION  "1.6.2"
>  #define ELAN_VENDOR_ID   0x04f3
>  #define ETP_MAX_PRESSURE 255
>  #define ETP_FWIDTH_REDUCE90
> @@ -95,6 +95,8 @@ struct elan_tp_data {
>   boolbaseline_ready;
>  };
>  
> +static int check_ASUS_special_fw(struct elan_tp_data *data);
> +
>  static int elan_get_fwinfo(u8 iap_version, u16 *validpage_count,
>  u16 *signature_address)
>  {
> @@ -210,21 +212,40 @@ static int __elan_initialize(struct elan_tp_data *data)
>   return error;
>   }
>  
> - data->mode |= ETP_ENABLE_ABS;
> - error = data->ops->set_mode(client, data->mode);
> - if (error) {
> - dev_err(&client->dev,
> - "failed to switch to absolute mode: %d\n", error);
> - return error;
> - }
> + /* If it's the special FW, it need a different flow for mode change.*/
> + if (check_ASUS_special_fw(data)) {
> + error = data->ops->sleep_control(client, false);
> + if (error) {
> + dev_err(&client->dev,
> + "failed to wake device up: %d\n", error);
> + return error;
> + }
>  
> - error = data->ops->sleep_control(client, false);
> - if (error) {
> - dev_err(&client->dev,
> - "failed to wake device up: %d\n", error);
> - return error;
> - }
> + msleep(200);
>  
> + data->mode |= ETP_ENABLE_ABS;
> + error = data->ops->set_mode(client, data->mode);
> + if (error) {
> + dev_err(&client->dev,
> + "failed to switch to absolute mode: %d\n", 
> error);
> + return error;
> + }
> + } else {
> + data->mode |= ETP_ENABLE_ABS;
> + error = data->ops->set_mode(client, data->mode);
> + if (error) {
> + dev_err(&client->dev,
> + "failed to switch to absolute mode: %d\n", 
> error);
> + return error;
> + }
> +
> + error = data->ops->sleep_control(client, false);
> + if (error) {
> + dev_err(&client->dev,
> + "failed to wake device up: %d\n", error);
> + return error;
> + }
> + }
>   return 0;
>  }
>  
> @@ -757,6 +778,34 @@ out:
>   return retval;
>  }
>  
> +static int check_ASUS_special_fw(struct elan_tp_data *data) {
> + struct i2c_client *client = data->client;
> + int error;
> +
> + error = data->ops->get_product_id(client, &data->product_id);
> + if (error)
> + return false;
> +
> + error = data->ops->get_sm_version(client, &data->ic_type,
> +   &data->sm_version);
> + if (error)
> + return false;

That means we'd be fetching product ID and IC type twice when initializing the 
device. Can we come with a way to do it once?
[KT]:Because the elan_query_device_info() is behind the elan_initialize(). 
That's why I fetching product ID and IC type in the elan_initialize()
I will discuss with FW team and then execute elan_query_device_info() first 
to get product_id and ic_type. 
> +
> + if (data->ic_type == 0x0E) {

Maybe

if (dat->ic_type != 0x0e)
   

RE: [PATCH] Input: elan_i2c - +200 ms delay before setting to ABS mode

2016-06-27 Thread
Hi Daniel, Chris

-Original Message-
From: Daniel Drake [mailto:dr...@endlessm.com] 
Sent: Tuesday, June 21, 2016 10:42 PM
To: 廖崇榮
Cc: Dmitry Torokhov; Chris Chiu; Charlie Mooney; Michele Curti; Krzysztof 
Kozlowski; Benson Leung; linux-in...@vger.kernel.org; Linux Kernel; Linux 
Upstreaming Team; 黃世鵬 經理
Subject: Re: [PATCH] Input: elan_i2c - +200 ms delay before setting to ABS mode

On Tue, Jun 21, 2016 at 6:40 AM, 廖崇榮  wrote:
> KT, is this feasible?
> [KT] After internal discussion, we don't agree this patch.
> It's a work-around to fix firmware bug for specific touchpad and 
> not tested by other device.

For better or worse, Linux often takes on the responsibility of working around 
firmware bugs. This is a real issue that affects multiple Asus laptops; you'll 
have no touchpad input upon reboot from any OS that drives the touchpad in 
"generic hid" mode (e.g. older version of Linux, some embedded OS, etc).

If the sleep is really that controversial, we could make it specific to the 
ELAN1000 model that is the one in question here?

[KT] : I list all ASUS models with firmware issue which cause TP no function in 
Endless OOB , please focuses on below types only
 Ic_type : 14
 Product_id : 0x14, 0x15, 0x16, 0x25, 0x18, 0x29, 0x2c, 0x31, 0x32, 0x35

 You can add if branch in "__elan_initialize", something like that

 if(data->ic_ytpe == 14 && 
(data->product_id == 0x14 || data->product_id == 0x15 || 
data->product_id == 0x16 || 
data->product_id == 0x25 || data->product_id == 0x18 || 
data->product_id == 0x29 || 
data->product_id == 0x2c || data->product_id == 0x31 || 
data->product_id == 0x32 || data->product_id == 0x35))
 { //flow for Endless}
 Else {//original flow}
 




RE: [PATCH] Input: elan_i2c - +200 ms delay before setting to ABS mode

2016-06-22 Thread
-Original Message-
From: Daniel Drake [mailto:dr...@endlessm.com] 
Sent: Tuesday, June 21, 2016 10:42 PM
To: 廖崇榮
Cc: Dmitry Torokhov; Chris Chiu; Charlie Mooney; Michele Curti; Krzysztof 
Kozlowski; Benson Leung; linux-in...@vger.kernel.org; Linux Kernel; Linux 
Upstreaming Team; 黃世鵬 經理
Subject: Re: [PATCH] Input: elan_i2c - +200 ms delay before setting to ABS mode

On Tue, Jun 21, 2016 at 6:40 AM, 廖崇榮  wrote:
> KT, is this feasible?
> [KT] After internal discussion, we don't agree this patch.
> It's a work-around to fix firmware bug for specific touchpad and 
> not tested by other device.

For better or worse, Linux often takes on the responsibility of working around 
firmware bugs. This is a real issue that affects multiple Asus laptops; you'll 
have no touchpad input upon reboot from any OS that drives the touchpad in 
"generic hid" mode (e.g. older version of Linux, some embedded OS, etc).

If the sleep is really that controversial, we could make it specific to the 
ELAN1000 model that is the one in question here?

[KT]:We can consider to control specific module, our FW engineer is checking it 
and confirm which part number for ASUS should adopt work-around.
we will reply you once we confirm.

Thanks
Daniel




RE: [PATCH] Input: elan_i2c - +200 ms delay before setting to ABS mode

2016-06-21 Thread
Hi Dmitry,

-Original Message-
From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com] 
Sent: Tuesday, June 21, 2016 1:43 AM
To: Chris Chiu; kt.l...@emc.com.tw
Cc: Charlie Mooney; Michele Curti; Krzysztof Kozlowski; Benson Leung;
linux-in...@vger.kernel.org; linux-kernel@vger.kernel.org;
li...@endlessm.com
Subject: Re: [PATCH] Input: elan_i2c - +200 ms delay before setting to ABS
mode

On Tue, Jun 07, 2016 at 09:34:09PM +0800, Chris Chiu wrote:
> When performing a warm reboot from a system which does not correctly 
> support ELAN I2C touchpads, the touchpad will sometimes enter standard 
> mouse mode, cursor then never respond to touchpad event, and making 
> the driver discard the HID reports and flood dmesg with following 
> error messages.
> "elan_i2c i2c-ELAN1000:00: invalid report id data (1)"
> 
> This change is from ELAN's correction. It needs 200ms delay before
> set_mode() so that the mode setting will correctly take effect.

KT, is this feasible?
[KT] After internal discussion, we don't agree this patch. 
It's a work-around to fix firmware bug for specific touchpad and not
tested by other device.

> 
> Signed-off-by: Chris Chiu 
> ---
>  drivers/input/mouse/elan_i2c_core.c | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/input/mouse/elan_i2c_core.c 
> b/drivers/input/mouse/elan_i2c_core.c
> index 2f58985..95080f9 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -210,18 +210,20 @@ static int __elan_initialize(struct elan_tp_data
*data)
>   return error;
>   }
>  
> - data->mode |= ETP_ENABLE_ABS;
> - error = data->ops->set_mode(client, data->mode);
> + error = data->ops->sleep_control(client, false);
>   if (error) {
>   dev_err(&client->dev,
> - "failed to switch to absolute mode: %d\n", error);
> + "failed to wake device up: %d\n", error);
>   return error;
>   }
>  
> - error = data->ops->sleep_control(client, false);
> + msleep(200);
> +
> + data->mode |= ETP_ENABLE_ABS;
> + error = data->ops->set_mode(client, data->mode);
>   if (error) {
>   dev_err(&client->dev,
> - "failed to wake device up: %d\n", error);
> + "failed to switch to absolute mode: %d\n", error);
>   return error;
>   }
>  
> --
> 2.1.4
> 

Thanks.

-- 
Dmitry



RE: [PATCH] Input: elan_i2c - +200 ms delay before setting to ABS mode

2016-06-20 Thread
Hi Dmitry,

The modification from Chris is a special case.
Because the Touchpad FW is a little different from normal one, It cause
problem in Asus's OBE test. 

That's why Elan's driver use work-around to solve the problem. It's not
tested by other touchpad.

Let me discuss with internal FW team to confirm the harmless of the patch.

B.R  KT
-Original Message-
From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com] 
Sent: Tuesday, June 21, 2016 1:43 AM
To: Chris Chiu; kt.l...@emc.com.tw
Cc: Charlie Mooney; Michele Curti; Krzysztof Kozlowski; Benson Leung;
linux-in...@vger.kernel.org; linux-kernel@vger.kernel.org;
li...@endlessm.com
Subject: Re: [PATCH] Input: elan_i2c - +200 ms delay before setting to ABS
mode

On Tue, Jun 07, 2016 at 09:34:09PM +0800, Chris Chiu wrote:
> When performing a warm reboot from a system which does not correctly 
> support ELAN I2C touchpads, the touchpad will sometimes enter standard 
> mouse mode, cursor then never respond to touchpad event, and making 
> the driver discard the HID reports and flood dmesg with following 
> error messages.
> "elan_i2c i2c-ELAN1000:00: invalid report id data (1)"
> 
> This change is from ELAN's correction. It needs 200ms delay before
> set_mode() so that the mode setting will correctly take effect.

KT, is this feasible?

> 
> Signed-off-by: Chris Chiu 
> ---
>  drivers/input/mouse/elan_i2c_core.c | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/input/mouse/elan_i2c_core.c 
> b/drivers/input/mouse/elan_i2c_core.c
> index 2f58985..95080f9 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -210,18 +210,20 @@ static int __elan_initialize(struct elan_tp_data
*data)
>   return error;
>   }
>  
> - data->mode |= ETP_ENABLE_ABS;
> - error = data->ops->set_mode(client, data->mode);
> + error = data->ops->sleep_control(client, false);
>   if (error) {
>   dev_err(&client->dev,
> - "failed to switch to absolute mode: %d\n", error);
> + "failed to wake device up: %d\n", error);
>   return error;
>   }
>  
> - error = data->ops->sleep_control(client, false);
> + msleep(200);
> +
> + data->mode |= ETP_ENABLE_ABS;
> + error = data->ops->set_mode(client, data->mode);
>   if (error) {
>   dev_err(&client->dev,
> - "failed to wake device up: %d\n", error);
> + "failed to switch to absolute mode: %d\n", error);
>   return error;
>   }
>  
> --
> 2.1.4
> 

Thanks.

-- 
Dmitry



RE: [PATCH] Input: /input/input-mt.c Emit BTN_TOO_FINGER in function input_mt_report_pointer_emulation if touchpad meets hover condition Signed-off-by: KT Liao

2016-06-13 Thread
Hi Dmitry,

We would like to follow up if there is any update or feedback for
this patch.

Thanks & B.R  KT

-Original Message-
From: 廖崇榮 [mailto:kt.l...@emc.com.tw] 
Sent: Thursday, June 02, 2016 7:57 PM
To: 'linux-kernel@vger.kernel.org'; 'linux-in...@vger.kernel.org';
'dmitry.torok...@gmail.com'
Cc: 'phoe...@emc.com.tw'; 'jeff.ch...@emc.com.tw';
'charliemoo...@google.com'; 'agnesch...@google.com'; 'jeff'
Subject: RE: [PATCH] Input: /input/input-mt.c Emit BTN_TOO_FINGER in
function input_mt_report_pointer_emulation if touchpad meets hover condition
Signed-off-by: KT Liao 

Hi Dmitry,

I up-streamed the patch last Sunday, please let me know if you have other
concern.

Thanks your support   KT

-Original Message-
From: KT Liao [mailto:ktalex.l...@gmail.com] 
Sent: Sunday, May 29, 2016 11:39 AM
To: linux-kernel@vger.kernel.org; linux-in...@vger.kernel.org;
dmitry.torok...@gmail.com
Cc: phoe...@emc.com.tw; kt.l...@emc.com.tw; jeff.ch...@emc.com.tw;
charliemoo...@google.com
Subject: [PATCH] Input: /input/input-mt.c Emit BTN_TOO_FINGER in function
input_mt_report_pointer_emulation if touchpad meets hover condition
Signed-off-by: KT Liao 

---
 drivers/input/input-mt.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c index
54fce56..b89aaa7 100644
--- a/drivers/input/input-mt.c
+++ b/drivers/input/input-mt.c
@@ -218,8 +218,17 @@ void input_mt_report_pointer_emulation(struct input_dev
*dev, bool use_count)
}
 
input_event(dev, EV_KEY, BTN_TOUCH, count > 0);
-   if (use_count)
+
+   if (use_count) {
+   if (count == 0 &&
+   !test_bit(ABS_MT_DISTANCE, dev->absbit) &&
+   test_bit(ABS_DISTANCE, dev->absbit) &&
+   input_abs_get_val(dev, ABS_DISTANCE) != 0) {
+   count = 1;
+   }
input_mt_report_finger_count(dev, count);
+   }
+
 
if (oldest) {
int x = input_mt_get_value(oldest, ABS_MT_POSITION_X);
--
2.7.4




RE: [PATCH] Input: /input/input-mt.c Emit BTN_TOO_FINGER in function input_mt_report_pointer_emulation if touchpad meets hover condition Signed-off-by: KT Liao

2016-06-02 Thread
Hi Dmitry,

I up-streamed the patch last Sunday, please let me know if you have other
concern.

Thanks your support   KT

-Original Message-
From: KT Liao [mailto:ktalex.l...@gmail.com] 
Sent: Sunday, May 29, 2016 11:39 AM
To: linux-kernel@vger.kernel.org; linux-in...@vger.kernel.org;
dmitry.torok...@gmail.com
Cc: phoe...@emc.com.tw; kt.l...@emc.com.tw; jeff.ch...@emc.com.tw;
charliemoo...@google.com
Subject: [PATCH] Input: /input/input-mt.c Emit BTN_TOO_FINGER in function
input_mt_report_pointer_emulation if touchpad meets hover condition
Signed-off-by: KT Liao 

---
 drivers/input/input-mt.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c index
54fce56..b89aaa7 100644
--- a/drivers/input/input-mt.c
+++ b/drivers/input/input-mt.c
@@ -218,8 +218,17 @@ void input_mt_report_pointer_emulation(struct input_dev
*dev, bool use_count)
}
 
input_event(dev, EV_KEY, BTN_TOUCH, count > 0);
-   if (use_count)
+
+   if (use_count) {
+   if (count == 0 &&
+   !test_bit(ABS_MT_DISTANCE, dev->absbit) &&
+   test_bit(ABS_DISTANCE, dev->absbit) &&
+   input_abs_get_val(dev, ABS_DISTANCE) != 0) {
+   count = 1;
+   }
input_mt_report_finger_count(dev, count);
+   }
+
 
if (oldest) {
int x = input_mt_get_value(oldest, ABS_MT_POSITION_X);
--
2.7.4




[PATCH] Emit BTN_TOO_FINGER in function input_mt_report_pointer_emulation if touchpad meets hover condition

2016-05-25 Thread
Hi Dmitry,

Update /input/mouse/input-mt.c
Emit BTN_TOO_FINGER in input_mt_report_pointer_emulation if touchpad meets 
hover condition.

Thanks & BR KT

-Original Message-
From: 'Dmitry Torokhov' [mailto:dmitry.torok...@gmail.com] 
Sent: Friday, May 20, 2016 8:52 AM
To: 廖崇榮
Cc: linux-in...@vger.kernel.org; linux-kernel@vger.kernel.org; 
zac.hs...@emc.com.tw; '黃世鵬 經理'; 'Charles Mooney'; 'Agnes Cheng'; 'jeff'
Subject: Re: [PATCH] Input: Change BTN_TOOL_FINGER flag when hover event trigger

On Tue, May 17, 2016 at 10:20:40PM +0800, 廖崇榮 wrote:
> Hi Dmitry,
> 
> I want to confirm my thought for your idea to avoid misunderstanding.
> I think you want to encapsulate " BTN_TOOL_FINGER" in the 
> [input_mt_report_pointer_emulation] if hover happen.
> Vendor driver only report "ABS_DISTANCE" and let 
> [input_mt_report_pointer_emulation] emit BTN_TOOL_FINGER report without 
> change function parameter.
> 
> Please let me know if my misunderstand about your idea.

Yes, that is correct. Something like this:

diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c index 
54fce56..a1bbec9 100644
--- a/drivers/input/input-mt.c
+++ b/drivers/input/input-mt.c
@@ -218,8 +218,23 @@ void input_mt_report_pointer_emulation(struct input_dev 
*dev, bool use_count)
}
 
input_event(dev, EV_KEY, BTN_TOUCH, count > 0);
-   if (use_count)
+
+   if (use_count) {
+   if (count == 0 &&
+   !test_bit(ABS_MT_DISTANCE, dev->absbit) &&
+   test_bit(ABS_DISTANCE, dev->absbit) &&
+   input_abs_get_val(dev, ABS_DISTANCE) != 0) {
+   /*
+* Force reporting BTN_TOOL_FINGER for devices that
+* only report general hover (and not per-contact
+* distance) when contact is in proximity but not
+* on the surface.
+*/
+   count = 1;
+   }
+
input_mt_report_finger_count(dev, count);
+   }
 
if (oldest) {
int x = input_mt_get_value(oldest, ABS_MT_POSITION_X);

--
Dmitry


0001-Input-drivers-input-input-mt.c.patch
Description: Binary data


RE: [PATCH] Input: Change BTN_TOOL_FINGER flag when hover event trigger

2016-05-23 Thread
Hi Dmitry,

Thanks for your confirmation.

I think the best and simplest way is to add below code to emit BTN_TOOL_FINGER 
if hover condition meets.

input_report_key(dev, BTN_TOOL_FINGER, input_mt_get_value(dev, ABS_DISTANCE));

Is it ok for you?

if (use_count) {
if (count == 0 &&
!test_bit(ABS_MT_DISTANCE, dev->absbit) &&
test_bit(ABS_DISTANCE, dev->absbit) &&
input_abs_get_val(dev, ABS_DISTANCE) != 0) {

/* Emit BTN_TOOL_FINGER*/
input_report_key(dev, BTN_TOOL_FINGER, 
input_mt_get_value(dev, ABS_DISTANCE));

count = 1;
}

B.R  KT
-Original Message-
From: 'Dmitry Torokhov' [mailto:dmitry.torok...@gmail.com] 
Sent: Friday, May 20, 2016 8:52 AM
To: 廖崇榮
Cc: linux-in...@vger.kernel.org; linux-kernel@vger.kernel.org; 
zac.hs...@emc.com.tw; '黃世鵬 經理'; 'Charles Mooney'; 'Agnes Cheng'; 'jeff'
Subject: Re: [PATCH] Input: Change BTN_TOOL_FINGER flag when hover event trigger

On Tue, May 17, 2016 at 10:20:40PM +0800, 廖崇榮 wrote:
> Hi Dmitry,
> 
> I want to confirm my thought for your idea to avoid misunderstanding.
> I think you want to encapsulate " BTN_TOOL_FINGER" in the 
> [input_mt_report_pointer_emulation] if hover happen.
> Vendor driver only report "ABS_DISTANCE" and let 
> [input_mt_report_pointer_emulation] emit BTN_TOOL_FINGER report without 
> change function parameter.
> 
> Please let me know if my misunderstand about your idea.

Yes, that is correct. Something like this:

diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c index 
54fce56..a1bbec9 100644
--- a/drivers/input/input-mt.c
+++ b/drivers/input/input-mt.c
@@ -218,8 +218,23 @@ void input_mt_report_pointer_emulation(struct input_dev 
*dev, bool use_count)
}
 
input_event(dev, EV_KEY, BTN_TOUCH, count > 0);
-   if (use_count)
+
+   if (use_count) {
+   if (count == 0 &&
+   !test_bit(ABS_MT_DISTANCE, dev->absbit) &&
+   test_bit(ABS_DISTANCE, dev->absbit) &&
+   input_abs_get_val(dev, ABS_DISTANCE) != 0) {
+   /*
+* Force reporting BTN_TOOL_FINGER for devices that
+* only report general hover (and not per-contact
+* distance) when contact is in proximity but not
+* on the surface.
+*/
+   count = 1;
+   }
+
input_mt_report_finger_count(dev, count);
+   }
 
if (oldest) {
int x = input_mt_get_value(oldest, ABS_MT_POSITION_X);

--
Dmitry



RE: [PATCH] Input: Change BTN_TOOL_FINGER flag when hover event trigger

2016-05-17 Thread
Hi Dmitry,

I want to confirm my thought for your idea to avoid misunderstanding.
I think you want to encapsulate " BTN_TOOL_FINGER" in the 
[input_mt_report_pointer_emulation] if hover happen.
Vendor driver only report "ABS_DISTANCE" and let 
[input_mt_report_pointer_emulation] emit BTN_TOOL_FINGER report without change 
function parameter.

Please let me know if my misunderstand about your idea.

Thanks  KT

-Original Message-
From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com] 
Sent: Tuesday, May 17, 2016 1:54 AM
To: 廖崇榮
Cc: linux-in...@vger.kernel.org; linux-kernel@vger.kernel.org; 
zac.hs...@emc.com.tw; 黃世鵬 經理; 'Charles Mooney'; 'Agnes Cheng'
Subject: Re: [PATCH] Input: Change BTN_TOOL_FINGER flag when hover event trigger

Hi Kt,

On Mon, May 16, 2016 at 07:27:25PM +0800, 廖崇榮 wrote:
> 
> Only ABS_DISTANCE is not enough for upper OS to distingiush hover 
> event be triggered from object from faraway to and close touchpad 
> surface or from object prepare to leave the touchpad surface. Add 
> BTN_TOOL_FINGER flag to help it.
> 
>  object_from_farawayobject_inside_hover_area
> object_touch_surface
> BTN_TOUCH 0  0 1
> BTN_TOOL_FINGER   0  1  1
> ABS_DISTANCE   0   1 0
> 
> Signed-off by: Duson Lin 

I was wondering if we could do without modifying all the drivers that are using 
input_mt_report_pointer_emulation(), by figuring out if we should be emitting 
BTN_TOOL_FINGER right there:

"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



[PATCH] Input: Change BTN_TOOL_FINGER flag when hover event trigger

2016-05-16 Thread

Only ABS_DISTANCE is not enough for upper OS to distingiush hover event be
triggered from object from faraway to and close touchpad surface or from
object prepare to leave the touchpad surface. Add BTN_TOOL_FINGER flag to
help it.

 object_from_farawayobject_inside_hover_area
object_touch_surface
BTN_TOUCH 0  0 1
BTN_TOOL_FINGER   0  1  1
ABS_DISTANCE   0   1 0

Signed-off by: Duson Lin 
---
 drivers/hid/hid-magicmouse.c |  2 +-
 drivers/input/input-mt.c |  7 +--
 drivers/input/mouse/elan_i2c_core.c  | 15 +--
 drivers/input/mouse/elantech.c   |  2 +-
 drivers/input/mouse/focaltech.c  |  2 +-
 drivers/input/mouse/synaptics.c  |  2 +-
 drivers/input/touchscreen/atmel_mxt_ts.c |  2 +-
 drivers/input/touchscreen/edt-ft5x06.c   |  2 +-
 drivers/input/touchscreen/egalax_ts.c|  2 +-
 drivers/input/touchscreen/ili210x.c  |  2 +-
 drivers/input/touchscreen/mms114.c   |  4 ++--
 drivers/input/touchscreen/penmount.c |  2 +-
 drivers/input/touchscreen/rohm_bu21023.c |  2 +-
drivers/input/touchscreen/wacom_w8001.c  |  2 +-
 include/linux/input/mt.h |  3 ++-
 15 files changed, 29 insertions(+), 22 deletions(-)

diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index d6fa496..584e98b 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -353,7 +353,7 @@ static int magicmouse_raw_event(struct hid_device *hdev,
input_report_rel(input, REL_Y, y);
} else { /* USB_DEVICE_ID_APPLE_MAGICTRACKPAD */
input_report_key(input, BTN_MOUSE, clicks & 1);
-   input_mt_report_pointer_emulation(input, true);
+   input_mt_report_pointer_emulation(input, true, false);
}
 
input_sync(input);
diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c index
54fce56..30855a5 100644
--- a/drivers/input/input-mt.c
+++ b/drivers/input/input-mt.c
@@ -184,6 +184,7 @@ EXPORT_SYMBOL(input_mt_report_finger_count);
  * input_mt_report_pointer_emulation() - common pointer emulation
  * @dev: input device with allocated MT slots
  * @use_count: report number of active contacts as finger count
+ * @hover: report ABS_DISTANCE as hover event
  *
  * Performs legacy pointer emulation via BTN_TOUCH, ABS_X, ABS_Y and
  * ABS_PRESSURE. Touchpad finger count is emulated if use_count is true.
@@ -191,7 +192,8 @@ EXPORT_SYMBOL(input_mt_report_finger_count);
  * The input core ensures only the KEY and ABS axes already setup for
  * this device will produce output.
  */
-void input_mt_report_pointer_emulation(struct input_dev *dev, bool
use_count)
+void input_mt_report_pointer_emulation(struct input_dev *dev, bool
use_count,
+   bool hover)
 {
struct input_mt *mt = dev->mt;
struct input_mt_slot *oldest;
@@ -220,6 +222,7 @@ void input_mt_report_pointer_emulation(struct input_dev
*dev, bool use_count)
input_event(dev, EV_KEY, BTN_TOUCH, count > 0);
if (use_count)
input_mt_report_finger_count(dev, count);
+   input_event(dev, EV_ABS, ABS_DISTANCE, hover);
 
if (oldest) {
int x = input_mt_get_value(oldest, ABS_MT_POSITION_X); @@
-290,7 +293,7 @@ void input_mt_sync_frame(struct input_dev *dev)
if ((mt->flags & INPUT_MT_POINTER) && !(mt->flags &
INPUT_MT_SEMI_MT))
use_count = true;
 
-   input_mt_report_pointer_emulation(dev, use_count);
+   input_mt_report_pointer_emulation(dev, use_count, false);
 
mt->frame++;
 }
diff --git a/drivers/input/mouse/elan_i2c_core.c
b/drivers/input/mouse/elan_i2c_core.c
index 2f58985..8b5e75a 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.2
  *
  * Based on cyapa driver:
  * copyright (c) 2011-2012 Cypress Semiconductor, Inc.
@@ -40,7 +40,7 @@
 #include "elan_i2c.h"
 
 #define DRIVER_NAME"elan_i2c"
-#define ELAN_DRIVER_VERSION"1.6.1"
+#define ELAN_DRIVER_VERSION"1.6.2"
 #define ELAN_VENDOR_ID 0x04f3
 #define ETP_MAX_PRESSURE   255
 #define ETP_FWIDTH_REDUCE  90
@@ -845,7 +845,7 @@ static void elan_report_absolute(struct elan_tp_data
*data, u8 *packet)  {
struct input_dev *input = data->input;
u8 *finger_data = &packet[ETP_FINGER_DATA_OFFSET];
-   int i;
+   int i, valid_count = 0;
u8 tp_info = packet[ETP_TOUCH_INFO_OFFSET];
u8 hover_info = packet[ETP_HOVER_INFO_OFFSET];
bool contact_valid, hover_event;
@@ -855,13 +855,16 @@ static void elan_report_absolute(struct elan_tp_data
*data, u8 *packet)
contact_valid = tp_info & (1