[PATCH 0/5] HID: add battery driver for the UNCTRL RAGE Wireless Mouse

[PATCH 0/5] HID: add battery driver for the UNCTRL RAGE Wireless Mouse

[Bishwa Bhusan Saha via B4 Relay]

This patchset adds battery reporting for the UNCTRL RAGE Wireless Mouse.
Parts of the driver are inspired by the Kysona HID driver
(drivers/hid/hid-kysona.c).

It was tested on an UNCTRL RAGE Wireless Mouse.

Signed-off-by: Bishwa Bhusan Saha <contact@bishwasaha.com>
---
Bishwa Bhusan Saha (5):
      HID: add IDs for UNCTRL RAGE Mouse
      HID: UNCTRL: add basic battery reporting for UNCTRL RAGE Mouse
      HID: UNCTRL: add periodic battery reporting using a workqueue
      HID: UNCTRL: keep a single battery when a mouse has multiple connections
      HID: UNCTRL: only report to userspace on battery changes

 drivers/hid/Kconfig         |   8 +
 drivers/hid/Makefile        |   1 +
 drivers/hid/hid-ids.h       |   2 +
 drivers/hid/hid-unctrl-m1.c | 426 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 437 insertions(+)
---
base-commit: 145c2b2e9a5c0f794fb4009bcb072ab19f8ccfcd
change-id: 20261013-b4-unctrl-m1-battery-4afbaf17c0ea

Best regards,
--  
Bishwa Bhusan Saha <contact@bishwasaha.com>

[PATCH 1/5] HID: add IDs for UNCTRL RAGE Mouse

[Bishwa Bhusan Saha via B4 Relay]

From: Bishwa Bhusan Saha <contact@bishwasaha.com>

Add the product IDs for both wired and wireless (dongle) mode of
UNCTRL RAGE mouse (also known as UNCTRL M1).

Signed-off-by: Bishwa Bhusan Saha <contact@bishwasaha.com>
---
 drivers/hid/hid-ids.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index a9537b7bb..fb4c25c71 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -1008,6 +1008,8 @@
 #define USB_DEVICE_ID_MADCATZ_RAT5	0x1705
 #define USB_DEVICE_ID_MADCATZ_RAT9	0x1709
 #define USB_DEVICE_ID_MADCATZ_MMO7  0x1713
+#define USB_DEVICE_ID_UNCTRL_M1_WIRED	0xA09F
+#define USB_DEVICE_ID_UNCTRL_M1_DONGLE	0xA0AF
 
 #define USB_VENDOR_ID_MCC		0x09db
 #define USB_DEVICE_ID_MCC_PMD1024LS	0x0076

-- 
2.56.0

[PATCH 2/5] HID: UNCTRL: add basic battery reporting for UNCTRL RAGE Mouse

[Bishwa Bhusan Saha via B4 Relay]

From: Bishwa Bhusan Saha <contact@bishwasaha.com>

This initial patch will probe the battery only once. A later patch
will add periodic checking.

Signed-off-by: Bishwa Bhusan Saha <contact@bishwasaha.com>
Assisted-by: GLM:glm-5.3
---
 drivers/hid/Kconfig         |   8 ++
 drivers/hid/Makefile        |   1 +
 drivers/hid/hid-unctrl-m1.c | 314 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 323 insertions(+)

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 0e3a0ccd6..f3730c94f 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1362,6 +1362,14 @@ config HID_PULSAR
 
 	  Additional supported devices: Kysona M600, ATK VXE R1 SE+
 
+config HID_UNCTRL_M1
+	tristate "UNCTRL RAGE Wireless Mouse battery support"
+	depends on USB_HID
+	select POWER_SUPPLY
+	help
+	  Report the battery status of the UNCTRL RAGE Wireless Mouse
+	  over its 2.4 GHz receiver and while wired.
+
 config HID_WACOM
 	tristate "Wacom Intuos/Graphire tablet support (USB)"
 	depends on USB_HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 79384d905..8a9d00a98 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -150,6 +150,7 @@ hid-uclogic-objs		:= hid-uclogic-core.o \
 				   hid-uclogic-params.o
 obj-$(CONFIG_HID_UCLOGIC)	+= hid-uclogic.o
 obj-$(CONFIG_HID_UDRAW_PS3)	+= hid-udraw-ps3.o
+obj-$(CONFIG_HID_UNCTRL_M1)	+= hid-unctrl-m1.o
 obj-$(CONFIG_HID_UNIVERSAL_PIDFF)	+= hid-universal-pidff.o
 obj-$(CONFIG_HID_LED)		+= hid-led.o
 obj-$(CONFIG_HID_XIAOMI)	+= hid-xiaomi.o
