All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] HID: nintendo: use slots for led patterns
@ 2023-12-08  7:59 Ryan McClelland
  2023-12-08 16:15 ` [PATCH] HID: nintendo: use ida for LED player id Martino Fontana
  0 siblings, 1 reply; 3+ messages in thread
From: Ryan McClelland @ 2023-12-08  7:59 UTC (permalink / raw)
  To: linux-input; +Cc: djogorchock, benjamin.tissoires, jikos, Ryan McClelland

Previously, the leds pattern would just increment with every controller
connected. This wouldn't take into consideration when controllers are
disconnected. The same controller could be connected and disconnected
with the pattern increasing player count each time.

This changes it so now a bitmask is used defining the free controller
slots for each "player". When a controller is connected, it searches
for the first bit in the bitmask that is available and acquires that
"slot" for it's led pattern.

When a controller is disconnected, it sets the position back to 'free'
allowing for a new controller that is to be connected to obtain that
slot.

Signed-off-by: Ryan McClelland <rymcclel@gmail.com>
---
 drivers/hid/hid-nintendo.c | 45 ++++++++++++++++++++++++++++----------
 1 file changed, 33 insertions(+), 12 deletions(-)

diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index 138f154fecef..cfe43a6b5a46 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -410,7 +410,8 @@ static const char * const joycon_player_led_names[] = {
 	LED_FUNCTION_PLAYER4,
 };
 #define JC_NUM_LEDS		ARRAY_SIZE(joycon_player_led_names)
-#define JC_NUM_LED_PATTERNS 8
+#define JC_NUM_LED_PATTERNS	9
+#define JC_LED_DEF_PATTERN	8
 /* Taken from https://www.nintendo.com/my/support/qa/detail/33822 */
 static const enum led_brightness joycon_player_led_patterns[JC_NUM_LED_PATTERNS][JC_NUM_LEDS] = {
 	{ 1, 0, 0, 0 },
@@ -421,8 +422,13 @@ static const enum led_brightness joycon_player_led_patterns[JC_NUM_LED_PATTERNS]
 	{ 1, 0, 1, 0 },
 	{ 1, 0, 1, 1 },
 	{ 0, 1, 1, 0 },
+	{ 1, 1, 1, 1 }, /* >8 players */
 };
 
+/* Used to set the number of leds on each controller */
+static DEFINE_SPINLOCK(joycon_input_bm_spinlock);
+static int jc_input_free_bm = GENMASK(7, 0);
+
 /* Each physical controller is associated with a joycon_ctlr struct */
 struct joycon_ctlr {
 	struct hid_device *hdev;
@@ -491,6 +497,9 @@ struct joycon_ctlr {
 	unsigned int imu_delta_samples_count;
 	unsigned int imu_delta_samples_sum;
 	unsigned int imu_avg_delta_ms;
+
+	/* led */
+	int player_led_pattern;
 };
 
 /* Helper macros for checking controller type */
@@ -1917,7 +1926,6 @@ static int joycon_home_led_brightness_set(struct led_classdev *led,
 	return ret;
 }
 
-static DEFINE_SPINLOCK(joycon_input_num_spinlock);
 static int joycon_leds_create(struct joycon_ctlr *ctlr)
 {
 	struct hid_device *hdev = ctlr->hdev;
@@ -1929,17 +1937,24 @@ static int joycon_leds_create(struct joycon_ctlr *ctlr)
 	int ret;
 	int i;
 	unsigned long flags;
-	int player_led_pattern;
-	static int input_num;
 
 	/*
-	 * Set the player leds based on controller number
-	 * Because there is no standard concept of "player number", the pattern
-	 * number will simply increase by 1 every time a controller is connected.
+	 * Set the player leds based on a free slot. If there
+	 * are more than 8 controllers. Then load the default
+	 * pattern.
 	 */
-	spin_lock_irqsave(&joycon_input_num_spinlock, flags);
-	player_led_pattern = input_num++ % JC_NUM_LED_PATTERNS;
-	spin_unlock_irqrestore(&joycon_input_num_spinlock, flags);
+	spin_lock_irqsave(&joycon_input_bm_spinlock, flags);
+	if (!jc_input_free_bm) {
+		ctlr->player_led_pattern = JC_LED_DEF_PATTERN;
+	} else {
+		ctlr->player_led_pattern = ffs(jc_input_free_bm) - 1;
+		jc_input_free_bm &= ~(BIT(ctlr->player_led_pattern));
+	}
+	spin_unlock_irqrestore(&joycon_input_bm_spinlock, flags);
+	if (ctlr->player_led_pattern == JC_LED_DEF_PATTERN)
+		hid_info(ctlr->hdev, "more than 8 controllers connected, assigning default led pattern");
+	else
+		hid_info(ctlr->hdev, "assigned player %d led pattern", ctlr->player_led_pattern + 1);
 
 	/* configure the player LEDs */
 	for (i = 0; i < JC_NUM_LEDS; i++) {
@@ -1952,13 +1967,13 @@ static int joycon_leds_create(struct joycon_ctlr *ctlr)
 
 		led = &ctlr->leds[i];
 		led->name = name;
-		led->brightness = joycon_player_led_patterns[player_led_pattern][i];
+		led->brightness = joycon_player_led_patterns[ctlr->player_led_pattern][i];
 		led->max_brightness = 1;
 		led->brightness_set_blocking =
 					joycon_player_led_brightness_set;
 		led->flags = LED_CORE_SUSPENDRESUME | LED_HW_PLUGGABLE;
 
-		led_val |= joycon_player_led_patterns[player_led_pattern][i] << i;
+		led_val |= joycon_player_led_patterns[ctlr->player_led_pattern][i] << i;
 	}
 	mutex_lock(&ctlr->output_mutex);
 	ret = joycon_set_player_leds(ctlr, 0, led_val);
@@ -2409,6 +2424,12 @@ static void nintendo_hid_remove(struct hid_device *hdev)
 	ctlr->ctlr_state = JOYCON_CTLR_STATE_REMOVED;
 	spin_unlock_irqrestore(&ctlr->lock, flags);
 
+	/* Controller removed, so free the slot */
+	spin_lock_irqsave(&joycon_input_bm_spinlock, flags);
+	if (ctlr->player_led_pattern != JC_LED_DEF_PATTERN)
+		jc_input_free_bm |= BIT(ctlr->player_led_pattern);
+	spin_unlock_irqrestore(&joycon_input_bm_spinlock, flags);
+
 	destroy_workqueue(ctlr->rumble_queue);
 
 	hid_hw_close(hdev);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH] HID: nintendo: use ida for LED player id
  2023-12-08  7:59 [PATCH] HID: nintendo: use slots for led patterns Ryan McClelland
@ 2023-12-08 16:15 ` Martino Fontana
  0 siblings, 0 replies; 3+ messages in thread
