On Tue, Feb 28, 2017 at 7:24 PM, Peter Hutterer
<[email protected]> 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).
This is libinput-1.5.4 on Fedora 24. I attached both the
libinput-debug-events output as well as evemu-report output, which
hopefully fills in all the details.
Linus
[I forgot to add Dmitry in the loop, sorry for the noise.]
On Mar 01 2017 or thereabouts, Benjamin Tissoires wrote:
> On Feb 28 2017 or thereabouts, Linus Torvalds wrote:
> > On Tue, Feb 28, 2017 at 7:24 PM, Peter Hutterer
> > <[email protected]> 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 <[email protected]>
> 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 <[email protected]>
> ---
> 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
On Feb 28 2017 or thereabouts, Linus Torvalds wrote:
> On Tue, Feb 28, 2017 at 7:24 PM, Peter Hutterer
> <[email protected]> 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 <[email protected]>
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 <[email protected]>
---
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
On 02/28/2017 09:05 PM, Linus Torvalds wrote:
> On Tue, Feb 28, 2017 at 7:24 PM, Peter Hutterer
> <[email protected]> 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).
>
> This is libinput-1.5.4 on Fedora 24. I attached both the
> libinput-debug-events output as well as evemu-report output, which
> hopefully fills in all the details.
>
> Linus
Actually, it looks like this is a regression which was introduced by
bf3e8502eefd ("Input: synaptics-rmi4 - clean up F30 implementation").
From the bug report I noticed that the INPUT_PROP_BUTTONPAD property
was not set for the touchpad. The previous behavior was to set
INPUT_PROP_BUTTONPAD if F30 detected only one valid button (or if there
was a flag in the platform data). But, now INPUT_PROP_BUTTONPAD is not
being set and libinput does not know that the touchpad is a clickpad.
After updating I was able to reproduce the issue with bf3e8502eefd applied.
Andrew
On Wed, Mar 01, 2017 at 09:54:07AM -0800, Linus Torvalds wrote:
> On Wed, Mar 1, 2017 at 1:03 AM, Benjamin Tissoires
> <[email protected]> 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 <[email protected]>
> > 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 <[email protected]>
>
> 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
Hi Benjamin,
On Wed, Mar 01, 2017 at 10:06:35AM +0100, Benjamin Tissoires wrote:
> [I forgot to add Dmitry in the loop, sorry for the noise.]
>
> On Mar 01 2017 or thereabouts, Benjamin Tissoires wrote:
> > On Feb 28 2017 or thereabouts, Linus Torvalds wrote:
> > > On Tue, Feb 28, 2017 at 7:24 PM, Peter Hutterer
> > > <[email protected]> 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 <[email protected]>
> > 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 <[email protected]>
> > ---
> > 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);
Would prefer if we changed button_mapped to n_buttons_mapped counter and
used that, instead of doing calculations on event code.
Thanks.
--
Dmitry
On Wed, Mar 1, 2017 at 1:03 AM, Benjamin Tissoires
<[email protected]> 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 <[email protected]>
> 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 <[email protected]>
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
On Wed, Mar 1, 2017 at 9:58 AM, Dmitry Torokhov
<[email protected]> 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