{PATCH 0/2] HID: wiimote: Minor change to spinlock usage

{PATCH 0/2] HID: wiimote: Minor change to spinlock usage

[Ian Abbott]

[I have posted these patches previously, but that was over a year ago.
--IA]

A couple of minor changes to the wiimote core module.  They do not
change functionality of the driver:

1) HID: wiimote: make handlers[] const
2) HID: wiimote: narrow spinlock range in wiimote_hid_event()

 drivers/hid/hid-wiimote-core.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

[PATCH 1/2] HID: wiimote: make handlers[] const

[Ian Abbott]

The `handlers[]` array contents are never modified, so use the `const`
qualifier.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/hid/hid-wiimote-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-wiimote-core.c b/drivers/hid/hid-wiimote-core.c
index e484c3618dec..e03a0ba5611a 100644
--- a/drivers/hid/hid-wiimote-core.c
+++ b/drivers/hid/hid-wiimote-core.c
@@ -1586,7 +1586,7 @@ struct wiiproto_handler {
 	void (*func)(struct wiimote_data *wdata, const __u8 *payload);
 };
 
-static struct wiiproto_handler handlers[] = {
+static const struct wiiproto_handler handlers[] = {
 	{ .id = WIIPROTO_REQ_STATUS, .size = 6, .func = handler_status },
 	{ .id = WIIPROTO_REQ_STATUS, .size = 2, .func = handler_status_K },
 	{ .id = WIIPROTO_REQ_DATA, .size = 21, .func = handler_data },
@@ -1618,7 +1618,7 @@ static int wiimote_hid_event(struct hid_device *hdev, struct hid_report *report,
 							u8 *raw_data, int size)
 {
 	struct wiimote_data *wdata = hid_get_drvdata(hdev);
-	struct wiiproto_handler *h;
+	const struct wiiproto_handler *h;
 	int i;
 	unsigned long flags;
 
-- 
2.28.0

[PATCH 2/2] HID: wiimote: narrow spinlock range in wiimote_hid_event()

[Ian Abbott]

In `wiimote_hid_event()`, the `wdata->state.lock` spinlock does not need
to be held while searching `handlers[]` for a suitable handler function.
Change it so the spinlock is only held during the call to the handler
function itself.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/hid/hid-wiimote-core.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-wiimote-core.c b/drivers/hid/hid-wiimote-core.c
index e03a0ba5611a..41012681cafd 100644
--- a/drivers/hid/hid-wiimote-core.c
+++ b/drivers/hid/hid-wiimote-core.c
@@ -1625,12 +1625,12 @@ static int wiimote_hid_event(struct hid_device *hdev, struct hid_report *report,
 	if (size < 1)
 		return -EINVAL;
 
-	spin_lock_irqsave(&wdata->state.lock, flags);
-
 	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);
 			break;
 		}
 	}
@@ -1639,8 +1639,6 @@ static int wiimote_hid_event(struct hid_device *hdev, struct hid_report *report,
 		hid_warn(hdev, "Unhandled report %hhu size %d\n", raw_data[0],
 									size);
 
-	spin_unlock_irqrestore(&wdata->state.lock, flags);
-
 	return 0;
 }
 
-- 
2.28.0

Re: {PATCH 0/2] HID: wiimote: Minor change to spinlock usage

[David Rheinsberg]

Hi

On Fri, 4 Sep 2020 at 15:21, Ian Abbott <abbotti@mev.co.uk> wrote:
>
> [I have posted these patches previously, but that was over a year ago.
> --IA]
>
> A couple of minor changes to the wiimote core module.  They do not
> change functionality of the driver:
>
> 1) HID: wiimote: make handlers[] const
> 2) HID: wiimote: narrow spinlock range in wiimote_hid_event()
>
>  drivers/hid/hid-wiimote-core.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)

Looks still good ;)

Reviewed-by: David Rheinsberg <david.rheinsberg@gmail.com>

Thanks!
David

Re: {PATCH 0/2] HID: wiimote: Minor change to spinlock usage

[Jiri Kosina]

On Fri, 4 Sep 2020, Ian Abbott wrote:

> [I have posted these patches previously, but that was over a year ago.
> --IA]
> 
> A couple of minor changes to the wiimote core module.  They do not
> change functionality of the driver:
> 
> 1) HID: wiimote: make handlers[] const
> 2) HID: wiimote: narrow spinlock range in wiimote_hid_event()
> 
>  drivers/hid/hid-wiimote-core.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)

Applied to for-5.10/wiimote. Thanks,

-- 
Jiri Kosina
SUSE Labs