Hi,

On 08/31/2014 05:14 PM, ulrik.debie-os@e2big.org wrote:

Hi
On Sun, Aug 31, 2014 at 11:54:25AM +0200, Hans de Goede wrote:

quoted

Date:	Sun, 31 Aug 2014 11:54:25 +0200
From: Hans de Goede <redacted>
To: Ulrik De Bie <redacted>, Dmitry Torokhov
 [off-list ref]
CC: linux-input@vger.kernel.org, David Herrmann <redacted>
Subject: Re: [PATCH 0/5] Input: elantech - support the Fujitsu H730 laptop
X-Mailing-List:	linux-input@vger.kernel.org

Hi,

On 08/30/2014 04:10 PM, Ulrik De Bie wrote:

quoted

This patch set makes the elantech driver work for the Fujitsu H730 laptop. It
also adds a sysfs knob to allow other laptops that are facing similar
problems as the Fujitsu H730 to have working touchpad.

I'm considering adding a WARN_ONCE when the crc_enabled signature check
fails. The message would then point the user to the crc_enabled sysfs knob,
and kindly ask the user to report the laptop to linux-input mailinglist ?
Any suggestions ?

WARN_ONCE includes a backtrace, which will just scare users, simply use a
function static variable to build your own warn_once, which really only does
warn. Other then that I think having a warning pointing to to the sysfs knob is
a good idea. Although maybe it should only trigger on 2 consecutive crc errors
to avoid false positives?

The static variable variant exists as printk_once. But of course, that one
will not allow easy check on 2 consecutive.

Right, still spurious triggering is rather unlikely to actually happen, so lets
just go with printk_once.

quoted

quoted

Two users have tested the combination of this patchset and confirmed that
it makes the H730 touchpad/trackpoint work.

Here an overview of the patchset:

Patch 1/ : Input: elantech - use elantech_report_trackpoint for hardware v4 too
The Fujitsu H730 is the first v4 hardware identified that also sends the
trackpoint packets. This patch enables the trackpoint on v4 hardware.
With this patch the trackpoint will work.

Patch 2/ : Input: elantech - Fix crc_enabled for Fujitsu H730
Uses DMI to detect the H730 and ,if detected, will set crc_enabled to 1. 
With this patch the touchpad and left/right button will start to work.

Patch 3/ : Input: elantech - report the middle button of the touchpad
The Fujitsu H730 is the first laptop listed in the elantech.c file with
3 touchpad buttons. This patch enables the middle button for all elantech
hardware and enables the reporting for v4 hardware.
I want to hear feedback here on what preferences exist for the conditions
to enable the middle button:
- DMI
- enable only for v4
- enable for all/report for v3+v4

I assume you've tested this on a v4 model without a middle button ? Assuming
that that is the case I think that always enabling it on v4 is fine. I see no
reason to also enable it on v3 as long as we've no reports of v3 models with
3 buttons.

No I have only tested myself on L530 (v3 with 2 touchpad buttons) and the two
testers have tested on Fujitsu H730 (v4 with 3 touchpad buttons).

Looking at the list in elantech, that would preferably be a test on Asus
G46VW or G750JX. Since you were able to come up with the list, do you
happen also to know contact details of some with such a laptop ?

I made that list by manual scraping info from forum and bug tracker posts,
so I'm afraid I've no contact info. All the other touchpad drivers sofar
are able to test for a middle button press without getting spurious
presses on laptops which don't have a middle button. So lets just move forward
with your patch as is, we can always go the dmi quirk route if it turns out
to cause troubles.

quoted

quoted

...

Patch 4/ : Input: Elantech - provide a sysfs knob for crc_enabled
Probably H730 will not remain the only elantech laptop that has this failing
detection for the crc_enabled. This provides users with a workaround when
the crc_enabled detection fails.

Patch 5/ : Elantech - Update the documentation: trackpoint,v3/v4,crc_enabled
This provides an update of the elantech documentation. 

Patch 4 depends on patch 2, and for consistency, patch 5 depends on patch 1-2-4.

Ulrik De Bie (5):
  Input: elantech - use elantech_report_trackpoint for hardware v4 too
  Input: elantech - Fix crc_enabled for Fujitsu H730
  Input: elantech - report the middle button of the touchpad
  Input: elantech - provide a sysfs knob for crc_enabled
  Input: elantech - Update the documentation:
    trackpoint,v3/v4,crc_enabled

The entire series looks good to me (the adding of the warning can be done in a follow
up patch), and is:

Acked-by: Hans de Goede <redacted>

Regards,

Hans

Thanks for your feedback !

Regards,
Ulrik