[PATCH 0/6] HID: wacom: cleanup

[PATCH 0/6] HID: wacom: cleanup

[Benjamin Tissoires]

Hi,

While working on the wacom.ko driver, I ended up having those 6 patches that
the current wacom.ko could benefit right now.

Besides cleaning, the only gain users will see is in patch 6 which allows to
actually use the wireless receiver (for development) without having to
unplug/replug it.

Cheers,
Benjamin

Benjamin Tissoires (6):
  HID: wacom: break out wacom_intuos_get_tool_type
  HID: wacom: break out parsing of device and registering of input
  HID: wacom: move down wireless_work()
  HID: wacom: reuse wacom_parse_and_register() in wireless_work
  HID: wacom: cleanup input devices
  HID: wacom: close the wireless receiver on remove()

 drivers/hid/wacom_sys.c | 363 ++++++++++++++++++++++++------------------------
 drivers/hid/wacom_wac.c | 166 +++++++++++-----------
 2 files changed, 265 insertions(+), 264 deletions(-)

-- 
2.5.0

[PATCH 1/6] HID: wacom: break out wacom_intuos_get_tool_type

[Benjamin Tissoires]

Allow to reuse the code in a later series and simplifies
the reading of wacom_intuos_inout().

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/wacom_wac.c | 166 +++++++++++++++++++++++++-----------------------
 1 file changed, 87 insertions(+), 79 deletions(-)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index e92e1e8..bd198bb 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -575,6 +575,91 @@ static int wacom_intuos_pad(struct wacom_wac *wacom)
 	return 1;
 }
 
