Skip to contentSkip to conversation

HID: wacom: Use calculated pkglen for wireless touch interface

PATCH Applied Jason Gerecke 3 participants 5 messages Aug 3, 2015 – Aug 4, 2015

Subsystems (2)
HID HID wacom driver
lore
Share & tools
lore

Jason Gerecke

author
#0 lore raw

PATCH 1/3 HID: wacom: Use calculated pkglen for wireless touch interface

Commit 01c846f introduced the 'wacom_compute_pktlen' function which automatically determines the correct value for an interface's pkglen by scanning the HID descriptor. This function returns the correct value for the wireless receiver's touch interface, removing the need for us to set it manually here.

Signed-off-by Jason Gerecke <[email protected]>
---
 drivers/hid/wacom_sys.c | 1 -
 1 file changed, 1 deletion(-)
drivers/hid/wacom_sys.c +0 -1
index d932349..a334332 100644
@@ -457,7 +457,6 @@ static void wacom_retrieve_hid_descriptor(struct hid_device *hdev,
 			features->device_type = WACOM_DEVICETYPE_NONE;
 		} else if (intf->cur_altsetting->desc.bInterfaceNumber == 2) {
 			features->device_type |= WACOM_DEVICETYPE_TOUCH;
-			features->pktlen = WACOM_PKGLEN_BBTOUCH3;
 		}
 	}
Signature · 1 line
2.4.6
author
#1 lore raw

PATCH 2/3 HID: wacom: Replace WACOM_QUIRK_MONITOR with WACOM_DEVICETYPE_WL_MONITOR

The monitor interface on the wireless receiver is more logically expressed as a type of device instead of a quirk.

Signed-off-by Jason Gerecke <[email protected]>
---
 drivers/hid/wacom_sys.c | 6 +++---
 drivers/hid/wacom_wac.c | 4 +---
 drivers/hid/wacom_wac.h | 2 +-
 3 files changed, 5 insertions(+), 7 deletions(-)
