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

Hi

On Sun, Aug 2, 2026, at 2:21 AM, Rafael Passos wrote:

On Fri Jul 31, 2026 at 6:02 AM -03, David Rheinsberg wrote:

quoted

quoted

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

`wiimote_create()` should initialize `wdata->player_id` to -1, so `ida_free()` is a no-op in the `err_free` path. Right now it is implicitly initialized to 0, and thus ida_free() will warn about an unallocated ID.

Also: Why not just use IDs starting from 0?

I chose to start from 1 to avoid "id 0 = player 1".

I think this would be perfectly fine, but I don't insist on changing this.

Wouldn't setting an u8 to -1 result into 255 ? If so,
this would not be a no-op. Also, comparing player_id < 0 would never be
true. If the IDA starts from 0, there is no usable "empty value" in a u8
when deciding if I should call ida_free.
If I let "failure" to be 0, I can just use this in destroy:

if (wdata->player_id)
	ida_free(&wiimote_ida, wdata->player_id);

Whatever you decide on using as invalid value, you need to check it before calling into ida_free(). If you reserve 0, use what you suggested. If you put -1 in, check for -1 / 255 (even though technically you should then also limit the allocations to 254). Or change the type to s8 or int, or whatever works.

Thanks
David