diff mbox series

Revert "Input: clear BTN_RIGHT/MIDDLE on buttonpads"

Message ID 20220321184404.20025-1-jose.exposito89@gmail.com
State Accepted
Commit 8b188fba75195745026e11d408e4a7e94e01d701
Headers show
Series Revert "Input: clear BTN_RIGHT/MIDDLE on buttonpads" | expand

Commit Message

José Expósito March 21, 2022, 6:44 p.m. UTC
This reverts commit 37ef4c19b4c659926ce65a7ac709ceaefb211c40.

The touchpad present in the Dell Precision 7550 and 7750 laptops
reports a HID_DG_BUTTONTYPE of type MT_BUTTONTYPE_CLICKPAD. However,
the device is not a clickpad, it is a touchpad with physical buttons.

In order to fix this issue, a quirk for the device was introduced in
libinput [1] [2] to disable the INPUT_PROP_BUTTONPAD property:

	[Precision 7x50 Touchpad]
	MatchBus=i2c
	MatchUdevType=touchpad
	MatchDMIModalias=dmi:*svnDellInc.:pnPrecision7?50*
	AttrInputPropDisable=INPUT_PROP_BUTTONPAD

However, because of the change introduced in 37ef4c19b4 ("Input: clear
BTN_RIGHT/MIDDLE on buttonpads") the BTN_RIGHT key bit is not mapped
anymore breaking the device right click button and making impossible to
workaround it in user space.

In order to avoid breakage on other present or future devices, revert
the patch causing the issue.

Cc: stable@vger.kernel.org
Link: https://gitlab.freedesktop.org/libinput/libinput/-/merge_requests/481 [1]
Link: https://bugzilla.redhat.com/show_bug.cgi?id=1868789  [2]
Signed-off-by: José Expósito <jose.exposito89@gmail.com>
---
 drivers/input/input.c | 6 ------
 1 file changed, 6 deletions(-)

Comments

Hans de Goede March 22, 2022, 11:15 a.m. UTC | #1
Hi,

On 3/21/22 19:44, José Expósito wrote:
> This reverts commit 37ef4c19b4c659926ce65a7ac709ceaefb211c40.
> 
> The touchpad present in the Dell Precision 7550 and 7750 laptops
> reports a HID_DG_BUTTONTYPE of type MT_BUTTONTYPE_CLICKPAD. However,
> the device is not a clickpad, it is a touchpad with physical buttons.
> 
> In order to fix this issue, a quirk for the device was introduced in
> libinput [1] [2] to disable the INPUT_PROP_BUTTONPAD property:
> 
> 	[Precision 7x50 Touchpad]
> 	MatchBus=i2c
> 	MatchUdevType=touchpad
> 	MatchDMIModalias=dmi:*svnDellInc.:pnPrecision7?50*
> 	AttrInputPropDisable=INPUT_PROP_BUTTONPAD
> 
> However, because of the change introduced in 37ef4c19b4 ("Input: clear
> BTN_RIGHT/MIDDLE on buttonpads") the BTN_RIGHT key bit is not mapped
> anymore breaking the device right click button and making impossible to
> workaround it in user space.
> 
> In order to avoid breakage on other present or future devices, revert
> the patch causing the issue.
> 
> Cc: stable@vger.kernel.org
> Link: https://gitlab.freedesktop.org/libinput/libinput/-/merge_requests/481 [1]
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1868789  [2]
> Signed-off-by: José Expósito <jose.exposito89@gmail.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> ---
>  drivers/input/input.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index c3139bc2aa0d..ccaeb2426385 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -2285,12 +2285,6 @@ int input_register_device(struct input_dev *dev)
>  	/* KEY_RESERVED is not supposed to be transmitted to userspace. */
>  	__clear_bit(KEY_RESERVED, dev->keybit);
>  
> -	/* Buttonpads should not map BTN_RIGHT and/or BTN_MIDDLE. */
> -	if (test_bit(INPUT_PROP_BUTTONPAD, dev->propbit)) {
> -		__clear_bit(BTN_RIGHT, dev->keybit);
> -		__clear_bit(BTN_MIDDLE, dev->keybit);
> -	}
> -
>  	/* Make sure that bitmasks not mentioned in dev->evbit are clean. */
>  	input_cleanse_bitmasks(dev);
>
Benjamin Tissoires March 30, 2022, 12:30 p.m. UTC | #2
On Wed, Mar 30, 2022 at 2:27 PM Thorsten Leemhuis
<regressions@leemhuis.info> wrote:
>
> Hi, this is your Linux kernel regression tracker.
>
> On 21.03.22 19:44, José Expósito wrote:
> > This reverts commit 37ef4c19b4c659926ce65a7ac709ceaefb211c40.
> >
> > The touchpad present in the Dell Precision 7550 and 7750 laptops
> > reports a HID_DG_BUTTONTYPE of type MT_BUTTONTYPE_CLICKPAD. However,
> > the device is not a clickpad, it is a touchpad with physical buttons.
> >
> > In order to fix this issue, a quirk for the device was introduced in
> > libinput [1] [2] to disable the INPUT_PROP_BUTTONPAD property:
> >
> >       [Precision 7x50 Touchpad]
> >       MatchBus=i2c
> >       MatchUdevType=touchpad
> >       MatchDMIModalias=dmi:*svnDellInc.:pnPrecision7?50*
> >       AttrInputPropDisable=INPUT_PROP_BUTTONPAD
> >
> > However, because of the change introduced in 37ef4c19b4 ("Input: clear
> > BTN_RIGHT/MIDDLE on buttonpads") the BTN_RIGHT key bit is not mapped
> > anymore breaking the device right click button and making impossible to
> > workaround it in user space.
> >
> > In order to avoid breakage on other present or future devices, revert
> > the patch causing the issue.
> >
> > Cc: stable@vger.kernel.org
> > Link: https://gitlab.freedesktop.org/libinput/libinput/-/merge_requests/481 [1]
> > Link: https://bugzilla.redhat.com/show_bug.cgi?id=1868789  [2]
> > Signed-off-by: José Expósito <jose.exposito89@gmail.com>
> > [...]
>
> Jiri, Benjamin, what the status here? Sure, this is not a crucial
> regression and we are in the middle of the merge window, but it looks
> like nothing has happened for a week now. Or was progress made somewhere
> and I just missed it?

No, I think it just wasn't picked up by the input maintainer yet
(Dmitry, now in CC).

FWIW:
Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

José, please do not forget to add the input maintainer when you target
the input tree, not the HID one :)

Cheers,
Benjamin

>
> #regzbot ^backmonitor:
> https://lore.kernel.org/stable/s5htubv32s8.wl-tiwai@suse.de/
>
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
>
> P.S.: As the Linux kernel's regression tracker I'm getting a lot of
> reports on my table. I can only look briefly into most of them and lack
> knowledge about most of the areas they concern. I thus unfortunately
> will sometimes get things wrong or miss something important. I hope
> that's not the case here; if you think it is, don't hesitate to tell me
> in a public reply, it's in everyone's interest to set the public record
> straight.
>
>
Linux regression tracking (Thorsten Leemhuis) April 5, 2022, 9:35 a.m. UTC | #3
Just a quick add-on note for the record about another affected device:

On 31.03.22 06:38, Peter Hutterer wrote:
> On Wed, Mar 30, 2022 at 09:13:30PM -0700, Dmitry Torokhov wrote:
>> On Wed, Mar 30, 2022 at 02:30:37PM +0200, Benjamin Tissoires wrote:
>>> On Wed, Mar 30, 2022 at 2:27 PM Thorsten Leemhuis
>>> <regressions@leemhuis.info> wrote:
>>>> On 21.03.22 19:44, José Expósito wrote:
>>>>> This reverts commit 37ef4c19b4c659926ce65a7ac709ceaefb211c40.
>>>>>
>>>>> The touchpad present in the Dell Precision 7550 and 7750 laptops
>>>>> reports a HID_DG_BUTTONTYPE of type MT_BUTTONTYPE_CLICKPAD. However,
>>>>> the device is not a clickpad, it is a touchpad with physical buttons.
>>>>>
>>>>> In order to fix this issue, a quirk for the device was introduced in
>>>>> libinput [1] [2] to disable the INPUT_PROP_BUTTONPAD property:
>>>>>
>>>>>       [Precision 7x50 Touchpad]
>>>>>       MatchBus=i2c
>>>>>       MatchUdevType=touchpad
>>>>>       MatchDMIModalias=dmi:*svnDellInc.:pnPrecision7?50*
>>>>>       AttrInputPropDisable=INPUT_PROP_BUTTONPAD
>>>>>
>>>>> However, because of the change introduced in 37ef4c19b4 ("Input: clear
>>>>> BTN_RIGHT/MIDDLE on buttonpads") the BTN_RIGHT key bit is not mapped
>>>>> anymore breaking the device right click button and making impossible to
>>>>> workaround it in user space.
>>>>>
>>>>> In order to avoid breakage on other present or future devices, revert
>>>>> the patch causing the issue.
>>>>>
>>>>> Cc: stable@vger.kernel.org
>>>>> Link: https://gitlab.freedesktop.org/libinput/libinput/-/merge_requests/481 [1]
>>>>> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1868789  [2]
>>>>> Signed-off-by: José Expósito <jose.exposito89@gmail.com>
>>>>> [...]
>>>>
>>>> Jiri, Benjamin, what the status here? Sure, this is not a crucial
>>>> regression and we are in the middle of the merge window, but it looks
>>>> like nothing has happened for a week now. Or was progress made somewhere
>>>> and I just missed it?
>>>
>>> No, I think it just wasn't picked up by the input maintainer yet
>>> (Dmitry, now in CC).
>>>
>>> FWIW:
>>> Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>>>
>>> José, please do not forget to add the input maintainer when you target
>>> the input tree, not the HID one :)
>>
>> I see that there were several ACKs, but how many devices misuse the
>> HID_DG_BUTTONTYPE? Would it be better to quirk against either affected
>> Dell models, or particular touchpads (by HID IDs) instead of reverting
>> wholesale?
> 
> fwiw, a quick git grep in libinput shows 12 entries for disabling BTN_RIGHT
> and 9 entries for enabling/disabling INPUT_PROP_BUTTONPAD. That's not the
> number of devices affected by this bug, merely devices we know advertise the
> wrong combination.
> 
> Note that the cause for the revert is loss of functionality. Previously, a
> device was just advertising buttons incorrectly but still worked fine. This
> was mostly a cosmetic issue (and could be worked around in userspace). With
> the patch in place some devices right button no longer works because it's
> filtered by the kernel. That's why the revert is needed.
> 
> The device could/should still be quirked to drop INPUT_PROP_BUTTONPAD but that
> is only required to work around the cosmetic issues then.

I anyone cares now or in the future: seems the patch meanwhile reverted
(thx) also broke the ICL Si1516:
https://bugzilla.kernel.org/show_bug.cgi?id=215771

Ciao, Thorsten

P.S.: Any while at it, let me let regzbot about the fix as well:

#regzbot fixed-by: 8b188fba75195745026e11d408e4a7e94e01d701
diff mbox series

Patch

diff --git a/drivers/input/input.c b/drivers/input/input.c
index c3139bc2aa0d..ccaeb2426385 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -2285,12 +2285,6 @@  int input_register_device(struct input_dev *dev)
 	/* KEY_RESERVED is not supposed to be transmitted to userspace. */
 	__clear_bit(KEY_RESERVED, dev->keybit);
 
-	/* Buttonpads should not map BTN_RIGHT and/or BTN_MIDDLE. */
-	if (test_bit(INPUT_PROP_BUTTONPAD, dev->propbit)) {
-		__clear_bit(BTN_RIGHT, dev->keybit);
-		__clear_bit(BTN_MIDDLE, dev->keybit);
-	}
-
 	/* Make sure that bitmasks not mentioned in dev->evbit are clean. */
 	input_cleanse_bitmasks(dev);