+static int wacom_intuos_get_tool_type(int tool_id)
+{
+	int tool_type;
+
+	switch (tool_id) {
+	case 0x812: /* Inking pen */
+	case 0x801: /* Intuos3 Inking pen */
+	case 0x120802: /* Intuos4/5 Inking Pen */
+	case 0x012:
+		tool_type = BTN_TOOL_PENCIL;
+		break;
+
+	case 0x822: /* Pen */
+	case 0x842:
+	case 0x852:
+	case 0x823: /* Intuos3 Grip Pen */
+	case 0x813: /* Intuos3 Classic Pen */
+	case 0x885: /* Intuos3 Marker Pen */
+	case 0x802: /* Intuos4/5 13HD/24HD General Pen */
+	case 0x804: /* Intuos4/5 13HD/24HD Marker Pen */
+	case 0x8e2: /* IntuosHT2 pen */
+	case 0x022:
+	case 0x100804: /* Intuos4/5 13HD/24HD Art Pen */
+	case 0x140802: /* Intuos4/5 13HD/24HD Classic Pen */
+	case 0x160802: /* Cintiq 13HD Pro Pen */
+	case 0x180802: /* DTH2242 Pen */
+	case 0x100802: /* Intuos4/5 13HD/24HD General Pen */
+		tool_type = BTN_TOOL_PEN;
+		break;
+
+	case 0x832: /* Stroke pen */
+	case 0x032:
+		tool_type = BTN_TOOL_BRUSH;
+		break;
+
+	case 0x007: /* Mouse 4D and 2D */
+	case 0x09c:
+	case 0x094:
+	case 0x017: /* Intuos3 2D Mouse */
+	case 0x806: /* Intuos4 Mouse */
+		tool_type = BTN_TOOL_MOUSE;
+		break;
+
+	case 0x096: /* Lens cursor */
+	case 0x097: /* Intuos3 Lens cursor */
+	case 0x006: /* Intuos4 Lens cursor */
+		tool_type = BTN_TOOL_LENS;
+		break;
+
+	case 0x82a: /* Eraser */
+	case 0x85a:
+	case 0x91a:
+	case 0xd1a:
+	case 0x0fa:
+	case 0x82b: /* Intuos3 Grip Pen Eraser */
+	case 0x81b: /* Intuos3 Classic Pen Eraser */
+	case 0x91b: /* Intuos3 Airbrush Eraser */
+	case 0x80c: /* Intuos4/5 13HD/24HD Marker Pen Eraser */
+	case 0x80a: /* Intuos4/5 13HD/24HD General Pen Eraser */
+	case 0x90a: /* Intuos4/5 13HD/24HD Airbrush Eraser */
+	case 0x14080a: /* Intuos4/5 13HD/24HD Classic Pen Eraser */
+	case 0x10090a: /* Intuos4/5 13HD/24HD Airbrush Eraser */
+	case 0x10080c: /* Intuos4/5 13HD/24HD Art Pen Eraser */
+	case 0x16080a: /* Cintiq 13HD Pro Pen Eraser */
+	case 0x18080a: /* DTH2242 Eraser */
+	case 0x10080a: /* Intuos4/5 13HD/24HD General Pen Eraser */
+		tool_type = BTN_TOOL_RUBBER;
+		break;
+
+	case 0xd12:
+	case 0x912:
+	case 0x112:
+	case 0x913: /* Intuos3 Airbrush */
+	case 0x902: /* Intuos4/5 13HD/24HD Airbrush */
+	case 0x100902: /* Intuos4/5 13HD/24HD Airbrush */
+		tool_type = BTN_TOOL_AIRBRUSH;
+		break;
+
+	default: /* Unknown tool */
+		tool_type = BTN_TOOL_PEN;
+		break;
+	}
+	return tool_type;
+}
+
 static int wacom_intuos_inout(struct wacom_wac *wacom)
 {
 	struct wacom_features *features = &wacom->features;
@@ -597,85 +682,8 @@ static int wacom_intuos_inout(struct wacom_wac *wacom)
 		wacom->id[idx] = (data[2] << 4) | (data[3] >> 4) |
 			((data[7] & 0x0f) << 20) | ((data[8] & 0xf0) << 12);
 
-		switch (wacom->id[idx]) {
-		case 0x812: /* Inking pen */
-		case 0x801: /* Intuos3 Inking pen */
-		case 0x120802: /* Intuos4/5 Inking Pen */
-		case 0x012:
-			wacom->tool[idx] = BTN_TOOL_PENCIL;
-			break;
-
-		case 0x822: /* Pen */
-		case 0x842:
-		case 0x852:
-		case 0x823: /* Intuos3 Grip Pen */
-		case 0x813: /* Intuos3 Classic Pen */
-		case 0x885: /* Intuos3 Marker Pen */
-		case 0x802: /* Intuos4/5 13HD/24HD General Pen */
-		case 0x804: /* Intuos4/5 13HD/24HD Marker Pen */
-		case 0x8e2: /* IntuosHT2 pen */
-		case 0x022:
-		case 0x100804: /* Intuos4/5 13HD/24HD Art Pen */
-		case 0x140802: /* Intuos4/5 13HD/24HD Classic Pen */
-		case 0x160802: /* Cintiq 13HD Pro Pen */
-		case 0x180802: /* DTH2242 Pen */
-		case 0x100802: /* Intuos4/5 13HD/24HD General Pen */
-			wacom->tool[idx] = BTN_TOOL_PEN;
-			break;
-
-		case 0x832: /* Stroke pen */
-		case 0x032:
-			wacom->tool[idx] = BTN_TOOL_BRUSH;
-			break;
-
-		case 0x007: /* Mouse 4D and 2D */
-		case 0x09c:
-		case 0x094:
-		case 0x017: /* Intuos3 2D Mouse */
-		case 0x806: /* Intuos4 Mouse */
-			wacom->tool[idx] = BTN_TOOL_MOUSE;
-			break;
-
-		case 0x096: /* Lens cursor */
-		case 0x097: /* Intuos3 Lens cursor */
-		case 0x006: /* Intuos4 Lens cursor */
-			wacom->tool[idx] = BTN_TOOL_LENS;
-			break;
-
-		case 0x82a: /* Eraser */
-		case 0x85a:
-		case 0x91a:
-		case 0xd1a:
-		case 0x0fa:
-		case 0x82b: /* Intuos3 Grip Pen Eraser */
-		case 0x81b: /* Intuos3 Classic Pen Eraser */
-		case 0x91b: /* Intuos3 Airbrush Eraser */
-		case 0x80c: /* Intuos4/5 13HD/24HD Marker Pen Eraser */
-		case 0x80a: /* Intuos4/5 13HD/24HD General Pen Eraser */
-		case 0x90a: /* Intuos4/5 13HD/24HD Airbrush Eraser */
-		case 0x14080a: /* Intuos4/5 13HD/24HD Classic Pen Eraser */
-		case 0x10090a: /* Intuos4/5 13HD/24HD Airbrush Eraser */
-		case 0x10080c: /* Intuos4/5 13HD/24HD Art Pen Eraser */
-		case 0x16080a: /* Cintiq 13HD Pro Pen Eraser */
-		case 0x18080a: /* DTH2242 Eraser */
-		case 0x10080a: /* Intuos4/5 13HD/24HD General Pen Eraser */
-			wacom->tool[idx] = BTN_TOOL_RUBBER;
-			break;
-
-		case 0xd12:
-		case 0x912:
-		case 0x112:
-		case 0x913: /* Intuos3 Airbrush */
-		case 0x902: /* Intuos4/5 13HD/24HD Airbrush */
-		case 0x100902: /* Intuos4/5 13HD/24HD Airbrush */
-			wacom->tool[idx] = BTN_TOOL_AIRBRUSH;
-			break;
-
-		default: /* Unknown tool */
-			wacom->tool[idx] = BTN_TOOL_PEN;
-			break;
-		}
-		wacom->shared->stylus_in_proximity = true;
+		wacom->tool[idx] = wacom_intuos_get_tool_type(wacom->id[idx]);
+
 		return 1;
 	}
 
