| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAEVj2tkb5WTj9gAJmxgnymPRi7sdO3HbvMn88wOHgby2XgXHjQ@mail.gmail.com>
Date: Sun, 14 Sep 2025 00:40:09 -0400
From: Daniel Ogorchock <djogorchock@...il.com>
To: Noa <coolreader18@...il.com>
Cc: linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] HID: nintendo: enable HW LED blinking
Hi Noa,
I tried building with this version of the patch and hit the following error:
drivers/hid/hid-nintendo.c:2229:14: error: type defaults to ‘int’ in
declaration of ‘JC_LED_BLINK_DELAY_ON_MS’ [-Wimplicit-int]
2229 | static const JC_LED_BLINK_DELAY_ON_MS = 500;
| ^~~~~~~~~~~~~~~~~~~~~~~~
drivers/hid/hid-nintendo.c:2230:14: error: type defaults to ‘int’ in
declaration of ‘JC_LED_BLINK_DELAY_OFF_MS’ [-Wimplicit-int]
2230 | static const JC_LED_BLINK_DELAY_OFF_MS = 200;
| ^~~~~~~~~~~~~~~~~~~~~~~~~
make[4]: *** [scripts/Makefile.build:287: drivers/hid/hid-nintendo.o] Error 1
make[3]: *** [scripts/Makefile.build:555: drivers/hid] Error 2
Making these 'static const unsigned long' fixed that issue. Probably
toolchain version dependent whether it succeeds or fails with the
implicit int.
I also noticed that the HW blink functionality only works in bluetooth
mode. In USB mode it forces the LEDs on.
That seems to match up with the note here:
https://github.com/dekuNukem/Nintendo_Switch_Reverse_Engineering/blob/master/bluetooth_hid_subcommands_notes.md#subcommand-0x30-set-player-lights
I was able to modify your patch to gate the new HW logic to only be
active when using bluetooth.
Also please include the commit summary message/description above your
signed-off-by line like you did in v1 of the patch.
Below is my quick modification to your patch that seemed to get things
working right on my local setup. Please feel free to
incorporate it or make your own modifications in v3 to fix the USB
mode. It's probably also worth adding a comment describing
why it doesn't use the HW support for USB:
---
drivers/hid/hid-nintendo.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index 14aed2500a96..03f24e709949 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -2201,7 +2201,8 @@ static int joycon_update_player_leds(struct device *dev)
}
for (i = 0; i < JC_NUM_LEDS; i++) {
- if (ctlr->leds[i].blink_delay_on || ctlr->leds[i].blink_delay_off)
+ if ((ctlr->leds[i].blink_delay_on || ctlr->leds[i].blink_delay_off) &&
+ !joycon_using_usb(ctlr))
flash |= 1 << i;
else if (ctlr->leds[i].brightness)
val |= 1 << i;
@@ -2217,17 +2218,26 @@ static int joycon_update_player_leds(struct device *dev)
static int joycon_player_led_brightness_set(struct led_classdev *led,
enum led_brightness brightness)
{
+ struct hid_device *hdev = to_hid_device(led->dev->parent);
+ struct joycon_ctlr *ctlr;
+
+ ctlr = hid_get_drvdata(hdev);
+ if (!ctlr) {
+ hid_err(hdev, "No controller data\n");
+ return -ENODEV;
+ }
+
led->brightness = brightness;
- if (!brightness)
+ if (!brightness && !joycon_using_usb(ctlr))
led->blink_delay_on = led->blink_delay_off = 0;
return joycon_update_player_leds(led->dev->parent);
}
/* the blink period of the leds can't be changed, and is always these values */
-static const JC_LED_BLINK_DELAY_ON_MS = 500;
-static const JC_LED_BLINK_DELAY_OFF_MS = 200;
+static const unsigned long JC_LED_BLINK_DELAY_ON_MS = 500;
+static const unsigned long JC_LED_BLINK_DELAY_OFF_MS = 200;
/* the different leds on a joycon can't actually be set to hw blink
independently
* of each other, since they all use the same one subcommand, so this function
* actually resets the cycle of all the leds */
@@ -2299,7 +2309,8 @@ static int joycon_leds_create(struct joycon_ctlr *ctlr)
led->max_brightness = 1;
led->brightness_set_blocking =
joycon_player_led_brightness_set;
- led->blink_set = joycon_player_led_blink_set;
+ if (!joycon_using_usb(ctlr))
+ led->blink_set = joycon_player_led_blink_set;
led->flags = LED_CORE_SUSPENDRESUME | LED_HW_PLUGGABLE;
led_val |= joycon_player_led_patterns[player_led_pattern][i] << i;
--
2.51.0
On Fri, Feb 14, 2025 at 5:58 PM Noa <coolreader18@...il.com> wrote:
>
> Updated with proper commit title and clearer constant names.
>
> Signed-off-by: Noa <coolreader18@...il.com>
> ---
> drivers/hid/hid-nintendo.c | 45 ++++++++++++++++++++++++++++++++------
> 1 file changed, 38 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> index 11ac246176ae..f4cdd35eef2a 100644
> --- a/drivers/hid/hid-nintendo.c
> +++ b/drivers/hid/hid-nintendo.c
> @@ -2183,14 +2183,13 @@ 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)
> +/* Update the on/flash status of the leds according to their led_classdev fields */
> +static int joycon_update_player_leds(struct device *dev)
> {
> - struct device *dev = led->dev->parent;
> struct hid_device *hdev = to_hid_device(dev);
> struct joycon_ctlr *ctlr;
> int val = 0;
> + int flash = 0;
> int i;
> int ret;
>
> @@ -2200,16 +2199,47 @@ static int joycon_player_led_brightness_set(struct led_classdev *led,
> return -ENODEV;
> }
>
> - for (i = 0; i < JC_NUM_LEDS; i++)
> - val |= ctlr->leds[i].brightness << i;
> + for (i = 0; i < JC_NUM_LEDS; i++) {
> + if (ctlr->leds[i].blink_delay_on || ctlr->leds[i].blink_delay_off)
> + flash |= 1 << i;
> + else if (ctlr->leds[i].brightness)
> + val |= 1 << i;
> + }
>
> mutex_lock(&ctlr->output_mutex);
> - ret = joycon_set_player_leds(ctlr, 0, val);
> + ret = joycon_set_player_leds(ctlr, flash, val);
> mutex_unlock(&ctlr->output_mutex);
>
> return ret;
> }
>
> +static int joycon_player_led_brightness_set(struct led_classdev *led,
> + enum led_brightness brightness)
> +{
> + led->brightness = brightness;
> +
> + if (!brightness)
> + led->blink_delay_on = led->blink_delay_off = 0;
> +
> + return joycon_update_player_leds(led->dev->parent);
> +}
> +
> +/* the blink period of the leds can't be changed, and is always these values */
> +static const JC_LED_BLINK_DELAY_ON_MS = 500;
> +static const JC_LED_BLINK_DELAY_OFF_MS = 200;
> +/* the different leds on a joycon can't actually be set to hw blink independently
> + * of each other, since they all use the same one subcommand, so this function
> + * actually resets the cycle of all the leds */
> +static int joycon_player_led_blink_set(struct led_classdev *led,
> + unsigned long *delay_on,
> + unsigned long *delay_off)
> +{
> + led->blink_delay_on = *delay_on = JC_LED_BLINK_DELAY_ON_MS;
> + led->blink_delay_off = *delay_off = JC_LED_BLINK_DELAY_OFF_MS;
> +
> + return joycon_update_player_leds(led->dev->parent);
> +}
> +
> static int joycon_home_led_brightness_set(struct led_classdev *led,
> enum led_brightness brightness)
> {
> @@ -2268,6 +2298,7 @@ static int joycon_leds_create(struct joycon_ctlr *ctlr)
> led->max_brightness = 1;
> led->brightness_set_blocking =
> joycon_player_led_brightness_set;
> + led->blink_set = joycon_player_led_blink_set;
> led->flags = LED_CORE_SUSPENDRESUME | LED_HW_PLUGGABLE;
>
> led_val |= joycon_player_led_patterns[player_led_pattern][i] << i;
> --
> 2.48.1
>
Thanks,
Daniel
Powered by blists - more mailing lists