[PATCH 0/5] HID: wacom: introduce generic HID handling

[PATCH 0/5] HID: wacom: introduce generic HID handling

[Benjamin Tissoires]

Hi guys,

So, this patch series aims at supporting natively any future HID compliant wacom
tablet. Those found on the various laptops (ISDv4/5) already are HID compliant
and they should work in the future without any modification of the kernel.

Few things to note here:
- I used the PID 0x00E6 found on the Lenovo X230 for the tests
- I did not removed its entry in the list because there is a slightly different
  behavior while using this patch set: when more than two fingers are on the
  screen, the tablet stops sending finger events, while with the wacom
  proprietary bits, it continues to send them. Besides that, the events emitted
  before and after the patch series are the same (at least on the E6)
- I can not rely on hid-input directly because wacom wants to be in control of
  the input devices it creates. This might be solved in the future, but for now
  it is just easier to rewrite the few mapping/events handling than trying to
  fit in the hid-input model.
- there will still be more specific use cases to handle later (see the joke of
  the MS surface 3 pen[1] for instance), but this should give a roughly working
  pen/touch support for those future devices.

Jason, I would be very glad if you could conduct a few tests for this on the
ISDv4/5 sensors you have.

Cheers,
Benjamin

[1] http://who-t.blogspot.com/2014/09/stylus-behaviour-on-microsoft-surface-3.html

Benjamin Tissoires (5):
  HID: wacom: rename failN with some meaningful information
  HID: wacom: split out input allocation and registration
  HID: wacom: move allocation of inputs earlier
  HID: wacom: implement generic HID handling for pen generic devices
  HID: wacom: implement the finger part of the HID generic handling

 drivers/hid/hid-core.c  |   3 +
 drivers/hid/wacom.h     |   6 +
 drivers/hid/wacom_sys.c | 184 +++++++++++++++++++++--------
 drivers/hid/wacom_wac.c | 299 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/hid/wacom_wac.h |  17 +++
 include/linux/hid.h     |   2 +
 6 files changed, 462 insertions(+), 49 deletions(-)

-- 
2.1.0

[PATCH 1/5] HID: wacom: rename failN with some meaningful information

[Benjamin Tissoires]

When we have to deal with new elements in probe, having the exit labels
named sequencially is a pain to maintain. Put a meaningful name instead
so that we do not have to renumber them on inserts.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/wacom_sys.c | 49 +++++++++++++++++++++++++++++--------------------
 1 file changed, 29 insertions(+), 20 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 2508628..97e1fef 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1075,7 +1075,7 @@ static int wacom_register_inputs(struct wacom *wacom)
 	pad_input_dev = wacom_allocate_input(wacom);
 	if (!input_dev || !pad_input_dev) {
 		error = -ENOMEM;
-		goto fail1;
+		goto fail_allocate_input;
 	}
 
 	wacom_wac->input = input_dev;
@@ -1084,11 +1084,11 @@ static int wacom_register_inputs(struct wacom *wacom)
 
 	error = wacom_setup_input_capabilities(input_dev, wacom_wac);
 	if (error)
-		goto fail2;
+		goto fail_input_cap;
 
 	error = input_register_device(input_dev);
 	if (error)
-		goto fail2;
+		goto fail_register_input;
 
 	error = wacom_setup_pad_input_capabilities(pad_input_dev, wacom_wac);
 	if (error) {
@@ -1099,25 +1099,26 @@ static int wacom_register_inputs(struct wacom *wacom)
 	} else {
 		error = input_register_device(pad_input_dev);
 		if (error)
-			goto fail3;
+			goto fail_register_pad_input;
 
 		error = wacom_initialize_leds(wacom);
 		if (error)
-			goto fail4;
+			goto fail_leds;
 	}
 
 	return 0;
 
-fail4:
+fail_leds:
 	input_unregister_device(pad_input_dev);
 	pad_input_dev = NULL;
-fail3:
+fail_register_pad_input:
 	input_unregister_device(input_dev);
 	input_dev = NULL;
-fail2:
+fail_register_input:
+fail_input_cap:
 	wacom_wac->input = NULL;
 	wacom_wac->pad_input = NULL;
-fail1:
+fail_allocate_input:
 	if (input_dev)
 		input_free_device(input_dev);
 	if (pad_input_dev)