-- 
2.5.0

[PATCH 2/6] HID: wacom: break out parsing of device and registering of input

[Benjamin Tissoires]

Simplifies the .probe() and will allow to reuse this path in the future.
Few things are reshuffled in .probe():
- init wacom struct earlier
- then retrieve the report descriptor
- then parse it and allocate/register inputs.

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

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 5cb21dd..92a2c81 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1685,61 +1685,21 @@ static void wacom_update_name(struct wacom *wacom)
 		"%s Pad", name);
 }
 
-static int wacom_probe(struct hid_device *hdev,
-		const struct hid_device_id *id)
+static int wacom_parse_and_register(struct wacom *wacom)
 {
-	struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
-	struct usb_device *dev = interface_to_usbdev(intf);
-	struct wacom *wacom;
-	struct wacom_wac *wacom_wac;
-	struct wacom_features *features;
+	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
+	struct wacom_features *features = &wacom_wac->features;
+	struct hid_device *hdev = wacom->hdev;
 	int error;
 	unsigned int connect_mask = HID_CONNECT_HIDRAW;
 
-	if (!id->driver_data)
-		return -EINVAL;
-
-	hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS;
-
-	/* hid-core sets this quirk for the boot interface */
-	hdev->quirks &= ~HID_QUIRK_NOGET;
-
-	wacom = kzalloc(sizeof(struct wacom), GFP_KERNEL);
-	if (!wacom)
-		return -ENOMEM;
-
-	hid_set_drvdata(hdev, wacom);
-	wacom->hdev = hdev;
-
-	/* ask for the report descriptor to be loaded by HID */
-	error = hid_parse(hdev);
-	if (error) {
-		hid_err(hdev, "parse failed\n");
-		goto fail_parse;
-	}
-
-	wacom_wac = &wacom->wacom_wac;
-	wacom_wac->features = *((struct wacom_features *)id->driver_data);
-	features = &wacom_wac->features;
 	features->pktlen = wacom_compute_pktlen(hdev);
-	if (features->pktlen > WACOM_PKGLEN_MAX) {
-		error = -EINVAL;
-		goto fail_pktlen;
-	}
-
-	if (features->check_for_hid_type && features->hid_type != hdev->type) {
-		error = -ENODEV;
-		goto fail_type;
-	}
-
-	wacom->usbdev = dev;
-	wacom->intf = intf;
-	mutex_init(&wacom->lock);
-	INIT_WORK(&wacom->work, wacom_wireless_work);
+	if (features->pktlen > WACOM_PKGLEN_MAX)
+		return -EINVAL;
 
 	error = wacom_allocate_inputs(wacom);
 	if (error)
-		goto fail_allocate_inputs;
+		return error;
 
 	/*
 	 * Bamboo Pad has a generic hid handling for the Pen, and we switch it
@@ -1752,7 +1712,7 @@ static int wacom_probe(struct hid_device *hdev,
 		} else if ((features->pktlen != WACOM_PKGLEN_BPAD_TOUCH) &&
 			   (features->pktlen != WACOM_PKGLEN_BPAD_TOUCH_USB)) {
 			error = -ENODEV;
-			goto fail_shared_data;
+			goto fail_allocate_inputs;
 		}
 	}
 
@@ -1772,7 +1732,7 @@ static int wacom_probe(struct hid_device *hdev,
 			 error ? "Ignoring" : "Assuming pen");
 
 		if (error)
-			goto fail_shared_data;
+			goto fail_parsed;
 
 		features->device_type |= WACOM_DEVICETYPE_PEN;
 	}
@@ -1796,14 +1756,6 @@ static int wacom_probe(struct hid_device *hdev,
 	if (error)
 		goto fail_register_inputs;
 
-	if (hdev->bus == BUS_BLUETOOTH) {
-		error = device_create_file(&hdev->dev, &dev_attr_speed);
-		if (error)
-			hid_warn(hdev,
-				 "can't create sysfs speed attribute err: %d\n",
-				 error);
-	}
-
 	if (features->type == HID_GENERIC)
 		connect_mask |= HID_CONNECT_DRIVER;
 
@@ -1844,18 +1796,80 @@ static int wacom_probe(struct hid_device *hdev,
 	return 0;
 
 fail_hw_start:
-	if (hdev->bus == BUS_BLUETOOTH)
-		device_remove_file(&hdev->dev, &dev_attr_speed);
+	hid_hw_stop(hdev);
 fail_register_inputs:
 	wacom_clean_inputs(wacom);
 	wacom_destroy_battery(wacom);
 fail_battery:
 	wacom_remove_shared_data(wacom);
 fail_shared_data:
-	wacom_clean_inputs(wacom);
+fail_parsed:
 fail_allocate_inputs:
+	wacom_clean_inputs(wacom);
+	return error;
+}
+
+static int wacom_probe(struct hid_device *hdev,
+		const struct hid_device_id *id)
+{
+	struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+	struct usb_device *dev = interface_to_usbdev(intf);
+	struct wacom *wacom;
+	struct wacom_wac *wacom_wac;
+	struct wacom_features *features;
+	int error;
+
+	if (!id->driver_data)
+		return -EINVAL;
+
+	hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS;
+
+	/* hid-core sets this quirk for the boot interface */
+	hdev->quirks &= ~HID_QUIRK_NOGET;
+
+	wacom = kzalloc(sizeof(struct wacom), GFP_KERNEL);
+	if (!wacom)
+		return -ENOMEM;
+
+	hid_set_drvdata(hdev, wacom);
+	wacom->hdev = hdev;
+
+	wacom_wac = &wacom->wacom_wac;
+	wacom_wac->features = *((struct wacom_features *)id->driver_data);
+	features = &wacom_wac->features;
+
+	if (features->check_for_hid_type && features->hid_type != hdev->type) {
+		error = -ENODEV;
+		goto fail_type;
+	}
+
+	wacom->usbdev = dev;
+	wacom->intf = intf;
+	mutex_init(&wacom->lock);
+	INIT_WORK(&wacom->work, wacom_wireless_work);
+
+	/* ask for the report descriptor to be loaded by HID */
+	error = hid_parse(hdev);
+	if (error) {
+		hid_err(hdev, "parse failed\n");
+		goto fail_parse;
+	}
+
+	error = wacom_parse_and_register(wacom);
+	if (error)
+		goto fail_parse;
+
+	if (hdev->bus == BUS_BLUETOOTH) {
+		error = device_create_file(&hdev->dev, &dev_attr_speed);
+		if (error)
+			hid_warn(hdev,
+				 "can't create sysfs speed attribute err: %d\n",
+				 error);
+	}
+
+	return 0;
+
 fail_type:
-fail_pktlen:
 fail_parse:
 	kfree(wacom);
 	hid_set_drvdata(hdev, NULL);
-- 
2.5.0

[PATCH 3/6] HID: wacom: move down wireless_work()

[Benjamin Tissoires]

If wireless_work() wants to reuse parse_and_register(), we need to have
it declared after this function.

No functional changes.

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

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 92a2c81..78e9e25 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1494,123 +1494,6 @@ static void wacom_calculate_res(struct wacom_features *features)
 						    features->unitExpo);
 }
 
