[RFC 0/3] HID: logitech-dj: Dinovo keyboard fixes and improvements

[RFC 0/3] HID: logitech-dj: Dinovo keyboard fixes and improvements

[Hans de Goede]

Hi Benjamin,

Here is my patch series for the discussed Dinovo keyboard (receiver)
support improvements.

I've marked this as a RFC since it has not been tested with a Dinovo Mini
(nor a Dinovo Mini receiver) yet.

I have tested it extensively with a Dinovo Edge, a MX5000 and a MX5500
keyboard. In case of the Dinovo Edge and MX5000 I've not only tested
them against their own receiver but also against each-others receiver.

Once you have tested this series on your Dinovo Mini, it is ready to
go upstream. The first patch should probably go to 5.10 as a fix in
case someone pairs the Dinovo Mini with a MX5x00 receiver like the
reporter of this bug did with his Dinovo Edge:
https://bugzilla.redhat.com/show_bug.cgi?id=1811424

The other 2 are 5.11 material.

Regards,

Hans

[RFC 1/3] HID: logitech-dj: Fix Dinovo Mini when paired with a MX5x00 receiver

[Hans de Goede]

Some users are pairing the Dinovo keyboards with the MX5000 or MX5500
receivers, instead of with the Dinovo receivers. The receivers are
mostly the same (and the air protocol obviously is compatible) but
currently the Dinovo receivers are handled by hid-lg.c while the
MX5x00 receivers are handled by logitech-dj.c.

When using a Dinovo keyboard, with its builtin touchpad, through
logitech-dj.c then the touchpad stops working because when asking the
receiver for paired devices, we get only 1 paired device with
a device_type of REPORT_TYPE_KEYBOARD. And since we don't see a paired
mouse, we have nowhere to send mouse-events to, so we drop them.

Extend the existing fix for the Dinovo Edge for this to also cover the
Dinovo Mini keyboard and also add a mapping to logitech-hidpp for the
Media key on the Dinovo Mini, so that that keeps working too.

BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1811424
Fixes: f2113c3020ef ("HID: logitech-dj: add support for Logitech Bluetooth Mini-Receiver")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/hid/hid-logitech-dj.c    |  1 +
 drivers/hid/hid-logitech-hidpp.c | 25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index cda50c420f0b..4976ed827b3e 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -876,6 +876,7 @@ static void logi_dj_recv_queue_notification(struct dj_receiver_dev *djrcv_dev,
  */
 static const u16 kbd_builtin_touchpad_ids[] = {
 	0xb309, /* Dinovo Edge */
+	0xb30c, /* Dinovo Mini */
 };
 
 static void logi_hidpp_dev_conn_notif_equad(struct hid_device *hdev,
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 9fcd7f6c5a71..d062fffc6ebb 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -95,6 +95,8 @@ MODULE_PARM_DESC(disable_tap_to_click,
 #define HIDPP_CAPABILITY_BATTERY_LEVEL_STATUS	BIT(3)
 #define HIDPP_CAPABILITY_BATTERY_VOLTAGE	BIT(4)
 
+#define lg_map_key_clear(c)  hid_map_usage_clear(hi, usage, bit, max, EV_KEY, (c))
+
 /*
  * There are two hidpp protocols in use, the first version hidpp10 is known
  * as register access protocol or RAP, the second version hidpp20 is known as
@@ -2952,6 +2954,26 @@ static int g920_get_config(struct hidpp_device *hidpp,
 	return g920_ff_set_autocenter(hidpp, data);
 }
 
+/* -------------------------------------------------------------------------- */
+/* Logitech Dinovo Mini keyboard with builtin touchpad                        */
+/* -------------------------------------------------------------------------- */
+#define DINOVO_MINI_PRODUCT_ID		0xb30c
+
+static int lg_dinovo_input_mapping(struct hid_device *hdev, struct hid_input *hi,
+		struct hid_field *field, struct hid_usage *usage,
+		unsigned long **bit, int *max)
+{
+	if ((usage->hid & HID_USAGE_PAGE) != HID_UP_LOGIVENDOR)
+		return 0;
+
+	switch (usage->hid & HID_USAGE) {
+	case 0x00d: lg_map_key_clear(KEY_MEDIA);	break;
+	default:
+		return 0;
+	}
+	return 1;
+}
+
 /* -------------------------------------------------------------------------- */
 /* HID++1.0 devices which use HID++ reports for their wheels                  */
 /* -------------------------------------------------------------------------- */
@@ -3187,6 +3209,9 @@ static int hidpp_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 			field->application != HID_GD_MOUSE)
 		return m560_input_mapping(hdev, hi, field, usage, bit, max);
 
+	if (hdev->product == DINOVO_MINI_PRODUCT_ID)
+		return lg_dinovo_input_mapping(hdev, hi, field, usage, bit, max);
+
 	return 0;
 }
 
-- 
2.28.0

[RFC 2/3] HID: logitech-dj: Use hid-ids.h defines for USB device-ids for all supported devices

[Hans de Goede]

The logitech-dj code already uses hid-ids.h defines for almost all devices
it supports. Lets be consistent: add and use hid-ids.h defines for the
G700, MX5000 and MX5500 receivers too.

Also add / update some comments to make the comment style in the
hid_device_id table consistent too.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/hid/hid-ids.h         |  5 +++++
 drivers/hid/hid-logitech-dj.c | 40 ++++++++++++++++++++---------------
 2 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index c810c7a28bdd..108081ab5ae3 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -788,6 +788,7 @@
 #define USB_DEVICE_ID_LOGITECH_27MHZ_MOUSE_RECEIVER	0xc51b
 #define USB_DEVICE_ID_LOGITECH_UNIFYING_RECEIVER	0xc52b
 #define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER		0xc52f
+#define USB_DEVICE_ID_LOGITECH_G700_RECEIVER		0xc531
 #define USB_DEVICE_ID_LOGITECH_UNIFYING_RECEIVER_2	0xc532
 #define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_2		0xc534
 #define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1	0xc539
@@ -798,6 +799,10 @@
 #define USB_DEVICE_ID_DINOVO_DESKTOP	0xc704
 #define USB_DEVICE_ID_DINOVO_EDGE	0xc714
 #define USB_DEVICE_ID_DINOVO_MINI	0xc71f
+#define USB_DEVICE_ID_MX5000_RECEIVER_MOUSE_DEV		0xc70a
+#define USB_DEVICE_ID_MX5000_RECEIVER_KBD_DEV		0xc70e
+#define USB_DEVICE_ID_MX5500_RECEIVER_KBD_DEV		0xc71b
+#define USB_DEVICE_ID_MX5500_RECEIVER_MOUSE_DEV		0xc71c
 #define USB_DEVICE_ID_LOGITECH_MOMO_WHEEL2	0xca03
 #define USB_DEVICE_ID_LOGITECH_VIBRATION_WHEEL	0xca04
 
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 4976ed827b3e..eaaedbb84a8d 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -1857,39 +1857,44 @@ static void logi_dj_remove(struct hid_device *hdev)
 }
 
 static const struct hid_device_id logi_dj_receivers[] = {
-	{HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
+	{ /* Logitech unifying receiver (0xc52b) */
+	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
 		USB_DEVICE_ID_LOGITECH_UNIFYING_RECEIVER),
 	 .driver_data = recvr_type_dj},
-	{HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
+	{ /* Logitech unifying receiver (0xc532) */
+	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
 		USB_DEVICE_ID_LOGITECH_UNIFYING_RECEIVER_2),
 	 .driver_data = recvr_type_dj},
-	{ /* Logitech Nano mouse only receiver */
+
+	{ /* Logitech Nano mouse only receiver (0xc52f) */
 	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
 			 USB_DEVICE_ID_LOGITECH_NANO_RECEIVER),
 	 .driver_data = recvr_type_mouse_only},
-	{ /* Logitech Nano (non DJ) receiver */
+	{ /* Logitech Nano (non DJ) receiver (0xc534) */
 	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
 			 USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_2),
 	 .driver_data = recvr_type_hidpp},
+
 	{ /* Logitech G700(s) receiver (0xc531) */
 	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
-		0xc531),
+			 USB_DEVICE_ID_LOGITECH_G700_RECEIVER),
 	 .driver_data = recvr_type_gaming_hidpp},
 	{ /* Logitech lightspeed receiver (0xc539) */
 	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
 		USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1),
 	 .driver_data = recvr_type_gaming_hidpp},
+	{ /* Logitech powerplay receiver (0xc53a) */
+	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
+		USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_POWERPLAY),
+	 .driver_data = recvr_type_gaming_hidpp},
 	{ /* Logitech lightspeed receiver (0xc53f) */
 	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
 		USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1_1),
 	 .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),
 	 .driver_data = recvr_type_27mhz},
-	{ /* Logitech powerplay receiver (0xc53a) */
-	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
-		USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_POWERPLAY),
-	 .driver_data = recvr_type_gaming_hidpp},
 	{ /* Logitech 27 MHz HID++ 1.0 receiver (0xc517) */
 	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
 		USB_DEVICE_ID_S510_RECEIVER_2),
@@ -1898,21 +1903,22 @@ static const struct hid_device_id logi_dj_receivers[] = {
 	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
 		USB_DEVICE_ID_LOGITECH_27MHZ_MOUSE_RECEIVER),
 	 .driver_data = recvr_type_27mhz},
