Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 18 Mar 2022 15:37:20 +0100
Message-ID: <s5hh77v2uov.wl-tiwai@suse.de>
From:   Takashi Iwai <tiwai@suse.de>
To:     Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc:     Takashi Iwai <tiwai@suse.de>,
        =?UTF-8?B?Sm9zw6kgRXhww7NzaXRv?= <jose.exposito89@gmail.com>,
        linux-input@vger.kernel.org,
        Peter Hutterer <peter.hutterer@who-t.net>,
        Jiri Kosina <jkosina@suse.cz>, stable@vger.kernel.org,
        regressions@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [REGRESSION] Right touchpad button disabled on Dell 7750
In-Reply-To: <CAO-hwJK8QMjYhQAC8tp7hLWZjSB3JMBJXgpKmFZRSEqPUn3_iw@mail.gmail.com>
References: <s5htubv32s8.wl-tiwai@suse.de>
        <20220318130740.GA33535@elementary>
        <s5hlex72yno.wl-tiwai@suse.de>
        <CAO-hwJK8QMjYhQAC8tp7hLWZjSB3JMBJXgpKmFZRSEqPUn3_iw@mail.gmail.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka)
 FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3
 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka")
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

On Fri, 18 Mar 2022 15:06:55 +0100,
Benjamin Tissoires wrote:
> 
> > > A couple of days ago another user with the same laptop (Dell Precision
> > > 7550 or 7750) emailed me to report the issue and I sent him a patch for
> > > testing.
> > >
> > > I he confirms that the patch works, I'll send it to the mailing list.
> > >
> > > I believe that your analysis of the regression is correct and I think
> > > that we'd need to add a quirk for the device.
> > >
> > > In case you want to have a look to the patch, I added it to this
> > > libinput [1] report.
> >
> > Great, I'll try to build and ask the reporter to test with the patch.
> >
> 
> As noticed on the libinput bug, I think the patch is wrong (not by a lot).
> We should base the class on MT_CLS_WIN8, not MT_CLS_DEFAULT.
> 
> The testers might say that it's working, but this might create some
> corner cases where it's not leading to more and more headaches with
> your users.

So is it like below?  I'll build another kernel with that.


Thanks!

Takashi

-- 8< --
From: José Expósito <jose.exposito89@gmail.com>
Subject: [PATCH] HID: multitouch: fix Dell Precision 7550 and 7750 button type

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.

In order to fix the issue, create a quirk for the device forcing its
button type to touchpad regardless of the value reported by the
firmware.

[1] https://gitlab.freedesktop.org/libinput/libinput/-/merge_requests/481
[2] https://bugzilla.redhat.com/show_bug.cgi?id=1868789

[ modified MT_CLS_BUTTONTYPE_TOUCHPAD quirk bits to base on MT_CLS_WIN8
  as suggested by Benjamin -- tiwai ]

Fixes: 37ef4c19b4 ("Input: clear BTN_RIGHT/MIDDLE on buttonpads")
Signed-off-by: José Expósito <jose.exposito89@gmail.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>

---
 drivers/hid/hid-ids.h        |    3 +++
 drivers/hid/hid-multitouch.c |   20 ++++++++++++++++++++
 2 files changed, 23 insertions(+)

--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -285,6 +285,9 @@
 
 #define USB_VENDOR_ID_CIDC		0x1677
 
+#define USB_VENDOR_ID_CIRQUE_CORP		0x0488
+#define USB_DEVICE_ID_DELL_PRECISION_7X50	0x120A
+
 #define USB_VENDOR_ID_CJTOUCH		0x24b8
 #define USB_DEVICE_ID_CJTOUCH_MULTI_TOUCH_0020	0x0020
 #define USB_DEVICE_ID_CJTOUCH_MULTI_TOUCH_0040	0x0040
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -71,6 +71,7 @@ MODULE_LICENSE("GPL");
 #define MT_QUIRK_SEPARATE_APP_REPORT	BIT(19)
 #define MT_QUIRK_FORCE_MULTI_INPUT	BIT(20)
 #define MT_QUIRK_DISABLE_WAKEUP		BIT(21)
+#define MT_QUIRK_BUTTONTYPE_TOUCHPAD	BIT(22)
 
 #define MT_INPUTMODE_TOUCHSCREEN	0x02
 #define MT_INPUTMODE_TOUCHPAD		0x03
@@ -194,6 +195,7 @@ static void mt_post_parse(struct mt_devi
 #define MT_CLS_WIN_8_FORCE_MULTI_INPUT		0x0015
 #define MT_CLS_WIN_8_DISABLE_WAKEUP		0x0016
 #define MT_CLS_WIN_8_NO_STICKY_FINGERS		0x0017
+#define MT_CLS_BUTTONTYPE_TOUCHPAD		0x0018
 
 /* vendor specific classes */
 #define MT_CLS_3M				0x0101
@@ -302,6 +304,15 @@ static const struct mt_class mt_classes[
 			MT_QUIRK_CONTACT_CNT_ACCURATE |
 			MT_QUIRK_WIN8_PTP_BUTTONS,
 		.export_all_inputs = true },
+	{ .name = MT_CLS_BUTTONTYPE_TOUCHPAD,
+		.quirks = MT_QUIRK_ALWAYS_VALID |
+			MT_QUIRK_IGNORE_DUPLICATES |
+			MT_QUIRK_HOVERING |
+			MT_QUIRK_CONTACT_CNT_ACCURATE |
+			MT_QUIRK_STICKY_FINGERS |
+			MT_QUIRK_WIN8_PTP_BUTTONS |,
+			MT_QUIRK_BUTTONTYPE_TOUCHPAD,
+		.export_all_inputs = true },
 
 	/*
 	 * vendor specific classes
@@ -1286,6 +1297,9 @@ static int mt_touch_input_configured(str
 	    (app->buttons_count == 1))
 		td->is_buttonpad = true;
 
+	if (app->quirks & MT_QUIRK_BUTTONTYPE_TOUCHPAD)
+		td->is_buttonpad = false;
+
 	if (td->is_buttonpad)
 		__set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
 
@@ -1875,6 +1889,12 @@ static const struct hid_device_id mt_dev
 		MT_USB_DEVICE(USB_VENDOR_ID_CHUNGHWAT,
 			USB_DEVICE_ID_CHUNGHWAT_MULTITOUCH) },
 
+	/* Cirque Corp (Dell Precision 7550 and 7750 touchpad) */
+	{ .driver_data = MT_CLS_BUTTONTYPE_TOUCHPAD,
+		HID_DEVICE(BUS_I2C, HID_GROUP_MULTITOUCH_WIN_8,
+			USB_VENDOR_ID_CIRQUE_CORP,
+			USB_DEVICE_ID_DELL_PRECISION_7X50) },
+
 	/* CJTouch panels */
 	{ .driver_data = MT_CLS_NSMU,
 		MT_USB_DEVICE(USB_VENDOR_ID_CJTOUCH,