Skip to contentSkip to conversation

HID: wacom: Simplify 'wacom_update_name'

PATCH In review Jason Gerecke 2 participants 6 messages Jun 16, 2015 – Jun 18, 2015

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

Jason Gerecke

author
#0 lore raw

PATCH v2 1/5 HID: wacom: Simplify 'wacom_update_name'

A little bit of cleanup work for 'wacom_update_name' to make it easier on the eyes. Creates a temporary 'name' variable on which we'll perform our edits. Once the name is in its final form, it will be copied (with appropriate suffix) to 'wacom_wac->name' and 'wacom_wac->pad_name'.

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

---
No changes from v1.

 drivers/hid/wacom_sys.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)
drivers/hid/wacom_sys.c +14 -11
index eea18a6..bdf31c9 100644
@@ -1417,6 +1417,7 @@ static void wacom_update_name(struct wacom *wacom)
 {
 	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
 	struct wacom_features *features = &wacom_wac->features;
+	char name[WACOM_NAME_MAX];
 
 	/* Generic devices name unspecified */
 	if ((features->type == HID_GENERIC) && !strcmp("Wacom HID", features->name)) {

@@ -1424,41 +1425,43 @@ static void wacom_update_name(struct wacom *wacom)
 		    strstr(wacom->hdev->name, "wacom") ||
 		    strstr(wacom->hdev->name, "WACOM")) {
 			/* name is in HID descriptor, use it */
-			strlcpy(wacom_wac->name, wacom->hdev->name,
-				sizeof(wacom_wac->name));
+			strlcpy(name, wacom->hdev->name, sizeof(name));
 
 			/* strip out excess whitespaces */
 			while (1) {
-				char *gap = strstr(wacom_wac->name, "  ");
+				char *gap = strstr(name, "  ");
 				if (gap == NULL)
 					break;
 				/* shift everything including the terminator */
 				memmove(gap, gap+1, strlen(gap));
 			}
 			/* get rid of trailing whitespace */
-			if (wacom_wac->name[strlen(wacom_wac->name)-1] == ' ')
-				wacom_wac->name[strlen(wacom_wac->name)-1] = '\0';
+			if (name[strlen(name)-1] == ' ')
+				name[strlen(name)-1] = '\0';
 		} else {
 			/* no meaningful name retrieved. use product ID */
-			snprintf(wacom_wac->name, sizeof(wacom_wac->name),
+			snprintf(name, sizeof(name),
 				 "%s %X", features->name, wacom->hdev->product);
 		}
 	} else {
-		strlcpy(wacom_wac->name, features->name, sizeof(wacom_wac->name));
+		strlcpy(name, features->name, sizeof(name));
 	}
 
 	/* Append the device type to the name */
 	snprintf(wacom_wac->pad_name, sizeof(wacom_wac->pad_name),
-		"%s Pad", wacom_wac->name);
+		"%s Pad", name);
 
 	if (features->device_type == BTN_TOOL_PEN) {
-		strlcat(wacom_wac->name, " Pen", WACOM_NAME_MAX);
+		snprintf(wacom_wac->name, sizeof(wacom_wac->name),
+			"%s Pen", name);
 	}
 	else if (features->device_type == BTN_TOOL_FINGER) {
 		if (features->touch_max)
-			strlcat(wacom_wac->name, " Finger", WACOM_NAME_MAX);
+			snprintf(wacom_wac->name, sizeof(wacom_wac->name),
+				"%s Finger", name);
 		else
-			strlcat(wacom_wac->name, " Pad", WACOM_NAME_MAX);
+			snprintf(wacom_wac->name, sizeof(wacom_wac->name),
+				"%s Pad", name);
 	}
 }
Signature · 1 line
2.4.2
author
#1 lore raw

PATCH v2 2/5 HID: wacom: Treat features->device_type values as flags

The USB devices that this driver has historically supported segregate the pen and touch portions of the tablet. Oftentimes the segregation would be done at the interface level, though on occasion (e.g. Cintiq 24HDT) the tablet would combine two totally independent USB devices behind an internal USB hub. Because pen and touch never shared the same interface, it made sense for the 'device_type' to store a single value: "pen" or "touch".

Recently, however, some I2C devices have been created which combine the two. A first step to accomodating this is to expand 'device_type' so that it can represent two (or potentially more) types simultaneously. To do this, we treat it as a bitfield and set/check individual bits rather than using the '=' and '==' operators.