diff --git a/drivers/hid/hid-unctrl-m1.c b/drivers/hid/hid-unctrl-m1.c
new file mode 100644
index 000000000..59259dee7
--- /dev/null
+++ b/drivers/hid/hid-unctrl-m1.c
@@ -0,0 +1,314 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * HID battery driver for the UNCTRL RAGE Wireless Mouse (model MSPTM101,
+ * known as "UNCTRL M1" over Bluetooth. Mad Catz, receiver 0xA0AF,
+ * wired 0xA09F). The mouse also connects over Bluetooth, on which no
+ * battery queries are made.
+ *
+ * The same mouse appears as two USB devices at once when it is both
+ * docked on the cable and linked to the receiver. Without care that would
+ * expose two batteries for one mouse, and only the cable connection
+ * actually reaches it, so the driver keeps a single power supply and
+ * hands it to whichever connection can answer queries.
+ *
+ * The device echoes every query before answering. The mode byte in the
+ * reply tells the answer from the echoes. The wired connection only
+ * answers over the interrupt out endpoint, where the queries go.
+ */
+
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/module.h>
+#include <linux/power_supply.h>
+#include <linux/slab.h>
+#include <linux/usb.h>
+
+#include "hid-ids.h"
+
+#define UNCTRL_VENDOR_IFNUM 2
+
+#define UNCTRL_MODE_RECEIVER 0x01
+#define UNCTRL_MODE_WIRED 0x44
+
+/* the battery query: report number, 4-byte header, padding, terminator */
+static const u8 unctrl_battery_query[33] = {
+	0x00,
+	0x0A, 0xAA, 0xFF, 0xDD,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00,
+	0x6F,
+};
+
+struct unctrl_dev {
+	struct hid_device *hdev;
+
+	struct power_supply_desc battery_desc;
+	struct power_supply *battery;
+
+	u8 battery_capacity;
+	u8 mode;
+	bool online;
+	bool wired;
+};
+
+static enum power_supply_property unctrl_battery_props[] = {
+	POWER_SUPPLY_PROP_STATUS,     POWER_SUPPLY_PROP_PRESENT,
+	POWER_SUPPLY_PROP_CAPACITY,   POWER_SUPPLY_PROP_SCOPE,
+	POWER_SUPPLY_PROP_MODEL_NAME, POWER_SUPPLY_PROP_SERIAL_NUMBER,
+	POWER_SUPPLY_PROP_ONLINE,
+};
+
+static int unctrl_battery_get_property(struct power_supply *psy,
+				       enum power_supply_property psp,
+				       union power_supply_propval *val)
+{
+	struct unctrl_dev *dev = power_supply_get_drvdata(psy);
+	int ret = 0;
+	u8 capacity, mode;
+	bool online;
+
+	capacity = dev->battery_capacity;
+	mode = dev->mode;
+	online = dev->online;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_PRESENT:
+		val->intval = 1;
+		break;
+	case POWER_SUPPLY_PROP_ONLINE:
+		val->intval = online;
+		break;
+	case POWER_SUPPLY_PROP_STATUS:
+		if (!online)
+			val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
+		else if (mode == UNCTRL_MODE_WIRED && capacity < 100)
+			val->intval = POWER_SUPPLY_STATUS_CHARGING;
+		else if (capacity >= 100)
+			val->intval = POWER_SUPPLY_STATUS_FULL;
+		else
+			val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
+		break;
+	case POWER_SUPPLY_PROP_SCOPE:
+		val->intval = POWER_SUPPLY_SCOPE_DEVICE;
+		break;
+	case POWER_SUPPLY_PROP_CAPACITY:
+		val->intval = capacity;
+		break;
+	case POWER_SUPPLY_PROP_MODEL_NAME:
+		val->strval = dev->hdev->name;
+		break;
+	case POWER_SUPPLY_PROP_SERIAL_NUMBER:
+		val->strval = "unctrl-m1";
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
+static void unctrl_fetch_battery(struct hid_device *hdev)
+{
+	u8 *write_buf;
+	int ret;
+
+	/* buf[0] is the report number, and hid_hw_output_report() sends the
+	 * buffer verbatim.
+	 */
+	write_buf = kmemdup(unctrl_battery_query, sizeof(unctrl_battery_query),
+			    GFP_KERNEL);
+	if (!write_buf)
+		return;
+
+	ret = hid_hw_output_report(hdev, write_buf,
+				   sizeof(unctrl_battery_query));
+	if (ret < 0)
+		hid_dbg(hdev, "%s:battery query failed with error %d\n",
+			__func__, ret);
+
+	kfree(write_buf);
+}
+
+/* Register the power supply of this connection and start reporting. */
+static int unctrl_battery_register(struct unctrl_dev *dev)
+{
+	struct power_supply_config config = { .drv_data = dev };
+	struct power_supply *battery;
+	int ret;
+
+	battery = power_supply_register(&dev->hdev->dev, &dev->battery_desc,
+					&config);
+	if (IS_ERR(battery)) {
+		ret = PTR_ERR(battery);
+		hid_err(dev->hdev,
+			"%s:power_supply_register failed with error %d\n",
+			__func__, ret);
+		return ret;
+	}
+
+	power_supply_powers(battery, &dev->hdev->dev);
+	dev->battery = battery;
+	hid_info(dev->hdev, "%s:battery registered as %s\n", __func__,
+		 dev->battery_desc.name);
+
+	unctrl_fetch_battery(dev->hdev);
+
+	return 0;
+}
+
+static int unctrl_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+	struct unctrl_dev *dev;
+	struct usb_interface *usbif;
+	unsigned int connect_mask;
+	bool vendor;
+	int ret;
+
+	if (!hid_is_usb(hdev))
+		return -EINVAL;
+
+	usbif = to_usb_interface(hdev->dev.parent);
+	vendor = usbif->cur_altsetting->desc.bInterfaceNumber ==
+		 UNCTRL_VENDOR_IFNUM;
+
+	/* the pointer, buttons and hidraw have to keep working */
+	connect_mask = vendor ? HID_CONNECT_HIDRAW : HID_CONNECT_DEFAULT;
+
+	ret = hid_parse(hdev);
+	if (ret) {
+		hid_err(hdev, "%s:parse failed with error %d\n", __func__, ret);
+		return ret;
+	}
+
+	ret = hid_hw_start(hdev, connect_mask);
+	if (ret) {
+		hid_err(hdev, "%s:hw start failed with error %d\n", __func__,
+			ret);
+		return ret;
+	}
+
+	if (!vendor)
+		return 0;
+
+	dev = kzalloc_obj(*dev);
+	if (!dev) {
+		hid_hw_stop(hdev);
+		return -ENOMEM;
+	}
+
+	dev->hdev = hdev;
+	dev->battery_capacity = 100;
+	dev->mode = UNCTRL_MODE_RECEIVER;
+	dev->wired = hdev->product == USB_DEVICE_ID_UNCTRL_M1_WIRED;
+
+	dev->battery_desc.name = devm_kasprintf(&hdev->dev, GFP_KERNEL,
+						"unctrl-m1-%04x-battery",
+						hdev->product);
+	if (!dev->battery_desc.name) {
+		hid_hw_stop(hdev);
+		kfree(dev);
+		return -ENOMEM;
+	}
+	dev->battery_desc.type = POWER_SUPPLY_TYPE_BATTERY;
+	dev->battery_desc.properties = unctrl_battery_props;
+	dev->battery_desc.num_properties = ARRAY_SIZE(unctrl_battery_props);
+	dev->battery_desc.get_property = unctrl_battery_get_property;
+	dev->battery_desc.use_for_apm = 0;
+
+	/* usbhid polls the interrupt in endpoint only while it is open */
+	ret = hid_hw_open(hdev);
+	if (ret) {
+		hid_err(hdev, "%s:open failed with error %d\n", __func__, ret);
+		hid_hw_stop(hdev);
+		kfree(dev);
+		return ret;
+	}
+
+	hid_set_drvdata(hdev, dev);
+
+	ret = unctrl_battery_register(dev);
+	if (ret)
+		goto err_del;
+
+	return 0;
+
+err_del:
+	hid_hw_close(hdev);
+	hid_hw_stop(hdev);
+	hid_set_drvdata(hdev, NULL);
+	kfree(dev);
+	return ret;
+}
+
+static int unctrl_raw_event(struct hid_device *hdev, struct hid_report *report,
+			    u8 *data, int size)
+{
+	struct unctrl_dev *dev = hid_get_drvdata(hdev);
+	struct power_supply *battery = NULL;
+
+	if (!dev || size < 8)
+		return 0;
+
+	if (data[0] != 0x0A || data[1] != 0xAA || data[2] != 0xFF ||
+	    data[3] != 0xDD)
+		return 0;
+
+	/* real answers carry the mode here, echoes of the query do not */
+	if (data[5] != UNCTRL_MODE_RECEIVER && data[5] != UNCTRL_MODE_WIRED)
+		return 0;
+
+	/* the receiver reports 0% while the mouse is switched off */
+	dev->battery_capacity = min_t(u8, data[4], 100);
+	dev->mode = data[5];
+	dev->online = dev->battery_capacity != 0;
+	battery = dev->battery;
+
+	/* only schedules a work, so this is safe from the completion path */
+	if (battery)
+		power_supply_changed(battery);
+
+	return 0;
+}
+
+static void unctrl_remove(struct hid_device *hdev)
+{
+	struct unctrl_dev *dev = hid_get_drvdata(hdev);
+
+	hid_hw_close(hdev);
+
+	hid_hw_stop(hdev);
+
+	if (!dev)
+		return;
+
+	if (dev->battery)
+		power_supply_unregister(dev->battery);
+
+	hid_set_drvdata(hdev, NULL);
+	kfree(dev);
+}
+
+static const struct hid_device_id unctrl_devices[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_MADCATZ,
+			 USB_DEVICE_ID_UNCTRL_M1_DONGLE) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_MADCATZ,
+			 USB_DEVICE_ID_UNCTRL_M1_WIRED) },
+	{}
+};
+MODULE_DEVICE_TABLE(hid, unctrl_devices);
+
+static struct hid_driver unctrl_driver = {
+	.name = "unctrl-m1",
+	.id_table = unctrl_devices,
+	.probe = unctrl_probe,
+	.raw_event = unctrl_raw_event,
+	.remove = unctrl_remove,
+};
+module_hid_driver(unctrl_driver);
+
+MODULE_AUTHOR("Bishwa Bhusan Saha <contact@bishwasaha.com>");
+MODULE_DESCRIPTION("HID battery driver for the UNCTRL M1 Mouse");
+MODULE_LICENSE("GPL");