-	{ /* Logitech MX5000 HID++ / bluetooth receiver keyboard intf. */
+
+	{ /* Logitech MX5000 HID++ / bluetooth receiver keyboard intf. (0xc70e) */
 	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
-		0xc70e),
+		USB_DEVICE_ID_MX5000_RECEIVER_KBD_DEV),
 	 .driver_data = recvr_type_bluetooth},
-	{ /* Logitech MX5000 HID++ / bluetooth receiver mouse intf. */
+	{ /* Logitech MX5000 HID++ / bluetooth receiver mouse intf. (0xc70a) */
 	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
-		0xc70a),
+		USB_DEVICE_ID_MX5000_RECEIVER_MOUSE_DEV),
 	 .driver_data = recvr_type_bluetooth},
-	{ /* Logitech MX5500 HID++ / bluetooth receiver keyboard intf. */
+	{ /* Logitech MX5500 HID++ / bluetooth receiver keyboard intf. (0xc71b) */
 	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
-		0xc71b),
+		USB_DEVICE_ID_MX5500_RECEIVER_KBD_DEV),
 	 .driver_data = recvr_type_bluetooth},
-	{ /* Logitech MX5500 HID++ / bluetooth receiver mouse intf. */
+	{ /* Logitech MX5500 HID++ / bluetooth receiver mouse intf. (0xc71c) */
 	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
-		0xc71c),
+		USB_DEVICE_ID_MX5500_RECEIVER_MOUSE_DEV),
 	 .driver_data = recvr_type_bluetooth},
 	{}
 };
-- 
2.28.0

[RFC 3/3] HID: logitech-dj: Handle newer quad/bt2.0 receivers in HID proxy mode

[Hans de Goede]

The Dinovo Edge and Dinovo Mini keyboards with builtin touchpad come with
a different version of the quad/bt2.0 combo receivers shipped with the
MX5000 and MX5500 keyboards. These receivers are compatible with one
another, e.g. the Dinovo Edge keyboard can be paired with the MX5000
receiver.

Like the MX5x00 receivers in HID proxy mode these receivers present
themselves as a hub with multiple USB-HID devices, one for the keyboard
and one for the mouse.

Where they differ is that the mouse USB-device has 2 input reports for
reporting mice events. It has the exact same INPUT(2) report as the
MX5x00 receivers, but it also has a second INPUT(5) mouse report which
is different; and when the Dinovo receivers are paired with the Dinovo
keyboards the second INPUT(5) mouse report is actually used for events
on the builtin touchpad.

Add support for handling the Dinovo quad/bluetooth-2.0 combo receivers
in HID proxy mode to logitech-dj, like we already do for the similar
MX5000 and MX5500 receivers.

This adds battery monitoring functionality (through logitech-hidpp) and
fixes the Phone (Fn + F1) and "[A]" - "[D]" (Fn + F9 - F12) hotkeys not
working on the Dinovo Edge.

Note these receivers present themselves as a hub with 2 separate USB
devices for the keyboard and mouse; and the logitech-dj code needs to
bind to both devices (just as with the MX5x00 receivers).

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/hid/hid-ids.h         |  6 ++-
 drivers/hid/hid-lg.c          | 24 ---------
 drivers/hid/hid-logitech-dj.c | 91 +++++++++++++++++++++++++++++++++--
 drivers/hid/hid-quirks.c      |  2 -
 4 files changed, 92 insertions(+), 31 deletions(-)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 108081ab5ae3..e17f252ba60f 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -797,12 +797,14 @@
 #define USB_DEVICE_ID_SPACETRAVELLER	0xc623
 #define USB_DEVICE_ID_SPACENAVIGATOR	0xc626
 #define USB_DEVICE_ID_DINOVO_DESKTOP	0xc704
-#define USB_DEVICE_ID_DINOVO_EDGE	0xc714
-#define USB_DEVICE_ID_DINOVO_MINI	0xc71f
 #define USB_DEVICE_ID_MX5000_RECEIVER_MOUSE_DEV		0xc70a
 #define USB_DEVICE_ID_MX5000_RECEIVER_KBD_DEV		0xc70e
+#define USB_DEVICE_ID_DINOVO_EDGE_RECEIVER_KBD_DEV	0xc713
+#define USB_DEVICE_ID_DINOVO_EDGE_RECEIVER_MOUSE_DEV	0xc714
 #define USB_DEVICE_ID_MX5500_RECEIVER_KBD_DEV		0xc71b
 #define USB_DEVICE_ID_MX5500_RECEIVER_MOUSE_DEV		0xc71c
+#define USB_DEVICE_ID_DINOVO_MINI_RECEIVER_KBD_DEV	0xc71e
+#define USB_DEVICE_ID_DINOVO_MINI_RECEIVER_MOUSE_DEV	0xc71f
 #define USB_DEVICE_ID_LOGITECH_MOMO_WHEEL2	0xca03
 #define USB_DEVICE_ID_LOGITECH_VIBRATION_WHEEL	0xca04
 
diff --git a/drivers/hid/hid-lg.c b/drivers/hid/hid-lg.c
index 0dc7cdfc56f7..d40af911df63 100644
--- a/drivers/hid/hid-lg.c
+++ b/drivers/hid/hid-lg.c
@@ -568,22 +568,6 @@ static int lg_ultrax_remote_mapping(struct hid_input *hi,
 	return 1;
 }
 
-static int lg_dinovo_mapping(struct hid_input *hi, struct hid_usage *usage,
-		unsigned long **bit, int *max)
-{
-	if ((usage->hid & HID_USAGE_PAGE) != HID_UP_LOGIVENDOR)
-		return 0;
-
-	switch (usage->hid & HID_USAGE) {
-
-	case 0x00d: lg_map_key_clear(KEY_MEDIA);	break;
-	default:
-		return 0;
-
-	}
-	return 1;
-}
-
 static int lg_wireless_mapping(struct hid_input *hi, struct hid_usage *usage,
 		unsigned long **bit, int *max)
 {
@@ -668,10 +652,6 @@ static int lg_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 			lg_ultrax_remote_mapping(hi, usage, bit, max))
 		return 1;
 
-	if (hdev->product == USB_DEVICE_ID_DINOVO_MINI &&
-			lg_dinovo_mapping(hi, usage, bit, max))
-		return 1;
-
 	if ((drv_data->quirks & LG_WIRELESS) && lg_wireless_mapping(hi, usage, bit, max))
 		return 1;
 
@@ -879,10 +859,6 @@ static const struct hid_device_id lg_devices[] = {
 
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_DESKTOP),
 		.driver_data = LG_DUPLICATE_USAGES },
-	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_EDGE),
-		.driver_data = LG_DUPLICATE_USAGES },
-	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_MINI),
-		.driver_data = LG_DUPLICATE_USAGES },
 
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_ELITE_KBD),
 		.driver_data = LG_IGNORE_DOUBLED_WHEEL | LG_EXPANDED_KEYMAP },
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index eaaedbb84a8d..e9e294b66e59 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -84,6 +84,7 @@
 #define STD_MOUSE				BIT(2)
 #define MULTIMEDIA				BIT(3)
 #define POWER_KEYS				BIT(4)
+#define KBD_MOUSE				BIT(5)
 #define MEDIA_CENTER				BIT(8)
 #define KBD_LEDS				BIT(14)
 /* Fake (bitnr > NUMBER_OF_HID_REPORTS) bit to track HID++ capability */