-static void wacom_wireless_work(struct work_struct *work)
-{
-	struct wacom *wacom = container_of(work, struct wacom, work);
-	struct usb_device *usbdev = wacom->usbdev;
-	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
-	struct hid_device *hdev1, *hdev2;
-	struct wacom *wacom1, *wacom2;
-	struct wacom_wac *wacom_wac1, *wacom_wac2;
-	int error;
-
-	/*
-	 * Regardless if this is a disconnect or a new tablet,
-	 * remove any existing input and battery devices.
-	 */
-
-	wacom_destroy_battery(wacom);
-
-	/* Stylus interface */
-	hdev1 = usb_get_intfdata(usbdev->config->interface[1]);
-	wacom1 = hid_get_drvdata(hdev1);
-	wacom_wac1 = &(wacom1->wacom_wac);
-	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_clean_inputs(wacom2);
-
-	if (wacom_wac->pid == 0) {
-		hid_info(wacom->hdev, "wireless tablet disconnected\n");
-		wacom_wac1->shared->type = 0;
-	} else {
-		const struct hid_device_id *id = wacom_ids;
-
-		hid_info(wacom->hdev, "wireless tablet connected with PID %x\n",
-			 wacom_wac->pid);
-
-		while (id->bus) {
-			if (id->vendor == USB_VENDOR_ID_WACOM &&
-			    id->product == wacom_wac->pid)
-				break;
-			id++;
-		}
-
-		if (!id->bus) {
-			hid_info(wacom->hdev, "ignoring unknown PID.\n");
-			return;
-		}
-
-		/* Stylus interface */
-		wacom_wac1->features =
-			*((struct wacom_features *)id->driver_data);
-		wacom_wac1->features.device_type |= WACOM_DEVICETYPE_PEN;
-		wacom_set_default_phy(&wacom_wac1->features);
-		wacom_calculate_res(&wacom_wac1->features);
-		snprintf(wacom_wac1->pen_name, WACOM_NAME_MAX, "%s (WL) Pen",
-			 wacom_wac1->features.name);
-		if (wacom_wac1->features.type < BAMBOO_PEN ||
-		    wacom_wac1->features.type > BAMBOO_PT) {
-			snprintf(wacom_wac1->pad_name, WACOM_NAME_MAX, "%s (WL) Pad",
-				 wacom_wac1->features.name);
-			wacom_wac1->features.device_type |= WACOM_DEVICETYPE_PAD;
-		}
-		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_allocate_inputs(wacom1) ||
-			wacom_register_inputs(wacom1);
-		if (error)
-			goto fail;
-
-		/* Touch interface */
-		if (wacom_wac1->features.touch_max ||
-		    (wacom_wac1->features.type >= INTUOSHT &&
-		    wacom_wac1->features.type <= BAMBOO_PT)) {
-			wacom_wac2->features =
-				*((struct wacom_features *)id->driver_data);
-			wacom_wac2->features.pktlen = WACOM_PKGLEN_BBTOUCH3;
-			wacom_set_default_phy(&wacom_wac2->features);
-			wacom_wac2->features.x_max = wacom_wac2->features.y_max = 4096;
-			wacom_calculate_res(&wacom_wac2->features);
-			snprintf(wacom_wac2->touch_name, WACOM_NAME_MAX,
-				 "%s (WL) Finger",wacom_wac2->features.name);
-			if (wacom_wac1->features.touch_max)
-				wacom_wac2->features.device_type |= WACOM_DEVICETYPE_TOUCH;
-			if (wacom_wac1->features.type >= INTUOSHT &&
-			    wacom_wac1->features.type <= BAMBOO_PT) {
-				snprintf(wacom_wac2->pad_name, WACOM_NAME_MAX,
-					 "%s (WL) Pad",wacom_wac2->features.name);
-				wacom_wac2->features.device_type |= WACOM_DEVICETYPE_PAD;
-			}
-			wacom_wac2->pid = wacom_wac->pid;
-			error = wacom_allocate_inputs(wacom2) ||
-				wacom_register_inputs(wacom2);
-			if (error)
-				goto fail;
-
-			if ((wacom_wac1->features.type == INTUOSHT ||
-			    wacom_wac1->features.type == INTUOSHT2) &&
-			    wacom_wac1->features.touch_max)
-				wacom_wac->shared->touch_input = wacom_wac2->touch_input;
-		}
-
-		error = wacom_initialize_battery(wacom);
-		if (error)
-			goto fail;
-	}
-
-	return;
-
-fail:
-	wacom_clean_inputs(wacom1);
-	wacom_clean_inputs(wacom2);
-	return;
-}
-
 void wacom_battery_work(struct work_struct *work)
 {
 	struct wacom *wacom = container_of(work, struct wacom, work);
@@ -1809,6 +1692,123 @@ fail_allocate_inputs:
 	return error;
 }
 
