Thread (14 messages) flat view 14 messages, 4 authors, 10h ago

Re: [PATCH v4 1/4] HID: wiimote: turn on the LEDs indicating the controller id

From: "David Rheinsberg" <david@readahead.eu>
Date: 2026-09-20 08:24:43

Hi

On Mon, Aug 17, 2026, at 11:38 PM, Rafael Passos wrote:
quoted hunk ↗ jump to hunk
The behavior in a Wii/Wii U console is to have each controller turn on
a different LED indicating the controller id.
This commit implements the same behavior using the ida struct.
Unlike switch controllers, each ID only turns one LED (from 1 to 4).

Signed-off-by: Rafael Passos <redacted>
---
 drivers/hid/hid-wiimote-core.c  | 52 +++++++++++++++++++++++++++++----
 drivers/hid/hid-wiimote-debug.c |  5 +++-
 drivers/hid/hid-wiimote.h       |  1 +
 3 files changed, 51 insertions(+), 7 deletions(-)
diff --git a/drivers/hid/hid-wiimote-core.c 
b/drivers/hid/hid-wiimote-core.c
index 63c4fa8fbb9b..acf31d8b6991 100644
--- a/drivers/hid/hid-wiimote-core.c
+++ b/drivers/hid/hid-wiimote-core.c
@@ -621,6 +621,13 @@ static const __u8 * const 
wiimote_devtype_mods[WIIMOTE_DEV_NUM] = {
 	},
 };