@@ -118,6 +119,7 @@ enum recvr_type {
 	recvr_type_mouse_only,
 	recvr_type_27mhz,
 	recvr_type_bluetooth,
+	recvr_type_dinovo,
 };
 
 struct dj_report {
@@ -334,6 +336,47 @@ static const char mse_bluetooth_descriptor[] = {
 	0xC0,			/*  END_COLLECTION                      */
 };
 
+/* Mouse descriptor (5) for Bluetooth receiver, normal-res hwheel, 8 buttons */
+static const char mse5_bluetooth_descriptor[] = {
+	0x05, 0x01,		/*  USAGE_PAGE (Generic Desktop)        */
+	0x09, 0x02,		/*  Usage (Mouse)                       */
+	0xa1, 0x01,		/*  Collection (Application)            */
+	0x85, 0x05,		/*   Report ID (5)                      */
+	0x09, 0x01,		/*   Usage (Pointer)                    */
+	0xa1, 0x00,		/*   Collection (Physical)              */
+	0x05, 0x09,		/*    Usage Page (Button)               */
+	0x19, 0x01,		/*    Usage Minimum (1)                 */
+	0x29, 0x08,		/*    Usage Maximum (8)                 */
+	0x15, 0x00,		/*    Logical Minimum (0)               */
+	0x25, 0x01,		/*    Logical Maximum (1)               */
+	0x95, 0x08,		/*    Report Count (8)                  */
+	0x75, 0x01,		/*    Report Size (1)                   */
+	0x81, 0x02,		/*    Input (Data,Var,Abs)              */
+	0x05, 0x01,		/*    Usage Page (Generic Desktop)      */
+	0x16, 0x01, 0xf8,	/*    Logical Minimum (-2047)           */
+	0x26, 0xff, 0x07,	/*    Logical Maximum (2047)            */
+	0x75, 0x0c,		/*    Report Size (12)                  */
+	0x95, 0x02,		/*    Report Count (2)                  */
+	0x09, 0x30,		/*    Usage (X)                         */
+	0x09, 0x31,		/*    Usage (Y)                         */
+	0x81, 0x06,		/*    Input (Data,Var,Rel)              */
+	0x15, 0x81,		/*    Logical Minimum (-127)            */
+	0x25, 0x7f,		/*    Logical Maximum (127)             */
+	0x75, 0x08,		/*    Report Size (8)                   */
+	0x95, 0x01,		/*    Report Count (1)                  */
+	0x09, 0x38,		/*    Usage (Wheel)                     */
+	0x81, 0x06,		/*    Input (Data,Var,Rel)              */
+	0x05, 0x0c,		/*    Usage Page (Consumer Devices)     */
+	0x0a, 0x38, 0x02,	/*    Usage (AC Pan)                    */
+	0x15, 0x81,		/*    Logical Minimum (-127)            */
+	0x25, 0x7f,		/*    Logical Maximum (127)             */
+	0x75, 0x08,		/*    Report Size (8)                   */
+	0x95, 0x01,		/*    Report Count (1)                  */
+	0x81, 0x06,		/*    Input (Data,Var,Rel)              */
+	0xc0,			/*   End Collection                     */
+	0xc0,			/*  End Collection                      */
+};
+
 /* Gaming Mouse descriptor (2) */
 static const char mse_high_res_descriptor[] = {
 	0x05, 0x01,		/*  USAGE_PAGE (Generic Desktop)        */
@@ -481,6 +524,7 @@ static const char hidpp_descriptor[] = {
 #define MAX_RDESC_SIZE				\
 	(sizeof(kbd_descriptor) +		\
 	 sizeof(mse_bluetooth_descriptor) +	\
+	 sizeof(mse5_bluetooth_descriptor) +	\
 	 sizeof(consumer_descriptor) +		\
 	 sizeof(syscontrol_descriptor) +	\
 	 sizeof(media_descriptor) +	\
@@ -518,6 +562,11 @@ static void delayedwork_callback(struct work_struct *work);
 static LIST_HEAD(dj_hdev_list);
 static DEFINE_MUTEX(dj_hdev_list_lock);
 
+static bool recvr_type_is_bluetooth(enum recvr_type type)
+{
+	return type == recvr_type_bluetooth || type == recvr_type_dinovo;
+}
+
 /*
  * dj/HID++ receivers are really a single logical entity, but for BIOS/Windows
  * compatibility they have multiple USB interfaces. On HID++ receivers we need
@@ -535,7 +584,7 @@ static struct dj_receiver_dev *dj_find_receiver_dev(struct hid_device *hdev,
 	 * The bluetooth receiver contains a built-in hub and has separate
 	 * USB-devices for the keyboard and mouse interfaces.
 	 */
-	sep = (type == recvr_type_bluetooth) ? '.' : '/';
+	sep = recvr_type_is_bluetooth(type) ? '.' : '/';
 
 	/* Try to find an already-probed interface from the same device */
 	list_for_each_entry(djrcv_dev, &dj_hdev_list, list) {
@@ -873,6 +922,14 @@ static void logi_dj_recv_queue_notification(struct dj_receiver_dev *djrcv_dev,
  * touchpad to work we must also forward mouse input reports to the dj_hiddev
  * created for the keyboard (instead of forwarding them to a second paired
  * device with a device_type of REPORT_TYPE_MOUSE as we normally would).
+ *
+ * On Dinovo receivers the keyboard's touchpad and an optional paired actual
+ * mouse send separate input reports, INPUT(2) aka STD_MOUSE for the mouse
+ * and INPUT(5) aka KBD_MOUSE for the keyboard's touchpad.
+ *
+ * On MX5x00 receivers (which can also be paired with a Dinovo keyboard)
+ * INPUT(2) is used for both an optional paired actual mouse and for the
+ * keyboard's touchpad.
  */
 static const u16 kbd_builtin_touchpad_ids[] = {
 	0xb309, /* Dinovo Edge */
@@ -899,7 +956,10 @@ static void logi_hidpp_dev_conn_notif_equad(struct hid_device *hdev,
 		id = (workitem->quad_id_msb << 8) | workitem->quad_id_lsb;
 		for (i = 0; i < ARRAY_SIZE(kbd_builtin_touchpad_ids); i++) {
 			if (id == kbd_builtin_touchpad_ids[i]) {
-				workitem->reports_supported |= STD_MOUSE;
+				if (djrcv_dev->type == recvr_type_dinovo)
+					workitem->reports_supported |= KBD_MOUSE;
+				else
+					workitem->reports_supported |= STD_MOUSE;
 				break;
 			}
 		}
@@ -1367,7 +1427,7 @@ static int logi_dj_ll_parse(struct hid_device *hid)
 		else if (djdev->dj_receiver_dev->type == recvr_type_27mhz)
 			rdcat(rdesc, &rsize, mse_27mhz_descriptor,
 			      sizeof(mse_27mhz_descriptor));
-		else if (djdev->dj_receiver_dev->type == recvr_type_bluetooth)
+		else if (recvr_type_is_bluetooth(djdev->dj_receiver_dev->type))
 			rdcat(rdesc, &rsize, mse_bluetooth_descriptor,
 			      sizeof(mse_bluetooth_descriptor));
 		else
@@ -1375,6 +1435,13 @@ static int logi_dj_ll_parse(struct hid_device *hid)
 			      sizeof(mse_descriptor));
 	}
 
+	if (djdev->reports_supported & KBD_MOUSE) {
+		dbg_hid("%s: sending a kbd-mouse descriptor, reports_supported: %llx\n",
+			__func__, djdev->reports_supported);
+		rdcat(rdesc, &rsize, mse5_bluetooth_descriptor,
+		      sizeof(mse5_bluetooth_descriptor));
+	}
+
 	if (djdev->reports_supported & MULTIMEDIA) {
 		dbg_hid("%s: sending a multimedia report descriptor: %llx\n",
 			__func__, djdev->reports_supported);
@@ -1692,6 +1759,7 @@ static int logi_dj_probe(struct hid_device *hdev,
 	case recvr_type_mouse_only:	no_dj_interfaces = 2; break;
 	case recvr_type_27mhz:		no_dj_interfaces = 2; break;
 	case recvr_type_bluetooth:	no_dj_interfaces = 2; break;
+	case recvr_type_dinovo:		no_dj_interfaces = 2; break;
 	}
 	if (hid_is_using_ll_driver(hdev, &usb_hid_driver)) {
 		intf = to_usb_interface(hdev->dev.parent);
@@ -1920,6 +1988,23 @@ static const struct hid_device_id logi_dj_receivers[] = {
 	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
 		USB_DEVICE_ID_MX5500_RECEIVER_MOUSE_DEV),
 	 .driver_data = recvr_type_bluetooth},
+
+	{ /* Logitech Dinovo Edge HID++ / bluetooth receiver keyboard intf. (0xc713) */
+	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
+		USB_DEVICE_ID_DINOVO_EDGE_RECEIVER_KBD_DEV),
+	 .driver_data = recvr_type_dinovo},
+	{ /* Logitech Dinovo Edge HID++ / bluetooth receiver mouse intf. (0xc714) */
+	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
+		USB_DEVICE_ID_DINOVO_EDGE_RECEIVER_MOUSE_DEV),
+	 .driver_data = recvr_type_dinovo},
+	{ /* Logitech DiNovo Mini HID++ / bluetooth receiver mouse intf. (0xc71e) */
+	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
+		USB_DEVICE_ID_DINOVO_MINI_RECEIVER_KBD_DEV),
+	 .driver_data = recvr_type_dinovo},
+	{ /* Logitech DiNovo Mini HID++ / bluetooth receiver keyboard intf. (0xc71f) */
+	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
+		USB_DEVICE_ID_DINOVO_MINI_RECEIVER_MOUSE_DEV),
+	 .driver_data = recvr_type_dinovo},
 	{}
 };
 
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 7a2be0205dfd..8fce238f120d 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -439,8 +439,6 @@ static const struct hid_device_id hid_have_special_driver[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RECEIVER) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_DESKTOP) },
-	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_EDGE) },
-	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_MINI) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_ELITE_KBD) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_CORDLESS_DESKTOP_LX500) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_EXTREME_3D) },
-- 
2.28.0

Re: [RFC 3/3] HID: logitech-dj: Handle newer quad/bt2.0 receivers in HID proxy mode

[Benjamin Tissoires]

Hi Hans,

Not  full review but...