From: Martino Fontana @ 2023-12-08 16:15 UTC (permalink / raw)
  To: rymcclel
  Cc: benjamin.tissoires, djogorchock, jikos, linux-input, Martino Fontana

Signed-off-by: Martino Fontana <tinozzo123@gmail.com>
---
 drivers/hid/hid-nintendo.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index 138f154fe..22a3c97a0 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -28,6 +28,7 @@
 #include <linux/device.h>
 #include <linux/kernel.h>
 #include <linux/hid.h>
+#include <linux/idr.h>
 #include <linux/input.h>
 #include <linux/jiffies.h>
 #include <linux/leds.h>
@@ -427,6 +428,7 @@ static const enum led_brightness joycon_player_led_patterns[JC_NUM_LED_PATTERNS]
 struct joycon_ctlr {
 	struct hid_device *hdev;
 	struct input_dev *input;
+	u32 player_id;
 	struct led_classdev leds[JC_NUM_LEDS]; /* player leds */
 	struct led_classdev home_led;
 	enum joycon_ctlr_state ctlr_state;
@@ -1917,7 +1919,8 @@ static int joycon_home_led_brightness_set(struct led_classdev *led,
 	return ret;
 }
 
-static DEFINE_SPINLOCK(joycon_input_num_spinlock);
+static DEFINE_IDA(nintendo_player_id_allocator);
+
 static int joycon_leds_create(struct joycon_ctlr *ctlr)
 {
 	struct hid_device *hdev = ctlr->hdev;
@@ -1928,20 +1931,15 @@ static int joycon_leds_create(struct joycon_ctlr *ctlr)
 	char *name;
 	int ret;
 	int i;
-	unsigned long flags;
-	int player_led_pattern;
-	static int input_num;
-
-	/*
-	 * Set the player leds based on controller number
-	 * Because there is no standard concept of "player number", the pattern
-	 * number will simply increase by 1 every time a controller is connected.
-	 */
-	spin_lock_irqsave(&joycon_input_num_spinlock, flags);
-	player_led_pattern = input_num++ % JC_NUM_LED_PATTERNS;
-	spin_unlock_irqrestore(&joycon_input_num_spinlock, flags);
 
 	/* configure the player LEDs */
+	ret = ida_alloc(&nintendo_player_id_allocator, GFP_KERNEL);
+	if (ret < 0) {
+		hid_warn(hdev, "Failed to allocate player ID, skipping; ret=%d\n", ret);
+		goto home_led;
+	}
+	ctlr->player_id = ret % JC_NUM_LED_PATTERNS;
+
 	for (i = 0; i < JC_NUM_LEDS; i++) {
 		name = devm_kasprintf(dev, GFP_KERNEL, "%s:%s:%s",
 				      d_name,
@@ -1952,13 +1950,13 @@ static int joycon_leds_create(struct joycon_ctlr *ctlr)
 
 		led = &ctlr->leds[i];
 		led->name = name;
-		led->brightness = joycon_player_led_patterns[player_led_pattern][i];
+		led->brightness = joycon_player_led_patterns[ctlr->player_id][i];
 		led->max_brightness = 1;
 		led->brightness_set_blocking =
 					joycon_player_led_brightness_set;
 		led->flags = LED_CORE_SUSPENDRESUME | LED_HW_PLUGGABLE;
 
-		led_val |= joycon_player_led_patterns[player_led_pattern][i] << i;
+		led_val |= joycon_player_led_patterns[ctlr->player_id][i] << i;
 	}
 	mutex_lock(&ctlr->output_mutex);
 	ret = joycon_set_player_leds(ctlr, 0, led_val);
@@ -2410,6 +2408,7 @@ static void nintendo_hid_remove(struct hid_device *hdev)
 	spin_unlock_irqrestore(&ctlr->lock, flags);
 
 	destroy_workqueue(ctlr->rumble_queue);
+	ida_free(&nintendo_player_id_allocator, ctlr->player_id);
 
 	hid_hw_close(hdev);
 	hid_hw_stop(hdev);
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] HID: nintendo: use slots for led patterns
@ 2023-12-08 16:18 Martino Fontana
  0 siblings, 0 replies; 3+ messages in thread
From: Martino Fontana @ 2023-12-08 16:18 UTC (permalink / raw)
  To: rymcclel
  Cc: Benjamin Tissoires, Daniel J. Ogorchock, Jiri Kosina, linux-input

Funny timing, I was working on a patch for the same thing too.
Instead of using a bitmask, I used ida_alloc (just like hid-playstation does).
I'll leave my version of the patch here, for comparison's sake.
(It has no description, and it wraps on the 8th player)
I can't properly test either of them, since I have only one controller (though
every time I connect it, only the first LED light up, as expected).

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-12-08 16:18 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-08  7:59 [PATCH] HID: nintendo: use slots for led patterns Ryan McClelland
2023-12-08 16:15 ` [PATCH] HID: nintendo: use ida for LED player id Martino Fontana
2023-12-08 16:18 [PATCH] HID: nintendo: use slots for led patterns Martino Fontana

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.