Re: [PATCH v2 1/4] HID: multitouch: set INPUT_PROP_PRESSUREPAD based on Digitizer/Button Type

[Rong Zhang]

Hi Peter,

On Tue, 2026-06-16 at 14:24 +1000, Peter Hutterer wrote:
> Hi Rong,
> 
> On Tue, Jun 02, 2026 at 01:25:57AM +0800, Rong Zhang wrote:
> > 
> > Hi all,
> > 
> > Hopefully I'm not too late to show up here.
> > 
> > > From: Peter Hutterer <peter.hutterer@xxxxxxxxx>
> > > 
> > > A Digitizer/Button Type value of 1 indicates the device is a
> > > pressurepad, see
> > > https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/touchpad-windows-precision-touchpad-collection#device-capabilities-feature-report
> > > 
> > > Signed-off-by: Peter Hutterer <peter.hutterer@xxxxxxxxx>
> > > ---
> > >  drivers/hid/hid-multitouch.c | 12 +++++++++++-
> > >  1 file changed, 11 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> > > index 179dc316b4b518d78bdc900d9fd15756c5eba83e..382e6f50c4f7e663af7d028abb8be7cb2e6e7b8e 100644
> > > --- a/drivers/hid/hid-multitouch.c
> > > +++ b/drivers/hid/hid-multitouch.c
> > > @@ -81,6 +81,7 @@ MODULE_LICENSE("GPL");
> > >  #define MT_INPUTMODE_TOUCHPAD		0x03
> > >  
> > >  #define MT_BUTTONTYPE_CLICKPAD		0
> > > +#define MT_BUTTONTYPE_PRESSUREPAD	1
> > >  
> > >  enum latency_mode {
> > >  	HID_LATENCY_NORMAL = 0,
> > > @@ -179,6 +180,7 @@ struct mt_device {
> > >  	__u8 inputmode_value;	/* InputMode HID feature value */
> > >  	__u8 maxcontacts;
> > >  	bool is_buttonpad;	/* is this device a button pad? */
> > > +	bool is_pressurepad;	/* is this device a pressurepad? */
> > >  	bool is_haptic_touchpad;	/* is this device a haptic touchpad? */
> > >  	bool serial_maybe;	/* need to check for serial protocol */
> > >  
> > > @@ -530,8 +532,14 @@ static void mt_feature_mapping(struct hid_device *hdev,
> > >  		}
> > >  
> > >  		mt_get_feature(hdev, field->report);
> > > -		if (field->value[usage->usage_index] == MT_BUTTONTYPE_CLICKPAD)
> > > +		switch (field->value[usage->usage_index]) {
> > > +		case MT_BUTTONTYPE_CLICKPAD:
> > >  			td->is_buttonpad = true;
> > > +			break;
> > > +		case MT_BUTTONTYPE_PRESSUREPAD:
> > > +			td->is_pressurepad = true;
> > > +			break;
> > > +		}
> > >  
> > >  		break;
> > >  	case 0xff0000c5:
> > > @@ -1393,6 +1401,8 @@ static int mt_touch_input_configured(struct hid_device *hdev,
> > >  
> > >  	if (td->is_buttonpad)
> > >  		__set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
> > > +	if (td->is_pressurepad)
> > > +		__set_bit(INPUT_PROP_PRESSUREPAD, input->propbit);
> > 
> > I noticed that this leads to dual reporting on my device.
> > 
> > Consider previous checks:
> > 
> > 	if (application == HID_DG_TOUCHPAD) {
> > 		mt_application->mt_flags |= INPUT_MT_POINTER;
> > 		td->inputmode_value = MT_INPUTMODE_TOUCHPAD;
> > 	}
> > 
> > ...
> > 
> > 	/* check for clickpads */
> > 	if ((app->mt_flags & INPUT_MT_POINTER) &&
> > 	    (app->buttons_count == 1))
> > 		td->is_buttonpad = true;
> > 
> > ... where `td->is_buttonpad' is set to true when a pressure pad has only
> > one button, i.e., the "touchpad button integrated with digitizer" [1].
> > Most (if not all) pressure pads fall into this category. As a result,
> > the presence of INPUT_PROP_PRESSUREPAD is always accompanied by the
> > presence of INPUT_PROP_BUTTONPAD.
> 
> Yes, this is intended, see commit
> ae8966b7b5bd69b86209cc34bcca1ba9f18b68e6 which lists this in the commit
> message:
> 
> ```
>     This means:
>     - clickpad: INPUT_PROP_BUTTONPAD
>     - pressurepad: INPUT_PROP_BUTTONPAD + INPUT_PROP_PRESSUREPAD
>     - pressurepad with configurable haptics:
>       INPUT_PROP_BUTTONPAD + INPUT_PROP_PRESSUREPAD + FF_HAPTIC
> ```
> 
> We have to keep setting BUTTONPAD on all pressurepads because otherwise 
> we'd break existing userspace which relies on this.

Thanks a lot for your explanation. It makes sense to me.

Thanks,
Rong

> 
> Cheers,
>   Peter
>