[PATCH synaptics] conf: ship a quirk for Cypress touchpads

[Peter Hutterer]

On Fri, Mar 21, 2014 at 11:03:36AM +0100, Hans de Goede wrote:
> Hi,
> 
> On 03/21/2014 10:31 AM, Hans de Goede wrote:
> > Hi,
> > 
> > On 03/20/2014 11:18 PM, Peter Hutterer wrote:
> >> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> >> ---
> >>  conf/11-x11-synaptics.fdi |  9 +++++++++
> >>  conf/50-synaptics.conf    | 12 ++++++++++++
> >>  2 files changed, 21 insertions(+)
> >>
> >> diff --git a/conf/11-x11-synaptics.fdi b/conf/11-x11-synaptics.fdi
> >> index a898875..44dfce3 100644
> >> --- a/conf/11-x11-synaptics.fdi
> >> +++ b/conf/11-x11-synaptics.fdi
> >> @@ -34,6 +34,15 @@
> >>          <match key="info.product" contains="Apple|bcm5974">
> >>              <merge key="input.x11_options.SoftButtonAreas" type="string">0 0 0 0 0 0 0 0</merge>
> >>          </match>
> >> +
> >> +        <!--The Cypress touchpads provide BTN_RIGHT in firmware, together with
> >> +            clickfinger, and two-finger scrolling. Disable Clickpads, otherwise we
> >> +            get flaky button behaviour.
> >> +            https://bugs.freedesktop.org/show_bug.cgi?id=70819
> >> +            https://bugs.freedesktop.org/show_bug.cgi?id=76341 -->
> > 
> > I'm not so sure that using a xorg.conf snippet for this is the best way to go about this, I would
> > prefer to see a fix to the kernel to not report the clickpad flag for non clickpads. Esp. as some
> > newer or future Cypress touchpads may very well get rid of the firmware mode and do everything in
> > the drivers as other vendors do.
> > 
> > I really think that "CyPS/2 Cypress Trackpad" is a way to generic match string and is
> > going to bite us in the future. At a minimum we should use the still to be merged DMI matching
> > here to limit this to certain laptop models, but IHMO it would be much better to simply fix
> > this in the kernel. For starters we could add a dmi quirk in the kernel for the XPS 13, until we
> > figure out a better way to decide which Cypress Trackpads use this firmware emulation and which
> > models don't.
> > 
> > And maybe the one in the Dell XPS 13 even has a command we can send to it to turn of the
> > emulation too, which would allow us to really fix 70819 too.
> 
> So I've spend some time reading the kernel cypress_ps2 driver, conclusions:
> 
> 1) It can only do multi-touch for upto 2 fingers, it can detect tripple / quad
> fingers being down but will only report coordinates for 2.
> 
> 2) It seems to do a lot of processing in firmware, so in general at least for
> the currently supported generation not reporting INPUT_PROP_BUTTONPAD seems to be
> the right thing to do for all currently supported Cypress Trackpads.
> 
> I still believe the kernel is the right place to fix this. Adam, can you build a kernel
> with the attached patch, remove your xorg.conf changes and then test that setup please ?
> 
> Regards,
> 
> Hans

> From 4b0668abbea0068cbdf7f9b0d149ccc245d79fab Mon Sep 17 00:00:00 2001
> From: Hans de Goede <hdegoede at redhat.com>
> Date: Fri, 21 Mar 2014 10:55:11 +0100
> Subject: [PATCH] input: cypress_ps2: Don't report the cypress PS/2 trackpads
>  as a button pad
> 
> The cypress PS/2 trackpad models supported by the cypress_ps2 driver do
> right button emulation in firmware, filtering out a bunch of events userspace
> needs to do clickpad handling on its own. So reporting them as clickpads
> actually causes a bunch of issues:
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=70819
> https://bugs.freedesktop.org/show_bug.cgi?id=76341
> 
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
>  drivers/input/mouse/cypress_ps2.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/input/mouse/cypress_ps2.c b/drivers/input/mouse/cypress_ps2.c
> index 87095e2..8af34ff 100644
> --- a/drivers/input/mouse/cypress_ps2.c
> +++ b/drivers/input/mouse/cypress_ps2.c
> @@ -409,7 +409,6 @@ static int cypress_set_input_params(struct input_dev *input,
>  	__clear_bit(REL_X, input->relbit);
>  	__clear_bit(REL_Y, input->relbit);
>  
> -	__set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
>  	__set_bit(EV_KEY, input->evbit);
>  	__set_bit(BTN_LEFT, input->keybit);
>  	__set_bit(BTN_RIGHT, input->keybit);
> -- 
> 1.9.0

I recommend that you expand the commit message with a blurb on why it causes
issues before you send it to the lkml, it'll make it easier to review.
Specifically that BTN_RIGHT is emulated in firmware based on the finger
position, and that no motion events are sent when the finger is in the
button area.

other than that this seems to be the best solution, thanks.
Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
though I'll still ship the xorg.conf snippet in fedora until this filters
down.

Cheers,
   Peter