- Support player LED patterns up to 8 players. (Note that the behavior still consinsts in increasing the player number every time a controller is connected, never decreasing it. It should be as is described in https://bugzilla.kernel.org/show_bug.cgi?id=216225. However, any implementation here would stop making sense as soon as a non-Nintendo controller is connected, which is why I'm not bothering.) - Split part of `joycon_home_led_brightness_set` (which is called by hid) into `joycon_set_home_led` (which is what actually sets the LEDs), for consistency with player LEDs. - `joycon_player_led_brightness_set` won't try it to "determine which player led this is" anymore: it's already looking at every LED brightness value. - Instead of first registering the `led_classdev`, then attempting to set the LED and unregistering the `led_classdev` if it fails, first attempt to set the LED, then register the `led_classdev` only if it succeeds (the class is still filled up in either case). - If setting the player LEDs fails, still attempt setting the home LED. (I don't know there's a third party controller where this may actually happen, but who knows...) - Use `JC_NUM_LEDS` where appropriate instead of 4. - Print return codes in more places. - Use spinlock instead of mutex for `input_num`. Copy its value to a local variable, so that it can be unlocked immediately. - `input_num` starts counting from 0 - Less holding of mutexes in general. Signed-off-by: Martino Fontana <tinozzo123@xxxxxxxxx> --- Changes for v2: Applied suggestions, commit message explains more stuff, restored `return ret` when `devm_led_classdev_register` fails (since all other hid drivers do this). If setting LED fails, `hid_warn` now explicitly says "skipping registration". Changes for v3 and v4: Fixed errors and warnings from test robot. Changes for v5: I thought that when connecting the controller on an actual Nintendo Switch, only the nth player LED would turn on, which is true... on Wii and Wii U. So I reverted that, and to compensate, now this supports the LED patterns up to 8 players. drivers/hid/hid-nintendo.c | 133 +++++++++++++++++++++---------------- 1 file changed, 76 insertions(+), 57 deletions(-) diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c index 10468f727..138f154fe 100644 --- a/drivers/hid/hid-nintendo.c +++ b/drivers/hid/hid-nintendo.c @@ -410,6 +410,18 @@ 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 +/* 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 }, + { 1, 1, 0, 0 }, + { 1, 1, 1, 0 }, + { 1, 1, 1, 1 }, + { 1, 0, 0, 1 }, + { 1, 0, 1, 0 }, + { 1, 0, 1, 1 }, + { 0, 1, 1, 0 }, +}; /* Each physical controller is associated with a joycon_ctlr struct */ struct joycon_ctlr { @@ -699,6 +711,25 @@ static int joycon_set_player_leds(struct joycon_ctlr *ctlr, u8 flash, u8 on) return joycon_send_subcmd(ctlr, req, 1, HZ/4); } +static int joycon_set_home_led(struct joycon_ctlr *ctlr, enum led_brightness brightness) +{ + struct joycon_subcmd_request *req; + u8 buffer[sizeof(*req) + 5] = { 0 }; + u8 *data; + + req = (struct joycon_subcmd_request *)buffer; + req->subcmd_id = JC_SUBCMD_SET_HOME_LIGHT; + data = req->data; + data[0] = 0x01; + data[1] = brightness << 4; + data[2] = brightness | (brightness << 4); + data[3] = 0x11; + data[4] = 0x11; + + hid_dbg(ctlr->hdev, "setting home led brightness\n"); + return joycon_send_subcmd(ctlr, req, 5, HZ/4); +} + static int joycon_request_spi_flash_read(struct joycon_ctlr *ctlr, u32 start_addr, u8 size, u8 **reply) { @@ -1840,6 +1871,7 @@ static int joycon_input_create(struct joycon_ctlr *ctlr) return 0; } +/* Because the subcommand sets all the leds at once, the brightness argument is ignored */ static int joycon_player_led_brightness_set(struct led_classdev *led, enum led_brightness brightness) { @@ -1849,7 +1881,6 @@ static int joycon_player_led_brightness_set(struct led_classdev *led, int val = 0; int i; int ret; - int num; ctlr = hid_get_drvdata(hdev); if (!ctlr) { @@ -1857,21 +1888,10 @@ static int joycon_player_led_brightness_set(struct led_classdev *led, return -ENODEV; } - /* determine which player led this is */ - for (num = 0; num < JC_NUM_LEDS; num++) { - if (&ctlr->leds[num] == led) - break; - } - if (num >= JC_NUM_LEDS) - return -EINVAL; + for (i = 0; i < JC_NUM_LEDS; i++) + val |= ctlr->leds[i].brightness << i; mutex_lock(&ctlr->output_mutex); - for (i = 0; i < JC_NUM_LEDS; i++) { - if (i == num) - val |= brightness << i; - else - val |= ctlr->leds[i].brightness << i; - } ret = joycon_set_player_leds(ctlr, 0, val); mutex_unlock(&ctlr->output_mutex); @@ -1884,9 +1904,6 @@ static int joycon_home_led_brightness_set(struct led_classdev *led, struct device *dev = led->dev->parent; struct hid_device *hdev = to_hid_device(dev); struct joycon_ctlr *ctlr; - struct joycon_subcmd_request *req; - u8 buffer[sizeof(*req) + 5] = { 0 }; - u8 *data; int ret; ctlr = hid_get_drvdata(hdev); @@ -1894,43 +1911,35 @@ static int joycon_home_led_brightness_set(struct led_classdev *led, hid_err(hdev, "No controller data\n"); return -ENODEV; } - - req = (struct joycon_subcmd_request *)buffer; - req->subcmd_id = JC_SUBCMD_SET_HOME_LIGHT; - data = req->data; - data[0] = 0x01; - data[1] = brightness << 4; - data[2] = brightness | (brightness << 4); - data[3] = 0x11; - data[4] = 0x11; - - hid_dbg(hdev, "setting home led brightness\n"); mutex_lock(&ctlr->output_mutex); - ret = joycon_send_subcmd(ctlr, req, 5, HZ/4); + ret = joycon_set_home_led(ctlr, brightness); mutex_unlock(&ctlr->output_mutex); - return ret; } -static DEFINE_MUTEX(joycon_input_num_mutex); +static DEFINE_SPINLOCK(joycon_input_num_spinlock); static int joycon_leds_create(struct joycon_ctlr *ctlr) { struct hid_device *hdev = ctlr->hdev; struct device *dev = &hdev->dev; const char *d_name = dev_name(dev); struct led_classdev *led; + int led_val = 0; char *name; - int ret = 0; + int ret; int i; - static int input_num = 1; + unsigned long flags; + int player_led_pattern; + static int input_num; - /* Set the default controller player leds based on controller number */ - mutex_lock(&joycon_input_num_mutex); - mutex_lock(&ctlr->output_mutex); - ret = joycon_set_player_leds(ctlr, 0, 0xF >> (4 - input_num)); - if (ret) - hid_warn(ctlr->hdev, "Failed to set leds; ret=%d\n", ret); - mutex_unlock(&ctlr->output_mutex); + /* + * 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 */ for (i = 0; i < JC_NUM_LEDS; i++) { @@ -1938,31 +1947,37 @@ static int joycon_leds_create(struct joycon_ctlr *ctlr) d_name, "green", joycon_player_led_names[i]); - if (!name) { - mutex_unlock(&joycon_input_num_mutex); + if (!name) return -ENOMEM; - } led = &ctlr->leds[i]; led->name = name; - led->brightness = ((i + 1) <= input_num) ? 1 : 0; + led->brightness = joycon_player_led_patterns[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; + } + mutex_lock(&ctlr->output_mutex); + ret = joycon_set_player_leds(ctlr, 0, led_val); + mutex_unlock(&ctlr->output_mutex); + if (ret) { + hid_warn(hdev, "Failed to set players LEDs, skipping registration; ret=%d\n", ret); + goto home_led; + } + + for (i = 0; i < JC_NUM_LEDS; i++) { + led = &ctlr->leds[i]; ret = devm_led_classdev_register(&hdev->dev, led); if (ret) { - hid_err(hdev, "Failed registering %s LED\n", led->name); - mutex_unlock(&joycon_input_num_mutex); + hid_err(hdev, "Failed to register player %d LED; ret=%d\n", i + 1, ret); return ret; } } - if (++input_num > 4) - input_num = 1; - mutex_unlock(&joycon_input_num_mutex); - +home_led: /* configure the home LED */ if (jc_type_has_right(ctlr)) { name = devm_kasprintf(dev, GFP_KERNEL, "%s:%s:%s", @@ -1978,16 +1993,20 @@ static int joycon_leds_create(struct joycon_ctlr *ctlr) led->max_brightness = 0xF; led->brightness_set_blocking = joycon_home_led_brightness_set; led->flags = LED_CORE_SUSPENDRESUME | LED_HW_PLUGGABLE; - ret = devm_led_classdev_register(&hdev->dev, led); + + /* Set the home LED to 0 as default state */ + mutex_lock(&ctlr->output_mutex); + ret = joycon_set_home_led(ctlr, 0); + mutex_unlock(&ctlr->output_mutex); if (ret) { - hid_err(hdev, "Failed registering home led\n"); - return ret; + hid_warn(hdev, "Failed to set home LED, skipping registration; ret=%d\n", ret); + return 0; } - /* Set the home LED to 0 as default state */ - ret = joycon_home_led_brightness_set(led, 0); + + ret = devm_led_classdev_register(&hdev->dev, led); if (ret) { - hid_warn(hdev, "Failed to set home LED default, unregistering home LED"); - devm_led_classdev_unregister(&hdev->dev, led); + hid_err(hdev, "Failed to register home LED; ret=%d\n", ret); + return ret; } } -- 2.42.0