+static void wacom_wireless_work(struct work_struct *work)
+{
+	struct wacom *wacom = container_of(work, struct wacom, work);
+	struct usb_device *usbdev = wacom->usbdev;
+	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
+	struct hid_device *hdev1, *hdev2;
+	struct wacom *wacom1, *wacom2;
+	struct wacom_wac *wacom_wac1, *wacom_wac2;
+	int error;
+
+	/*
+	 * Regardless if this is a disconnect or a new tablet,
+	 * remove any existing input and battery devices.
+	 */
+
+	wacom_destroy_battery(wacom);
+
+	/* Stylus interface */
+	hdev1 = usb_get_intfdata(usbdev->config->interface[1]);
+	wacom1 = hid_get_drvdata(hdev1);
+	wacom_wac1 = &(wacom1->wacom_wac);
+	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_clean_inputs(wacom2);
+
+	if (wacom_wac->pid == 0) {
+		hid_info(wacom->hdev, "wireless tablet disconnected\n");
+		wacom_wac1->shared->type = 0;
+	} else {
+		const struct hid_device_id *id = wacom_ids;
+
+		hid_info(wacom->hdev, "wireless tablet connected with PID %x\n",
+			 wacom_wac->pid);
+
+		while (id->bus) {
+			if (id->vendor == USB_VENDOR_ID_WACOM &&
+			    id->product == wacom_wac->pid)
+				break;
+			id++;
+		}
+
+		if (!id->bus) {
+			hid_info(wacom->hdev, "ignoring unknown PID.\n");
+			return;
+		}
+
+		/* Stylus interface */
+		wacom_wac1->features =
+			*((struct wacom_features *)id->driver_data);
+		wacom_wac1->features.device_type |= WACOM_DEVICETYPE_PEN;
+		wacom_set_default_phy(&wacom_wac1->features);
+		wacom_calculate_res(&wacom_wac1->features);
+		snprintf(wacom_wac1->pen_name, WACOM_NAME_MAX, "%s (WL) Pen",
+			 wacom_wac1->features.name);
+		if (wacom_wac1->features.type < BAMBOO_PEN ||
+		    wacom_wac1->features.type > BAMBOO_PT) {
+			snprintf(wacom_wac1->pad_name, WACOM_NAME_MAX,
+				 "%s (WL) Pad", wacom_wac1->features.name);
+			wacom_wac1->features.device_type |= WACOM_DEVICETYPE_PAD;
+		}
+		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_allocate_inputs(wacom1) ||
+			wacom_register_inputs(wacom1);
+		if (error)
+			goto fail;
+
+		/* Touch interface */
+		if (wacom_wac1->features.touch_max ||
+		    (wacom_wac1->features.type >= INTUOSHT &&
+		    wacom_wac1->features.type <= BAMBOO_PT)) {
+			wacom_wac2->features =
+				*((struct wacom_features *)id->driver_data);
+			wacom_wac2->features.pktlen = WACOM_PKGLEN_BBTOUCH3;
+			wacom_set_default_phy(&wacom_wac2->features);
+			wacom_wac2->features.x_max = wacom_wac2->features.y_max = 4096;
+			wacom_calculate_res(&wacom_wac2->features);
+			snprintf(wacom_wac2->touch_name, WACOM_NAME_MAX,
+				 "%s (WL) Finger", wacom_wac2->features.name);
+			if (wacom_wac1->features.touch_max)
+				wacom_wac2->features.device_type |= WACOM_DEVICETYPE_TOUCH;
+			if (wacom_wac1->features.type >= INTUOSHT &&
+			    wacom_wac1->features.type <= BAMBOO_PT) {
+				snprintf(wacom_wac2->pad_name, WACOM_NAME_MAX,
+					 "%s (WL) Pad", wacom_wac2->features.name);
+				wacom_wac2->features.device_type |= WACOM_DEVICETYPE_PAD;
+			}
+			wacom_wac2->pid = wacom_wac->pid;
+			error = wacom_allocate_inputs(wacom2) ||
+				wacom_register_inputs(wacom2);
+			if (error)
+				goto fail;
+
+			if ((wacom_wac1->features.type == INTUOSHT ||
+			    wacom_wac1->features.type == INTUOSHT2) &&
+			    wacom_wac1->features.touch_max)
+				wacom_wac->shared->touch_input = wacom_wac2->touch_input;
+		}
+
+		error = wacom_initialize_battery(wacom);
+		if (error)
+			goto fail;
+	}
+
+	return;
+
+fail:
+	wacom_clean_inputs(wacom1);
+	wacom_clean_inputs(wacom2);
+	return;
+}
+
 static int wacom_probe(struct hid_device *hdev,
 		const struct hid_device_id *id)
 {
-- 
2.5.0

[PATCH 4/6] HID: wacom: reuse wacom_parse_and_register() in wireless_work

[Benjamin Tissoires]

Removes duplicated code.
The only difference is that we now need to stop and start the attached hid
device, but this is a small cost.

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

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 78e9e25..5cb8528 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1525,7 +1525,7 @@ static size_t wacom_compute_pktlen(struct hid_device *hdev)
 	return size;
 }
 
