All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] HID: multitouch: Add support for button type usage
@ 2015-02-27 21:00 Seth Forshee
  2015-03-01  3:34 ` [PATCH v2] " Seth Forshee
  0 siblings, 1 reply; 10+ messages in thread
From: Seth Forshee @ 2015-02-27 21:00 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: Seth Forshee, linux-input, linux-kernel

According to [1], Windows Precision Touchpad devices must supply
a button type usage in the device capabilities feature report. A
value of 0 indicates that the device contains a depressible
button (i.e. it's a click-pad) whereas a value of 1 indicates
a non-depressible button. Add support for this usage and set
INPUT_PROP_BUTTONPAD on the touchpad input device whenever a
depressible button is present.

[1] https://msdn.microsoft.com/en-us/library/windows/hardware/dn467314(v=vs.85).aspx

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/hid/hid-multitouch.c | 19 +++++++++++++++++++
 include/linux/hid.h          |  1 +
 2 files changed, 20 insertions(+)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index f65e78b..b19d721 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -72,6 +72,10 @@ MODULE_LICENSE("GPL");
 #define MT_INPUTMODE_TOUCHSCREEN	0x02
 #define MT_INPUTMODE_TOUCHPAD		0x03
 
+#define MT_BUTTONTYPE_DEPRESSIBLE	0
+#define MT_BUTTONTYPE_NONDEPRESSIBLE	1
+#define MT_BUTTONTYPE_MAX		MT_BUTTONTYPE_NONDEPRESSIBLE
+
 struct mt_slot {
 	__s32 x, y, cx, cy, p, w, h;
 	__s32 contactid;	/* the device ContactID assigned to this slot */
@@ -116,6 +120,7 @@ struct mt_device {
 	__u8 touches_by_report;	/* how many touches are present in one report:
 				* 1 means we should use a serial protocol
 				* > 1 means hybrid (multitouch) protocol */
+	__u8 buttontype;	/* depressible or non-depressible touchpad */
 	bool serial_maybe;	/* need to check for serial protocol */
 	bool curvalid;		/* is the current contact valid? */
 	unsigned mt_flags;	/* flags to pass to input-mt */
@@ -334,6 +339,16 @@ static void mt_feature_mapping(struct hid_device *hdev,
 			td->maxcontacts = td->mtclass.maxcontacts;
 
 		break;
+	case HID_DG_BUTTONTYPE:
+		if (usage->usage_index >= field->report_count) {
+			dev_err(&hdev->dev, "HID_DG_BUTTONTYPE out of range\n");
+			break;
+		}
+
+		if (field->value[usage->usage_index] <= MT_BUTTONTYPE_MAX)
+			td->buttontype = field->value[usage->usage_index];
+
+		break;
 	}
 }
 
@@ -728,6 +743,9 @@ static void mt_touch_input_configured(struct hid_device *hdev,
 	if (cls->quirks & MT_QUIRK_NOT_SEEN_MEANS_UP)
 		td->mt_flags |= INPUT_MT_DROP_UNUSED;
 
+	if (td->buttontype == MT_BUTTONTYPE_DEPRESSIBLE)
+		__set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
+
 	input_mt_init_slots(input, td->maxcontacts, td->mt_flags);
 
 	td->mt_flags = 0;
@@ -1009,6 +1027,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	td->inputmode_value = MT_INPUTMODE_TOUCHSCREEN;
 	td->cc_index = -1;
 	td->mt_report_id = -1;
+	td->buttontype = MT_BUTTONTYPE_NONDEPRESSIBLE;
 	hid_set_drvdata(hdev, td);
 
 	td->fields = devm_kzalloc(&hdev->dev, sizeof(struct mt_fields),
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 06c4607..498ddad 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -269,6 +269,7 @@ struct hid_item {
 #define HID_DG_DEVICEINDEX	0x000d0053
 #define HID_DG_CONTACTCOUNT	0x000d0054
 #define HID_DG_CONTACTMAX	0x000d0055
+#define HID_DG_BUTTONTYPE	0x000d0059
 #define HID_DG_BARRELSWITCH2	0x000d005a
 #define HID_DG_TOOLSERIALNUMBER	0x000d005b
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2] HID: multitouch: Add support for button type usage
  2015-02-27 21:00 [PATCH] HID: multitouch: Add support for button type usage Seth Forshee
@ 2015-03-01  3:34 ` Seth Forshee
  2015-03-11 16:19   ` Jiri Kosina
  0 siblings, 1 reply; 10+ messages in thread
From: Seth Forshee @ 2015-03-01  3:34 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: Seth Forshee, linux-input, linux-kernel

According to [1], Windows Precision Touchpad devices must supply
a button type usage in the device capabilities feature report. A
value of 0 indicates that the device contains a depressible
button (i.e. it's a click-pad) whereas a value of 1 indicates
a non-depressible button. Add support for this usage and set
INPUT_PROP_BUTTONPAD on the touchpad input device whenever a
depressible button is present.

v2: Add string for button type usage in debugfs.

[1] https://msdn.microsoft.com/en-us/library/windows/hardware/dn467314(v=vs.85).aspx

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/hid/hid-debug.c      |  1 +
 drivers/hid/hid-multitouch.c | 19 +++++++++++++++++++
 include/linux/hid.h          |  1 +
 3 files changed, 21 insertions(+)

diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
index 8bf61d2..4b2a18a 100644
--- a/drivers/hid/hid-debug.c
+++ b/drivers/hid/hid-debug.c
@@ -165,6 +165,7 @@ static const struct hid_usage_entry hid_usage_table[] = {
     {0, 0x53, "DeviceIndex"},
     {0, 0x54, "ContactCount"},
     {0, 0x55, "ContactMaximumNumber"},
+    {0, 0x59, "ButtonType"},
     {0, 0x5A, "SecondaryBarrelSwitch"},
     {0, 0x5B, "TransducerSerialNumber"},
   { 15, 0, "PhysicalInterfaceDevice" },
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index f65e78b..b19d721 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -72,6 +72,10 @@ MODULE_LICENSE("GPL");
 #define MT_INPUTMODE_TOUCHSCREEN	0x02
 #define MT_INPUTMODE_TOUCHPAD		0x03
 
+#define MT_BUTTONTYPE_DEPRESSIBLE	0
+#define MT_BUTTONTYPE_NONDEPRESSIBLE	1
+#define MT_BUTTONTYPE_MAX		MT_BUTTONTYPE_NONDEPRESSIBLE
+
 struct mt_slot {
 	__s32 x, y, cx, cy, p, w, h;
 	__s32 contactid;	/* the device ContactID assigned to this slot */
@@ -116,6 +120,7 @@ struct mt_device {
 	__u8 touches_by_report;	/* how many touches are present in one report:
 				* 1 means we should use a serial protocol
 				* > 1 means hybrid (multitouch) protocol */
+	__u8 buttontype;	/* depressible or non-depressible touchpad */
 	bool serial_maybe;	/* need to check for serial protocol */
 	bool curvalid;		/* is the current contact valid? */
 	unsigned mt_flags;	/* flags to pass to input-mt */
@@ -334,6 +339,16 @@ static void mt_feature_mapping(struct hid_device *hdev,
 			td->maxcontacts = td->mtclass.maxcontacts;
 
 		break;
+	case HID_DG_BUTTONTYPE:
+		if (usage->usage_index >= field->report_count) {
+			dev_err(&hdev->dev, "HID_DG_BUTTONTYPE out of range\n");
+			break;
+		}
+
+		if (field->value[usage->usage_index] <= MT_BUTTONTYPE_MAX)
+			td->buttontype = field->value[usage->usage_index];
+
+		break;
 	}
 }
 
@@ -728,6 +743,9 @@ static void mt_touch_input_configured(struct hid_device *hdev,
 	if (cls->quirks & MT_QUIRK_NOT_SEEN_MEANS_UP)
 		td->mt_flags |= INPUT_MT_DROP_UNUSED;
 
+	if (td->buttontype == MT_BUTTONTYPE_DEPRESSIBLE)
+		__set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
+
 	input_mt_init_slots(input, td->maxcontacts, td->mt_flags);
 
 	td->mt_flags = 0;
@@ -1009,6 +1027,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	td->inputmode_value = MT_INPUTMODE_TOUCHSCREEN;
 	td->cc_index = -1;
 	td->mt_report_id = -1;
+	td->buttontype = MT_BUTTONTYPE_NONDEPRESSIBLE;
 	hid_set_drvdata(hdev, td);
 
 	td->fields = devm_kzalloc(&hdev->dev, sizeof(struct mt_fields),
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 06c4607..498ddad 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -269,6 +269,7 @@ struct hid_item {
 #define HID_DG_DEVICEINDEX	0x000d0053
 #define HID_DG_CONTACTCOUNT	0x000d0054
 #define HID_DG_CONTACTMAX	0x000d0055
+#define HID_DG_BUTTONTYPE	0x000d0059
 #define HID_DG_BARRELSWITCH2	0x000d005a
 #define HID_DG_TOOLSERIALNUMBER	0x000d005b
 
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] HID: multitouch: Add support for button type usage
  2015-03-01  3:34 ` [PATCH v2] " Seth Forshee
