Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1590130 > unrolled thread

Re: [GIT PULL] HID for 4.11

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [GIT PULL] HID for 4.11 Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2017-03-01 10:30 +0100
    Re: [GIT PULL] HID for 4.11 Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-03-01 19:00 +0100
      Re: [GIT PULL] HID for 4.11 Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-01 21:10 +0100
    Re: [GIT PULL] HID for 4.11 Linus Torvalds <torvalds@linux-foundation.org> - 2017-03-01 20:00 +0100

#1590130 — Re: [GIT PULL] HID for 4.11

On Feb 28 2017 or thereabouts, Linus Torvalds wrote:
> On Tue, Feb 28, 2017 at 7:24 PM, Peter Hutterer
> <peter.hutterer@who-t.net> wrote:
> >
> > I suspect you're just triggering a bug that wasn't triggered by the ps/2
> > emulation. you can run linput-debug-events --verbose and have a look at the
> > various state debugging information, that may hint at what's going on (e.g.
> > a finger mistaken as palm touch, or something). Or record one such
> > interaction with evemu-record and send it to me (preferrably here [1], if
> > you're using libinput). Also, what version of libinput/synaptics are you on?
> 
> bug reported (it's bug 100014).
> 

Thanks for the report.

As Peter mentioned in the bug, there is a missing property on the kernel
node (INPUT_PROP_BUTTONPAD). 

The thing is this property is solely driven in the current driver by the
provided platform_data, so there is no way we ever set it through
hid-rmi. I wonder how we missed that.

Anyway, the good news is that the evemu record shows only one exportted
button, so we can infer the property quite easily in the module. Would
something like that work for you?

From 5f28af88f2c67d1c533500765c5190cdd3006539 Mon Sep 17 00:00:00 2001
From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Date: Wed, 1 Mar 2017 09:57:00 +0100
Subject: [PATCH] Input: rmi4 - f30: detect INPUT_PROP_BUTTONPAD from the
 button count

INPUT_PROP_BUTTONPAD is currently only set through the platform data.
The RMI4 header doc says that this property is there to force the
buttonpad property, so we also need to detect it by looking at
the exported buttons count.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/rmi4/rmi_f30.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/input/rmi4/rmi_f30.c b/drivers/input/rmi4/rmi_f30.c
index 3422464..1986786 100644
--- a/drivers/input/rmi4/rmi_f30.c
+++ b/drivers/input/rmi4/rmi_f30.c
@@ -258,9 +258,10 @@ static int rmi_f30_map_gpios(struct rmi_function *fn,
 
 	/*
 	 * Buttonpad could be also inferred from f30->has_mech_mouse_btns,
-	 * but I am not sure, so use only the pdata info.
+	 * but I am not sure, so use only the pdata info and the number of
+	 * mapped buttons.
 	 */
-	if (pdata->f30_data.buttonpad)
+	if (pdata->f30_data.buttonpad || (button - BTN_LEFT == 1))
 		__set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
 
 	return 0;
-- 
2.9.3

Dmitry, Andrew, would this work for you too?

Cheers,
Benjamin

[toc] | [next] | [standalone]

#1590543

On Wed, Mar 01, 2017 at 09:54:07AM -0800, Linus Torvalds wrote:
> On Wed, Mar 1, 2017 at 1:03 AM, Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
> >
> > As Peter mentioned in the bug, there is a missing property on the kernel
> > node (INPUT_PROP_BUTTONPAD).
> >
> > The thing is this property is solely driven in the current driver by the
> > provided platform_data, so there is no way we ever set it through
> > hid-rmi. I wonder how we missed that.
> >
> > Anyway, the good news is that the evemu record shows only one exportted
> > button, so we can infer the property quite easily in the module. Would
> > something like that work for you?
> >
> > From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > Date: Wed, 1 Mar 2017 09:57:00 +0100
> > Subject: [PATCH] Input: rmi4 - f30: detect INPUT_PROP_BUTTONPAD from the button count
> 
> Yes, this fixes the problem for me. My click-and-drag works again, so
> you can add a
> 
>   Reported-and-tested-by: Linus Torvalds <torvalds@linux-foundation.org>
> 
> I see that Dmitry doesn't love the patch, but I'm assuming I'll get
> that or something equivalent soon. In the meantime, I'll just keep it
> on my laptop as a workaround.

Given that it does work for you just apply it. The objections I raised
was more of a bikeshedding.

Thanks.

-- 
Dmitry

[toc] | [prev] | [next] | [standalone]

#1590605

On Wed, Mar 1, 2017 at 9:58 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Given that it does work for you just apply it. The objections I raised
> was more of a bikeshedding.

Ok. Patch applied directly and in my tree now,

                  Linus

[toc] | [prev] | [next] | [standalone]

#1590569

On Wed, Mar 1, 2017 at 1:03 AM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> As Peter mentioned in the bug, there is a missing property on the kernel
> node (INPUT_PROP_BUTTONPAD).
>
> The thing is this property is solely driven in the current driver by the
> provided platform_data, so there is no way we ever set it through
> hid-rmi. I wonder how we missed that.
>
> Anyway, the good news is that the evemu record shows only one exportted
> button, so we can infer the property quite easily in the module. Would
> something like that work for you?
>
> From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Date: Wed, 1 Mar 2017 09:57:00 +0100
> Subject: [PATCH] Input: rmi4 - f30: detect INPUT_PROP_BUTTONPAD from the button count

Yes, this fixes the problem for me. My click-and-drag works again, so
you can add a

  Reported-and-tested-by: Linus Torvalds <torvalds@linux-foundation.org>

I see that Dmitry doesn't love the patch, but I'm assuming I'll get
that or something equivalent soon. In the meantime, I'll just keep it
on my laptop as a workaround.

Thanks,

                   Linus

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web