diff mbox series

[RESEND,V2] HID: hid-logitech-hidpp: Implement support for some Bluetooth HID++ devices

Message ID ATZsWjU_J53a3qyKgb8BWLXY7N7UqGilAA9eAyGhQSBUw2z30AwA8rpsdNFejxH4UvYxb6dsY8-jJknYfafTVWF4mMuVDGdtXki7_x7l77g=@protonmail.com (mailing list archive)
State Superseded, archived
Delegated to: Jiri Kosina
Headers show
Series [RESEND,V2] HID: hid-logitech-hidpp: Implement support for some Bluetooth HID++ devices | expand

Commit Message

Mazin Rezk Sept. 18, 2019, 12:44 p.m. UTC 
On Thursday, September 12, 2019 10:40 PM, Mazin Rezk <mnrzk@protonmail.com> wrote:

> This patch adds support the MX Anywhere 2, MX Anywhere 2S, MX Master, and
> MX Master 2S over Bluetooth to the hid-logitech-hidpp module. This patch also
> implements a foundation for other Bluetooth devices to be added to the module.
>
> Changes include:
>
> 1.  Adding the device IDs for the aforementioned mice over Bluetooth (these
> IDs have been copied from the libratbag device database). Their quirks have
> been based on their DJ device counterparts.
> 2.  Adding an additional HIDPP_QUIRK_BLUETOOTH_MISSING_SHORT_ID as these
> devices do not support Short HID++ reports. This quirk causes short reports to
> be sent as long reports to the device and overrides the device validation to
> only check if the long report descriptor exists. Without this quirk, these
> devices will fail the HID++ device validation.
>
> Note about these changes: I only own an MX Master (b01e) so I have not been
> able to test this patch on other devices. However, I have also noticed that
> the MX Master 2S over Bluetooth does not pass the original HID++ report
> descriptor tests which leads me to believe all MX Bluetooth LE devices are
> missing short report descriptors. Further testing with the other devices may
> be required.

In addition to the previous changes:

I recently noticed that the patch I previously submitted does not properly apply
and contains a bug that would cause the added devices not to properly parse
connection events.

The added Bluetooth devices do not support 0x41 connection events and instead
use feature 0x1d4b (WirelessDeviceStatus). HIDPP_QUIRK_WIRELESS_DEVICE_STATUS
causes hidpp_report_is_connect_event to check if the feature ID of the event is
0x1d4b instead.

In order to avoid sending device reports while processing events, an array of
all feature IDs is created and stored in the hidpp_device when any HID++ 2.0
device is probed.

This array is created by calling FeatureSet (0x0001), getting the feature count,
and setting each feature index in the array to its corresponding feature.

Since these devices are both missing a short report and use WirelessDeviceStatus,
I decided to call the quirk HIDPP_QUIRK_CLASS_BLUETOOTH and I added the additional
quirks HIDPP_QUIRK_MISSING_SHORT_REPORTS and HIDPP_QUIRK_WIRELESS_DEVICE_STATUS
as aliases and potential catch-alls in the future.

Signed-off-by: Mazin Rezk <mnrzk@protonmail.com>
---
 drivers/hid/hid-logitech-hidpp.c | 152 ++++++++++++++++++++++++++++++-
 1 file changed, 149 insertions(+), 3 deletions(-)

--
2.23.0
diff mbox series

