From 99acedadde157a02b21761fd406ef7adc7615533 Mon Sep 17 00:00:00 2001 From: Jason Gerecke Date: Fri, 10 Nov 2017 11:50:00 -0800 Subject: [PATCH 1/5] HID: wacom: Properly handle AES serial number and tool type Current AES sensors relay tool type and serial number information with a different set of usages than those prescribed by the modern (i.e. MobileStudio Pro and newer) EMR tablet standard. To ensure the driver properly understands these usages, we modify them to be compatible. The identifying information is split across three consecutive fields: a 16-bit WACOM_HID_WT_SERIALNUMBER (which is more accurately described as WACOM_HID_WD_TOOLTYPE), a 32-bit HID_DG_TOOLSERIALNUMBER, and an 8-bit 0xFF000000 (which should be WACOM_HID_WD_SERIALHI). While we're at it, we also define proper min/max values since may may be undefined on some devices. Signed-off-by: Jason Gerecke Reviewed-by: Benjamin Tissoires Signed-off-by: Jiri Kosina --- drivers/hid/wacom_wac.c | 42 ++++++++++++++++++++++++++++++++--------- drivers/hid/wacom_wac.h | 3 +++ 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c index 16af6886e828..ff679ee3b358 100644 --- a/drivers/hid/wacom_wac.c +++ b/drivers/hid/wacom_wac.c @@ -2086,6 +2086,27 @@ static void wacom_wac_pen_usage_mapping(struct hid_device *hdev, break; case HID_DG_TOOLSERIALNUMBER: wacom_map_usage(input, usage, field, EV_MSC, MSC_SERIAL, 0); + + /* Adjust AES usages to match modern convention */ + if (usage->hid == WACOM_HID_WT_SERIALNUMBER && field->report_size == 16) { + if (field->index + 2 < field->report->maxfield) { + struct hid_field *a = field->report->field[field->index + 1]; + struct hid_field *b = field->report->field[field->index + 2]; + + if (a->maxusage > 0 && a->usage[0].hid == HID_DG_TOOLSERIALNUMBER && a->report_size == 32 && + b->maxusage > 0 && b->usage[0].hid == 0xFF000000 && b->report_size == 8) { + features->quirks |= WACOM_QUIRK_AESPEN; + usage->hid = WACOM_HID_WD_TOOLTYPE; + field->logical_minimum = S16_MIN; + field->logical_maximum = S16_MAX; + a->logical_minimum = S32_MIN; + a->logical_maximum = S32_MAX; + b->usage[0].hid = WACOM_HID_WD_SERIALHI; + b->logical_minimum = 0; + b->logical_maximum = U8_MAX; + } + } + } break; case WACOM_HID_WD_SENSE: features->quirks |= WACOM_QUIRK_SENSE; @@ -2093,15 +2114,18 @@ static void wacom_wac_pen_usage_mapping(struct hid_device *hdev, break; case WACOM_HID_WD_SERIALHI: wacom_map_usage(input, usage, field, EV_ABS, ABS_MISC, 0); - set_bit(EV_KEY, input->evbit); - input_set_capability(input, EV_KEY, BTN_TOOL_PEN); - input_set_capability(input, EV_KEY, BTN_TOOL_RUBBER); - input_set_capability(input, EV_KEY, BTN_TOOL_BRUSH); - input_set_capability(input, EV_KEY, BTN_TOOL_PENCIL); - input_set_capability(input, EV_KEY, BTN_TOOL_AIRBRUSH); - if (!(features->device_type & WACOM_DEVICETYPE_DIRECT)) { - input_set_capability(input, EV_KEY, BTN_TOOL_MOUSE); - input_set_capability(input, EV_KEY, BTN_TOOL_LENS); + + if (!(features->quirks & WACOM_QUIRK_AESPEN)) { + set_bit(EV_KEY, input->evbit); + input_set_capability(input, EV_KEY, BTN_TOOL_PEN); + input_set_capability(input, EV_KEY, BTN_TOOL_RUBBER); + input_set_capability(input, EV_KEY, BTN_TOOL_BRUSH); + input_set_capability(input, EV_KEY, BTN_TOOL_PENCIL); + input_set_capability(input, EV_KEY, BTN_TOOL_AIRBRUSH); + if (!(features->device_type & WACOM_DEVICETYPE_DIRECT)) { + input_set_capability(input, EV_KEY, BTN_TOOL_MOUSE); + input_set_capability(input, EV_KEY, BTN_TOOL_LENS); + } } break; case WACOM_HID_WD_FINGERWHEEL: diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h index 64d8f014602e..6fe6d60f9ab5 100644 --- a/drivers/hid/wacom_wac.h +++ b/drivers/hid/wacom_wac.h @@ -86,6 +86,7 @@ /* device quirks */ #define WACOM_QUIRK_BBTOUCH_LOWRES 0x0001 #define WACOM_QUIRK_SENSE 0x0002 +#define WACOM_QUIRK_AESPEN 0x0004 #define WACOM_QUIRK_BATTERY 0x0008 /* device types */ @@ -107,6 +108,7 @@ #define WACOM_HID_WD_PEN (WACOM_HID_UP_WACOMDIGITIZER | 0x02) #define WACOM_HID_WD_SENSE (WACOM_HID_UP_WACOMDIGITIZER | 0x36) #define WACOM_HID_WD_DIGITIZERFNKEYS (WACOM_HID_UP_WACOMDIGITIZER | 0x39) +#define WACOM_HID_WD_SERIALNUMBER (WACOM_HID_UP_WACOMDIGITIZER | 0x5b) #define WACOM_HID_WD_SERIALHI (WACOM_HID_UP_WACOMDIGITIZER | 0x5c) #define WACOM_HID_WD_TOOLTYPE (WACOM_HID_UP_WACOMDIGITIZER | 0x77) #define WACOM_HID_WD_DISTANCE (WACOM_HID_UP_WACOMDIGITIZER | 0x0132) @@ -150,6 +152,7 @@ #define WACOM_HID_WT_TOUCHSCREEN (WACOM_HID_UP_WACOMTOUCH | 0x04) #define WACOM_HID_WT_TOUCHPAD (WACOM_HID_UP_WACOMTOUCH | 0x05) #define WACOM_HID_WT_CONTACTMAX (WACOM_HID_UP_WACOMTOUCH | 0x55) +#define WACOM_HID_WT_SERIALNUMBER (WACOM_HID_UP_WACOMTOUCH | 0x5b) #define WACOM_HID_WT_X (WACOM_HID_UP_WACOMTOUCH | 0x130) #define WACOM_HID_WT_Y (WACOM_HID_UP_WACOMTOUCH | 0x131) From 83417206427bdf0fef9fa69957807194f25923c3 Mon Sep 17 00:00:00 2001 From: Jason Gerecke Date: Fri, 10 Nov 2017 11:50:01 -0800 Subject: [PATCH 2/5] HID: wacom: Queue events with missing type/serial data for later processing Userspace expects to receive tool type and serial number information for the active pen in the very first kernel report, if such data is supported by the hardware. While this expectation is not an issue for EMR devices, AES sensors will often send several packets worth of in- range data before relaying type/serial data to the kernel. Sending this data "late" can result in proximity-tracking issues by xf86-input-wacom, or an inability to distinguish different pens by input-wacom. Options for dealing with this situation include ignoring reports from the tablet until we get the necessary data, or using the information from the last-seen pen instead of the (eventual) real data. Neither option is particularly attractive: the former results in truncated strokes and the latter causes issues with switching between pens. This commit instead opts to queue up events with missing information until we receive a report which contains it. At that point, we can update the driver's state variables (id[0] and serial[0]) and replay the queued events. Signed-off-by: Jason Gerecke Reviewed-by: Benjamin Tissoires Signed-off-by: Jiri Kosina --- drivers/hid/wacom_sys.c | 110 ++++++++++++++++++++++++++++++++++++++++ drivers/hid/wacom_wac.c | 1 + drivers/hid/wacom_wac.h | 3 ++ 3 files changed, 114 insertions(+) diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c index ee71ad9b6cc1..c9eadf632564 100644 --- a/drivers/hid/wacom_sys.c +++ b/drivers/hid/wacom_sys.c @@ -56,6 +56,107 @@ static int wacom_set_report(struct hid_device *hdev, u8 type, u8 *buf, return retval; } +static void wacom_wac_queue_insert(struct hid_device *hdev, + struct kfifo_rec_ptr_2 *fifo, + u8 *raw_data, int size) +{ + bool warned = false; + + while (kfifo_avail(fifo) < size) { + if (!warned) + hid_warn(hdev, "%s: kfifo has filled, starting to drop events\n", __func__); + warned = true; + + kfifo_skip(fifo); + } + + kfifo_in(fifo, raw_data, size); +} + +static void wacom_wac_queue_flush(struct hid_device *hdev, + struct kfifo_rec_ptr_2 *fifo) +{ + while (!kfifo_is_empty(fifo)) { + u8 buf[WACOM_PKGLEN_MAX]; + int size; + int err; + + size = kfifo_out(fifo, buf, sizeof(buf)); + err = hid_report_raw_event(hdev, HID_INPUT_REPORT, buf, size, false); + if (err) { + hid_warn(hdev, "%s: unable to flush event due to error %d\n", + __func__, err); + } + } +} + +static int wacom_wac_pen_serial_enforce(struct hid_device *hdev, + struct hid_report *report, u8 *raw_data, int size) +{ + struct wacom *wacom = hid_get_drvdata(hdev); + struct wacom_wac *wacom_wac = &wacom->wacom_wac; + struct wacom_features *features = &wacom_wac->features; + bool flush = false; + bool insert = false; + int i, j; + + if (wacom_wac->serial[0] || !(features->quirks & WACOM_QUIRK_TOOLSERIAL)) + return 0; + + /* Queue events which have invalid tool type or serial number */ + for (i = 0; i < report->maxfield; i++) { + for (j = 0; j < report->field[i]->maxusage; j++) { + struct hid_field *field = report->field[i]; + struct hid_usage *usage = &field->usage[j]; + unsigned int equivalent_usage = wacom_equivalent_usage(usage->hid); + unsigned int offset; + unsigned int size; + unsigned int value; + + if (equivalent_usage != HID_DG_INRANGE && + equivalent_usage != HID_DG_TOOLSERIALNUMBER && + equivalent_usage != WACOM_HID_WD_SERIALHI && + equivalent_usage != WACOM_HID_WD_TOOLTYPE) + continue; + + offset = field->report_offset; + size = field->report_size; + value = hid_field_extract(hdev, raw_data+1, offset + j * size, size); + + /* If we go out of range, we need to flush the queue ASAP */ + if (equivalent_usage == HID_DG_INRANGE) + value = !value; + + if (value) { + flush = true; + switch (equivalent_usage) { + case HID_DG_TOOLSERIALNUMBER: + wacom_wac->serial[0] = value; + break; + + case WACOM_HID_WD_SERIALHI: + wacom_wac->serial[0] |= ((__u64)value) << 32; + break; + + case WACOM_HID_WD_TOOLTYPE: + wacom_wac->id[0] = value; + break; + } + } + else { + insert = true; + } + } + } + + if (flush) + wacom_wac_queue_flush(hdev, &wacom_wac->pen_fifo); + else if (insert) + wacom_wac_queue_insert(hdev, &wacom_wac->pen_fifo, raw_data, size); + + return insert && !flush; +} + static int wacom_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *raw_data, int size) { @@ -64,6 +165,9 @@ static int wacom_raw_event(struct hid_device *hdev, struct hid_report *report, if (size > WACOM_PKGLEN_MAX) return 1; + if (wacom_wac_pen_serial_enforce(hdev, report, raw_data, size)) + return -1; + memcpy(wacom->wacom_wac.data, raw_data, size); wacom_wac_irq(&wacom->wacom_wac, size); @@ -2580,6 +2684,10 @@ static int wacom_probe(struct hid_device *hdev, goto fail; } + error = kfifo_alloc(&wacom_wac->pen_fifo, WACOM_PKGLEN_MAX, GFP_KERNEL); + if (error) + goto fail; + wacom_wac->hid_data.inputmode = -1; wacom_wac->mode_report = -1; @@ -2643,6 +2751,8 @@ static void wacom_remove(struct hid_device *hdev) if (wacom->wacom_wac.features.type != REMOTE) wacom_release_resources(wacom); + kfifo_free(&wacom_wac->pen_fifo); + hid_set_drvdata(hdev, NULL); } diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c index ff679ee3b358..7fa373225d8a 100644 --- a/drivers/hid/wacom_wac.c +++ b/drivers/hid/wacom_wac.c @@ -2085,6 +2085,7 @@ static void wacom_wac_pen_usage_mapping(struct hid_device *hdev, wacom_map_usage(input, usage, field, EV_KEY, BTN_STYLUS2, 0); break; case HID_DG_TOOLSERIALNUMBER: + features->quirks |= WACOM_QUIRK_TOOLSERIAL; wacom_map_usage(input, usage, field, EV_MSC, MSC_SERIAL, 0); /* Adjust AES usages to match modern convention */ diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h index 6fe6d60f9ab5..15d9c14fbdf7 100644 --- a/drivers/hid/wacom_wac.h +++ b/drivers/hid/wacom_wac.h @@ -11,6 +11,7 @@ #include #include +#include /* maximum packet length for USB/BT devices */ #define WACOM_PKGLEN_MAX 361 @@ -88,6 +89,7 @@ #define WACOM_QUIRK_SENSE 0x0002 #define WACOM_QUIRK_AESPEN 0x0004 #define WACOM_QUIRK_BATTERY 0x0008 +#define WACOM_QUIRK_TOOLSERIAL 0x0010 /* device types */ #define WACOM_DEVICETYPE_NONE 0x0000 @@ -339,6 +341,7 @@ struct wacom_wac { struct input_dev *pen_input; struct input_dev *touch_input; struct input_dev *pad_input; + struct kfifo_rec_ptr_2 pen_fifo; int pid; int num_contacts_left; u8 bt_features; From 791ae273731fa85d3332e45064dab177ae663e80 Mon Sep 17 00:00:00 2001 From: Aaron Armstrong Skomra Date: Thu, 7 Dec 2017 12:31:56 -0800 Subject: [PATCH 3/5] HID: wacom: EKR: ensure devres groups at higher indexes are released Background: ExpressKey Remotes communicate their events via usb dongle. Each dongle can hold up to 5 pairings at one time and one EKR (identified by its serial number) can unfortunately be paired with its dongle more than once. The pairing takes place in a round-robin fashion. Input devices are only created once per EKR, when a new serial number is seen in the list of pairings. However, if a device is created for a "higher" paring index and subsequently a second pairing occurs at a lower pairing index, unpairing the remote with that serial number from any pairing index will currently cause a driver crash. This occurs infrequently, as two remotes are necessary to trigger this bug and most users have only one remote. As an illustration, to trigger the bug you need to have two remotes, and pair them in this order: 1. slot 0 -> remote 1 (input device created for remote 1) 2. slot 1 -> remote 1 (duplicate pairing - no device created) 3. slot 2 -> remote 1 (duplicate pairing - no device created) 4. slot 3 -> remote 1 (duplicate pairing - no device created) 5. slot 4 -> remote 2 (input device created for remote 2) 6. slot 0 -> remote 2 (1 destroyed and recreated at slot 1) 7. slot 1 -> remote 2 (1 destroyed and recreated at slot 2) 8. slot 2 -> remote 2 (1 destroyed and recreated at slot 3) 9. slot 3 -> remote 2 (1 destroyed and not recreated) 10. slot 4 -> remote 2 (2 was already in this slot so no changes) 11. slot 0 -> remote 1 (The current code sees remote 2 was paired over in one of the dongle slots it occupied and attempts to remove all information about remote 2 [1]. It calls wacom_remote_destroy_one for remote 2, but the destroy function assumes the lowest index is where the remote's input device was created. The code "cleans up" the other remote 2 pairings including the one which the input device was based on, assuming they were were just duplicate pairings. However, the cleanup doesn't call the devres release function for the input device that was created in slot 4). This issue is fixed by this commit. [1] Remote 2 should subsequently be re-created on the next packet from the EKR at the lowest numbered slot that it occupies (here slot 1). Fixes: f9036bd43602 ("HID: wacom: EKR: use devres groups to manage resources") Cc: stable #4.9 Signed-off-by: Aaron Armstrong Skomra Signed-off-by: Jiri Kosina --- drivers/hid/wacom_sys.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c index c9eadf632564..409543160af7 100644 --- a/drivers/hid/wacom_sys.c +++ b/drivers/hid/wacom_sys.c @@ -2451,23 +2451,23 @@ static void wacom_remote_destroy_one(struct wacom *wacom, unsigned int index) int i; unsigned long flags; - spin_lock_irqsave(&remote->remote_lock, flags); - remote->remotes[index].registered = false; - spin_unlock_irqrestore(&remote->remote_lock, flags); - - if (remote->remotes[index].battery.battery) - devres_release_group(&wacom->hdev->dev, - &remote->remotes[index].battery.bat_desc); - - if (remote->remotes[index].group.name) - devres_release_group(&wacom->hdev->dev, - &remote->remotes[index]); - for (i = 0; i < WACOM_MAX_REMOTES; i++) { if (remote->remotes[i].serial == serial) { + + spin_lock_irqsave(&remote->remote_lock, flags); + remote->remotes[i].registered = false; + spin_unlock_irqrestore(&remote->remote_lock, flags); + + if (remote->remotes[i].battery.battery) + devres_release_group(&wacom->hdev->dev, + &remote->remotes[i].battery.bat_desc); + + if (remote->remotes[i].group.name) + devres_release_group(&wacom->hdev->dev, + &remote->remotes[i]); + remote->remotes[i].serial = 0; remote->remotes[i].group.name = NULL; - remote->remotes[i].registered = false; remote->remotes[i].battery.battery = NULL; wacom->led.groups[i].select = WACOM_STATUS_UNKNOWN; } From 403c0f681c1964ff1db8c2fb8de8c4067779d081 Mon Sep 17 00:00:00 2001 From: Jason Gerecke Date: Tue, 26 Dec 2017 14:53:55 -0800 Subject: [PATCH 4/5] HID: wacom: Fix reporting of touch toggle (WACOM_HID_WD_MUTE_DEVICE) events Touch toggle softkeys send a '1' while pressed and a '0' while released, requring the kernel to keep track of wether touch should be enabled or disabled. The code does not handle the state transitions properly, however. If the key is pressed repeatedly, the following four states of states are cycled through (assuming touch starts out enabled): Press: shared->is_touch_on => 0, SW_MUTE_DEVICE => 1 Release: shared->is_touch_on => 0, SW_MUTE_DEVICE => 1 Press: shared->is_touch_on => 1, SW_MUTE_DEVICE => 0 Release: shared->is_touch_on => 1, SW_MUTE_DEVICE => 1 The hardware always properly enables/disables touch when the key is pressed but applications that listen for SW_MUTE_DEVICE events to provide feedback about the state will only ever show touch as being enabled while the key is held, and only every-other time. This sequence occurs because the fallthrough WACOM_HID_WD_TOUCHONOFF case is always handled, and it uses the value of the *local* is_touch_on variable as the value to report to userspace. The local value is equal to the shared value when the button is pressed, but equal to zero when the button is released. Reporting the shared value to userspace fixes this problem, but the fallthrough case needs to update the shared value in an incompatible way (which is why the local variable was introduced in the first place). To work around this, we just handle both cases in a single block of code and update the shared variable as appropriate. Fixes: d793ff8187 ("HID: wacom: generic: support touch on/off softkey") Cc: # v4.12+ Signed-off-by: Jason Gerecke Reviewed-by: Aaron Skomra Tested-by: Aaron Skomra Signed-off-by: Jiri Kosina --- drivers/hid/wacom_wac.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c index 7fa373225d8a..061e4d7a0647 100644 --- a/drivers/hid/wacom_wac.c +++ b/drivers/hid/wacom_wac.c @@ -1924,7 +1924,6 @@ static void wacom_wac_pad_event(struct hid_device *hdev, struct hid_field *field struct wacom_features *features = &wacom_wac->features; unsigned equivalent_usage = wacom_equivalent_usage(usage->hid); int i; - bool is_touch_on = value; bool do_report = false; /* @@ -1969,16 +1968,17 @@ static void wacom_wac_pad_event(struct hid_device *hdev, struct hid_field *field break; case WACOM_HID_WD_MUTE_DEVICE: - if (wacom_wac->shared->touch_input && value) { - wacom_wac->shared->is_touch_on = !wacom_wac->shared->is_touch_on; - is_touch_on = wacom_wac->shared->is_touch_on; - } - - /* fall through*/ case WACOM_HID_WD_TOUCHONOFF: if (wacom_wac->shared->touch_input) { + bool *is_touch_on = &wacom_wac->shared->is_touch_on; + + if (equivalent_usage == WACOM_HID_WD_MUTE_DEVICE && value) + *is_touch_on = !(*is_touch_on); + else if (equivalent_usage == WACOM_HID_WD_TOUCHONOFF) + *is_touch_on = value; + input_report_switch(wacom_wac->shared->touch_input, - SW_MUTE_DEVICE, !is_touch_on); + SW_MUTE_DEVICE, !(*is_touch_on)); input_sync(wacom_wac->shared->touch_input); } break; From c947218951da68e3fd18a25e9af19556308caf45 Mon Sep 17 00:00:00 2001 From: Jason Gerecke Date: Tue, 26 Dec 2017 15:50:18 -0800 Subject: [PATCH 5/5] HID: wacom: Add support for One by Wacom (CTL-472 / CTL-672) Adds support for the second-generation "One by Wacom" tablets. These devices are similar to the last generation, but a slightly different size and reporting a higher number of pressure levels. Signed-off-by: Mx Jing Signed-off-by: Jason Gerecke Signed-off-by: Jiri Kosina --- drivers/hid/wacom_wac.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c index 061e4d7a0647..90c38a0523e9 100644 --- a/drivers/hid/wacom_wac.c +++ b/drivers/hid/wacom_wac.c @@ -4415,6 +4415,12 @@ static const struct wacom_features wacom_features_0x360 = static const struct wacom_features wacom_features_0x361 = { "Wacom Intuos Pro L", 62200, 43200, 8191, 63, INTUOSP2_BT, WACOM_INTUOS3_RES, WACOM_INTUOS3_RES, 9, .touch_max = 10 }; +static const struct wacom_features wacom_features_0x37A = + { "Wacom One by Wacom S", 15200, 9500, 2047, 63, + BAMBOO_PEN, WACOM_INTUOS_RES, WACOM_INTUOS_RES }; +static const struct wacom_features wacom_features_0x37B = + { "Wacom One by Wacom M", 21600, 13500, 2047, 63, + BAMBOO_PEN, WACOM_INTUOS_RES, WACOM_INTUOS_RES }; static const struct wacom_features wacom_features_HID_ANY_ID = { "Wacom HID", .type = HID_GENERIC, .oVid = HID_ANY_ID, .oPid = HID_ANY_ID }; @@ -4583,6 +4589,8 @@ const struct hid_device_id wacom_ids[] = { { USB_DEVICE_WACOM(0x343) }, { BT_DEVICE_WACOM(0x360) }, { BT_DEVICE_WACOM(0x361) }, + { USB_DEVICE_WACOM(0x37A) }, + { USB_DEVICE_WACOM(0x37B) }, { USB_DEVICE_WACOM(0x4001) }, { USB_DEVICE_WACOM(0x4004) }, { USB_DEVICE_WACOM(0x5000) },