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

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <nycvar.YFH.7.76.2310042107570.3534@cbobk.fhfr.pm>
Date:   Wed, 4 Oct 2023 21:08:10 +0200 (CEST)
From:   Jiri Kosina <jikos@...nel.org>
To:     Martino Fontana <tinozzo123@...il.com>
cc:     djogorchock@...il.com, benjamin.tissoires@...hat.com,
        linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 RESEND] HID: nintendo: cleanup LED code

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@...il.com>
> ---
> 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