Patch

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 0179f7ed77e5..3f40d53dcfa1 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -58,6 +58,7 @@  MODULE_PARM_DESC(disable_tap_to_click,
 #define HIDPP_QUIRK_CLASS_K400			BIT(2)
 #define HIDPP_QUIRK_CLASS_G920			BIT(3)
 #define HIDPP_QUIRK_CLASS_K750			BIT(4)
+#define HIDPP_QUIRK_CLASS_BLUETOOTH		BIT(5)

 /* bits 2..20 are reserved for classes */
 /* #define HIDPP_QUIRK_CONNECT_EVENTS		BIT(21) disabled */
@@ -81,6 +82,10 @@  MODULE_PARM_DESC(disable_tap_to_click,
 					 HIDPP_QUIRK_HI_RES_SCROLL_X2120 | \
 					 HIDPP_QUIRK_HI_RES_SCROLL_X2121)

+/* Just an alias for now, may possibly be a catch-all in the future */
+#define HIDPP_QUIRK_MISSING_SHORT_REPORTS	HIDPP_QUIRK_CLASS_BLUETOOTH
+#define HIDPP_QUIRK_WIRELESS_DEVICE_STATUS	HIDPP_QUIRK_CLASS_BLUETOOTH
+
 #define HIDPP_QUIRK_DELAYED_INIT		HIDPP_QUIRK_NO_HIDINPUT

 #define HIDPP_CAPABILITY_HIDPP10_BATTERY	BIT(0)
@@ -186,6 +191,9 @@  struct hidpp_device {

 	struct hidpp_battery battery;
 	struct hidpp_scroll_counter vertical_wheel_counter;
+
+	u16 *features;
+	u8 feature_count;
 };

 /* HID++ 1.0 error codes */
@@ -340,6 +348,11 @@  static int hidpp_send_rap_command_sync(struct hidpp_device *hidpp_dev,
 	struct hidpp_report *message;
 	int ret, max_count;

+	/* Force long reports on devices that do not support short reports */
+	if (hidpp_dev->quirks & HIDPP_QUIRK_MISSING_SHORT_REPORTS &&
+	    report_id == REPORT_ID_HIDPP_SHORT)
+		report_id = REPORT_ID_HIDPP_LONG;
+
 	switch (report_id) {
 	case REPORT_ID_HIDPP_SHORT:
 		max_count = HIDPP_REPORT_SHORT_LENGTH - 4;
@@ -393,9 +406,22 @@  static inline bool hidpp_match_error(struct hidpp_report *question,
 	    (answer->fap.params[0] == question->fap.funcindex_clientid);
 }

-static inline bool hidpp_report_is_connect_event(struct hidpp_report *report)
+#define HIDPP_PAGE_WIRELESS_DEVICE_STATUS		0x1d4b
+
+static inline bool hidpp_report_is_connect_event(struct hidpp_device *hidpp,
+						 struct hidpp_report *report)
 {
-	return (report->report_id == REPORT_ID_HIDPP_SHORT) &&
+	if (hidpp->quirks & HIDPP_QUIRK_WIRELESS_DEVICE_STATUS) {
+		/* If feature is invalid, skip array check */
+		if (report->fap.feature_index > hidpp->feature_count)
+			return false;
+
+		return (hidpp->features[report->fap.feature_index] ==
+			 HIDPP_PAGE_WIRELESS_DEVICE_STATUS);
+	}
+
+	return ((report->report_id == REPORT_ID_HIDPP_SHORT) ||
+		(hidpp->quirks & HIDPP_QUIRK_MISSING_SHORT_REPORTS)) &&
 		(report->rap.sub_id == 0x41);
 }

@@ -901,6 +927,84 @@  static int hidpp_root_get_protocol_version(struct hidpp_device *hidpp)
 	return 0;
 }

+/* -------------------------------------------------------------------------- */
+/* 0x0001: FeatureSet                                                         */
+/* -------------------------------------------------------------------------- */
+
+#define HIDPP_PAGE_FEATURESET				0x0001
+
+#define CMD_FEATURESET_GET_COUNT			0x00
+#define CMD_FEATURESET_GET_FEATURE			0x11
+
+static int hidpp20_featureset_get_feature(struct hidpp_device *hidpp,
+	u8 featureset_index, u8 feature_index, u16 *feature_id)
+{
+	struct hidpp_report response;
+	int ret;
+
+	ret = hidpp_send_fap_command_sync(hidpp, featureset_index,
+		CMD_FEATURESET_GET_FEATURE, &feature_index, 1, &response);
+
+	if (ret)
+		return ret;
+
+	*feature_id = (response.fap.params[0] << 8) | response.fap.params[1];
+
+	return ret;
+}
+
+static int hidpp20_featureset_get_count(struct hidpp_device *hidpp,
+	u8 feature_index, u8 *count)
+{
+	struct hidpp_report response;
+	int ret;
+
+	ret = hidpp_send_fap_command_sync(hidpp, feature_index,
+		CMD_FEATURESET_GET_COUNT, NULL, 0, &response);
+
+	if (ret)
+		return ret;
+
+	*count = response.fap.params[0];
+
+	return ret;
+}
+
+static int hidpp20_get_features(struct hidpp_device *hidpp)
+{
+	int ret;
+	u8 featureset_index, featureset_type;
+	u8 i;
+
+	hidpp->feature_count = 0;
+
+	ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_FEATURESET,
+				     &featureset_index, &featureset_type);
+
+	if (ret == -ENOENT) {
+		hid_warn(hidpp->hid_dev, "Unable to retrieve feature set.");
+		return 0;
+	}
+
+	if (ret)
+		return ret;
+
+	ret = hidpp20_featureset_get_count(hidpp, featureset_index,
+		&hidpp->feature_count);
+
+	if (ret)
+		return ret;
+
+	hidpp->features = devm_kzalloc(&hidpp->hid_dev->dev,
+			hidpp->feature_count * sizeof(u16), GFP_KERNEL);
+
+	for (i = 0; i < hidpp->feature_count && !ret; i++)
+		ret = hidpp20_featureset_get_feature(hidpp, featureset_index,
+				i, &(hidpp->features[i]));
+
+	return ret;
+}
+
 /* -------------------------------------------------------------------------- */
 /* 0x0005: GetDeviceNameType                                                  */
 /* -------------------------------------------------------------------------- */
@@ -3068,7 +3172,7 @@  static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
 		}
 	}