This should not result in any functional change since no supported devices (that I'm aware of, at least) have HID descriptors that indicate both pen and touch reports on a single interface.

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

---
No changes from v1.

 drivers/hid/wacom_sys.c | 35 ++++++++++++++++++-----------------
 drivers/hid/wacom_wac.c | 30 +++++++++++++++---------------
 drivers/hid/wacom_wac.h |  5 +++++
 3 files changed, 38 insertions(+), 32 deletions(-)
drivers/hid/wacom_sys.c +18 -17
index bdf31c9..2b4cbd8 100644
@@ -197,9 +197,9 @@ static void wacom_usage_mapping(struct hid_device *hdev,
 	* values commonly reported.
 	*/
 	if (pen)
-		features->device_type = BTN_TOOL_PEN;
+		features->device_type |= WACOM_DEVICETYPE_PEN;
 	else if (finger)
-		features->device_type = BTN_TOOL_FINGER;
+		features->device_type |= WACOM_DEVICETYPE_TOUCH;
 	else
 		return;
 

@@ -411,7 +411,7 @@ static int wacom_query_tablet_data(struct hid_device *hdev,
 	if (features->type == HID_GENERIC)
 		return wacom_hid_set_device_mode(hdev);
 
-	if (features->device_type == BTN_TOOL_FINGER) {
+	if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
 		if (features->type > TABLETPC) {
 			/* MT Tablet PC touch */
 			return wacom_set_device_mode(hdev, 3, 4, 4);

@@ -425,7 +425,7 @@ static int wacom_query_tablet_data(struct hid_device *hdev,
 		else if (features->type == BAMBOO_PAD) {
 			return wacom_set_device_mode(hdev, 2, 2, 2);
 		}
-	} else if (features->device_type == BTN_TOOL_PEN) {
+	} else if (features->device_type & WACOM_DEVICETYPE_PEN) {
 		if (features->type <= BAMBOO_PT && features->type != WIRELESS) {
 			return wacom_set_device_mode(hdev, 2, 2, 2);
 		}

@@ -454,9 +454,9 @@ static void wacom_retrieve_hid_descriptor(struct hid_device *hdev,
 	 */
 	if (features->type == WIRELESS) {
 		if (intf->cur_altsetting->desc.bInterfaceNumber == 0) {
-			features->device_type = 0;
+			features->device_type = WACOM_DEVICETYPE_NONE;
 		} else if (intf->cur_altsetting->desc.bInterfaceNumber == 2) {
-			features->device_type = BTN_TOOL_FINGER;
+			features->device_type |= WACOM_DEVICETYPE_TOUCH;
 			features->pktlen = WACOM_PKGLEN_BBTOUCH3;
 		}
 	}

@@ -538,9 +538,9 @@ static int wacom_add_shared_data(struct hid_device *hdev)
 
 	wacom_wac->shared = &data->shared;
 
-	if (wacom_wac->features.device_type == BTN_TOOL_FINGER)
+	if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
 		wacom_wac->shared->touch = hdev;
-	else if (wacom_wac->features.device_type == BTN_TOOL_PEN)
+	else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
 		wacom_wac->shared->pen = hdev;
 
 out:

@@ -892,7 +892,7 @@ static int wacom_initialize_leds(struct wacom *wacom)
 	case INTUOSPS:
 	case INTUOSPM:
 	case INTUOSPL:
-		if (wacom->wacom_wac.features.device_type == BTN_TOOL_PEN) {
+		if (wacom->wacom_wac.features.device_type & WACOM_DEVICETYPE_PEN) {
 			wacom->led.select[0] = 0;
 			wacom->led.select[1] = 0;
 			wacom->led.llv = 32;

@@ -948,7 +948,7 @@ static void wacom_destroy_leds(struct wacom *wacom)
 	case INTUOSPS:
 	case INTUOSPM:
 	case INTUOSPL:
-		if (wacom->wacom_wac.features.device_type == BTN_TOOL_PEN)
+		if (wacom->wacom_wac.features.device_type & WACOM_DEVICETYPE_PEN)
 			sysfs_remove_group(&wacom->hdev->dev.kobj,
 					   &intuos5_led_attr_group);
 		break;

@@ -1296,7 +1296,7 @@ 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 = BTN_TOOL_PEN;
+		wacom_wac1->features.device_type |= WACOM_DEVICETYPE_PEN;
 		snprintf(wacom_wac1->name, WACOM_NAME_MAX, "%s (WL) Pen",
 			 wacom_wac1->features.name);
 		snprintf(wacom_wac1->pad_name, WACOM_NAME_MAX, "%s (WL) Pad",

@@ -1315,7 +1315,7 @@ static void wacom_wireless_work(struct work_struct *work)
 			wacom_wac2->features =
 				*((struct wacom_features *)id->driver_data);
 			wacom_wac2->features.pktlen = WACOM_PKGLEN_BBTOUCH3;
-			wacom_wac2->features.device_type = BTN_TOOL_FINGER;
+			wacom_wac2->features.device_type |= WACOM_DEVICETYPE_TOUCH;
 			wacom_wac2->features.x_max = wacom_wac2->features.y_max = 4096;
 			if (wacom_wac2->features.touch_max)
 				snprintf(wacom_wac2->name, WACOM_NAME_MAX,

@@ -1451,11 +1451,11 @@ static void wacom_update_name(struct wacom *wacom)
 	snprintf(wacom_wac->pad_name, sizeof(wacom_wac->pad_name),
 		"%s Pad", name);
 
-	if (features->device_type == BTN_TOOL_PEN) {
+	if (features->device_type & WACOM_DEVICETYPE_PEN) {
 		snprintf(wacom_wac->name, sizeof(wacom_wac->name),
 			"%s Pen", name);
 	}
-	else if (features->device_type == BTN_TOOL_FINGER) {
+	else if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
 		if (features->touch_max)
 			snprintf(wacom_wac->name, sizeof(wacom_wac->name),
 				"%s Finger", name);

@@ -1545,7 +1545,8 @@ static int wacom_probe(struct hid_device *hdev,
 	wacom_retrieve_hid_descriptor(hdev, features);
 	wacom_setup_device_quirks(wacom);
 
-	if (!features->device_type && features->type != WIRELESS) {
+	if (features->device_type == WACOM_DEVICETYPE_NONE &&
+	    features->type != WIRELESS) {
 		error = features->type == HID_GENERIC ? -ENODEV : 0;
 
 		dev_warn(&hdev->dev, "Unknown device_type for '%s'. %s.",

@@ -1555,7 +1556,7 @@ static int wacom_probe(struct hid_device *hdev,
 		if (error)
 			goto fail_shared_data;
 
-		features->device_type = BTN_TOOL_PEN;
+		features->device_type |= WACOM_DEVICETYPE_PEN;
 	}
 
 	wacom_calculate_res(features);

@@ -1604,7 +1605,7 @@ static int wacom_probe(struct hid_device *hdev,
 		error = hid_hw_open(hdev);
 
 	if (wacom_wac->features.type == INTUOSHT && wacom_wac->features.touch_max) {
-		if (wacom_wac->features.device_type == BTN_TOOL_FINGER)
+		if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
 			wacom_wac->shared->touch_input = wacom_wac->input;
 	}
drivers/hid/wacom_wac.c +15 -15
index a52fc25..5e7710d 100644
@@ -2168,7 +2168,7 @@ void wacom_setup_device_quirks(struct wacom *wacom)
 	struct wacom_features *features = &wacom->wacom_wac.features;
 
 	/* touch device found but size is not defined. use default */
-	if (features->device_type == BTN_TOOL_FINGER && !features->x_max) {
+	if (features->device_type & WACOM_DEVICETYPE_TOUCH && !features->x_max) {
 		features->x_max = 1023;
 		features->y_max = 1023;
 	}

@@ -2182,7 +2182,7 @@ void wacom_setup_device_quirks(struct wacom *wacom)
 	if ((features->type >= INTUOS5S && features->type <= INTUOSHT) ||
 		(features->type == BAMBOO_PT)) {
 		if (features->pktlen == WACOM_PKGLEN_BBTOUCH3) {
-			features->device_type = BTN_TOOL_FINGER;
+			features->device_type |= WACOM_DEVICETYPE_TOUCH;
 
 			features->x_max = 4096;
 			features->y_max = 4096;

@@ -2197,7 +2197,7 @@ void wacom_setup_device_quirks(struct wacom *wacom)
 	 * so rewrite this one to be of type BTN_TOOL_FINGER.
 	 */
 	if (features->type == BAMBOO_PAD)
-		features->device_type = BTN_TOOL_FINGER;
+		features->device_type |= WACOM_DEVICETYPE_TOUCH;
 
 	if (wacom->hdev->bus == BUS_BLUETOOTH)
 		features->quirks |= WACOM_QUIRK_BATTERY;

@@ -2218,7 +2218,7 @@ void wacom_setup_device_quirks(struct wacom *wacom)
 		features->quirks |= WACOM_QUIRK_NO_INPUT;
 
 		/* must be monitor interface if no device_type set */
-		if (!features->device_type) {
+		if (features->device_type == WACOM_DEVICETYPE_NONE) {
 			features->quirks |= WACOM_QUIRK_MONITOR;
 			features->quirks |= WACOM_QUIRK_BATTERY;
 		}

@@ -2230,7 +2230,7 @@ static void wacom_abs_set_axis(struct input_dev *input_dev,
 {
 	struct wacom_features *features = &wacom_wac->features;
 
-	if (features->device_type == BTN_TOOL_PEN) {
+	if (features->device_type & WACOM_DEVICETYPE_PEN) {
 		input_set_abs_params(input_dev, ABS_X, features->x_min,
 				     features->x_max, features->x_fuzz, 0);
 		input_set_abs_params(input_dev, ABS_Y, features->y_min,

@@ -2349,7 +2349,7 @@ int wacom_setup_pentouch_input_capabilities(struct input_dev *input_dev,
 	case INTUOSPS:
 		__set_bit(INPUT_PROP_POINTER, input_dev->propbit);
 
-		if (features->device_type == BTN_TOOL_PEN) {
+		if (features->device_type & WACOM_DEVICETYPE_PEN) {
 			input_set_abs_params(input_dev, ABS_DISTANCE, 0,
 					      features->distance_max,
 					      0, 0);

@@ -2358,7 +2358,7 @@ int wacom_setup_pentouch_input_capabilities(struct input_dev *input_dev,
 			input_abs_set_res(input_dev, ABS_Z, 287);
 
 			wacom_setup_intuos(wacom_wac);
-		} else if (features->device_type == BTN_TOOL_FINGER) {
+		} else if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
 			__clear_bit(ABS_MISC, input_dev->absbit);
 
 			input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,

@@ -2370,7 +2370,7 @@ int wacom_setup_pentouch_input_capabilities(struct input_dev *input_dev,
 		break;
 
 	case WACOM_24HDT:
-		if (features->device_type == BTN_TOOL_FINGER) {
+		if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
 			input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0, features->x_max, 0, 0);
 			input_set_abs_params(input_dev, ABS_MT_WIDTH_MAJOR, 0, features->x_max, 0, 0);
 			input_set_abs_params(input_dev, ABS_MT_WIDTH_MINOR, 0, features->y_max, 0, 0);

@@ -2383,7 +2383,7 @@ int wacom_setup_pentouch_input_capabilities(struct input_dev *input_dev,
 	case MTTPC:
 	case MTTPC_B:
 	case TABLETPC2FG:
-		if (features->device_type == BTN_TOOL_FINGER && features->touch_max > 1)
+		if (features->device_type & WACOM_DEVICETYPE_TOUCH && features->touch_max > 1)
 			input_mt_init_slots(input_dev, features->touch_max, INPUT_MT_DIRECT);
 		/* fall through */
 

@@ -2393,7 +2393,7 @@ int wacom_setup_pentouch_input_capabilities(struct input_dev *input_dev,
 
 		__set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
 
-		if (features->device_type != BTN_TOOL_PEN)
+		if (!(features->device_type & WACOM_DEVICETYPE_PEN))
 			break;  /* no need to process stylus stuff */
 
 		/* fall through */

@@ -2424,7 +2424,7 @@ int wacom_setup_pentouch_input_capabilities(struct input_dev *input_dev,
 
 	case INTUOSHT:
 		if (features->touch_max &&
-		    features->device_type == BTN_TOOL_FINGER) {
+		    features->device_type & WACOM_DEVICETYPE_TOUCH) {
 			input_dev->evbit[0] |= BIT_MASK(EV_SW);
 			__set_bit(SW_MUTE_DEVICE, input_dev->swbit);
 		}

@@ -2433,7 +2433,7 @@ int wacom_setup_pentouch_input_capabilities(struct input_dev *input_dev,
 	case BAMBOO_PT:
 		__clear_bit(ABS_MISC, input_dev->absbit);
 
-		if (features->device_type == BTN_TOOL_FINGER) {
+		if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
 
 			if (features->touch_max) {
 				if (features->pktlen == WACOM_PKGLEN_BBTOUCH3) {

@@ -2454,7 +2454,7 @@ int wacom_setup_pentouch_input_capabilities(struct input_dev *input_dev,
 				/* PAD is setup by wacom_setup_pad_input_capabilities later */
 				return 1;
 			}
-		} else if (features->device_type == BTN_TOOL_PEN) {
+		} else if (features->device_type & WACOM_DEVICETYPE_PEN) {
 			__set_bit(INPUT_PROP_POINTER, input_dev->propbit);
 			__set_bit(BTN_TOOL_RUBBER, input_dev->keybit);
 			__set_bit(BTN_TOOL_PEN, input_dev->keybit);

@@ -2619,7 +2619,7 @@ int wacom_setup_pad_input_capabilities(struct input_dev *input_dev,
 	case INTUOS5S:
 	case INTUOSPS:
 		/* touch interface does not have the pad device */
-		if (features->device_type != BTN_TOOL_PEN)
+		if (!(features->device_type & WACOM_DEVICETYPE_PEN))
 			return -ENODEV;
 
 		for (i = 0; i < 7; i++)

@@ -2664,7 +2664,7 @@ int wacom_setup_pad_input_capabilities(struct input_dev *input_dev,
 	case INTUOSHT:
 	case BAMBOO_PT:
 		/* pad device is on the touch interface */
-		if ((features->device_type != BTN_TOOL_FINGER) ||
+		if (!(features->device_type & WACOM_DEVICETYPE_TOUCH) ||
 		    /* Bamboo Pen only tablet does not have pad */
 		    ((features->type == BAMBOO_PT) && !features->touch_max))
 			return -ENODEV;
drivers/hid/wacom_wac.h +5 -0
index 9a5ee62..da2b309 100644
@@ -72,6 +72,11 @@
 #define WACOM_QUIRK_MONITOR		0x0004
 #define WACOM_QUIRK_BATTERY		0x0008
 
+/* device types */
+#define WACOM_DEVICETYPE_NONE           0x0000
+#define WACOM_DEVICETYPE_PEN            0x0001
+#define WACOM_DEVICETYPE_TOUCH          0x0002
+
 #define WACOM_VENDORDEFINED_PEN		0xff0d0001
 
 #define WACOM_PEN_FIELD(f)	(((f)->logical == HID_DG_STYLUS) || \
Signature · 1 line
2.4.2
author
#2 lore raw

PATCH v2 3/5 HID: wacom: Introduce a new WACOM_DEVICETYPE_PAD device_type

In reply to Jason Gerecke

Historically, both the touch and pad tools would have shared the 'BTN_TOOL_FINGER' type. Any time you needed to distinguish the two, you had to use some other bit of knowledge (e.g. that the pad was on the same interface as the pen, and thus 'touch_max' would be zero).

To make these checks more readable, we introduce WACOM_DEVICETYPE_PAD. Although we still have to rely on other bits of knowledge to set this bit on the right interface (since it cannot be detected from the HID descriptor), it can be done just once inside 'wacom_setup_device_quirks'.

This patch introduces no functional changes.

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

---
Changes from v1:
* Correct conditions under which 'wacom_wireless_work' sets the pad device type flag. Fixes issue with some pad devices not showing up and others crashing.

* Set pad/touch flags for INTUOSHT and BAMBOO_PT inside the block of 'wacom_setup_device_quirks' which handles them. Ensures types are properly set on the devices before we get too far.

 drivers/hid/wacom_sys.c | 65 +++++++++++++++++++++++---------------------
 drivers/hid/wacom_wac.c | 71 ++++++++++++++++++++++++++-----------------------
 drivers/hid/wacom_wac.h |  1 +
 3 files changed, 73 insertions(+), 64 deletions(-)
drivers/hid/wacom_sys.c +35 -30
index 2b4cbd8..aaa9c84 100644
@@ -859,6 +859,9 @@ static int wacom_initialize_leds(struct wacom *wacom)
 {
 	int error;
 
+	if (!(wacom->wacom_wac.features.device_type & WACOM_DEVICETYPE_PAD))
+		return 0;
+
 	/* Initialize default values */
 	switch (wacom->wacom_wac.features.type) {
 	case INTUOS4S:

@@ -892,17 +895,14 @@ static int wacom_initialize_leds(struct wacom *wacom)
 	case INTUOSPS:
 	case INTUOSPM:
 	case INTUOSPL:
-		if (wacom->wacom_wac.features.device_type & WACOM_DEVICETYPE_PEN) {
-			wacom->led.select[0] = 0;
-			wacom->led.select[1] = 0;
-			wacom->led.llv = 32;
-			wacom->led.hlv = 0;
-			wacom->led.img_lum = 0;
-
-			error = sysfs_create_group(&wacom->hdev->dev.kobj,
-						  &intuos5_led_attr_group);
-		} else
-			return 0;
+		wacom->led.select[0] = 0;
+		wacom->led.select[1] = 0;
+		wacom->led.llv = 32;
+		wacom->led.hlv = 0;
+		wacom->led.img_lum = 0;
+
+		error = sysfs_create_group(&wacom->hdev->dev.kobj,
+					  &intuos5_led_attr_group);
 		break;
 
 	default:

@@ -925,6 +925,9 @@ static void wacom_destroy_leds(struct wacom *wacom)
 	if (!wacom->led_initialized)
 		return;
 
+	if (!(wacom->wacom_wac.features.device_type & WACOM_DEVICETYPE_PAD))
+		return;
+
 	wacom->led_initialized = false;
 
 	switch (wacom->wacom_wac.features.type) {

@@ -948,9 +951,8 @@ static void wacom_destroy_leds(struct wacom *wacom)
 	case INTUOSPS:
 	case INTUOSPM:
 	case INTUOSPL:
-		if (wacom->wacom_wac.features.device_type & WACOM_DEVICETYPE_PEN)
-			sysfs_remove_group(&wacom->hdev->dev.kobj,
-					   &intuos5_led_attr_group);
+		sysfs_remove_group(&wacom->hdev->dev.kobj,
+				   &intuos5_led_attr_group);
 		break;
 	}
 }

@@ -1297,6 +1299,9 @@ static void wacom_wireless_work(struct work_struct *work)
 		wacom_wac1->features =
 			*((struct wacom_features *)id->driver_data);
 		wacom_wac1->features.device_type |= WACOM_DEVICETYPE_PEN;
+		if (wacom_wac1->features.type != INTUOSHT &&
+		    wacom_wac1->features.type != BAMBOO_PT)
+			wacom_wac1->features.device_type |= WACOM_DEVICETYPE_PAD;
 		snprintf(wacom_wac1->name, WACOM_NAME_MAX, "%s (WL) Pen",
 			 wacom_wac1->features.name);
 		snprintf(wacom_wac1->pad_name, WACOM_NAME_MAX, "%s (WL) Pad",

@@ -1315,16 +1320,16 @@ static void wacom_wireless_work(struct work_struct *work)
 			wacom_wac2->features =
 				*((struct wacom_features *)id->driver_data);
 			wacom_wac2->features.pktlen = WACOM_PKGLEN_BBTOUCH3;
-			wacom_wac2->features.device_type |= WACOM_DEVICETYPE_TOUCH;
 			wacom_wac2->features.x_max = wacom_wac2->features.y_max = 4096;
-			if (wacom_wac2->features.touch_max)
-				snprintf(wacom_wac2->name, WACOM_NAME_MAX,
-					 "%s (WL) Finger",wacom_wac2->features.name);
-			else
-				snprintf(wacom_wac2->name, WACOM_NAME_MAX,
-					 "%s (WL) Pad",wacom_wac2->features.name);
+			snprintf(wacom_wac2->name, WACOM_NAME_MAX,
+				 "%s (WL) Finger",wacom_wac2->features.name);
 			snprintf(wacom_wac2->pad_name, WACOM_NAME_MAX,
-				 "%s (WL) Pad", wacom_wac2->features.name);
+				 "%s (WL) Pad",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)
+				wacom_wac2->features.device_type |= WACOM_DEVICETYPE_PAD;
 			wacom_wac2->pid = wacom_wac->pid;
 			error = wacom_allocate_inputs(wacom2) ||
 				wacom_register_inputs(wacom2);

@@ -1456,12 +1461,12 @@ static void wacom_update_name(struct wacom *wacom)
 			"%s Pen", name);
 	}
 	else if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
-		if (features->touch_max)
-			snprintf(wacom_wac->name, sizeof(wacom_wac->name),
-				"%s Finger", name);
-		else
-			snprintf(wacom_wac->name, sizeof(wacom_wac->name),
-				"%s Pad", name);
+		snprintf(wacom_wac->name, sizeof(wacom_wac->name),
+			"%s Finger", name);
+	}
+	else if (features->device_type & WACOM_DEVICETYPE_PAD) {
+		snprintf(wacom_wac->name, sizeof(wacom_wac->name),
+			"%s Pad", name);
 	}
 }
 

@@ -1604,8 +1609,8 @@ static int wacom_probe(struct hid_device *hdev,
 	if (features->quirks & WACOM_QUIRK_MONITOR)
 		error = hid_hw_open(hdev);
 
-	if (wacom_wac->features.type == INTUOSHT && wacom_wac->features.touch_max) {
-		if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
+	if (wacom_wac->features.type == INTUOSHT && 
+	    wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH) {
 			wacom_wac->shared->touch_input = wacom_wac->input;
 	}
drivers/hid/wacom_wac.c +37 -34

120 lines not on this page · raw message

drivers/hid/wacom_wac.h +1 -0
index da2b309..c873c9f 100644
@@ -76,6 +76,7 @@
 #define WACOM_DEVICETYPE_NONE           0x0000
 #define WACOM_DEVICETYPE_PEN            0x0001
 #define WACOM_DEVICETYPE_TOUCH          0x0002
+#define WACOM_DEVICETYPE_PAD            0x0004
 
 #define WACOM_VENDORDEFINED_PEN		0xff0d0001
Signature · 1 line
2.4.2
author
#3 lore raw

PATCH v2 4/5 HID: wacom: Split apart 'wacom_setup_pentouch_input_capabilites'

In reply to Jason Gerecke

This splits the 'wacom_setup_pentouch_input_capabilites' function into pieces dedicated to doing setup for just the pen interface and just the touch interface. This makes it easier to focus on the relevant piece when making changes.

This patch introduces no functional changes.

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

---
No changes from v1.

 drivers/hid/wacom.h     |   4 +-
 drivers/hid/wacom_sys.c |   8 +-
 drivers/hid/wacom_wac.c | 234 +++++++++++++++++++++++++-----------------------
 3 files changed, 131 insertions(+), 115 deletions(-)
drivers/hid/wacom.h +3 -1
index c76e21f..a533787 100644
@@ -135,7 +135,9 @@ extern const struct hid_device_id wacom_ids[];
 
 void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len);
 void wacom_setup_device_quirks(struct wacom *wacom);
-int wacom_setup_pentouch_input_capabilities(struct input_dev *input_dev,
+int wacom_setup_pen_input_capabilities(struct input_dev *input_dev,
+				   struct wacom_wac *wacom_wac);
+int wacom_setup_touch_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);
drivers/hid/wacom_sys.c +6 -2
index aaa9c84..ca15c7f 100644
@@ -1199,7 +1199,8 @@ 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;
+	struct wacom_features *features = &wacom_wac->features;
+	int error = 0;
 
 	input_dev = wacom_wac->input;
 	pad_input_dev = wacom_wac->pad_input;

@@ -1207,7 +1208,10 @@ static int wacom_register_inputs(struct wacom *wacom)
 	if (!input_dev || !pad_input_dev)
 		return -EINVAL;
 
-	error = wacom_setup_pentouch_input_capabilities(input_dev, wacom_wac);
+	if (features->device_type & WACOM_DEVICETYPE_PEN)
+		error = wacom_setup_pen_input_capabilities(input_dev, wacom_wac);
+	if (!error && features->device_type & WACOM_DEVICETYPE_TOUCH)
+		error = wacom_setup_touch_input_capabilities(input_dev, wacom_wac);
 	if (!error) {
 		error = input_register_device(input_dev);
 		if (error)
drivers/hid/wacom_wac.c +122 -112

284 lines not on this page · raw message

Signature · 1 line
2.4.2
author
#4 lore raw

PATCH v2 5/5 HID: wacom: Introduce new 'touch_input' device

In reply to Jason Gerecke

Instead of having a single 'input_dev' device that will take either pen or touch data depending on the type of the device, create seperate devices devices for each. By splitting things like this, we can support devices (e.g. the I2C "AES" sensors in some newer tablet PCs) that send both pen and touch reports from a single endpoint.

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

---
Changes from v1:
* Fixed mis-named devices ("Pad" -> "Pen"; "Touch" -> "Finger")

* Do not update touch state in 'wacom_bpt3_touch' unless a touchpad is actually present.

* In 'wacom_wireless_work' and 'wacom_probe', have 'wacom_wac->shared->touch_input' point to the touch input instead of the pen input.

 drivers/hid/wacom_sys.c | 116 +++++++++++++++++++++++++++++-------------------
 drivers/hid/wacom_wac.c | 115 +++++++++++++++++++++++++----------------------
 drivers/hid/wacom_wac.h |   9 ++--
 3 files changed, 139 insertions(+), 101 deletions(-)
drivers/hid/wacom_sys.c +70 -46
index ca15c7f..4c0ffca 100644
@@ -253,7 +253,7 @@ static void wacom_post_parse_hid(struct hid_device *hdev,
 	if (features->type == HID_GENERIC) {
 		/* Any last-minute generic device setup */
 		if (features->touch_max > 1) {
-			input_mt_init_slots(wacom_wac->input, wacom_wac->features.touch_max,
+			input_mt_init_slots(wacom_wac->touch_input, wacom_wac->features.touch_max,
 				    INPUT_MT_DIRECT);
 		}
 	}

@@ -1130,7 +1130,7 @@ static struct input_dev *wacom_allocate_input(struct wacom *wacom)
 	if (!input_dev)
 		return NULL;
 
-	input_dev->name = wacom_wac->name;
+	input_dev->name = wacom_wac->pen_name;
 	input_dev->phys = hdev->phys;
 	input_dev->dev.parent = &hdev->dev;
 	input_dev->open = wacom_open;

@@ -1149,27 +1149,33 @@ static void wacom_free_inputs(struct wacom *wacom)
 {
 	struct wacom_wac *wacom_wac = &(wacom->wacom_wac);
 
-	if (wacom_wac->input)
-		input_free_device(wacom_wac->input);
+	if (wacom_wac->pen_input)
+		input_free_device(wacom_wac->pen_input);
+	if (wacom_wac->touch_input)
+		input_free_device(wacom_wac->touch_input);
 	if (wacom_wac->pad_input)
 		input_free_device(wacom_wac->pad_input);
-	wacom_wac->input = NULL;
+	wacom_wac->pen_input = NULL;
+	wacom_wac->touch_input = NULL;
 	wacom_wac->pad_input = NULL;
 }
 
 static int wacom_allocate_inputs(struct wacom *wacom)
 {
-	struct input_dev *input_dev, *pad_input_dev;
+	struct input_dev *pen_input_dev, *touch_input_dev, *pad_input_dev;
 	struct wacom_wac *wacom_wac = &(wacom->wacom_wac);
 
-	input_dev = wacom_allocate_input(wacom);
+	pen_input_dev = wacom_allocate_input(wacom);
+	touch_input_dev = wacom_allocate_input(wacom);
 	pad_input_dev = wacom_allocate_input(wacom);
-	if (!input_dev || !pad_input_dev) {
+	if (!pen_input_dev || !touch_input_dev || !pad_input_dev) {
 		wacom_free_inputs(wacom);
 		return -ENOMEM;
 	}
 
-	wacom_wac->input = input_dev;
+	wacom_wac->pen_input = pen_input_dev;
+	wacom_wac->touch_input = touch_input_dev;
+	wacom_wac->touch_input->name = wacom_wac->touch_name;
 	wacom_wac->pad_input = pad_input_dev;
 	wacom_wac->pad_input->name = wacom_wac->pad_name;
 

@@ -1178,11 +1184,17 @@ static int wacom_allocate_inputs(struct wacom *wacom)
 
 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);
+	if (wacom->wacom_wac.pen_input) {
+		if (wacom->wacom_wac.pen_registered)
+			input_unregister_device(wacom->wacom_wac.pen_input);
 		else
-			input_free_device(wacom->wacom_wac.input);
+			input_free_device(wacom->wacom_wac.pen_input);
+	}
+	if (wacom->wacom_wac.touch_input) {
+		if (wacom->wacom_wac.touch_registered)
+			input_unregister_device(wacom->wacom_wac.touch_input);
+		else
+			input_free_device(wacom->wacom_wac.touch_input);
 	}
 	if (wacom->wacom_wac.pad_input) {
 		if (wacom->wacom_wac.pad_registered)

@@ -1190,33 +1202,49 @@ static void wacom_clean_inputs(struct wacom *wacom)
 		else
 			input_free_device(wacom->wacom_wac.pad_input);
 	}
-	wacom->wacom_wac.input = NULL;
+	wacom->wacom_wac.pen_input = NULL;
+	wacom->wacom_wac.touch_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 input_dev *pen_input_dev, *touch_input_dev, *pad_input_dev;
 	struct wacom_wac *wacom_wac = &(wacom->wacom_wac);
-	struct wacom_features *features = &wacom_wac->features;
 	int error = 0;
 
-	input_dev = wacom_wac->input;
+	pen_input_dev = wacom_wac->pen_input;
+	touch_input_dev = wacom_wac->touch_input;
 	pad_input_dev = wacom_wac->pad_input;
 
-	if (!input_dev || !pad_input_dev)
+	if (!pen_input_dev || !touch_input_dev || !pad_input_dev)
 		return -EINVAL;
 
-	if (features->device_type & WACOM_DEVICETYPE_PEN)
-		error = wacom_setup_pen_input_capabilities(input_dev, wacom_wac);
-	if (!error && features->device_type & WACOM_DEVICETYPE_TOUCH)
-		error = wacom_setup_touch_input_capabilities(input_dev, wacom_wac);
-	if (!error) {
-		error = input_register_device(input_dev);
+	error = wacom_setup_pen_input_capabilities(pen_input_dev, wacom_wac);
+	if (error) {
+		/* no pen in use on this interface */
+		input_free_device(pen_input_dev);
+		wacom_wac->pen_input = NULL;
+		pen_input_dev = NULL;
+	} else {
+		error = input_register_device(pen_input_dev);
+		if (error)
+			goto fail_register_pen_input;
+		wacom_wac->pen_registered = true;
+	}
+
+	error = wacom_setup_touch_input_capabilities(touch_input_dev, wacom_wac);
+	if (error) {
+		/* no touch in use on this interface */
+		input_free_device(touch_input_dev);
+		wacom_wac->touch_input = NULL;
+		touch_input_dev = NULL;
+	} else {
+		error = input_register_device(touch_input_dev);
 		if (error)
-			return error;
-		wacom_wac->input_registered = true;
+			goto fail_register_touch_input;
+		wacom_wac->touch_registered = true;
 	}
 
 	error = wacom_setup_pad_input_capabilities(pad_input_dev, wacom_wac);

@@ -1243,9 +1271,14 @@ fail_leds:
 	pad_input_dev = NULL;
 	wacom_wac->pad_registered = false;
 fail_register_pad_input:
-	input_unregister_device(input_dev);
-	wacom_wac->input = NULL;
-	wacom_wac->input_registered = false;
+	input_unregister_device(touch_input_dev);
+	wacom_wac->touch_input = NULL;
+	wacom_wac->touch_registered = false;
+fail_register_touch_input:
+	input_unregister_device(pen_input_dev);
+	wacom_wac->pen_input = NULL;
+	wacom_wac->pen_registered = false;
+fail_register_pen_input:
 	return error;
 }
 

@@ -1306,7 +1339,7 @@ static void wacom_wireless_work(struct work_struct *work)
 		if (wacom_wac1->features.type != INTUOSHT &&
 		    wacom_wac1->features.type != BAMBOO_PT)
 			wacom_wac1->features.device_type |= WACOM_DEVICETYPE_PAD;
-		snprintf(wacom_wac1->name, WACOM_NAME_MAX, "%s (WL) Pen",
+		snprintf(wacom_wac1->pen_name, WACOM_NAME_MAX, "%s (WL) Pen",
 			 wacom_wac1->features.name);
 		snprintf(wacom_wac1->pad_name, WACOM_NAME_MAX, "%s (WL) Pad",
 			 wacom_wac1->features.name);

@@ -1325,7 +1358,7 @@ static void wacom_wireless_work(struct work_struct *work)
 				*((struct wacom_features *)id->driver_data);
 			wacom_wac2->features.pktlen = WACOM_PKGLEN_BBTOUCH3;
 			wacom_wac2->features.x_max = wacom_wac2->features.y_max = 4096;
-			snprintf(wacom_wac2->name, WACOM_NAME_MAX,
+			snprintf(wacom_wac2->touch_name, WACOM_NAME_MAX,
 				 "%s (WL) Finger",wacom_wac2->features.name);
 			snprintf(wacom_wac2->pad_name, WACOM_NAME_MAX,
 				 "%s (WL) Pad",wacom_wac2->features.name);

@@ -1342,7 +1375,7 @@ static void wacom_wireless_work(struct work_struct *work)
 
 			if (wacom_wac1->features.type == INTUOSHT &&
 			    wacom_wac1->features.touch_max)
-				wacom_wac->shared->touch_input = wacom_wac2->input;
+				wacom_wac->shared->touch_input = wacom_wac2->touch_input;
 		}
 
 		error = wacom_initialize_battery(wacom);

@@ -1457,21 +1490,12 @@ 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);
+	snprintf(wacom_wac->touch_name, sizeof(wacom_wac->touch_name),
+		"%s Finger", name);
 	snprintf(wacom_wac->pad_name, sizeof(wacom_wac->pad_name),
 		"%s Pad", name);
-
-	if (features->device_type & WACOM_DEVICETYPE_PEN) {
-		snprintf(wacom_wac->name, sizeof(wacom_wac->name),
-			"%s Pen", name);
-	}
-	else if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
-		snprintf(wacom_wac->name, sizeof(wacom_wac->name),
-			"%s Finger", name);
-	}
-	else if (features->device_type & WACOM_DEVICETYPE_PAD) {
-		snprintf(wacom_wac->name, sizeof(wacom_wac->name),
-			"%s Pad", name);
-	}
 }
 
 static int wacom_probe(struct hid_device *hdev,

@@ -1615,7 +1639,7 @@ static int wacom_probe(struct hid_device *hdev,
 
 	if (wacom_wac->features.type == INTUOSHT && 
 	    wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH) {
-			wacom_wac->shared->touch_input = wacom_wac->input;
+			wacom_wac->shared->touch_input = wacom_wac->touch_input;
 	}
 
 	return 0;
drivers/hid/wacom_wac.c +63 -52

369 lines not on this page · raw message

drivers/hid/wacom_wac.h +6 -3
index c873c9f..2978c30 100644
@@ -196,7 +196,8 @@ struct hid_data {
 };
 
 struct wacom_wac {
-	char name[WACOM_NAME_MAX];
+	char pen_name[WACOM_NAME_MAX];
+	char touch_name[WACOM_NAME_MAX];
 	char pad_name[WACOM_NAME_MAX];
 	char bat_name[WACOM_NAME_MAX];
 	char ac_name[WACOM_NAME_MAX];

@@ -207,9 +208,11 @@ struct wacom_wac {
 	bool reporting_data;
 	struct wacom_features features;
 	struct wacom_shared *shared;
-	struct input_dev *input;
+	struct input_dev *pen_input;
+	struct input_dev *touch_input;
 	struct input_dev *pad_input;
-	bool input_registered;
+	bool pen_registered;
+	bool touch_registered;
 	bool pad_registered;
 	int pid;
 	int battery_capacity;
Signature · 1 line
2.4.2