2023-03-02 13:01:27

by Bastien Nocera

Subject: [PATCH 1/2] HID: logitech-hidpp: Don't use the USB serial for USB devices

For devices that support the 0x0003 feature (Device Information) version 4,
set the serial based on the output of that feature, rather than relying
on the usbhid code setting the USB serial.

This should allow the serial when connected through USB to (nearly)
match the one when connected through a unifying receiver.

For example, on the serials on a G903 wired/wireless mouse:
- Unifying: 4067-e8-ce-cd-45
- USB before patch: 017C385C3837
- USB after patch: c086-e8-ce-cd-45

Signed-off-by: Bastien Nocera <[email protected]>
---
drivers/hid/hid-logitech-hidpp.c | 51 ++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index c7d81b4241ad..99ce2c49facc 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -969,6 +969,55 @@ static int hidpp_root_get_protocol_version(struct hidpp_device *hidpp)
return 0;
}

+/* -------------------------------------------------------------------------- */
+/* 0x0003: Device Information */
+/* -------------------------------------------------------------------------- */
+
+#define HIDPP_PAGE_DEVICE_INFORMATION 0x0003
+
+#define CMD_GET_DEVICE_INFO 0x00
+
+static int hidpp_get_serial(struct hidpp_device *hidpp, u32 *serial)
+{
+ struct hidpp_report response;
+ u8 feature_type;
+ u8 feature_index;
+ int ret;
+
+ ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_DEVICE_INFORMATION,
+ &feature_index,
+ &feature_type);
+ if (ret)
+ return ret;
+
+ ret = hidpp_send_fap_command_sync(hidpp, feature_index,
+ CMD_GET_DEVICE_INFO,
+ NULL, 0, &response);
+ if (ret)
+ return ret;
+
+ /* See hidpp_unifying_get_serial() */
+ *serial = *((u32 *)&response.rap.params[1]);
+ return 0;
+}
+
+static int hidpp_serial_init(struct hidpp_device *hidpp)
+{
+ struct hid_device *hdev = hidpp->hid_dev;
+ u32 serial;
+ int ret;
+
+ ret = hidpp_get_serial(hidpp, &serial);
+ if (ret)
+ return ret;
+
+ snprintf(hdev->uniq, sizeof(hdev->uniq), "%04x-%4phD",
+ hdev->product, &serial);
+ dbg_hid("HID++ DeviceInformation: Got serial: %s\n", hdev->uniq);
+
+ return 0;
+}
+
/* -------------------------------------------------------------------------- */
/* 0x0005: GetDeviceNameType */
/* -------------------------------------------------------------------------- */
@@ -4398,6 +4447,8 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)

if (hidpp->quirks & HIDPP_QUIRK_UNIFYING)
hidpp_unifying_init(hidpp);
+ else if (hid_is_usb(hidpp->hid_dev))
+ hidpp_serial_init(hidpp);

connected = hidpp_root_get_protocol_version(hidpp) == 0;
atomic_set(&hidpp->connected, connected);
--
2.39.2

2023-03-02 13:01:31

by Bastien Nocera

Subject: [PATCH 2/2] HID: logitech-hidpp: Reconcile USB and Unifying serials

Now that USB HID++ devices can gather a serial number that matches the
one that would be gathered when connected through a Unifying receiver,
remove the last difference by dropping the product ID as devices
usually have different product IDs when connected through USB or
Unifying.

For example, on the serials on a G903 wired/wireless mouse:
- Unifying before patch: 4067-e8-ce-cd-45
- USB before patch: c086-e8-ce-cd-45
- Unifying and USB after patch: e8-ce-cd-45

Signed-off-by: Bastien Nocera <[email protected]>
---
drivers/hid/hid-logitech-hidpp.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 99ce2c49facc..0cda1d07641f 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -875,8 +875,7 @@ static int hidpp_unifying_init(struct hidpp_device *hidpp)
if (ret)
return ret;

- snprintf(hdev->uniq, sizeof(hdev->uniq), "%04x-%4phD",
- hdev->product, &serial);
+ snprintf(hdev->uniq, sizeof(hdev->uniq), "%4phD", &serial);
dbg_hid("HID++ Unifying: Got serial: %s\n", hdev->uniq);

name = hidpp_unifying_get_name(hidpp);
@@ -1011,8 +1010,7 @@ static int hidpp_serial_init(struct hidpp_device *hidpp)
if (ret)
return ret;

- snprintf(hdev->uniq, sizeof(hdev->uniq), "%04x-%4phD",
- hdev->product, &serial);
+ snprintf(hdev->uniq, sizeof(hdev->uniq), "%4phD", &serial);
dbg_hid("HID++ DeviceInformation: Got serial: %s\n", hdev->uniq);

return 0;
--
2.39.2

2023-04-03 13:46:17

by Benjamin Tissoires

Subject: Re: [PATCH 1/2] HID: logitech-hidpp: Don't use the USB serial for USB devices

On Thu, 02 Mar 2023 14:01:16 +0100, Bastien Nocera wrote:
> For devices that support the 0x0003 feature (Device Information) version 4,
> set the serial based on the output of that feature, rather than relying
> on the usbhid code setting the USB serial.
>
> This should allow the serial when connected through USB to (nearly)
> match the one when connected through a unifying receiver.
>
> [...]

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git (for-6.4/logitech-hidpp), thanks!

[1/2] HID: logitech-hidpp: Don't use the USB serial for USB devices
https://git.kernel.org/hid/hid/c/7ad1fe0da0fa
[2/2] HID: logitech-hidpp: Reconcile USB and Unifying serials
https://git.kernel.org/hid/hid/c/5b3691d15e04

Cheers,
--
Benjamin Tissoires <[email protected]>