From e82e62e390d39c3819641cd721695702180d54fb Mon Sep 17 00:00:00 2001 From: Sebastian Parschauer Date: Wed, 7 Nov 2018 13:36:40 +0100 Subject: [PATCH 01/15] HID: Add quirk for Microsoft PIXART OEM mouse The PixArt OEM mice are known for disconnecting every minute in runlevel 1 or 3 if they are not always polled. So add quirk ALWAYS_POLL for this one as well. References: https://www.spinics.net/lists/linux-usb/msg88965.html http://linet.gr.jp/~kojima/PlamoWeb/ML/htdocs/201808/msg00019.html Signed-off-by: Sebastian Parschauer CC: stable@vger.kernel.org Signed-off-by: Jiri Kosina --- drivers/hid/hid-ids.h | 1 + drivers/hid/hid-quirks.c | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index c0d668944dbe..02ce75f5ea4e 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -805,6 +805,7 @@ #define USB_DEVICE_ID_MS_TYPE_COVER_2 0x07a9 #define USB_DEVICE_ID_MS_POWER_COVER 0x07da #define USB_DEVICE_ID_MS_XBOX_ONE_S_CONTROLLER 0x02fd +#define USB_DEVICE_ID_MS_PIXART_MOUSE 0x00cb #define USB_VENDOR_ID_MOJO 0x8282 #define USB_DEVICE_ID_RETRO_ADAPTER 0x3201 diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c index 8237dd86fb17..d82e5ab48837 100644 --- a/drivers/hid/hid-quirks.c +++ b/drivers/hid/hid-quirks.c @@ -107,6 +107,7 @@ static const struct hid_device_id hid_quirks[] = { { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_MOUSE_C05A), HID_QUIRK_ALWAYS_POLL }, { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_MOUSE_C06A), HID_QUIRK_ALWAYS_POLL }, { HID_USB_DEVICE(USB_VENDOR_ID_MCS, USB_DEVICE_ID_MCS_GAMEPADBLOCK), HID_QUIRK_MULTI_INPUT }, + { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_PIXART_MOUSE), HID_QUIRK_ALWAYS_POLL }, { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_POWER_COVER), HID_QUIRK_NO_INIT_REPORTS }, { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_SURFACE_PRO_2), HID_QUIRK_NO_INIT_REPORTS }, { HID_USB_DEVICE(USB_VENDOR_ID_MICROSOFT, USB_DEVICE_ID_MS_TOUCH_COVER_2), HID_QUIRK_NO_INIT_REPORTS }, From 0fd791841a6d67af1155a9c3de54dea51220721e Mon Sep 17 00:00:00 2001 From: Benson Leung Date: Thu, 8 Nov 2018 15:59:21 -0800 Subject: [PATCH 02/15] HID: input: Ignore battery reported by Symbol DS4308 The Motorola/Zebra Symbol DS4308-HD is a handheld USB barcode scanner which does not have a battery, but reports one anyway that always has capacity 2. Let's apply the IGNORE quirk to prevent it from being treated like a power supply so that userspaces don't get confused that this accessory is almost out of power and warn the user that they need to charge their wired barcode scanner. Reported here: https://bugs.chromium.org/p/chromium/issues/detail?id=804720 Signed-off-by: Benson Leung Reviewed-by: Benjamin Tissoires Signed-off-by: Benjamin Tissoires --- drivers/hid/hid-ids.h | 1 + drivers/hid/hid-input.c | 3 +++ 2 files changed, 4 insertions(+) diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 02ce75f5ea4e..3004322eb8e4 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -1044,6 +1044,7 @@ #define USB_VENDOR_ID_SYMBOL 0x05e0 #define USB_DEVICE_ID_SYMBOL_SCANNER_1 0x0800 #define USB_DEVICE_ID_SYMBOL_SCANNER_2 0x1300 +#define USB_DEVICE_ID_SYMBOL_SCANNER_3 0x1200 #define USB_VENDOR_ID_SYNAPTICS 0x06cb #define USB_DEVICE_ID_SYNAPTICS_TP 0x0001 diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c index a2f74e6adc70..44ea8e7c71a9 100644 --- a/drivers/hid/hid-input.c +++ b/drivers/hid/hid-input.c @@ -325,6 +325,9 @@ static const struct hid_device_id hid_battery_quirks[] = { { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_ELECOM, USB_DEVICE_ID_ELECOM_BM084), HID_BATTERY_QUIRK_IGNORE }, + { HID_USB_DEVICE(USB_VENDOR_ID_SYMBOL, + USB_DEVICE_ID_SYMBOL_SCANNER_3), + HID_BATTERY_QUIRK_IGNORE }, {} }; From 8c01db7619f07c85c5cd81ec5eb83608b56c88f5 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Wed, 14 Nov 2018 13:55:09 -0800 Subject: [PATCH 03/15] HID: uhid: forbid UHID_CREATE under KERNEL_DS or elevated privileges When a UHID_CREATE command is written to the uhid char device, a copy_from_user() is done from a user pointer embedded in the command. When the address limit is KERNEL_DS, e.g. as is the case during sys_sendfile(), this can read from kernel memory. Alternatively, information can be leaked from a setuid binary that is tricked to write to the file descriptor. Therefore, forbid UHID_CREATE in these cases. No other commands in uhid_char_write() are affected by this bug and UHID_CREATE is marked as "obsolete", so apply the restriction to UHID_CREATE only rather than to uhid_char_write() entirely. Thanks to Dmitry Vyukov for adding uhid definitions to syzkaller and to Jann Horn for commit 9da3f2b740544 ("x86/fault: BUG() when uaccess helpers fault on kernel addresses"), allowing this bug to be found. Reported-by: syzbot+72473edc9bf4eb1c6556@syzkaller.appspotmail.com Fixes: d365c6cfd337 ("HID: uhid: add UHID_CREATE and UHID_DESTROY events") Cc: # v3.6+ Cc: Jann Horn Cc: Andy Lutomirski Signed-off-by: Eric Biggers Reviewed-by: Jann Horn Signed-off-by: Jiri Kosina --- drivers/hid/uhid.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c index 3c5507313606..051639c09f72 100644 --- a/drivers/hid/uhid.c +++ b/drivers/hid/uhid.c @@ -12,6 +12,7 @@ #include #include +#include #include #include #include @@ -722,6 +723,17 @@ static ssize_t uhid_char_write(struct file *file, const char __user *buffer, switch (uhid->input_buf.type) { case UHID_CREATE: + /* + * 'struct uhid_create_req' contains a __user pointer which is + * copied from, so it's unsafe to allow this with elevated + * privileges (e.g. from a setuid binary) or via kernel_write(). + */ + if (file->f_cred != current_cred() || uaccess_kernel()) { + pr_err_once("UHID_CREATE from different security context by process %d (%s), this is not allowed.\n", + task_tgid_vnr(current), current->comm); + ret = -EACCES; + goto unlock; + } ret = uhid_dev_create(uhid, &uhid->input_buf); break; case UHID_CREATE2: From 4d26d1d1e8065bb3326a7c06d5d4698e581443a9 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Wed, 14 Nov 2018 14:16:42 +0100 Subject: [PATCH 04/15] Revert "HID: uhid: use strlcpy() instead of strncpy()" This reverts commit 336fd4f5f25157e9e8bd50e898a1bbcd99eaea46. Please note that `strlcpy()` does *NOT* do what you think it does. strlcpy() *ALWAYS* reads the full input string, regardless of the 'length' parameter. That is, if the input is not zero-terminated, strlcpy() will *READ* beyond input boundaries. It does this, because it always returns the size it *would* copy if the target was big enough, not the truncated size it actually copied. The original code was perfectly fine. The hid device is zero-initialized and the strncpy() functions copied up to n-1 characters. The result is always zero-terminated this way. This is the third time someone tried to replace strncpy with strlcpy in this function, and gets it wrong. I now added a comment that should at least make people reconsider. Signed-off-by: David Herrmann Signed-off-by: Jiri Kosina --- drivers/hid/uhid.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c index 051639c09f72..840634e0f1e3 100644 --- a/drivers/hid/uhid.c +++ b/drivers/hid/uhid.c @@ -497,12 +497,13 @@ static int uhid_dev_create2(struct uhid_device *uhid, goto err_free; } - len = min(sizeof(hid->name), sizeof(ev->u.create2.name)); - strlcpy(hid->name, ev->u.create2.name, len); - len = min(sizeof(hid->phys), sizeof(ev->u.create2.phys)); - strlcpy(hid->phys, ev->u.create2.phys, len); - len = min(sizeof(hid->uniq), sizeof(ev->u.create2.uniq)); - strlcpy(hid->uniq, ev->u.create2.uniq, len); + /* @hid is zero-initialized, strncpy() is correct, strlcpy() not */ + len = min(sizeof(hid->name), sizeof(ev->u.create2.name)) - 1; + strncpy(hid->name, ev->u.create2.name, len); + len = min(sizeof(hid->phys), sizeof(ev->u.create2.phys)) - 1; + strncpy(hid->phys, ev->u.create2.phys, len); + len = min(sizeof(hid->uniq), sizeof(ev->u.create2.uniq)) - 1; + strncpy(hid->uniq, ev->u.create2.uniq, len); hid->ll_driver = &uhid_hid_driver; hid->bus = ev->u.create2.bus; From 385a4886778f6d6e61eff1d4d295af332d7130e1 Mon Sep 17 00:00:00 2001 From: Rodrigo Rivas Costa Date: Sun, 14 Oct 2018 19:36:43 +0200 Subject: [PATCH 05/15] HID: steam: remove input device when a hid client is running. Previously, when a HID client such as the Steam Client was running, this driver disabled its input device to avoid doubling the input events. While it worked mostly fine, some games got confused by the idle gamepad, and switched to two player mode, or asked the user to choose which gamepad to use. Other games just crashed, probably a bug in Unity [1]. With this commit, when a HID client starts, the input device is removed; when the HID client ends the input device is recreated. [1]: https://github.com/ValveSoftware/steam-for-linux/issues/5645 Signed-off-by: Rodrigo Rivas Costa Signed-off-by: Jiri Kosina --- drivers/hid/hid-steam.c | 154 +++++++++++++++++++++++----------------- 1 file changed, 90 insertions(+), 64 deletions(-) diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c index 0422ec2b13d2..dc4128bfe2ca 100644 --- a/drivers/hid/hid-steam.c +++ b/drivers/hid/hid-steam.c @@ -23,8 +23,9 @@ * In order to avoid breaking them this driver creates a layered hidraw device, * so it can detect when the client is running and then: * - it will not send any command to the controller. - * - this input device will be disabled, to avoid double input of the same + * - this input device will be removed, to avoid double input of the same * user action. + * When the client is closed, this input device will be created again. * * For additional functions, such as changing the right-pad margin or switching * the led, you can use the user-space tool at: @@ -113,7 +114,7 @@ struct steam_device { spinlock_t lock; struct hid_device *hdev, *client_hdev; struct mutex mutex; - bool client_opened, input_opened; + bool client_opened; struct input_dev __rcu *input; unsigned long quirks; struct work_struct work_connect; @@ -279,18 +280,6 @@ static void steam_set_lizard_mode(struct steam_device *steam, bool enable) } } -static void steam_update_lizard_mode(struct steam_device *steam) -{ - mutex_lock(&steam->mutex); - if (!steam->client_opened) { - if (steam->input_opened) - steam_set_lizard_mode(steam, false); - else - steam_set_lizard_mode(steam, lizard_mode); - } - mutex_unlock(&steam->mutex); -} - static int steam_input_open(struct input_dev *dev) { struct steam_device *steam = input_get_drvdata(dev); @@ -301,7 +290,6 @@ static int steam_input_open(struct input_dev *dev) return ret; mutex_lock(&steam->mutex); - steam->input_opened = true; if (!steam->client_opened && lizard_mode) steam_set_lizard_mode(steam, false); mutex_unlock(&steam->mutex); @@ -313,7 +301,6 @@ static void steam_input_close(struct input_dev *dev) struct steam_device *steam = input_get_drvdata(dev); mutex_lock(&steam->mutex); - steam->input_opened = false; if (!steam->client_opened && lizard_mode) steam_set_lizard_mode(steam, true); mutex_unlock(&steam->mutex); @@ -400,7 +387,7 @@ static int steam_battery_register(struct steam_device *steam) return 0; } -static int steam_register(struct steam_device *steam) +static int steam_input_register(struct steam_device *steam) { struct hid_device *hdev = steam->hdev; struct input_dev *input; @@ -414,17 +401,6 @@ static int steam_register(struct steam_device *steam) return 0; } - /* - * Unlikely, but getting the serial could fail, and it is not so - * important, so make up a serial number and go on. - */ - if (steam_get_serial(steam) < 0) - strlcpy(steam->serial_no, "XXXXXXXXXX", - sizeof(steam->serial_no)); - - hid_info(hdev, "Steam Controller '%s' connected", - steam->serial_no); - input = input_allocate_device(); if (!input) return -ENOMEM; @@ -492,11 +468,6 @@ static int steam_register(struct steam_device *steam) goto input_register_fail; rcu_assign_pointer(steam->input, input); - - /* ignore battery errors, we can live without it */ - if (steam->quirks & STEAM_QUIRK_WIRELESS) - steam_battery_register(steam); - return 0; input_register_fail: @@ -504,27 +475,88 @@ input_register_fail: return ret; } -static void steam_unregister(struct steam_device *steam) +static void steam_input_unregister(struct steam_device *steam) { struct input_dev *input; + rcu_read_lock(); + input = rcu_dereference(steam->input); + rcu_read_unlock(); + if (!input) + return; + RCU_INIT_POINTER(steam->input, NULL); + synchronize_rcu(); + input_unregister_device(input); +} + +static void steam_battery_unregister(struct steam_device *steam) +{ struct power_supply *battery; rcu_read_lock(); - input = rcu_dereference(steam->input); battery = rcu_dereference(steam->battery); rcu_read_unlock(); - if (battery) { - RCU_INIT_POINTER(steam->battery, NULL); - synchronize_rcu(); - power_supply_unregister(battery); + if (!battery) + return; + RCU_INIT_POINTER(steam->battery, NULL); + synchronize_rcu(); + power_supply_unregister(battery); +} + +static int steam_register(struct steam_device *steam) +{ + int ret; + + /* + * This function can be called several times in a row with the + * wireless adaptor, without steam_unregister() between them, because + * another client send a get_connection_status command, for example. + * The battery and serial number are set just once per device. + */ + if (!steam->serial_no[0]) { + /* + * Unlikely, but getting the serial could fail, and it is not so + * important, so make up a serial number and go on. + */ + if (steam_get_serial(steam) < 0) + strlcpy(steam->serial_no, "XXXXXXXXXX", + sizeof(steam->serial_no)); + + hid_info(steam->hdev, "Steam Controller '%s' connected", + steam->serial_no); + + /* ignore battery errors, we can live without it */ + if (steam->quirks & STEAM_QUIRK_WIRELESS) + steam_battery_register(steam); + + mutex_lock(&steam_devices_lock); + list_add(&steam->list, &steam_devices); + mutex_unlock(&steam_devices_lock); } - if (input) { - RCU_INIT_POINTER(steam->input, NULL); - synchronize_rcu(); + + mutex_lock(&steam->mutex); + if (!steam->client_opened) { + steam_set_lizard_mode(steam, lizard_mode); + ret = steam_input_register(steam); + } else { + ret = 0; + } + mutex_unlock(&steam->mutex); + + return ret; +} + +static void steam_unregister(struct steam_device *steam) +{ + steam_battery_unregister(steam); + steam_input_unregister(steam); + if (steam->serial_no[0]) { hid_info(steam->hdev, "Steam Controller '%s' disconnected", steam->serial_no); - input_unregister_device(input); + mutex_lock(&steam_devices_lock); + list_del(&steam->list); + mutex_unlock(&steam_devices_lock); + steam->serial_no[0] = 0; } } @@ -600,6 +632,9 @@ static int steam_client_ll_open(struct hid_device *hdev) mutex_lock(&steam->mutex); steam->client_opened = true; mutex_unlock(&steam->mutex); + + steam_input_unregister(steam); + return ret; } @@ -609,13 +644,13 @@ static void steam_client_ll_close(struct hid_device *hdev) mutex_lock(&steam->mutex); steam->client_opened = false; - if (steam->input_opened) - steam_set_lizard_mode(steam, false); - else - steam_set_lizard_mode(steam, lizard_mode); mutex_unlock(&steam->mutex); hid_hw_close(steam->hdev); + if (steam->connected) { + steam_set_lizard_mode(steam, lizard_mode); + steam_input_register(steam); + } } static int steam_client_ll_raw_request(struct hid_device *hdev, @@ -744,11 +779,6 @@ static int steam_probe(struct hid_device *hdev, } } - mutex_lock(&steam_devices_lock); - steam_update_lizard_mode(steam); - list_add(&steam->list, &steam_devices); - mutex_unlock(&steam_devices_lock); - return 0; hid_hw_open_fail: @@ -774,10 +804,6 @@ static void steam_remove(struct hid_device *hdev) return; } - mutex_lock(&steam_devices_lock); - list_del(&steam->list); - mutex_unlock(&steam_devices_lock); - hid_destroy_device(steam->client_hdev); steam->client_opened = false; cancel_work_sync(&steam->work_connect); @@ -792,12 +818,14 @@ static void steam_remove(struct hid_device *hdev) static void steam_do_connect_event(struct steam_device *steam, bool connected) { unsigned long flags; + bool changed; spin_lock_irqsave(&steam->lock, flags); + changed = steam->connected != connected; steam->connected = connected; spin_unlock_irqrestore(&steam->lock, flags); - if (schedule_work(&steam->work_connect) == 0) + if (changed && schedule_work(&steam->work_connect) == 0) dbg_hid("%s: connected=%d event already queued\n", __func__, connected); } @@ -1019,13 +1047,8 @@ static int steam_raw_event(struct hid_device *hdev, return 0; rcu_read_lock(); input = rcu_dereference(steam->input); - if (likely(input)) { + if (likely(input)) steam_do_input_event(steam, input, data); - } else { - dbg_hid("%s: input data without connect event\n", - __func__); - steam_do_connect_event(steam, true); - } rcu_read_unlock(); break; case STEAM_EV_CONNECT: @@ -1074,7 +1097,10 @@ static int steam_param_set_lizard_mode(const char *val, mutex_lock(&steam_devices_lock); list_for_each_entry(steam, &steam_devices, list) { - steam_update_lizard_mode(steam); + mutex_lock(&steam->mutex); + if (!steam->client_opened) + steam_set_lizard_mode(steam, lizard_mode); + mutex_unlock(&steam->mutex); } mutex_unlock(&steam_devices_lock); return 0; From 12d43aacf9a74d0eb66fd0ea54ebeb79ca28940f Mon Sep 17 00:00:00 2001 From: Kai-Heng Feng Date: Wed, 14 Nov 2018 05:35:20 +0000 Subject: [PATCH 06/15] HID: multitouch: Add pointstick support for Cirque Touchpad Cirque Touchpad/Pointstick combo is similar to Alps devices, it requires MT_CLS_WIN_8_DUAL to expose its pointstick as a mouse. Signed-off-by: Kai-Heng Feng Signed-off-by: Jiri Kosina --- drivers/hid/hid-ids.h | 3 +++ drivers/hid/hid-multitouch.c | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 3004322eb8e4..3917fc4cf96a 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -275,6 +275,9 @@ #define USB_VENDOR_ID_CIDC 0x1677 +#define I2C_VENDOR_ID_CIRQUE 0x0488 +#define I2C_PRODUCT_ID_CIRQUE_121F 0x121F + #define USB_VENDOR_ID_CJTOUCH 0x24b8 #define USB_DEVICE_ID_CJTOUCH_MULTI_TOUCH_0020 0x0020 #define USB_DEVICE_ID_CJTOUCH_MULTI_TOUCH_0040 0x0040 diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index f7c6de2b6730..dca0a3a90fb8 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c @@ -1814,6 +1814,12 @@ static const struct hid_device_id mt_devices[] = { MT_USB_DEVICE(USB_VENDOR_ID_CHUNGHWAT, USB_DEVICE_ID_CHUNGHWAT_MULTITOUCH) }, + /* Cirque devices */ + { .driver_data = MT_CLS_WIN_8_DUAL, + HID_DEVICE(BUS_I2C, HID_GROUP_MULTITOUCH_WIN_8, + I2C_VENDOR_ID_CIRQUE, + I2C_PRODUCT_ID_CIRQUE_121F) }, + /* CJTouch panels */ { .driver_data = MT_CLS_NSMU, MT_USB_DEVICE(USB_VENDOR_ID_CJTOUCH, From 86c31524b27c7e686841dd4a79eda95cfd989f16 Mon Sep 17 00:00:00 2001 From: Kai-Heng Feng Date: Wed, 14 Nov 2018 07:24:57 +0000 Subject: [PATCH 07/15] HID: i2c-hid: Disable runtime PM for LG touchscreen LG touchscreen (1fd2:8001) stops working after reboot: [ 4.859153] i2c_hid i2c-SAPS2101:00: i2c_hid_get_input: incomplete report (64/66) [ 4.936070] i2c_hid i2c-SAPS2101:00: i2c_hid_get_input: incomplete report (64/66) [ 9.948224] i2c_hid i2c-SAPS2101:00: failed to reset device. The device in question stops working after receives SLEEP, ON, SLEEP commands in a short period. The scenario is like this: - Once the desktop session closes, it also closed the hid device, so the device gets runtime suspended and receives a SLEEP command. - Before calling shutdown callback, it gets runtime resumed and received an ON command. - In the shutdown callback, it receives another SLEEP command. I failed to find a reliable interval between ON/SLEEP commands that can make it work, so let's simply disable runtime PM for the device. Signed-off-by: Kai-Heng Feng Signed-off-by: Jiri Kosina --- drivers/hid/hid-ids.h | 1 + drivers/hid/i2c-hid/i2c-hid-core.c | 2 ++ 2 files changed, 3 insertions(+) diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 3917fc4cf96a..2114ac647bef 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -710,6 +710,7 @@ #define USB_VENDOR_ID_LG 0x1fd2 #define USB_DEVICE_ID_LG_MULTITOUCH 0x0064 #define USB_DEVICE_ID_LG_MELFAS_MT 0x6007 +#define I2C_DEVICE_ID_LG_8001 0x8001 #define USB_VENDOR_ID_LOGITECH 0x046d #define USB_DEVICE_ID_LOGITECH_AUDIOHUB 0x0a0e diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c index 3cde7c1b9c33..8555ce7e737b 100644 --- a/drivers/hid/i2c-hid/i2c-hid-core.c +++ b/drivers/hid/i2c-hid/i2c-hid-core.c @@ -177,6 +177,8 @@ static const struct i2c_hid_quirks { I2C_HID_QUIRK_NO_RUNTIME_PM }, { I2C_VENDOR_ID_RAYDIUM, I2C_PRODUCT_ID_RAYDIUM_4B33, I2C_HID_QUIRK_DELAY_AFTER_SLEEP }, + { USB_VENDOR_ID_LG, I2C_DEVICE_ID_LG_8001, + I2C_HID_QUIRK_NO_RUNTIME_PM }, { 0, 0 } }; From fb862c3b199d28bee238d52e8270eae8650d6cb0 Mon Sep 17 00:00:00 2001 From: Sebastian Parschauer Date: Tue, 20 Nov 2018 07:25:08 +0100 Subject: [PATCH 08/15] HID: Add quirk for Primax PIXART OEM mice The PixArt OEM mice are known for disconnecting every minute in runlevel 1 or 3 if they are not always polled. So add quirk ALWAYS_POLL for two Primax mice as well. 0x4e22 is the Dell MS111-P and 0x4d0f is the unbranded HP Portia mouse HP 697738-001. Both were built until approx. 2014. Those were the standard mice from those vendors and are still around - even as new old stock. Reference: https://github.com/sriemer/fix-linux-mouse/issues/11 Signed-off-by: Sebastian Parschauer CC: stable@vger.kernel.org Signed-off-by: Jiri Kosina --- drivers/hid/hid-ids.h | 2 ++ drivers/hid/hid-quirks.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 2114ac647bef..ed35c9a9a110 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -1210,6 +1210,8 @@ #define USB_DEVICE_ID_PRIMAX_MOUSE_4D22 0x4d22 #define USB_DEVICE_ID_PRIMAX_KEYBOARD 0x4e05 #define USB_DEVICE_ID_PRIMAX_REZEL 0x4e72 +#define USB_DEVICE_ID_PRIMAX_PIXART_MOUSE_4D0F 0x4d0f +#define USB_DEVICE_ID_PRIMAX_PIXART_MOUSE_4E22 0x4e22 #define USB_VENDOR_ID_RISO_KAGAKU 0x1294 /* Riso Kagaku Corp. */ diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c index d82e5ab48837..c85a79986b6a 100644 --- a/drivers/hid/hid-quirks.c +++ b/drivers/hid/hid-quirks.c @@ -130,6 +130,8 @@ static const struct hid_device_id hid_quirks[] = { { HID_USB_DEVICE(USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN), HID_QUIRK_NO_INIT_REPORTS }, { HID_USB_DEVICE(USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_USB_OPTICAL_MOUSE), HID_QUIRK_ALWAYS_POLL }, { HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_MOUSE_4D22), HID_QUIRK_ALWAYS_POLL }, + { HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_PIXART_MOUSE_4D0F), HID_QUIRK_ALWAYS_POLL }, + { HID_USB_DEVICE(USB_VENDOR_ID_PRIMAX, USB_DEVICE_ID_PRIMAX_PIXART_MOUSE_4E22), HID_QUIRK_ALWAYS_POLL }, { HID_USB_DEVICE(USB_VENDOR_ID_PRODIGE, USB_DEVICE_ID_PRODIGE_CORDLESS), HID_QUIRK_NOGET }, { HID_USB_DEVICE(USB_VENDOR_ID_QUANTA, USB_DEVICE_ID_QUANTA_OPTICAL_TOUCH_3001), HID_QUIRK_NOGET }, { HID_USB_DEVICE(USB_VENDOR_ID_QUANTA, USB_DEVICE_ID_QUANTA_OPTICAL_TOUCH_3003), HID_QUIRK_NOGET }, From e2b95b27196cc90f37016d79432fb2af3f9c9291 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 21 Nov 2018 16:27:06 +0100 Subject: [PATCH 09/15] Revert "HID: input: simplify/fix high-res scroll event handling" This reverts commit 044ee890286153a1aefb40cb8b6659921aecb38b. It turns out the current API is not that compatible with some Microsoft mice, so better start again from scratch. Signed-off-by: Benjamin Tissoires Acked-by: Harry Cutts Acked-by: Dmitry Torokhov Acked-by: Jiri Kosina --- drivers/hid/hid-input.c | 43 +++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c index 44ea8e7c71a9..28ee2ed88a1a 100644 --- a/drivers/hid/hid-input.c +++ b/drivers/hid/hid-input.c @@ -1858,30 +1858,31 @@ EXPORT_SYMBOL_GPL(hidinput_disconnect); void hid_scroll_counter_handle_scroll(struct hid_scroll_counter *counter, int hi_res_value) { - int low_res_value, remainder, multiplier; + int low_res_scroll_amount; + /* Some wheels will rest 7/8ths of a notch from the previous notch + * after slow movement, so we want the threshold for low-res events to + * be in the middle of the notches (e.g. after 4/8ths) as opposed to on + * the notches themselves (8/8ths). + */ + int threshold = counter->resolution_multiplier / 2; input_report_rel(counter->dev, REL_WHEEL_HI_RES, hi_res_value * counter->microns_per_hi_res_unit); - /* - * Update the low-res remainder with the high-res value, - * but reset if the direction has changed. - */ - remainder = counter->remainder; - if ((remainder ^ hi_res_value) < 0) - remainder = 0; - remainder += hi_res_value; - - /* - * Then just use the resolution multiplier to see if - * we should send a low-res (aka regular wheel) event. - */ - multiplier = counter->resolution_multiplier; - low_res_value = remainder / multiplier; - remainder -= low_res_value * multiplier; - counter->remainder = remainder; - - if (low_res_value) - input_report_rel(counter->dev, REL_WHEEL, low_res_value); + counter->remainder += hi_res_value; + if (abs(counter->remainder) >= threshold) { + /* Add (or subtract) 1 because we want to trigger when the wheel + * is half-way to the next notch (i.e. scroll 1 notch after a + * 1/2 notch movement, 2 notches after a 1 1/2 notch movement, + * etc.). + */ + low_res_scroll_amount = + counter->remainder / counter->resolution_multiplier + + (hi_res_value > 0 ? 1 : -1); + input_report_rel(counter->dev, REL_WHEEL, + low_res_scroll_amount); + counter->remainder -= + low_res_scroll_amount * counter->resolution_multiplier; + } } EXPORT_SYMBOL_GPL(hid_scroll_counter_handle_scroll); From d0341efe7449860e1bbfdd282b3b7f0c34309222 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 21 Nov 2018 16:27:07 +0100 Subject: [PATCH 10/15] Revert "HID: logitech: fix a used uninitialized GCC warning" This reverts commit 5fe2ccbef9d7aecf5c4402c753444f1a12096cfd. It turns out the current API is not that compatible with some Microsoft mice, so better start again from scratch. Signed-off-by: Benjamin Tissoires Acked-by: Harry Cutts Acked-by: Dmitry Torokhov Acked-by: Jiri Kosina --- drivers/hid/hid-logitech-hidpp.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c index f01280898b24..5f0c080059c6 100644 --- a/drivers/hid/hid-logitech-hidpp.c +++ b/drivers/hid/hid-logitech-hidpp.c @@ -1231,6 +1231,7 @@ static int hidpp_hrw_get_wheel_capability(struct hidpp_device *hidpp, *multiplier = response.fap.params[0]; return 0; return_default: + *multiplier = 8; hid_warn(hidpp->hid_dev, "Couldn't get wheel multiplier (error %d), assuming %d.\n", ret, *multiplier); @@ -2695,7 +2696,7 @@ static int hi_res_scroll_look_up_microns(__u32 product_id) static int hi_res_scroll_enable(struct hidpp_device *hidpp) { int ret; - u8 multiplier = 8; + u8 multiplier; if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL_X2121) { ret = hidpp_hrw_set_wheel_mode(hidpp, false, true, false); @@ -2703,9 +2704,10 @@ static int hi_res_scroll_enable(struct hidpp_device *hidpp) } else if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL_X2120) { ret = hidpp_hrs_set_highres_scrolling_mode(hidpp, true, &multiplier); - } else /* if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL_1P0) */ + } else /* if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL_1P0) */ { ret = hidpp10_enable_scrolling_acceleration(hidpp); - + multiplier = 8; + } if (ret) return ret; From a69616d5b3e13f18cdec1c759818aec87866ac0c Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 21 Nov 2018 16:27:08 +0100 Subject: [PATCH 11/15] Revert "HID: logitech: Use LDJ_DEVICE macro for existing Logitech mice" This reverts commit 3fe1d6bbcd16f384d2c7dab2caf8e4b2df9ea7e6. It turns out the current API is not that compatible with some Microsoft mice, so better start again from scratch. Signed-off-by: Benjamin Tissoires Acked-by: Harry Cutts Acked-by: Dmitry Torokhov Acked-by: Jiri Kosina --- drivers/hid/hid-logitech-hidpp.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c index 5f0c080059c6..fd6a8c325fa0 100644 --- a/drivers/hid/hid-logitech-hidpp.c +++ b/drivers/hid/hid-logitech-hidpp.c @@ -3314,11 +3314,13 @@ static void hidpp_remove(struct hid_device *hdev) static const struct hid_device_id hidpp_devices[] = { { /* wireless touchpad */ - LDJ_DEVICE(0x4011), + HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE, + USB_VENDOR_ID_LOGITECH, 0x4011), .driver_data = HIDPP_QUIRK_CLASS_WTP | HIDPP_QUIRK_DELAYED_INIT | HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS }, { /* wireless touchpad T650 */ - LDJ_DEVICE(0x4101), + HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE, + USB_VENDOR_ID_LOGITECH, 0x4101), .driver_data = HIDPP_QUIRK_CLASS_WTP | HIDPP_QUIRK_DELAYED_INIT }, { /* wireless touchpad T651 */ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, @@ -3358,13 +3360,16 @@ static const struct hid_device_id hidpp_devices[] = { { /* Mouse Logitech Performance MX */ LDJ_DEVICE(0x101a), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_1P0 }, { /* Keyboard logitech K400 */ - LDJ_DEVICE(0x4024), + HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE, + USB_VENDOR_ID_LOGITECH, 0x4024), .driver_data = HIDPP_QUIRK_CLASS_K400 }, { /* Solar Keyboard Logitech K750 */ - LDJ_DEVICE(0x4002), + HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE, + USB_VENDOR_ID_LOGITECH, 0x4002), .driver_data = HIDPP_QUIRK_CLASS_K750 }, - { LDJ_DEVICE(HID_ANY_ID) }, + { HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE, + USB_VENDOR_ID_LOGITECH, HID_ANY_ID)}, { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G920_WHEEL), .driver_data = HIDPP_QUIRK_CLASS_G920 | HIDPP_QUIRK_FORCE_OUTPUT_REPORTS}, From 5372fc3797691065b8318f0cc9ab8695cebb17e1 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 21 Nov 2018 16:27:09 +0100 Subject: [PATCH 12/15] Revert "HID: logitech: Enable high-resolution scrolling on Logitech mice" This reverts commit d56ca9855bf924f3bc9807a3e42f38539df3f41f. It turns out the current API is not that compatible with some Microsoft mice, so better start again from scratch. Signed-off-by: Benjamin Tissoires Acked-by: Harry Cutts Acked-by: Dmitry Torokhov Acked-by: Jiri Kosina --- drivers/hid/hid-logitech-hidpp.c | 249 +------------------------------ 1 file changed, 4 insertions(+), 245 deletions(-) diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c index fd6a8c325fa0..7f8218f6ff56 100644 --- a/drivers/hid/hid-logitech-hidpp.c +++ b/drivers/hid/hid-logitech-hidpp.c @@ -64,14 +64,6 @@ MODULE_PARM_DESC(disable_tap_to_click, #define HIDPP_QUIRK_NO_HIDINPUT BIT(23) #define HIDPP_QUIRK_FORCE_OUTPUT_REPORTS BIT(24) #define HIDPP_QUIRK_UNIFYING BIT(25) -#define HIDPP_QUIRK_HI_RES_SCROLL_1P0 BIT(26) -#define HIDPP_QUIRK_HI_RES_SCROLL_X2120 BIT(27) -#define HIDPP_QUIRK_HI_RES_SCROLL_X2121 BIT(28) - -/* Convenience constant to check for any high-res support. */ -#define HIDPP_QUIRK_HI_RES_SCROLL (HIDPP_QUIRK_HI_RES_SCROLL_1P0 | \ - HIDPP_QUIRK_HI_RES_SCROLL_X2120 | \ - HIDPP_QUIRK_HI_RES_SCROLL_X2121) #define HIDPP_QUIRK_DELAYED_INIT HIDPP_QUIRK_NO_HIDINPUT @@ -157,7 +149,6 @@ struct hidpp_device { unsigned long capabilities; struct hidpp_battery battery; - struct hid_scroll_counter vertical_wheel_counter; }; /* HID++ 1.0 error codes */ @@ -1166,101 +1157,6 @@ static int hidpp_battery_get_property(struct power_supply *psy, return ret; } -/* -------------------------------------------------------------------------- */ -/* 0x2120: Hi-resolution scrolling */ -/* -------------------------------------------------------------------------- */ - -#define HIDPP_PAGE_HI_RESOLUTION_SCROLLING 0x2120 - -#define CMD_HI_RESOLUTION_SCROLLING_SET_HIGHRES_SCROLLING_MODE 0x10 - -static int hidpp_hrs_set_highres_scrolling_mode(struct hidpp_device *hidpp, - bool enabled, u8 *multiplier) -{ - u8 feature_index; - u8 feature_type; - int ret; - u8 params[1]; - struct hidpp_report response; - - ret = hidpp_root_get_feature(hidpp, - HIDPP_PAGE_HI_RESOLUTION_SCROLLING, - &feature_index, - &feature_type); - if (ret) - return ret; - - params[0] = enabled ? BIT(0) : 0; - ret = hidpp_send_fap_command_sync(hidpp, feature_index, - CMD_HI_RESOLUTION_SCROLLING_SET_HIGHRES_SCROLLING_MODE, - params, sizeof(params), &response); - if (ret) - return ret; - *multiplier = response.fap.params[1]; - return 0; -} - -/* -------------------------------------------------------------------------- */ -/* 0x2121: HiRes Wheel */ -/* -------------------------------------------------------------------------- */ - -#define HIDPP_PAGE_HIRES_WHEEL 0x2121 - -#define CMD_HIRES_WHEEL_GET_WHEEL_CAPABILITY 0x00 -#define CMD_HIRES_WHEEL_SET_WHEEL_MODE 0x20 - -static int hidpp_hrw_get_wheel_capability(struct hidpp_device *hidpp, - u8 *multiplier) -{ - u8 feature_index; - u8 feature_type; - int ret; - struct hidpp_report response; - - ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_HIRES_WHEEL, - &feature_index, &feature_type); - if (ret) - goto return_default; - - ret = hidpp_send_fap_command_sync(hidpp, feature_index, - CMD_HIRES_WHEEL_GET_WHEEL_CAPABILITY, - NULL, 0, &response); - if (ret) - goto return_default; - - *multiplier = response.fap.params[0]; - return 0; -return_default: - *multiplier = 8; - hid_warn(hidpp->hid_dev, - "Couldn't get wheel multiplier (error %d), assuming %d.\n", - ret, *multiplier); - return ret; -} - -static int hidpp_hrw_set_wheel_mode(struct hidpp_device *hidpp, bool invert, - bool high_resolution, bool use_hidpp) -{ - u8 feature_index; - u8 feature_type; - int ret; - u8 params[1]; - struct hidpp_report response; - - ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_HIRES_WHEEL, - &feature_index, &feature_type); - if (ret) - return ret; - - params[0] = (invert ? BIT(2) : 0) | - (high_resolution ? BIT(1) : 0) | - (use_hidpp ? BIT(0) : 0); - - return hidpp_send_fap_command_sync(hidpp, feature_index, - CMD_HIRES_WHEEL_SET_WHEEL_MODE, - params, sizeof(params), &response); -} - /* -------------------------------------------------------------------------- */ /* 0x4301: Solar Keyboard */ /* -------------------------------------------------------------------------- */ @@ -2524,8 +2420,7 @@ static int m560_raw_event(struct hid_device *hdev, u8 *data, int size) input_report_rel(mydata->input, REL_Y, v); v = hid_snto32(data[6], 8); - hid_scroll_counter_handle_scroll( - &hidpp->vertical_wheel_counter, v); + input_report_rel(mydata->input, REL_WHEEL, v); input_sync(mydata->input); } @@ -2653,73 +2548,6 @@ static int g920_get_config(struct hidpp_device *hidpp) return 0; } -/* -------------------------------------------------------------------------- */ -/* High-resolution scroll wheels */ -/* -------------------------------------------------------------------------- */ - -/** - * struct hi_res_scroll_info - Stores info on a device's high-res scroll wheel. - * @product_id: the HID product ID of the device being described. - * @microns_per_hi_res_unit: the distance moved by the user's finger for each - * high-resolution unit reported by the device, in - * 256ths of a millimetre. - */ -struct hi_res_scroll_info { - __u32 product_id; - int microns_per_hi_res_unit; -}; - -static struct hi_res_scroll_info hi_res_scroll_devices[] = { - { /* Anywhere MX */ - .product_id = 0x1017, .microns_per_hi_res_unit = 445 }, - { /* Performance MX */ - .product_id = 0x101a, .microns_per_hi_res_unit = 406 }, - { /* M560 */ - .product_id = 0x402d, .microns_per_hi_res_unit = 435 }, - { /* MX Master 2S */ - .product_id = 0x4069, .microns_per_hi_res_unit = 406 }, -}; - -static int hi_res_scroll_look_up_microns(__u32 product_id) -{ - int i; - int num_devices = sizeof(hi_res_scroll_devices) - / sizeof(hi_res_scroll_devices[0]); - for (i = 0; i < num_devices; i++) { - if (hi_res_scroll_devices[i].product_id == product_id) - return hi_res_scroll_devices[i].microns_per_hi_res_unit; - } - /* We don't have a value for this device, so use a sensible default. */ - return 406; -} - -static int hi_res_scroll_enable(struct hidpp_device *hidpp) -{ - int ret; - u8 multiplier; - - if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL_X2121) { - ret = hidpp_hrw_set_wheel_mode(hidpp, false, true, false); - hidpp_hrw_get_wheel_capability(hidpp, &multiplier); - } else if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL_X2120) { - ret = hidpp_hrs_set_highres_scrolling_mode(hidpp, true, - &multiplier); - } else /* if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL_1P0) */ { - ret = hidpp10_enable_scrolling_acceleration(hidpp); - multiplier = 8; - } - if (ret) - return ret; - - hidpp->vertical_wheel_counter.resolution_multiplier = multiplier; - hidpp->vertical_wheel_counter.microns_per_hi_res_unit = - hi_res_scroll_look_up_microns(hidpp->hid_dev->product); - hid_info(hidpp->hid_dev, "multiplier = %d, microns = %d\n", - multiplier, - hidpp->vertical_wheel_counter.microns_per_hi_res_unit); - return 0; -} - /* -------------------------------------------------------------------------- */ /* Generic HID++ devices */ /* -------------------------------------------------------------------------- */ @@ -2765,11 +2593,6 @@ static void hidpp_populate_input(struct hidpp_device *hidpp, wtp_populate_input(hidpp, input, origin_is_hid_core); else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560) m560_populate_input(hidpp, input, origin_is_hid_core); - - if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL) { - input_set_capability(input, EV_REL, REL_WHEEL_HI_RES); - hidpp->vertical_wheel_counter.dev = input; - } } static int hidpp_input_configured(struct hid_device *hdev, @@ -2888,27 +2711,6 @@ static int hidpp_raw_event(struct hid_device *hdev, struct hid_report *report, return 0; } -static int hidpp_event(struct hid_device *hdev, struct hid_field *field, - struct hid_usage *usage, __s32 value) -{ - /* This function will only be called for scroll events, due to the - * restriction imposed in hidpp_usages. - */ - struct hidpp_device *hidpp = hid_get_drvdata(hdev); - struct hid_scroll_counter *counter = &hidpp->vertical_wheel_counter; - /* A scroll event may occur before the multiplier has been retrieved or - * the input device set, or high-res scroll enabling may fail. In such - * cases we must return early (falling back to default behaviour) to - * avoid a crash in hid_scroll_counter_handle_scroll. - */ - if (!(hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL) || value == 0 - || counter->dev == NULL || counter->resolution_multiplier == 0) - return 0; - - hid_scroll_counter_handle_scroll(counter, value); - return 1; -} - static int hidpp_initialize_battery(struct hidpp_device *hidpp) { static atomic_t battery_no = ATOMIC_INIT(0); @@ -3120,9 +2922,6 @@ static void hidpp_connect_event(struct hidpp_device *hidpp) if (hidpp->battery.ps) power_supply_changed(hidpp->battery.ps); - if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL) - hi_res_scroll_enable(hidpp); - if (!(hidpp->quirks & HIDPP_QUIRK_NO_HIDINPUT) || hidpp->delayed_input) /* if the input nodes are already created, we can stop now */ return; @@ -3308,10 +3107,6 @@ static void hidpp_remove(struct hid_device *hdev) mutex_destroy(&hidpp->send_mutex); } -#define LDJ_DEVICE(product) \ - HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE, \ - USB_VENDOR_ID_LOGITECH, (product)) - static const struct hid_device_id hidpp_devices[] = { { /* wireless touchpad */ HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE, @@ -3326,39 +3121,10 @@ static const struct hid_device_id hidpp_devices[] = { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_T651), .driver_data = HIDPP_QUIRK_CLASS_WTP }, - { /* Mouse Logitech Anywhere MX */ - LDJ_DEVICE(0x1017), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_1P0 }, - { /* Mouse Logitech Cube */ - LDJ_DEVICE(0x4010), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2120 }, - { /* Mouse Logitech M335 */ - LDJ_DEVICE(0x4050), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 }, - { /* Mouse Logitech M515 */ - LDJ_DEVICE(0x4007), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2120 }, { /* Mouse logitech M560 */ - LDJ_DEVICE(0x402d), - .driver_data = HIDPP_QUIRK_DELAYED_INIT | HIDPP_QUIRK_CLASS_M560 - | HIDPP_QUIRK_HI_RES_SCROLL_X2120 }, - { /* Mouse Logitech M705 (firmware RQM17) */ - LDJ_DEVICE(0x101b), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_1P0 }, - { /* Mouse Logitech M705 (firmware RQM67) */ - LDJ_DEVICE(0x406d), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 }, - { /* Mouse Logitech M720 */ - LDJ_DEVICE(0x405e), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 }, - { /* Mouse Logitech MX Anywhere 2 */ - LDJ_DEVICE(0x404a), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 }, - { LDJ_DEVICE(0xb013), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 }, - { LDJ_DEVICE(0xb018), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 }, - { LDJ_DEVICE(0xb01f), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 }, - { /* Mouse Logitech MX Anywhere 2S */ - LDJ_DEVICE(0x406a), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 }, - { /* Mouse Logitech MX Master */ - LDJ_DEVICE(0x4041), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 }, - { LDJ_DEVICE(0x4060), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 }, - { LDJ_DEVICE(0x4071), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 }, - { /* Mouse Logitech MX Master 2S */ - LDJ_DEVICE(0x4069), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 }, - { /* Mouse Logitech Performance MX */ - LDJ_DEVICE(0x101a), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_1P0 }, + HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE, + USB_VENDOR_ID_LOGITECH, 0x402d), + .driver_data = HIDPP_QUIRK_DELAYED_INIT | HIDPP_QUIRK_CLASS_M560 }, { /* Keyboard logitech K400 */ HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE, USB_VENDOR_ID_LOGITECH, 0x4024), @@ -3378,19 +3144,12 @@ static const struct hid_device_id hidpp_devices[] = { MODULE_DEVICE_TABLE(hid, hidpp_devices); -static const struct hid_usage_id hidpp_usages[] = { - { HID_GD_WHEEL, EV_REL, REL_WHEEL }, - { HID_ANY_ID - 1, HID_ANY_ID - 1, HID_ANY_ID - 1} -}; - static struct hid_driver hidpp_driver = { .name = "logitech-hidpp-device", .id_table = hidpp_devices, .probe = hidpp_probe, .remove = hidpp_remove, .raw_event = hidpp_raw_event, - .usage_table = hidpp_usages, - .event = hidpp_event, .input_configured = hidpp_input_configured, .input_mapping = hidpp_input_mapping, .input_mapped = hidpp_input_mapped, From 00acc9e2c4a55045d73f2615429742369996d801 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 21 Nov 2018 16:27:10 +0100 Subject: [PATCH 13/15] Revert "HID: logitech: Add function to enable HID++ 1.0 "scrolling acceleration"" This reverts commit 051dc9b0579602bd63e9df74d0879b5293e71581. It turns out the current API is not that compatible with some Microsoft mice, so better start again from scratch. Signed-off-by: Benjamin Tissoires Acked-by: Harry Cutts Acked-by: Dmitry Torokhov Acked-by: Jiri Kosina --- drivers/hid/hid-logitech-hidpp.c | 47 +++++++++----------------------- 1 file changed, 13 insertions(+), 34 deletions(-) diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c index 7f8218f6ff56..19cc980eebce 100644 --- a/drivers/hid/hid-logitech-hidpp.c +++ b/drivers/hid/hid-logitech-hidpp.c @@ -400,53 +400,32 @@ static void hidpp_prefix_name(char **name, int name_length) #define HIDPP_SET_LONG_REGISTER 0x82 #define HIDPP_GET_LONG_REGISTER 0x83 -/** - * hidpp10_set_register_bit() - Sets a single bit in a HID++ 1.0 register. - * @hidpp_dev: the device to set the register on. - * @register_address: the address of the register to modify. - * @byte: the byte of the register to modify. Should be less than 3. - * Return: 0 if successful, otherwise a negative error code. - */ -static int hidpp10_set_register_bit(struct hidpp_device *hidpp_dev, - u8 register_address, u8 byte, u8 bit) +#define HIDPP_REG_GENERAL 0x00 + +static int hidpp10_enable_battery_reporting(struct hidpp_device *hidpp_dev) { struct hidpp_report response; int ret; u8 params[3] = { 0 }; ret = hidpp_send_rap_command_sync(hidpp_dev, - REPORT_ID_HIDPP_SHORT, - HIDPP_GET_REGISTER, - register_address, - NULL, 0, &response); + REPORT_ID_HIDPP_SHORT, + HIDPP_GET_REGISTER, + HIDPP_REG_GENERAL, + NULL, 0, &response); if (ret) return ret; memcpy(params, response.rap.params, 3); - params[byte] |= BIT(bit); + /* Set the battery bit */ + params[0] |= BIT(4); return hidpp_send_rap_command_sync(hidpp_dev, - REPORT_ID_HIDPP_SHORT, - HIDPP_SET_REGISTER, - register_address, - params, 3, &response); -} - - -#define HIDPP_REG_GENERAL 0x00 - -static int hidpp10_enable_battery_reporting(struct hidpp_device *hidpp_dev) -{ - return hidpp10_set_register_bit(hidpp_dev, HIDPP_REG_GENERAL, 0, 4); -} - -#define HIDPP_REG_FEATURES 0x01 - -/* On HID++ 1.0 devices, high-res scroll was called "scrolling acceleration". */ -static int hidpp10_enable_scrolling_acceleration(struct hidpp_device *hidpp_dev) -{ - return hidpp10_set_register_bit(hidpp_dev, HIDPP_REG_FEATURES, 0, 6); + REPORT_ID_HIDPP_SHORT, + HIDPP_SET_REGISTER, + HIDPP_REG_GENERAL, + params, 3, &response); } #define HIDPP_REG_BATTERY_STATUS 0x07 From f1539a0c2545d7bd82e451bd1464f2a820f55de4 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 21 Nov 2018 16:27:11 +0100 Subject: [PATCH 14/15] Revert "HID: input: Create a utility class for counting scroll events" This reverts commit 1ff2e1a44e02d4bdbb9be67c7d9acc240a67141f. It turns out the current API is not that compatible with some Microsoft mice, so better start again from scratch. Signed-off-by: Benjamin Tissoires Acked-by: Harry Cutts Acked-by: Dmitry Torokhov Acked-by: Jiri Kosina --- drivers/hid/hid-input.c | 45 ----------------------------------------- include/linux/hid.h | 28 ------------------------- 2 files changed, 73 deletions(-) diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c index 28ee2ed88a1a..d6fab5798487 100644 --- a/drivers/hid/hid-input.c +++ b/drivers/hid/hid-input.c @@ -1841,48 +1841,3 @@ void hidinput_disconnect(struct hid_device *hid) } EXPORT_SYMBOL_GPL(hidinput_disconnect); -/** - * hid_scroll_counter_handle_scroll() - Send high- and low-resolution scroll - * events given a high-resolution wheel - * movement. - * @counter: a hid_scroll_counter struct describing the wheel. - * @hi_res_value: the movement of the wheel, in the mouse's high-resolution - * units. - * - * Given a high-resolution movement, this function converts the movement into - * microns and emits high-resolution scroll events for the input device. It also - * uses the multiplier from &struct hid_scroll_counter to emit low-resolution - * scroll events when appropriate for backwards-compatibility with userspace - * input libraries. - */ -void hid_scroll_counter_handle_scroll(struct hid_scroll_counter *counter, - int hi_res_value) -{ - int low_res_scroll_amount; - /* Some wheels will rest 7/8ths of a notch from the previous notch - * after slow movement, so we want the threshold for low-res events to - * be in the middle of the notches (e.g. after 4/8ths) as opposed to on - * the notches themselves (8/8ths). - */ - int threshold = counter->resolution_multiplier / 2; - - input_report_rel(counter->dev, REL_WHEEL_HI_RES, - hi_res_value * counter->microns_per_hi_res_unit); - - counter->remainder += hi_res_value; - if (abs(counter->remainder) >= threshold) { - /* Add (or subtract) 1 because we want to trigger when the wheel - * is half-way to the next notch (i.e. scroll 1 notch after a - * 1/2 notch movement, 2 notches after a 1 1/2 notch movement, - * etc.). - */ - low_res_scroll_amount = - counter->remainder / counter->resolution_multiplier - + (hi_res_value > 0 ? 1 : -1); - input_report_rel(counter->dev, REL_WHEEL, - low_res_scroll_amount); - counter->remainder -= - low_res_scroll_amount * counter->resolution_multiplier; - } -} -EXPORT_SYMBOL_GPL(hid_scroll_counter_handle_scroll); diff --git a/include/linux/hid.h b/include/linux/hid.h index 387c70df6f29..a355d61940f2 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -1139,34 +1139,6 @@ static inline u32 hid_report_len(struct hid_report *report) int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size, int interrupt); - -/** - * struct hid_scroll_counter - Utility class for processing high-resolution - * scroll events. - * @dev: the input device for which events should be reported. - * @microns_per_hi_res_unit: the amount moved by the user's finger for each - * high-resolution unit reported by the mouse, in - * microns. - * @resolution_multiplier: the wheel's resolution in high-resolution mode as a - * multiple of its lower resolution. For example, if - * moving the wheel by one "notch" would result in a - * value of 1 in low-resolution mode but 8 in - * high-resolution, the multiplier is 8. - * @remainder: counts the number of high-resolution units moved since the last - * low-resolution event (REL_WHEEL or REL_HWHEEL) was sent. Should - * only be used by class methods. - */ -struct hid_scroll_counter { - struct input_dev *dev; - int microns_per_hi_res_unit; - int resolution_multiplier; - - int remainder; -}; - -void hid_scroll_counter_handle_scroll(struct hid_scroll_counter *counter, - int hi_res_value); - /* HID quirks API */ unsigned long hid_lookup_quirk(const struct hid_device *hdev); int hid_quirks_init(char **quirks_param, __u16 bus, int count); From ffe0e7cf290f5c9d1392134b4ef8da2a3761a4cd Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 21 Nov 2018 16:27:12 +0100 Subject: [PATCH 15/15] Revert "Input: Add the `REL_WHEEL_HI_RES` event code" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit aaf9978c3c0291ef3beaa97610bc9c3084656a85. Quoting Peter: There is a HID feature report called "Resolution Multiplier" Described in the "Enhanced Wheel Support in Windows" doc and the "USB HID Usage Tables" page 30. http://download.microsoft.com/download/b/d/1/bd1f7ef4-7d72-419e-bc5c-9f79ad7bb66e/wheel.docx https://www.usb.org/sites/default/files/documents/hut1_12v2.pdf This was new for Windows Vista, so we're only a decade behind here. I only accidentally found this a few days ago while debugging a stuck button on a Microsoft mouse. The docs above describe it like this: a wheel control by default sends value 1 per notch. If the resolution multiplier is active, the wheel is expected to send a value of $multiplier per notch (e.g. MS Sculpt mouse) or just send events more often, i.e. for less physical motion (e.g. MS Comfort mouse). For the latter, you need the right HW of course. The Sculpt mouse has tactile wheel clicks, so nothing really changes. The Comfort mouse has continuous motion with no tactile clicks. Similar to the free-wheeling Logitech mice but without any inertia. Note that the doc also says that Vista and onwards *always* enable this feature where available. An example HID definition looks like this: Usage Page Generic Desktop (0x01) Usage Resolution Multiplier (0x48) Logical Minimum 0 Logical Maximum 1 Physical Minimum 1 Physical Maximum 16 Report Size 2 # in bits Report Count 1 Feature (Data, Var, Abs) So the actual bits have values 0 or 1 and that reflects real values 1 or 16. We've only seen single-bits so far, so there's low-res and hi-res, but nothing in between. The multiplier is available for HID usages "Wheel" and "AC Pan" (horiz wheel). Microsoft suggests that > Vendors should ship their devices with smooth scrolling disabled and allow > Windows to enable it. This ensures that the device works like a regular HID > device on legacy operating systems that do not support smooth scrolling. (see the wheel doc linked above) The mice that we tested so far do reset on unplug. Device Support looks to be all (?) Microsoft mice but nothing else Not supported: - Logitech G500s, G303 - Roccat Kone XTD - all the cheap Lenovo, HP, Dell, Logitech USB mice that come with a workstation that I could find don't have it. - Etekcity something something - Razer Imperator Supported: - Microsoft Comfort Optical Mouse 3000 - yes, physical: 1:4 - Microsoft Sculpt Ergonomic Mouse - yes, physical: 1:12 - Microsoft Surface mouse - yes, physical: 1:4 So again, I think this is really just available on Microsoft mice, but probably all decent MS mice released over the last decade. Looking at the hardware itself: - no noticeable notches in the weel - low-res: 18 events per 360deg rotation (click angle 20 deg) - high-res: 72 events per 360deg → matches multiplier of 4 - I can feel the notches during wheel turns - low-res: 24 events per 360 deg rotation (click angle 15 deg) - horiz wheel is tilt-based, continuous output value 1 - high-res: 24 events per 360deg with value 12 → matches multiplier of 12 - horiz wheel output rate doubles/triples?, values is 3 - It's a touch strip, not a wheel so no notches - high-res: events have value 4 instead of 1 a bit strange given that it doesn't actually have notches. Ok, why is this an issue for the current API? First, because the logitech multiplier used in Harry's patches looks suspiciously like the Resolution Multiplier so I think we should assume it's the same thing. Nestor, can you shed some light on that? - `REL_WHEEL` is defined as the number of notches, emulated where needed. - `REL_WHEEL_HI_RES` is the movement of the user's finger in microns. - `WM_MOUSEWHEEL` (Windows) is is a multiple of 120, defined as "the threshold for action to be taken and one such action" https://docs.microsoft.com/en-us/windows/desktop/inputdev/wm-mousewheel If the multiplier is set to M, this means we need an accumulated value of M until we can claim there was a wheel click. So after enabling the multiplier and setting it to the maximum (like Windows): - M units are 15deg rotation → 1 unit is 2620/M micron (see below). This is the `REL_WHEEL_HI_RES` value. - wheel diameter 20mm: 15 deg rotation is 2.62mm, 2620 micron (pi * 20mm / (360deg/15deg)) - For every M units accumulated, send one `REL_WHEEL` event The problem here is that we've now hardcoded 20mm/15 deg into the kernel and we have no way of getting the size of the wheel or the click angle into the kernel. In userspace we now have to undo the kernel's calculation. If our click angle is e.g. 20 degree we have to undo the (lossy) calculation from the kernel and calculate the correct angle instead. This also means the 15 is a hardcoded option forever and cannot be changed. In hid-logitech-hidpp.c, the microns per unit is hardcoded per device. Harry, did you measure those by hand? We'd need to update the kernel for every device and there are 10 years worth of devices from MS alone. The multiplier default is 8 which is in the right ballpark, so I'm pretty sure this is the same as the Resolution Multiplier, just in HID++ lingo. And given that the 120 magic factor is what Windows uses in the end, I can't imagine Logitech rolling their own thing here. Nestor? And we're already fairly inaccurate with the microns anyway. The MX Anywhere 2S has a click angle of 20 degrees (18 stops) and a 17mm wheel, so a wheel notch is approximately 2.67mm, one event at multiplier 8 (1/8 of a notch) would be 334 micron. That's only 80% of the fallback value of 406 in the kernel. Multiplier 6 gives us 445micron (10% off). I'm assuming multiplier 7 doesn't exist because it's not a factor of 120. Summary: Best option may be to simply do what Windows is doing, all the HW manufacturers have to use that approach after all. Switch `REL_WHEEL_HI_RES` to report in fractions of 120, with 120 being one notch and divide that by the multiplier for the actual events. So e.g. the Logitech multiplier 8 would send value 15 for each event in hi-res mode. This can be converted in userspace to whatever userspace needs (combined with a hwdb there that tells you wheel size/click angle/...). Conflicts: include/uapi/linux/input-event-codes.h -> I kept the new reserved event in the code, so I had to adapt the revert slightly Signed-off-by: Benjamin Tissoires Acked-by: Harry Cutts Acked-by: Dmitry Torokhov Acked-by: Jiri Kosina --- Documentation/input/event-codes.rst | 11 +---------- include/uapi/linux/input-event-codes.h | 10 ---------- 2 files changed, 1 insertion(+), 20 deletions(-) diff --git a/Documentation/input/event-codes.rst b/Documentation/input/event-codes.rst index cef220c176a4..a8c0873beb95 100644 --- a/Documentation/input/event-codes.rst +++ b/Documentation/input/event-codes.rst @@ -190,16 +190,7 @@ A few EV_REL codes have special meanings: * REL_WHEEL, REL_HWHEEL: - These codes are used for vertical and horizontal scroll wheels, - respectively. The value is the number of "notches" moved on the wheel, the - physical size of which varies by device. For high-resolution wheels (which - report multiple events for each notch of movement, or do not have notches) - this may be an approximation based on the high-resolution scroll events. - -* REL_WHEEL_HI_RES: - - - If a vertical scroll wheel supports high-resolution scrolling, this code - will be emitted in addition to REL_WHEEL. The value is the (approximate) - distance travelled by the user's finger, in microns. + respectively. EV_ABS ------ diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h index 6d180cc60a5d..3eb5a4c3d60a 100644 --- a/include/uapi/linux/input-event-codes.h +++ b/include/uapi/linux/input-event-codes.h @@ -716,7 +716,6 @@ * the situation described above. */ #define REL_RESERVED 0x0a -#define REL_WHEEL_HI_RES 0x0b #define REL_MAX 0x0f #define REL_CNT (REL_MAX+1) @@ -753,15 +752,6 @@ #define ABS_MISC 0x28 -/* - * 0x2e is reserved and should not be used in input drivers. - * It was used by HID as ABS_MISC+6 and userspace needs to detect if - * the next ABS_* event is correct or is just ABS_MISC + n. - * We define here ABS_RESERVED so userspace can rely on it and detect - * the situation described above. - */ -#define ABS_RESERVED 0x2e - #define ABS_MT_SLOT 0x2f /* MT slot being modified */ #define ABS_MT_TOUCH_MAJOR 0x30 /* Major axis of touching ellipse */ #define ABS_MT_TOUCH_MINOR 0x31 /* Minor axis (omit if circular) */