-	if (unlikely(hidpp_report_is_connect_event(report))) {
+	if (unlikely(hidpp_report_is_connect_event(hidpp, report))) {
 		atomic_set(&hidpp->connected,
 				!(report->rap.params[0] & (1 << 6)));
 		if (schedule_work(&hidpp->work) == 0)
@@ -3482,6 +3586,12 @@  static bool hidpp_validate_report(struct hid_device *hdev, int id,

 static bool hidpp_validate_device(struct hid_device *hdev)
 {
+	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+	/* Skip the short report check if the device does not support it */
+	if (hidpp->quirks & HIDPP_QUIRK_MISSING_SHORT_REPORTS)
+		return hidpp_validate_report(hdev, REPORT_ID_HIDPP_LONG,
+					     HIDPP_REPORT_LONG_LENGTH, false);
+
 	return hidpp_validate_report(hdev, REPORT_ID_HIDPP_SHORT,
 				     HIDPP_REPORT_SHORT_LENGTH, false) &&
 	       hidpp_validate_report(hdev, REPORT_ID_HIDPP_LONG,
@@ -3619,6 +3729,17 @@  static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 			goto hid_hw_init_fail;
 	}

+	/* Cache feature indexes and IDs to check reports faster */
+	if (hidpp->protocol_major >= 2) {
+		if (hidpp20_get_features(hidpp)) {
+			hid_err(hdev, "%s:hidpp20_get_features returned error\n",
+				__func__);
+			goto hid_hw_init_fail;
+		}
+	} else {
+		hidpp->feature_count = 0;
+	}
+
 	hidpp_connect_event(hidpp);

 	/* Reset the HID node state */
@@ -3773,6 +3894,31 @@  static const struct hid_device_id hidpp_devices[] = {
 	{ /* MX5500 keyboard over Bluetooth */
 	  HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb30b),
 	  .driver_data = HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS },
+	{ /* MX Anywhere 2 mouse over Bluetooth */
+	  HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb013),
+	  .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 |
+			HIDPP_QUIRK_CLASS_BLUETOOTH },
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb018),
+	  .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 |
+			 HIDPP_QUIRK_CLASS_BLUETOOTH },
+	{ /* MX Anywhere 2S mouse over Bluetooth */
+	  HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb01a),
+	  .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 |
+			 HIDPP_QUIRK_CLASS_BLUETOOTH },
+	{ /* MX Master mouse over Bluetooth */
+	  HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb012),
+	  .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 |
+			 HIDPP_QUIRK_CLASS_BLUETOOTH },
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb017),
+	  .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 |
+			 HIDPP_QUIRK_CLASS_BLUETOOTH },
+	{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb01e),
+	  .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 |
+			 HIDPP_QUIRK_CLASS_BLUETOOTH },
+	{ /* MX Master 2S mouse over Bluetooth */
+	  HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb019),
+	  .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 |
+			 HIDPP_QUIRK_CLASS_BLUETOOTH },
 	{}
 };