-static void wacom_update_name(struct wacom *wacom)
+static void wacom_update_name(struct wacom *wacom, const char *suffix)
 {
 	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
 	struct wacom_features *features = &wacom_wac->features;
@@ -1561,14 +1561,14 @@ static void wacom_update_name(struct wacom *wacom)
 
 	/* Append the device type to the name */
 	snprintf(wacom_wac->pen_name, sizeof(wacom_wac->pen_name),
-		"%s Pen", name);
+		"%s%s Pen", name, suffix);
 	snprintf(wacom_wac->touch_name, sizeof(wacom_wac->touch_name),
-		"%s Finger", name);
+		"%s%s Finger", name, suffix);
 	snprintf(wacom_wac->pad_name, sizeof(wacom_wac->pad_name),
-		"%s Pad", name);
+		"%s%s Pad", name, suffix);
 }
 
-static int wacom_parse_and_register(struct wacom *wacom)
+static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
 {
 	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
 	struct wacom_features *features = &wacom_wac->features;
@@ -1622,7 +1622,7 @@ static int wacom_parse_and_register(struct wacom *wacom)
 
 	wacom_calculate_res(features);
 
-	wacom_update_name(wacom);
+	wacom_update_name(wacom, wireless ? " (WL)" : "");
 
 	error = wacom_add_shared_data(hdev);
 	if (error)
@@ -1649,8 +1649,10 @@ static int wacom_parse_and_register(struct wacom *wacom)
 		goto fail_hw_start;
 	}
 
