From: "David Rheinsberg" <david@readahead.eu>
To: "Rafael Passos" <rafael@rcpassos.me>,
"Benjamin Tissoires" <bentiss@kernel.org>,
"Jiri Kosina" <jikos@kernel.org>
Cc: "Shuah Khan" <skhan@linuxfoundation.org>,
"Brigham Campbell" <me@brighamcampbell.com>,
"Jori Koolstra" <jkoolstra@xs4all.nl>,
linux-input@vger.kernel.org
Subject: Re: [PATCH v4 2/4] HID: wiimote: replace spinlock pairs with scoped_guard
Date: Sun, 20 Sep 2026 10:12:07 +0200 [thread overview]
Message-ID: <95661df0-11cf-47dd-976b-3ccc895c0db5@app.fastmail.com> (raw)
In-Reply-To: <20260817213840.1053216-3-rafael@rcpassos.me>
Hi
On Mon, Aug 17, 2026, at 11:38 PM, Rafael Passos wrote:
> Cleanup code replacing manual lock/unlock with scoped guards.
> This does not change any behavior, but makes it safer to modify.
>
> The multi line spinlock blocks were replaced by braced scoped_guard,
> and one-liners by a scoped_guard without braces nor indentation.
>
> There are two cases left in this driver using lock/unlock, because
> guard would make the code more complex than current implementation.
>
> Signed-off-by: Rafael Passos <rafael@rcpassos.me>
There is some whitespace/tab indentation mixup, otherwise:
Reviewed-by: David Rheinsberg <david@readahead.eu>
Thanks
David
> ---
> drivers/hid/hid-wiimote-core.c | 224 +++++++++++++-----------------
> drivers/hid/hid-wiimote-debug.c | 62 ++++-----
> drivers/hid/hid-wiimote-modules.c | 7 +-
> 3 files changed, 127 insertions(+), 166 deletions(-)
>
> diff --git a/drivers/hid/hid-wiimote-core.c b/drivers/hid/hid-wiimote-core.c
> index acf31d8b6991..05f8ddb7909b 100644
> --- a/drivers/hid/hid-wiimote-core.c
> +++ b/drivers/hid/hid-wiimote-core.c
> @@ -7,6 +7,7 @@
> /*
> */
>
> +#include <linux/cleanup.h>
> #include <linux/completion.h>
> #include <linux/device.h>
> #include <linux/hid.h>
> @@ -362,13 +363,12 @@ void wiiproto_req_rmem(struct wiimote_data
> *wdata, bool eeprom, __u32 offset,
> int wiimote_cmd_write(struct wiimote_data *wdata, __u32 offset,
> const __u8 *wmem, __u8 size)
> {
> - unsigned long flags;
> int ret;
>
> - spin_lock_irqsave(&wdata->state.lock, flags);
> - wiimote_cmd_set(wdata, WIIPROTO_REQ_WMEM, 0);
> - wiiproto_req_wreg(wdata, offset, wmem, size);
> - spin_unlock_irqrestore(&wdata->state.lock, flags);
> + scoped_guard(spinlock_irqsave, &wdata->state.lock) {
> + wiimote_cmd_set(wdata, WIIPROTO_REQ_WMEM, 0);
> + wiiproto_req_wreg(wdata, offset, wmem, size);
> + }
>
> ret = wiimote_cmd_wait(wdata);
> if (!ret && wdata->state.cmd_err)
> @@ -381,21 +381,19 @@ int wiimote_cmd_write(struct wiimote_data *wdata,
> __u32 offset,
> ssize_t wiimote_cmd_read(struct wiimote_data *wdata, __u32 offset,
> __u8 *rmem,
> __u8 size)
> {
> - unsigned long flags;
> ssize_t ret;
>
> - spin_lock_irqsave(&wdata->state.lock, flags);
> - wdata->state.cmd_read_size = size;
> - wdata->state.cmd_read_buf = rmem;
> - wiimote_cmd_set(wdata, WIIPROTO_REQ_RMEM, offset & 0xffff);
> - wiiproto_req_rreg(wdata, offset, size);
> - spin_unlock_irqrestore(&wdata->state.lock, flags);
> + scoped_guard(spinlock_irqsave, &wdata->state.lock) {
> + wdata->state.cmd_read_size = size;
> + wdata->state.cmd_read_buf = rmem;
> + wiimote_cmd_set(wdata, WIIPROTO_REQ_RMEM, offset & 0xffff);
> + wiiproto_req_rreg(wdata, offset, size);
> + }
>
> ret = wiimote_cmd_wait(wdata);
>
> - spin_lock_irqsave(&wdata->state.lock, flags);
> - wdata->state.cmd_read_buf = NULL;
> - spin_unlock_irqrestore(&wdata->state.lock, flags);
> + scoped_guard(spinlock_irqsave, &wdata->state.lock)
> + wdata->state.cmd_read_buf = NULL;
>
> if (!ret) {
> if (wdata->state.cmd_read_size == 0)
> @@ -675,11 +673,8 @@ static void wiimote_modules_load(struct
> wiimote_data *wdata,
> goto error;
> }
>
> - spin_lock_irq(&wdata->state.lock);
> - wdata->state.devtype = devtype;
> - spin_unlock_irq(&wdata->state.lock);
> -
> scoped_guard(spinlock_irqsave, &wdata->state.lock) {
> + wdata->state.devtype = devtype;
> /* after loading modules, set the Player ID LED cycling from 1 to 4*/
> wiiproto_req_leds(wdata, player_leds[(wdata->player_id - 1) % 4]);
> }
> @@ -703,13 +698,11 @@ static void wiimote_modules_unload(struct
> wiimote_data *wdata)
> {
> const __u8 *mods, *iter;
> const struct wiimod_ops *ops;
> - unsigned long flags;
>
> mods = wiimote_devtype_mods[wdata->state.devtype];
>
> - spin_lock_irqsave(&wdata->state.lock, flags);
> - wdata->state.devtype = WIIMOTE_DEV_UNKNOWN;
> - spin_unlock_irqrestore(&wdata->state.lock, flags);
> + scoped_guard(spinlock_irqsave, &wdata->state.lock)
> + wdata->state.devtype = WIIMOTE_DEV_UNKNOWN;
>
> /* find end of list */
> for (iter = mods; *iter != WIIMOD_NULL; ++iter)
> @@ -736,7 +729,6 @@ static void wiimote_modules_unload(struct
> wiimote_data *wdata)
>
> static void wiimote_ext_load(struct wiimote_data *wdata, unsigned int ext)
> {
> - unsigned long flags;
> const struct wiimod_ops *ops;
> int ret;
>
> @@ -748,22 +740,20 @@ static void wiimote_ext_load(struct wiimote_data
> *wdata, unsigned int ext)
> ext = WIIMOTE_EXT_UNKNOWN;
> }
>
> - spin_lock_irqsave(&wdata->state.lock, flags);
> - wdata->state.exttype = ext;
> - spin_unlock_irqrestore(&wdata->state.lock, flags);
> + scoped_guard(spinlock_irqsave, &wdata->state.lock)
> + wdata->state.exttype = ext;
> }
>
> static void wiimote_ext_unload(struct wiimote_data *wdata)
> {
> - unsigned long flags;
> const struct wiimod_ops *ops;
>
> ops = wiimod_ext_table[wdata->state.exttype];
>
> - spin_lock_irqsave(&wdata->state.lock, flags);
> - wdata->state.exttype = WIIMOTE_EXT_UNKNOWN;
> - wdata->state.flags &= ~WIIPROTO_FLAG_EXT_USED;
> - spin_unlock_irqrestore(&wdata->state.lock, flags);
> + scoped_guard(spinlock_irqsave, &wdata->state.lock) {
> + wdata->state.exttype = WIIMOTE_EXT_UNKNOWN;
> + wdata->state.flags &= ~WIIPROTO_FLAG_EXT_USED;
> + }
>
> if (ops->remove)
> ops->remove(ops, wdata);
> @@ -771,7 +761,6 @@ static void wiimote_ext_unload(struct wiimote_data *wdata)
>
> static void wiimote_mp_load(struct wiimote_data *wdata)
> {
> - unsigned long flags;
> const struct wiimod_ops *ops;
> int ret;
> __u8 mode = 2;
> @@ -783,14 +772,12 @@ static void wiimote_mp_load(struct wiimote_data *wdata)
> mode = 1;
> }
>
> - spin_lock_irqsave(&wdata->state.lock, flags);
> - wdata->state.mp = mode;
> - spin_unlock_irqrestore(&wdata->state.lock, flags);
> + scoped_guard(spinlock_irqsave, &wdata->state.lock)
> + wdata->state.mp = mode;
> }
>
> static void wiimote_mp_unload(struct wiimote_data *wdata)
> {
> - unsigned long flags;
> const struct wiimod_ops *ops;
>
> if (wdata->state.mp < 2)
> @@ -798,10 +785,10 @@ static void wiimote_mp_unload(struct wiimote_data *wdata)
>
> ops = &wiimod_mp;
>
> - spin_lock_irqsave(&wdata->state.lock, flags);
> - wdata->state.mp = 0;
> - wdata->state.flags &= ~WIIPROTO_FLAG_MP_USED;
> - spin_unlock_irqrestore(&wdata->state.lock, flags);
> + scoped_guard(spinlock_irqsave, &wdata->state.lock) {
> + wdata->state.mp = 0;
> + wdata->state.flags &= ~WIIPROTO_FLAG_MP_USED;
> + }
>
> if (ops->remove)
> ops->remove(ops, wdata);
> @@ -885,19 +872,19 @@ static void wiimote_init_detect(struct
> wiimote_data *wdata)
>
> wiimote_cmd_acquire_noint(wdata);
>
> - spin_lock_irq(&wdata->state.lock);
> - wdata->state.devtype = WIIMOTE_DEV_UNKNOWN;
> - wiimote_cmd_set(wdata, WIIPROTO_REQ_SREQ, 0);
> - wiiproto_req_status(wdata);
> - spin_unlock_irq(&wdata->state.lock);
> + scoped_guard(spinlock_irq, &wdata->state.lock) {
> + wdata->state.devtype = WIIMOTE_DEV_UNKNOWN;
> + wiimote_cmd_set(wdata, WIIPROTO_REQ_SREQ, 0);
> + wiiproto_req_status(wdata);
> + }
> +
>
> ret = wiimote_cmd_wait_noint(wdata);
> if (ret)
> goto out_release;
>
> - spin_lock_irq(&wdata->state.lock);
> - ext = wdata->state.flags & WIIPROTO_FLAG_EXT_PLUGGED;
> - spin_unlock_irq(&wdata->state.lock);
> + scoped_guard(spinlock_irq, &wdata->state.lock)
> + ext = wdata->state.flags & WIIPROTO_FLAG_EXT_PLUGGED;
>
> if (!ext)
> goto out_release;
> @@ -910,11 +897,11 @@ static void wiimote_init_detect(struct
> wiimote_data *wdata)
> wiimote_init_set_type(wdata, exttype);
>
> /* schedule MP timer */
> - spin_lock_irq(&wdata->state.lock);
> - if (!(wdata->state.flags & WIIPROTO_FLAG_BUILTIN_MP) &&
> - !(wdata->state.flags & WIIPROTO_FLAG_NO_MP))
> - mod_timer(&wdata->timer, jiffies + HZ * 4);
> - spin_unlock_irq(&wdata->state.lock);
> + scoped_guard(spinlock_irq, &wdata->state.lock) {
> + if (!(wdata->state.flags & WIIPROTO_FLAG_BUILTIN_MP) &&
> + !(wdata->state.flags & WIIPROTO_FLAG_NO_MP))
> + mod_timer(&wdata->timer, jiffies + HZ * 4);
> + }
> }
>
> /*
> @@ -962,9 +949,8 @@ static bool wiimote_init_check(struct wiimote_data *wdata)
> __u8 type, data[6];
> bool ret, poll_mp;
>
> - spin_lock_irq(&wdata->state.lock);
> - flags = wdata->state.flags;
> - spin_unlock_irq(&wdata->state.lock);
> + scoped_guard(spinlock_irq, &wdata->state.lock)
> + flags = wdata->state.flags;
>
> wiimote_cmd_acquire_noint(wdata);
>
> @@ -980,11 +966,11 @@ static bool wiimote_init_check(struct wiimote_data *wdata)
> type = wiimote_cmd_read_mp_mapped(wdata);
> ret = type == WIIMOTE_MP_SINGLE;
>
> - spin_lock_irq(&wdata->state.lock);
> - ret = ret && !(wdata->state.flags & WIIPROTO_FLAG_EXT_ACTIVE);
> - ret = ret && !(wdata->state.flags & WIIPROTO_FLAG_MP_PLUGGED);
> - ret = ret && (wdata->state.flags & WIIPROTO_FLAG_MP_ACTIVE);
> - spin_unlock_irq(&wdata->state.lock);
> + scoped_guard(spinlock_irq, &wdata->state.lock) {
> + ret = ret && !(wdata->state.flags & WIIPROTO_FLAG_EXT_ACTIVE);
> + ret = ret && !(wdata->state.flags & WIIPROTO_FLAG_MP_PLUGGED);
> + ret = ret && (wdata->state.flags & WIIPROTO_FLAG_MP_ACTIVE);
> + }
>
> if (!ret)
> hid_dbg(wdata->hdev, "state left: !EXT && MP\n");
> @@ -1005,10 +991,10 @@ static bool wiimote_init_check(struct
> wiimote_data *wdata)
> type = wiimote_cmd_read_ext(wdata, data);
> ret = type == wdata->state.exttype;
>
> - spin_lock_irq(&wdata->state.lock);
> - ret = ret && !(wdata->state.flags & WIIPROTO_FLAG_MP_ACTIVE);
> - ret = ret && (wdata->state.flags & WIIPROTO_FLAG_EXT_ACTIVE);
> - spin_unlock_irq(&wdata->state.lock);
> + scoped_guard(spinlock_irq, &wdata->state.lock) {
> + ret = ret && !(wdata->state.flags & WIIPROTO_FLAG_MP_ACTIVE);
> + ret = ret && (wdata->state.flags & WIIPROTO_FLAG_EXT_ACTIVE);
> + }
>
> if (!ret)
> hid_dbg(wdata->hdev, "state left: EXT && !MP\n");
> @@ -1031,11 +1017,11 @@ static bool wiimote_init_check(struct
> wiimote_data *wdata)
> type = wiimote_cmd_read_ext(wdata, data);
> ret = type == wdata->state.exttype;
>
> - spin_lock_irq(&wdata->state.lock);
> - ret = ret && !(wdata->state.flags & WIIPROTO_FLAG_EXT_ACTIVE);
> - ret = ret && !(wdata->state.flags & WIIPROTO_FLAG_MP_ACTIVE);
> - ret = ret && !(wdata->state.flags & WIIPROTO_FLAG_EXT_PLUGGED);
> - spin_unlock_irq(&wdata->state.lock);
> + scoped_guard(spinlock_irq, &wdata->state.lock) {
> + ret = ret && !(wdata->state.flags & WIIPROTO_FLAG_EXT_ACTIVE);
> + ret = ret && !(wdata->state.flags & WIIPROTO_FLAG_MP_ACTIVE);
> + ret = ret && !(wdata->state.flags & WIIPROTO_FLAG_EXT_PLUGGED);
> + }
>
> if (!ret)
> hid_dbg(wdata->hdev, "state left: !EXT && !MP\n");
> @@ -1061,11 +1047,11 @@ static bool wiimote_init_check(struct
> wiimote_data *wdata)
> ret = ret && type != WIIMOTE_MP_UNKNOWN;
> ret = ret && type != WIIMOTE_MP_SINGLE;
>
> - spin_lock_irq(&wdata->state.lock);
> - ret = ret && (wdata->state.flags & WIIPROTO_FLAG_EXT_PLUGGED);
> - ret = ret && (wdata->state.flags & WIIPROTO_FLAG_EXT_ACTIVE);
> - ret = ret && (wdata->state.flags & WIIPROTO_FLAG_MP_ACTIVE);
> - spin_unlock_irq(&wdata->state.lock);
> + scoped_guard(spinlock_irq, &wdata->state.lock) {
> + ret = ret && (wdata->state.flags & WIIPROTO_FLAG_EXT_PLUGGED);
> + ret = ret && (wdata->state.flags & WIIPROTO_FLAG_EXT_ACTIVE);
> + ret = ret && (wdata->state.flags & WIIPROTO_FLAG_MP_ACTIVE);
> + }
>
> if (!ret)
> hid_dbg(wdata->hdev, "state left: EXT && MP\n");
> @@ -1120,16 +1106,15 @@ static void wiimote_init_hotplug(struct
> wiimote_data *wdata)
>
> wiimote_cmd_acquire_noint(wdata);
>
> - spin_lock_irq(&wdata->state.lock);
> + scoped_guard(spinlock_irq, &wdata->state.lock) {
>
> - /* get state snapshot that we will then work on */
> - flags = wdata->state.flags;
> + /* get state snapshot that we will then work on */
> + flags = wdata->state.flags;
>
> - /* disable event forwarding temporarily */
> - wdata->state.flags &= ~WIIPROTO_FLAG_EXT_ACTIVE;
> - wdata->state.flags &= ~WIIPROTO_FLAG_MP_ACTIVE;
> -
> - spin_unlock_irq(&wdata->state.lock);
> + /* disable event forwarding temporarily */
> + wdata->state.flags &= ~WIIPROTO_FLAG_EXT_ACTIVE;
> + wdata->state.flags &= ~WIIPROTO_FLAG_MP_ACTIVE;
> + }
>
> /* init extension and MP (deactivates current extension or MP) */
> wiimote_cmd_init_ext(wdata);
> @@ -1152,9 +1137,8 @@ static void wiimote_init_hotplug(struct
> wiimote_data *wdata)
> hid_info(wdata->hdev, "cannot detect extension; %6phC\n",
> extdata);
> } else if (exttype == WIIMOTE_EXT_NONE) {
> - spin_lock_irq(&wdata->state.lock);
> - wdata->state.exttype = WIIMOTE_EXT_NONE;
> - spin_unlock_irq(&wdata->state.lock);
> + scoped_guard(spinlock_irq, &wdata->state.lock)
> + wdata->state.exttype = WIIMOTE_EXT_NONE;
> } else {
> hid_info(wdata->hdev, "detected extension: %s\n",
> wiimote_exttype_names[exttype]);
> @@ -1192,28 +1176,26 @@ static void wiimote_init_hotplug(struct
> wiimote_data *wdata)
> mod_timer(&wdata->timer, jiffies + HZ * 4);
> }
>
> - spin_lock_irq(&wdata->state.lock);
> -
> - /* enable data forwarding again and set expected hotplug state */
> - if (mp) {
> - wdata->state.flags |= WIIPROTO_FLAG_MP_ACTIVE;
> - if (wdata->state.exttype == WIIMOTE_EXT_NONE) {
> - wdata->state.flags &= ~WIIPROTO_FLAG_EXT_PLUGGED;
> - wdata->state.flags &= ~WIIPROTO_FLAG_MP_PLUGGED;
> - } else {
> - wdata->state.flags &= ~WIIPROTO_FLAG_EXT_PLUGGED;
> - wdata->state.flags |= WIIPROTO_FLAG_MP_PLUGGED;
> + scoped_guard(spinlock_irq, &wdata->state.lock) {
> + /* enable data forwarding again and set expected hotplug state */
> + if (mp) {
> + wdata->state.flags |= WIIPROTO_FLAG_MP_ACTIVE;
> + if (wdata->state.exttype == WIIMOTE_EXT_NONE) {
> + wdata->state.flags &= ~WIIPROTO_FLAG_EXT_PLUGGED;
> + wdata->state.flags &= ~WIIPROTO_FLAG_MP_PLUGGED;
> + } else {
> + wdata->state.flags &= ~WIIPROTO_FLAG_EXT_PLUGGED;
> + wdata->state.flags |= WIIPROTO_FLAG_MP_PLUGGED;
> + wdata->state.flags |= WIIPROTO_FLAG_EXT_ACTIVE;
> + }
> + } else if (wdata->state.exttype != WIIMOTE_EXT_NONE) {
> wdata->state.flags |= WIIPROTO_FLAG_EXT_ACTIVE;
> }
> - } else if (wdata->state.exttype != WIIMOTE_EXT_NONE) {
> - wdata->state.flags |= WIIPROTO_FLAG_EXT_ACTIVE;
> +
> + /* request status report for hotplug state updates */
> + wiiproto_req_status(wdata);
> }
>
> - /* request status report for hotplug state updates */
> - wiiproto_req_status(wdata);
> -
> - spin_unlock_irq(&wdata->state.lock);
> -
> hid_dbg(wdata->hdev, "detected extensions: MP: %d EXT: %d\n",
> wdata->state.mp, wdata->state.exttype);
> }
> @@ -1244,11 +1226,8 @@ void __wiimote_schedule(struct wiimote_data *wdata)
>
> static void wiimote_schedule(struct wiimote_data *wdata)
> {
> - unsigned long flags;
> -
> - spin_lock_irqsave(&wdata->state.lock, flags);
> - __wiimote_schedule(wdata);
> - spin_unlock_irqrestore(&wdata->state.lock, flags);
> + scoped_guard(spinlock_irqsave, &wdata->state.lock)
> + __wiimote_schedule(wdata);
> }
>
> static void wiimote_init_timeout(struct timer_list *t)
> @@ -1638,7 +1617,6 @@ static int wiimote_hid_event(struct hid_device
> *hdev, struct hid_report *report,
> struct wiimote_data *wdata = hid_get_drvdata(hdev);
> const struct wiiproto_handler *h;
> int i;
> - unsigned long flags;
>
> if (size < 1)
> return -EINVAL;
> @@ -1646,9 +1624,8 @@ static int wiimote_hid_event(struct hid_device
> *hdev, struct hid_report *report,
> for (i = 0; handlers[i].id; ++i) {
> h = &handlers[i];
> if (h->id == raw_data[0] && h->size < size) {
> - spin_lock_irqsave(&wdata->state.lock, flags);
> - h->func(wdata, &raw_data[1]);
> - spin_unlock_irqrestore(&wdata->state.lock, flags);
> + scoped_guard(spinlock_irqsave, &wdata->state.lock)
> + h->func(wdata, &raw_data[1]);
> break;
> }
> }
> @@ -1666,11 +1643,9 @@ static ssize_t wiimote_ext_show(struct device
> *dev,
> {
> struct wiimote_data *wdata = dev_to_wii(dev);
> __u8 type;
> - unsigned long flags;
>
> - spin_lock_irqsave(&wdata->state.lock, flags);
> - type = wdata->state.exttype;
> - spin_unlock_irqrestore(&wdata->state.lock, flags);
> + scoped_guard(spinlock_irqsave, &wdata->state.lock)
> + type = wdata->state.exttype;
>
> switch (type) {
> case WIIMOTE_EXT_NONE:
> @@ -1719,11 +1694,9 @@ static ssize_t wiimote_dev_show(struct device *dev,
> {
> struct wiimote_data *wdata = dev_to_wii(dev);
> __u8 type;
> - unsigned long flags;
>
> - spin_lock_irqsave(&wdata->state.lock, flags);
> - type = wdata->state.devtype;
> - spin_unlock_irqrestore(&wdata->state.lock, flags);
> + scoped_guard(spinlock_irqsave, &wdata->state.lock)
> + type = wdata->state.devtype;
>
> switch (type) {
> case WIIMOTE_DEV_GENERIC:
> @@ -1778,16 +1751,13 @@ static DEFINE_IDA(wiimote_ida);
>
> static void wiimote_destroy(struct wiimote_data *wdata)
> {
> - unsigned long flags;
> -
> wiidebug_deinit(wdata);
>
> ida_free(&wiimote_ida, wdata->player_id);
>
> /* prevent init_worker from being scheduled again */
> - spin_lock_irqsave(&wdata->state.lock, flags);
> - wdata->state.flags |= WIIPROTO_FLAG_EXITING;
> - spin_unlock_irqrestore(&wdata->state.lock, flags);
> + scoped_guard(spinlock_irqsave, &wdata->state.lock)
> + wdata->state.flags |= WIIPROTO_FLAG_EXITING;
>
> cancel_work_sync(&wdata->init_worker);
> timer_shutdown_sync(&wdata->timer);
> diff --git a/drivers/hid/hid-wiimote-debug.c b/drivers/hid/hid-wiimote-debug.c
> index fc847c2a1c1f..b8027bb23608 100644
> --- a/drivers/hid/hid-wiimote-debug.c
> +++ b/drivers/hid/hid-wiimote-debug.c
> @@ -7,12 +7,13 @@
> /*
> */
>
> -#include <linux/debugfs.h>
> -#include <linux/module.h>
> -#include <linux/seq_file.h>
> -#include <linux/spinlock.h>
> -#include <linux/uaccess.h>
> -#include "hid-wiimote.h"
> + #include <linux/cleanup.h>
> + #include <linux/debugfs.h>
> + #include <linux/module.h>
> + #include <linux/seq_file.h>
> + #include <linux/spinlock.h>
> + #include <linux/uaccess.h>
> + #include "hid-wiimote.h"
>
> struct wiimote_debug {
> struct wiimote_data *wdata;
> @@ -25,7 +26,6 @@ static ssize_t wiidebug_eeprom_read(struct file *f,
> char __user *u, size_t s,
> {
> struct wiimote_debug *dbg = f->private_data;
> struct wiimote_data *wdata = dbg->wdata;
> - unsigned long flags;
> ssize_t ret;
> char buf[16];
> __u16 size = 0;
> @@ -41,20 +41,19 @@ static ssize_t wiidebug_eeprom_read(struct file *f,
> char __user *u, size_t s,
> if (ret)
> return ret;
>
> - spin_lock_irqsave(&wdata->state.lock, flags);
> - wdata->state.cmd_read_size = s;
> - wdata->state.cmd_read_buf = buf;
> - wiimote_cmd_set(wdata, WIIPROTO_REQ_RMEM, *off & 0xffff);
> - wiiproto_req_reeprom(wdata, *off, s);
> - spin_unlock_irqrestore(&wdata->state.lock, flags);
> + scoped_guard(spinlock_irqsave, &wdata->state.lock) {
> + wdata->state.cmd_read_size = s;
> + wdata->state.cmd_read_buf = buf;
> + wiimote_cmd_set(wdata, WIIPROTO_REQ_RMEM, *off & 0xffff);
> + wiiproto_req_reeprom(wdata, *off, s);
> + }
>
> ret = wiimote_cmd_wait(wdata);
> if (!ret)
> size = wdata->state.cmd_read_size;
>
> - spin_lock_irqsave(&wdata->state.lock, flags);
> - wdata->state.cmd_read_buf = NULL;
> - spin_unlock_irqrestore(&wdata->state.lock, flags);
> + scoped_guard(spinlock_irqsave, &wdata->state.lock)
> + wdata->state.cmd_read_buf = NULL;
>
> wiimote_cmd_release(wdata);
>
> @@ -99,12 +98,10 @@ static int wiidebug_drm_show(struct seq_file *f, void *p)
> {
> struct wiimote_debug *dbg = f->private;
> const char *str = NULL;
> - unsigned long flags;
> __u8 drm;
>
> - spin_lock_irqsave(&dbg->wdata->state.lock, flags);
> - drm = dbg->wdata->state.drm;
> - spin_unlock_irqrestore(&dbg->wdata->state.lock, flags);
> + scoped_guard(spinlock_irqsave, &dbg->wdata->state.lock)
> + drm = dbg->wdata->state.drm;
>
> if (drm < WIIPROTO_REQ_MAX)
> str = wiidebug_drmmap[drm];
> @@ -126,7 +123,6 @@ static ssize_t wiidebug_drm_write(struct file *f,
> const char __user *u,
> {
> struct seq_file *sf = f->private_data;
> struct wiimote_debug *dbg = sf->private;
> - unsigned long flags;
> char buf[16];
> ssize_t len;
> int i;
> @@ -150,12 +146,12 @@ static ssize_t wiidebug_drm_write(struct file *f,
> const char __user *u,
> if (i == WIIPROTO_REQ_MAX)
> i = simple_strtoul(buf, NULL, 16);
>
> - spin_lock_irqsave(&dbg->wdata->state.lock, flags);
> - dbg->wdata->state.flags &= ~WIIPROTO_FLAG_DRM_LOCKED;
> - wiiproto_req_drm(dbg->wdata, (__u8) i);
> - if (i != WIIPROTO_REQ_NULL)
> - dbg->wdata->state.flags |= WIIPROTO_FLAG_DRM_LOCKED;
> - spin_unlock_irqrestore(&dbg->wdata->state.lock, flags);
> + scoped_guard(spinlock_irqsave, &dbg->wdata->state.lock) {
> + dbg->wdata->state.flags &= ~WIIPROTO_FLAG_DRM_LOCKED;
> + wiiproto_req_drm(dbg->wdata, (__u8) i);
> + if (i != WIIPROTO_REQ_NULL)
> + dbg->wdata->state.flags |= WIIPROTO_FLAG_DRM_LOCKED;
> + }
>
> return len;
> }
> @@ -172,7 +168,6 @@ static const struct file_operations wiidebug_drm_fops = {
> int wiidebug_init(struct wiimote_data *wdata)
> {
> struct wiimote_debug *dbg;
> - unsigned long flags;
>
> dbg = kzalloc_obj(*dbg);
> if (!dbg)
> @@ -189,9 +184,8 @@ int wiidebug_init(struct wiimote_data *wdata)
> debugfs_create_u8("player_id", S_IRUSR,
> dbg->wdata->hdev->debug_dir, &wdata->player_id);
>
> - spin_lock_irqsave(&wdata->state.lock, flags);
> - wdata->debug = dbg;
> - spin_unlock_irqrestore(&wdata->state.lock, flags);
> + scoped_guard(spinlock_irqsave, &wdata->state.lock)
> + wdata->debug = dbg;
>
> return 0;
> }
> @@ -199,14 +193,12 @@ int wiidebug_init(struct wiimote_data *wdata)
> void wiidebug_deinit(struct wiimote_data *wdata)
> {
> struct wiimote_debug *dbg = wdata->debug;
> - unsigned long flags;
>
> if (!dbg)
> return;
>
> - spin_lock_irqsave(&wdata->state.lock, flags);
> - wdata->debug = NULL;
> - spin_unlock_irqrestore(&wdata->state.lock, flags);
> + scoped_guard(spinlock_irqsave, &wdata->state.lock)
> + wdata->debug = NULL;
>
> debugfs_remove(dbg->drm);
> debugfs_remove(dbg->eeprom);
> diff --git a/drivers/hid/hid-wiimote-modules.c
> b/drivers/hid/hid-wiimote-modules.c
> index dccb78bb3afd..3cd614466740 100644
> --- a/drivers/hid/hid-wiimote-modules.c
> +++ b/drivers/hid/hid-wiimote-modules.c
> @@ -30,6 +30,7 @@
> * input devices.
> */
>
> +#include <linux/cleanup.h>
> #include <linux/device.h>
> #include <linux/hid.h>
> #include <linux/input.h>
> @@ -341,7 +342,6 @@ static int wiimod_led_probe(const struct wiimod_ops *ops,
> struct device *dev = &wdata->hdev->dev;
> size_t namesz = strlen(dev_name(dev)) + 9;
> struct led_classdev *led;
> - unsigned long flags;
> char *name;
> int ret;
>
> @@ -364,9 +364,8 @@ static int wiimod_led_probe(const struct wiimod_ops *ops,
>
> /* enable LED1 to stop initial LED-blinking */
> if (ops->arg == 0) {
> - spin_lock_irqsave(&wdata->state.lock, flags);
> - wiiproto_req_leds(wdata, WIIPROTO_FLAG_LED1);
> - spin_unlock_irqrestore(&wdata->state.lock, flags);
> + scoped_guard(spinlock_irqsave, &wdata->state.lock)
> + wiiproto_req_leds(wdata, WIIPROTO_FLAG_LED1);
> }
>
> return 0;
> --
> 2.55.0
next prev parent reply other threads:[~2026-09-20 8:12 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-17 21:38 [PATCH v4 0/4] HID: wiimote: new LED behavior on connect + scoped_guards Rafael Passos
2026-08-17 21:38 ` [PATCH v4 1/4] HID: wiimote: turn on the LEDs indicating the controller id Rafael Passos
2026-08-17 21:55 ` sashiko-bot
2026-09-20 8:24 ` David Rheinsberg
2026-08-17 21:38 ` [PATCH v4 2/4] HID: wiimote: replace spinlock pairs with scoped_guard Rafael Passos
2026-08-17 21:59 ` sashiko-bot
2026-09-20 8:12 ` David Rheinsberg [this message]
2026-08-17 21:38 ` [PATCH v4 3/4] HID: wiimote: led_probe with scoped cleanup Rafael Passos
2026-09-20 8:17 ` David Rheinsberg
2026-08-17 21:38 ` [PATCH v4 4/4] HID: wiimote: wiimote_probe " Rafael Passos
2026-08-17 21:54 ` sashiko-bot
2026-09-20 8:21 ` David Rheinsberg
2026-10-04 15:03 ` Rafael Passos
2026-09-03 13:30 ` [PATCH v4 0/4] HID: wiimote: new LED behavior on connect + scoped_guards Rafael Passos
2026-09-11 16:44 ` Jiri Kosina
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=95661df0-11cf-47dd-976b-3ccc895c0db5@app.fastmail.com \
--to=david@readahead.eu \
--cc=bentiss@kernel.org \
--cc=jikos@kernel.org \
--cc=jkoolstra@xs4all.nl \
--cc=linux-input@vger.kernel.org \
--cc=me@brighamcampbell.com \
--cc=rafael@rcpassos.me \
--cc=skhan@linuxfoundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox