Re: [PATCH v5 RESEND] HID: nintendo: cleanup LED code

From: Jiri Kosina
Date: Wed Oct 04 2023 - 15:09:59 EST


On Sun, 24 Sep 2023, Martino Fontana wrote:

> - 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(-)

Applied, thank you.

--
Jiri Kosina
SUSE Labs