@ 2015-03-11 16:19   ` Jiri Kosina
  2015-03-11 17:25     ` Seth Forshee
  0 siblings, 1 reply; 10+ messages in thread
From: Jiri Kosina @ 2015-03-11 16:19 UTC (permalink / raw)
  To: Seth Forshee; +Cc: linux-input, linux-kernel, Benjamin Tissoires

On Sat, 28 Feb 2015, Seth Forshee wrote:

> According to [1], Windows Precision Touchpad devices must supply
> a button type usage in the device capabilities feature report. A
> value of 0 indicates that the device contains a depressible
> button (i.e. it's a click-pad) whereas a value of 1 indicates
> a non-depressible button. Add support for this usage and set
> INPUT_PROP_BUTTONPAD on the touchpad input device whenever a
> depressible button is present.
> 
> v2: Add string for button type usage in debugfs.
> 
> [1] https://msdn.microsoft.com/en-us/library/windows/hardware/dn467314(v=vs.85).aspx
> 
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>

I'd like to have Benjamin's Ack on this. Adding him to CC. Please also see 
015fdaa9f8ed ("HID: multitouch: add support of clickpads").

> ---
>  drivers/hid/hid-debug.c      |  1 +
>  drivers/hid/hid-multitouch.c | 19 +++++++++++++++++++
>  include/linux/hid.h          |  1 +
>  3 files changed, 21 insertions(+)
> 
> diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
> index 8bf61d2..4b2a18a 100644
> --- a/drivers/hid/hid-debug.c
> +++ b/drivers/hid/hid-debug.c
> @@ -165,6 +165,7 @@ static const struct hid_usage_entry hid_usage_table[] = {
>      {0, 0x53, "DeviceIndex"},
>      {0, 0x54, "ContactCount"},
>      {0, 0x55, "ContactMaximumNumber"},
> +    {0, 0x59, "ButtonType"},
>      {0, 0x5A, "SecondaryBarrelSwitch"},
>      {0, 0x5B, "TransducerSerialNumber"},
>    { 15, 0, "PhysicalInterfaceDevice" },
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index f65e78b..b19d721 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -72,6 +72,10 @@ MODULE_LICENSE("GPL");
>  #define MT_INPUTMODE_TOUCHSCREEN	0x02
>  #define MT_INPUTMODE_TOUCHPAD		0x03
>  
> +#define MT_BUTTONTYPE_DEPRESSIBLE	0
> +#define MT_BUTTONTYPE_NONDEPRESSIBLE	1
> +#define MT_BUTTONTYPE_MAX		MT_BUTTONTYPE_NONDEPRESSIBLE
> +
>  struct mt_slot {
>  	__s32 x, y, cx, cy, p, w, h;
>  	__s32 contactid;	/* the device ContactID assigned to this slot */
> @@ -116,6 +120,7 @@ struct mt_device {
>  	__u8 touches_by_report;	/* how many touches are present in one report:
>  				* 1 means we should use a serial protocol
>  				* > 1 means hybrid (multitouch) protocol */
> +	__u8 buttontype;	/* depressible or non-depressible touchpad */
>  	bool serial_maybe;	/* need to check for serial protocol */
>  	bool curvalid;		/* is the current contact valid? */
>  	unsigned mt_flags;	/* flags to pass to input-mt */
> @@ -334,6 +339,16 @@ static void mt_feature_mapping(struct hid_device *hdev,
>  			td->maxcontacts = td->mtclass.maxcontacts;
>  
>  		break;
> +	case HID_DG_BUTTONTYPE:
> +		if (usage->usage_index >= field->report_count) {
> +			dev_err(&hdev->dev, "HID_DG_BUTTONTYPE out of range\n");
> +			break;
> +		}
> +
> +		if (field->value[usage->usage_index] <= MT_BUTTONTYPE_MAX)
> +			td->buttontype = field->value[usage->usage_index];
> +
> +		break;
>  	}
>  }
>  
> @@ -728,6 +743,9 @@ static void mt_touch_input_configured(struct hid_device *hdev,
>  	if (cls->quirks & MT_QUIRK_NOT_SEEN_MEANS_UP)
>  		td->mt_flags |= INPUT_MT_DROP_UNUSED;
>  
> +	if (td->buttontype == MT_BUTTONTYPE_DEPRESSIBLE)
> +		__set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
> +
>  	input_mt_init_slots(input, td->maxcontacts, td->mt_flags);
>  
>  	td->mt_flags = 0;
> @@ -1009,6 +1027,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  	td->inputmode_value = MT_INPUTMODE_TOUCHSCREEN;
>  	td->cc_index = -1;
>  	td->mt_report_id = -1;
> +	td->buttontype = MT_BUTTONTYPE_NONDEPRESSIBLE;
>  	hid_set_drvdata(hdev, td);
>  
>  	td->fields = devm_kzalloc(&hdev->dev, sizeof(struct mt_fields),
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index 06c4607..498ddad 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -269,6 +269,7 @@ struct hid_item {
>  #define HID_DG_DEVICEINDEX	0x000d0053
>  #define HID_DG_CONTACTCOUNT	0x000d0054
>  #define HID_DG_CONTACTMAX	0x000d0055
> +#define HID_DG_BUTTONTYPE	0x000d0059
>  #define HID_DG_BARRELSWITCH2	0x000d005a
>  #define HID_DG_TOOLSERIALNUMBER	0x000d005b
>  
> -- 
> 2.1.4
> 

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] HID: multitouch: Add support for button type usage
  2015-03-11 16:19   ` Jiri Kosina
@ 2015-03-11 17:25     ` Seth Forshee
  2015-03-11 17:29       ` [PATCH v3] " Seth Forshee
  0 siblings, 1 reply; 10+ messages in thread
From: Seth Forshee @ 2015-03-11 17:25 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: linux-input, linux-kernel, Benjamin Tissoires

On Wed, Mar 11, 2015 at 12:19:13PM -0400, Jiri Kosina wrote:
> On Sat, 28 Feb 2015, Seth Forshee wrote:
> 
> > According to [1], Windows Precision Touchpad devices must supply
> > a button type usage in the device capabilities feature report. A
> > value of 0 indicates that the device contains a depressible
> > button (i.e. it's a click-pad) whereas a value of 1 indicates
> > a non-depressible button. Add support for this usage and set
> > INPUT_PROP_BUTTONPAD on the touchpad input device whenever a
> > depressible button is present.
> > 
> > v2: Add string for button type usage in debugfs.
> > 
> > [1] https://msdn.microsoft.com/en-us/library/windows/hardware/dn467314(v=vs.85).aspx
> > 
> > Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> 
> I'd like to have Benjamin's Ack on this. Adding him to CC. Please also see 
> 015fdaa9f8ed ("HID: multitouch: add support of clickpads").

That commit does also gets INPUT_PROP_BUTTONPAD set for my touchpad.
I'll leave it up to the two of you to decide whether or not there's
benefit in also using the usage to set the property bit. I'll also
follow up with a v3 patch which resolves the conflicts with Benjamin's
patch.

Also, I should probably mention that I don't have any non-depressible
touchpads which set the button type usage that I can test with. If
anyone has such hardware it would be great to get this tested against
it.

Thanks,
Seth

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3] HID: multitouch: Add support for button type usage
  2015-03-11 17:25     ` Seth Forshee
@ 2015-03-11 17:29       ` Seth Forshee
  2015-03-11 20:55         ` Benjamin Tissoires
  0 siblings, 1 reply; 10+ messages in thread
From: Seth Forshee @ 2015-03-11 17:29 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: linux-input, linux-kernel, Benjamin Tissoires, Seth Forshee

According to [1], Windows Precision Touchpad devices must supply
a button type usage in the device capabilities feature report. A
value of 0 indicates that the device contains a depressible
button (i.e. it's a click-pad) whereas a value of 1 indicates
a non-depressible button. Add support for this usage and set
INPUT_PROP_BUTTONPAD on the touchpad input device whenever a
depressible button is present.

v2: Add string for button type usage in debugfs.
v3: Fix conflicts with 015fdaa9f8ed ("HID: multitouch: add
    support of clickpads").

[1] https://msdn.microsoft.com/en-us/library/windows/hardware/dn467314(v=vs.85).aspx

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/hid/hid-debug.c      |  1 +
 drivers/hid/hid-multitouch.c | 19 +++++++++++++++++++
 include/linux/hid.h          |  1 +
 3 files changed, 21 insertions(+)

diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
index 8bf61d295ffd..4b2a18a8b7ec 100644
--- a/drivers/hid/hid-debug.c
+++ b/drivers/hid/hid-debug.c
@@ -165,6 +165,7 @@ static const struct hid_usage_entry hid_usage_table[] = {
     {0, 0x53, "DeviceIndex"},
     {0, 0x54, "ContactCount"},
     {0, 0x55, "ContactMaximumNumber"},
+    {0, 0x59, "ButtonType"},
     {0, 0x5A, "SecondaryBarrelSwitch"},
     {0, 0x5B, "TransducerSerialNumber"},
   { 15, 0, "PhysicalInterfaceDevice" },
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index a8bec33f4764..2e5bb788c0eb 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -71,6 +71,10 @@ MODULE_LICENSE("GPL");
 #define MT_INPUTMODE_TOUCHSCREEN	0x02
 #define MT_INPUTMODE_TOUCHPAD		0x03
 
+#define MT_BUTTONTYPE_DEPRESSIBLE	0
+#define MT_BUTTONTYPE_NONDEPRESSIBLE	1
+#define MT_BUTTONTYPE_MAX		MT_BUTTONTYPE_NONDEPRESSIBLE
+
 struct mt_slot {
 	__s32 x, y, cx, cy, p, w, h;
 	__s32 contactid;	/* the device ContactID assigned to this slot */
@@ -116,6 +120,7 @@ struct mt_device {
 				* 1 means we should use a serial protocol
 				* > 1 means hybrid (multitouch) protocol */
 	__u8 buttons_count;	/* number of physical buttons per touchpad */
+	__u8 buttontype;	/* depressible or non-depressible touchpad */
 	bool serial_maybe;	/* need to check for serial protocol */
 	bool curvalid;		/* is the current contact valid? */
 	unsigned mt_flags;	/* flags to pass to input-mt */
@@ -334,6 +339,16 @@ static void mt_feature_mapping(struct hid_device *hdev,
 			td->maxcontacts = td->mtclass.maxcontacts;
 
 		break;
+	case HID_DG_BUTTONTYPE:
+		if (usage->usage_index >= field->report_count) {
+			dev_err(&hdev->dev, "HID_DG_BUTTONTYPE out of range\n");
+			break;
+		}
+
+		if (field->value[usage->usage_index] <= MT_BUTTONTYPE_MAX)
+			td->buttontype = field->value[usage->usage_index];
+
+		break;
 	}
 }
 
@@ -734,6 +749,9 @@ static void mt_touch_input_configured(struct hid_device *hdev,
 
 	/* check for clickpads */
 	if ((td->mt_flags & INPUT_MT_POINTER) && (td->buttons_count == 1))
+		td->buttontype = MT_BUTTONTYPE_DEPRESSIBLE;
+
+	if (td->buttontype == MT_BUTTONTYPE_DEPRESSIBLE)
 		__set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
 
 	input_mt_init_slots(input, td->maxcontacts, td->mt_flags);
@@ -1017,6 +1035,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	td->inputmode_value = MT_INPUTMODE_TOUCHSCREEN;
 	td->cc_index = -1;
 	td->mt_report_id = -1;
+	td->buttontype = MT_BUTTONTYPE_NONDEPRESSIBLE;
 	hid_set_drvdata(hdev, td);
 
 	td->fields = devm_kzalloc(&hdev->dev, sizeof(struct mt_fields),
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 69f9cf7f078d..0167e0e0bf88 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -270,6 +270,7 @@ struct hid_item {
 #define HID_DG_DEVICEINDEX	0x000d0053
 #define HID_DG_CONTACTCOUNT	0x000d0054
 #define HID_DG_CONTACTMAX	0x000d0055
+#define HID_DG_BUTTONTYPE	0x000d0059
 #define HID_DG_BARRELSWITCH2	0x000d005a
 #define HID_DG_TOOLSERIALNUMBER	0x000d005b
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] HID: multitouch: Add support for button type usage
  2015-03-11 17:29       ` [PATCH v3] " Seth Forshee
@ 2015-03-11 20:55         ` Benjamin Tissoires
  2015-03-11 21:55           ` Seth Forshee
  0 siblings, 1 reply; 10+ messages in thread
From: Benjamin Tissoires @ 2015-03-11 20:55 UTC (permalink / raw)
  To: Seth Forshee; +Cc: Jiri Kosina, linux-input, linux-kernel

Hi Seth,

On Mar 11 2015 or thereabouts, Seth Forshee wrote:
> According to [1], Windows Precision Touchpad devices must supply
> a button type usage in the device capabilities feature report. A
> value of 0 indicates that the device contains a depressible
> button (i.e. it's a click-pad) whereas a value of 1 indicates
> a non-depressible button. Add support for this usage and set
> INPUT_PROP_BUTTONPAD on the touchpad input device whenever a
> depressible button is present.
> 
> v2: Add string for button type usage in debugfs.
> v3: Fix conflicts with 015fdaa9f8ed ("HID: multitouch: add
>     support of clickpads").
> 
> [1] https://msdn.microsoft.com/en-us/library/windows/hardware/dn467314(v=vs.85).aspx
> 
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> ---

[Replying to the previous thread here for convenience]

Sorry I might have overlooked your patch and forgot about it when I sent
mine. I should definitively have looked at this in detail because that's
a better approach IMO.
However, the patch I sent allows to handle clickpads which are not
following MS spec, so I think your v3 on top of mine makes sense.

I have a few bike-shedding if you don't mind:

>  drivers/hid/hid-debug.c      |  1 +
>  drivers/hid/hid-multitouch.c | 19 +++++++++++++++++++
>  include/linux/hid.h          |  1 +
>  3 files changed, 21 insertions(+)
> 
> diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
> index 8bf61d295ffd..4b2a18a8b7ec 100644
> --- a/drivers/hid/hid-debug.c
> +++ b/drivers/hid/hid-debug.c
> @@ -165,6 +165,7 @@ static const struct hid_usage_entry hid_usage_table[] = {
>      {0, 0x53, "DeviceIndex"},
>      {0, 0x54, "ContactCount"},
>      {0, 0x55, "ContactMaximumNumber"},
> +    {0, 0x59, "ButtonType"},
>      {0, 0x5A, "SecondaryBarrelSwitch"},
>      {0, 0x5B, "TransducerSerialNumber"},
>    { 15, 0, "PhysicalInterfaceDevice" },
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index a8bec33f4764..2e5bb788c0eb 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -71,6 +71,10 @@ MODULE_LICENSE("GPL");
>  #define MT_INPUTMODE_TOUCHSCREEN	0x02
>  #define MT_INPUTMODE_TOUCHPAD		0x03
>  
> +#define MT_BUTTONTYPE_DEPRESSIBLE	0
> +#define MT_BUTTONTYPE_NONDEPRESSIBLE	1
> +#define MT_BUTTONTYPE_MAX		MT_BUTTONTYPE_NONDEPRESSIBLE

As mentioned later, just use CLICKPAD and not DEPRESSIBLE.
Also, please use '0x' prefixed values. It does not matter now but I like
having all things which deals with hardware in the hexa form.

> +
>  struct mt_slot {
>  	__s32 x, y, cx, cy, p, w, h;
>  	__s32 contactid;	/* the device ContactID assigned to this slot */
> @@ -116,6 +120,7 @@ struct mt_device {
>  				* 1 means we should use a serial protocol
>  				* > 1 means hybrid (multitouch) protocol */
>  	__u8 buttons_count;	/* number of physical buttons per touchpad */
> +	__u8 buttontype;	/* depressible or non-depressible touchpad */

In the kernel we use button pad. On the user space more likely clickpad,
so I would rather not adding a third denomination for these touchpads.

Also, I would prefer having a is_buttonpad boolean instead of a
buttontype.

This would allow to get rid of MT_BUTTONTYPE_NONDEPRESSIBLE and
MT_BUTTONTYPE_MAX in one shot.

>  	bool serial_maybe;	/* need to check for serial protocol */
>  	bool curvalid;		/* is the current contact valid? */
>  	unsigned mt_flags;	/* flags to pass to input-mt */
> @@ -334,6 +339,16 @@ static void mt_feature_mapping(struct hid_device *hdev,
>  			td->maxcontacts = td->mtclass.maxcontacts;
>  
>  		break;
> +	case HID_DG_BUTTONTYPE:
> +		if (usage->usage_index >= field->report_count) {
> +			dev_err(&hdev->dev, "HID_DG_BUTTONTYPE out of range\n");
> +			break;
> +		}
> +
> +		if (field->value[usage->usage_index] <= MT_BUTTONTYPE_MAX)
> +			td->buttontype = field->value[usage->usage_index];

This can then be replaced by:
		td->is_buttonpad = field->value[usage->usage_index] == MT_BUTTONTYPE_CLICKPAD;

> +
> +		break;
>  	}
>  }
>  
> @@ -734,6 +749,9 @@ static void mt_touch_input_configured(struct hid_device *hdev,
>  
>  	/* check for clickpads */
>  	if ((td->mt_flags & INPUT_MT_POINTER) && (td->buttons_count == 1))
> +		td->buttontype = MT_BUTTONTYPE_DEPRESSIBLE;
> +
> +	if (td->buttontype == MT_BUTTONTYPE_DEPRESSIBLE)
>  		__set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
>  
>  	input_mt_init_slots(input, td->maxcontacts, td->mt_flags);
> @@ -1017,6 +1035,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  	td->inputmode_value = MT_INPUTMODE_TOUCHSCREEN;
>  	td->cc_index = -1;
>  	td->mt_report_id = -1;
> +	td->buttontype = MT_BUTTONTYPE_NONDEPRESSIBLE;

with the boolean, no need to init it to a special value.

Cheers,
Benjamin

>  	hid_set_drvdata(hdev, td);
>  
>  	td->fields = devm_kzalloc(&hdev->dev, sizeof(struct mt_fields),
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index 69f9cf7f078d..0167e0e0bf88 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -270,6 +270,7 @@ struct hid_item {
>  #define HID_DG_DEVICEINDEX	0x000d0053
>  #define HID_DG_CONTACTCOUNT	0x000d0054
>  #define HID_DG_CONTACTMAX	0x000d0055
> +#define HID_DG_BUTTONTYPE	0x000d0059
>  #define HID_DG_BARRELSWITCH2	0x000d005a
>  #define HID_DG_TOOLSERIALNUMBER	0x000d005b
>  
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] HID: multitouch: Add support for button type usage
  2015-03-11 20:55         ` Benjamin Tissoires
@ 2015-03-11 21:55           ` Seth Forshee
  2015-03-11 22:26             ` [PATCH v4] " Seth Forshee
  0 siblings, 1 reply; 10+ messages in thread
From: Seth Forshee @ 2015-03-11 21:55 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Jiri Kosina, linux-input, linux-kernel

On Wed, Mar 11, 2015 at 04:55:59PM -0400, Benjamin Tissoires wrote:
> Hi Seth,
> 
> On Mar 11 2015 or thereabouts, Seth Forshee wrote:
> > According to [1], Windows Precision Touchpad devices must supply
> > a button type usage in the device capabilities feature report. A
> > value of 0 indicates that the device contains a depressible
> > button (i.e. it's a click-pad) whereas a value of 1 indicates
> > a non-depressible button. Add support for this usage and set
> > INPUT_PROP_BUTTONPAD on the touchpad input device whenever a
> > depressible button is present.
> > 
> > v2: Add string for button type usage in debugfs.
> > v3: Fix conflicts with 015fdaa9f8ed ("HID: multitouch: add
> >     support of clickpads").
> > 
> > [1] https://msdn.microsoft.com/en-us/library/windows/hardware/dn467314(v=vs.85).aspx
> > 
> > Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> > ---
> 
> [Replying to the previous thread here for convenience]
> 
> Sorry I might have overlooked your patch and forgot about it when I sent
> mine. I should definitively have looked at this in detail because that's
> a better approach IMO.
> However, the patch I sent allows to handle clickpads which are not
> following MS spec, so I think your v3 on top of mine makes sense.
> 
> I have a few bike-shedding if you don't mind:

Sure, I opted to maintain the terminology from the MS spec but I have no
problem doing it that way. I'll follow up with another patch once I get
it tested.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v4] HID: multitouch: Add support for button type usage
  2015-03-11 21:55           ` Seth Forshee
@ 2015-03-11 22:26             ` Seth Forshee
  2015-03-12  1:13               ` Benjamin Tissoires
  0 siblings, 1 reply; 10+ messages in thread
From: Seth Forshee @ 2015-03-11 22:26 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: linux-input, linux-kernel, Benjamin Tissoires, Seth Forshee

According to [1], Windows Precision Touchpad devices must supply
a button type usage in the device capabilities feature report. A
value of 0 indicates that the device contains a depressible
button (i.e. it's a click-pad) whereas a value of 1 indicates
a non-depressible button. Add support for this usage and set
INPUT_PROP_BUTTONPAD on the touchpad input device whenever a
depressible button is present.

[1] https://msdn.microsoft.com/en-us/library/windows/hardware/dn467314(v=vs.85).aspx

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/hid/hid-debug.c      |  1 +
 drivers/hid/hid-multitouch.c | 16 ++++++++++++++++
 include/linux/hid.h          |  1 +
 3 files changed, 18 insertions(+)

diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
index 8bf61d295ffd..4b2a18a8b7ec 100644
--- a/drivers/hid/hid-debug.c
+++ b/drivers/hid/hid-debug.c
@@ -165,6 +165,7 @@ static const struct hid_usage_entry hid_usage_table[] = {
     {0, 0x53, "DeviceIndex"},
     {0, 0x54, "ContactCount"},
     {0, 0x55, "ContactMaximumNumber"},
+    {0, 0x59, "ButtonType"},
     {0, 0x5A, "SecondaryBarrelSwitch"},
     {0, 0x5B, "TransducerSerialNumber"},
   { 15, 0, "PhysicalInterfaceDevice" },
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index a8bec33f4764..6a9b05b328a9 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -71,6 +71,8 @@ MODULE_LICENSE("GPL");
 #define MT_INPUTMODE_TOUCHSCREEN	0x02
 #define MT_INPUTMODE_TOUCHPAD		0x03
 
+#define MT_BUTTONTYPE_CLICKPAD		0
+
 struct mt_slot {
 	__s32 x, y, cx, cy, p, w, h;
 	__s32 contactid;	/* the device ContactID assigned to this slot */
@@ -116,6 +118,7 @@ struct mt_device {
 				* 1 means we should use a serial protocol
 				* > 1 means hybrid (multitouch) protocol */
 	__u8 buttons_count;	/* number of physical buttons per touchpad */
+	bool is_buttonpad;	/* is this device a button pad? */
 	bool serial_maybe;	/* need to check for serial protocol */
 	bool curvalid;		/* is the current contact valid? */
 	unsigned mt_flags;	/* flags to pass to input-mt */
@@ -334,6 +337,16 @@ static void mt_feature_mapping(struct hid_device *hdev,
 			td->maxcontacts = td->mtclass.maxcontacts;
 
 		break;
+	case HID_DG_BUTTONTYPE:
+		if (usage->usage_index >= field->report_count) {
+			dev_err(&hdev->dev, "HID_DG_BUTTONTYPE out of range\n");
+			break;
+		}
+
+		if (field->value[usage->usage_index] == MT_BUTTONTYPE_CLICKPAD)
+			td->is_buttonpad = true;
+
+		break;
 	}
 }
 
@@ -734,6 +747,9 @@ static void mt_touch_input_configured(struct hid_device *hdev,
 
 	/* check for clickpads */
 	if ((td->mt_flags & INPUT_MT_POINTER) && (td->buttons_count == 1))
+		td->is_buttonpad = true;
+
+	if (td->is_buttonpad)
 		__set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
 
 	input_mt_init_slots(input, td->maxcontacts, td->mt_flags);
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 69f9cf7f078d..0167e0e0bf88 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -270,6 +270,7 @@ struct hid_item {
 #define HID_DG_DEVICEINDEX	0x000d0053
 #define HID_DG_CONTACTCOUNT	0x000d0054
 #define HID_DG_CONTACTMAX	0x000d0055
+#define HID_DG_BUTTONTYPE	0x000d0059
 #define HID_DG_BARRELSWITCH2	0x000d005a
 #define HID_DG_TOOLSERIALNUMBER	0x000d005b
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v4] HID: multitouch: Add support for button type usage
  2015-03-11 22:26             ` [PATCH v4] " Seth Forshee
@ 2015-03-12  1:13               ` Benjamin Tissoires
  2015-03-12  4:06                 ` Jiri Kosina
  0 siblings, 1 reply; 10+ messages in thread
From: Benjamin Tissoires @ 2015-03-12  1:13 UTC (permalink / raw)
  To: Seth Forshee; +Cc: Jiri Kosina, linux-input, linux-kernel

On Mar 11 2015 or thereabouts, Seth Forshee wrote:
> According to [1], Windows Precision Touchpad devices must supply
> a button type usage in the device capabilities feature report. A
> value of 0 indicates that the device contains a depressible
> button (i.e. it's a click-pad) whereas a value of 1 indicates
> a non-depressible button. Add support for this usage and set
> INPUT_PROP_BUTTONPAD on the touchpad input device whenever a
> depressible button is present.
> 
> [1] https://msdn.microsoft.com/en-us/library/windows/hardware/dn467314(v=vs.85).aspx
> 
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> ---

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

Thanks for the quick re-spin

Cheers,
Benjamin

>  drivers/hid/hid-debug.c      |  1 +
>  drivers/hid/hid-multitouch.c | 16 ++++++++++++++++
>  include/linux/hid.h          |  1 +
>  3 files changed, 18 insertions(+)
> 
> diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
> index 8bf61d295ffd..4b2a18a8b7ec 100644
> --- a/drivers/hid/hid-debug.c
> +++ b/drivers/hid/hid-debug.c
> @@ -165,6 +165,7 @@ static const struct hid_usage_entry hid_usage_table[] = {
>      {0, 0x53, "DeviceIndex"},
>      {0, 0x54, "ContactCount"},
>      {0, 0x55, "ContactMaximumNumber"},
> +    {0, 0x59, "ButtonType"},
>      {0, 0x5A, "SecondaryBarrelSwitch"},
>      {0, 0x5B, "TransducerSerialNumber"},
>    { 15, 0, "PhysicalInterfaceDevice" },
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index a8bec33f4764..6a9b05b328a9 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -71,6 +71,8 @@ MODULE_LICENSE("GPL");
>  #define MT_INPUTMODE_TOUCHSCREEN	0x02
>  #define MT_INPUTMODE_TOUCHPAD		0x03
>  
> +#define MT_BUTTONTYPE_CLICKPAD		0
> +
>  struct mt_slot {
>  	__s32 x, y, cx, cy, p, w, h;
>  	__s32 contactid;	/* the device ContactID assigned to this slot */
> @@ -116,6 +118,7 @@ struct mt_device {
>  				* 1 means we should use a serial protocol
>  				* > 1 means hybrid (multitouch) protocol */
>  	__u8 buttons_count;	/* number of physical buttons per touchpad */
> +	bool is_buttonpad;	/* is this device a button pad? */
>  	bool serial_maybe;	/* need to check for serial protocol */
>  	bool curvalid;		/* is the current contact valid? */
>  	unsigned mt_flags;	/* flags to pass to input-mt */
> @@ -334,6 +337,16 @@ static void mt_feature_mapping(struct hid_device *hdev,
>  			td->maxcontacts = td->mtclass.maxcontacts;
>  
>  		break;
> +	case HID_DG_BUTTONTYPE:
> +		if (usage->usage_index >= field->report_count) {
> +			dev_err(&hdev->dev, "HID_DG_BUTTONTYPE out of range\n");
> +			break;
> +		}
> +
> +		if (field->value[usage->usage_index] == MT_BUTTONTYPE_CLICKPAD)
> +			td->is_buttonpad = true;
> +
> +		break;
>  	}
>  }
>  
> @@ -734,6 +747,9 @@ static void mt_touch_input_configured(struct hid_device *hdev,
>  
>  	/* check for clickpads */
>  	if ((td->mt_flags & INPUT_MT_POINTER) && (td->buttons_count == 1))
> +		td->is_buttonpad = true;
> +
> +	if (td->is_buttonpad)
>  		__set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
>  
>  	input_mt_init_slots(input, td->maxcontacts, td->mt_flags);
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index 69f9cf7f078d..0167e0e0bf88 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -270,6 +270,7 @@ struct hid_item {
>  #define HID_DG_DEVICEINDEX	0x000d0053
>  #define HID_DG_CONTACTCOUNT	0x000d0054
>  #define HID_DG_CONTACTMAX	0x000d0055
> +#define HID_DG_BUTTONTYPE	0x000d0059
>  #define HID_DG_BARRELSWITCH2	0x000d005a
>  #define HID_DG_TOOLSERIALNUMBER	0x000d005b
>  
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4] HID: multitouch: Add support for button type usage
  2015-03-12  1:13               ` Benjamin Tissoires
@ 2015-03-12  4:06                 ` Jiri Kosina
  0 siblings, 0 replies; 10+ messages in thread
From: Jiri Kosina @ 2015-03-12  4:06 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Seth Forshee, linux-input, linux-kernel

On Wed, 11 Mar 2015, Benjamin Tissoires wrote:

> On Mar 11 2015 or thereabouts, Seth Forshee wrote:
> > According to [1], Windows Precision Touchpad devices must supply
> > a button type usage in the device capabilities feature report. A
> > value of 0 indicates that the device contains a depressible
> > button (i.e. it's a click-pad) whereas a value of 1 indicates
> > a non-depressible button. Add support for this usage and set
> > INPUT_PROP_BUTTONPAD on the touchpad input device whenever a
> > depressible button is present.
> > 
> > [1] https://msdn.microsoft.com/en-us/library/windows/hardware/dn467314(v=vs.85).aspx
> > 
> > Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> > ---
> 
> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> 
> Thanks for the quick re-spin

Applied to for-4.1/multitouch.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-03-12  4:06 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-27 21:00 [PATCH] HID: multitouch: Add support for button type usage Seth Forshee
2015-03-01  3:34 ` [PATCH v2] " Seth Forshee
2015-03-11 16:19   ` Jiri Kosina
2015-03-11 17:25     ` Seth Forshee
2015-03-11 17:29       ` [PATCH v3] " Seth Forshee
2015-03-11 20:55         ` Benjamin Tissoires
2015-03-11 21:55           ` Seth Forshee
2015-03-11 22:26             ` [PATCH v4] " Seth Forshee
2015-03-12  1:13               ` Benjamin Tissoires
2015-03-12  4:06                 ` Jiri Kosina

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.