-- 
2.56.0

[PATCH 3/5] HID: UNCTRL: add periodic battery reporting using a workqueue

[Bishwa Bhusan Saha via B4 Relay]

From: Bishwa Bhusan Saha <contact@bishwasaha.com>

This patch uses a workqueue to check battery status every 5 seconds.

Signed-off-by: Bishwa Bhusan Saha <contact@bishwasaha.com>
Assisted-by: GLM:glm-5.3
---
 drivers/hid/hid-unctrl-m1.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-unctrl-m1.c b/drivers/hid/hid-unctrl-m1.c
index 59259dee7..1e73cad1e 100644
--- a/drivers/hid/hid-unctrl-m1.c
+++ b/drivers/hid/hid-unctrl-m1.c
@@ -22,11 +22,14 @@
 #include <linux/power_supply.h>
 #include <linux/slab.h>
 #include <linux/usb.h>
+#include <linux/workqueue.h>
 
 #include "hid-ids.h"
 
 #define UNCTRL_VENDOR_IFNUM 2
 
+#define UNCTRL_BATTERY_TIMEOUT_MS 5000
+
 #define UNCTRL_MODE_RECEIVER 0x01
 #define UNCTRL_MODE_WIRED 0x44
 
@@ -47,6 +50,8 @@ struct unctrl_dev {
 	struct power_supply_desc battery_desc;
 	struct power_supply *battery;
 
+	struct delayed_work battery_work;
+
 	u8 battery_capacity;
 	u8 mode;
 	bool online;
@@ -132,6 +137,17 @@ static void unctrl_fetch_battery(struct hid_device *hdev)
 	kfree(write_buf);
 }
 
