[REGRESSION] Right touchpad button disabled on Dell 7750

[REGRESSION] Right touchpad button disabled on Dell 7750

[Takashi Iwai]

Hi,

we received a bug report about the regression of the touchpad on Dell
7750 laptop, the right touchpad button is disabled on recent kernels:
  https://bugzilla.suse.com/show_bug.cgi?id=1197243

Note that it's a physical button, not a virtual clickpad button.

The regression seems introduced by the upstream commit
37ef4c19b4c659926ce65a7ac709ceaefb211c40 ("Input: clear
BTN_RIGHT/MIDDLE on buttonpads") that was backported to stable 5.16.x
kernel.

The device is managed by hid-multitouch driver, and the further
investigation revealed that it's rather an incorrectly recognized
buttonpad property; namely, ID_DG_BUTTONTYPE reports it being 0 =
clickable touchpad although it's not.  I built a test kernel to ignore
this check and it was confirmed to make the right button working again
by the reporter.

Is this check really correct in general?  Or do we need some
device-specific quirk?


thanks,

Takashi

Re: [REGRESSION] Right touchpad button disabled on Dell 7750

[José Expósito]

Hi Takashi,

Thanks for reporting the regression here.

On Fri, Mar 18, 2022 at 12:42:31PM +0100, Takashi Iwai wrote:
> Hi,
> 
> we received a bug report about the regression of the touchpad on Dell
> 7750 laptop, the right touchpad button is disabled on recent kernels:
>   https://bugzilla.suse.com/show_bug.cgi?id=1197243
> 
> Note that it's a physical button, not a virtual clickpad button.
> 
> The regression seems introduced by the upstream commit
> 37ef4c19b4c659926ce65a7ac709ceaefb211c40 ("Input: clear
> BTN_RIGHT/MIDDLE on buttonpads") that was backported to stable 5.16.x
> kernel.
> 
> The device is managed by hid-multitouch driver, and the further
> investigation revealed that it's rather an incorrectly recognized
> buttonpad property; namely, ID_DG_BUTTONTYPE reports it being 0 =
> clickable touchpad although it's not.  I built a test kernel to ignore
> this check and it was confirmed to make the right button working again
> by the reporter.
> 
> Is this check really correct in general?  Or do we need some
> device-specific quirk?

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.

Thanks,
Jose

[1] https://gitlab.freedesktop.org/libinput/libinput/-/merge_requests/726#note_1303623

Re: [REGRESSION] Right touchpad button disabled on Dell 7750

[Takashi Iwai]

On Fri, 18 Mar 2022 14:07:40 +0100,
José Expósito wrote:
> 
> Hi Takashi,
> 
> Thanks for reporting the regression here.
> 
> On Fri, Mar 18, 2022 at 12:42:31PM +0100, Takashi Iwai wrote:
> > Hi,
> > 
> > we received a bug report about the regression of the touchpad on Dell
> > 7750 laptop, the right touchpad button is disabled on recent kernels:
> >   https://bugzilla.suse.com/show_bug.cgi?id=1197243
> > 
> > Note that it's a physical button, not a virtual clickpad button.
> > 
> > The regression seems introduced by the upstream commit
> > 37ef4c19b4c659926ce65a7ac709ceaefb211c40 ("Input: clear
> > BTN_RIGHT/MIDDLE on buttonpads") that was backported to stable 5.16.x
> > kernel.
> > 
> > The device is managed by hid-multitouch driver, and the further
> > investigation revealed that it's rather an incorrectly recognized
> > buttonpad property; namely, ID_DG_BUTTONTYPE reports it being 0 =
> > clickable touchpad although it's not.  I built a test kernel to ignore
> > this check and it was confirmed to make the right button working again
> > by the reporter.
> > 
> > Is this check really correct in general?  Or do we need some
> > device-specific quirk?
> 
> 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.

Thanks!


Takashi
> 
> Thanks,
> Jose
> 
> [1] https://gitlab.freedesktop.org/libinput/libinput/-/merge_requests/726#note_1303623
> 

Re: [REGRESSION] Right touchpad button disabled on Dell 7750

[Benjamin Tissoires]

On Fri, Mar 18, 2022 at 2:11 PM Takashi Iwai <tiwai@suse.de> wrote:
>
> On Fri, 18 Mar 2022 14:07:40 +0100,
> José Expósito wrote:
> >
> > Hi Takashi,
> >
> > Thanks for reporting the regression here.
> >
> > On Fri, Mar 18, 2022 at 12:42:31PM +0100, Takashi Iwai wrote:
> > > Hi,
> > >
> > > we received a bug report about the regression of the touchpad on Dell
> > > 7750 laptop, the right touchpad button is disabled on recent kernels:
> > >   https://bugzilla.suse.com/show_bug.cgi?id=1197243
> > >
> > > Note that it's a physical button, not a virtual clickpad button.
> > >
> > > The regression seems introduced by the upstream commit
> > > 37ef4c19b4c659926ce65a7ac709ceaefb211c40 ("Input: clear
> > > BTN_RIGHT/MIDDLE on buttonpads") that was backported to stable 5.16.x
> > > kernel.
> > >
> > > The device is managed by hid-multitouch driver, and the further
> > > investigation revealed that it's rather an incorrectly recognized
> > > buttonpad property; namely, ID_DG_BUTTONTYPE reports it being 0 =
> > > clickable touchpad although it's not.  I built a test kernel to ignore
> > > this check and it was confirmed to make the right button working again
> > > by the reporter.

Yep, I came to the same conclusion this morning with the reporter of
the libinput bug José was mentioning.

> > >
> > > Is this check really correct in general?  Or do we need some
> > > device-specific quirk?

The device firmware is clearly wrong here. It's the first time I see
this failing like that and I hope this is just an isolated case.
The device advertises itself as a buttonpad, when it's not.

However, the fact that it passed MS certification (even if I doubt
Microsoft ever got that touchpad in their own hands) leads me to
believe that the certification doesn't enforce that setting too much,
and that we might see more devices coming in with that same bug.

To sum up, I am not happy that this commit introduced a regression
that we can not work around in userspace: given that BTN_RIGHT gets
removed from the device, all of the values are filtered out and
userspace can not resolve that situation by itself.

I wish I had a better way of fixing this than having to quirk the device.

> >
> > 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.

Cheers,
Benjamin

> Thanks!
>
>
> Takashi
> >
> > Thanks,
> > Jose
> >
> > [1] https://gitlab.freedesktop.org/libinput/libinput/-/merge_requests/726#note_1303623
> >
>

Re: [REGRESSION] Right touchpad button disabled on Dell 7750

[José Expósito]

On Fri, Mar 18, 2022 at 03:06:55PM +0100, Benjamin Tissoires wrote:
> On Fri, Mar 18, 2022 at 2:11 PM Takashi Iwai <tiwai@suse.de> wrote:
> > > > Hi,
> > > >
> > > > we received a bug report about the regression of the touchpad on Dell
> > > > 7750 laptop, the right touchpad button is disabled on recent kernels:
> > > >   https://bugzilla.suse.com/show_bug.cgi?id=1197243
> > > >
> > > > Note that it's a physical button, not a virtual clickpad button.
> > > >
> > > > The regression seems introduced by the upstream commit
> > > > 37ef4c19b4c659926ce65a7ac709ceaefb211c40 ("Input: clear
> > > > BTN_RIGHT/MIDDLE on buttonpads") that was backported to stable 5.16.x
> > > > kernel.
> > > >
> > > > The device is managed by hid-multitouch driver, and the further
> > > > investigation revealed that it's rather an incorrectly recognized
> > > > buttonpad property; namely, ID_DG_BUTTONTYPE reports it being 0 =
> > > > clickable touchpad although it's not.  I built a test kernel to ignore
> > > > this check and it was confirmed to make the right button working again
> > > > by the reporter.
> 
> Yep, I came to the same conclusion this morning with the reporter of
> the libinput bug José was mentioning.
> 
> > > >
> > > > Is this check really correct in general?  Or do we need some
> > > > device-specific quirk?
> 
> The device firmware is clearly wrong here. It's the first time I see
> this failing like that and I hope this is just an isolated case.
> The device advertises itself as a buttonpad, when it's not.
> 
> However, the fact that it passed MS certification (even if I doubt
> Microsoft ever got that touchpad in their own hands) leads me to
> believe that the certification doesn't enforce that setting too much,
> and that we might see more devices coming in with that same bug.
> 
> To sum up, I am not happy that this commit introduced a regression
> that we can not work around in userspace: given that BTN_RIGHT gets
> removed from the device, all of the values are filtered out and
> userspace can not resolve that situation by itself.
> 
> I wish I had a better way of fixing this than having to quirk the device.
> 
> > >
> > > 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.

Thanks for providing the right class Benjamin, I updated the patch:
https://gitlab.freedesktop.org/libinput/libinput/-/merge_requests/726#note_1303724
 
Jose

Re: [REGRESSION] Right touchpad button disabled on Dell 7750

[Takashi Iwai]

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,

Re: [REGRESSION] Right touchpad button disabled on Dell 7750

[José Expósito]

On Fri, Mar 18, 2022 at 03:37:20PM +0100, Takashi Iwai wrote:
> 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,


Yes, that is the correct patch. The original reporter just emailed me
and confirmed that the patch works and that the class used by the
device is MT_CLS_WIN_8, as Benjamin pointed out.

I'll wait a couple of days before sending the patch to the mailing list
so the other users can test it as well.

Thanks for the quick response,
Jose

Re: [PATCH] HID: multitouch: fix Dell Precision 7550 and 7750 button type

[kernel test robot]

Hi Takashi,

I love your patch! Yet something to improve:

[auto build test ERROR on hid/for-next]
[also build test ERROR on v5.17-rc8 next-20220318]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Takashi-Iwai/HID-multitouch-fix-Dell-Precision-7550-and-7750-button-type/20220318-223749
base:   https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
config: i386-randconfig-a002 (https://download.01.org/0day-ci/archive/20220319/202203190553.mMQsoOV3-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project a6e70e4056dff962ec634c5bd4f2f4105a0bef71)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/34d08d524d0942a3242bf820e364dc3f496dbd6c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Takashi-Iwai/HID-multitouch-fix-Dell-Precision-7550-and-7750-button-type/20220318-223749
        git checkout 34d08d524d0942a3242bf820e364dc3f496dbd6c
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/hid/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/hid/hid-multitouch.c:313:31: error: expected expression
                           MT_QUIRK_WIN8_PTP_BUTTONS |,
                                                      ^
   1 error generated.


vim +313 drivers/hid/hid-multitouch.c

   242	
   243	static const struct mt_class mt_classes[] = {
   244		{ .name = MT_CLS_DEFAULT,
   245			.quirks = MT_QUIRK_ALWAYS_VALID |
   246				MT_QUIRK_CONTACT_CNT_ACCURATE },
   247		{ .name = MT_CLS_NSMU,
   248			.quirks = MT_QUIRK_NOT_SEEN_MEANS_UP },
   249		{ .name = MT_CLS_SERIAL,
   250			.quirks = MT_QUIRK_ALWAYS_VALID},
   251		{ .name = MT_CLS_CONFIDENCE,
   252			.quirks = MT_QUIRK_VALID_IS_CONFIDENCE },
   253		{ .name = MT_CLS_CONFIDENCE_CONTACT_ID,
   254			.quirks = MT_QUIRK_VALID_IS_CONFIDENCE |
   255				MT_QUIRK_SLOT_IS_CONTACTID },
   256		{ .name = MT_CLS_CONFIDENCE_MINUS_ONE,
   257			.quirks = MT_QUIRK_VALID_IS_CONFIDENCE |
   258				MT_QUIRK_SLOT_IS_CONTACTID_MINUS_ONE },
   259		{ .name = MT_CLS_DUAL_INRANGE_CONTACTID,
   260			.quirks = MT_QUIRK_VALID_IS_INRANGE |
   261				MT_QUIRK_SLOT_IS_CONTACTID,
   262			.maxcontacts = 2 },
   263		{ .name = MT_CLS_DUAL_INRANGE_CONTACTNUMBER,
   264			.quirks = MT_QUIRK_VALID_IS_INRANGE |
   265				MT_QUIRK_SLOT_IS_CONTACTNUMBER,
   266			.maxcontacts = 2 },
   267		{ .name = MT_CLS_INRANGE_CONTACTNUMBER,
   268			.quirks = MT_QUIRK_VALID_IS_INRANGE |
   269				MT_QUIRK_SLOT_IS_CONTACTNUMBER },
   270		{ .name = MT_CLS_WIN_8,
   271			.quirks = MT_QUIRK_ALWAYS_VALID |
   272				MT_QUIRK_IGNORE_DUPLICATES |
   273				MT_QUIRK_HOVERING |
   274				MT_QUIRK_CONTACT_CNT_ACCURATE |
   275				MT_QUIRK_STICKY_FINGERS |
   276				MT_QUIRK_WIN8_PTP_BUTTONS,
   277			.export_all_inputs = true },
   278		{ .name = MT_CLS_EXPORT_ALL_INPUTS,
   279			.quirks = MT_QUIRK_ALWAYS_VALID |
   280				MT_QUIRK_CONTACT_CNT_ACCURATE,
   281			.export_all_inputs = true },
   282		{ .name = MT_CLS_WIN_8_FORCE_MULTI_INPUT,
   283			.quirks = MT_QUIRK_ALWAYS_VALID |
   284				MT_QUIRK_IGNORE_DUPLICATES |
   285				MT_QUIRK_HOVERING |
   286				MT_QUIRK_CONTACT_CNT_ACCURATE |
   287				MT_QUIRK_STICKY_FINGERS |
   288				MT_QUIRK_WIN8_PTP_BUTTONS |
   289				MT_QUIRK_FORCE_MULTI_INPUT,
   290			.export_all_inputs = true },
   291		{ .name = MT_CLS_WIN_8_DISABLE_WAKEUP,
   292			.quirks = MT_QUIRK_ALWAYS_VALID |
   293				MT_QUIRK_IGNORE_DUPLICATES |
   294				MT_QUIRK_HOVERING |
   295				MT_QUIRK_CONTACT_CNT_ACCURATE |
   296				MT_QUIRK_STICKY_FINGERS |
   297				MT_QUIRK_WIN8_PTP_BUTTONS |
   298				MT_QUIRK_DISABLE_WAKEUP,
   299			.export_all_inputs = true },
   300		{ .name = MT_CLS_WIN_8_NO_STICKY_FINGERS,
   301			.quirks = MT_QUIRK_ALWAYS_VALID |
   302				MT_QUIRK_IGNORE_DUPLICATES |
   303				MT_QUIRK_HOVERING |
   304				MT_QUIRK_CONTACT_CNT_ACCURATE |
   305				MT_QUIRK_WIN8_PTP_BUTTONS,
   306			.export_all_inputs = true },
   307		{ .name = MT_CLS_BUTTONTYPE_TOUCHPAD,
   308			.quirks = MT_QUIRK_ALWAYS_VALID |
   309				MT_QUIRK_IGNORE_DUPLICATES |
   310				MT_QUIRK_HOVERING |
   311				MT_QUIRK_CONTACT_CNT_ACCURATE |
   312				MT_QUIRK_STICKY_FINGERS |
 > 313				MT_QUIRK_WIN8_PTP_BUTTONS |,
   314				MT_QUIRK_BUTTONTYPE_TOUCHPAD,
   315			.export_all_inputs = true },
   316	
   317		/*
   318		 * vendor specific classes
   319		 */
   320		{ .name = MT_CLS_3M,
   321			.quirks = MT_QUIRK_VALID_IS_CONFIDENCE |
   322				MT_QUIRK_SLOT_IS_CONTACTID |
   323				MT_QUIRK_TOUCH_SIZE_SCALING,
   324			.sn_move = 2048,
   325			.sn_width = 128,
   326			.sn_height = 128,
   327			.maxcontacts = 60,
   328		},
   329		{ .name = MT_CLS_EGALAX,
   330			.quirks =  MT_QUIRK_SLOT_IS_CONTACTID |
   331				MT_QUIRK_VALID_IS_INRANGE,
   332			.sn_move = 4096,
   333			.sn_pressure = 32,
   334		},
   335		{ .name = MT_CLS_EGALAX_SERIAL,
   336			.quirks =  MT_QUIRK_SLOT_IS_CONTACTID |
   337				MT_QUIRK_ALWAYS_VALID,
   338			.sn_move = 4096,
   339			.sn_pressure = 32,
   340		},
   341		{ .name = MT_CLS_TOPSEED,
   342			.quirks = MT_QUIRK_ALWAYS_VALID,
   343			.is_indirect = true,
   344			.maxcontacts = 2,
   345		},
   346		{ .name = MT_CLS_PANASONIC,
   347			.quirks = MT_QUIRK_NOT_SEEN_MEANS_UP,
   348			.maxcontacts = 4 },
   349		{ .name	= MT_CLS_GENERALTOUCH_TWOFINGERS,
   350			.quirks	= MT_QUIRK_NOT_SEEN_MEANS_UP |
   351				MT_QUIRK_VALID_IS_INRANGE |
   352				MT_QUIRK_SLOT_IS_CONTACTID,
   353			.maxcontacts = 2
   354		},
   355		{ .name	= MT_CLS_GENERALTOUCH_PWT_TENFINGERS,
   356			.quirks	= MT_QUIRK_NOT_SEEN_MEANS_UP |
   357				MT_QUIRK_SLOT_IS_CONTACTID
   358		},
   359	
   360		{ .name = MT_CLS_FLATFROG,
   361			.quirks = MT_QUIRK_NOT_SEEN_MEANS_UP |
   362				MT_QUIRK_NO_AREA,
   363			.sn_move = 2048,
   364			.maxcontacts = 40,
   365		},
   366		{ .name = MT_CLS_LG,
   367			.quirks = MT_QUIRK_ALWAYS_VALID |
   368				MT_QUIRK_FIX_CONST_CONTACT_ID |
   369				MT_QUIRK_IGNORE_DUPLICATES |
   370				MT_QUIRK_HOVERING |
   371				MT_QUIRK_CONTACT_CNT_ACCURATE },
   372		{ .name = MT_CLS_ASUS,
   373			.quirks = MT_QUIRK_ALWAYS_VALID |
   374				MT_QUIRK_CONTACT_CNT_ACCURATE |
   375				MT_QUIRK_ASUS_CUSTOM_UP },
   376		{ .name = MT_CLS_VTL,
   377			.quirks = MT_QUIRK_ALWAYS_VALID |
   378				MT_QUIRK_CONTACT_CNT_ACCURATE |
   379				MT_QUIRK_FORCE_GET_FEATURE,
   380		},
   381		{ .name = MT_CLS_GOOGLE,
   382			.quirks = MT_QUIRK_ALWAYS_VALID |
   383				MT_QUIRK_CONTACT_CNT_ACCURATE |
   384				MT_QUIRK_SLOT_IS_CONTACTID |
   385				MT_QUIRK_HOVERING
   386		},
   387		{ .name = MT_CLS_RAZER_BLADE_STEALTH,
   388			.quirks = MT_QUIRK_ALWAYS_VALID |
   389				MT_QUIRK_IGNORE_DUPLICATES |
   390				MT_QUIRK_HOVERING |
   391				MT_QUIRK_CONTACT_CNT_ACCURATE |
   392				MT_QUIRK_WIN8_PTP_BUTTONS,
   393		},
   394		{ .name = MT_CLS_SMART_TECH,
   395			.quirks = MT_QUIRK_ALWAYS_VALID |
   396				MT_QUIRK_IGNORE_DUPLICATES |
   397				MT_QUIRK_CONTACT_CNT_ACCURATE |
   398				MT_QUIRK_SEPARATE_APP_REPORT,
   399		},
   400		{ }
   401	};
   402	

---
0-DAY CI Kernel Test Service
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [REGRESSION] Right touchpad button disabled on Dell 7750

[Takashi Iwai]

On Fri, 18 Mar 2022 15:06:55 +0100,
Benjamin Tissoires wrote:
> 
> 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.

Indeed the first patch caused the wrong button mapping.
Jose's revised patch was confirmed to work fine.


thanks,

Takashi

Re: [REGRESSION] Right touchpad button disabled on Dell 7750

[José Expósito]

On Sat, Mar 19, 2022 at 09:10:57AM +0100, Takashi Iwai wrote:
> Indeed the first patch caused the wrong button mapping.
> Jose's revised patch was confirmed to work fine.

Thanks for confirming that the patch was tested Takashi.

I just emailed it:
https://lore.kernel.org/linux-input/20220320190602.7484-1-jose.exposito89@gmail.com/

Jose

[PATCH] HID: multitouch: fix Dell Precision 7550 and 7750 button type

[José Expósito]

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

Fixes: 37ef4c19b4 ("Input: clear BTN_RIGHT/MIDDLE on buttonpads")
Signed-off-by: José Expósito <jose.exposito89@gmail.com>
---
 drivers/hid/hid-ids.h        |  3 +++
 drivers/hid/hid-multitouch.c | 20 ++++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 78bd3ddda442..6cf7a5b6835b 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -287,6 +287,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
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 99eabfb4145b..f012cf8e0b8c 100644
--- 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_device *td, struct mt_application *app);
 #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(struct hid_device *hdev,
 	    (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);
 
@@ -1872,6 +1886,12 @@ static const struct hid_device_id mt_devices[] = {
 		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,
-- 
2.25.1

Re: [PATCH] HID: multitouch: fix Dell Precision 7550 and 7750 button type

[Thorsten Leemhuis]

thx for working on this

On 20.03.22 20:06, José Expósito wrote:
> 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

Nitpicking: those should be "Link" tags, as explained in
'Documentation/process/submitting-patches.rst' and
'Documentation/process/5.Posting.rst'? Never tried, but if you want to
make them footnote-style this should work according to the docs:

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

In anyone wonders why I care: there are internal and publicly used tools
and scripts out there that reply on proper "Link" tags. I don't known
how many, but there is at least one public tool I'm running that cares:
regzbot, my regression tracking bot, which I use to track Linux kernel
regressions and generate the regression reports sent to Linus. Proper
"Link:" tags allow the bot to automatically connect regression reports
with fixes being posted or applied to resolve the particular regression
-- which makes regression tracking a whole lot easier and feasible for
the Linux kernel. That's why it's a great help for me if people set
proper "Link" tags.

While at it, let me tell regzbot about this thread:
#regzbot ^backmonitor:
https://linux-regtracking.leemhuis.info/regzbot/regression/s5htubv32s8.wl-tiwai@suse.de/


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

Ciao, Thorsten

Re: [PATCH] HID: multitouch: fix Dell Precision 7550 and 7750 button type

[Benjamin Tissoires]

Hi José,

On Sun, Mar 20, 2022 at 8:06 PM José Expósito <jose.exposito89@gmail.com> wrote:
>
> 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
>
> Fixes: 37ef4c19b4 ("Input: clear BTN_RIGHT/MIDDLE on buttonpads")
> Signed-off-by: José Expósito <jose.exposito89@gmail.com>
> ---

Thanks for the patch.
However, I'd like to put this one on hold for a bit. I am discussing
it right now with Peter and we are trying to see what are the possible
implications of starting to fix those in the kernel one by one.

So Jiri, please hold on before applying this one.

Cheers,
Benjamin

>  drivers/hid/hid-ids.h        |  3 +++
>  drivers/hid/hid-multitouch.c | 20 ++++++++++++++++++++
>  2 files changed, 23 insertions(+)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 78bd3ddda442..6cf7a5b6835b 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -287,6 +287,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
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 99eabfb4145b..f012cf8e0b8c 100644
> --- 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_device *td, struct mt_application *app);
>  #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(struct hid_device *hdev,
>             (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);
>
> @@ -1872,6 +1886,12 @@ static const struct hid_device_id mt_devices[] = {
>                 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,
> --
> 2.25.1
>

Re: [PATCH] HID: multitouch: fix Dell Precision 7550 and 7750 button type

[Benjamin Tissoires]

On Mon, Mar 21, 2022 at 10:25 AM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> Hi José,
>
> On Sun, Mar 20, 2022 at 8:06 PM José Expósito <jose.exposito89@gmail.com> wrote:
> >
> > 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
> >
> > Fixes: 37ef4c19b4 ("Input: clear BTN_RIGHT/MIDDLE on buttonpads")
> > Signed-off-by: José Expósito <jose.exposito89@gmail.com>
> > ---
>
> Thanks for the patch.
> However, I'd like to put this one on hold for a bit. I am discussing
> it right now with Peter and we are trying to see what are the possible
> implications of starting to fix those in the kernel one by one.
>
> So Jiri, please hold on before applying this one.

Giving a little bit more context here (and quoting Peter).

"""
The problem with [37ef4c19b4] is that it removes functionality -
before a clickpad was falsely advertised but the button worked, now in
the affected devices it simply no longer works because the button code
gets filtered. And user-space can't work around this.
...
So the main question remains: why are we doing this?

And the answer here is: because libinput can't handle clickpads with
right buttons. But that's not really true either, libinput just
doesn't want to, and for no other reason than that it's easier to
handle it this way.
"""

So basically, we tried to fix a choice on libinput assuming that all
devices are perfect, for the only sake of making it easy for libinput.
But the solution prevents further tweaks, and we then need to manually
quirk devices in the kernel which involves a slightly heavier
difficulty for end users than just dropping a config file or changing
a setting in their UI.

With that said, this patch is:
Nacked-by: me

José, could you send a revert of 37ef4c19b4, and add "Cc:
stable@vger.kernel.org" and all the other tags for the regression
tracker bot?

Thanks in advance.

Cheers,
Benjamin

>
> Cheers,
> Benjamin
>
> >  drivers/hid/hid-ids.h        |  3 +++
> >  drivers/hid/hid-multitouch.c | 20 ++++++++++++++++++++
> >  2 files changed, 23 insertions(+)
> >
> > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > index 78bd3ddda442..6cf7a5b6835b 100644
> > --- a/drivers/hid/hid-ids.h
> > +++ b/drivers/hid/hid-ids.h
> > @@ -287,6 +287,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
> > diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> > index 99eabfb4145b..f012cf8e0b8c 100644
> > --- 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_device *td, struct mt_application *app);
> >  #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(struct hid_device *hdev,
> >             (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);
> >
> > @@ -1872,6 +1886,12 @@ static const struct hid_device_id mt_devices[] = {
> >                 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,
> > --
> > 2.25.1
> >

Re: [PATCH] HID: multitouch: fix Dell Precision 7550 and 7750 button type

[José Expósito]

On Mon, Mar 21, 2022 at 07:22:45PM +0100, Benjamin Tissoires wrote:
> Giving a little bit more context here (and quoting Peter).
> 
> """
> The problem with [37ef4c19b4] is that it removes functionality -
> before a clickpad was falsely advertised but the button worked, now in
> the affected devices it simply no longer works because the button code
> gets filtered. And user-space can't work around this.
> ...
> So the main question remains: why are we doing this?
> 
> And the answer here is: because libinput can't handle clickpads with
> right buttons. But that's not really true either, libinput just
> doesn't want to, and for no other reason than that it's easier to
> handle it this way.
> """
> 
> So basically, we tried to fix a choice on libinput assuming that all
> devices are perfect, for the only sake of making it easy for libinput.
> But the solution prevents further tweaks, and we then need to manually
> quirk devices in the kernel which involves a slightly heavier
> difficulty for end users than just dropping a config file or changing
> a setting in their UI.
> 
> With that said, this patch is:
> Nacked-by: me
> 
> José, could you send a revert of 37ef4c19b4, and add "Cc:
> stable@vger.kernel.org" and all the other tags for the regression
> tracker bot?

Sure, for reference, this patch should be ignored in favor of:
https://lore.kernel.org/linux-input/20220321184404.20025-1-jose.exposito89@gmail.com/T/

Jose