* [PATCH 0/5] HID: wacom: Support tablets with pen and touch on same interface
@ 2015-06-04 1:18 Jason Gerecke
2015-06-04 1:18 ` [PATCH 1/5] HID: wacom: Simplify 'wacom_update_name' Jason Gerecke
` (6 more replies)
0 siblings, 7 replies; 16+ messages in thread
From: Jason Gerecke @ 2015-06-04 1:18 UTC (permalink / raw)
To: Ping Cheng, Aaron Skomra, Benjamin Tissoires, Jiri Kosina
Cc: linux-input, Jason Gerecke, Jason Gerecke
I've recently got my hands on a device which has an I2C sensor that sends
both pen and touch reports from a single interface. To userspace, it shows
up as a single input device which blends both the report types (e.g. it has
ABS_PRESSURE for the pen, and ABS_MT_POSITION_X for the touch). This patch
set modifies the driver to be able to handle devices which place both pen
and touch on a the same interface. It does this by treating the pen, touch,
and pad (which was already special-cased) independently. If a device has
the appropriate device_type, one or more of pen/touch/pad input devices
will be created, initialized, and used to send data to userspace.
Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
Jason Gerecke (5):
HID: wacom: Simplify 'wacom_update_name'
HID: wacom: Treat features->device_type values as flags
HID: wacom: Introduce a new WACOM_DEVICETYPE_PAD device_type
HID: wacom: Split apart 'wacom_setup_pentouch_input_capabilites'
HID: wacom: Introduce new 'touch_input' device
drivers/hid/wacom.h | 4 +-
drivers/hid/wacom_sys.c | 194 +++++++++++++++----------
drivers/hid/wacom_wac.c | 379 ++++++++++++++++++++++++++----------------------
drivers/hid/wacom_wac.h | 15 +-
4 files changed, 332 insertions(+), 260 deletions(-)
--
2.4.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/5] HID: wacom: Simplify 'wacom_update_name'
2015-06-04 1:18 [PATCH 0/5] HID: wacom: Support tablets with pen and touch on same interface Jason Gerecke
@ 2015-06-04 1:18 ` Jason Gerecke
2015-06-04 1:18 ` [PATCH 2/5] HID: wacom: Treat features->device_type values as flags Jason Gerecke
` (5 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Jason Gerecke @ 2015-06-04 1:18 UTC (permalink / raw)
To: Ping Cheng, Aaron Skomra, Benjamin Tissoires, Jiri Kosina
Cc: linux-input, Jason Gerecke, Jason Gerecke
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 <jason.gerecke@wacom.com>
---
drivers/hid/wacom_sys.c | 25 ++++++++++++++-----------
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index eea18a6..bdf31c9 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -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);
}
}
--
2.4.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/5] HID: wacom: Treat features->device_type values as flags
2015-06-04 1:18 [PATCH 0/5] HID: wacom: Support tablets with pen and touch on same interface Jason Gerecke
2015-06-04 1:18 ` [PATCH 1/5] HID: wacom: Simplify 'wacom_update_name' Jason Gerecke
@ 2015-06-04 1:18 ` Jason Gerecke
2015-06-04 18:59 ` Benjamin Tissoires
2015-06-04 1:18 ` [PATCH 3/5] HID: wacom: Introduce a new WACOM_DEVICETYPE_PAD device_type Jason Gerecke
` (4 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Jason Gerecke @ 2015-06-04 1:18 UTC (permalink / raw)
To: Ping Cheng, Aaron Skomra, Benjamin Tissoires, Jiri Kosina
Cc: linux-input, Jason Gerecke, Jason Gerecke
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 <jason.gerecke@wacom.com>
---
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(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index bdf31c9..2b4cbd8 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -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;
}
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index a52fc25..5e7710d 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -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;
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 9a5ee62..da2b309 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -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) || \
--
2.4.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/5] HID: wacom: Introduce a new WACOM_DEVICETYPE_PAD device_type
2015-06-04 1:18 [PATCH 0/5] HID: wacom: Support tablets with pen and touch on same interface Jason Gerecke
2015-06-04 1:18 ` [PATCH 1/5] HID: wacom: Simplify 'wacom_update_name' Jason Gerecke
2015-06-04 1:18 ` [PATCH 2/5] HID: wacom: Treat features->device_type values as flags Jason Gerecke
@ 2015-06-04 1:18 ` Jason Gerecke
2015-06-04 1:18 ` [PATCH 4/5] HID: wacom: Split apart 'wacom_setup_pentouch_input_capabilites' Jason Gerecke
` (3 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Jason Gerecke @ 2015-06-04 1:18 UTC (permalink / raw)
To: Ping Cheng, Aaron Skomra, Benjamin Tissoires, Jiri Kosina
Cc: linux-input, Jason Gerecke, 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 <jason.gerecke@wacom.com>
---
drivers/hid/wacom_sys.c | 54 +++++++++++++++++++-----------------
drivers/hid/wacom_wac.c | 73 +++++++++++++++++++++++++++----------------------
drivers/hid/wacom_wac.h | 1 +
3 files changed, 70 insertions(+), 58 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 2b4cbd8..daf8051 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -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;
}
}
@@ -1315,14 +1317,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)
+ if (wacom_wac2->features.touch_max) {
+ wacom_wac2->features.device_type |= WACOM_DEVICETYPE_TOUCH;
snprintf(wacom_wac2->name, WACOM_NAME_MAX,
"%s (WL) Finger",wacom_wac2->features.name);
- else
+ } else {
+ wacom_wac2->features.device_type |= WACOM_DEVICETYPE_PAD;
snprintf(wacom_wac2->name, WACOM_NAME_MAX,
"%s (WL) Pad",wacom_wac2->features.name);
+ }
snprintf(wacom_wac2->pad_name, WACOM_NAME_MAX,
"%s (WL) Pad", wacom_wac2->features.name);
wacom_wac2->pid = wacom_wac->pid;
@@ -1456,12 +1460,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 +1608,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;
}
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 5e7710d..1e23bd8 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -2167,6 +2167,22 @@ void wacom_setup_device_quirks(struct wacom *wacom)
{
struct wacom_features *features = &wacom->wacom_wac.features;
+ /* The pen and pad share the same interface on most devices */
+ if (features->type == GRAPHIRE_BT || features->type == WACOM_G4 ||
+ features->type == DTUS || features->type == WACOM_MO ||
+ (features->type >= INTUOS3S && features->type <= WACOM_13HD &&
+ features->type != INTUOSHT)) {
+ if (features->device_type & WACOM_DEVICETYPE_PEN)
+ features->device_type |= WACOM_DEVICETYPE_PAD;
+ }
+ /* A few devices have touch and pad share the same interface */
+ if (features->type == BAMBOO_PT || features->type == INTUOSHT) {
+ if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
+ if (features->touch_max)
+ features->device_type |= WACOM_DEVICETYPE_PAD;
+ }
+ }
+
/* touch device found but size is not defined. use default */
if (features->device_type & WACOM_DEVICETYPE_TOUCH && !features->x_max) {
features->x_max = 1023;
@@ -2241,7 +2257,7 @@ static void wacom_abs_set_axis(struct input_dev *input_dev,
/* penabled devices have fixed resolution for each model */
input_abs_set_res(input_dev, ABS_X, features->x_resolution);
input_abs_set_res(input_dev, ABS_Y, features->y_resolution);
- } else {
+ } else if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
if (features->touch_max == 1) {
input_set_abs_params(input_dev, ABS_X, 0,
features->x_max, features->x_fuzz, 0);
@@ -2423,8 +2439,7 @@ int wacom_setup_pentouch_input_capabilities(struct input_dev *input_dev,
break;
case INTUOSHT:
- if (features->touch_max &&
- features->device_type & WACOM_DEVICETYPE_TOUCH) {
+ if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
input_dev->evbit[0] |= BIT_MASK(EV_SW);
__set_bit(SW_MUTE_DEVICE, input_dev->swbit);
}
@@ -2434,27 +2449,26 @@ int wacom_setup_pentouch_input_capabilities(struct input_dev *input_dev,
__clear_bit(ABS_MISC, input_dev->absbit);
if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
-
- if (features->touch_max) {
- if (features->pktlen == WACOM_PKGLEN_BBTOUCH3) {
- input_set_abs_params(input_dev,
- ABS_MT_TOUCH_MAJOR,
- 0, features->x_max, 0, 0);
- input_set_abs_params(input_dev,
- ABS_MT_TOUCH_MINOR,
- 0, features->y_max, 0, 0);
- }
- input_mt_init_slots(input_dev, features->touch_max, INPUT_MT_POINTER);
- } else {
- /* buttons/keys only interface */
- __clear_bit(ABS_X, input_dev->absbit);
- __clear_bit(ABS_Y, input_dev->absbit);
- __clear_bit(BTN_TOUCH, input_dev->keybit);
-
- /* PAD is setup by wacom_setup_pad_input_capabilities later */
- return 1;
+ if (features->pktlen == WACOM_PKGLEN_BBTOUCH3) {
+ input_set_abs_params(input_dev,
+ ABS_MT_TOUCH_MAJOR,
+ 0, features->x_max, 0, 0);
+ input_set_abs_params(input_dev,
+ ABS_MT_TOUCH_MINOR,
+ 0, features->y_max, 0, 0);
}
- } else if (features->device_type & WACOM_DEVICETYPE_PEN) {
+ input_mt_init_slots(input_dev, features->touch_max, INPUT_MT_POINTER);
+ }
+ if (features->device_type & WACOM_DEVICETYPE_PAD) {
+ /* buttons/keys only interface */
+ __clear_bit(ABS_X, input_dev->absbit);
+ __clear_bit(ABS_Y, input_dev->absbit);
+ __clear_bit(BTN_TOUCH, input_dev->keybit);
+
+ /* PAD is setup by wacom_setup_pad_input_capabilities later */
+ return 1;
+ }
+ 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);
@@ -2482,6 +2496,9 @@ int wacom_setup_pad_input_capabilities(struct input_dev *input_dev,
struct wacom_features *features = &wacom_wac->features;
int i;
+ if (!(features->device_type & WACOM_DEVICETYPE_PAD))
+ return -ENODEV;
+
input_dev->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
/* kept for making legacy xf86-input-wacom working with the wheels */
@@ -2618,10 +2635,6 @@ 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 & WACOM_DEVICETYPE_PEN))
- return -ENODEV;
-
for (i = 0; i < 7; i++)
__set_bit(BTN_0 + i, input_dev->keybit);
@@ -2663,12 +2676,6 @@ 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 & WACOM_DEVICETYPE_TOUCH) ||
- /* Bamboo Pen only tablet does not have pad */
- ((features->type == BAMBOO_PT) && !features->touch_max))
- return -ENODEV;
-
__clear_bit(ABS_MISC, input_dev->absbit);
__set_bit(BTN_LEFT, input_dev->keybit);
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index da2b309..c873c9f 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -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
--
2.4.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 4/5] HID: wacom: Split apart 'wacom_setup_pentouch_input_capabilites'
2015-06-04 1:18 [PATCH 0/5] HID: wacom: Support tablets with pen and touch on same interface Jason Gerecke
` (2 preceding siblings ...)
2015-06-04 1:18 ` [PATCH 3/5] HID: wacom: Introduce a new WACOM_DEVICETYPE_PAD device_type Jason Gerecke
@ 2015-06-04 1:18 ` Jason Gerecke
2015-06-04 1:18 ` [PATCH 5/5] HID: wacom: Introduce new 'touch_input' device Jason Gerecke
` (2 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Jason Gerecke @ 2015-06-04 1:18 UTC (permalink / raw)
To: Ping Cheng, Aaron Skomra, Benjamin Tissoires, Jiri Kosina
Cc: linux-input, Jason Gerecke, 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 <jason.gerecke@wacom.com>
---
drivers/hid/wacom.h | 4 +-
drivers/hid/wacom_sys.c | 8 +-
drivers/hid/wacom_wac.c | 234 +++++++++++++++++++++++++-----------------------
3 files changed, 131 insertions(+), 115 deletions(-)
diff --git a/drivers/hid/wacom.h b/drivers/hid/wacom.h
index c76e21f..a533787 100644
--- a/drivers/hid/wacom.h
+++ b/drivers/hid/wacom.h
@@ -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);
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index daf8051..a213a8a 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -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)
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 1e23bd8..318d9d3 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -2241,54 +2241,16 @@ void wacom_setup_device_quirks(struct wacom *wacom)
}
}
-static void wacom_abs_set_axis(struct input_dev *input_dev,
- struct wacom_wac *wacom_wac)
-{
- struct wacom_features *features = &wacom_wac->features;
-
- 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,
- features->y_max, features->y_fuzz, 0);
- input_set_abs_params(input_dev, ABS_PRESSURE, 0,
- features->pressure_max, features->pressure_fuzz, 0);
-
- /* penabled devices have fixed resolution for each model */
- input_abs_set_res(input_dev, ABS_X, features->x_resolution);
- input_abs_set_res(input_dev, ABS_Y, features->y_resolution);
- } else if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
- if (features->touch_max == 1) {
- input_set_abs_params(input_dev, ABS_X, 0,
- features->x_max, features->x_fuzz, 0);
- input_set_abs_params(input_dev, ABS_Y, 0,
- features->y_max, features->y_fuzz, 0);
- input_abs_set_res(input_dev, ABS_X,
- features->x_resolution);
- input_abs_set_res(input_dev, ABS_Y,
- features->y_resolution);
- }
-
- if (features->touch_max > 1) {
- input_set_abs_params(input_dev, ABS_MT_POSITION_X, 0,
- features->x_max, features->x_fuzz, 0);
- input_set_abs_params(input_dev, ABS_MT_POSITION_Y, 0,
- features->y_max, features->y_fuzz, 0);
- input_abs_set_res(input_dev, ABS_MT_POSITION_X,
- features->x_resolution);
- input_abs_set_res(input_dev, ABS_MT_POSITION_Y,
- features->y_resolution);
- }
- }
-}
-
-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)
{
struct wacom_features *features = &wacom_wac->features;
input_dev->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
+ if (!(features->device_type & WACOM_DEVICETYPE_PEN))
+ return -ENODEV;
+
if (features->type == HID_GENERIC)
/* setup has already been done */
return 0;
@@ -2296,7 +2258,17 @@ int wacom_setup_pentouch_input_capabilities(struct input_dev *input_dev,
__set_bit(BTN_TOUCH, input_dev->keybit);
__set_bit(ABS_MISC, input_dev->absbit);
- wacom_abs_set_axis(input_dev, wacom_wac);
+ 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,
+ features->y_max, features->y_fuzz, 0);
+ input_set_abs_params(input_dev, ABS_PRESSURE, 0,
+ features->pressure_max, features->pressure_fuzz, 0);
+
+ /* penabled devices have fixed resolution for each model */
+ input_abs_set_res(input_dev, ABS_X, features->x_resolution);
+ input_abs_set_res(input_dev, ABS_Y, features->y_resolution);
+
switch (features->type) {
case GRAPHIRE_BT:
@@ -2365,53 +2337,25 @@ 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 & WACOM_DEVICETYPE_PEN) {
- input_set_abs_params(input_dev, ABS_DISTANCE, 0,
- features->distance_max,
- 0, 0);
-
- input_set_abs_params(input_dev, ABS_Z, -900, 899, 0, 0);
- input_abs_set_res(input_dev, ABS_Z, 287);
+ input_set_abs_params(input_dev, ABS_DISTANCE, 0,
+ features->distance_max,
+ 0, 0);
- wacom_setup_intuos(wacom_wac);
- } else if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
- __clear_bit(ABS_MISC, input_dev->absbit);
+ input_set_abs_params(input_dev, ABS_Z, -900, 899, 0, 0);
+ input_abs_set_res(input_dev, ABS_Z, 287);
- input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR,
- 0, features->x_max, 0, 0);
- input_set_abs_params(input_dev, ABS_MT_TOUCH_MINOR,
- 0, features->y_max, 0, 0);
- input_mt_init_slots(input_dev, features->touch_max, INPUT_MT_POINTER);
- }
+ wacom_setup_intuos(wacom_wac);
break;
case WACOM_24HDT:
- 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);
- input_set_abs_params(input_dev, ABS_MT_ORIENTATION, 0, 1, 0, 0);
- }
- /* fall through */
-
case WACOM_27QHDT:
case MTSCREEN:
case MTTPC:
case MTTPC_B:
case TABLETPC2FG:
- 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 */
-
case TABLETPC:
case TABLETPCE:
__clear_bit(ABS_MISC, input_dev->absbit);
-
- __set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
-
- if (!(features->device_type & WACOM_DEVICETYPE_PEN))
- break; /* no need to process stylus stuff */
-
/* fall through */
case DTUS:
@@ -2439,48 +2383,114 @@ int wacom_setup_pentouch_input_capabilities(struct input_dev *input_dev,
break;
case INTUOSHT:
- if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
- input_dev->evbit[0] |= BIT_MASK(EV_SW);
- __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
- }
- /* fall through */
-
case BAMBOO_PT:
__clear_bit(ABS_MISC, input_dev->absbit);
- if (features->device_type & WACOM_DEVICETYPE_TOUCH) {
- if (features->pktlen == WACOM_PKGLEN_BBTOUCH3) {
- input_set_abs_params(input_dev,
- ABS_MT_TOUCH_MAJOR,
- 0, features->x_max, 0, 0);
- input_set_abs_params(input_dev,
- ABS_MT_TOUCH_MINOR,
- 0, features->y_max, 0, 0);
- }
- input_mt_init_slots(input_dev, features->touch_max, INPUT_MT_POINTER);
- }
- if (features->device_type & WACOM_DEVICETYPE_PAD) {
- /* buttons/keys only interface */
- __clear_bit(ABS_X, input_dev->absbit);
- __clear_bit(ABS_Y, input_dev->absbit);
- __clear_bit(BTN_TOUCH, input_dev->keybit);
+ __set_bit(INPUT_PROP_POINTER, input_dev->propbit);
+ __set_bit(BTN_TOOL_RUBBER, input_dev->keybit);
+ __set_bit(BTN_TOOL_PEN, input_dev->keybit);
+ __set_bit(BTN_STYLUS, input_dev->keybit);
+ __set_bit(BTN_STYLUS2, input_dev->keybit);
+ input_set_abs_params(input_dev, ABS_DISTANCE, 0,
+ features->distance_max,
+ 0, 0);
+ break;
+ case BAMBOO_PAD:
+ __clear_bit(ABS_MISC, input_dev->absbit);
+ break;
+ }
+ return 0;
+}
- /* PAD is setup by wacom_setup_pad_input_capabilities later */
- return 1;
- }
- 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);
- __set_bit(BTN_STYLUS, input_dev->keybit);
- __set_bit(BTN_STYLUS2, input_dev->keybit);
- input_set_abs_params(input_dev, ABS_DISTANCE, 0,
- features->distance_max,
- 0, 0);
+int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
+ struct wacom_wac *wacom_wac)
+{
+ struct wacom_features *features = &wacom_wac->features;
+
+ input_dev->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
+
+ if (!(features->device_type & WACOM_DEVICETYPE_TOUCH))
+ return -ENODEV;
+
+ if (features->type == HID_GENERIC)
+ /* setup has already been done */
+ return 0;
+
+ __set_bit(BTN_TOUCH, input_dev->keybit);
+
+ if (features->touch_max == 1) {
+ input_set_abs_params(input_dev, ABS_X, 0,
+ features->x_max, features->x_fuzz, 0);
+ input_set_abs_params(input_dev, ABS_Y, 0,
+ features->y_max, features->y_fuzz, 0);
+ input_abs_set_res(input_dev, ABS_X,
+ features->x_resolution);
+ input_abs_set_res(input_dev, ABS_Y,
+ features->y_resolution);
+ }
+ else if (features->touch_max > 1) {
+ input_set_abs_params(input_dev, ABS_MT_POSITION_X, 0,
+ features->x_max, features->x_fuzz, 0);
+ input_set_abs_params(input_dev, ABS_MT_POSITION_Y, 0,
+ features->y_max, features->y_fuzz, 0);
+ input_abs_set_res(input_dev, ABS_MT_POSITION_X,
+ features->x_resolution);
+ input_abs_set_res(input_dev, ABS_MT_POSITION_Y,
+ features->y_resolution);
+ }
+
+ switch (features->type) {
+ case INTUOS5:
+ case INTUOS5L:
+ case INTUOSPM:
+ case INTUOSPL:
+ case INTUOS5S:
+ case INTUOSPS:
+ __set_bit(INPUT_PROP_POINTER, input_dev->propbit);
+
+ input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0, features->x_max, 0, 0);
+ input_set_abs_params(input_dev, ABS_MT_TOUCH_MINOR, 0, features->y_max, 0, 0);
+ input_mt_init_slots(input_dev, features->touch_max, INPUT_MT_POINTER);
+ break;
+
+ case WACOM_24HDT:
+ 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);
+ input_set_abs_params(input_dev, ABS_MT_ORIENTATION, 0, 1, 0, 0);
+ /* fall through */
+
+ case WACOM_27QHDT:
+ case MTSCREEN:
+ case MTTPC:
+ case MTTPC_B:
+ case TABLETPC2FG:
+ input_mt_init_slots(input_dev, features->touch_max, INPUT_MT_DIRECT);
+ /*fall through */
+
+ case TABLETPC:
+ case TABLETPCE:
+ __set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
+ break;
+
+ case INTUOSHT:
+ input_dev->evbit[0] |= BIT_MASK(EV_SW);
+ __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
+ /* fall through */
+
+ case BAMBOO_PT:
+ if (features->pktlen == WACOM_PKGLEN_BBTOUCH3) {
+ input_set_abs_params(input_dev,
+ ABS_MT_TOUCH_MAJOR,
+ 0, features->x_max, 0, 0);
+ input_set_abs_params(input_dev,
+ ABS_MT_TOUCH_MINOR,
+ 0, features->y_max, 0, 0);
}
+ input_mt_init_slots(input_dev, features->touch_max, INPUT_MT_POINTER);
break;
+
case BAMBOO_PAD:
- __clear_bit(ABS_MISC, input_dev->absbit);
input_mt_init_slots(input_dev, features->touch_max,
INPUT_MT_POINTER);
__set_bit(BTN_LEFT, input_dev->keybit);
--
2.4.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 5/5] HID: wacom: Introduce new 'touch_input' device
2015-06-04 1:18 [PATCH 0/5] HID: wacom: Support tablets with pen and touch on same interface Jason Gerecke
` (3 preceding siblings ...)
2015-06-04 1:18 ` [PATCH 4/5] HID: wacom: Split apart 'wacom_setup_pentouch_input_capabilites' Jason Gerecke
@ 2015-06-04 1:18 ` Jason Gerecke
2015-06-04 18:56 ` Benjamin Tissoires
2015-06-04 14:18 ` [PATCH 0/5] HID: wacom: Support tablets with pen and touch on same interface Benjamin Tissoires
2015-06-08 15:59 ` Benjamin Tissoires
6 siblings, 1 reply; 16+ messages in thread
From: Jason Gerecke @ 2015-06-04 1:18 UTC (permalink / raw)
To: Ping Cheng, Aaron Skomra, Benjamin Tissoires, Jiri Kosina
Cc: linux-input, Jason Gerecke, 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 <jason.gerecke@wacom.com>
---
drivers/hid/wacom_sys.c | 118 +++++++++++++++++++++++++++++-------------------
drivers/hid/wacom_wac.c | 108 ++++++++++++++++++++++++--------------------
drivers/hid/wacom_wac.h | 9 ++--
3 files changed, 135 insertions(+), 100 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index a213a8a..a928c1d 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -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)
- return error;
- wacom_wac->input_registered = true;
+ 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)
+ 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;
}
@@ -1303,7 +1336,7 @@ 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;
- 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);
@@ -1324,11 +1357,11 @@ static void wacom_wireless_work(struct work_struct *work)
wacom_wac2->features.x_max = wacom_wac2->features.y_max = 4096;
if (wacom_wac2->features.touch_max) {
wacom_wac2->features.device_type |= WACOM_DEVICETYPE_TOUCH;
- snprintf(wacom_wac2->name, WACOM_NAME_MAX,
+ snprintf(wacom_wac2->touch_name, WACOM_NAME_MAX,
"%s (WL) Finger",wacom_wac2->features.name);
} else {
wacom_wac2->features.device_type |= WACOM_DEVICETYPE_PAD;
- snprintf(wacom_wac2->name, WACOM_NAME_MAX,
+ snprintf(wacom_wac2->pad_name, WACOM_NAME_MAX,
"%s (WL) Pad",wacom_wac2->features.name);
}
snprintf(wacom_wac2->pad_name, WACOM_NAME_MAX,
@@ -1341,7 +1374,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->pen_input;
}
error = wacom_initialize_battery(wacom);
@@ -1456,21 +1489,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 Pad", name);
+ snprintf(wacom_wac->touch_name, sizeof(wacom_wac->touch_name),
+ "%s Touch", 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,
@@ -1614,7 +1638,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->pen_input;
}
return 0;
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 318d9d3..94334be 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -69,7 +69,7 @@ static void wacom_notify_battery(struct wacom_wac *wacom_wac,
static int wacom_penpartner_irq(struct wacom_wac *wacom)
{
unsigned char *data = wacom->data;
- struct input_dev *input = wacom->input;
+ struct input_dev *input = wacom->pen_input;
switch (data[0]) {
case 1:
@@ -114,7 +114,7 @@ static int wacom_pl_irq(struct wacom_wac *wacom)
{
struct wacom_features *features = &wacom->features;
unsigned char *data = wacom->data;
- struct input_dev *input = wacom->input;
+ struct input_dev *input = wacom->pen_input;
int prox, pressure;
if (data[0] != WACOM_REPORT_PENABLED) {
@@ -186,7 +186,7 @@ static int wacom_pl_irq(struct wacom_wac *wacom)
static int wacom_ptu_irq(struct wacom_wac *wacom)
{
unsigned char *data = wacom->data;
- struct input_dev *input = wacom->input;
+ struct input_dev *input = wacom->pen_input;
if (data[0] != WACOM_REPORT_PENABLED) {
dev_dbg(input->dev.parent,
@@ -215,7 +215,7 @@ static int wacom_ptu_irq(struct wacom_wac *wacom)
static int wacom_dtu_irq(struct wacom_wac *wacom)
{
unsigned char *data = wacom->data;
- struct input_dev *input = wacom->input;
+ struct input_dev *input = wacom->pen_input;
int prox = data[1] & 0x20;
dev_dbg(input->dev.parent,
@@ -245,7 +245,7 @@ static int wacom_dtu_irq(struct wacom_wac *wacom)
static int wacom_dtus_irq(struct wacom_wac *wacom)
{
char *data = wacom->data;
- struct input_dev *input = wacom->input;
+ struct input_dev *input = wacom->pen_input;
unsigned short prox, pressure = 0;
if (data[0] != WACOM_REPORT_DTUS && data[0] != WACOM_REPORT_DTUSPAD) {
@@ -297,7 +297,7 @@ static int wacom_graphire_irq(struct wacom_wac *wacom)
{
struct wacom_features *features = &wacom->features;
unsigned char *data = wacom->data;
- struct input_dev *input = wacom->input;
+ struct input_dev *input = wacom->pen_input;
struct input_dev *pad_input = wacom->pad_input;
int battery_capacity, ps_connected;
int prox;
@@ -464,7 +464,7 @@ static int wacom_intuos_inout(struct wacom_wac *wacom)
{
struct wacom_features *features = &wacom->features;
unsigned char *data = wacom->data;
- struct input_dev *input = wacom->input;
+ struct input_dev *input = wacom->pen_input;
int idx = 0;
/* tool number */
@@ -649,7 +649,7 @@ static void wacom_intuos_general(struct wacom_wac *wacom)
{
struct wacom_features *features = &wacom->features;
unsigned char *data = wacom->data;
- struct input_dev *input = wacom->input;
+ struct input_dev *input = wacom->pen_input;
unsigned int t;
/* general pen packet */
@@ -681,7 +681,7 @@ static int wacom_intuos_irq(struct wacom_wac *wacom)
{
struct wacom_features *features = &wacom->features;
unsigned char *data = wacom->data;
- struct input_dev *input = wacom->input;
+ struct input_dev *input = wacom->pen_input;
unsigned int t;
int idx = 0, result;
@@ -1025,7 +1025,7 @@ static void wacom_intuos_bt_process_data(struct wacom_wac *wacom,
memcpy(wacom->data, data, 10);
wacom_intuos_irq(wacom);
- input_sync(wacom->input);
+ input_sync(wacom->pen_input);
if (wacom->pad_input)
input_sync(wacom->pad_input);
}
@@ -1057,7 +1057,7 @@ static int wacom_intuos_bt_irq(struct wacom_wac *wacom, size_t len)
ps_connected);
break;
default:
- dev_dbg(wacom->input->dev.parent,
+ dev_dbg(wacom->pen_input->dev.parent,
"Unknown report: %d,%d size:%zu\n",
data[0], data[1], len);
return 0;
@@ -1067,7 +1067,7 @@ static int wacom_intuos_bt_irq(struct wacom_wac *wacom, size_t len)
static int wacom_wac_finger_count_touches(struct wacom_wac *wacom)
{
- struct input_dev *input = wacom->input;
+ struct input_dev *input = wacom->touch_input;
unsigned touch_max = wacom->features.touch_max;
int count = 0;
int i;
@@ -1088,7 +1088,7 @@ static int wacom_wac_finger_count_touches(struct wacom_wac *wacom)
static int wacom_24hdt_irq(struct wacom_wac *wacom)
{
- struct input_dev *input = wacom->input;
+ struct input_dev *input = wacom->touch_input;
unsigned char *data = wacom->data;
int i;
int current_num_contacts = data[61];
@@ -1156,7 +1156,7 @@ static int wacom_24hdt_irq(struct wacom_wac *wacom)
static int wacom_mt_touch(struct wacom_wac *wacom)
{
- struct input_dev *input = wacom->input;
+ struct input_dev *input = wacom->touch_input;
unsigned char *data = wacom->data;
int i;
int current_num_contacts = data[2];
@@ -1207,7 +1207,7 @@ static int wacom_mt_touch(struct wacom_wac *wacom)
static int wacom_tpc_mt_touch(struct wacom_wac *wacom)
{
- struct input_dev *input = wacom->input;
+ struct input_dev *input = wacom->touch_input;
unsigned char *data = wacom->data;
int i;
@@ -1236,7 +1236,7 @@ static int wacom_tpc_mt_touch(struct wacom_wac *wacom)
static int wacom_tpc_single_touch(struct wacom_wac *wacom, size_t len)
{
unsigned char *data = wacom->data;
- struct input_dev *input = wacom->input;
+ struct input_dev *input = wacom->touch_input;
bool prox = !wacom->shared->stylus_in_proximity;
int x = 0, y = 0;
@@ -1272,7 +1272,7 @@ static int wacom_tpc_single_touch(struct wacom_wac *wacom, size_t len)
static int wacom_tpc_pen(struct wacom_wac *wacom)
{
unsigned char *data = wacom->data;
- struct input_dev *input = wacom->input;
+ struct input_dev *input = wacom->pen_input;
bool prox = data[1] & 0x20;
if (!wacom->shared->stylus_in_proximity) /* first in prox */
@@ -1301,8 +1301,12 @@ static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len)
{
unsigned char *data = wacom->data;
- dev_dbg(wacom->input->dev.parent,
- "%s: received report #%d\n", __func__, data[0]);
+ if (wacom->pen_input)
+ dev_dbg(wacom->pen_input->dev.parent,
+ "%s: received report #%d\n", __func__, data[0]);
+ else if (wacom->touch_input)
+ dev_dbg(wacom->touch_input->dev.parent,
+ "%s: received report #%d\n", __func__, data[0]);
switch (len) {
case WACOM_PKGLEN_TPC1FG:
@@ -1334,11 +1338,9 @@ 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,
+static void wacom_map_usage(struct input_dev *input, 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;
@@ -1366,36 +1368,38 @@ 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);
+ struct wacom_wac *wacom_wac = &wacom->wacom_wac;
+ struct input_dev *input = wacom_wac->pen_input;
switch (usage->hid) {
case HID_GD_X:
- wacom_map_usage(wacom, usage, field, EV_ABS, ABS_X, 4);
+ wacom_map_usage(input, usage, field, EV_ABS, ABS_X, 4);
break;
case HID_GD_Y:
- wacom_map_usage(wacom, usage, field, EV_ABS, ABS_Y, 4);
+ wacom_map_usage(input, usage, field, EV_ABS, ABS_Y, 4);
break;
case HID_DG_TIPPRESSURE:
- wacom_map_usage(wacom, usage, field, EV_ABS, ABS_PRESSURE, 0);
+ wacom_map_usage(input, usage, field, EV_ABS, ABS_PRESSURE, 0);
break;
case HID_DG_INRANGE:
- wacom_map_usage(wacom, usage, field, EV_KEY, BTN_TOOL_PEN, 0);
+ wacom_map_usage(input, usage, field, EV_KEY, BTN_TOOL_PEN, 0);
break;
case HID_DG_INVERT:
- wacom_map_usage(wacom, usage, field, EV_KEY,
+ wacom_map_usage(input, 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);
+ wacom_map_usage(input, usage, field, EV_KEY, BTN_TOUCH, 0);
break;
case HID_DG_BARRELSWITCH:
- wacom_map_usage(wacom, usage, field, EV_KEY, BTN_STYLUS, 0);
+ wacom_map_usage(input, usage, field, EV_KEY, BTN_STYLUS, 0);
break;
case HID_DG_BARRELSWITCH2:
- wacom_map_usage(wacom, usage, field, EV_KEY, BTN_STYLUS2, 0);
+ wacom_map_usage(input, usage, field, EV_KEY, BTN_STYLUS2, 0);
break;
case HID_DG_TOOLSERIALNUMBER:
- wacom_map_usage(wacom, usage, field, EV_MSC, MSC_SERIAL, 0);
+ wacom_map_usage(input, usage, field, EV_MSC, MSC_SERIAL, 0);
break;
}
}
@@ -1405,7 +1409,7 @@ static int wacom_wac_pen_event(struct hid_device *hdev, struct hid_field *field,
{
struct wacom *wacom = hid_get_drvdata(hdev);
struct wacom_wac *wacom_wac = &wacom->wacom_wac;
- struct input_dev *input = wacom_wac->input;
+ struct input_dev *input = wacom_wac->pen_input;
/* checking which Tool / tip switch to send */
switch (usage->hid) {
@@ -1435,7 +1439,7 @@ static void wacom_wac_pen_report(struct hid_device *hdev,
{
struct wacom *wacom = hid_get_drvdata(hdev);
struct wacom_wac *wacom_wac = &wacom->wacom_wac;
- struct input_dev *input = wacom_wac->input;
+ struct input_dev *input = wacom_wac->pen_input;
bool prox = wacom_wac->hid_data.inrange_state;
if (!wacom_wac->shared->stylus_in_proximity) /* first in prox */
@@ -1464,23 +1468,24 @@ static void wacom_wac_finger_usage_mapping(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;
+ struct input_dev *input = wacom_wac->touch_input;
unsigned touch_max = wacom_wac->features.touch_max;
switch (usage->hid) {
case HID_GD_X:
features->last_slot_field = usage->hid;
if (touch_max == 1)
- wacom_map_usage(wacom, usage, field, EV_ABS, ABS_X, 4);
+ wacom_map_usage(input, usage, field, EV_ABS, ABS_X, 4);
else
- wacom_map_usage(wacom, usage, field, EV_ABS,
+ wacom_map_usage(input, usage, field, EV_ABS,
ABS_MT_POSITION_X, 4);
break;
case HID_GD_Y:
features->last_slot_field = usage->hid;
if (touch_max == 1)
- wacom_map_usage(wacom, usage, field, EV_ABS, ABS_Y, 4);
+ wacom_map_usage(input, usage, field, EV_ABS, ABS_Y, 4);
else
- wacom_map_usage(wacom, usage, field, EV_ABS,
+ wacom_map_usage(input, usage, field, EV_ABS,
ABS_MT_POSITION_Y, 4);
break;
case HID_DG_CONTACTID:
@@ -1494,7 +1499,7 @@ static void wacom_wac_finger_usage_mapping(struct hid_device *hdev,
break;
case HID_DG_TIPSWITCH:
features->last_slot_field = usage->hid;
- wacom_map_usage(wacom, usage, field, EV_KEY, BTN_TOUCH, 0);
+ wacom_map_usage(input, usage, field, EV_KEY, BTN_TOUCH, 0);
break;
}
}
@@ -1550,7 +1555,7 @@ static int wacom_wac_finger_event(struct hid_device *hdev,
if (usage->usage_index + 1 == field->report_count) {
if (usage->hid == wacom_wac->features.last_slot_field)
- wacom_wac_finger_slot(wacom_wac, wacom_wac->input);
+ wacom_wac_finger_slot(wacom_wac, wacom_wac->touch_input);
}
return 0;
@@ -1561,7 +1566,7 @@ static void wacom_wac_finger_report(struct hid_device *hdev,
{
struct wacom *wacom = hid_get_drvdata(hdev);
struct wacom_wac *wacom_wac = &wacom->wacom_wac;
- struct input_dev *input = wacom_wac->input;
+ struct input_dev *input = wacom_wac->touch_input;
unsigned touch_max = wacom_wac->features.touch_max;
if (touch_max > 1)
@@ -1578,10 +1583,10 @@ void wacom_wac_usage_mapping(struct hid_device *hdev,
{
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);
+ __set_bit(INPUT_PROP_DIRECT, wacom_wac->pen_input->propbit);
+ __set_bit(INPUT_PROP_DIRECT, wacom_wac->touch_input->propbit);
if (WACOM_PEN_FIELD(field))
return wacom_wac_pen_usage_mapping(hdev, field, usage);
@@ -1626,7 +1631,7 @@ void wacom_wac_report(struct hid_device *hdev, struct hid_report *report)
static int wacom_bpt_touch(struct wacom_wac *wacom)
{
struct wacom_features *features = &wacom->features;
- struct input_dev *input = wacom->input;
+ struct input_dev *input = wacom->touch_input;
struct input_dev *pad_input = wacom->pad_input;
unsigned char *data = wacom->data;
int i;
@@ -1674,7 +1679,7 @@ static int wacom_bpt_touch(struct wacom_wac *wacom)
static void wacom_bpt3_touch_msg(struct wacom_wac *wacom, unsigned char *data)
{
struct wacom_features *features = &wacom->features;
- struct input_dev *input = wacom->input;
+ struct input_dev *input = wacom->touch_input;
bool touch = data[1] & 0x80;
int slot = input_mt_get_slot_by_key(input, data[0]);
@@ -1732,7 +1737,7 @@ static void wacom_bpt3_button_msg(struct wacom_wac *wacom, unsigned char *data)
static int wacom_bpt3_touch(struct wacom_wac *wacom)
{
- struct input_dev *input = wacom->input;
+ struct input_dev *input = wacom->touch_input;
unsigned char *data = wacom->data;
int count = data[1] & 0x07;
int i;
@@ -1760,7 +1765,7 @@ static int wacom_bpt3_touch(struct wacom_wac *wacom)
static int wacom_bpt_pen(struct wacom_wac *wacom)
{
struct wacom_features *features = &wacom->features;
- struct input_dev *input = wacom->input;
+ struct input_dev *input = wacom->pen_input;
unsigned char *data = wacom->data;
int prox = 0, x = 0, y = 0, p = 0, d = 0, pen = 0, btn1 = 0, btn2 = 0;
@@ -1869,7 +1874,7 @@ static void wacom_bamboo_pad_pen_event(struct wacom_wac *wacom,
static int wacom_bamboo_pad_touch_event(struct wacom_wac *wacom,
unsigned char *data)
{
- struct input_dev *input = wacom->input;
+ struct input_dev *input = wacom->touch_input;
unsigned char *finger_data, prefix;
unsigned id;
int x, y;
@@ -2113,7 +2118,10 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
}
if (sync) {
- input_sync(wacom_wac->input);
+ if (wacom_wac->pen_input)
+ input_sync(wacom_wac->pen_input);
+ if (wacom_wac->touch_input)
+ input_sync(wacom_wac->touch_input);
if (wacom_wac->pad_input)
input_sync(wacom_wac->pad_input);
}
@@ -2121,7 +2129,7 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
static void wacom_setup_cintiq(struct wacom_wac *wacom_wac)
{
- struct input_dev *input_dev = wacom_wac->input;
+ struct input_dev *input_dev = wacom_wac->pen_input;
input_set_capability(input_dev, EV_MSC, MSC_SERIAL);
@@ -2144,7 +2152,7 @@ static void wacom_setup_cintiq(struct wacom_wac *wacom_wac)
static void wacom_setup_intuos(struct wacom_wac *wacom_wac)
{
- struct input_dev *input_dev = wacom_wac->input;
+ struct input_dev *input_dev = wacom_wac->pen_input;
input_set_capability(input_dev, EV_REL, REL_WHEEL);
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index c873c9f..2978c30 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -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;
--
2.4.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 0/5] HID: wacom: Support tablets with pen and touch on same interface
2015-06-04 1:18 [PATCH 0/5] HID: wacom: Support tablets with pen and touch on same interface Jason Gerecke
` (4 preceding siblings ...)
2015-06-04 1:18 ` [PATCH 5/5] HID: wacom: Introduce new 'touch_input' device Jason Gerecke
@ 2015-06-04 14:18 ` Benjamin Tissoires
2015-06-04 17:34 ` Jason Gerecke
2015-06-08 15:59 ` Benjamin Tissoires
6 siblings, 1 reply; 16+ messages in thread
From: Benjamin Tissoires @ 2015-06-04 14:18 UTC (permalink / raw)
To: Jason Gerecke
Cc: Ping Cheng, Aaron Skomra, Jiri Kosina, linux-input, Jason Gerecke
Hi Jason,
On Jun 03 2015 or thereabouts, Jason Gerecke wrote:
> I've recently got my hands on a device which has an I2C sensor that sends
> both pen and touch reports from a single interface. To userspace, it shows
> up as a single input device which blends both the report types (e.g. it has
> ABS_PRESSURE for the pen, and ABS_MT_POSITION_X for the touch). This patch
> set modifies the driver to be able to handle devices which place both pen
> and touch on a the same interface. It does this by treating the pen, touch,
> and pad (which was already special-cased) independently. If a device has
> the appropriate device_type, one or more of pen/touch/pad input devices
> will be created, initialized, and used to send data to userspace.
>
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
This all seems sensible. I gave a quick look yesterday and could not
found any obvious problem, but I'd like to review it more thoroughly
before giving my rev-by (and do a little bit of testing too).
I don't believe there will be any problems for the series, besides the
Bamboo PAD. Have you tested on this?
Cheers,
Benjamin
>
> Jason Gerecke (5):
> HID: wacom: Simplify 'wacom_update_name'
> HID: wacom: Treat features->device_type values as flags
> HID: wacom: Introduce a new WACOM_DEVICETYPE_PAD device_type
> HID: wacom: Split apart 'wacom_setup_pentouch_input_capabilites'
> HID: wacom: Introduce new 'touch_input' device
>
> drivers/hid/wacom.h | 4 +-
> drivers/hid/wacom_sys.c | 194 +++++++++++++++----------
> drivers/hid/wacom_wac.c | 379 ++++++++++++++++++++++++++----------------------
> drivers/hid/wacom_wac.h | 15 +-
> 4 files changed, 332 insertions(+), 260 deletions(-)
>
> --
> 2.4.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/5] HID: wacom: Support tablets with pen and touch on same interface
2015-06-04 14:18 ` [PATCH 0/5] HID: wacom: Support tablets with pen and touch on same interface Benjamin Tissoires
@ 2015-06-04 17:34 ` Jason Gerecke
2015-06-08 15:46 ` Benjamin Tissoires
0 siblings, 1 reply; 16+ messages in thread
From: Jason Gerecke @ 2015-06-04 17:34 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Ping Cheng, Aaron Skomra, Jiri Kosina, linux-input, Jason Gerecke
On 6/4/2015 7:18 AM, Benjamin Tissoires wrote:
> Hi Jason,
>
> On Jun 03 2015 or thereabouts, Jason Gerecke wrote:
>> I've recently got my hands on a device which has an I2C sensor that sends
>> both pen and touch reports from a single interface. To userspace, it shows
>> up as a single input device which blends both the report types (e.g. it has
>> ABS_PRESSURE for the pen, and ABS_MT_POSITION_X for the touch). This patch
>> set modifies the driver to be able to handle devices which place both pen
>> and touch on a the same interface. It does this by treating the pen, touch,
>> and pad (which was already special-cased) independently. If a device has
>> the appropriate device_type, one or more of pen/touch/pad input devices
>> will be created, initialized, and used to send data to userspace.
>>
>> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
>
> This all seems sensible. I gave a quick look yesterday and could not
> found any obvious problem, but I'd like to review it more thoroughly
> before giving my rev-by (and do a little bit of testing too).
>
> I don't believe there will be any problems for the series, besides the
> Bamboo PAD. Have you tested on this?
>
> Cheers,
> Benjamin
>
I don't have a Bamboo PAD to test with, but carefully looking at each
interface's HID descriptor makes me think you may be right. The debug
interface in particular uses a WACOM_VENDORDEFINED_PEN application
collection and will be marked as WACOM_DEVICETYPE_PEN instead of
WACOM_DEVICETYPE_TOUCH.
The smallest fix that should do the trick would be to just swap the
device_type in 'wacom_setup_device_quirks'. Alternatively, we could only
allow the debug interface to be probed, set the touch device type flag
(in addition to the automatically-set pen flag), and extend the IRQ
function to explicitly handle pen events. The alternative is a bit more
work (and we'd need somebody with a device to test it...) but I think
might make the codepath for the Bamboo PAD a bit easier to follow.
--
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....
>>
>> Jason Gerecke (5):
>> HID: wacom: Simplify 'wacom_update_name'
>> HID: wacom: Treat features->device_type values as flags
>> HID: wacom: Introduce a new WACOM_DEVICETYPE_PAD device_type
>> HID: wacom: Split apart 'wacom_setup_pentouch_input_capabilites'
>> HID: wacom: Introduce new 'touch_input' device
>>
>> drivers/hid/wacom.h | 4 +-
>> drivers/hid/wacom_sys.c | 194 +++++++++++++++----------
>> drivers/hid/wacom_wac.c | 379 ++++++++++++++++++++++++++----------------------
>> drivers/hid/wacom_wac.h | 15 +-
>> 4 files changed, 332 insertions(+), 260 deletions(-)
>>
>> --
>> 2.4.1
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 5/5] HID: wacom: Introduce new 'touch_input' device
2015-06-04 1:18 ` [PATCH 5/5] HID: wacom: Introduce new 'touch_input' device Jason Gerecke
@ 2015-06-04 18:56 ` Benjamin Tissoires
2015-06-04 20:49 ` Jason Gerecke
0 siblings, 1 reply; 16+ messages in thread
From: Benjamin Tissoires @ 2015-06-04 18:56 UTC (permalink / raw)
To: Jason Gerecke
Cc: Ping Cheng, Aaron Skomra, Jiri Kosina, linux-input, Jason Gerecke
Hi Jason,
On Jun 03 2015 or thereabouts, Jason Gerecke wrote:
> 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 <jason.gerecke@wacom.com>
> ---
Besides the Wacom PAD problem already mentioned in the 0/5 thread (I
will test it tomorrow and come with a following patch if need), I have 2
nitpicks here:
> drivers/hid/wacom_sys.c | 118 +++++++++++++++++++++++++++++-------------------
> drivers/hid/wacom_wac.c | 108 ++++++++++++++++++++++++--------------------
> drivers/hid/wacom_wac.h | 9 ++--
> 3 files changed, 135 insertions(+), 100 deletions(-)
>
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index a213a8a..a928c1d 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -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);
I know you are not introducing anything new here, but that means that
now, we reserve 3 input devices, while we might not even use one.
I just wonder if we should not start having a smarter input allocation
in wacom.ko where we would create the inputs only if they are needed.
IIRC, I came to use this extra pad input node to have all the bits in
place while parsing the report descriptor, but maybe we could be
smarter.
> - 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)
> - return error;
> - wacom_wac->input_registered = true;
> + 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)
> + 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;
> }
>
> @@ -1303,7 +1336,7 @@ 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;
> - 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);
> @@ -1324,11 +1357,11 @@ static void wacom_wireless_work(struct work_struct *work)
> wacom_wac2->features.x_max = wacom_wac2->features.y_max = 4096;
> if (wacom_wac2->features.touch_max) {
> wacom_wac2->features.device_type |= WACOM_DEVICETYPE_TOUCH;
> - snprintf(wacom_wac2->name, WACOM_NAME_MAX,
> + snprintf(wacom_wac2->touch_name, WACOM_NAME_MAX,
> "%s (WL) Finger",wacom_wac2->features.name);
> } else {
> wacom_wac2->features.device_type |= WACOM_DEVICETYPE_PAD;
> - snprintf(wacom_wac2->name, WACOM_NAME_MAX,
> + snprintf(wacom_wac2->pad_name, WACOM_NAME_MAX,
> "%s (WL) Pad",wacom_wac2->features.name);
> }
> snprintf(wacom_wac2->pad_name, WACOM_NAME_MAX,
> @@ -1341,7 +1374,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->pen_input;
> }
>
> error = wacom_initialize_battery(wacom);
> @@ -1456,21 +1489,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 Pad", name);
typo, this should be "%s Pen"
The rest of the series looks fine. I have one more comment to add in 2/5
but it does not change the whole behaviour. I will try to go a little
bit deeper in the review tomorrow, but for now, this is a good start :)
Cheers,
Benjamin
> + snprintf(wacom_wac->touch_name, sizeof(wacom_wac->touch_name),
> + "%s Touch", 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,
> @@ -1614,7 +1638,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->pen_input;
> }
>
> return 0;
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 318d9d3..94334be 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -69,7 +69,7 @@ static void wacom_notify_battery(struct wacom_wac *wacom_wac,
> static int wacom_penpartner_irq(struct wacom_wac *wacom)
> {
> unsigned char *data = wacom->data;
> - struct input_dev *input = wacom->input;
> + struct input_dev *input = wacom->pen_input;
>
> switch (data[0]) {
> case 1:
> @@ -114,7 +114,7 @@ static int wacom_pl_irq(struct wacom_wac *wacom)
> {
> struct wacom_features *features = &wacom->features;
> unsigned char *data = wacom->data;
> - struct input_dev *input = wacom->input;
> + struct input_dev *input = wacom->pen_input;
> int prox, pressure;
>
> if (data[0] != WACOM_REPORT_PENABLED) {
> @@ -186,7 +186,7 @@ static int wacom_pl_irq(struct wacom_wac *wacom)
> static int wacom_ptu_irq(struct wacom_wac *wacom)
> {
> unsigned char *data = wacom->data;
> - struct input_dev *input = wacom->input;
> + struct input_dev *input = wacom->pen_input;
>
> if (data[0] != WACOM_REPORT_PENABLED) {
> dev_dbg(input->dev.parent,
> @@ -215,7 +215,7 @@ static int wacom_ptu_irq(struct wacom_wac *wacom)
> static int wacom_dtu_irq(struct wacom_wac *wacom)
> {
> unsigned char *data = wacom->data;
> - struct input_dev *input = wacom->input;
> + struct input_dev *input = wacom->pen_input;
> int prox = data[1] & 0x20;
>
> dev_dbg(input->dev.parent,
> @@ -245,7 +245,7 @@ static int wacom_dtu_irq(struct wacom_wac *wacom)
> static int wacom_dtus_irq(struct wacom_wac *wacom)
> {
> char *data = wacom->data;
> - struct input_dev *input = wacom->input;
> + struct input_dev *input = wacom->pen_input;
> unsigned short prox, pressure = 0;
>
> if (data[0] != WACOM_REPORT_DTUS && data[0] != WACOM_REPORT_DTUSPAD) {
> @@ -297,7 +297,7 @@ static int wacom_graphire_irq(struct wacom_wac *wacom)
> {
> struct wacom_features *features = &wacom->features;
> unsigned char *data = wacom->data;
> - struct input_dev *input = wacom->input;
> + struct input_dev *input = wacom->pen_input;
> struct input_dev *pad_input = wacom->pad_input;
> int battery_capacity, ps_connected;
> int prox;
> @@ -464,7 +464,7 @@ static int wacom_intuos_inout(struct wacom_wac *wacom)
> {
> struct wacom_features *features = &wacom->features;
> unsigned char *data = wacom->data;
> - struct input_dev *input = wacom->input;
> + struct input_dev *input = wacom->pen_input;
> int idx = 0;
>
> /* tool number */
> @@ -649,7 +649,7 @@ static void wacom_intuos_general(struct wacom_wac *wacom)
> {
> struct wacom_features *features = &wacom->features;
> unsigned char *data = wacom->data;
> - struct input_dev *input = wacom->input;
> + struct input_dev *input = wacom->pen_input;
> unsigned int t;
>
> /* general pen packet */
> @@ -681,7 +681,7 @@ static int wacom_intuos_irq(struct wacom_wac *wacom)
> {
> struct wacom_features *features = &wacom->features;
> unsigned char *data = wacom->data;
> - struct input_dev *input = wacom->input;
> + struct input_dev *input = wacom->pen_input;
> unsigned int t;
> int idx = 0, result;
>
> @@ -1025,7 +1025,7 @@ static void wacom_intuos_bt_process_data(struct wacom_wac *wacom,
> memcpy(wacom->data, data, 10);
> wacom_intuos_irq(wacom);
>
> - input_sync(wacom->input);
> + input_sync(wacom->pen_input);
> if (wacom->pad_input)
> input_sync(wacom->pad_input);
> }
> @@ -1057,7 +1057,7 @@ static int wacom_intuos_bt_irq(struct wacom_wac *wacom, size_t len)
> ps_connected);
> break;
> default:
> - dev_dbg(wacom->input->dev.parent,
> + dev_dbg(wacom->pen_input->dev.parent,
> "Unknown report: %d,%d size:%zu\n",
> data[0], data[1], len);
> return 0;
> @@ -1067,7 +1067,7 @@ static int wacom_intuos_bt_irq(struct wacom_wac *wacom, size_t len)
>
> static int wacom_wac_finger_count_touches(struct wacom_wac *wacom)
> {
> - struct input_dev *input = wacom->input;
> + struct input_dev *input = wacom->touch_input;
> unsigned touch_max = wacom->features.touch_max;
> int count = 0;
> int i;
> @@ -1088,7 +1088,7 @@ static int wacom_wac_finger_count_touches(struct wacom_wac *wacom)
>
> static int wacom_24hdt_irq(struct wacom_wac *wacom)
> {
> - struct input_dev *input = wacom->input;
> + struct input_dev *input = wacom->touch_input;
> unsigned char *data = wacom->data;
> int i;
> int current_num_contacts = data[61];
> @@ -1156,7 +1156,7 @@ static int wacom_24hdt_irq(struct wacom_wac *wacom)
>
> static int wacom_mt_touch(struct wacom_wac *wacom)
> {
> - struct input_dev *input = wacom->input;
> + struct input_dev *input = wacom->touch_input;
> unsigned char *data = wacom->data;
> int i;
> int current_num_contacts = data[2];
> @@ -1207,7 +1207,7 @@ static int wacom_mt_touch(struct wacom_wac *wacom)
>
> static int wacom_tpc_mt_touch(struct wacom_wac *wacom)
> {
> - struct input_dev *input = wacom->input;
> + struct input_dev *input = wacom->touch_input;
> unsigned char *data = wacom->data;
> int i;
>
> @@ -1236,7 +1236,7 @@ static int wacom_tpc_mt_touch(struct wacom_wac *wacom)
> static int wacom_tpc_single_touch(struct wacom_wac *wacom, size_t len)
> {
> unsigned char *data = wacom->data;
> - struct input_dev *input = wacom->input;
> + struct input_dev *input = wacom->touch_input;
> bool prox = !wacom->shared->stylus_in_proximity;
> int x = 0, y = 0;
>
> @@ -1272,7 +1272,7 @@ static int wacom_tpc_single_touch(struct wacom_wac *wacom, size_t len)
> static int wacom_tpc_pen(struct wacom_wac *wacom)
> {
> unsigned char *data = wacom->data;
> - struct input_dev *input = wacom->input;
> + struct input_dev *input = wacom->pen_input;
> bool prox = data[1] & 0x20;
>
> if (!wacom->shared->stylus_in_proximity) /* first in prox */
> @@ -1301,8 +1301,12 @@ static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len)
> {
> unsigned char *data = wacom->data;
>
> - dev_dbg(wacom->input->dev.parent,
> - "%s: received report #%d\n", __func__, data[0]);
> + if (wacom->pen_input)
> + dev_dbg(wacom->pen_input->dev.parent,
> + "%s: received report #%d\n", __func__, data[0]);
> + else if (wacom->touch_input)
> + dev_dbg(wacom->touch_input->dev.parent,
> + "%s: received report #%d\n", __func__, data[0]);
>
> switch (len) {
> case WACOM_PKGLEN_TPC1FG:
> @@ -1334,11 +1338,9 @@ 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,
> +static void wacom_map_usage(struct input_dev *input, 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;
>
> @@ -1366,36 +1368,38 @@ 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);
> + struct wacom_wac *wacom_wac = &wacom->wacom_wac;
> + struct input_dev *input = wacom_wac->pen_input;
>
> switch (usage->hid) {
> case HID_GD_X:
> - wacom_map_usage(wacom, usage, field, EV_ABS, ABS_X, 4);
> + wacom_map_usage(input, usage, field, EV_ABS, ABS_X, 4);
> break;
> case HID_GD_Y:
> - wacom_map_usage(wacom, usage, field, EV_ABS, ABS_Y, 4);
> + wacom_map_usage(input, usage, field, EV_ABS, ABS_Y, 4);
> break;
> case HID_DG_TIPPRESSURE:
> - wacom_map_usage(wacom, usage, field, EV_ABS, ABS_PRESSURE, 0);
> + wacom_map_usage(input, usage, field, EV_ABS, ABS_PRESSURE, 0);
> break;
> case HID_DG_INRANGE:
> - wacom_map_usage(wacom, usage, field, EV_KEY, BTN_TOOL_PEN, 0);
> + wacom_map_usage(input, usage, field, EV_KEY, BTN_TOOL_PEN, 0);
> break;
> case HID_DG_INVERT:
> - wacom_map_usage(wacom, usage, field, EV_KEY,
> + wacom_map_usage(input, 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);
> + wacom_map_usage(input, usage, field, EV_KEY, BTN_TOUCH, 0);
> break;
> case HID_DG_BARRELSWITCH:
> - wacom_map_usage(wacom, usage, field, EV_KEY, BTN_STYLUS, 0);
> + wacom_map_usage(input, usage, field, EV_KEY, BTN_STYLUS, 0);
> break;
> case HID_DG_BARRELSWITCH2:
> - wacom_map_usage(wacom, usage, field, EV_KEY, BTN_STYLUS2, 0);
> + wacom_map_usage(input, usage, field, EV_KEY, BTN_STYLUS2, 0);
> break;
> case HID_DG_TOOLSERIALNUMBER:
> - wacom_map_usage(wacom, usage, field, EV_MSC, MSC_SERIAL, 0);
> + wacom_map_usage(input, usage, field, EV_MSC, MSC_SERIAL, 0);
> break;
> }
> }
> @@ -1405,7 +1409,7 @@ static int wacom_wac_pen_event(struct hid_device *hdev, struct hid_field *field,
> {
> struct wacom *wacom = hid_get_drvdata(hdev);
> struct wacom_wac *wacom_wac = &wacom->wacom_wac;
> - struct input_dev *input = wacom_wac->input;
> + struct input_dev *input = wacom_wac->pen_input;
>
> /* checking which Tool / tip switch to send */
> switch (usage->hid) {
> @@ -1435,7 +1439,7 @@ static void wacom_wac_pen_report(struct hid_device *hdev,
> {
> struct wacom *wacom = hid_get_drvdata(hdev);
> struct wacom_wac *wacom_wac = &wacom->wacom_wac;
> - struct input_dev *input = wacom_wac->input;
> + struct input_dev *input = wacom_wac->pen_input;
> bool prox = wacom_wac->hid_data.inrange_state;
>
> if (!wacom_wac->shared->stylus_in_proximity) /* first in prox */
> @@ -1464,23 +1468,24 @@ static void wacom_wac_finger_usage_mapping(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;
> + struct input_dev *input = wacom_wac->touch_input;
> unsigned touch_max = wacom_wac->features.touch_max;
>
> switch (usage->hid) {
> case HID_GD_X:
> features->last_slot_field = usage->hid;
> if (touch_max == 1)
> - wacom_map_usage(wacom, usage, field, EV_ABS, ABS_X, 4);
> + wacom_map_usage(input, usage, field, EV_ABS, ABS_X, 4);
> else
> - wacom_map_usage(wacom, usage, field, EV_ABS,
> + wacom_map_usage(input, usage, field, EV_ABS,
> ABS_MT_POSITION_X, 4);
> break;
> case HID_GD_Y:
> features->last_slot_field = usage->hid;
> if (touch_max == 1)
> - wacom_map_usage(wacom, usage, field, EV_ABS, ABS_Y, 4);
> + wacom_map_usage(input, usage, field, EV_ABS, ABS_Y, 4);
> else
> - wacom_map_usage(wacom, usage, field, EV_ABS,
> + wacom_map_usage(input, usage, field, EV_ABS,
> ABS_MT_POSITION_Y, 4);
> break;
> case HID_DG_CONTACTID:
> @@ -1494,7 +1499,7 @@ static void wacom_wac_finger_usage_mapping(struct hid_device *hdev,
> break;
> case HID_DG_TIPSWITCH:
> features->last_slot_field = usage->hid;
> - wacom_map_usage(wacom, usage, field, EV_KEY, BTN_TOUCH, 0);
> + wacom_map_usage(input, usage, field, EV_KEY, BTN_TOUCH, 0);
> break;
> }
> }
> @@ -1550,7 +1555,7 @@ static int wacom_wac_finger_event(struct hid_device *hdev,
>
> if (usage->usage_index + 1 == field->report_count) {
> if (usage->hid == wacom_wac->features.last_slot_field)
> - wacom_wac_finger_slot(wacom_wac, wacom_wac->input);
> + wacom_wac_finger_slot(wacom_wac, wacom_wac->touch_input);
> }
>
> return 0;
> @@ -1561,7 +1566,7 @@ static void wacom_wac_finger_report(struct hid_device *hdev,
> {
> struct wacom *wacom = hid_get_drvdata(hdev);
> struct wacom_wac *wacom_wac = &wacom->wacom_wac;
> - struct input_dev *input = wacom_wac->input;
> + struct input_dev *input = wacom_wac->touch_input;
> unsigned touch_max = wacom_wac->features.touch_max;
>
> if (touch_max > 1)
> @@ -1578,10 +1583,10 @@ void wacom_wac_usage_mapping(struct hid_device *hdev,
> {
> 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);
> + __set_bit(INPUT_PROP_DIRECT, wacom_wac->pen_input->propbit);
> + __set_bit(INPUT_PROP_DIRECT, wacom_wac->touch_input->propbit);
>
> if (WACOM_PEN_FIELD(field))
> return wacom_wac_pen_usage_mapping(hdev, field, usage);
> @@ -1626,7 +1631,7 @@ void wacom_wac_report(struct hid_device *hdev, struct hid_report *report)
> static int wacom_bpt_touch(struct wacom_wac *wacom)
> {
> struct wacom_features *features = &wacom->features;
> - struct input_dev *input = wacom->input;
> + struct input_dev *input = wacom->touch_input;
> struct input_dev *pad_input = wacom->pad_input;
> unsigned char *data = wacom->data;
> int i;
> @@ -1674,7 +1679,7 @@ static int wacom_bpt_touch(struct wacom_wac *wacom)
> static void wacom_bpt3_touch_msg(struct wacom_wac *wacom, unsigned char *data)
> {
> struct wacom_features *features = &wacom->features;
> - struct input_dev *input = wacom->input;
> + struct input_dev *input = wacom->touch_input;
> bool touch = data[1] & 0x80;
> int slot = input_mt_get_slot_by_key(input, data[0]);
>
> @@ -1732,7 +1737,7 @@ static void wacom_bpt3_button_msg(struct wacom_wac *wacom, unsigned char *data)
>
> static int wacom_bpt3_touch(struct wacom_wac *wacom)
> {
> - struct input_dev *input = wacom->input;
> + struct input_dev *input = wacom->touch_input;
> unsigned char *data = wacom->data;
> int count = data[1] & 0x07;
> int i;
> @@ -1760,7 +1765,7 @@ static int wacom_bpt3_touch(struct wacom_wac *wacom)
> static int wacom_bpt_pen(struct wacom_wac *wacom)
> {
> struct wacom_features *features = &wacom->features;
> - struct input_dev *input = wacom->input;
> + struct input_dev *input = wacom->pen_input;
> unsigned char *data = wacom->data;
> int prox = 0, x = 0, y = 0, p = 0, d = 0, pen = 0, btn1 = 0, btn2 = 0;
>
> @@ -1869,7 +1874,7 @@ static void wacom_bamboo_pad_pen_event(struct wacom_wac *wacom,
> static int wacom_bamboo_pad_touch_event(struct wacom_wac *wacom,
> unsigned char *data)
> {
> - struct input_dev *input = wacom->input;
> + struct input_dev *input = wacom->touch_input;
> unsigned char *finger_data, prefix;
> unsigned id;
> int x, y;
> @@ -2113,7 +2118,10 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
> }
>
> if (sync) {
> - input_sync(wacom_wac->input);
> + if (wacom_wac->pen_input)
> + input_sync(wacom_wac->pen_input);
> + if (wacom_wac->touch_input)
> + input_sync(wacom_wac->touch_input);
> if (wacom_wac->pad_input)
> input_sync(wacom_wac->pad_input);
> }
> @@ -2121,7 +2129,7 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
>
> static void wacom_setup_cintiq(struct wacom_wac *wacom_wac)
> {
> - struct input_dev *input_dev = wacom_wac->input;
> + struct input_dev *input_dev = wacom_wac->pen_input;
>
> input_set_capability(input_dev, EV_MSC, MSC_SERIAL);
>
> @@ -2144,7 +2152,7 @@ static void wacom_setup_cintiq(struct wacom_wac *wacom_wac)
>
> static void wacom_setup_intuos(struct wacom_wac *wacom_wac)
> {
> - struct input_dev *input_dev = wacom_wac->input;
> + struct input_dev *input_dev = wacom_wac->pen_input;
>
> input_set_capability(input_dev, EV_REL, REL_WHEEL);
>
> diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
> index c873c9f..2978c30 100644
> --- a/drivers/hid/wacom_wac.h
> +++ b/drivers/hid/wacom_wac.h
> @@ -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;
> --
> 2.4.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/5] HID: wacom: Treat features->device_type values as flags
2015-06-04 1:18 ` [PATCH 2/5] HID: wacom: Treat features->device_type values as flags Jason Gerecke
@ 2015-06-04 18:59 ` Benjamin Tissoires
2015-06-04 21:04 ` Jason Gerecke
0 siblings, 1 reply; 16+ messages in thread
From: Benjamin Tissoires @ 2015-06-04 18:59 UTC (permalink / raw)
To: Jason Gerecke
Cc: Ping Cheng, Aaron Skomra, Jiri Kosina, linux-input, Jason Gerecke
On Jun 03 2015 or thereabouts, Jason Gerecke wrote:
> 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 <jason.gerecke@wacom.com>
> ---
> 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(-)
>
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index bdf31c9..2b4cbd8 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -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;
> }
>
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index a52fc25..5e7710d 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -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;
As you mentioned, we are safe right now because there is no device that
shares both pen and touch on the same HID interface (expect the one you
are making the patch series for).
I am just wondering if this will not bite us back when we will have to
use a features->x_pen_max and features_x_touch_max for the same
interface.
No need to change it now (I guess we are fine with HID generic devices
right now), but this is something we might want to have in our heads in
the future.
Cheers,
Benjamin
> }
> @@ -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;
> diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
> index 9a5ee62..da2b309 100644
> --- a/drivers/hid/wacom_wac.h
> +++ b/drivers/hid/wacom_wac.h
> @@ -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) || \
> --
> 2.4.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 5/5] HID: wacom: Introduce new 'touch_input' device
2015-06-04 18:56 ` Benjamin Tissoires
@ 2015-06-04 20:49 ` Jason Gerecke
0 siblings, 0 replies; 16+ messages in thread
From: Jason Gerecke @ 2015-06-04 20:49 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Ping Cheng, Aaron Skomra, Jiri Kosina, linux-input, Jason Gerecke
On 6/4/2015 11:56 AM, Benjamin Tissoires wrote:
> Hi Jason,
>
> On Jun 03 2015 or thereabouts, Jason Gerecke wrote:
>> 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 <jason.gerecke@wacom.com>
>> ---
>
> Besides the Wacom PAD problem already mentioned in the 0/5 thread (I
> will test it tomorrow and come with a following patch if need), I have 2
> nitpicks here:
>
>> drivers/hid/wacom_sys.c | 118 +++++++++++++++++++++++++++++-------------------
>> drivers/hid/wacom_wac.c | 108 ++++++++++++++++++++++++--------------------
>> drivers/hid/wacom_wac.h | 9 ++--
>> 3 files changed, 135 insertions(+), 100 deletions(-)
>>
>> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
>> index a213a8a..a928c1d 100644
>> --- a/drivers/hid/wacom_sys.c
>> +++ b/drivers/hid/wacom_sys.c
>> @@ -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);
>
> I know you are not introducing anything new here, but that means that
> now, we reserve 3 input devices, while we might not even use one.
>
> I just wonder if we should not start having a smarter input allocation
> in wacom.ko where we would create the inputs only if they are needed.
> IIRC, I came to use this extra pad input node to have all the bits in
> place while parsing the report descriptor, but maybe we could be
> smarter.
>
I've been considering this as well. I was thinking about creating some
kind of structure to allow us to better separate the individual tools
(e.g. a kind of tool-specific "wacom_wac" with name, input device, and
features [addressing your concerns in patch 2]) but haven't come up with
a good design yet. Its on my list of back-burner tasks, though we might
be able to do something about the excess allocations themselves without
quite as much work.
>> - 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)
>> - return error;
>> - wacom_wac->input_registered = true;
>> + 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)
>> + 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;
>> }
>>
>> @@ -1303,7 +1336,7 @@ 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;
>> - 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);
>> @@ -1324,11 +1357,11 @@ static void wacom_wireless_work(struct work_struct *work)
>> wacom_wac2->features.x_max = wacom_wac2->features.y_max = 4096;
>> if (wacom_wac2->features.touch_max) {
>> wacom_wac2->features.device_type |= WACOM_DEVICETYPE_TOUCH;
>> - snprintf(wacom_wac2->name, WACOM_NAME_MAX,
>> + snprintf(wacom_wac2->touch_name, WACOM_NAME_MAX,
>> "%s (WL) Finger",wacom_wac2->features.name);
>> } else {
>> wacom_wac2->features.device_type |= WACOM_DEVICETYPE_PAD;
>> - snprintf(wacom_wac2->name, WACOM_NAME_MAX,
>> + snprintf(wacom_wac2->pad_name, WACOM_NAME_MAX,
>> "%s (WL) Pad",wacom_wac2->features.name);
>> }
>> snprintf(wacom_wac2->pad_name, WACOM_NAME_MAX,
>> @@ -1341,7 +1374,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->pen_input;
>> }
>>
>> error = wacom_initialize_battery(wacom);
>> @@ -1456,21 +1489,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 Pad", name);
>
> typo, this should be "%s Pen"
>
ACK. Will be fixed in v2 :)
>
> The rest of the series looks fine. I have one more comment to add in 2/5
> but it does not change the whole behaviour. I will try to go a little
> bit deeper in the review tomorrow, but for now, this is a good start :)
>
> Cheers,
> Benjamin
>
Thanks. There's definitely a chance for subtle interactions, and while I
hope I've covered all my bases I'm glad to have the extra eyes and brain
cells :D
--
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....
>> + snprintf(wacom_wac->touch_name, sizeof(wacom_wac->touch_name),
>> + "%s Touch", 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,
>> @@ -1614,7 +1638,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->pen_input;
>> }
>>
>> return 0;
>> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
>> index 318d9d3..94334be 100644
>> --- a/drivers/hid/wacom_wac.c
>> +++ b/drivers/hid/wacom_wac.c
>> @@ -69,7 +69,7 @@ static void wacom_notify_battery(struct wacom_wac *wacom_wac,
>> static int wacom_penpartner_irq(struct wacom_wac *wacom)
>> {
>> unsigned char *data = wacom->data;
>> - struct input_dev *input = wacom->input;
>> + struct input_dev *input = wacom->pen_input;
>>
>> switch (data[0]) {
>> case 1:
>> @@ -114,7 +114,7 @@ static int wacom_pl_irq(struct wacom_wac *wacom)
>> {
>> struct wacom_features *features = &wacom->features;
>> unsigned char *data = wacom->data;
>> - struct input_dev *input = wacom->input;
>> + struct input_dev *input = wacom->pen_input;
>> int prox, pressure;
>>
>> if (data[0] != WACOM_REPORT_PENABLED) {
>> @@ -186,7 +186,7 @@ static int wacom_pl_irq(struct wacom_wac *wacom)
>> static int wacom_ptu_irq(struct wacom_wac *wacom)
>> {
>> unsigned char *data = wacom->data;
>> - struct input_dev *input = wacom->input;
>> + struct input_dev *input = wacom->pen_input;
>>
>> if (data[0] != WACOM_REPORT_PENABLED) {
>> dev_dbg(input->dev.parent,
>> @@ -215,7 +215,7 @@ static int wacom_ptu_irq(struct wacom_wac *wacom)
>> static int wacom_dtu_irq(struct wacom_wac *wacom)
>> {
>> unsigned char *data = wacom->data;
>> - struct input_dev *input = wacom->input;
>> + struct input_dev *input = wacom->pen_input;
>> int prox = data[1] & 0x20;
>>
>> dev_dbg(input->dev.parent,
>> @@ -245,7 +245,7 @@ static int wacom_dtu_irq(struct wacom_wac *wacom)
>> static int wacom_dtus_irq(struct wacom_wac *wacom)
>> {
>> char *data = wacom->data;
>> - struct input_dev *input = wacom->input;
>> + struct input_dev *input = wacom->pen_input;
>> unsigned short prox, pressure = 0;
>>
>> if (data[0] != WACOM_REPORT_DTUS && data[0] != WACOM_REPORT_DTUSPAD) {
>> @@ -297,7 +297,7 @@ static int wacom_graphire_irq(struct wacom_wac *wacom)
>> {
>> struct wacom_features *features = &wacom->features;
>> unsigned char *data = wacom->data;
>> - struct input_dev *input = wacom->input;
>> + struct input_dev *input = wacom->pen_input;
>> struct input_dev *pad_input = wacom->pad_input;
>> int battery_capacity, ps_connected;
>> int prox;
>> @@ -464,7 +464,7 @@ static int wacom_intuos_inout(struct wacom_wac *wacom)
>> {
>> struct wacom_features *features = &wacom->features;
>> unsigned char *data = wacom->data;
>> - struct input_dev *input = wacom->input;
>> + struct input_dev *input = wacom->pen_input;
>> int idx = 0;
>>
>> /* tool number */
>> @@ -649,7 +649,7 @@ static void wacom_intuos_general(struct wacom_wac *wacom)
>> {
>> struct wacom_features *features = &wacom->features;
>> unsigned char *data = wacom->data;
>> - struct input_dev *input = wacom->input;
>> + struct input_dev *input = wacom->pen_input;
>> unsigned int t;
>>
>> /* general pen packet */
>> @@ -681,7 +681,7 @@ static int wacom_intuos_irq(struct wacom_wac *wacom)
>> {
>> struct wacom_features *features = &wacom->features;
>> unsigned char *data = wacom->data;
>> - struct input_dev *input = wacom->input;
>> + struct input_dev *input = wacom->pen_input;
>> unsigned int t;
>> int idx = 0, result;
>>
>> @@ -1025,7 +1025,7 @@ static void wacom_intuos_bt_process_data(struct wacom_wac *wacom,
>> memcpy(wacom->data, data, 10);
>> wacom_intuos_irq(wacom);
>>
>> - input_sync(wacom->input);
>> + input_sync(wacom->pen_input);
>> if (wacom->pad_input)
>> input_sync(wacom->pad_input);
>> }
>> @@ -1057,7 +1057,7 @@ static int wacom_intuos_bt_irq(struct wacom_wac *wacom, size_t len)
>> ps_connected);
>> break;
>> default:
>> - dev_dbg(wacom->input->dev.parent,
>> + dev_dbg(wacom->pen_input->dev.parent,
>> "Unknown report: %d,%d size:%zu\n",
>> data[0], data[1], len);
>> return 0;
>> @@ -1067,7 +1067,7 @@ static int wacom_intuos_bt_irq(struct wacom_wac *wacom, size_t len)
>>
>> static int wacom_wac_finger_count_touches(struct wacom_wac *wacom)
>> {
>> - struct input_dev *input = wacom->input;
>> + struct input_dev *input = wacom->touch_input;
>> unsigned touch_max = wacom->features.touch_max;
>> int count = 0;
>> int i;
>> @@ -1088,7 +1088,7 @@ static int wacom_wac_finger_count_touches(struct wacom_wac *wacom)
>>
>> static int wacom_24hdt_irq(struct wacom_wac *wacom)
>> {
>> - struct input_dev *input = wacom->input;
>> + struct input_dev *input = wacom->touch_input;
>> unsigned char *data = wacom->data;
>> int i;
>> int current_num_contacts = data[61];
>> @@ -1156,7 +1156,7 @@ static int wacom_24hdt_irq(struct wacom_wac *wacom)
>>
>> static int wacom_mt_touch(struct wacom_wac *wacom)
>> {
>> - struct input_dev *input = wacom->input;
>> + struct input_dev *input = wacom->touch_input;
>> unsigned char *data = wacom->data;
>> int i;
>> int current_num_contacts = data[2];
>> @@ -1207,7 +1207,7 @@ static int wacom_mt_touch(struct wacom_wac *wacom)
>>
>> static int wacom_tpc_mt_touch(struct wacom_wac *wacom)
>> {
>> - struct input_dev *input = wacom->input;
>> + struct input_dev *input = wacom->touch_input;
>> unsigned char *data = wacom->data;
>> int i;
>>
>> @@ -1236,7 +1236,7 @@ static int wacom_tpc_mt_touch(struct wacom_wac *wacom)
>> static int wacom_tpc_single_touch(struct wacom_wac *wacom, size_t len)
>> {
>> unsigned char *data = wacom->data;
>> - struct input_dev *input = wacom->input;
>> + struct input_dev *input = wacom->touch_input;
>> bool prox = !wacom->shared->stylus_in_proximity;
>> int x = 0, y = 0;
>>
>> @@ -1272,7 +1272,7 @@ static int wacom_tpc_single_touch(struct wacom_wac *wacom, size_t len)
>> static int wacom_tpc_pen(struct wacom_wac *wacom)
>> {
>> unsigned char *data = wacom->data;
>> - struct input_dev *input = wacom->input;
>> + struct input_dev *input = wacom->pen_input;
>> bool prox = data[1] & 0x20;
>>
>> if (!wacom->shared->stylus_in_proximity) /* first in prox */
>> @@ -1301,8 +1301,12 @@ static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len)
>> {
>> unsigned char *data = wacom->data;
>>
>> - dev_dbg(wacom->input->dev.parent,
>> - "%s: received report #%d\n", __func__, data[0]);
>> + if (wacom->pen_input)
>> + dev_dbg(wacom->pen_input->dev.parent,
>> + "%s: received report #%d\n", __func__, data[0]);
>> + else if (wacom->touch_input)
>> + dev_dbg(wacom->touch_input->dev.parent,
>> + "%s: received report #%d\n", __func__, data[0]);
>>
>> switch (len) {
>> case WACOM_PKGLEN_TPC1FG:
>> @@ -1334,11 +1338,9 @@ 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,
>> +static void wacom_map_usage(struct input_dev *input, 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;
>>
>> @@ -1366,36 +1368,38 @@ 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);
>> + struct wacom_wac *wacom_wac = &wacom->wacom_wac;
>> + struct input_dev *input = wacom_wac->pen_input;
>>
>> switch (usage->hid) {
>> case HID_GD_X:
>> - wacom_map_usage(wacom, usage, field, EV_ABS, ABS_X, 4);
>> + wacom_map_usage(input, usage, field, EV_ABS, ABS_X, 4);
>> break;
>> case HID_GD_Y:
>> - wacom_map_usage(wacom, usage, field, EV_ABS, ABS_Y, 4);
>> + wacom_map_usage(input, usage, field, EV_ABS, ABS_Y, 4);
>> break;
>> case HID_DG_TIPPRESSURE:
>> - wacom_map_usage(wacom, usage, field, EV_ABS, ABS_PRESSURE, 0);
>> + wacom_map_usage(input, usage, field, EV_ABS, ABS_PRESSURE, 0);
>> break;
>> case HID_DG_INRANGE:
>> - wacom_map_usage(wacom, usage, field, EV_KEY, BTN_TOOL_PEN, 0);
>> + wacom_map_usage(input, usage, field, EV_KEY, BTN_TOOL_PEN, 0);
>> break;
>> case HID_DG_INVERT:
>> - wacom_map_usage(wacom, usage, field, EV_KEY,
>> + wacom_map_usage(input, 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);
>> + wacom_map_usage(input, usage, field, EV_KEY, BTN_TOUCH, 0);
>> break;
>> case HID_DG_BARRELSWITCH:
>> - wacom_map_usage(wacom, usage, field, EV_KEY, BTN_STYLUS, 0);
>> + wacom_map_usage(input, usage, field, EV_KEY, BTN_STYLUS, 0);
>> break;
>> case HID_DG_BARRELSWITCH2:
>> - wacom_map_usage(wacom, usage, field, EV_KEY, BTN_STYLUS2, 0);
>> + wacom_map_usage(input, usage, field, EV_KEY, BTN_STYLUS2, 0);
>> break;
>> case HID_DG_TOOLSERIALNUMBER:
>> - wacom_map_usage(wacom, usage, field, EV_MSC, MSC_SERIAL, 0);
>> + wacom_map_usage(input, usage, field, EV_MSC, MSC_SERIAL, 0);
>> break;
>> }
>> }
>> @@ -1405,7 +1409,7 @@ static int wacom_wac_pen_event(struct hid_device *hdev, struct hid_field *field,
>> {
>> struct wacom *wacom = hid_get_drvdata(hdev);
>> struct wacom_wac *wacom_wac = &wacom->wacom_wac;
>> - struct input_dev *input = wacom_wac->input;
>> + struct input_dev *input = wacom_wac->pen_input;
>>
>> /* checking which Tool / tip switch to send */
>> switch (usage->hid) {
>> @@ -1435,7 +1439,7 @@ static void wacom_wac_pen_report(struct hid_device *hdev,
>> {
>> struct wacom *wacom = hid_get_drvdata(hdev);
>> struct wacom_wac *wacom_wac = &wacom->wacom_wac;
>> - struct input_dev *input = wacom_wac->input;
>> + struct input_dev *input = wacom_wac->pen_input;
>> bool prox = wacom_wac->hid_data.inrange_state;
>>
>> if (!wacom_wac->shared->stylus_in_proximity) /* first in prox */
>> @@ -1464,23 +1468,24 @@ static void wacom_wac_finger_usage_mapping(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;
>> + struct input_dev *input = wacom_wac->touch_input;
>> unsigned touch_max = wacom_wac->features.touch_max;
>>
>> switch (usage->hid) {
>> case HID_GD_X:
>> features->last_slot_field = usage->hid;
>> if (touch_max == 1)
>> - wacom_map_usage(wacom, usage, field, EV_ABS, ABS_X, 4);
>> + wacom_map_usage(input, usage, field, EV_ABS, ABS_X, 4);
>> else
>> - wacom_map_usage(wacom, usage, field, EV_ABS,
>> + wacom_map_usage(input, usage, field, EV_ABS,
>> ABS_MT_POSITION_X, 4);
>> break;
>> case HID_GD_Y:
>> features->last_slot_field = usage->hid;
>> if (touch_max == 1)
>> - wacom_map_usage(wacom, usage, field, EV_ABS, ABS_Y, 4);
>> + wacom_map_usage(input, usage, field, EV_ABS, ABS_Y, 4);
>> else
>> - wacom_map_usage(wacom, usage, field, EV_ABS,
>> + wacom_map_usage(input, usage, field, EV_ABS,
>> ABS_MT_POSITION_Y, 4);
>> break;
>> case HID_DG_CONTACTID:
>> @@ -1494,7 +1499,7 @@ static void wacom_wac_finger_usage_mapping(struct hid_device *hdev,
>> break;
>> case HID_DG_TIPSWITCH:
>> features->last_slot_field = usage->hid;
>> - wacom_map_usage(wacom, usage, field, EV_KEY, BTN_TOUCH, 0);
>> + wacom_map_usage(input, usage, field, EV_KEY, BTN_TOUCH, 0);
>> break;
>> }
>> }
>> @@ -1550,7 +1555,7 @@ static int wacom_wac_finger_event(struct hid_device *hdev,
>>
>> if (usage->usage_index + 1 == field->report_count) {
>> if (usage->hid == wacom_wac->features.last_slot_field)
>> - wacom_wac_finger_slot(wacom_wac, wacom_wac->input);
>> + wacom_wac_finger_slot(wacom_wac, wacom_wac->touch_input);
>> }
>>
>> return 0;
>> @@ -1561,7 +1566,7 @@ static void wacom_wac_finger_report(struct hid_device *hdev,
>> {
>> struct wacom *wacom = hid_get_drvdata(hdev);
>> struct wacom_wac *wacom_wac = &wacom->wacom_wac;
>> - struct input_dev *input = wacom_wac->input;
>> + struct input_dev *input = wacom_wac->touch_input;
>> unsigned touch_max = wacom_wac->features.touch_max;
>>
>> if (touch_max > 1)
>> @@ -1578,10 +1583,10 @@ void wacom_wac_usage_mapping(struct hid_device *hdev,
>> {
>> 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);
>> + __set_bit(INPUT_PROP_DIRECT, wacom_wac->pen_input->propbit);
>> + __set_bit(INPUT_PROP_DIRECT, wacom_wac->touch_input->propbit);
>>
>> if (WACOM_PEN_FIELD(field))
>> return wacom_wac_pen_usage_mapping(hdev, field, usage);
>> @@ -1626,7 +1631,7 @@ void wacom_wac_report(struct hid_device *hdev, struct hid_report *report)
>> static int wacom_bpt_touch(struct wacom_wac *wacom)
>> {
>> struct wacom_features *features = &wacom->features;
>> - struct input_dev *input = wacom->input;
>> + struct input_dev *input = wacom->touch_input;
>> struct input_dev *pad_input = wacom->pad_input;
>> unsigned char *data = wacom->data;
>> int i;
>> @@ -1674,7 +1679,7 @@ static int wacom_bpt_touch(struct wacom_wac *wacom)
>> static void wacom_bpt3_touch_msg(struct wacom_wac *wacom, unsigned char *data)
>> {
>> struct wacom_features *features = &wacom->features;
>> - struct input_dev *input = wacom->input;
>> + struct input_dev *input = wacom->touch_input;
>> bool touch = data[1] & 0x80;
>> int slot = input_mt_get_slot_by_key(input, data[0]);
>>
>> @@ -1732,7 +1737,7 @@ static void wacom_bpt3_button_msg(struct wacom_wac *wacom, unsigned char *data)
>>
>> static int wacom_bpt3_touch(struct wacom_wac *wacom)
>> {
>> - struct input_dev *input = wacom->input;
>> + struct input_dev *input = wacom->touch_input;
>> unsigned char *data = wacom->data;
>> int count = data[1] & 0x07;
>> int i;
>> @@ -1760,7 +1765,7 @@ static int wacom_bpt3_touch(struct wacom_wac *wacom)
>> static int wacom_bpt_pen(struct wacom_wac *wacom)
>> {
>> struct wacom_features *features = &wacom->features;
>> - struct input_dev *input = wacom->input;
>> + struct input_dev *input = wacom->pen_input;
>> unsigned char *data = wacom->data;
>> int prox = 0, x = 0, y = 0, p = 0, d = 0, pen = 0, btn1 = 0, btn2 = 0;
>>
>> @@ -1869,7 +1874,7 @@ static void wacom_bamboo_pad_pen_event(struct wacom_wac *wacom,
>> static int wacom_bamboo_pad_touch_event(struct wacom_wac *wacom,
>> unsigned char *data)
>> {
>> - struct input_dev *input = wacom->input;
>> + struct input_dev *input = wacom->touch_input;
>> unsigned char *finger_data, prefix;
>> unsigned id;
>> int x, y;
>> @@ -2113,7 +2118,10 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
>> }
>>
>> if (sync) {
>> - input_sync(wacom_wac->input);
>> + if (wacom_wac->pen_input)
>> + input_sync(wacom_wac->pen_input);
>> + if (wacom_wac->touch_input)
>> + input_sync(wacom_wac->touch_input);
>> if (wacom_wac->pad_input)
>> input_sync(wacom_wac->pad_input);
>> }
>> @@ -2121,7 +2129,7 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
>>
>> static void wacom_setup_cintiq(struct wacom_wac *wacom_wac)
>> {
>> - struct input_dev *input_dev = wacom_wac->input;
>> + struct input_dev *input_dev = wacom_wac->pen_input;
>>
>> input_set_capability(input_dev, EV_MSC, MSC_SERIAL);
>>
>> @@ -2144,7 +2152,7 @@ static void wacom_setup_cintiq(struct wacom_wac *wacom_wac)
>>
>> static void wacom_setup_intuos(struct wacom_wac *wacom_wac)
>> {
>> - struct input_dev *input_dev = wacom_wac->input;
>> + struct input_dev *input_dev = wacom_wac->pen_input;
>>
>> input_set_capability(input_dev, EV_REL, REL_WHEEL);
>>
>> diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
>> index c873c9f..2978c30 100644
>> --- a/drivers/hid/wacom_wac.h
>> +++ b/drivers/hid/wacom_wac.h
>> @@ -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;
>> --
>> 2.4.1
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/5] HID: wacom: Treat features->device_type values as flags
2015-06-04 18:59 ` Benjamin Tissoires
@ 2015-06-04 21:04 ` Jason Gerecke
0 siblings, 0 replies; 16+ messages in thread
From: Jason Gerecke @ 2015-06-04 21:04 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Ping Cheng, Aaron Skomra, Jiri Kosina, linux-input, Jason Gerecke
On 6/4/2015 11:59 AM, Benjamin Tissoires wrote:
> On Jun 03 2015 or thereabouts, Jason Gerecke wrote:
>> 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 <jason.gerecke@wacom.com>
>> ---
>> 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(-)
>>
>> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
>> index bdf31c9..2b4cbd8 100644
>> --- a/drivers/hid/wacom_sys.c
>> +++ b/drivers/hid/wacom_sys.c
>> @@ -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;
>> }
>>
>> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
>> index a52fc25..5e7710d 100644
>> --- a/drivers/hid/wacom_wac.c
>> +++ b/drivers/hid/wacom_wac.c
>> @@ -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;
>
> As you mentioned, we are safe right now because there is no device that
> shares both pen and touch on the same HID interface (expect the one you
> are making the patch series for).
>
> I am just wondering if this will not bite us back when we will have to
> use a features->x_pen_max and features_x_touch_max for the same
> interface.
> No need to change it now (I guess we are fine with HID generic devices
> right now), but this is something we might want to have in our heads in
> the future.
>
> Cheers,
> Benjamin
>
See my comments on patch 5. I'm in complete agreement -- I don't think
it's an issue (thanks to HID_GENERIC), but the design is a little creaky
given the kinds of devices out there. I'd like to see the driver be more
tool-centric and not have to repeat myself three times in every function
to e.g. set the name of the pen, touch, and pad device.
--
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....
>> }
>> @@ -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;
>> diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
>> index 9a5ee62..da2b309 100644
>> --- a/drivers/hid/wacom_wac.h
>> +++ b/drivers/hid/wacom_wac.h
>> @@ -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) || \
>> --
>> 2.4.1
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/5] HID: wacom: Support tablets with pen and touch on same interface
2015-06-04 17:34 ` Jason Gerecke
@ 2015-06-08 15:46 ` Benjamin Tissoires
0 siblings, 0 replies; 16+ messages in thread
From: Benjamin Tissoires @ 2015-06-08 15:46 UTC (permalink / raw)
To: Jason Gerecke
Cc: Ping Cheng, Aaron Skomra, Jiri Kosina, linux-input, Jason Gerecke
On Jun 04 2015 or thereabouts, Jason Gerecke wrote:
> On 6/4/2015 7:18 AM, Benjamin Tissoires wrote:
> > Hi Jason,
> >
> > On Jun 03 2015 or thereabouts, Jason Gerecke wrote:
> >> I've recently got my hands on a device which has an I2C sensor that sends
> >> both pen and touch reports from a single interface. To userspace, it shows
> >> up as a single input device which blends both the report types (e.g. it has
> >> ABS_PRESSURE for the pen, and ABS_MT_POSITION_X for the touch). This patch
> >> set modifies the driver to be able to handle devices which place both pen
> >> and touch on a the same interface. It does this by treating the pen, touch,
> >> and pad (which was already special-cased) independently. If a device has
> >> the appropriate device_type, one or more of pen/touch/pad input devices
> >> will be created, initialized, and used to send data to userspace.
> >>
> >> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> >
> > This all seems sensible. I gave a quick look yesterday and could not
> > found any obvious problem, but I'd like to review it more thoroughly
> > before giving my rev-by (and do a little bit of testing too).
> >
> > I don't believe there will be any problems for the series, besides the
> > Bamboo PAD. Have you tested on this?
> >
> > Cheers,
> > Benjamin
> >
>
> I don't have a Bamboo PAD to test with, but carefully looking at each
> interface's HID descriptor makes me think you may be right. The debug
> interface in particular uses a WACOM_VENDORDEFINED_PEN application
> collection and will be marked as WACOM_DEVICETYPE_PEN instead of
> WACOM_DEVICETYPE_TOUCH.
Just a quick update (and I'll review the series, I promise!).
The Bamboo PAD still works with this series. The only difference from
the current code is that now, we have an extra "pen" input node (unused)
attached to the touch interface (it was maybe what you were saying, but
it was not entirely clear for me, sorry :-P ).
>
> The smallest fix that should do the trick would be to just swap the
> device_type in 'wacom_setup_device_quirks'. Alternatively, we could only
Yeah. That is pretty much what we currently have for handling the PAD.
When the parsing has been made, I just revert the tool.
> allow the debug interface to be probed, set the touch device type flag
> (in addition to the automatically-set pen flag), and extend the IRQ
> function to explicitly handle pen events. The alternative is a bit more
> work (and we'd need somebody with a device to test it...) but I think
> might make the codepath for the Bamboo PAD a bit easier to follow.
It might be easier to follow, but that would mean to implement a n-th
protocol, while the descriptor already gives us the protocol. So not
sure what is simpler here.
Cheers,
Benjamin
>
> --
> 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....
>
> >>
> >> Jason Gerecke (5):
> >> HID: wacom: Simplify 'wacom_update_name'
> >> HID: wacom: Treat features->device_type values as flags
> >> HID: wacom: Introduce a new WACOM_DEVICETYPE_PAD device_type
> >> HID: wacom: Split apart 'wacom_setup_pentouch_input_capabilites'
> >> HID: wacom: Introduce new 'touch_input' device
> >>
> >> drivers/hid/wacom.h | 4 +-
> >> drivers/hid/wacom_sys.c | 194 +++++++++++++++----------
> >> drivers/hid/wacom_wac.c | 379 ++++++++++++++++++++++++++----------------------
> >> drivers/hid/wacom_wac.h | 15 +-
> >> 4 files changed, 332 insertions(+), 260 deletions(-)
> >>
> >> --
> >> 2.4.1
> >>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/5] HID: wacom: Support tablets with pen and touch on same interface
2015-06-04 1:18 [PATCH 0/5] HID: wacom: Support tablets with pen and touch on same interface Jason Gerecke
` (5 preceding siblings ...)
2015-06-04 14:18 ` [PATCH 0/5] HID: wacom: Support tablets with pen and touch on same interface Benjamin Tissoires
@ 2015-06-08 15:59 ` Benjamin Tissoires
2015-06-08 17:49 ` Jason Gerecke
6 siblings, 1 reply; 16+ messages in thread
From: Benjamin Tissoires @ 2015-06-08 15:59 UTC (permalink / raw)
To: Jason Gerecke
Cc: Ping Cheng, Aaron Skomra, Jiri Kosina, linux-input, Jason Gerecke
Hi again,
On Jun 03 2015 or thereabouts, Jason Gerecke wrote:
> I've recently got my hands on a device which has an I2C sensor that sends
> both pen and touch reports from a single interface. To userspace, it shows
> up as a single input device which blends both the report types (e.g. it has
> ABS_PRESSURE for the pen, and ABS_MT_POSITION_X for the touch). This patch
> set modifies the driver to be able to handle devices which place both pen
> and touch on a the same interface. It does this by treating the pen, touch,
> and pad (which was already special-cased) independently. If a device has
> the appropriate device_type, one or more of pen/touch/pad input devices
> will be created, initialized, and used to send data to userspace.
>
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
So, besides the typo in 5/5, the series is:
Reviewed-by: Bnejamin Tissoires <benjamin.tissoires@redhat.com>
I'll send a follow up patch to fix the Bamboo PAD extra interface once
this will land in Jiri's tree.
Cheers,
Benjamin
>
> Jason Gerecke (5):
> HID: wacom: Simplify 'wacom_update_name'
> HID: wacom: Treat features->device_type values as flags
> HID: wacom: Introduce a new WACOM_DEVICETYPE_PAD device_type
> HID: wacom: Split apart 'wacom_setup_pentouch_input_capabilites'
> HID: wacom: Introduce new 'touch_input' device
>
> drivers/hid/wacom.h | 4 +-
> drivers/hid/wacom_sys.c | 194 +++++++++++++++----------
> drivers/hid/wacom_wac.c | 379 ++++++++++++++++++++++++++----------------------
> drivers/hid/wacom_wac.h | 15 +-
> 4 files changed, 332 insertions(+), 260 deletions(-)
>
> --
> 2.4.1
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/5] HID: wacom: Support tablets with pen and touch on same interface
2015-06-08 15:59 ` Benjamin Tissoires
@ 2015-06-08 17:49 ` Jason Gerecke
2015-06-08 19:36 ` Jiri Kosina
0 siblings, 1 reply; 16+ messages in thread
From: Jason Gerecke @ 2015-06-08 17:49 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Ping Cheng, Aaron Skomra, Jiri Kosina, linux-input, Jason Gerecke
On 6/8/2015 8:59 AM, Benjamin Tissoires wrote:
> Hi again,
>
> On Jun 03 2015 or thereabouts, Jason Gerecke wrote:
>> I've recently got my hands on a device which has an I2C sensor that sends
>> both pen and touch reports from a single interface. To userspace, it shows
>> up as a single input device which blends both the report types (e.g. it has
>> ABS_PRESSURE for the pen, and ABS_MT_POSITION_X for the touch). This patch
>> set modifies the driver to be able to handle devices which place both pen
>> and touch on a the same interface. It does this by treating the pen, touch,
>> and pad (which was already special-cased) independently. If a device has
>> the appropriate device_type, one or more of pen/touch/pad input devices
>> will be created, initialized, and used to send data to userspace.
>>
>> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
>
> So, besides the typo in 5/5, the series is:
> Reviewed-by: Bnejamin Tissoires <benjamin.tissoires@redhat.com>
>
> I'll send a follow up patch to fix the Bamboo PAD extra interface once
> this will land in Jiri's tree.
>
> Cheers,
> Benjamin
>
>
Just stumbled across a bug with wireless operation (missing pad device).
Hopefully Jiri hasn't merged anything yet :)
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....
>>
>> Jason Gerecke (5):
>> HID: wacom: Simplify 'wacom_update_name'
>> HID: wacom: Treat features->device_type values as flags
>> HID: wacom: Introduce a new WACOM_DEVICETYPE_PAD device_type
>> HID: wacom: Split apart 'wacom_setup_pentouch_input_capabilites'
>> HID: wacom: Introduce new 'touch_input' device
>>
>> drivers/hid/wacom.h | 4 +-
>> drivers/hid/wacom_sys.c | 194 +++++++++++++++----------
>> drivers/hid/wacom_wac.c | 379 ++++++++++++++++++++++++++----------------------
>> drivers/hid/wacom_wac.h | 15 +-
>> 4 files changed, 332 insertions(+), 260 deletions(-)
>>
>> --
>> 2.4.1
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/5] HID: wacom: Support tablets with pen and touch on same interface
2015-06-08 17:49 ` Jason Gerecke
@ 2015-06-08 19:36 ` Jiri Kosina
0 siblings, 0 replies; 16+ messages in thread
From: Jiri Kosina @ 2015-06-08 19:36 UTC (permalink / raw)
To: Jason Gerecke
Cc: Benjamin Tissoires, Ping Cheng, Aaron Skomra, linux-input, Jason Gerecke
On Mon, 8 Jun 2015, Jason Gerecke wrote:
> Just stumbled across a bug with wireless operation (missing pad device).
> Hopefully Jiri hasn't merged anything yet :)
Indeed, this was still in my 'hid-todo' queue. Given the comments so far,
I am removing this patchset from it, and waiting for v2 refresh.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2015-06-08 19:36 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-04 1:18 [PATCH 0/5] HID: wacom: Support tablets with pen and touch on same interface Jason Gerecke
2015-06-04 1:18 ` [PATCH 1/5] HID: wacom: Simplify 'wacom_update_name' Jason Gerecke
2015-06-04 1:18 ` [PATCH 2/5] HID: wacom: Treat features->device_type values as flags Jason Gerecke
2015-06-04 18:59 ` Benjamin Tissoires
2015-06-04 21:04 ` Jason Gerecke
2015-06-04 1:18 ` [PATCH 3/5] HID: wacom: Introduce a new WACOM_DEVICETYPE_PAD device_type Jason Gerecke
2015-06-04 1:18 ` [PATCH 4/5] HID: wacom: Split apart 'wacom_setup_pentouch_input_capabilites' Jason Gerecke
2015-06-04 1:18 ` [PATCH 5/5] HID: wacom: Introduce new 'touch_input' device Jason Gerecke
2015-06-04 18:56 ` Benjamin Tissoires
2015-06-04 20:49 ` Jason Gerecke
2015-06-04 14:18 ` [PATCH 0/5] HID: wacom: Support tablets with pen and touch on same interface Benjamin Tissoires
2015-06-04 17:34 ` Jason Gerecke
2015-06-08 15:46 ` Benjamin Tissoires
2015-06-08 15:59 ` Benjamin Tissoires
2015-06-08 17:49 ` Jason Gerecke
2015-06-08 19:36 ` Jiri Kosina
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.