drivers/hid/wacom_sys.c +3 -3
index a334332..13834ba 100644
@@ -454,7 +454,7 @@ static void wacom_retrieve_hid_descriptor(struct hid_device *hdev,
 	 */
 	if (features->type == WIRELESS) {
 		if (intf->cur_altsetting->desc.bInterfaceNumber == 0) {
-			features->device_type = WACOM_DEVICETYPE_NONE;
+			features->device_type = WACOM_DEVICETYPE_WL_MONITOR;
 		} else if (intf->cur_altsetting->desc.bInterfaceNumber == 2) {
 			features->device_type |= WACOM_DEVICETYPE_TOUCH;
 		}

@@ -1581,7 +1581,7 @@ static int wacom_probe(struct hid_device *hdev,
 	if (error)
 		goto fail_shared_data;
 
-	if (!(features->quirks & WACOM_QUIRK_MONITOR) &&
+	if (!(features->device_type & WACOM_DEVICETYPE_WL_MONITOR) &&
 	     (features->quirks & WACOM_QUIRK_BATTERY)) {
 		error = wacom_initialize_battery(wacom);
 		if (error)

@@ -1615,7 +1615,7 @@ 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->quirks & WACOM_QUIRK_MONITOR)
+	if (features->device_type & WACOM_DEVICETYPE_WL_MONITOR)
 		error = hid_hw_open(hdev);
 
 	if (wacom_wac->features.type == INTUOSHT &&
drivers/hid/wacom_wac.c +1 -3
index 280deb2..82bb0d3 100644
@@ -2330,9 +2330,7 @@ void wacom_setup_device_quirks(struct wacom *wacom)
 		/* monitor never has input and pen/touch have delayed create */
 		features->quirks |= WACOM_QUIRK_NO_INPUT;
 
-		/* must be monitor interface if no device_type set */
-		if (features->device_type == WACOM_DEVICETYPE_NONE) {
-			features->quirks |= WACOM_QUIRK_MONITOR;
+		if (features->device_type == WACOM_DEVICETYPE_WL_MONITOR) {
 			features->quirks |= WACOM_QUIRK_BATTERY;
 		}
 	}
drivers/hid/wacom_wac.h +1 -1
index c245a66..87df674 100644
@@ -69,7 +69,6 @@
 /* device quirks */
 #define WACOM_QUIRK_BBTOUCH_LOWRES	0x0001
 #define WACOM_QUIRK_NO_INPUT		0x0002
-#define WACOM_QUIRK_MONITOR		0x0004
 #define WACOM_QUIRK_BATTERY		0x0008
 
 /* device types */

@@ -77,6 +76,7 @@
 #define WACOM_DEVICETYPE_PEN            0x0001
 #define WACOM_DEVICETYPE_TOUCH          0x0002
 #define WACOM_DEVICETYPE_PAD            0x0004
+#define WACOM_DEVICETYPE_WL_MONITOR     0x0008
 
 #define WACOM_VENDORDEFINED_PEN		0xff0d0001
Signature · 1 line
2.4.6
author
#2 lore raw

PATCH 3/3 HID: wacom: Remove WACOM_QUIRK_NO_INPUT

In reply to Jason Gerecke

WACOM_QUIRK_NO_INPUT is a signal to the driver that input devices should not be created for a particular device. This quirk was used by the wireless receiver to prevent any devices from being created during the initial probe (defering it instead until we got a tablet connection event in 'wacom_wireless_work').

This quirk is not necessary now that a device_type is associated with each device. Any input device allocated by 'wacom_allocate_inputs' which is not necessary for a particular device is freed in 'wacom_register_inputs'. In particular, none of the wireless receivers devices have the pen, pad, or touch device types set so the same effect is achieved without the need to be explicit.

We now return early in wacom_retrieve_hid_descriptor for wireless devices (to prevent the device_type from being overridden) but since we ignore the HID descriptor for the wireless reciever anyway, this is not an issue.

Signed-off-by Jason Gerecke <[email protected]>
---
 drivers/hid/wacom_sys.c | 24 ++++++++++--------------
 drivers/hid/wacom_wac.c |  4 ----
 drivers/hid/wacom_wac.h |  1 -
 3 files changed, 10 insertions(+), 19 deletions(-)
drivers/hid/wacom_sys.c +10 -14
index 13834ba..20d15c5 100644
@@ -453,11 +453,11 @@ static void wacom_retrieve_hid_descriptor(struct hid_device *hdev,
 	 * interface number.
 	 */
 	if (features->type == WIRELESS) {
-		if (intf->cur_altsetting->desc.bInterfaceNumber == 0) {
+		if (intf->cur_altsetting->desc.bInterfaceNumber == 0)
 			features->device_type = WACOM_DEVICETYPE_WL_MONITOR;
-		} else if (intf->cur_altsetting->desc.bInterfaceNumber == 2) {
-			features->device_type |= WACOM_DEVICETYPE_TOUCH;
-		}
+		else
+			features->device_type = WACOM_DEVICETYPE_NONE;
+		return;
 	}
 
 	wacom_parse_hid(hdev, features);

@@ -1531,11 +1531,9 @@ 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;
-	}
+	error = wacom_allocate_inputs(wacom);
+	if (error)
+		goto fail_allocate_inputs;
 
 	/*
 	 * Bamboo Pad has a generic hid handling for the Pen, and we switch it

@@ -1588,11 +1586,9 @@ static int wacom_probe(struct hid_device *hdev,
 			goto fail_battery;
 	}
 
-	if (!(features->quirks & WACOM_QUIRK_NO_INPUT)) {
-		error = wacom_register_inputs(wacom);
-		if (error)
-			goto fail_register_inputs;
-	}
+	error = wacom_register_inputs(wacom);
+	if (error)
+		goto fail_register_inputs;
 
 	if (hdev->bus == BUS_BLUETOOTH) {
 		error = device_create_file(&hdev->dev, &dev_attr_speed);
drivers/hid/wacom_wac.c +0 -4
index 82bb0d3..4d11c78 100644
@@ -2326,10 +2326,6 @@ void wacom_setup_device_quirks(struct wacom *wacom)
 	}
 
 	if (features->type == WIRELESS) {
-
-		/* monitor never has input and pen/touch have delayed create */
-		features->quirks |= WACOM_QUIRK_NO_INPUT;
-
 		if (features->device_type == WACOM_DEVICETYPE_WL_MONITOR) {
 			features->quirks |= WACOM_QUIRK_BATTERY;
 		}
drivers/hid/wacom_wac.h +0 -1
index 87df674..6233eea 100644
@@ -68,7 +68,6 @@
 
 /* device quirks */
 #define WACOM_QUIRK_BBTOUCH_LOWRES	0x0001
-#define WACOM_QUIRK_NO_INPUT		0x0002
 #define WACOM_QUIRK_BATTERY		0x0008
 
 /* device types */
Signature · 1 line
2.4.6
Reviewed-by on v1 3/3
#3 lore raw

Re: [PATCH 3/3] HID: wacom: Remove WACOM_QUIRK_NO_INPUT

Quoted 13 lines · Jason Gerecke

WACOM_QUIRK_NO_INPUT is a signal to the driver that input devices should not be created for a particular device. This quirk was used by the wireless receiver to prevent any devices from being created during the initial probe (defering it instead until we got a tablet connection event in 'wacom_wireless_work').

This quirk is not necessary now that a device_type is associated with each device. Any input device allocated by 'wacom_allocate_inputs' which is not necessary for a particular device is freed in 'wacom_register_inputs'. In particular, none of the wireless receivers devices have the pen, pad, or touch device types set so the same effect is achieved without the need to be explicit.

Quoted 6 lines

We now return early in wacom_retrieve_hid_descriptor for wireless devices (to prevent the device_type from being overridden) but since we ignore the HID descriptor for the wireless reciever anyway, this is not an issue.

Signed-off-by Jason Gerecke <[email protected]>

---

This series is a nice cleanup.

For the 3 in this series:

Reviewed-by Benjamin Tissoires <[email protected]>

Cheers,
Benjamin

Quoted 85 lines
 drivers/hid/wacom_sys.c | 24 ++++++++++--------------
 drivers/hid/wacom_wac.c |  4 ----
 drivers/hid/wacom_wac.h |  1 -
 3 files changed, 10 insertions(+), 19 deletions(-)
drivers/hid/wacom_sys.c +10 -14
index 13834ba..20d15c5 100644
@@ -453,11 +453,11 @@ static void wacom_retrieve_hid_descriptor(struct hid_device *hdev,
 	 * interface number.
 	 */
 	if (features->type == WIRELESS) {
-		if (intf->cur_altsetting->desc.bInterfaceNumber == 0) {
+		if (intf->cur_altsetting->desc.bInterfaceNumber == 0)
 			features->device_type = WACOM_DEVICETYPE_WL_MONITOR;
-		} else if (intf->cur_altsetting->desc.bInterfaceNumber == 2) {
-			features->device_type |= WACOM_DEVICETYPE_TOUCH;
-		}
+		else
+			features->device_type = WACOM_DEVICETYPE_NONE;
+		return;
 	}
 
 	wacom_parse_hid(hdev, features);

@@ -1531,11 +1531,9 @@ 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;
-	}
+	error = wacom_allocate_inputs(wacom);
+	if (error)
+		goto fail_allocate_inputs;
 
 	/*
 	 * Bamboo Pad has a generic hid handling for the Pen, and we switch it

@@ -1588,11 +1586,9 @@ static int wacom_probe(struct hid_device *hdev,
 			goto fail_battery;
 	}
 
-	if (!(features->quirks & WACOM_QUIRK_NO_INPUT)) {
-		error = wacom_register_inputs(wacom);
-		if (error)
-			goto fail_register_inputs;
-	}
+	error = wacom_register_inputs(wacom);
+	if (error)
+		goto fail_register_inputs;
 
 	if (hdev->bus == BUS_BLUETOOTH) {
 		error = device_create_file(&hdev->dev, &dev_attr_speed);
drivers/hid/wacom_wac.c +0 -4
index 82bb0d3..4d11c78 100644
@@ -2326,10 +2326,6 @@ void wacom_setup_device_quirks(struct wacom *wacom)
 	}
 
 	if (features->type == WIRELESS) {
-
-		/* monitor never has input and pen/touch have delayed create */
-		features->quirks |= WACOM_QUIRK_NO_INPUT;
-
 		if (features->device_type == WACOM_DEVICETYPE_WL_MONITOR) {
 			features->quirks |= WACOM_QUIRK_BATTERY;
 		}
drivers/hid/wacom_wac.h +0 -1
index 87df674..6233eea 100644
@@ -68,7 +68,6 @@
 
 /* device quirks */
 #define WACOM_QUIRK_BBTOUCH_LOWRES	0x0001
-#define WACOM_QUIRK_NO_INPUT		0x0002
 #define WACOM_QUIRK_BATTERY		0x0008
 
 /* device types */
Signature · 1 line
2.4.6
Applied
#4 lore raw
In reply to Jason Gerecke
Quoted 7 lines · Jason Gerecke

Commit 01c846f introduced the 'wacom_compute_pktlen' function which automatically determines the correct value for an interface's pkglen by scanning the HID descriptor. This function returns the correct value for the wireless receiver's touch interface, removing the need for us to set it manually here.

Signed-off-by Jason Gerecke <[email protected]>

I have applied the series to for-4.3/wacom.

Thanks,

Signature · 2 lines
Jiri Kosina
SUSE Labs