-	/* Note that if query fails it is not a hard failure */
-	wacom_query_tablet_data(hdev, features);
+	if (!wireless) {
+		/* Note that if query fails it is not a hard failure */
+		wacom_query_tablet_data(hdev, features);
+	}
 
 	/* touch only Bamboo doesn't support pen */
 	if ((features->type == BAMBOO_TOUCH) &&
@@ -1745,22 +1747,10 @@ static void wacom_wireless_work(struct work_struct *work)
 		/* Stylus interface */
 		wacom_wac1->features =
 			*((struct wacom_features *)id->driver_data);
-		wacom_wac1->features.device_type |= WACOM_DEVICETYPE_PEN;
-		wacom_set_default_phy(&wacom_wac1->features);
-		wacom_calculate_res(&wacom_wac1->features);
-		snprintf(wacom_wac1->pen_name, WACOM_NAME_MAX, "%s (WL) Pen",
-			 wacom_wac1->features.name);
-		if (wacom_wac1->features.type < BAMBOO_PEN ||
-		    wacom_wac1->features.type > BAMBOO_PT) {
-			snprintf(wacom_wac1->pad_name, WACOM_NAME_MAX,
-				 "%s (WL) Pad", wacom_wac1->features.name);
-			wacom_wac1->features.device_type |= WACOM_DEVICETYPE_PAD;
-		}
-		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_allocate_inputs(wacom1) ||
-			wacom_register_inputs(wacom1);
+		hid_hw_stop(hdev1);
+		error = wacom_parse_and_register(wacom1, true);
 		if (error)
 			goto fail;
 
@@ -1770,30 +1760,11 @@ static void wacom_wireless_work(struct work_struct *work)
 		    wacom_wac1->features.type <= BAMBOO_PT)) {
 			wacom_wac2->features =
 				*((struct wacom_features *)id->driver_data);
-			wacom_wac2->features.pktlen = WACOM_PKGLEN_BBTOUCH3;
-			wacom_set_default_phy(&wacom_wac2->features);
-			wacom_wac2->features.x_max = wacom_wac2->features.y_max = 4096;
-			wacom_calculate_res(&wacom_wac2->features);
-			snprintf(wacom_wac2->touch_name, WACOM_NAME_MAX,
-				 "%s (WL) Finger", wacom_wac2->features.name);
-			if (wacom_wac1->features.touch_max)
-				wacom_wac2->features.device_type |= WACOM_DEVICETYPE_TOUCH;
-			if (wacom_wac1->features.type >= INTUOSHT &&
-			    wacom_wac1->features.type <= BAMBOO_PT) {
-				snprintf(wacom_wac2->pad_name, WACOM_NAME_MAX,
-					 "%s (WL) Pad", wacom_wac2->features.name);
-				wacom_wac2->features.device_type |= WACOM_DEVICETYPE_PAD;
-			}
 			wacom_wac2->pid = wacom_wac->pid;
-			error = wacom_allocate_inputs(wacom2) ||
-				wacom_register_inputs(wacom2);
+			hid_hw_stop(hdev2);
+			error = wacom_parse_and_register(wacom2, true);
 			if (error)
 				goto fail;
-
-			if ((wacom_wac1->features.type == INTUOSHT ||
-			    wacom_wac1->features.type == INTUOSHT2) &&
-			    wacom_wac1->features.touch_max)
-				wacom_wac->shared->touch_input = wacom_wac2->touch_input;
 		}
 
 		error = wacom_initialize_battery(wacom);