On Sat, Nov 14, 2020 at 10:21 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> The Dinovo Edge and Dinovo Mini keyboards with builtin touchpad come with
> a different version of the quad/bt2.0 combo receivers shipped with the
> MX5000 and MX5500 keyboards. These receivers are compatible with one
> another, e.g. the Dinovo Edge keyboard can be paired with the MX5000
> receiver.
>
> Like the MX5x00 receivers in HID proxy mode these receivers present
> themselves as a hub with multiple USB-HID devices, one for the keyboard
> and one for the mouse.
>
> Where they differ is that the mouse USB-device has 2 input reports for
> reporting mice events. It has the exact same INPUT(2) report as the
> MX5x00 receivers, but it also has a second INPUT(5) mouse report which
> is different; and when the Dinovo receivers are paired with the Dinovo
> keyboards the second INPUT(5) mouse report is actually used for events
> on the builtin touchpad.
>
> Add support for handling the Dinovo quad/bluetooth-2.0 combo receivers
> in HID proxy mode to logitech-dj, like we already do for the similar
> MX5000 and MX5500 receivers.
>
> This adds battery monitoring functionality (through logitech-hidpp) and
> fixes the Phone (Fn + F1) and "[A]" - "[D]" (Fn + F9 - F12) hotkeys not
> working on the Dinovo Edge.
>
> Note these receivers present themselves as a hub with 2 separate USB
> devices for the keyboard and mouse; and the logitech-dj code needs to
> bind to both devices (just as with the MX5x00 receivers).
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/hid/hid-ids.h         |  6 ++-
>  drivers/hid/hid-lg.c          | 24 ---------
>  drivers/hid/hid-logitech-dj.c | 91 +++++++++++++++++++++++++++++++++--
>  drivers/hid/hid-quirks.c      |  2 -
>  4 files changed, 92 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 108081ab5ae3..e17f252ba60f 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -797,12 +797,14 @@
>  #define USB_DEVICE_ID_SPACETRAVELLER   0xc623
>  #define USB_DEVICE_ID_SPACENAVIGATOR   0xc626
>  #define USB_DEVICE_ID_DINOVO_DESKTOP   0xc704
> -#define USB_DEVICE_ID_DINOVO_EDGE      0xc714
> -#define USB_DEVICE_ID_DINOVO_MINI      0xc71f
>  #define USB_DEVICE_ID_MX5000_RECEIVER_MOUSE_DEV                0xc70a
>  #define USB_DEVICE_ID_MX5000_RECEIVER_KBD_DEV          0xc70e
> +#define USB_DEVICE_ID_DINOVO_EDGE_RECEIVER_KBD_DEV     0xc713
> +#define USB_DEVICE_ID_DINOVO_EDGE_RECEIVER_MOUSE_DEV   0xc714
>  #define USB_DEVICE_ID_MX5500_RECEIVER_KBD_DEV          0xc71b
>  #define USB_DEVICE_ID_MX5500_RECEIVER_MOUSE_DEV                0xc71c
> +#define USB_DEVICE_ID_DINOVO_MINI_RECEIVER_KBD_DEV     0xc71e
> +#define USB_DEVICE_ID_DINOVO_MINI_RECEIVER_MOUSE_DEV   0xc71f
>  #define USB_DEVICE_ID_LOGITECH_MOMO_WHEEL2     0xca03
>  #define USB_DEVICE_ID_LOGITECH_VIBRATION_WHEEL 0xca04
>
> diff --git a/drivers/hid/hid-lg.c b/drivers/hid/hid-lg.c
> index 0dc7cdfc56f7..d40af911df63 100644
> --- a/drivers/hid/hid-lg.c
> +++ b/drivers/hid/hid-lg.c
> @@ -568,22 +568,6 @@ static int lg_ultrax_remote_mapping(struct hid_input *hi,
>         return 1;
>  }
>
> -static int lg_dinovo_mapping(struct hid_input *hi, struct hid_usage *usage,
> -               unsigned long **bit, int *max)
> -{
> -       if ((usage->hid & HID_USAGE_PAGE) != HID_UP_LOGIVENDOR)
> -               return 0;
> -
> -       switch (usage->hid & HID_USAGE) {
> -
> -       case 0x00d: lg_map_key_clear(KEY_MEDIA);        break;
> -       default:
> -               return 0;
> -
> -       }
> -       return 1;
> -}
> -

I am not sure these changes on hid-logitech-hidpp.c are something you
wanted to put in. Looks like the patch reverts the Dinovo Mini in 1/3.

Cheers,
Benjamin

