From a7156d818179ddab36006a33d24182a870a81298 Mon Sep 17 00:00:00 2001 From: Bastien Nocera Date: Tue, 20 Jun 2023 10:50:14 +0200 Subject: [PATCH 1/6] HID: logitech-hidpp: Rename HID++ "internal" error constant As per the upstream "hidpp" helpers commit: " There has been some confusion about error value 5 but feature specs that refer to it generally use NOT_ALLOWED. " Signed-off-by: Bastien Nocera Link: https://github.com/mrubli2/hidpp/commit/80c3fecfcd89c5efe0f16feabe90a55ddfd25aaa Signed-off-by: Jiri Kosina --- drivers/hid/hid-logitech-hidpp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c index 129b01be488d2..3f3ad6a8fedca 100644 --- a/drivers/hid/hid-logitech-hidpp.c +++ b/drivers/hid/hid-logitech-hidpp.c @@ -228,7 +228,7 @@ struct hidpp_device { #define HIDPP20_ERROR_INVALID_ARGS 0x02 #define HIDPP20_ERROR_OUT_OF_RANGE 0x03 #define HIDPP20_ERROR_HW_ERROR 0x04 -#define HIDPP20_ERROR_LOGITECH_INTERNAL 0x05 +#define HIDPP20_ERROR_NOT_ALLOWED 0x05 #define HIDPP20_ERROR_INVALID_FEATURE_INDEX 0x06 #define HIDPP20_ERROR_INVALID_FUNCTION_ID 0x07 #define HIDPP20_ERROR_BUSY 0x08 From c05b8a939a179aca7ee9eb2cf03a322e83522e07 Mon Sep 17 00:00:00 2001 From: Mavroudis Chatzilazaridis Date: Sun, 16 Jul 2023 18:23:52 +0000 Subject: [PATCH 2/6] HID: logitech-hidpp: Add support for the Pro X Superlight MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch adds support for the Pro X Superlight over wired USB. The device now reports the status of its battery. Co-developed-by: Filipe Laíns Signed-off-by: Filipe Laíns Signed-off-by: Mavroudis Chatzilazaridis Reviewed-by: Bastien Nocera Signed-off-by: Jiri Kosina --- drivers/hid/hid-logitech-hidpp.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c index 3f3ad6a8fedca..6ad344ba0d1fc 100644 --- a/drivers/hid/hid-logitech-hidpp.c +++ b/drivers/hid/hid-logitech-hidpp.c @@ -4620,6 +4620,8 @@ static const struct hid_device_id hidpp_devices[] = { .driver_data = HIDPP_QUIRK_CLASS_G920 | HIDPP_QUIRK_FORCE_OUTPUT_REPORTS }, { /* Logitech G Pro Gaming Mouse over USB */ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC088) }, + { /* Logitech G Pro X Superlight Gaming Mouse over USB */ + HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC094) }, { /* G935 Gaming Headset */ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0x0a87), From 9d1bd9346241cd6963b58da7ffb7ed303285f684 Mon Sep 17 00:00:00 2001 From: Mavroudis Chatzilazaridis Date: Sun, 16 Jul 2023 18:23:44 +0000 Subject: [PATCH 3/6] HID: logitech-dj: Add support for a new lightspeed receiver iteration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lightspeed receiver for the Pro X Superlight uses 13 byte mouse reports without a report id. The workaround for such cases has been adjusted to handle these larger packets. The device now reports the status of its battery in wireless mode and libratbag now recognizes the device and it can be configured with Piper. https://github.com/libratbag/libratbag/pull/1122 Co-developed-by: Filipe Laíns Signed-off-by: Filipe Laíns Signed-off-by: Mavroudis Chatzilazaridis Reviewed-by: Bastien Nocera Signed-off-by: Jiri Kosina --- drivers/hid/hid-ids.h | 1 + drivers/hid/hid-logitech-dj.c | 11 ++++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 8a310f8ff20f5..7ac7debe98f10 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -866,6 +866,7 @@ #define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_2 0xc534 #define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1 0xc539 #define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1_1 0xc53f +#define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1_2 0xc547 #define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_POWERPLAY 0xc53a #define USB_DEVICE_ID_SPACETRAVELLER 0xc623 #define USB_DEVICE_ID_SPACENAVIGATOR 0xc626 diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c index 62180414efccd..fef67da0de537 100644 --- a/drivers/hid/hid-logitech-dj.c +++ b/drivers/hid/hid-logitech-dj.c @@ -1692,11 +1692,12 @@ static int logi_dj_raw_event(struct hid_device *hdev, } /* * Mouse-only receivers send unnumbered mouse data. The 27 MHz - * receiver uses 6 byte packets, the nano receiver 8 bytes. + * receiver uses 6 byte packets, the nano receiver 8 bytes, + * the lightspeed receiver (Pro X Superlight) 13 bytes. */ if (djrcv_dev->unnumbered_application == HID_GD_MOUSE && - size <= 8) { - u8 mouse_report[9]; + size <= 13){ + u8 mouse_report[14]; /* Prepend report id */ mouse_report[0] = REPORT_TYPE_MOUSE; @@ -1980,6 +1981,10 @@ static const struct hid_device_id logi_dj_receivers[] = { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1_1), .driver_data = recvr_type_gaming_hidpp}, + { /* Logitech lightspeed receiver (0xc547) */ + HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, + USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1_2), + .driver_data = recvr_type_gaming_hidpp}, { /* Logitech 27 MHz HID++ 1.0 receiver (0xc513) */ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_MX3000_RECEIVER), From ed1fb63b6e45d933527c212c0b5339d613350a3a Mon Sep 17 00:00:00 2001 From: Maxim Mikityanskiy Date: Wed, 9 Aug 2023 14:38:32 +0300 Subject: [PATCH 4/6] HID: logitech-hidpp: Add support for Logitech MX Anywhere 3 mouse Add Logitech MX Anywhere 3 connected over Bluetooth to the device table to get hidpi scroll supported. USB connection over the Unifying receiver is already supported by the wildcard entry LDJ_DEVICE(HID_ANY_ID). Signed-off-by: Maxim Mikityanskiy Reviewed-by: Bastien Nocera Signed-off-by: Jiri Kosina --- drivers/hid/hid-logitech-hidpp.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c index 6ad344ba0d1fc..340c1ac442ad7 100644 --- a/drivers/hid/hid-logitech-hidpp.c +++ b/drivers/hid/hid-logitech-hidpp.c @@ -4649,6 +4649,8 @@ static const struct hid_device_id hidpp_devices[] = { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb02a) }, { /* MX Master 3 mouse over Bluetooth */ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb023) }, + { /* MX Anywhere 3 mouse over Bluetooth */ + HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb025) }, { /* MX Master 3S mouse over Bluetooth */ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb034) }, {} From 6f20d3261265885f6a6be4cda49d7019728760e0 Mon Sep 17 00:00:00 2001 From: Nikita Zhandarovich Date: Tue, 13 Jun 2023 03:16:35 -0700 Subject: [PATCH 5/6] HID: logitech-dj: Fix error handling in logi_dj_recv_switch_to_dj_mode() Presently, if a call to logi_dj_recv_send_report() fails, we do not learn about the error until after sending short HID_OUTPUT_REPORT with hid_hw_raw_request(). To handle this somewhat unlikely issue, return on error in logi_dj_recv_send_report() (minding ugly sleep workaround) and take into account the result of hid_hw_raw_request(). Found by Linux Verification Center (linuxtesting.org) with static analysis tool SVACE. Fixes: 6a9ddc897883 ("HID: logitech-dj: enable notifications on connect/disconnect") Signed-off-by: Nikita Zhandarovich Link: https://lore.kernel.org/r/20230613101635.77820-1-n.zhandarovich@fintech.ru Signed-off-by: Benjamin Tissoires --- drivers/hid/hid-logitech-dj.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c index fef67da0de537..8afe3be683ba2 100644 --- a/drivers/hid/hid-logitech-dj.c +++ b/drivers/hid/hid-logitech-dj.c @@ -1285,6 +1285,9 @@ static int logi_dj_recv_switch_to_dj_mode(struct dj_receiver_dev *djrcv_dev, * 50 msec should gives enough time to the receiver to be ready. */ msleep(50); + + if (retval) + return retval; } /* @@ -1306,7 +1309,7 @@ static int logi_dj_recv_switch_to_dj_mode(struct dj_receiver_dev *djrcv_dev, buf[5] = 0x09; buf[6] = 0x00; - hid_hw_raw_request(hdev, REPORT_ID_HIDPP_SHORT, buf, + retval = hid_hw_raw_request(hdev, REPORT_ID_HIDPP_SHORT, buf, HIDPP_REPORT_SHORT_LENGTH, HID_OUTPUT_REPORT, HID_REQ_SET_REPORT); From 60165ab774cb0c509680a73cf826d0e158454653 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 12 Jul 2023 17:02:34 +0200 Subject: [PATCH 6/6] HID: logitech-hidpp: rework one more time the retries attempts Extract the internal code inside a helper function, fix the initialization of the parameters used in the helper function (`hidpp->answer_available` was not reset and `*response` wasn't either), and use a `do {...} while();` loop. Fixes: 586e8fede795 ("HID: logitech-hidpp: Retry commands when device is busy") Cc: stable@vger.kernel.org Reviewed-by: Bastien Nocera Signed-off-by: Benjamin Tissoires Link: https://lore.kernel.org/r/20230621-logitech-fixes-v2-1-3635f7f9c8af@kernel.org Signed-off-by: Benjamin Tissoires --- drivers/hid/hid-logitech-hidpp.c | 115 ++++++++++++++++++++----------- 1 file changed, 75 insertions(+), 40 deletions(-) diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c index 340c1ac442ad7..05f5b5f588a28 100644 --- a/drivers/hid/hid-logitech-hidpp.c +++ b/drivers/hid/hid-logitech-hidpp.c @@ -275,21 +275,22 @@ static int __hidpp_send_report(struct hid_device *hdev, } /* - * hidpp_send_message_sync() returns 0 in case of success, and something else - * in case of a failure. - * - If ' something else' is positive, that means that an error has been raised - * by the protocol itself. - * - If ' something else' is negative, that means that we had a classic error - * (-ENOMEM, -EPIPE, etc...) + * Effectively send the message to the device, waiting for its answer. + * + * Must be called with hidpp->send_mutex locked + * + * Same return protocol than hidpp_send_message_sync(): + * - success on 0 + * - negative error means transport error + * - positive value means protocol error */ -static int hidpp_send_message_sync(struct hidpp_device *hidpp, +static int __do_hidpp_send_message_sync(struct hidpp_device *hidpp, struct hidpp_report *message, struct hidpp_report *response) { - int ret = -1; - int max_retries = 3; + int ret; - mutex_lock(&hidpp->send_mutex); + __must_hold(&hidpp->send_mutex); hidpp->send_receive_buf = response; hidpp->answer_available = false; @@ -300,47 +301,74 @@ static int hidpp_send_message_sync(struct hidpp_device *hidpp, */ *response = *message; - for (; max_retries != 0 && ret; max_retries--) { - ret = __hidpp_send_report(hidpp->hid_dev, message); + ret = __hidpp_send_report(hidpp->hid_dev, message); + if (ret) { + dbg_hid("__hidpp_send_report returned err: %d\n", ret); + memset(response, 0, sizeof(struct hidpp_report)); + return ret; + } - if (ret) { - dbg_hid("__hidpp_send_report returned err: %d\n", ret); - memset(response, 0, sizeof(struct hidpp_report)); - break; - } + if (!wait_event_timeout(hidpp->wait, hidpp->answer_available, + 5*HZ)) { + dbg_hid("%s:timeout waiting for response\n", __func__); + memset(response, 0, sizeof(struct hidpp_report)); + return -ETIMEDOUT; + } - if (!wait_event_timeout(hidpp->wait, hidpp->answer_available, - 5*HZ)) { - dbg_hid("%s:timeout waiting for response\n", __func__); - memset(response, 0, sizeof(struct hidpp_report)); - ret = -ETIMEDOUT; - break; - } + if (response->report_id == REPORT_ID_HIDPP_SHORT && + response->rap.sub_id == HIDPP_ERROR) { + ret = response->rap.params[1]; + dbg_hid("%s:got hidpp error %02X\n", __func__, ret); + return ret; + } - if (response->report_id == REPORT_ID_HIDPP_SHORT && - response->rap.sub_id == HIDPP_ERROR) { - ret = response->rap.params[1]; - dbg_hid("%s:got hidpp error %02X\n", __func__, ret); + if ((response->report_id == REPORT_ID_HIDPP_LONG || + response->report_id == REPORT_ID_HIDPP_VERY_LONG) && + response->fap.feature_index == HIDPP20_ERROR) { + ret = response->fap.params[1]; + dbg_hid("%s:got hidpp 2.0 error %02X\n", __func__, ret); + return ret; + } + + return 0; +} + +/* + * hidpp_send_message_sync() returns 0 in case of success, and something else + * in case of a failure. + * + * See __do_hidpp_send_message_sync() for a detailed explanation of the returned + * value. + */ +static int hidpp_send_message_sync(struct hidpp_device *hidpp, + struct hidpp_report *message, + struct hidpp_report *response) +{ + int ret; + int max_retries = 3; + + mutex_lock(&hidpp->send_mutex); + + do { + ret = __do_hidpp_send_message_sync(hidpp, message, response); + if (ret != HIDPP20_ERROR_BUSY) break; - } - if ((response->report_id == REPORT_ID_HIDPP_LONG || - response->report_id == REPORT_ID_HIDPP_VERY_LONG) && - response->fap.feature_index == HIDPP20_ERROR) { - ret = response->fap.params[1]; - if (ret != HIDPP20_ERROR_BUSY) { - dbg_hid("%s:got hidpp 2.0 error %02X\n", __func__, ret); - break; - } - dbg_hid("%s:got busy hidpp 2.0 error %02X, retrying\n", __func__, ret); - } - } + dbg_hid("%s:got busy hidpp 2.0 error %02X, retrying\n", __func__, ret); + } while (--max_retries); mutex_unlock(&hidpp->send_mutex); return ret; } +/* + * hidpp_send_fap_command_sync() returns 0 in case of success, and something else + * in case of a failure. + * + * See __do_hidpp_send_message_sync() for a detailed explanation of the returned + * value. + */ static int hidpp_send_fap_command_sync(struct hidpp_device *hidpp, u8 feat_index, u8 funcindex_clientid, u8 *params, int param_count, struct hidpp_report *response) @@ -373,6 +401,13 @@ static int hidpp_send_fap_command_sync(struct hidpp_device *hidpp, return ret; } +/* + * hidpp_send_rap_command_sync() returns 0 in case of success, and something else + * in case of a failure. + * + * See __do_hidpp_send_message_sync() for a detailed explanation of the returned + * value. + */ static int hidpp_send_rap_command_sync(struct hidpp_device *hidpp_dev, u8 report_id, u8 sub_id, u8 reg_address, u8 *params, int param_count, struct hidpp_report *response)