[PATCH v4 0/4] HID: wiimote: new LED behavior on connect + scoped_guards

Hi,
This patchset contains one feature change, and 3 patches appying 
scoped cleanup to locking and to the initialization functions for
the led probing and the main wiimote probe call.

The feature is turning different LEDs for each of the first 4 wiimotes connected.
From id 5 forward, the LED will cycle back to 1, and so on.
This uses the ida struct, so its quite simple and lightweight.
The hid_info log message prints out the controller id.

While implementing this feature, I decided to cleanup the code using
scoped_guard for the many spinlocks in the driver. There are two places
where the original lock/unlock version fits best, and I left them
untouched.
I also used the __free scope cleanup in the wiimote_probe and LED probe.
This was trivial for the LED probe.
The wiimote_probe required a new state tracker bitmask, now the wiimote_destroy
is used both on disconnect (hid_remove) and in the probe cleanup.

It was really fun working with this driver.
I tested it with 4 Wii Motion Plus remotes (gen2).
Video recording of my tests (48s video).
https://rcpassos.me/video/wiimote-led-linux-driver

Thanks,
Rafael Passos

---
V1: https://lore.kernel.org/linux-input/[email protected]/
Changes from v1:
    (1/3):
    - fix ida_alloc_min error handling to consider negative values
    - remove fallback to 1 on ida_alloc_min failure
    - move player_leds static array to hid-wiimote-core.c
    - s/instance_id/player_id/g
    - store player_id on an u8
    (2/3):
    - add header include for cleanup.h
    - add identation to one-liner scoped_guards
    (3/3):
    - add scoped cleanup function to wiimote_probe, with a bitmask to track state
      Patch used for testing this:
      https://lore.kernel.org/linux-input/[email protected]/
    (4/4) *new patch* :
    - sashiko found a pre-existing uaf. Unlikely, but correct.
      implemented using the playstation driver as an inspiration

V2: https://lore.kernel.org/linux-input/[email protected]/
Changes from v2:
    (2/4):
    - join the last two locks into a single scoped_guard lock in wiimote_modules_load

V3: https://lore.rcpassos.me/wiimote/[email protected]/
Changes from v3:
    - dropped false uaf patch (previous 4/4). It was a false alarm.
      Discussion in:
      https://lore.kernel.org/linux-input/[email protected]/T/#t

    (1/4):
    - I tested how the changes looked if using ida from 0 instead of 1,
      and I believe it ended up less clean. I decided to keep them as is,
      with a few additional comments.
    - explicit initialization of player_id to 0 before allocating an id
    - avoid using a new int during ida_alloc (use ret instead)
    - use u8 instead of __u8
    - move ida_remove from wiimote_hid_remove to wiimote_destroy
    - add debugfs entry for the player_id entry
    (3/4):
    - move changes to wiimote_probe from this patch to the next
    - update patch title
    (4/4): *new patch*
    - scoped cleanup in the wiimote_probe call, using a bitmask to keep
      track of state during initialization
    - update wiimote_destroy so it can be the cleanup function
    - add a debugfs entry for this new entry

As we discussed in the v3, tell me if you like the changes in patch 4/4.
If you prefer not to apply it, I can send a new patchset revision, or
just drop it from the set if nothing else needs changes.

Thanks!


Rafael Passos (4):
  HID: wiimote: turn on the LEDs indicating the controller id
  HID: wiimote: replace spinlock pairs with scoped_guard
  HID: wiimote: led_probe with scoped cleanup
  HID: wiimote: wiimote_probe with scoped cleanup

 drivers/hid/hid-wiimote-core.c    | 346 ++++++++++++++++--------------
 drivers/hid/hid-wiimote-debug.c   |  71 +++---
 drivers/hid/hid-wiimote-modules.c |  24 +--
 drivers/hid/hid-wiimote.h         |  10 +
 4 files changed, 240 insertions(+), 211 deletions(-)

-- 
2.55.0