>  static int lg_wireless_mapping(struct hid_input *hi, struct hid_usage *usage,
>                 unsigned long **bit, int *max)
>  {
> @@ -668,10 +652,6 @@ static int lg_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>                         lg_ultrax_remote_mapping(hi, usage, bit, max))
>                 return 1;
>
> -       if (hdev->product == USB_DEVICE_ID_DINOVO_MINI &&
> -                       lg_dinovo_mapping(hi, usage, bit, max))
> -               return 1;
> -
>         if ((drv_data->quirks & LG_WIRELESS) && lg_wireless_mapping(hi, usage, bit, max))
>                 return 1;
>
> @@ -879,10 +859,6 @@ static const struct hid_device_id lg_devices[] = {
>
>         { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_DESKTOP),
>                 .driver_data = LG_DUPLICATE_USAGES },
> -       { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_EDGE),
> -               .driver_data = LG_DUPLICATE_USAGES },
> -       { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_MINI),
> -               .driver_data = LG_DUPLICATE_USAGES },
>
>         { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_ELITE_KBD),
>                 .driver_data = LG_IGNORE_DOUBLED_WHEEL | LG_EXPANDED_KEYMAP },
> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
> index eaaedbb84a8d..e9e294b66e59 100644
> --- a/drivers/hid/hid-logitech-dj.c
> +++ b/drivers/hid/hid-logitech-dj.c
> @@ -84,6 +84,7 @@
>  #define STD_MOUSE                              BIT(2)
>  #define MULTIMEDIA                             BIT(3)
>  #define POWER_KEYS                             BIT(4)
> +#define KBD_MOUSE                              BIT(5)
>  #define MEDIA_CENTER                           BIT(8)
>  #define KBD_LEDS                               BIT(14)
>  /* Fake (bitnr > NUMBER_OF_HID_REPORTS) bit to track HID++ capability */
> @@ -118,6 +119,7 @@ enum recvr_type {
>         recvr_type_mouse_only,
>         recvr_type_27mhz,
>         recvr_type_bluetooth,
> +       recvr_type_dinovo,
>  };
>
>  struct dj_report {
> @@ -334,6 +336,47 @@ static const char mse_bluetooth_descriptor[] = {
>         0xC0,                   /*  END_COLLECTION                      */
>  };
>
> +/* Mouse descriptor (5) for Bluetooth receiver, normal-res hwheel, 8 buttons */
> +static const char mse5_bluetooth_descriptor[] = {
> +       0x05, 0x01,             /*  USAGE_PAGE (Generic Desktop)        */
> +       0x09, 0x02,             /*  Usage (Mouse)                       */
> +       0xa1, 0x01,             /*  Collection (Application)            */
> +       0x85, 0x05,             /*   Report ID (5)                      */
> +       0x09, 0x01,             /*   Usage (Pointer)                    */
> +       0xa1, 0x00,             /*   Collection (Physical)              */
> +       0x05, 0x09,             /*    Usage Page (Button)               */
> +       0x19, 0x01,             /*    Usage Minimum (1)                 */
> +       0x29, 0x08,             /*    Usage Maximum (8)                 */
> +       0x15, 0x00,             /*    Logical Minimum (0)               */
> +       0x25, 0x01,             /*    Logical Maximum (1)               */
> +       0x95, 0x08,             /*    Report Count (8)                  */
> +       0x75, 0x01,             /*    Report Size (1)                   */
> +       0x81, 0x02,             /*    Input (Data,Var,Abs)              */
> +       0x05, 0x01,             /*    Usage Page (Generic Desktop)      */
> +       0x16, 0x01, 0xf8,       /*    Logical Minimum (-2047)           */
> +       0x26, 0xff, 0x07,       /*    Logical Maximum (2047)            */
> +       0x75, 0x0c,             /*    Report Size (12)                  */
> +       0x95, 0x02,             /*    Report Count (2)                  */
> +       0x09, 0x30,             /*    Usage (X)                         */
> +       0x09, 0x31,             /*    Usage (Y)                         */
> +       0x81, 0x06,             /*    Input (Data,Var,Rel)              */
> +       0x15, 0x81,             /*    Logical Minimum (-127)            */
> +       0x25, 0x7f,             /*    Logical Maximum (127)             */
> +       0x75, 0x08,             /*    Report Size (8)                   */
> +       0x95, 0x01,             /*    Report Count (1)                  */
> +       0x09, 0x38,             /*    Usage (Wheel)                     */
> +       0x81, 0x06,             /*    Input (Data,Var,Rel)              */
> +       0x05, 0x0c,             /*    Usage Page (Consumer Devices)     */
> +       0x0a, 0x38, 0x02,       /*    Usage (AC Pan)                    */
> +       0x15, 0x81,             /*    Logical Minimum (-127)            */
> +       0x25, 0x7f,             /*    Logical Maximum (127)             */
> +       0x75, 0x08,             /*    Report Size (8)                   */
> +       0x95, 0x01,             /*    Report Count (1)                  */
> +       0x81, 0x06,             /*    Input (Data,Var,Rel)              */
> +       0xc0,                   /*   End Collection                     */
> +       0xc0,                   /*  End Collection                      */
> +};
> +
>  /* Gaming Mouse descriptor (2) */
>  static const char mse_high_res_descriptor[] = {
>         0x05, 0x01,             /*  USAGE_PAGE (Generic Desktop)        */
> @@ -481,6 +524,7 @@ static const char hidpp_descriptor[] = {
>  #define MAX_RDESC_SIZE                         \
>         (sizeof(kbd_descriptor) +               \
>          sizeof(mse_bluetooth_descriptor) +     \
> +        sizeof(mse5_bluetooth_descriptor) +    \
>          sizeof(consumer_descriptor) +          \
>          sizeof(syscontrol_descriptor) +        \
>          sizeof(media_descriptor) +     \
> @@ -518,6 +562,11 @@ static void delayedwork_callback(struct work_struct *work);
>  static LIST_HEAD(dj_hdev_list);
>  static DEFINE_MUTEX(dj_hdev_list_lock);
>
> +static bool recvr_type_is_bluetooth(enum recvr_type type)
> +{
> +       return type == recvr_type_bluetooth || type == recvr_type_dinovo;
> +}
> +
>  /*
>   * dj/HID++ receivers are really a single logical entity, but for BIOS/Windows
>   * compatibility they have multiple USB interfaces. On HID++ receivers we need
> @@ -535,7 +584,7 @@ static struct dj_receiver_dev *dj_find_receiver_dev(struct hid_device *hdev,
>          * The bluetooth receiver contains a built-in hub and has separate
>          * USB-devices for the keyboard and mouse interfaces.
>          */
> -       sep = (type == recvr_type_bluetooth) ? '.' : '/';
> +       sep = recvr_type_is_bluetooth(type) ? '.' : '/';
>
>         /* Try to find an already-probed interface from the same device */
>         list_for_each_entry(djrcv_dev, &dj_hdev_list, list) {
> @@ -873,6 +922,14 @@ static void logi_dj_recv_queue_notification(struct dj_receiver_dev *djrcv_dev,
>   * touchpad to work we must also forward mouse input reports to the dj_hiddev
>   * created for the keyboard (instead of forwarding them to a second paired
>   * device with a device_type of REPORT_TYPE_MOUSE as we normally would).
> + *
> + * On Dinovo receivers the keyboard's touchpad and an optional paired actual
> + * mouse send separate input reports, INPUT(2) aka STD_MOUSE for the mouse
> + * and INPUT(5) aka KBD_MOUSE for the keyboard's touchpad.
> + *
> + * On MX5x00 receivers (which can also be paired with a Dinovo keyboard)
> + * INPUT(2) is used for both an optional paired actual mouse and for the
> + * keyboard's touchpad.
>   */
>  static const u16 kbd_builtin_touchpad_ids[] = {
>         0xb309, /* Dinovo Edge */
> @@ -899,7 +956,10 @@ static void logi_hidpp_dev_conn_notif_equad(struct hid_device *hdev,
>                 id = (workitem->quad_id_msb << 8) | workitem->quad_id_lsb;
>                 for (i = 0; i < ARRAY_SIZE(kbd_builtin_touchpad_ids); i++) {
>                         if (id == kbd_builtin_touchpad_ids[i]) {
> -                               workitem->reports_supported |= STD_MOUSE;
> +                               if (djrcv_dev->type == recvr_type_dinovo)
> +                                       workitem->reports_supported |= KBD_MOUSE;
> +                               else
> +                                       workitem->reports_supported |= STD_MOUSE;
>                                 break;
>                         }
>                 }
> @@ -1367,7 +1427,7 @@ static int logi_dj_ll_parse(struct hid_device *hid)
>                 else if (djdev->dj_receiver_dev->type == recvr_type_27mhz)
>                         rdcat(rdesc, &rsize, mse_27mhz_descriptor,
>                               sizeof(mse_27mhz_descriptor));
> -               else if (djdev->dj_receiver_dev->type == recvr_type_bluetooth)
> +               else if (recvr_type_is_bluetooth(djdev->dj_receiver_dev->type))
>                         rdcat(rdesc, &rsize, mse_bluetooth_descriptor,
>                               sizeof(mse_bluetooth_descriptor));
>                 else
> @@ -1375,6 +1435,13 @@ static int logi_dj_ll_parse(struct hid_device *hid)
>                               sizeof(mse_descriptor));
>         }
>
> +       if (djdev->reports_supported & KBD_MOUSE) {
> +               dbg_hid("%s: sending a kbd-mouse descriptor, reports_supported: %llx\n",
> +                       __func__, djdev->reports_supported);
> +               rdcat(rdesc, &rsize, mse5_bluetooth_descriptor,
> +                     sizeof(mse5_bluetooth_descriptor));
> +       }
> +
>         if (djdev->reports_supported & MULTIMEDIA) {
>                 dbg_hid("%s: sending a multimedia report descriptor: %llx\n",
>                         __func__, djdev->reports_supported);
> @@ -1692,6 +1759,7 @@ static int logi_dj_probe(struct hid_device *hdev,
>         case recvr_type_mouse_only:     no_dj_interfaces = 2; break;
>         case recvr_type_27mhz:          no_dj_interfaces = 2; break;
>         case recvr_type_bluetooth:      no_dj_interfaces = 2; break;
> +       case recvr_type_dinovo:         no_dj_interfaces = 2; break;
>         }
>         if (hid_is_using_ll_driver(hdev, &usb_hid_driver)) {
>                 intf = to_usb_interface(hdev->dev.parent);
> @@ -1920,6 +1988,23 @@ static const struct hid_device_id logi_dj_receivers[] = {
>           HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
>                 USB_DEVICE_ID_MX5500_RECEIVER_MOUSE_DEV),
>          .driver_data = recvr_type_bluetooth},
> +
> +       { /* Logitech Dinovo Edge HID++ / bluetooth receiver keyboard intf. (0xc713) */
> +         HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
> +               USB_DEVICE_ID_DINOVO_EDGE_RECEIVER_KBD_DEV),
> +        .driver_data = recvr_type_dinovo},
> +       { /* Logitech Dinovo Edge HID++ / bluetooth receiver mouse intf. (0xc714) */
> +         HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
> +               USB_DEVICE_ID_DINOVO_EDGE_RECEIVER_MOUSE_DEV),
> +        .driver_data = recvr_type_dinovo},
> +       { /* Logitech DiNovo Mini HID++ / bluetooth receiver mouse intf. (0xc71e) */
> +         HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
> +               USB_DEVICE_ID_DINOVO_MINI_RECEIVER_KBD_DEV),
> +        .driver_data = recvr_type_dinovo},
> +       { /* Logitech DiNovo Mini HID++ / bluetooth receiver keyboard intf. (0xc71f) */
> +         HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
> +               USB_DEVICE_ID_DINOVO_MINI_RECEIVER_MOUSE_DEV),
> +        .driver_data = recvr_type_dinovo},
>         {}
>  };
>
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index 7a2be0205dfd..8fce238f120d 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -439,8 +439,6 @@ static const struct hid_device_id hid_have_special_driver[] = {
>         { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER) },
>         { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RECEIVER) },
>         { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_DESKTOP) },
> -       { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_EDGE) },
> -       { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_MINI) },
>         { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_ELITE_KBD) },
>         { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_CORDLESS_DESKTOP_LX500) },
>         { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_EXTREME_3D) },
> --
> 2.28.0
>

Re: [RFC 3/3] HID: logitech-dj: Handle newer quad/bt2.0 receivers in HID proxy mode

[Hans de Goede]

Hi,