+static void unctrl_battery_timer_tick(struct work_struct *work)
+{
+	struct unctrl_dev *dev =
+		container_of(work, struct unctrl_dev, battery_work.work);
+
+	unctrl_fetch_battery(dev->hdev);
+
+	schedule_delayed_work(&dev->battery_work,
+			      msecs_to_jiffies(UNCTRL_BATTERY_TIMEOUT_MS));
+}
+
 /* Register the power supply of this connection and start reporting. */
 static int unctrl_battery_register(struct unctrl_dev *dev)
 {
@@ -155,6 +171,8 @@ static int unctrl_battery_register(struct unctrl_dev *dev)
 		 dev->battery_desc.name);
 
 	unctrl_fetch_battery(dev->hdev);
+	schedule_delayed_work(&dev->battery_work,
+			      msecs_to_jiffies(UNCTRL_BATTERY_TIMEOUT_MS));
 
 	return 0;
 }
@@ -200,6 +218,8 @@ static int unctrl_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	}
 
 	dev->hdev = hdev;
+	INIT_DELAYED_WORK(&dev->battery_work, unctrl_battery_timer_tick);
+
 	dev->battery_capacity = 100;
 	dev->mode = UNCTRL_MODE_RECEIVER;
 	dev->wired = hdev->product == USB_DEVICE_ID_UNCTRL_M1_WIRED;
