NACK/cmnt: [PATCH 1/2] HID: multitouch: Support PTP Stick and Touchpad device

[Shrirang Bagul]

On Wed, 2017-08-23 at 16:17 +0200, Kleber Souza wrote:
> On 08/23/17 09:18, Shrirang Bagul wrote:
> > From: Masaki Ota <masaki.ota at jp.alps.com>
> > 
> > BugLink:  https://bugs.launchpad.net/bugs/1712481
> > 
> > Support PTP Stick and Touchpad device. This Touchpad is Precision Touchpad
> > (PTP), and Stick Pointer data is the same as Mouse; Stick Pointer works as
> > Mouse.
> > 
> > [jkosina at suse.cz: changelog deuglification]
> > Signed-off-by: Masaki Ota <masaki.ota at jp.alps.com>
> > Reviewed-by: Benjamin Tissoires <benjamin.tissoires at redhat.com>
> > Signed-off-by: Jiri Kosina <jkosina at suse.cz>
> > 
> > (backported from commit 504c932c7c47a6b4c572302a13873f7d83af1ff3)
> > Signed-off-by: Shrirang Bagul <shrirang.bagul at canonical.com>
> > ---
> >  drivers/hid/hid-ids.h        |  2 ++
> >  drivers/hid/hid-multitouch.c | 29 ++++++++++++++++++++++++++++-
> >  2 files changed, 30 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > index a1772a796946..fd1cd912e25f 100644
> > --- a/drivers/hid/hid-ids.h
> > +++ b/drivers/hid/hid-ids.h
> > @@ -69,6 +69,8 @@
> >  
> >  #define USB_VENDOR_ID_ALPS_JP		0x044E
> >  #define HID_DEVICE_ID_ALPS_U1_DUAL	0x120B
> > +#define HID_DEVICE_ID_ALPS_U1_DUAL_PTP	0x121F
> > +#define HID_DEVICE_ID_ALPS_U1_DUAL_3BTN_PTP	0x1220
> >  
> >  #define USB_VENDOR_ID_AMI		0x046b
> >  #define USB_DEVICE_ID_AMI_VIRT_KEYBOARD_AND_MOUSE	0xff10
> > diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> > index f62a9d6601cc..f501b06844fc 100644
> > --- a/drivers/hid/hid-multitouch.c
> > +++ b/drivers/hid/hid-multitouch.c
> > @@ -145,6 +145,7 @@ static void mt_post_parse(struct mt_device *td);
> >  /* reserved					0x0011 */
> >  #define MT_CLS_WIN_8				0x0012
> >  #define MT_CLS_EXPORT_ALL_INPUTS		0x0013
> > +#define MT_CLS_WIN_8_DUAL			0x0014
> >  
> >  /* vendor specific classes */
> >  #define MT_CLS_3M				0x0101
> > @@ -213,6 +214,12 @@ static struct mt_class mt_classes[] = {
> >  		.quirks = MT_QUIRK_ALWAYS_VALID |
> >  			MT_QUIRK_CONTACT_CNT_ACCURATE,
> >  		.export_all_inputs = true },
> > +	{ .name = MT_CLS_WIN_8_DUAL,
> > +		.quirks = MT_QUIRK_ALWAYS_VALID |
> > +			MT_QUIRK_IGNORE_DUPLICATES |
> > +			MT_QUIRK_HOVERING |
> > +			MT_QUIRK_CONTACT_CNT_ACCURATE,
> > +		.export_all_inputs = true },
> >  
> >  	/*
> >  	 * vendor specific classes
> > @@ -504,7 +511,8 @@ static int mt_touch_input_mapping(struct hid_device *hdev,
> > struct hid_input *hi,
> >  			mt_store_field(usage, td, hi);
> >  			return 1;
> >  		case HID_DG_CONFIDENCE:
> > -			if (cls->name == MT_CLS_WIN_8 &&
> > +			if ((cls->name == MT_CLS_WIN_8 ||
> > +				cls->name == MT_CLS_WIN_8_DUAL) &&
> >  				field->application == HID_DG_TOUCHPAD)
> >  				cls->quirks |= MT_QUIRK_CONFIDENCE;
> >  			mt_store_field(usage, td, hi);
> > @@ -567,6 +575,15 @@ static int mt_touch_input_mapping(struct hid_device *hdev,
> > struct hid_input *hi,
> >  
> >  	case HID_UP_BUTTON:
> >  		code = BTN_MOUSE + ((usage->hid - 1) & HID_USAGE);
> > +		/*
> > +		 * MS PTP spec says that external buttons left and right have
> > +		 * usages 2 and 3.
> > +		 */
> > +		if ((cls->name == MT_CLS_WIN_8 ||
> > +			cls->name == MT_CLS_WIN_8_DUAL) &&
> > +		    field->application == HID_DG_TOUCHPAD &&
> > +		    (usage->hid & HID_USAGE) > 1)
> > +			code--;
> 
> Hi Shrirang,
> 
> Part of the above chunk was introduced by
> 594312b88b0f451912c964c7ff2c0eaa71ad41b4 (HID: multitouch: handle
> external buttons for Precision Touchpads). Since it's quite small and
> the backport is straightforward, we should have it backported as well as
> part of the prereq for this patch instead of silently merging the two of
> them.

Sent v2 to to address this comment.

Thanks,
Shrirang
> 
> 
> Thanks,
> Kleber
> 
> >  		hid_map_usage(hi, usage, bit, max, EV_KEY, code);
> >  		input_set_capability(hi->input, EV_KEY, code);
> >  		return 1;
> > @@ -1201,6 +1218,16 @@ static const struct hid_device_id mt_devices[] = {
> >  		MT_USB_DEVICE(USB_VENDOR_ID_3M,
> >  			USB_DEVICE_ID_3M3266) },
> >  
> > +	/* Alps devices */
> > +	{ .driver_data = MT_CLS_WIN_8_DUAL,
> > +		HID_DEVICE(BUS_I2C, HID_GROUP_MULTITOUCH_WIN_8,
> > +			USB_VENDOR_ID_ALPS_JP,
> > +			HID_DEVICE_ID_ALPS_U1_DUAL_PTP) },
> > +	{ .driver_data = MT_CLS_WIN_8_DUAL,
> > +		HID_DEVICE(BUS_I2C, HID_GROUP_MULTITOUCH_WIN_8,
> > +			USB_VENDOR_ID_ALPS_JP,
> > +			HID_DEVICE_ID_ALPS_U1_DUAL_3BTN_PTP) },
> > +
> >  	/* Anton devices */
> >  	{ .driver_data = MT_CLS_EXPORT_ALL_INPUTS,
> >  		MT_USB_DEVICE(USB_VENDOR_ID_ANTON,
> >