On 11/16/20 9:30 AM, Benjamin Tissoires wrote:
> Hi Hans,
> 
> Not  full review but...
> 
> On Sat, Nov 14, 2020 at 10:21 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> The Dinovo Edge and Dinovo Mini keyboards with builtin touchpad come with
>> a different version of the quad/bt2.0 combo receivers shipped with the
>> MX5000 and MX5500 keyboards. These receivers are compatible with one
>> another, e.g. the Dinovo Edge keyboard can be paired with the MX5000
>> receiver.
>>
>> Like the MX5x00 receivers in HID proxy mode these receivers present
>> themselves as a hub with multiple USB-HID devices, one for the keyboard
>> and one for the mouse.
>>
>> Where they differ is that the mouse USB-device has 2 input reports for
>> reporting mice events. It has the exact same INPUT(2) report as the
>> MX5x00 receivers, but it also has a second INPUT(5) mouse report which
>> is different; and when the Dinovo receivers are paired with the Dinovo
>> keyboards the second INPUT(5) mouse report is actually used for events
>> on the builtin touchpad.
>>
>> Add support for handling the Dinovo quad/bluetooth-2.0 combo receivers
>> in HID proxy mode to logitech-dj, like we already do for the similar
>> MX5000 and MX5500 receivers.
>>
>> This adds battery monitoring functionality (through logitech-hidpp) and
>> fixes the Phone (Fn + F1) and "[A]" - "[D]" (Fn + F9 - F12) hotkeys not
>> working on the Dinovo Edge.
>>
>> Note these receivers present themselves as a hub with 2 separate USB
>> devices for the keyboard and mouse; and the logitech-dj code needs to
>> bind to both devices (just as with the MX5x00 receivers).
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/hid/hid-ids.h         |  6 ++-
>>  drivers/hid/hid-lg.c          | 24 ---------
>>  drivers/hid/hid-logitech-dj.c | 91 +++++++++++++++++++++++++++++++++--
>>  drivers/hid/hid-quirks.c      |  2 -
>>  4 files changed, 92 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
>> index 108081ab5ae3..e17f252ba60f 100644
>> --- a/drivers/hid/hid-ids.h
>> +++ b/drivers/hid/hid-ids.h
>> @@ -797,12 +797,14 @@
>>  #define USB_DEVICE_ID_SPACETRAVELLER   0xc623
>>  #define USB_DEVICE_ID_SPACENAVIGATOR   0xc626
>>  #define USB_DEVICE_ID_DINOVO_DESKTOP   0xc704
>> -#define USB_DEVICE_ID_DINOVO_EDGE      0xc714
>> -#define USB_DEVICE_ID_DINOVO_MINI      0xc71f
>>  #define USB_DEVICE_ID_MX5000_RECEIVER_MOUSE_DEV                0xc70a
>>  #define USB_DEVICE_ID_MX5000_RECEIVER_KBD_DEV          0xc70e
>> +#define USB_DEVICE_ID_DINOVO_EDGE_RECEIVER_KBD_DEV     0xc713
>> +#define USB_DEVICE_ID_DINOVO_EDGE_RECEIVER_MOUSE_DEV   0xc714
>>  #define USB_DEVICE_ID_MX5500_RECEIVER_KBD_DEV          0xc71b
>>  #define USB_DEVICE_ID_MX5500_RECEIVER_MOUSE_DEV                0xc71c
>> +#define USB_DEVICE_ID_DINOVO_MINI_RECEIVER_KBD_DEV     0xc71e
>> +#define USB_DEVICE_ID_DINOVO_MINI_RECEIVER_MOUSE_DEV   0xc71f
>>  #define USB_DEVICE_ID_LOGITECH_MOMO_WHEEL2     0xca03
>>  #define USB_DEVICE_ID_LOGITECH_VIBRATION_WHEEL 0xca04
>>
>> diff --git a/drivers/hid/hid-lg.c b/drivers/hid/hid-lg.c
>> index 0dc7cdfc56f7..d40af911df63 100644
>> --- a/drivers/hid/hid-lg.c
>> +++ b/drivers/hid/hid-lg.c
>> @@ -568,22 +568,6 @@ static int lg_ultrax_remote_mapping(struct hid_input *hi,
>>         return 1;
>>  }
>>
>> -static int lg_dinovo_mapping(struct hid_input *hi, struct hid_usage *usage,
>> -               unsigned long **bit, int *max)
>> -{
>> -       if ((usage->hid & HID_USAGE_PAGE) != HID_UP_LOGIVENDOR)
>> -               return 0;
>> -
>> -       switch (usage->hid & HID_USAGE) {
>> -
>> -       case 0x00d: lg_map_key_clear(KEY_MEDIA);        break;
>> -       default:
>> -               return 0;
>> -
>> -       }
>> -       return 1;
>> -}
>> -
> 
> I am not sure these changes on hid-logitech-hidpp.c

This is not a change to hid-logitech-hidpp.c, it it a change to hid-lg.c,
which after this patch will no longer handle the Dinovo Mini ever, since its
USB-ID is dropped from its id-table. Also note that the above code was
only run on an USB-ID match, so with the USB-ID dropped the hid-lg.c
copy of this is dead code (also see below).

> are something you
> wanted to put in. Looks like the patch reverts the Dinovo Mini in 1/3.

1/3 actually duplicates this bit from hid-lg.c to hid-logitech-hidpp.c
so that the Dinovo Mino Media key will work properly independent of it
being paired with a receiver handled by hid-lg.c or hid-logitech-dj.c.

This patch moves all receiver handling over to hid-logitech-dj.c, after
which the Dinovo Mini handling in hid-lg.c is just dead code.

I did things this way (with the temporary code duplication) so that 1/3
can go out as a fix for stable kernels while 2 + 3 are for 5.11.

Regards,

Hans





