Hi On Sun, Aug 31, 2014 at 11:54:25AM +0200, Hans de Goede wrote: > Date: Sun, 31 Aug 2014 11:54:25 +0200 > From: Hans de Goede <hdegoede@xxxxxxxxxx> > To: Ulrik De Bie <ulrik.debie-os@xxxxxxxxx>, Dmitry Torokhov > <dmitry.torokhov@xxxxxxxxx> > CC: linux-input@xxxxxxxxxxxxxxx, David Herrmann <dh.herrmann@xxxxxxxxx> > Subject: Re: [PATCH 0/5] Input: elantech - support the Fujitsu H730 laptop > X-Mailing-List: linux-input@xxxxxxxxxxxxxxx > > Hi, > > On 08/30/2014 04:10 PM, Ulrik De Bie wrote: > > 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. > > 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 ? > > ... > > > > 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 <hdegoede@xxxxxxxxxx> > > Regards, > > Hans Thanks for your feedback ! Regards, Ulrik -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