+static const __u8 player_leds[] = {
+	WIIPROTO_FLAG_LED1,
+	WIIPROTO_FLAG_LED2,
+	WIIPROTO_FLAG_LED3,
+	WIIPROTO_FLAG_LED4
+};
+
 static void wiimote_modules_load(struct wiimote_data *wdata,
 				 unsigned int devtype)
 {
@@ -671,6 +678,12 @@ static void wiimote_modules_load(struct 
wiimote_data *wdata,
 	spin_lock_irq(&wdata->state.lock);
 	wdata->state.devtype = devtype;
 	spin_unlock_irq(&wdata->state.lock);
+
+	scoped_guard(spinlock_irqsave, &wdata->state.lock) {
+		/* after loading modules, set the Player ID LED cycling from 1 to 4*/
+		wiiproto_req_leds(wdata, player_leds[(wdata->player_id - 1) % 4]);
+	}
+
Sorry, I unintentionally sent the previous review in private. Anyway, I also noticed that if you set LEDs unconditionally, you can drop the same call from `wiimod_led_probe()` (the entire block guarded by `ops->arg == 0`).

Not all wiimote devices have LEDs, but I think it is safe to always send the LED request, given that those are embedded in all data requests, IIRC.

Thanks
David
quoted hunk ↗ jump to hunk
 	return;

 error:
@@ -855,11 +868,11 @@ static void wiimote_init_set_type(struct 
wiimote_data *wdata,

 done:
 	if (devtype == WIIMOTE_DEV_GENERIC)
-		hid_info(wdata->hdev, "cannot detect device; NAME: %s VID: %04x PID: 
%04x EXT: %04x\n",
-			name, vendor, product, exttype);
+		hid_info(wdata->hdev, "cannot detect device; NAME: %s VID: %04x PID: 
%04x EXT: %04x (%d)\n",
+			name, vendor, product, exttype, wdata->player_id);
 	else
-		hid_info(wdata->hdev, "detected device: %s\n",
-			 wiimote_devtype_names[devtype]);
+		hid_info(wdata->hdev, "detected device: %s (%d)\n",
+			 wiimote_devtype_names[devtype], wdata->player_id);

 	wiimote_modules_load(wdata, devtype);
 }
@@ -1752,6 +1765,7 @@ static struct wiimote_data *wiimote_create(struct 
hid_device *hdev)
 	mutex_init(&wdata->state.sync);
 	wdata->state.drm = WIIPROTO_REQ_DRM_K;
 	wdata->state.cmd_battery = 0xff;
+	wdata->player_id = 0; // min 1, u8 0 is unasigned id

 	INIT_WORK(&wdata->init_worker, wiimote_init_worker);
 	timer_setup(&wdata->timer, wiimote_init_timeout, 0);
@@ -1759,12 +1773,17 @@ static struct wiimote_data 
*wiimote_create(struct hid_device *hdev)
 	return wdata;
 }

+/* Global id allocator for wii remotes */
+static DEFINE_IDA(wiimote_ida);
+
 static void wiimote_destroy(struct wiimote_data *wdata)
 {
 	unsigned long flags;

 	wiidebug_deinit(wdata);

+	ida_free(&wiimote_ida, wdata->player_id);
+
 	/* prevent init_worker from being scheduled again */
 	spin_lock_irqsave(&wdata->state.lock, flags);
 	wdata->state.flags |= WIIPROTO_FLAG_EXITING;
@@ -1834,7 +1853,14 @@ static int wiimote_hid_probe(struct hid_device *hdev,
 	if (ret)
 		goto err_free;

-	hid_info(hdev, "New device registered\n");
+	ret = ida_alloc_min(&wiimote_ida, 1, GFP_KERNEL);
+	if (ret < 1) {
+		hid_err(hdev, "cannot allocate controller id\n");
+		goto err_free;
+	}
+
+	wdata->player_id = ret;
+	hid_info(hdev, "New device registered (Wiimote %d)\n", ret);

 	/* schedule device detection */
 	wiimote_schedule(wdata);
@@ -1887,7 +1913,21 @@ static struct hid_driver wiimote_hid_driver = {
 	.remove = wiimote_hid_remove,
 	.raw_event = wiimote_hid_event,
 };
-module_hid_driver(wiimote_hid_driver);
+
+
+static int __init wiimote_init(void)
+{
+	return hid_register_driver(&wiimote_hid_driver);
+}
+
+static void __exit wiimote_exit(void)
+{
+	hid_unregister_driver(&wiimote_hid_driver);
+	ida_destroy(&wiimote_ida);
+}
+
+module_init(wiimote_init);
+module_exit(wiimote_exit);

 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("David Herrmann <dh.herrmann@gmail.com>");
diff --git a/drivers/hid/hid-wiimote-debug.c b/drivers/hid/hid-wiimote-debug.c
index 5f74917781f2..fc847c2a1c1f 100644
--- a/drivers/hid/hid-wiimote-debug.c
+++ b/drivers/hid/hid-wiimote-debug.c
@@ -186,12 +186,14 @@ int wiidebug_init(struct wiimote_data *wdata)
 	dbg->drm = debugfs_create_file("drm", S_IRUSR,
 			dbg->wdata->hdev->debug_dir, dbg, &wiidebug_drm_fops);

+	debugfs_create_u8("player_id", S_IRUSR,
+		   dbg->wdata->hdev->debug_dir, &wdata->player_id);
+
 	spin_lock_irqsave(&wdata->state.lock, flags);
 	wdata->debug = dbg;
 	spin_unlock_irqrestore(&wdata->state.lock, flags);

 	return 0;
-
 }

 void wiidebug_deinit(struct wiimote_data *wdata)
@@ -208,5 +210,6 @@ void wiidebug_deinit(struct wiimote_data *wdata)

 	debugfs_remove(dbg->drm);
 	debugfs_remove(dbg->eeprom);
+	debugfs_lookup_and_remove("player_id", dbg->wdata->hdev->debug_dir);
 	kfree(dbg);
 }
diff --git a/drivers/hid/hid-wiimote.h b/drivers/hid/hid-wiimote.h
index 9c12f63f6dd2..8e5002f515e2 100644
--- a/drivers/hid/hid-wiimote.h
+++ b/drivers/hid/hid-wiimote.h
@@ -153,6 +153,7 @@ struct wiimote_data {
 	struct input_dev *mp;
 	struct timer_list timer;
 	struct wiimote_debug *debug;
+	u8 player_id;

 	union {
 		struct input_dev *input;
-- 
2.55.0
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help