>>  static int lg_wireless_mapping(struct hid_input *hi, struct hid_usage *usage,
>>                 unsigned long **bit, int *max)
>>  {
>> @@ -668,10 +652,6 @@ static int lg_input_mapping(struct hid_device *hdev, struct hid_input *hi,
>>                         lg_ultrax_remote_mapping(hi, usage, bit, max))
>>                 return 1;
>>
>> -       if (hdev->product == USB_DEVICE_ID_DINOVO_MINI &&
>> -                       lg_dinovo_mapping(hi, usage, bit, max))
>> -               return 1;
>> -
>>         if ((drv_data->quirks & LG_WIRELESS) && lg_wireless_mapping(hi, usage, bit, max))
>>                 return 1;
>>
>> @@ -879,10 +859,6 @@ static const struct hid_device_id lg_devices[] = {
>>
>>         { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_DESKTOP),
>>                 .driver_data = LG_DUPLICATE_USAGES },
>> -       { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_EDGE),
>> -               .driver_data = LG_DUPLICATE_USAGES },
>> -       { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_MINI),
>> -               .driver_data = LG_DUPLICATE_USAGES },
>>
>>         { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_ELITE_KBD),
>>                 .driver_data = LG_IGNORE_DOUBLED_WHEEL | LG_EXPANDED_KEYMAP },
>> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
>> index eaaedbb84a8d..e9e294b66e59 100644
>> --- a/drivers/hid/hid-logitech-dj.c
>> +++ b/drivers/hid/hid-logitech-dj.c
>> @@ -84,6 +84,7 @@
>>  #define STD_MOUSE                              BIT(2)
>>  #define MULTIMEDIA                             BIT(3)
>>  #define POWER_KEYS                             BIT(4)
>> +#define KBD_MOUSE                              BIT(5)
>>  #define MEDIA_CENTER                           BIT(8)
>>  #define KBD_LEDS                               BIT(14)
>>  /* Fake (bitnr > NUMBER_OF_HID_REPORTS) bit to track HID++ capability */
>> @@ -118,6 +119,7 @@ enum recvr_type {
>>         recvr_type_mouse_only,
>>         recvr_type_27mhz,
>>         recvr_type_bluetooth,
>> +       recvr_type_dinovo,
>>  };
>>
>>  struct dj_report {
>> @@ -334,6 +336,47 @@ static const char mse_bluetooth_descriptor[] = {
>>         0xC0,                   /*  END_COLLECTION                      */
>>  };
>>
>> +/* Mouse descriptor (5) for Bluetooth receiver, normal-res hwheel, 8 buttons */
>> +static const char mse5_bluetooth_descriptor[] = {
>> +       0x05, 0x01,             /*  USAGE_PAGE (Generic Desktop)        */
>> +       0x09, 0x02,             /*  Usage (Mouse)                       */
>> +       0xa1, 0x01,             /*  Collection (Application)            */
>> +       0x85, 0x05,             /*   Report ID (5)                      */
>> +       0x09, 0x01,             /*   Usage (Pointer)                    */
>> +       0xa1, 0x00,             /*   Collection (Physical)              */
>> +       0x05, 0x09,             /*    Usage Page (Button)               */
>> +       0x19, 0x01,             /*    Usage Minimum (1)                 */
>> +       0x29, 0x08,             /*    Usage Maximum (8)                 */
>> +       0x15, 0x00,             /*    Logical Minimum (0)               */
>> +       0x25, 0x01,             /*    Logical Maximum (1)               */
>> +       0x95, 0x08,             /*    Report Count (8)                  */
>> +       0x75, 0x01,             /*    Report Size (1)                   */
>> +       0x81, 0x02,             /*    Input (Data,Var,Abs)              */
>> +       0x05, 0x01,             /*    Usage Page (Generic Desktop)      */
>> +       0x16, 0x01, 0xf8,       /*    Logical Minimum (-2047)           */
>> +       0x26, 0xff, 0x07,       /*    Logical Maximum (2047)            */
>> +       0x75, 0x0c,             /*    Report Size (12)                  */
>> +       0x95, 0x02,             /*    Report Count (2)                  */
>> +       0x09, 0x30,             /*    Usage (X)                         */
>> +       0x09, 0x31,             /*    Usage (Y)                         */
>> +       0x81, 0x06,             /*    Input (Data,Var,Rel)              */
>> +       0x15, 0x81,             /*    Logical Minimum (-127)            */
>> +       0x25, 0x7f,             /*    Logical Maximum (127)             */
>> +       0x75, 0x08,             /*    Report Size (8)                   */
>> +       0x95, 0x01,             /*    Report Count (1)                  */
>> +       0x09, 0x38,             /*    Usage (Wheel)                     */
>> +       0x81, 0x06,             /*    Input (Data,Var,Rel)              */
>> +       0x05, 0x0c,             /*    Usage Page (Consumer Devices)     */
>> +       0x0a, 0x38, 0x02,       /*    Usage (AC Pan)                    */
>> +       0x15, 0x81,             /*    Logical Minimum (-127)            */
>> +       0x25, 0x7f,             /*    Logical Maximum (127)             */
>> +       0x75, 0x08,             /*    Report Size (8)                   */
>> +       0x95, 0x01,             /*    Report Count (1)                  */
>> +       0x81, 0x06,             /*    Input (Data,Var,Rel)              */
>> +       0xc0,                   /*   End Collection                     */
>> +       0xc0,                   /*  End Collection                      */
>> +};
>> +
>>  /* Gaming Mouse descriptor (2) */
>>  static const char mse_high_res_descriptor[] = {
>>         0x05, 0x01,             /*  USAGE_PAGE (Generic Desktop)        */
>> @@ -481,6 +524,7 @@ static const char hidpp_descriptor[] = {
>>  #define MAX_RDESC_SIZE                         \
>>         (sizeof(kbd_descriptor) +               \
>>          sizeof(mse_bluetooth_descriptor) +     \
>> +        sizeof(mse5_bluetooth_descriptor) +    \
>>          sizeof(consumer_descriptor) +          \
>>          sizeof(syscontrol_descriptor) +        \
>>          sizeof(media_descriptor) +     \
>> @@ -518,6 +562,11 @@ static void delayedwork_callback(struct work_struct *work);
>>  static LIST_HEAD(dj_hdev_list);
>>  static DEFINE_MUTEX(dj_hdev_list_lock);
>>
>> +static bool recvr_type_is_bluetooth(enum recvr_type type)
>> +{
>> +       return type == recvr_type_bluetooth || type == recvr_type_dinovo;
>> +}
>> +
>>  /*
>>   * dj/HID++ receivers are really a single logical entity, but for BIOS/Windows
>>   * compatibility they have multiple USB interfaces. On HID++ receivers we need
>> @@ -535,7 +584,7 @@ static struct dj_receiver_dev *dj_find_receiver_dev(struct hid_device *hdev,
>>          * The bluetooth receiver contains a built-in hub and has separate
>>          * USB-devices for the keyboard and mouse interfaces.
>>          */
>> -       sep = (type == recvr_type_bluetooth) ? '.' : '/';
>> +       sep = recvr_type_is_bluetooth(type) ? '.' : '/';
>>
>>         /* Try to find an already-probed interface from the same device */
>>         list_for_each_entry(djrcv_dev, &dj_hdev_list, list) {
>> @@ -873,6 +922,14 @@ static void logi_dj_recv_queue_notification(struct dj_receiver_dev *djrcv_dev,
>>   * touchpad to work we must also forward mouse input reports to the dj_hiddev
>>   * created for the keyboard (instead of forwarding them to a second paired
>>   * device with a device_type of REPORT_TYPE_MOUSE as we normally would).
>> + *
>> + * On Dinovo receivers the keyboard's touchpad and an optional paired actual
>> + * mouse send separate input reports, INPUT(2) aka STD_MOUSE for the mouse
>> + * and INPUT(5) aka KBD_MOUSE for the keyboard's touchpad.
>> + *
>> + * On MX5x00 receivers (which can also be paired with a Dinovo keyboard)
>> + * INPUT(2) is used for both an optional paired actual mouse and for the
>> + * keyboard's touchpad.
>>   */
>>  static const u16 kbd_builtin_touchpad_ids[] = {
>>         0xb309, /* Dinovo Edge */
>> @@ -899,7 +956,10 @@ static void logi_hidpp_dev_conn_notif_equad(struct hid_device *hdev,
>>                 id = (workitem->quad_id_msb << 8) | workitem->quad_id_lsb;
>>                 for (i = 0; i < ARRAY_SIZE(kbd_builtin_touchpad_ids); i++) {
>>                         if (id == kbd_builtin_touchpad_ids[i]) {
>> -                               workitem->reports_supported |= STD_MOUSE;
>> +                               if (djrcv_dev->type == recvr_type_dinovo)
>> +                                       workitem->reports_supported |= KBD_MOUSE;
>> +                               else
>> +                                       workitem->reports_supported |= STD_MOUSE;
>>                                 break;
>>                         }
>>                 }
>> @@ -1367,7 +1427,7 @@ static int logi_dj_ll_parse(struct hid_device *hid)
>>                 else if (djdev->dj_receiver_dev->type == recvr_type_27mhz)
>>                         rdcat(rdesc, &rsize, mse_27mhz_descriptor,
>>                               sizeof(mse_27mhz_descriptor));
>> -               else if (djdev->dj_receiver_dev->type == recvr_type_bluetooth)
>> +               else if (recvr_type_is_bluetooth(djdev->dj_receiver_dev->type))
>>                         rdcat(rdesc, &rsize, mse_bluetooth_descriptor,
>>                               sizeof(mse_bluetooth_descriptor));
>>                 else
>> @@ -1375,6 +1435,13 @@ static int logi_dj_ll_parse(struct hid_device *hid)
>>                               sizeof(mse_descriptor));
>>         }
>>
>> +       if (djdev->reports_supported & KBD_MOUSE) {
>> +               dbg_hid("%s: sending a kbd-mouse descriptor, reports_supported: %llx\n",
>> +                       __func__, djdev->reports_supported);
>> +               rdcat(rdesc, &rsize, mse5_bluetooth_descriptor,
>> +                     sizeof(mse5_bluetooth_descriptor));
>> +       }
>> +
>>         if (djdev->reports_supported & MULTIMEDIA) {
>>                 dbg_hid("%s: sending a multimedia report descriptor: %llx\n",
>>                         __func__, djdev->reports_supported);
>> @@ -1692,6 +1759,7 @@ static int logi_dj_probe(struct hid_device *hdev,
>>         case recvr_type_mouse_only:     no_dj_interfaces = 2; break;
>>         case recvr_type_27mhz:          no_dj_interfaces = 2; break;
>>         case recvr_type_bluetooth:      no_dj_interfaces = 2; break;
>> +       case recvr_type_dinovo:         no_dj_interfaces = 2; break;
>>         }
>>         if (hid_is_using_ll_driver(hdev, &usb_hid_driver)) {
>>                 intf = to_usb_interface(hdev->dev.parent);
>> @@ -1920,6 +1988,23 @@ static const struct hid_device_id logi_dj_receivers[] = {
>>           HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
>>                 USB_DEVICE_ID_MX5500_RECEIVER_MOUSE_DEV),
>>          .driver_data = recvr_type_bluetooth},
>> +
>> +       { /* Logitech Dinovo Edge HID++ / bluetooth receiver keyboard intf. (0xc713) */
>> +         HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
>> +               USB_DEVICE_ID_DINOVO_EDGE_RECEIVER_KBD_DEV),
>> +        .driver_data = recvr_type_dinovo},
>> +       { /* Logitech Dinovo Edge HID++ / bluetooth receiver mouse intf. (0xc714) */
>> +         HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
>> +               USB_DEVICE_ID_DINOVO_EDGE_RECEIVER_MOUSE_DEV),
>> +        .driver_data = recvr_type_dinovo},
>> +       { /* Logitech DiNovo Mini HID++ / bluetooth receiver mouse intf. (0xc71e) */
>> +         HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
>> +               USB_DEVICE_ID_DINOVO_MINI_RECEIVER_KBD_DEV),
>> +        .driver_data = recvr_type_dinovo},
>> +       { /* Logitech DiNovo Mini HID++ / bluetooth receiver keyboard intf. (0xc71f) */
>> +         HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
>> +               USB_DEVICE_ID_DINOVO_MINI_RECEIVER_MOUSE_DEV),
>> +        .driver_data = recvr_type_dinovo},
>>         {}
>>  };
>>
>> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
>> index 7a2be0205dfd..8fce238f120d 100644
>> --- a/drivers/hid/hid-quirks.c
>> +++ b/drivers/hid/hid-quirks.c
>> @@ -439,8 +439,6 @@ static const struct hid_device_id hid_have_special_driver[] = {
>>         { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER) },
>>         { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_RECEIVER) },
>>         { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_DESKTOP) },
>> -       { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_EDGE) },
>> -       { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_MINI) },
>>         { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_ELITE_KBD) },
>>         { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_CORDLESS_DESKTOP_LX500) },
>>         { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_EXTREME_3D) },
>> --
>> 2.28.0
>>
> 

Re: [RFC 0/3] HID: logitech-dj: Dinovo keyboard fixes and improvements

[Benjamin Tissoires]

Hi Hans,

On Sat, Nov 14, 2020 at 10:21 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi Benjamin,
>
> Here is my patch series for the discussed Dinovo keyboard (receiver)
> support improvements.
>
> I've marked this as a RFC since it has not been tested with a Dinovo Mini
> (nor a Dinovo Mini receiver) yet.
>
> I have tested it extensively with a Dinovo Edge, a MX5000 and a MX5500
> keyboard. In case of the Dinovo Edge and MX5000 I've not only tested
> them against their own receiver but also against each-others receiver.
>
> Once you have tested this series on your Dinovo Mini, it is ready to
> go upstream.

That part is now done, so I guess we can push it upstream :)

FTR, the dinovo mini still works fine with this series. I have a weird
issue where the secondary button gives me a left click, but according
to the raw logs, this is emitted from the hardware itself and is the
same whether I am on hid-logitech-dj or not.


> The first patch should probably go to 5.10 as a fix in
> case someone pairs the Dinovo Mini with a MX5x00 receiver like the
> reporter of this bug did with his Dinovo Edge:
> https://bugzilla.redhat.com/show_bug.cgi?id=1811424

OK, then I can apply it on top of the previous fix. I guess we don't
need stable@vger.k.o for this one?

>
> The other 2 are 5.11 material.

If I have to break the series, I will have to wait for Linus to first
merge the 5.10 material, then I'll be able to apply the others on
top...