@@ -1305,7 +1306,7 @@ static int wacom_probe(struct hid_device *hdev,
 	error = hid_parse(hdev);
 	if (error) {
 		hid_err(hdev, "parse failed\n");
-		goto fail1;
+		goto fail_parse;
 	}
 
 	wacom_wac = &wacom->wacom_wac;
@@ -1314,12 +1315,12 @@ static int wacom_probe(struct hid_device *hdev,
 	features->pktlen = wacom_compute_pktlen(hdev);
 	if (features->pktlen > WACOM_PKGLEN_MAX) {
 		error = -EINVAL;
-		goto fail1;
+		goto fail_pktlen;
 	}
 
 	if (features->check_for_hid_type && features->hid_type != hdev->type) {
 		error = -ENODEV;
-		goto fail1;
+		goto fail_type;
 	}
 
 	wacom->usbdev = dev;
@@ -1388,20 +1389,20 @@ static int wacom_probe(struct hid_device *hdev,
 
 		error = wacom_add_shared_data(hdev);
 		if (error)
-			goto fail1;
+			goto fail_shared_data;
 	}
 
 	if (!(features->quirks & WACOM_QUIRK_MONITOR) &&
 	     (features->quirks & WACOM_QUIRK_BATTERY)) {
 		error = wacom_initialize_battery(wacom);
 		if (error)
-			goto fail2;
+			goto fail_battery;
 	}
 
 	if (!(features->quirks & WACOM_QUIRK_NO_INPUT)) {
 		error = wacom_register_inputs(wacom);
 		if (error)
-			goto fail3;
+			goto fail_register_inputs;
 	}
 
 	if (hdev->bus == BUS_BLUETOOTH) {
@@ -1419,7 +1420,7 @@ static int wacom_probe(struct hid_device *hdev,
 	error = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
 	if (error) {
 		hid_err(hdev, "hw start failed\n");
-		goto fail4;
+		goto fail_hw_start;
 	}
 
 	if (features->quirks & WACOM_QUIRK_MONITOR)
@@ -1432,12 +1433,20 @@ static int wacom_probe(struct hid_device *hdev,
 
 	return 0;
 
- fail4:	if (hdev->bus == BUS_BLUETOOTH)
+fail_hw_start:
+	wacom_unregister_inputs(wacom);
+	if (hdev->bus == BUS_BLUETOOTH)
 		device_remove_file(&hdev->dev, &dev_attr_speed);
+fail_register_inputs:
 	wacom_unregister_inputs(wacom);
- fail3:	wacom_destroy_battery(wacom);
- fail2:	wacom_remove_shared_data(wacom_wac);
- fail1:	kfree(wacom);
+	wacom_destroy_battery(wacom);
+fail_battery:
+	wacom_remove_shared_data(wacom_wac);
+fail_shared_data:
+fail_type:
+fail_pktlen:
+fail_parse:
+	kfree(wacom);
 	hid_set_drvdata(hdev, NULL);
 	return error;
 }
-- 
2.1.0

[PATCH 2/5] HID: wacom: split out input allocation and registration

[Benjamin Tissoires]

If the input can be created earlier during probe, we can already populate
them while reading the report descriptor. This way, we can rely on the
hid subsystem directly for tablets which already provide a meaningful
report descriptor (like ISDv4-5).

This patch only splits the allocation and registration, but do not
change where we allocate the input. This will come in a later patch.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/wacom_sys.c | 96 ++++++++++++++++++++++++++++++++-----------------
 drivers/hid/wacom_wac.h |  1 +
 2 files changed, 64 insertions(+), 33 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 97e1fef..62f3c89 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1054,41 +1054,75 @@ static struct input_dev *wacom_allocate_input(struct wacom *wacom)
 	return input_dev;
 }
 
-static void wacom_unregister_inputs(struct wacom *wacom)
+static void wacom_free_inputs(struct wacom *wacom)
 {
-	if (wacom->wacom_wac.input)
-		input_unregister_device(wacom->wacom_wac.input);
-	if (wacom->wacom_wac.pad_input)
-		input_unregister_device(wacom->wacom_wac.pad_input);
-	wacom->wacom_wac.input = NULL;
-	wacom->wacom_wac.pad_input = NULL;
-	wacom_destroy_leds(wacom);
+	struct wacom_wac *wacom_wac = &(wacom->wacom_wac);
+
+	if (wacom_wac->input)
+		input_free_device(wacom_wac->input);
+	if (wacom_wac->pad_input)
+		input_free_device(wacom_wac->pad_input);
+	wacom_wac->input = NULL;
+	wacom_wac->pad_input = NULL;
 }
 
-static int wacom_register_inputs(struct wacom *wacom)
+static int wacom_allocate_inputs(struct wacom *wacom)
 {
 	struct input_dev *input_dev, *pad_input_dev;
 	struct wacom_wac *wacom_wac = &(wacom->wacom_wac);
-	int error;
 
 	input_dev = wacom_allocate_input(wacom);
 	pad_input_dev = wacom_allocate_input(wacom);
 	if (!input_dev || !pad_input_dev) {
-		error = -ENOMEM;
-		goto fail_allocate_input;
+		wacom_free_inputs(wacom);
+		return -ENOMEM;
 	}
 
 	wacom_wac->input = input_dev;
 	wacom_wac->pad_input = pad_input_dev;
 	wacom_wac->pad_input->name = wacom_wac->pad_name;
 
+	return 0;
+}
+
+static void wacom_clean_inputs(struct wacom *wacom)
+{
+	if (wacom->wacom_wac.input) {
+		if (wacom->wacom_wac.input_registered)
+			input_unregister_device(wacom->wacom_wac.input);
+		else
+			input_free_device(wacom->wacom_wac.input);
+	}
+	if (wacom->wacom_wac.pad_input) {
+		if (wacom->wacom_wac.input_registered)
+			input_unregister_device(wacom->wacom_wac.pad_input);
+		else
+			input_free_device(wacom->wacom_wac.pad_input);
+	}
+	wacom->wacom_wac.input = NULL;
+	wacom->wacom_wac.pad_input = NULL;
+	wacom_destroy_leds(wacom);
+}
+
+static int wacom_register_inputs(struct wacom *wacom)
+{
+	struct input_dev *input_dev, *pad_input_dev;
+	struct wacom_wac *wacom_wac = &(wacom->wacom_wac);
+	int error;
+
+	input_dev = wacom_wac->input;
+	pad_input_dev = wacom_wac->pad_input;
+
+	if (!input_dev || !pad_input_dev)
+		return -EINVAL;
+
 	error = wacom_setup_input_capabilities(input_dev, wacom_wac);
 	if (error)
-		goto fail_input_cap;
+		return error;
 
 	error = input_register_device(input_dev);
 	if (error)
-		goto fail_register_input;
+		return error;
 
 	error = wacom_setup_pad_input_capabilities(pad_input_dev, wacom_wac);
 	if (error) {
@@ -1106,6 +1140,8 @@ static int wacom_register_inputs(struct wacom *wacom)
 			goto fail_leds;
 	}
 
+	wacom_wac->input_registered = true;
+
 	return 0;
 
 fail_leds:
@@ -1113,16 +1149,7 @@ fail_leds:
 	pad_input_dev = NULL;
 fail_register_pad_input:
 	input_unregister_device(input_dev);
-	input_dev = NULL;
-fail_register_input:
-fail_input_cap:
 	wacom_wac->input = NULL;
-	wacom_wac->pad_input = NULL;
-fail_allocate_input:
-	if (input_dev)
-		input_free_device(input_dev);
-	if (pad_input_dev)
-		input_free_device(pad_input_dev);
 	return error;
 }
 
@@ -1147,13 +1174,13 @@ static void wacom_wireless_work(struct work_struct *work)
 	hdev1 = usb_get_intfdata(usbdev->config->interface[1]);
 	wacom1 = hid_get_drvdata(hdev1);
 	wacom_wac1 = &(wacom1->wacom_wac);
-	wacom_unregister_inputs(wacom1);
+	wacom_clean_inputs(wacom1);
 
 	/* Touch interface */
 	hdev2 = usb_get_intfdata(usbdev->config->interface[2]);
 	wacom2 = hid_get_drvdata(hdev2);
 	wacom_wac2 = &(wacom2->wacom_wac);
-	wacom_unregister_inputs(wacom2);
+	wacom_clean_inputs(wacom2);
 
 	if (wacom_wac->pid == 0) {
 		hid_info(wacom->hdev, "wireless tablet disconnected\n");
@@ -1187,7 +1214,8 @@ static void wacom_wireless_work(struct work_struct *work)
 		wacom_wac1->shared->touch_max = wacom_wac1->features.touch_max;
 		wacom_wac1->shared->type = wacom_wac1->features.type;
 		wacom_wac1->pid = wacom_wac->pid;
-		error = wacom_register_inputs(wacom1);
+		error = wacom_allocate_inputs(wacom1) ||
+			wacom_register_inputs(wacom1);
 		if (error)
 			goto fail;
 
@@ -1208,7 +1236,8 @@ static void wacom_wireless_work(struct work_struct *work)
 			snprintf(wacom_wac2->pad_name, WACOM_NAME_MAX,
 				 "%s (WL) Pad", wacom_wac2->features.name);
 			wacom_wac2->pid = wacom_wac->pid;
-			error = wacom_register_inputs(wacom2);
+			error = wacom_allocate_inputs(wacom2) ||
+				wacom_register_inputs(wacom2);
 			if (error)
 				goto fail;
 
@@ -1225,8 +1254,8 @@ static void wacom_wireless_work(struct work_struct *work)
 	return;
 
 fail:
-	wacom_unregister_inputs(wacom1);
-	wacom_unregister_inputs(wacom2);
+	wacom_clean_inputs(wacom1);
+	wacom_clean_inputs(wacom2);
 	return;
 }
 
@@ -1400,7 +1429,8 @@ static int wacom_probe(struct hid_device *hdev,
 	}
 
 	if (!(features->quirks & WACOM_QUIRK_NO_INPUT)) {
-		error = wacom_register_inputs(wacom);
+		error = wacom_allocate_inputs(wacom) ||
+			wacom_register_inputs(wacom);
 		if (error)
 			goto fail_register_inputs;
 	}
@@ -1434,11 +1464,11 @@ static int wacom_probe(struct hid_device *hdev,
 	return 0;
 
 fail_hw_start:
-	wacom_unregister_inputs(wacom);
+	wacom_clean_inputs(wacom);
 	if (hdev->bus == BUS_BLUETOOTH)
 		device_remove_file(&hdev->dev, &dev_attr_speed);
 fail_register_inputs:
-	wacom_unregister_inputs(wacom);
+	wacom_clean_inputs(wacom);
 	wacom_destroy_battery(wacom);
 fail_battery:
 	wacom_remove_shared_data(wacom_wac);
@@ -1458,7 +1488,7 @@ static void wacom_remove(struct hid_device *hdev)
 	hid_hw_stop(hdev);
 
 	cancel_work_sync(&wacom->work);
-	wacom_unregister_inputs(wacom);
+	wacom_clean_inputs(wacom);
 	if (hdev->bus == BUS_BLUETOOTH)
 		device_remove_file(&hdev->dev, &dev_attr_speed);
 	wacom_destroy_battery(wacom);
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 339ab5d..72f9ca8 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -167,6 +167,7 @@ struct wacom_wac {
 	struct wacom_shared *shared;
 	struct input_dev *input;
 	struct input_dev *pad_input;
+	bool input_registered;
 	int pid;
 	int battery_capacity;
 	int num_contacts_left;
-- 
2.1.0

[PATCH 3/5] HID: wacom: move allocation of inputs earlier

[Benjamin Tissoires]

This allows to have the input devices ready in while parsing the reports
descriptor.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/wacom_sys.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 62f3c89..21ac2ba 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1357,6 +1357,12 @@ static int wacom_probe(struct hid_device *hdev,
 	mutex_init(&wacom->lock);
 	INIT_WORK(&wacom->work, wacom_wireless_work);
 
+	if (!(features->quirks & WACOM_QUIRK_NO_INPUT)) {
+		error = wacom_allocate_inputs(wacom);
+		if (error)
+			goto fail_allocate_inputs;
+	}
+
 	/* set the default size in case we do not get them from hid */
 	wacom_set_default_phy(features);
 
@@ -1429,8 +1435,7 @@ static int wacom_probe(struct hid_device *hdev,
 	}
 
 	if (!(features->quirks & WACOM_QUIRK_NO_INPUT)) {
-		error = wacom_allocate_inputs(wacom) ||
-			wacom_register_inputs(wacom);
+		error = wacom_register_inputs(wacom);
 		if (error)
 			goto fail_register_inputs;
 	}
@@ -1464,7 +1469,6 @@ static int wacom_probe(struct hid_device *hdev,
 	return 0;
 
 fail_hw_start:
-	wacom_clean_inputs(wacom);
 	if (hdev->bus == BUS_BLUETOOTH)
 		device_remove_file(&hdev->dev, &dev_attr_speed);
 fail_register_inputs:
@@ -1473,6 +1477,8 @@ fail_register_inputs:
 fail_battery:
 	wacom_remove_shared_data(wacom_wac);
 fail_shared_data:
+	wacom_clean_inputs(wacom);
+fail_allocate_inputs:
 fail_type:
 fail_pktlen:
 fail_parse:
-- 
2.1.0

[PATCH 4/5] HID: wacom: implement generic HID handling for pen generic devices

[Benjamin Tissoires]

ISDv4 and v5 are plain HID devices. We can directly implement a generic
HID parsing/handling and remove the need to manually add those PID in
the list of supported devices.

This patch implements the pen support only. The finger part will come in
a later patch.

To be properly notified of an .event() and a .report(), we need to force
hid-core to go through the HID parsing. By default, wacom.ko binds only
hidraw, so the hid parsing is not done by hid-core. When a true HID device
is there, we add the flag HID_CLAIMED_DRIVER to hid->claimed which will
force hid-core to parse the incoming reports.
(Note that this can be easily backported by directly setting the .claimed
flag to HID_CLAIMED_DRIVER even if hid-core does not support
HID_CONNECT_DRIVER)

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-core.c  |   3 +
 drivers/hid/wacom.h     |   6 ++
 drivers/hid/wacom_sys.c |  12 +++-
 drivers/hid/wacom_wac.c | 179 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/hid/wacom_wac.h |   8 +++
 include/linux/hid.h     |   2 +
 6 files changed, 208 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index eb50818..73bd9e2 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1591,6 +1591,9 @@ int hid_connect(struct hid_device *hdev, unsigned int connect_mask)
 	if ((connect_mask & HID_CONNECT_HIDRAW) && !hidraw_connect(hdev))
 		hdev->claimed |= HID_CLAIMED_HIDRAW;
 
+	if (connect_mask & HID_CONNECT_DRIVER)
+		hdev->claimed |= HID_CLAIMED_DRIVER;
+
 	/* Drivers with the ->raw_event callback set are not required to connect
 	 * to any other listener. */
 	if (!hdev->claimed && !hdev->driver->raw_event) {
diff --git a/drivers/hid/wacom.h b/drivers/hid/wacom.h
index 64bc1b2..0cc5344 100644
--- a/drivers/hid/wacom.h
+++ b/drivers/hid/wacom.h
@@ -89,6 +89,7 @@
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
+#include <linux/hid.h>
 #include <linux/usb/input.h>
 #include <linux/power_supply.h>
 #include <asm/unaligned.h>
@@ -143,4 +144,9 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
 				   struct wacom_wac *wacom_wac);
 int wacom_setup_pad_input_capabilities(struct input_dev *input_dev,
 				       struct wacom_wac *wacom_wac);
+void wacom_wac_usage_mapping(struct hid_device *hdev,
+		struct hid_field *field, struct hid_usage *usage);
+int wacom_wac_event(struct hid_device *hdev, struct hid_field *field,
+		struct hid_usage *usage, __s32 value);
+void wacom_wac_report(struct hid_device *hdev, struct hid_report *report);
 #endif
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 21ac2ba..dd288b2 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -13,7 +13,6 @@
 
 #include "wacom_wac.h"
 #include "wacom.h"
-#include <linux/hid.h>
 
 #define WAC_MSG_RETRIES		5
 
@@ -215,6 +214,9 @@ static void wacom_usage_mapping(struct hid_device *hdev,
 			features->pressure_max = field->logical_maximum;
 		break;
 	}
+
+	if (features->type == HID_GENERIC)
+		wacom_wac_usage_mapping(hdev, field, usage);
 }
 
 static void wacom_parse_hid(struct hid_device *hdev,
@@ -1318,6 +1320,7 @@ static int wacom_probe(struct hid_device *hdev,
 	struct wacom_wac *wacom_wac;
 	struct wacom_features *features;
 	int error;
+	unsigned int connect_mask = HID_CONNECT_HIDRAW;
 
 	if (!id->driver_data)
 		return -EINVAL;
@@ -1451,8 +1454,11 @@ static int wacom_probe(struct hid_device *hdev,
 	/* Note that if query fails it is not a hard failure */
 	wacom_query_tablet_data(hdev, features);
 
+	if (features->type == HID_GENERIC)
+		connect_mask |= HID_CONNECT_DRIVER;
+
 	/* Regular HID work starts now */
-	error = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
+	error = hid_hw_start(hdev, connect_mask);
 	if (error) {
 		hid_err(hdev, "hw start failed\n");
 		goto fail_hw_start;
@@ -1532,6 +1538,8 @@ static struct hid_driver wacom_driver = {
 	.id_table =	wacom_ids,
 	.probe =	wacom_probe,
 	.remove =	wacom_remove,
+	.event =	wacom_wac_event,
+	.report =	wacom_wac_report,
 #ifdef CONFIG_PM
 	.resume =	wacom_resume,
 	.reset_resume =	wacom_reset_resume,
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index b8180e4..e77d46d 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1248,6 +1248,176 @@ static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len)
 	return 0;
 }
 
+static void wacom_map_usage(struct wacom *wacom, struct hid_usage *usage,
+		struct hid_field *field, __u8 type, __u16 code, int fuzz)
+{
+	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
+	struct input_dev *input = wacom_wac->input;
+	int fmin = field->logical_minimum;
+	int fmax = field->logical_maximum;
+
+	usage->type = type;
+	usage->code = code;
+
+	set_bit(type, input->evbit);
+
+	switch (type) {
+	case EV_ABS:
+		input_set_abs_params(input, code, fmin, fmax, fuzz, 0);
+		input_abs_set_res(input, code,
+				  hidinput_calc_abs_res(field, code));
+		break;
+	case EV_KEY:
+		input_set_capability(input, EV_KEY, code);
+		break;
+	case EV_MSC:
+		input_set_capability(input, EV_MSC, code);
+		break;
+	}
+}
+
+static void wacom_wac_pen_usage_mapping(struct hid_device *hdev,
+		struct hid_field *field, struct hid_usage *usage)
+{
+	struct wacom *wacom = hid_get_drvdata(hdev);
+
+	switch (usage->hid) {
+	case HID_GD_X:
+		wacom_map_usage(wacom, usage, field, EV_ABS, ABS_X, 4);
+		break;
+	case HID_GD_Y:
+		wacom_map_usage(wacom, usage, field, EV_ABS, ABS_Y, 4);
+		break;
+	case HID_DG_TIPPRESSURE:
+		wacom_map_usage(wacom, usage, field, EV_ABS, ABS_PRESSURE, 0);
+		break;
+	case HID_DG_INRANGE:
+		wacom_map_usage(wacom, usage, field, EV_KEY, BTN_TOOL_PEN, 0);
+		break;
+	case HID_DG_INVERT:
+		wacom_map_usage(wacom, usage, field, EV_KEY,
+				BTN_TOOL_RUBBER, 0);
+		break;
+	case HID_DG_ERASER:
+	case HID_DG_TIPSWITCH:
+		wacom_map_usage(wacom, usage, field, EV_KEY, BTN_TOUCH, 0);
+		break;
+	case HID_DG_BARRELSWITCH:
+		wacom_map_usage(wacom, usage, field, EV_KEY, BTN_STYLUS, 0);
+		break;
+	case HID_DG_BARRELSWITCH2:
+		wacom_map_usage(wacom, usage, field, EV_KEY, BTN_STYLUS2, 0);
+		break;
+	case HID_DG_TOOLSERIALNUMBER:
+		wacom_map_usage(wacom, usage, field, EV_MSC, MSC_SERIAL, 0);
+		break;
+	}
+}
+
+static int wacom_wac_pen_event(struct hid_device *hdev, struct hid_field *field,
+		struct hid_usage *usage, __s32 value)
+{
+	struct wacom *wacom = hid_get_drvdata(hdev);
+	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
+	struct input_dev *input = wacom_wac->input;
+
+	/* checking which Tool / tip switch to send */
+	switch (usage->hid) {
+	case HID_DG_INRANGE:
+		wacom_wac->hid_data.inrange_state = value;
+		return 0;
+	case HID_DG_INVERT:
+		wacom_wac->hid_data.invert_state = value;
+		return 0;
+	case HID_DG_ERASER:
+	case HID_DG_TIPSWITCH:
+		wacom_wac->hid_data.tipswitch |= value;
+		return 0;
+	}
+
+	/* send pen events only when touch is up or forced out */
+	if (!usage->type || wacom_wac->shared->touch_down)
+		return 0;
+
+	input_event(input, usage->type, usage->code, value);
+
+	return 0;
+}
+
+static void wacom_wac_pen_report(struct hid_device *hdev,
+		struct hid_report *report)
+{
+	struct wacom *wacom = hid_get_drvdata(hdev);
+	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
+	struct input_dev *input = wacom_wac->input;
+	bool prox = wacom_wac->hid_data.inrange_state;
+
+	if (!wacom_wac->shared->stylus_in_proximity) /* first in prox */
+		/* Going into proximity select tool */
+		wacom_wac->tool[0] = wacom_wac->hid_data.invert_state ?
+						BTN_TOOL_RUBBER : BTN_TOOL_PEN;
+
+	/* keep pen state for touch events */
+	wacom_wac->shared->stylus_in_proximity = prox;
+
+	/* send pen events only when touch is up or forced out */
+	if (!wacom_wac->shared->touch_down) {
+		input_report_key(input, BTN_TOUCH,
+				wacom_wac->hid_data.tipswitch);
+		input_report_key(input, wacom_wac->tool[0], prox);
+
+		wacom_wac->hid_data.tipswitch = false;
+
+		input_sync(input);
+	}
+}
+
+#define WACOM_PEN_FIELD(f)	(((f)->logical == HID_DG_STYLUS) || \
+				 ((f)->physical == HID_DG_STYLUS))
+#define WACOM_FINGER_FIELD(f)	(((f)->logical == HID_DG_FINGER) || \
+				 ((f)->physical == HID_DG_FINGER))
+
+void wacom_wac_usage_mapping(struct hid_device *hdev,
+		struct hid_field *field, struct hid_usage *usage)
+{
+	struct wacom *wacom = hid_get_drvdata(hdev);
+	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
+	struct input_dev *input = wacom_wac->input;
+
+	/* currently, only direct devices have proper hid report descriptors */
+	__set_bit(INPUT_PROP_DIRECT, input->propbit);
+
+	if (WACOM_PEN_FIELD(field))
+		return wacom_wac_pen_usage_mapping(hdev, field, usage);
+}
+
+int wacom_wac_event(struct hid_device *hdev, struct hid_field *field,
+		struct hid_usage *usage, __s32 value)
+{
+	struct wacom *wacom = hid_get_drvdata(hdev);
+
+	if (wacom->wacom_wac.features.type != HID_GENERIC)
+		return 0;
+
+	if (WACOM_PEN_FIELD(field))
+		return wacom_wac_pen_event(hdev, field, usage, value);
+
+	return 0;
+}
+
+void wacom_wac_report(struct hid_device *hdev, struct hid_report *report)
+{
+	struct wacom *wacom = hid_get_drvdata(hdev);
+	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
+	struct hid_field *field = report->field[0];
+
+	if (wacom_wac->features.type != HID_GENERIC)
+		return;
+
+	if (WACOM_PEN_FIELD(field))
+		return wacom_wac_pen_report(hdev, report);
+}
+
 static int wacom_bpt_touch(struct wacom_wac *wacom)
 {
 	struct wacom_features *features = &wacom->features;
@@ -1746,6 +1916,10 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
 
 	input_dev->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
 
+	if (features->type == HID_GENERIC)
+		/* setup has already been done */
+		return 0;
+
 	__set_bit(BTN_TOUCH, input_dev->keybit);
 	__set_bit(ABS_MISC, input_dev->absbit);
 
@@ -2585,6 +2759,9 @@ static const struct wacom_features wacom_features_0x30C =
 	  .oVid = USB_VENDOR_ID_WACOM, .oPid = 0x30A, .touch_max = 10,
 	  .check_for_hid_type = true, .hid_type = HID_TYPE_USBNONE };
 
+static const struct wacom_features wacom_features_HID_ANY_ID =
+	{ "Wacom HID", .type = HID_GENERIC };
+
 #define USB_DEVICE_WACOM(prod)						\
 	HID_DEVICE(BUS_USB, HID_GROUP_WACOM, USB_VENDOR_ID_WACOM, prod),\
 	.driver_data = (kernel_ulong_t)&wacom_features_##prod
@@ -2729,6 +2906,8 @@ const struct hid_device_id wacom_ids[] = {
 	{ USB_DEVICE_WACOM(0x4004) },
 	{ USB_DEVICE_WACOM(0x5000) },
 	{ USB_DEVICE_WACOM(0x5002) },
+
+	{ USB_DEVICE_WACOM(HID_ANY_ID) },
 	{ }
 };
 MODULE_DEVICE_TABLE(hid, wacom_ids);
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 72f9ca8..f472eac 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -113,6 +113,7 @@ enum {
 	MTSCREEN,
 	MTTPC,
 	MTTPC_B,
+	HID_GENERIC,
 	MAX_TYPE
 };
 
@@ -154,6 +155,12 @@ struct wacom_shared {
 	struct input_dev *touch_input;
 };
 
+struct hid_data {
+	bool inrange_state;
+	bool invert_state;
+	bool tipswitch;
+};
+
 struct wacom_wac {
 	char name[WACOM_NAME_MAX];
 	char pad_name[WACOM_NAME_MAX];
@@ -175,6 +182,7 @@ struct wacom_wac {
 	int ps_connected;
 	u8 bt_features;
 	u8 bt_high_speed;
+	struct hid_data hid_data;
 };
 
 #endif
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 26ee25f..78ea9bf 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -265,6 +265,7 @@ struct hid_item {
 #define HID_CONNECT_HIDDEV		0x08
 #define HID_CONNECT_HIDDEV_FORCE	0x10
 #define HID_CONNECT_FF			0x20
+#define HID_CONNECT_DRIVER		0x40
 #define HID_CONNECT_DEFAULT	(HID_CONNECT_HIDINPUT|HID_CONNECT_HIDRAW| \
 		HID_CONNECT_HIDDEV|HID_CONNECT_FF)
 
@@ -441,6 +442,7 @@ struct hid_output_fifo {
 #define HID_CLAIMED_INPUT	1
 #define HID_CLAIMED_HIDDEV	2
 #define HID_CLAIMED_HIDRAW	4
+#define HID_CLAIMED_DRIVER	8
 
 #define HID_STAT_ADDED		1
 #define HID_STAT_PARSED		2
-- 
2.1.0

[PATCH 5/5] HID: wacom: implement the finger part of the HID generic handling

[Benjamin Tissoires]

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/wacom_sys.c |  39 ++++++++++++++--
 drivers/hid/wacom_wac.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/hid/wacom_wac.h |   8 ++++
 3 files changed, 164 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index dd288b2..8593047 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -109,6 +109,7 @@ static void wacom_feature_mapping(struct hid_device *hdev,
 {
 	struct wacom *wacom = hid_get_drvdata(hdev);
 	struct wacom_features *features = &wacom->wacom_wac.features;
+	struct hid_data *hid_data = &wacom->wacom_wac.hid_data;
 	u8 *data;
 	int ret;
 
@@ -128,6 +129,16 @@ static void wacom_feature_mapping(struct hid_device *hdev,
 			kfree(data);
 		}
 		break;
+	case HID_DG_INPUTMODE:
+		/* Ignore if value index is out of bounds. */
+		if (usage->usage_index >= field->report_count) {
+			dev_err(&hdev->dev, "HID_DG_INPUTMODE out of range\n");
+			break;
+		}
+
+		hid_data->inputmode = field->report->id;
+		hid_data->inputmode_index = usage->usage_index;
+		break;
 	}
 }
 
@@ -255,6 +266,25 @@ static void wacom_parse_hid(struct hid_device *hdev,
 	}
 }
 
+static int wacom_hid_set_device_mode(struct hid_device *hdev)
+{
+	struct wacom *wacom = hid_get_drvdata(hdev);
+	struct hid_data *hid_data = &wacom->wacom_wac.hid_data;
+	struct hid_report *r;
+	struct hid_report_enum *re;
+
+	if (hid_data->inputmode < 0)
+		return 0;
+
+	re = &(hdev->report_enum[HID_FEATURE_REPORT]);
+	r = re->report_id_hash[hid_data->inputmode];
+	if (r) {
+		r->field[0]->value[hid_data->inputmode_index] = 2;
+		hid_hw_request(hdev, r, HID_REQ_SET_REPORT);
+	}
+	return 0;
+}
+
 static int wacom_set_device_mode(struct hid_device *hdev, int report_id,
 		int length, int mode)
 {
@@ -347,6 +377,9 @@ static int wacom_query_tablet_data(struct hid_device *hdev,
 	if (hdev->bus == BUS_BLUETOOTH)
 		return wacom_bt_query_tablet_data(hdev, 1, features);
 
+	if (features->type == HID_GENERIC)
+		return wacom_hid_set_device_mode(hdev);
+
 	if (features->device_type == BTN_TOOL_FINGER) {
 		if (features->type > TABLETPC) {
 			/* MT Tablet PC touch */
@@ -1451,9 +1484,6 @@ static int wacom_probe(struct hid_device *hdev,
 				 error);
 	}
 
-	/* Note that if query fails it is not a hard failure */
-	wacom_query_tablet_data(hdev, features);
-
 	if (features->type == HID_GENERIC)
 		connect_mask |= HID_CONNECT_DRIVER;
 
@@ -1464,6 +1494,9 @@ static int wacom_probe(struct hid_device *hdev,
 		goto fail_hw_start;
 	}
 
+	/* Note that if query fails it is not a hard failure */
+	wacom_query_tablet_data(hdev, features);
+
 	if (features->quirks & WACOM_QUIRK_MONITOR)
 		error = hid_hw_open(hdev);
 
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index e77d46d..586b240 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1372,6 +1372,117 @@ static void wacom_wac_pen_report(struct hid_device *hdev,
 	}
 }
 
+static void wacom_wac_finger_usage_mapping(struct hid_device *hdev,
+		struct hid_field *field, struct hid_usage *usage)
+{
+	struct wacom *wacom = hid_get_drvdata(hdev);
+	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
+	struct input_dev *input = wacom_wac->input;
+	unsigned touch_max = wacom_wac->features.touch_max;
+
+	switch (usage->hid) {
+	case HID_GD_X:
+		if (touch_max == 1)
+			wacom_map_usage(wacom, usage, field, EV_ABS, ABS_X, 4);
+		else
+			wacom_map_usage(wacom, usage, field, EV_ABS,
+					ABS_MT_POSITION_X, 4);
+		break;
+	case HID_GD_Y:
+		if (touch_max == 1)
+			wacom_map_usage(wacom, usage, field, EV_ABS, ABS_Y, 4);
+		else
+			wacom_map_usage(wacom, usage, field, EV_ABS,
+					ABS_MT_POSITION_Y, 4);
+		break;
+	case HID_DG_CONTACTID:
+		input_mt_init_slots(input, wacom_wac->features.touch_max,
+			INPUT_MT_DIRECT);
+		break;
+	case HID_DG_INRANGE:
+		break;
+	case HID_DG_INVERT:
+		break;
+	case HID_DG_TIPSWITCH:
+		wacom_map_usage(wacom, usage, field, EV_KEY, BTN_TOUCH, 0);
+		break;
+	}
+}
+
+static int wacom_wac_finger_event(struct hid_device *hdev,
+		struct hid_field *field, struct hid_usage *usage, __s32 value)
+{
+	struct wacom *wacom = hid_get_drvdata(hdev);
+	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
+
+	switch (usage->hid) {
+	case HID_GD_X:
+		wacom_wac->hid_data.x = value;
+		break;
+	case HID_GD_Y:
+		wacom_wac->hid_data.y = value;
+		break;
+	case HID_DG_CONTACTID:
+		wacom_wac->hid_data.id = value;
+		break;
+	case HID_DG_TIPSWITCH:
+		wacom_wac->hid_data.tipswitch = value;
+		break;
+	}
+
+
+	return 0;
+}
+
+static void wacom_wac_finger_mt_report(struct wacom_wac *wacom_wac,
+		struct input_dev *input, bool touch)
+{
+	int slot;
+	struct hid_data *hid_data = &wacom_wac->hid_data;
+
+	slot = input_mt_get_slot_by_key(input, hid_data->id);
+
+	input_mt_slot(input, slot);
+	input_mt_report_slot_state(input, MT_TOOL_FINGER, touch);
+	if (touch) {
+		input_report_abs(input, ABS_MT_POSITION_X, hid_data->x);
+		input_report_abs(input, ABS_MT_POSITION_Y, hid_data->y);
+	}
+	input_mt_sync_frame(input);
+}
+
+static void wacom_wac_finger_single_touch_report(struct wacom_wac *wacom_wac,
+		struct input_dev *input, bool touch)
+{
+	struct hid_data *hid_data = &wacom_wac->hid_data;
+
+	if (touch) {
+		input_report_abs(input, ABS_X, hid_data->x);
+		input_report_abs(input, ABS_Y, hid_data->y);
+	}
+	input_report_key(input, BTN_TOUCH, touch);
+}
+
+static void wacom_wac_finger_report(struct hid_device *hdev,
+		struct hid_report *report)
+{
+	struct wacom *wacom = hid_get_drvdata(hdev);
+	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
+	struct input_dev *input = wacom_wac->input;
+	bool touch = wacom_wac->hid_data.tipswitch &&
+		     !wacom_wac->shared->stylus_in_proximity;
+	unsigned touch_max = wacom_wac->features.touch_max;
+
+	if (touch_max > 1)
+		wacom_wac_finger_mt_report(wacom_wac, input, touch);
+	else
+		wacom_wac_finger_single_touch_report(wacom_wac, input, touch);
+	input_sync(input);
+
+	/* keep touch state for pen event */
+	wacom_wac->shared->touch_down = touch;
+}
+
 #define WACOM_PEN_FIELD(f)	(((f)->logical == HID_DG_STYLUS) || \
 				 ((f)->physical == HID_DG_STYLUS))
 #define WACOM_FINGER_FIELD(f)	(((f)->logical == HID_DG_FINGER) || \
@@ -1389,6 +1500,9 @@ void wacom_wac_usage_mapping(struct hid_device *hdev,
 
 	if (WACOM_PEN_FIELD(field))
 		return wacom_wac_pen_usage_mapping(hdev, field, usage);
+
+	if (WACOM_FINGER_FIELD(field))
+		return wacom_wac_finger_usage_mapping(hdev, field, usage);
 }
 
 int wacom_wac_event(struct hid_device *hdev, struct hid_field *field,
@@ -1402,6 +1516,9 @@ int wacom_wac_event(struct hid_device *hdev, struct hid_field *field,
 	if (WACOM_PEN_FIELD(field))
 		return wacom_wac_pen_event(hdev, field, usage, value);
 
+	if (WACOM_FINGER_FIELD(field))
+		return wacom_wac_finger_event(hdev, field, usage, value);
+
 	return 0;
 }
 
@@ -1416,6 +1533,9 @@ void wacom_wac_report(struct hid_device *hdev, struct hid_report *report)
 
 	if (WACOM_PEN_FIELD(field))
 		return wacom_wac_pen_report(hdev, report);
+
+	if (WACOM_FINGER_FIELD(field))
+		return wacom_wac_finger_report(hdev, report);
 }
 
 static int wacom_bpt_touch(struct wacom_wac *wacom)
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index f472eac..0f0b85e 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -156,9 +156,17 @@ struct wacom_shared {
 };
 
 struct hid_data {
+	__s16 inputmode;	/* InputMode HID feature, -1 if non-existent */
+	__s16 inputmode_index;	/* InputMode HID feature index in the report */
 	bool inrange_state;
 	bool invert_state;
 	bool tipswitch;
+	int x;
+	int y;
+	int pressure;
+	int width;
+	int height;
+	int id;
 };
 
 struct wacom_wac {
-- 
2.1.0

Re: [PATCH 0/5] HID: wacom: introduce generic HID handling

[Jiri Kosina]

On Tue, 23 Sep 2014, Benjamin Tissoires wrote:

> Hi guys,
> 
> So, this patch series aims at supporting natively any future HID compliant wacom
> tablet. Those found on the various laptops (ISDv4/5) already are HID compliant
> and they should work in the future without any modification of the kernel.
> 
> Few things to note here:
> - I used the PID 0x00E6 found on the Lenovo X230 for the tests
> - I did not removed its entry in the list because there is a slightly different
>   behavior while using this patch set: when more than two fingers are on the
>   screen, the tablet stops sending finger events, while with the wacom
>   proprietary bits, it continues to send them. Besides that, the events emitted
>   before and after the patch series are the same (at least on the E6)
> - I can not rely on hid-input directly because wacom wants to be in control of
>   the input devices it creates. This might be solved in the future, but for now
>   it is just easier to rewrite the few mapping/events handling than trying to
>   fit in the hid-input model.
> - there will still be more specific use cases to handle later (see the joke of
>   the MS surface 3 pen[1] for instance), but this should give a roughly working
>   pen/touch support for those future devices.
> 
> Jason, I would be very glad if you could conduct a few tests for this on the
> ISDv4/5 sensors you have.

I am waiting with this one for testing results from Jason, but if you guys 
want this to go in the next merge window, I'd like to ask you not to 
postpone the testing too much.

Thanks.

> 
> Cheers,
> Benjamin
> 
> [1] http://who-t.blogspot.com/2014/09/stylus-behaviour-on-microsoft-surface-3.html
> 
> Benjamin Tissoires (5):
>   HID: wacom: rename failN with some meaningful information
>   HID: wacom: split out input allocation and registration
>   HID: wacom: move allocation of inputs earlier
>   HID: wacom: implement generic HID handling for pen generic devices
>   HID: wacom: implement the finger part of the HID generic handling
> 
>  drivers/hid/hid-core.c  |   3 +
>  drivers/hid/wacom.h     |   6 +
>  drivers/hid/wacom_sys.c | 184 +++++++++++++++++++++--------
>  drivers/hid/wacom_wac.c | 299 ++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/hid/wacom_wac.h |  17 +++
>  include/linux/hid.h     |   2 +
>  6 files changed, 462 insertions(+), 49 deletions(-)
> 
> -- 
> 2.1.0
> 

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 0/5] HID: wacom: introduce generic HID handling

[Jason Gerecke]

On Fri, Sep 26, 2014 at 4:03 AM, Jiri Kosina <jkosina@suse.cz> wrote:
> On Tue, 23 Sep 2014, Benjamin Tissoires wrote:
>
>> Hi guys,
>>
>> So, this patch series aims at supporting natively any future HID compliant wacom
>> tablet. Those found on the various laptops (ISDv4/5) already are HID compliant
>> and they should work in the future without any modification of the kernel.
>>
>> Few things to note here:
>> - I used the PID 0x00E6 found on the Lenovo X230 for the tests
>> - I did not removed its entry in the list because there is a slightly different
>>   behavior while using this patch set: when more than two fingers are on the
>>   screen, the tablet stops sending finger events, while with the wacom
>>   proprietary bits, it continues to send them. Besides that, the events emitted
>>   before and after the patch series are the same (at least on the E6)
>> - I can not rely on hid-input directly because wacom wants to be in control of
>>   the input devices it creates. This might be solved in the future, but for now
>>   it is just easier to rewrite the few mapping/events handling than trying to
>>   fit in the hid-input model.
>> - there will still be more specific use cases to handle later (see the joke of
>>   the MS surface 3 pen[1] for instance), but this should give a roughly working
>>   pen/touch support for those future devices.
>>
>> Jason, I would be very glad if you could conduct a few tests for this on the
>> ISDv4/5 sensors you have.
>
> I am waiting with this one for testing results from Jason, but if you guys
> want this to go in the next merge window, I'd like to ask you not to
> postpone the testing too much.
>
> Thanks.
>

I'm in the process of capturing traces from hid-record for multiple
tablet PCs both pre- and post-patch to locate any obvious issues. My
initial results are promising on the pen side (the only things I've
noticed so far is the 2nd barrel switch not being supported -- as
might be suspected) but less so on the touch side (single-finger
devices seem to work fine, but the new code doesn't seem to handle any
of the multitouch devices I've tried).

Expect more news next week :)


PS: I'm pretty sure ISDv5 shouldn't be relied on for HID data :| The
descriptor for the Cintiq Companion Hybrid, at least, is like our
branded sensors with useful data only in the touchscreen descriptor
(the pen descriptor is a bunch of opaque vendor-defined reports). Not
sure if the Cintiq Companion is the same though since it runs
Windows...

Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one  /
(That is to say, eight) to the two,     /
But you can’t take seven from three,    /
So you look at the sixty-fours....

>>
>> Cheers,
>> Benjamin
>>
>> [1] http://who-t.blogspot.com/2014/09/stylus-behaviour-on-microsoft-surface-3.html
>>
>> Benjamin Tissoires (5):
>>   HID: wacom: rename failN with some meaningful information
>>   HID: wacom: split out input allocation and registration
>>   HID: wacom: move allocation of inputs earlier
>>   HID: wacom: implement generic HID handling for pen generic devices
>>   HID: wacom: implement the finger part of the HID generic handling
>>
>>  drivers/hid/hid-core.c  |   3 +
>>  drivers/hid/wacom.h     |   6 +
>>  drivers/hid/wacom_sys.c | 184 +++++++++++++++++++++--------
>>  drivers/hid/wacom_wac.c | 299 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  drivers/hid/wacom_wac.h |  17 +++
>>  include/linux/hid.h     |   2 +
>>  6 files changed, 462 insertions(+), 49 deletions(-)
>>
>> --
>> 2.1.0
>>
>
> --
> Jiri Kosina
> SUSE Labs

Re: [PATCH 0/5] HID: wacom: introduce generic HID handling

[Benjamin Tissoires]

On Sep 26 2014 or thereabouts, Jason Gerecke wrote:
> On Fri, Sep 26, 2014 at 4:03 AM, Jiri Kosina <jkosina@suse.cz> wrote:
> > On Tue, 23 Sep 2014, Benjamin Tissoires wrote:
> >
> >> Hi guys,
> >>
> >> So, this patch series aims at supporting natively any future HID compliant wacom
> >> tablet. Those found on the various laptops (ISDv4/5) already are HID compliant
> >> and they should work in the future without any modification of the kernel.
> >>
> >> Few things to note here:
> >> - I used the PID 0x00E6 found on the Lenovo X230 for the tests
> >> - I did not removed its entry in the list because there is a slightly different
> >>   behavior while using this patch set: when more than two fingers are on the
> >>   screen, the tablet stops sending finger events, while with the wacom
> >>   proprietary bits, it continues to send them. Besides that, the events emitted
> >>   before and after the patch series are the same (at least on the E6)
> >> - I can not rely on hid-input directly because wacom wants to be in control of
> >>   the input devices it creates. This might be solved in the future, but for now
> >>   it is just easier to rewrite the few mapping/events handling than trying to
> >>   fit in the hid-input model.
> >> - there will still be more specific use cases to handle later (see the joke of
> >>   the MS surface 3 pen[1] for instance), but this should give a roughly working
> >>   pen/touch support for those future devices.
> >>
> >> Jason, I would be very glad if you could conduct a few tests for this on the
> >> ISDv4/5 sensors you have.
> >
> > I am waiting with this one for testing results from Jason, but if you guys
> > want this to go in the next merge window, I'd like to ask you not to
> > postpone the testing too much.
> >
> > Thanks.
> >
> 
> I'm in the process of capturing traces from hid-record for multiple
> tablet PCs both pre- and post-patch to locate any obvious issues. My
> initial results are promising on the pen side (the only things I've
> noticed so far is the 2nd barrel switch not being supported -- as
> might be suspected) but less so on the touch side (single-finger
> devices seem to work fine, but the new code doesn't seem to handle any
> of the multitouch devices I've tried).

Do you consider not having the second barrel switch a blocker or can we
ship it this and send a fix later?

As for the touch, I tested/developed it on the Lenovo X230. It presents
a hid-multitouch compatible touch interface, when the feature input_mode
is sent with the value '2'. By re-reading it, it seems to me that you
are trying to replay the touch traces you got before the patch, but this
will not work because the touch protocol is not the same. ISDv4 used to
rely on the proprietary touch protocol which is hard to describe in HID.

I would say that as long as there is no regression (actually there can
not be for old devices :-P), we should still carry this series for 3.18.
We might need to adjust the bits here and there, but if we can have a
*basic* support of the tablet out of the box, this is still much better
than silently ignoring it.

> 
> Expect more news next week :)

So?????

> 
> 
> PS: I'm pretty sure ISDv5 shouldn't be relied on for HID data :| The
> descriptor for the Cintiq Companion Hybrid, at least, is like our
> branded sensors with useful data only in the touchscreen descriptor
> (the pen descriptor is a bunch of opaque vendor-defined reports). Not
> sure if the Cintiq Companion is the same though since it runs
> Windows...

Ouch. For those, we will need to either fix the report descriptors or
handle them in the same way you always used to do: manually.

Cheers,
Benjamin

> >>
> >> [1] http://who-t.blogspot.com/2014/09/stylus-behaviour-on-microsoft-surface-3.html
> >>
> >> Benjamin Tissoires (5):
> >>   HID: wacom: rename failN with some meaningful information
> >>   HID: wacom: split out input allocation and registration
> >>   HID: wacom: move allocation of inputs earlier
> >>   HID: wacom: implement generic HID handling for pen generic devices
> >>   HID: wacom: implement the finger part of the HID generic handling
> >>
> >>  drivers/hid/hid-core.c  |   3 +
> >>  drivers/hid/wacom.h     |   6 +
> >>  drivers/hid/wacom_sys.c | 184 +++++++++++++++++++++--------
> >>  drivers/hid/wacom_wac.c | 299 ++++++++++++++++++++++++++++++++++++++++++++++++
> >>  drivers/hid/wacom_wac.h |  17 +++
> >>  include/linux/hid.h     |   2 +
> >>  6 files changed, 462 insertions(+), 49 deletions(-)
> >>
> >> --
> >> 2.1.0
> >>
> >
> > --
> > Jiri Kosina
> > SUSE Labs

Re: [PATCH 0/5] HID: wacom: introduce generic HID handling

[Jason Gerecke]

On Tue, Sep 30, 2014 at 10:27 AM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> On Sep 26 2014 or thereabouts, Jason Gerecke wrote:
>> On Fri, Sep 26, 2014 at 4:03 AM, Jiri Kosina <jkosina@suse.cz> wrote:
>> > On Tue, 23 Sep 2014, Benjamin Tissoires wrote:
>> >
>> >> Hi guys,
>> >>
>> >> So, this patch series aims at supporting natively any future HID compliant wacom
>> >> tablet. Those found on the various laptops (ISDv4/5) already are HID compliant
>> >> and they should work in the future without any modification of the kernel.
>> >>
>> >> Few things to note here:
>> >> - I used the PID 0x00E6 found on the Lenovo X230 for the tests
>> >> - I did not removed its entry in the list because there is a slightly different
>> >>   behavior while using this patch set: when more than two fingers are on the
>> >>   screen, the tablet stops sending finger events, while with the wacom
>> >>   proprietary bits, it continues to send them. Besides that, the events emitted
>> >>   before and after the patch series are the same (at least on the E6)
>> >> - I can not rely on hid-input directly because wacom wants to be in control of
>> >>   the input devices it creates. This might be solved in the future, but for now
>> >>   it is just easier to rewrite the few mapping/events handling than trying to
>> >>   fit in the hid-input model.
>> >> - there will still be more specific use cases to handle later (see the joke of
>> >>   the MS surface 3 pen[1] for instance), but this should give a roughly working
>> >>   pen/touch support for those future devices.
>> >>
>> >> Jason, I would be very glad if you could conduct a few tests for this on the
>> >> ISDv4/5 sensors you have.
>> >
>> > I am waiting with this one for testing results from Jason, but if you guys
>> > want this to go in the next merge window, I'd like to ask you not to
>> > postpone the testing too much.
>> >
>> > Thanks.
>> >
>>
>> I'm in the process of capturing traces from hid-record for multiple
>> tablet PCs both pre- and post-patch to locate any obvious issues. My
>> initial results are promising on the pen side (the only things I've
>> noticed so far is the 2nd barrel switch not being supported -- as
>> might be suspected) but less so on the touch side (single-finger
>> devices seem to work fine, but the new code doesn't seem to handle any
>> of the multitouch devices I've tried).
>
> Do you consider not having the second barrel switch a blocker or can we
> ship it this and send a fix later?
>
> As for the touch, I tested/developed it on the Lenovo X230. It presents
> a hid-multitouch compatible touch interface, when the feature input_mode
> is sent with the value '2'. By re-reading it, it seems to me that you
> are trying to replay the touch traces you got before the patch, but this
> will not work because the touch protocol is not the same. ISDv4 used to
> rely on the proprietary touch protocol which is hard to describe in HID.
>
> I would say that as long as there is no regression (actually there can
> not be for old devices :-P), we should still carry this series for 3.18.
> We might need to adjust the bits here and there, but if we can have a
> *basic* support of the tablet out of the box, this is still much better
> than silently ignoring it.
>
I'm in agreement. The lack of second-switch support was expected and
somewhat trivial at this moment. We can work on finessing it as we go.

Regarding the touch input, I haven't had enough time to dive in to
figuring out what is wrong, but traces aren't the issue (I've not been
able to use traces for multitouch testing because the driver can't
query features->touch_max ;)). Working with actual hardware, I don't
get _any_ events out of the driver when using the touchscreen. I
checked that the device is sending the non-vendor-defined reports,
which does seem to be the case. The descriptor for the reports also
seem to be reasonable, appearing to follow Microsoft's
recommendations...

>>
>> Expect more news next week :)
>
> So?????
>
I'm not sure if you want to fix touch first or just get these upstream
and fix during RC (as you said, it won't cause any regressions...) but
I'll give the following:

Acked-by: Jason Gerecke <killertofu@gmail.com>

Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one  /
(That is to say, eight) to the two,     /
But you can’t take seven from three,    /
So you look at the sixty-fours....

>>
>>
>> PS: I'm pretty sure ISDv5 shouldn't be relied on for HID data :| The
>> descriptor for the Cintiq Companion Hybrid, at least, is like our
>> branded sensors with useful data only in the touchscreen descriptor
>> (the pen descriptor is a bunch of opaque vendor-defined reports). Not
>> sure if the Cintiq Companion is the same though since it runs
>> Windows...
>
> Ouch. For those, we will need to either fix the report descriptors or
> handle them in the same way you always used to do: manually.
>
> Cheers,
> Benjamin
>
>> >>
>> >> [1] http://who-t.blogspot.com/2014/09/stylus-behaviour-on-microsoft-surface-3.html
>> >>
>> >> Benjamin Tissoires (5):
>> >>   HID: wacom: rename failN with some meaningful information
>> >>   HID: wacom: split out input allocation and registration
>> >>   HID: wacom: move allocation of inputs earlier
>> >>   HID: wacom: implement generic HID handling for pen generic devices
>> >>   HID: wacom: implement the finger part of the HID generic handling
>> >>
>> >>  drivers/hid/hid-core.c  |   3 +
>> >>  drivers/hid/wacom.h     |   6 +
>> >>  drivers/hid/wacom_sys.c | 184 +++++++++++++++++++++--------
>> >>  drivers/hid/wacom_wac.c | 299 ++++++++++++++++++++++++++++++++++++++++++++++++
>> >>  drivers/hid/wacom_wac.h |  17 +++
>> >>  include/linux/hid.h     |   2 +
>> >>  6 files changed, 462 insertions(+), 49 deletions(-)
>> >>
>> >> --
>> >> 2.1.0
>> >>
>> >
>> > --
>> > Jiri Kosina
>> > SUSE Labs

Re: [PATCH 0/5] HID: wacom: introduce generic HID handling

[Jiri Kosina]

Applied the series to for-3.18/wacom. Thanks,

-- 
Jiri Kosina
SUSE Labs