@@ -1855,7 +1826,7 @@ static int wacom_probe(struct hid_device *hdev,
 		goto fail_parse;
 	}
 
-	error = wacom_parse_and_register(wacom);
+	error = wacom_parse_and_register(wacom, false);
 	if (error)
 		goto fail_parse;
 
-- 
2.5.0

[PATCH 5/6] HID: wacom: cleanup input devices

[Benjamin Tissoires]

Just some cleaning up when the input devices are unregistered.

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

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 5cb8528..9a41035 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1357,6 +1357,9 @@ static void wacom_clean_inputs(struct wacom *wacom)
 	wacom->wacom_wac.pen_input = NULL;
 	wacom->wacom_wac.touch_input = NULL;
 	wacom->wacom_wac.pad_input = NULL;
+	wacom->wacom_wac.pen_registered = false;
+	wacom->wacom_wac.touch_registered = false;
+	wacom->wacom_wac.pad_registered = false;
 	wacom_destroy_leds(wacom);
 }
 
-- 
2.5.0

[PATCH 6/6] HID: wacom: close the wireless receiver on remove()

[Benjamin Tissoires]

rmmod/insmod the wacom.ko module does not work for the receiver because
it was not previously closed. Now, we can hack with the wireless receiver
without having to unplug/replug it.

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

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 9a41035..68a5609 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1853,6 +1853,11 @@ fail_parse:
 static void wacom_remove(struct hid_device *hdev)
 {
 	struct wacom *wacom = hid_get_drvdata(hdev);
+	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
+	struct wacom_features *features = &wacom_wac->features;
+
+	if (features->device_type & WACOM_DEVICETYPE_WL_MONITOR)
+		hid_hw_close(hdev);
 
 	hid_hw_stop(hdev);
 
-- 
2.5.0

Re: [PATCH 0/6] HID: wacom: cleanup

[Jiri Kosina]

On Fri, 12 Feb 2016, Benjamin Tissoires wrote:

> While working on the wacom.ko driver, I ended up having those 6 patches that
> the current wacom.ko could benefit right now.
> 
> Besides cleaning, the only gain users will see is in patch 6 which allows to
> actually use the wireless receiver (for development) without having to
> unplug/replug it.

Looks good to me. Ping, Jason, can I please have your Ack on this?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 0/6] HID: wacom: cleanup

[Jiri Kosina]

On Fri, 12 Feb 2016, Benjamin Tissoires wrote:

> While working on the wacom.ko driver, I ended up having those 6 patches that
> the current wacom.ko could benefit right now.
> 
> Besides cleaning, the only gain users will see is in patch 6 which allows to
> actually use the wireless receiver (for development) without having to
> unplug/replug it.

Applied to for-4.6/wacom. Thanks,

-- 
Jiri Kosina
SUSE Labs