@@ -277,7 +297,11 @@ static void unctrl_remove(struct hid_device *hdev)
 {
 	struct unctrl_dev *dev = hid_get_drvdata(hdev);
 
-	hid_hw_close(hdev);
+	/* Stop polling before the transport goes down. */
+	if (dev) {
+		cancel_delayed_work_sync(&dev->battery_work);
+		hid_hw_close(hdev);
+	}
 
 	hid_hw_stop(hdev);
 

-- 
2.56.0

[PATCH 4/5] HID: UNCTRL: keep a single battery when a mouse has multiple connections

[Bishwa Bhusan Saha via B4 Relay]

From: Bishwa Bhusan Saha <contact@bishwasaha.com>

The UNCTRL RAGE mouse can be connected over its 2.4 GHz dongle and
its wired connection at the same time. Both are visible as separate
devices on lsusb. Without this patch two separate batteries would be
detected in case of a single mouse connected over 2 interfaces. This
patch solves it by registering the power supply only on the connection
that can answer queries and taking it away when that connection goes
away.

Signed-off-by: Bishwa Bhusan Saha <contact@bishwasaha.com>
Assisted-by: GLM:glm-5.3
---
 drivers/hid/hid-unctrl-m1.c | 103 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 91 insertions(+), 12 deletions(-)

diff --git a/drivers/hid/hid-unctrl-m1.c b/drivers/hid/hid-unctrl-m1.c
index 1e73cad1e..ff15384c0 100644
--- a/drivers/hid/hid-unctrl-m1.c
+++ b/drivers/hid/hid-unctrl-m1.c
@@ -16,9 +16,12 @@
  * answers over the interrupt out endpoint, where the queries go.
  */
 
+#include <linux/cleanup.h>
 #include <linux/device.h>
 #include <linux/hid.h>
+#include <linux/list.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/power_supply.h>
 #include <linux/slab.h>
 #include <linux/usb.h>
@@ -45,19 +48,27 @@ static const u8 unctrl_battery_query[33] = {
 };
 
 struct unctrl_dev {
+	struct list_head list;
 	struct hid_device *hdev;
 
 	struct power_supply_desc battery_desc;
 	struct power_supply *battery;
 
+	/* raw_event runs in the completion path, so this is a spinlock. */
+	spinlock_t lock;
+
 	struct delayed_work battery_work;
 
 	u8 battery_capacity;
 	u8 mode;
 	bool online;
 	bool wired;
+	bool vendor;
 };
 
+static LIST_HEAD(unctrl_connections);
+static DEFINE_MUTEX(unctrl_lock);
+
 static enum power_supply_property unctrl_battery_props[] = {
 	POWER_SUPPLY_PROP_STATUS,     POWER_SUPPLY_PROP_PRESENT,
 	POWER_SUPPLY_PROP_CAPACITY,   POWER_SUPPLY_PROP_SCOPE,
@@ -74,9 +85,11 @@ static int unctrl_battery_get_property(struct power_supply *psy,
 	u8 capacity, mode;
 	bool online;
 
-	capacity = dev->battery_capacity;
-	mode = dev->mode;
-	online = dev->online;
+	scoped_guard(spinlock_irqsave, &dev->lock) {
+		capacity = dev->battery_capacity;
+		mode = dev->mode;
+		online = dev->online;
+	}
 
 	switch (psp) {
 	case POWER_SUPPLY_PROP_PRESENT:
@@ -148,13 +161,46 @@ static void unctrl_battery_timer_tick(struct work_struct *work)
 			      msecs_to_jiffies(UNCTRL_BATTERY_TIMEOUT_MS));
 }
 
-/* Register the power supply of this connection and start reporting. */
+static struct unctrl_dev *unctrl_battery_owner(void)
+{
+	struct unctrl_dev *dev;
+
+	list_for_each_entry(dev, &unctrl_connections, list) {
+		if (dev->battery)
+			return dev;
+	}
+
+	return NULL;
+}
+
+/*
+ * Register the power supply on the connection the mouse talks over,
+ * a wired connection takes it away from a receiver. Called with
+ * unctrl_lock held.
+ */
 static int unctrl_battery_register(struct unctrl_dev *dev)
 {
 	struct power_supply_config config = { .drv_data = dev };
+	struct unctrl_dev *owner;
 	struct power_supply *battery;
 	int ret;
 
+	owner = unctrl_battery_owner();
+	if (owner == dev || (owner && (!dev->wired || owner->wired)))
+		return 0;
+
+	if (owner) {
+		struct power_supply *owner_battery;
+
+		cancel_delayed_work_sync(&owner->battery_work);
+		/* so a raw_event cannot touch the supply being unregistered */
+		scoped_guard(spinlock_irqsave, &owner->lock) {
+			owner_battery = owner->battery;
+			owner->battery = NULL;
+		}
+		power_supply_unregister(owner_battery);
+	}
+
 	battery = power_supply_register(&dev->hdev->dev, &dev->battery_desc,
 					&config);
 	if (IS_ERR(battery)) {
@@ -166,7 +212,8 @@ static int unctrl_battery_register(struct unctrl_dev *dev)
 	}
 
 	power_supply_powers(battery, &dev->hdev->dev);
-	dev->battery = battery;
+	scoped_guard(spinlock_irqsave, &dev->lock)
+		dev->battery = battery;
 	hid_info(dev->hdev, "%s:battery registered as %s\n", __func__,
 		 dev->battery_desc.name);
 
@@ -223,6 +270,8 @@ static int unctrl_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	dev->battery_capacity = 100;
 	dev->mode = UNCTRL_MODE_RECEIVER;
 	dev->wired = hdev->product == USB_DEVICE_ID_UNCTRL_M1_WIRED;
+	dev->vendor = vendor;
+	spin_lock_init(&dev->lock);
 
 	dev->battery_desc.name = devm_kasprintf(&hdev->dev, GFP_KERNEL,
 						"unctrl-m1-%04x-battery",
@@ -249,13 +298,20 @@ static int unctrl_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 	hid_set_drvdata(hdev, dev);
 
+	mutex_lock(&unctrl_lock);
+	list_add_tail(&dev->list, &unctrl_connections);
 	ret = unctrl_battery_register(dev);
+	mutex_unlock(&unctrl_lock);
+
 	if (ret)
 		goto err_del;
 
 	return 0;
 
 err_del:
+	mutex_lock(&unctrl_lock);
+	list_del(&dev->list);
+	mutex_unlock(&unctrl_lock);
 	hid_hw_close(hdev);
 	hid_hw_stop(hdev);
 	hid_set_drvdata(hdev, NULL);
@@ -280,11 +336,13 @@ static int unctrl_raw_event(struct hid_device *hdev, struct hid_report *report,
 	if (data[5] != UNCTRL_MODE_RECEIVER && data[5] != UNCTRL_MODE_WIRED)
 		return 0;
 
-	/* the receiver reports 0% while the mouse is switched off */
-	dev->battery_capacity = min_t(u8, data[4], 100);
-	dev->mode = data[5];
-	dev->online = dev->battery_capacity != 0;
-	battery = dev->battery;
+	scoped_guard(spinlock_irqsave, &dev->lock) {
+		/* the receiver reports 0% while the mouse is switched off */
+		dev->battery_capacity = min_t(u8, data[4], 100);
+		dev->mode = data[5];
+		dev->online = dev->battery_capacity != 0;
+		battery = dev->battery;
+	}
 
 	/* only schedules a work, so this is safe from the completion path */
 	if (battery)
@@ -296,6 +354,7 @@ static int unctrl_raw_event(struct hid_device *hdev, struct hid_report *report,
 static void unctrl_remove(struct hid_device *hdev)
 {
 	struct unctrl_dev *dev = hid_get_drvdata(hdev);
+	struct unctrl_dev *other;
 
 	/* Stop polling before the transport goes down. */
 	if (dev) {
@@ -308,8 +367,28 @@ static void unctrl_remove(struct hid_device *hdev)
 	if (!dev)
 		return;
 
-	if (dev->battery)
-		power_supply_unregister(dev->battery);
+	/*
+	 * Pass the battery to a capable connection without one, so that it
+	 * keeps being reported when the owner disappears.
+	 */
+	mutex_lock(&unctrl_lock);
+	list_del(&dev->list);
+	if (dev->battery) {
+		struct power_supply *battery;
+
+		scoped_guard(spinlock_irqsave, &dev->lock) {
+			battery = dev->battery;
+			dev->battery = NULL;
+		}
+		power_supply_unregister(battery);
+		list_for_each_entry(other, &unctrl_connections, list) {
+			if (!other->vendor || other->battery)
+				continue;
+			unctrl_battery_register(other);
+			break;
+		}
+	}
+	mutex_unlock(&unctrl_lock);
 
 	hid_set_drvdata(hdev, NULL);
 	kfree(dev);

-- 
2.56.0

[PATCH 5/5] HID: UNCTRL: only report to userspace on battery changes

[Bishwa Bhusan Saha via B4 Relay]

From: Bishwa Bhusan Saha <contact@bishwasaha.com>

This patch calls power_supply_changed() only when the reported
capacity or mode actually changes.

Signed-off-by: Bishwa Bhusan Saha <contact@bishwasaha.com>
Assisted-by: GLM:glm-5.3
---
 drivers/hid/hid-unctrl-m1.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-unctrl-m1.c b/drivers/hid/hid-unctrl-m1.c
index ff15384c0..df283cbae 100644
--- a/drivers/hid/hid-unctrl-m1.c
+++ b/drivers/hid/hid-unctrl-m1.c
@@ -324,6 +324,8 @@ static int unctrl_raw_event(struct hid_device *hdev, struct hid_report *report,
 {
 	struct unctrl_dev *dev = hid_get_drvdata(hdev);
 	struct power_supply *battery = NULL;
+	bool changed;
+	u8 old_capacity, old_mode;
 
 	if (!dev || size < 8)
 		return 0;
@@ -338,14 +340,21 @@ static int unctrl_raw_event(struct hid_device *hdev, struct hid_report *report,
 
 	scoped_guard(spinlock_irqsave, &dev->lock) {
 		/* the receiver reports 0% while the mouse is switched off */
+		old_capacity = dev->battery_capacity;
+		old_mode = dev->mode;
+
 		dev->battery_capacity = min_t(u8, data[4], 100);
 		dev->mode = data[5];
 		dev->online = dev->battery_capacity != 0;
+
+		changed = dev->battery_capacity != old_capacity ||
+			  dev->mode != old_mode;
+
 		battery = dev->battery;
 	}
 
 	/* only schedules a work, so this is safe from the completion path */
-	if (battery)
+	if (battery && changed)
 		power_supply_changed(battery);
 
 	return 0;

-- 
2.56.0

Re: [PATCH 2/5] HID: UNCTRL: add basic battery reporting for UNCTRL RAGE Mouse

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] hid: unctrl: initial battery query response may be dropped in probe
--

commit 6b35740bb80be8312a87135a9844c03a48dc1cca
Author: Bishwa Bhusan Saha <contact@bishwasaha.com>

HID: UNCTRL: add basic battery reporting for UNCTRL RAGE Mouse

This commit adds basic battery reporting for the UNCTRL RAGE Wireless Mouse
over both 2.4 GHz and wired connections. It queries the battery once during
device probe by sending an output report to the hardware.

> diff --git a/drivers/hid/hid-unctrl-m1.c b/drivers/hid/hid-unctrl-m1.c
> new file mode 100644
> index 0000000000000..59259dee7e429
> --- /dev/null
> +++ b/drivers/hid/hid-unctrl-m1.c
[ ... ]
> +static void unctrl_fetch_battery(struct hid_device *hdev)
> +{
> +	u8 *write_buf;
> +	int ret;
> +
> +	/* buf[0] is the report number, and hid_hw_output_report() sends the
> +	 * buffer verbatim.
> +	 */
> +	write_buf = kmemdup(unctrl_battery_query, sizeof(unctrl_battery_query),
> +			    GFP_KERNEL);
> +	if (!write_buf)
> +		return;
> +
> +	ret = hid_hw_output_report(hdev, write_buf,
> +				   sizeof(unctrl_battery_query));
[ ... ]
> +static int unctrl_probe(struct hid_device *hdev, const struct hid_device_id *id)
> +{
[ ... ]
> +	/* usbhid polls the interrupt in endpoint only while it is open */
> +	ret = hid_hw_open(hdev);
> +	if (ret) {
> +		hid_err(hdev, "%s:open failed with error %d\n", __func__, ret);
> +		hid_hw_stop(hdev);
> +		kfree(dev);
> +		return ret;
> +	}
> +
> +	hid_set_drvdata(hdev, dev);
> +
> +	ret = unctrl_battery_register(dev);

[Severity: Low]
Since hid_device_io_start() has not been called in unctrl_probe(), will the
HID core's driver_input_lock block the incoming battery report reply?

If the device responds to the output report sent by unctrl_fetch_battery()
before the probe function returns, it appears __hid_input_report() might drop
the reply because the IO path is still gated.

Does this mean the initial battery level will remain incorrectly reported as
100% until a subsequent poll occurs?

> +	if (ret)
> +		goto err_del;
> +
> +	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013-b4-unctrl-m1-battery-v1-0-4a50203b7177@bishwasaha.com?part=2

Re: [PATCH 3/5] HID: UNCTRL: add periodic battery reporting using a workqueue

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] hid: unctrl: initial battery query reply dropped during probe
- [Medium] pm: battery polling work wakes CPU every 5s during s2idle
--

commit 96156a773d135475c7c9f3eca5f3c8ad662fe691
Author: Bishwa Bhusan Saha <contact@bishwasaha.com>

HID: UNCTRL: add periodic battery reporting using a workqueue

This patch adds a delayed workqueue to check the battery status every
5 seconds for the UNCTRL HID device.

> diff --git a/drivers/hid/hid-unctrl-m1.c b/drivers/hid/hid-unctrl-m1.c
> index 59259dee7e429..1e73cad1e3f91 100644
> --- a/drivers/hid/hid-unctrl-m1.c
> +++ b/drivers/hid/hid-unctrl-m1.c

[ ... ]

> @@ -132,6 +137,17 @@ static void unctrl_fetch_battery(struct hid_device *hdev)
>  	kfree(write_buf);
>  }
>  
> +static void unctrl_battery_timer_tick(struct work_struct *work)
> +{
> +	struct unctrl_dev *dev =
> +		container_of(work, struct unctrl_dev, battery_work.work);
> +
> +	unctrl_fetch_battery(dev->hdev);
> +
> +	schedule_delayed_work(&dev->battery_work,
> +			      msecs_to_jiffies(UNCTRL_BATTERY_TIMEOUT_MS));
> +}

[Severity: Medium]
Will this scheduled work continuously wake up the CPU during system suspend?

Since schedule_delayed_work() implicitly queues to the non-freezable system_wq
and the driver does not implement suspend/resume callbacks to cancel this work,
it appears this timer will keep firing every 5 seconds.

If the system enters s2idle suspend, could this prevent deep sleep and cause
unnecessary power drain?

[ ... ]

> @@ -155,6 +171,8 @@ static int unctrl_battery_register(struct unctrl_dev *dev)
>  		 dev->battery_desc.name);
>  
>  	unctrl_fetch_battery(dev->hdev);
> +	schedule_delayed_work(&dev->battery_work,
> +			      msecs_to_jiffies(UNCTRL_BATTERY_TIMEOUT_MS));
>  
>  	return 0;
>  }

[Severity: Medium]
Could the initial battery query reply be dropped by the HID core?

This function is called during unctrl_probe(), and it sends a battery query
via unctrl_fetch_battery(), but hid_device_io_start() hasn't been called to
unblock the input path.

If the device replies quickly before unctrl_probe() finishes and the
driver_input_lock is released, wouldn't the HID core silently drop the incoming
report with -EBUSY?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013-b4-unctrl-m1-battery-v1-0-4a50203b7177@bishwasaha.com?part=3

Re: [PATCH 4/5] HID: UNCTRL: keep a single battery when a mouse has multiple connections

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] hid: uaf in unctrl_remove() due to early work cancellation and list exposure
- [High] hid: uaf of power_supply in unctrl_raw_event() racing with cross-device probe
- [High] hid: uaf of dev in unctrl_probe() error path due to concurrent battery transfer
- [Medium] hid: permanent battery loss due to missing rollback in unctrl_battery_register()
--

commit 01440f9100f714c61f17b835003fbfa0e20c249f
Author: Bishwa Bhusan Saha <contact@bishwasaha.com>

    HID: UNCTRL: keep a single battery when a mouse has multiple connections

    This patch modifies the unctrl_m1 driver to register a single power
    supply across multiple connections (e.g., wired and dongle) of the same
    mouse. It transfers the power supply ownership when a more capable
    connection appears or disappears.

> diff --git a/drivers/hid/hid-unctrl-m1.c b/drivers/hid/hid-unctrl-m1.c
> --- a/drivers/hid/hid-unctrl-m1.c
> +++ b/drivers/hid/hid-unctrl-m1.c
[ ... ]
> @@ -181,32 +181,32 @@ static int unctrl_battery_register(struct unctrl_dev *dev)
>  	if (owner) {
>  		struct power_supply *owner_battery;
>  
>  		cancel_delayed_work_sync(&owner->battery_work);
>  		/* so a raw_event cannot touch the supply being unregistered */
>  		scoped_guard(spinlock_irqsave, &owner->lock) {
>  			owner_battery = owner->battery;
>  			owner->battery = NULL;
>  		}
>  		power_supply_unregister(owner_battery);
>  	}
>  
>  	battery = power_supply_register(&dev->hdev->dev, &dev->battery_desc,
>  					&config);
>  	if (IS_ERR(battery)) {
>  		ret = PTR_ERR(battery);
>  		hid_err(dev->hdev,
>  			"%s:power_supply_register failed with error %d\n",
>  			__func__, ret);
>  		return ret;

[Severity: Medium]
If power_supply_register() fails here, does the original owner's battery 
remain unregistered forever?

We just cancelled the owner's work and unregistered its battery above, but 
the error path doesn't restore it. Could this leave the active device 
permanently without battery reporting?

>  	}
>  
>  	power_supply_powers(battery, &dev->hdev->dev);
[ ... ]
> @@ -312,11 +312,11 @@ static int unctrl_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  	if (ret)
>  		goto err_del;
>  
>  	return 0;
>  
>  err_del:
>  	mutex_lock(&unctrl_lock);
>  	list_del(&dev->list);
>  	mutex_unlock(&unctrl_lock);

[Severity: High]
Is there a use-after-free risk in this error path due to a concurrent 
battery transfer?

During the time between the failing unctrl_battery_register() returning and 
re-acquiring the lock here at err_del, could a concurrent unctrl_remove() 
on another connection find this dev in unctrl_connections?

If that happens, the other thread would transfer its battery to this failing
dev and schedule dev->battery_work. This work would remain active after 
kfree(dev) is called at the end of this function.

>  	hid_hw_close(hdev);
>  	hid_hw_stop(hdev);
>  	hid_set_drvdata(hdev, NULL);
>  	kfree(dev);
>  	return ret;
>  }
[ ... ]
> @@ -336,15 +336,15 @@ static int unctrl_raw_event(struct hid_device *hdev, struct hid_report *report,
[ ... ]
>  	scoped_guard(spinlock_irqsave, &dev->lock) {
>  		/* the receiver reports 0% while the mouse is switched off */
>  		dev->battery_capacity = min_t(u8, data[4], 100);
>  		dev->mode = data[5];
>  		dev->online = dev->battery_capacity != 0;
>  		battery = dev->battery;
>  	}
>  
>  	/* only schedules a work, so this is safe from the completion path */
>  	if (battery)
>  		power_supply_changed(battery);

[Severity: High]
Can power_supply_changed() dereference a freed pointer here?

If a new connection probes concurrently, it can call unctrl_battery_register(), 
which acquires this dev->lock, sets dev->battery = NULL, and immediately 
calls power_supply_unregister(). 

Since power_supply_unregister() synchronously frees the battery, and this 
code drops dev->lock before using the cached battery pointer, could this 
lead to a use-after-free?

>  	return 0;
>  }
>  
>  static void unctrl_remove(struct hid_device *hdev)
>  {
>  	struct unctrl_dev *dev = hid_get_drvdata(hdev);
>  	struct unctrl_dev *other;
>  
>  	/* Stop polling before the transport goes down. */
>  	if (dev) {
>  		cancel_delayed_work_sync(&dev->battery_work);

[Severity: High]
Could dev->battery_work be rescheduled after this cancellation, leading to
a use-after-free?

Since dev is still in the global unctrl_connections list here (it isn't 
removed until the mutex_lock(&unctrl_lock) later in the function), could a 
concurrent unctrl_probe() or unctrl_remove() from another device find it, 
transfer a battery to it, and call schedule_delayed_work()?

If so, the rescheduled work might execute after kfree(dev) is called.

>  		hid_hw_close(hdev);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013-b4-unctrl-m1-battery-v1-0-4a50203b7177@bishwasaha.com?part=4