Cheers,
Benjamin

>
> Regards,
>
> Hans
>
>

Re: [RFC 0/3] HID: logitech-dj: Dinovo keyboard fixes and improvements

[Hans de Goede]

Hi,

On 11/19/20 4:25 PM, Benjamin Tissoires wrote:
> Hi Hans,
> 
> On Sat, Nov 14, 2020 at 10:21 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi Benjamin,
>>
>> Here is my patch series for the discussed Dinovo keyboard (receiver)
>> support improvements.
>>
>> I've marked this as a RFC since it has not been tested with a Dinovo Mini
>> (nor a Dinovo Mini receiver) yet.
>>
>> I have tested it extensively with a Dinovo Edge, a MX5000 and a MX5500
>> keyboard. In case of the Dinovo Edge and MX5000 I've not only tested
>> them against their own receiver but also against each-others receiver.
>>
>> Once you have tested this series on your Dinovo Mini, it is ready to
>> go upstream.
> 
> That part is now done, so I guess we can push it upstream :)

Great thank you.

> FTR, the dinovo mini still works fine with this series. I have a weird
> issue where the secondary button gives me a left click, but according
> to the raw logs, this is emitted from the hardware itself and is the
> same whether I am on hid-logitech-dj or not.

A bit offtopic for this thread, but if it is a HID++ 1.0 device, then
you could try setting the HIDPP_QUIRK_HIDPP_EXTRA_MOUSE_BTNS quirk on
it and see if that helps. I've seen several cases with HID++ 1.0 devices
where some keyboard-keys / buttons would not report (or report wrongly)
unless the reporting of them was switched over from the regular HID
input report to the HID++ version of the report.

>> The first patch should probably go to 5.10 as a fix in
>> case someone pairs the Dinovo Mini with a MX5x00 receiver like the
>> reporter of this bug did with his Dinovo Edge:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1811424
> 
> OK, then I can apply it on top of the previous fix. I guess we don't
> need stable@vger.k.o for this one?

Actually this is intended for stable, to avoid getting a repeat of:
https://bugzilla.redhat.com/show_bug.cgi?id=1811424
with a Dinovo Mini. So if you can add a Cc: stable that would be
great.

>> The other 2 are 5.11 material.
> 
> If I have to break the series, I will have to wait for Linus to first
> merge the 5.10 material, then I'll be able to apply the others on
> top...

Ack, that works for me there is no rush to get the other 2 merged.

Regards,

Hans

Re: [RFC 0/3] HID: logitech-dj: Dinovo keyboard fixes and improvements

[Benjamin Tissoires]

On Thu, Nov 19, 2020 at 4:48 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 11/19/20 4:25 PM, Benjamin Tissoires wrote:
> > Hi Hans,
> >
> > On Sat, Nov 14, 2020 at 10:21 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >>
> >> Hi Benjamin,
> >>
> >> Here is my patch series for the discussed Dinovo keyboard (receiver)
> >> support improvements.
> >>
> >> I've marked this as a RFC since it has not been tested with a Dinovo Mini
> >> (nor a Dinovo Mini receiver) yet.
> >>
> >> I have tested it extensively with a Dinovo Edge, a MX5000 and a MX5500
> >> keyboard. In case of the Dinovo Edge and MX5000 I've not only tested
> >> them against their own receiver but also against each-others receiver.
> >>
> >> Once you have tested this series on your Dinovo Mini, it is ready to
> >> go upstream.
> >
> > That part is now done, so I guess we can push it upstream :)
>
> Great thank you.
>
> > FTR, the dinovo mini still works fine with this series. I have a weird
> > issue where the secondary button gives me a left click, but according
> > to the raw logs, this is emitted from the hardware itself and is the
> > same whether I am on hid-logitech-dj or not.
>
> A bit offtopic for this thread, but if it is a HID++ 1.0 device, then
> you could try setting the HIDPP_QUIRK_HIDPP_EXTRA_MOUSE_BTNS quirk on
> it and see if that helps. I've seen several cases with HID++ 1.0 devices
> where some keyboard-keys / buttons would not report (or report wrongly)
> unless the reporting of them was switched over from the regular HID
> input report to the HID++ version of the report.

I'll have to test this, yes. Thanks.

>
> >> The first patch should probably go to 5.10 as a fix in
> >> case someone pairs the Dinovo Mini with a MX5x00 receiver like the
> >> reporter of this bug did with his Dinovo Edge:
> >> https://bugzilla.redhat.com/show_bug.cgi?id=1811424
> >
> > OK, then I can apply it on top of the previous fix. I guess we don't
> > need stable@vger.k.o for this one?
>
> Actually this is intended for stable, to avoid getting a repeat of:
> https://bugzilla.redhat.com/show_bug.cgi?id=1811424
> with a Dinovo Mini. So if you can add a Cc: stable that would be
> great.
>

Oops, I have already pushed it without the tag. I guess we can always
request it later...

> >> The other 2 are 5.11 material.
> >
> > If I have to break the series, I will have to wait for Linus to first
> > merge the 5.10 material, then I'll be able to apply the others on
> > top...
>
> Ack, that works for me there is no rush to get the other 2 merged.

Great, thanks!

Cheers,
Benjamin

>
> Regards,
>
> Hans
>

Re: [RFC 0/3] HID: logitech-dj: Dinovo keyboard fixes and improvements

[Hans de Goede]

Hi,

On 11/19/20 4:52 PM, Benjamin Tissoires wrote:
> On Thu, Nov 19, 2020 at 4:48 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi,
>>
>> On 11/19/20 4:25 PM, Benjamin Tissoires wrote:
>>> Hi Hans,
>>>
>>> On Sat, Nov 14, 2020 at 10:21 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>>>
>>>> Hi Benjamin,
>>>>
>>>> Here is my patch series for the discussed Dinovo keyboard (receiver)
>>>> support improvements.
>>>>
>>>> I've marked this as a RFC since it has not been tested with a Dinovo Mini
>>>> (nor a Dinovo Mini receiver) yet.
>>>>
>>>> I have tested it extensively with a Dinovo Edge, a MX5000 and a MX5500
>>>> keyboard. In case of the Dinovo Edge and MX5000 I've not only tested
>>>> them against their own receiver but also against each-others receiver.
>>>>
>>>> Once you have tested this series on your Dinovo Mini, it is ready to
>>>> go upstream.
>>>
>>> That part is now done, so I guess we can push it upstream :)
>>
>> Great thank you.
>>
>>> FTR, the dinovo mini still works fine with this series. I have a weird
>>> issue where the secondary button gives me a left click, but according
>>> to the raw logs, this is emitted from the hardware itself and is the
>>> same whether I am on hid-logitech-dj or not.
>>
>> A bit offtopic for this thread, but if it is a HID++ 1.0 device, then
>> you could try setting the HIDPP_QUIRK_HIDPP_EXTRA_MOUSE_BTNS quirk on
>> it and see if that helps. I've seen several cases with HID++ 1.0 devices
>> where some keyboard-keys / buttons would not report (or report wrongly)
>> unless the reporting of them was switched over from the regular HID
>> input report to the HID++ version of the report.
> 
> I'll have to test this, yes. Thanks.
> 
>>
>>>> The first patch should probably go to 5.10 as a fix in
>>>> case someone pairs the Dinovo Mini with a MX5x00 receiver like the
>>>> reporter of this bug did with his Dinovo Edge:
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1811424
>>>
>>> OK, then I can apply it on top of the previous fix. I guess we don't
>>> need stable@vger.k.o for this one?
>>
>> Actually this is intended for stable, to avoid getting a repeat of:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1811424
>> with a Dinovo Mini. So if you can add a Cc: stable that would be
>> great.
>>
> 
> Oops, I have already pushed it without the tag. I guess we can always
> request it later...

No problem, the Fixes tag which it has alone should be enough for it to get
picked into the stable series.

Regards,

Hans

Re: [RFC 0/3] HID: logitech-dj: Dinovo keyboard fixes and improvements

[Hans de Goede]

Hi,

On 11/14/20 10:20 PM, Hans de Goede wrote:
> Hi Benjamin,
> 
> Here is my patch series for the discussed Dinovo keyboard (receiver)
> support improvements.
> 
> I've marked this as a RFC since it has not been tested with a Dinovo Mini
> (nor a Dinovo Mini receiver) yet.
> 
> I have tested it extensively with a Dinovo Edge, a MX5000 and a MX5500
> keyboard. In case of the Dinovo Edge and MX5000 I've not only tested
> them against their own receiver but also against each-others receiver.
> 
> Once you have tested this series on your Dinovo Mini, it is ready to
> go upstream. The first patch should probably go to 5.10 as a fix in
> case someone pairs the Dinovo Mini with a MX5x00 receiver like the
> reporter of this bug did with his Dinovo Edge:
> https://bugzilla.redhat.com/show_bug.cgi?id=1811424
> 
> The other 2 are 5.11 material.

IIRC we agreed (after Benjamin tested this series with his Dinovo Mini)
that this series was ready to go upstream as is.

Patch 1/3 of this has already been merged since it is a bug-fix.

But patch 2/3 and 3/3 are not in for-next. They should be ready
to go as-is, but let me know if you want me to resend them as
non RFC patches.

Regards,

Hans