* [PATCH 0/6] Convert joystick drivers to use new cleanup facilities
@ 2024-09-04 4:30 Dmitry Torokhov
2024-09-04 4:30 ` [PATCH 1/6] Input: db9 - use guard notation when acquiring mutex Dmitry Torokhov
` (6 more replies)
0 siblings, 7 replies; 10+ messages in thread
From: Dmitry Torokhov @ 2024-09-04 4:30 UTC (permalink / raw)
To: linux-input; +Cc: Erick Archer, Christophe JAILLET, linux-kernel
Hi,
This series converts drivers found in drivers/input/keyboard to use new
__free() and guard() cleanup facilities that simplify the code and
ensure that all resources are released appropriately.
Thanks!
Dmitry Torokhov (6):
Input: db9 - use guard notation when acquiring mutex
Input: gamecon - use guard notation when acquiring mutex
Input: iforce - use guard notation when acquiring mutex and spinlock
Input: n64joy - use guard notation when acquiring mutex
Input: turbografx - use guard notation when acquiring mutex
Input: xpad - use guard notation when acquiring mutex and spinlock
drivers/input/joystick/db9.c | 30 +++---
drivers/input/joystick/gamecon.c | 22 ++---
drivers/input/joystick/iforce/iforce-ff.c | 48 +++++----
.../input/joystick/iforce/iforce-packets.c | 57 +++++------
drivers/input/joystick/iforce/iforce-serio.c | 36 +++----
drivers/input/joystick/iforce/iforce-usb.c | 13 ++-
drivers/input/joystick/n64joy.c | 35 +++----
drivers/input/joystick/turbografx.c | 22 ++---
drivers/input/joystick/xpad.c | 99 +++++++------------
9 files changed, 152 insertions(+), 210 deletions(-)
--
Dmitry
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/6] Input: db9 - use guard notation when acquiring mutex
2024-09-04 4:30 [PATCH 0/6] Convert joystick drivers to use new cleanup facilities Dmitry Torokhov
@ 2024-09-04 4:30 ` Dmitry Torokhov
2024-11-20 18:33 ` David Lechner
2024-09-04 4:30 ` [PATCH 2/6] Input: gamecon " Dmitry Torokhov
` (5 subsequent siblings)
6 siblings, 1 reply; 10+ messages in thread
From: Dmitry Torokhov @ 2024-09-04 4:30 UTC (permalink / raw)
To: linux-input; +Cc: Erick Archer, Christophe JAILLET, linux-kernel
Using guard notation makes the code more compact and error handling
more robust by ensuring that mutexes are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/joystick/db9.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/input/joystick/db9.c b/drivers/input/joystick/db9.c
index 682a29c27832..7ac0cfc3e786 100644
--- a/drivers/input/joystick/db9.c
+++ b/drivers/input/joystick/db9.c
@@ -505,24 +505,22 @@ static int db9_open(struct input_dev *dev)
{
struct db9 *db9 = input_get_drvdata(dev);
struct parport *port = db9->pd->port;
- int err;
- err = mutex_lock_interruptible(&db9->mutex);
- if (err)
- return err;
-
- if (!db9->used++) {
- parport_claim(db9->pd);
- parport_write_data(port, 0xff);
- if (db9_modes[db9->mode].reverse) {
- parport_data_reverse(port);
- parport_write_control(port, DB9_NORMAL);
+ scoped_guard(mutex_intr, &db9->mutex) {
+ if (!db9->used++) {
+ parport_claim(db9->pd);
+ parport_write_data(port, 0xff);
+ if (db9_modes[db9->mode].reverse) {
+ parport_data_reverse(port);
+ parport_write_control(port, DB9_NORMAL);
+ }
+ mod_timer(&db9->timer, jiffies + DB9_REFRESH_TIME);
}
- mod_timer(&db9->timer, jiffies + DB9_REFRESH_TIME);
+
+ return 0;
}
- mutex_unlock(&db9->mutex);
- return 0;
+ return -EINTR;
}
static void db9_close(struct input_dev *dev)
@@ -530,14 +528,14 @@ static void db9_close(struct input_dev *dev)
struct db9 *db9 = input_get_drvdata(dev);
struct parport *port = db9->pd->port;
- mutex_lock(&db9->mutex);
+ guard(mutex)(&db9->mutex);
+
if (!--db9->used) {
del_timer_sync(&db9->timer);
parport_write_control(port, 0x00);
parport_data_forward(port);
parport_release(db9->pd);
}
- mutex_unlock(&db9->mutex);
}
static void db9_attach(struct parport *pp)
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/6] Input: gamecon - use guard notation when acquiring mutex
2024-09-04 4:30 [PATCH 0/6] Convert joystick drivers to use new cleanup facilities Dmitry Torokhov
2024-09-04 4:30 ` [PATCH 1/6] Input: db9 - use guard notation when acquiring mutex Dmitry Torokhov
@ 2024-09-04 4:30 ` Dmitry Torokhov
2024-09-04 4:31 ` [PATCH 3/6] Input: iforce - use guard notation when acquiring mutex and spinlock Dmitry Torokhov
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Dmitry Torokhov @ 2024-09-04 4:30 UTC (permalink / raw)
To: linux-input; +Cc: Erick Archer, Christophe JAILLET, linux-kernel
Using guard notation makes the code more compact and error handling
more robust by ensuring that mutexes are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/joystick/gamecon.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/input/joystick/gamecon.c b/drivers/input/joystick/gamecon.c
index c38de3094553..968c0e653f2e 100644
--- a/drivers/input/joystick/gamecon.c
+++ b/drivers/input/joystick/gamecon.c
@@ -765,33 +765,31 @@ static void gc_timer(struct timer_list *t)
static int gc_open(struct input_dev *dev)
{
struct gc *gc = input_get_drvdata(dev);
- int err;
- err = mutex_lock_interruptible(&gc->mutex);
- if (err)
- return err;
+ scoped_guard(mutex_intr, &gc->mutex) {
+ if (!gc->used++) {
+ parport_claim(gc->pd);
+ parport_write_control(gc->pd->port, 0x04);
+ mod_timer(&gc->timer, jiffies + GC_REFRESH_TIME);
+ }
- if (!gc->used++) {
- parport_claim(gc->pd);
- parport_write_control(gc->pd->port, 0x04);
- mod_timer(&gc->timer, jiffies + GC_REFRESH_TIME);
+ return 0;
}
- mutex_unlock(&gc->mutex);
- return 0;
+ return -EINTR;
}
static void gc_close(struct input_dev *dev)
{
struct gc *gc = input_get_drvdata(dev);
- mutex_lock(&gc->mutex);
+ guard(mutex)(&gc->mutex);
+
if (!--gc->used) {
del_timer_sync(&gc->timer);
parport_write_control(gc->pd->port, 0x00);
parport_release(gc->pd);
}
- mutex_unlock(&gc->mutex);
}
static int gc_setup_pad(struct gc *gc, int idx, int pad_type)
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/6] Input: iforce - use guard notation when acquiring mutex and spinlock
2024-09-04 4:30 [PATCH 0/6] Convert joystick drivers to use new cleanup facilities Dmitry Torokhov
2024-09-04 4:30 ` [PATCH 1/6] Input: db9 - use guard notation when acquiring mutex Dmitry Torokhov
2024-09-04 4:30 ` [PATCH 2/6] Input: gamecon " Dmitry Torokhov
@ 2024-09-04 4:31 ` Dmitry Torokhov
2024-09-04 4:31 ` [PATCH 4/6] Input: n64joy - use guard notation when acquiring mutex Dmitry Torokhov
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Dmitry Torokhov @ 2024-09-04 4:31 UTC (permalink / raw)
To: linux-input; +Cc: Erick Archer, Christophe JAILLET, linux-kernel
Using guard notation makes the code more compact and error handling
more robust by ensuring that locks are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/joystick/iforce/iforce-ff.c | 48 +++++++---------
.../input/joystick/iforce/iforce-packets.c | 57 ++++++++-----------
drivers/input/joystick/iforce/iforce-serio.c | 36 +++++-------
drivers/input/joystick/iforce/iforce-usb.c | 13 ++---
4 files changed, 68 insertions(+), 86 deletions(-)
diff --git a/drivers/input/joystick/iforce/iforce-ff.c b/drivers/input/joystick/iforce/iforce-ff.c
index 95c0348843e6..8c78cbe553c8 100644
--- a/drivers/input/joystick/iforce/iforce-ff.c
+++ b/drivers/input/joystick/iforce/iforce-ff.c
@@ -21,14 +21,13 @@ static int make_magnitude_modifier(struct iforce* iforce,
unsigned char data[3];
if (!no_alloc) {
- mutex_lock(&iforce->mem_mutex);
- if (allocate_resource(&(iforce->device_memory), mod_chunk, 2,
- iforce->device_memory.start, iforce->device_memory.end, 2L,
- NULL, NULL)) {
- mutex_unlock(&iforce->mem_mutex);
+ guard(mutex)(&iforce->mem_mutex);
+
+ if (allocate_resource(&iforce->device_memory, mod_chunk, 2,
+ iforce->device_memory.start,
+ iforce->device_memory.end,
+ 2L, NULL, NULL))
return -ENOSPC;
- }
- mutex_unlock(&iforce->mem_mutex);
}
data[0] = LO(mod_chunk->start);
@@ -54,14 +53,13 @@ static int make_period_modifier(struct iforce* iforce,
period = TIME_SCALE(period);
if (!no_alloc) {
- mutex_lock(&iforce->mem_mutex);
- if (allocate_resource(&(iforce->device_memory), mod_chunk, 0x0c,
- iforce->device_memory.start, iforce->device_memory.end, 2L,
- NULL, NULL)) {
- mutex_unlock(&iforce->mem_mutex);
+ guard(mutex)(&iforce->mem_mutex);
+
+ if (allocate_resource(&iforce->device_memory, mod_chunk, 0x0c,
+ iforce->device_memory.start,
+ iforce->device_memory.end,
+ 2L, NULL, NULL))
return -ENOSPC;
- }
- mutex_unlock(&iforce->mem_mutex);
}
data[0] = LO(mod_chunk->start);
@@ -94,14 +92,13 @@ static int make_envelope_modifier(struct iforce* iforce,
fade_duration = TIME_SCALE(fade_duration);
if (!no_alloc) {
- mutex_lock(&iforce->mem_mutex);
+ guard(mutex)(&iforce->mem_mutex);
+
if (allocate_resource(&(iforce->device_memory), mod_chunk, 0x0e,
- iforce->device_memory.start, iforce->device_memory.end, 2L,
- NULL, NULL)) {
- mutex_unlock(&iforce->mem_mutex);
+ iforce->device_memory.start,
+ iforce->device_memory.end,
+ 2L, NULL, NULL))
return -ENOSPC;
- }
- mutex_unlock(&iforce->mem_mutex);
}
data[0] = LO(mod_chunk->start);
@@ -131,14 +128,13 @@ static int make_condition_modifier(struct iforce* iforce,
unsigned char data[10];
if (!no_alloc) {
- mutex_lock(&iforce->mem_mutex);
+ guard(mutex)(&iforce->mem_mutex);
+
if (allocate_resource(&(iforce->device_memory), mod_chunk, 8,
- iforce->device_memory.start, iforce->device_memory.end, 2L,
- NULL, NULL)) {
- mutex_unlock(&iforce->mem_mutex);
+ iforce->device_memory.start,
+ iforce->device_memory.end,
+ 2L, NULL, NULL))
return -ENOSPC;
- }
- mutex_unlock(&iforce->mem_mutex);
}
data[0] = LO(mod_chunk->start);
diff --git a/drivers/input/joystick/iforce/iforce-packets.c b/drivers/input/joystick/iforce/iforce-packets.c
index 763642c8cee9..8c2531e2977c 100644
--- a/drivers/input/joystick/iforce/iforce-packets.c
+++ b/drivers/input/joystick/iforce/iforce-packets.c
@@ -31,49 +31,42 @@ int iforce_send_packet(struct iforce *iforce, u16 cmd, unsigned char* data)
int c;
int empty;
int head, tail;
- unsigned long flags;
/*
* Update head and tail of xmit buffer
*/
- spin_lock_irqsave(&iforce->xmit_lock, flags);
-
- head = iforce->xmit.head;
- tail = iforce->xmit.tail;
-
-
- if (CIRC_SPACE(head, tail, XMIT_SIZE) < n+2) {
- dev_warn(&iforce->dev->dev,
- "not enough space in xmit buffer to send new packet\n");
- spin_unlock_irqrestore(&iforce->xmit_lock, flags);
- return -1;
- }
+ scoped_guard(spinlock_irqsave, &iforce->xmit_lock) {
+ head = iforce->xmit.head;
+ tail = iforce->xmit.tail;
+
+ if (CIRC_SPACE(head, tail, XMIT_SIZE) < n + 2) {
+ dev_warn(&iforce->dev->dev,
+ "not enough space in xmit buffer to send new packet\n");
+ return -1;
+ }
- empty = head == tail;
- XMIT_INC(iforce->xmit.head, n+2);
+ empty = head == tail;
+ XMIT_INC(iforce->xmit.head, n + 2);
/*
* Store packet in xmit buffer
*/
- iforce->xmit.buf[head] = HI(cmd);
- XMIT_INC(head, 1);
- iforce->xmit.buf[head] = LO(cmd);
- XMIT_INC(head, 1);
-
- c = CIRC_SPACE_TO_END(head, tail, XMIT_SIZE);
- if (n < c) c=n;
-
- memcpy(&iforce->xmit.buf[head],
- data,
- c);
- if (n != c) {
- memcpy(&iforce->xmit.buf[0],
- data + c,
- n - c);
+ iforce->xmit.buf[head] = HI(cmd);
+ XMIT_INC(head, 1);
+ iforce->xmit.buf[head] = LO(cmd);
+ XMIT_INC(head, 1);
+
+ c = CIRC_SPACE_TO_END(head, tail, XMIT_SIZE);
+ if (n < c)
+ c = n;
+
+ memcpy(&iforce->xmit.buf[head], data, c);
+ if (n != c)
+ memcpy(&iforce->xmit.buf[0], data + c, n - c);
+
+ XMIT_INC(head, n);
}
- XMIT_INC(head, n);
- spin_unlock_irqrestore(&iforce->xmit_lock, flags);
/*
* If necessary, start the transmission
*/
diff --git a/drivers/input/joystick/iforce/iforce-serio.c b/drivers/input/joystick/iforce/iforce-serio.c
index 2380546d7978..75b85c46dfa4 100644
--- a/drivers/input/joystick/iforce/iforce-serio.c
+++ b/drivers/input/joystick/iforce/iforce-serio.c
@@ -28,45 +28,39 @@ static void iforce_serio_xmit(struct iforce *iforce)
iforce);
unsigned char cs;
int i;
- unsigned long flags;
if (test_and_set_bit(IFORCE_XMIT_RUNNING, iforce->xmit_flags)) {
set_bit(IFORCE_XMIT_AGAIN, iforce->xmit_flags);
return;
}
- spin_lock_irqsave(&iforce->xmit_lock, flags);
+ guard(spinlock_irqsave)(&iforce->xmit_lock);
-again:
- if (iforce->xmit.head == iforce->xmit.tail) {
- iforce_clear_xmit_and_wake(iforce);
- spin_unlock_irqrestore(&iforce->xmit_lock, flags);
- return;
- }
+ do {
+ if (iforce->xmit.head == iforce->xmit.tail)
+ break;
- cs = 0x2b;
+ cs = 0x2b;
- serio_write(iforce_serio->serio, 0x2b);
+ serio_write(iforce_serio->serio, 0x2b);
- serio_write(iforce_serio->serio, iforce->xmit.buf[iforce->xmit.tail]);
- cs ^= iforce->xmit.buf[iforce->xmit.tail];
- XMIT_INC(iforce->xmit.tail, 1);
-
- for (i=iforce->xmit.buf[iforce->xmit.tail]; i >= 0; --i) {
serio_write(iforce_serio->serio,
iforce->xmit.buf[iforce->xmit.tail]);
cs ^= iforce->xmit.buf[iforce->xmit.tail];
XMIT_INC(iforce->xmit.tail, 1);
- }
- serio_write(iforce_serio->serio, cs);
+ for (i = iforce->xmit.buf[iforce->xmit.tail]; i >= 0; --i) {
+ serio_write(iforce_serio->serio,
+ iforce->xmit.buf[iforce->xmit.tail]);
+ cs ^= iforce->xmit.buf[iforce->xmit.tail];
+ XMIT_INC(iforce->xmit.tail, 1);
+ }
- if (test_and_clear_bit(IFORCE_XMIT_AGAIN, iforce->xmit_flags))
- goto again;
+ serio_write(iforce_serio->serio, cs);
- iforce_clear_xmit_and_wake(iforce);
+ } while (test_and_clear_bit(IFORCE_XMIT_AGAIN, iforce->xmit_flags));
- spin_unlock_irqrestore(&iforce->xmit_lock, flags);
+ iforce_clear_xmit_and_wake(iforce);
}
static int iforce_serio_get_id(struct iforce *iforce, u8 id,
diff --git a/drivers/input/joystick/iforce/iforce-usb.c b/drivers/input/joystick/iforce/iforce-usb.c
index cba92bd590a8..1f00f76b0174 100644
--- a/drivers/input/joystick/iforce/iforce-usb.c
+++ b/drivers/input/joystick/iforce/iforce-usb.c
@@ -25,13 +25,11 @@ static void __iforce_usb_xmit(struct iforce *iforce)
struct iforce_usb *iforce_usb = container_of(iforce, struct iforce_usb,
iforce);
int n, c;
- unsigned long flags;
- spin_lock_irqsave(&iforce->xmit_lock, flags);
+ guard(spinlock_irqsave)(&iforce->xmit_lock);
if (iforce->xmit.head == iforce->xmit.tail) {
iforce_clear_xmit_and_wake(iforce);
- spin_unlock_irqrestore(&iforce->xmit_lock, flags);
return;
}
@@ -45,7 +43,8 @@ static void __iforce_usb_xmit(struct iforce *iforce)
/* Copy rest of data then */
c = CIRC_CNT_TO_END(iforce->xmit.head, iforce->xmit.tail, XMIT_SIZE);
- if (n < c) c=n;
+ if (n < c)
+ c = n;
memcpy(iforce_usb->out->transfer_buffer + 1,
&iforce->xmit.buf[iforce->xmit.tail],
@@ -53,11 +52,12 @@ static void __iforce_usb_xmit(struct iforce *iforce)
if (n != c) {
memcpy(iforce_usb->out->transfer_buffer + 1 + c,
&iforce->xmit.buf[0],
- n-c);
+ n - c);
}
XMIT_INC(iforce->xmit.tail, n);
- if ( (n=usb_submit_urb(iforce_usb->out, GFP_ATOMIC)) ) {
+ n=usb_submit_urb(iforce_usb->out, GFP_ATOMIC);
+ if (n) {
dev_warn(&iforce_usb->intf->dev,
"usb_submit_urb failed %d\n", n);
iforce_clear_xmit_and_wake(iforce);
@@ -66,7 +66,6 @@ static void __iforce_usb_xmit(struct iforce *iforce)
/* The IFORCE_XMIT_RUNNING bit is not cleared here. That's intended.
* As long as the urb completion handler is not called, the transmiting
* is considered to be running */
- spin_unlock_irqrestore(&iforce->xmit_lock, flags);
}
static void iforce_usb_xmit(struct iforce *iforce)
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/6] Input: n64joy - use guard notation when acquiring mutex
2024-09-04 4:30 [PATCH 0/6] Convert joystick drivers to use new cleanup facilities Dmitry Torokhov
` (2 preceding siblings ...)
2024-09-04 4:31 ` [PATCH 3/6] Input: iforce - use guard notation when acquiring mutex and spinlock Dmitry Torokhov
@ 2024-09-04 4:31 ` Dmitry Torokhov
2024-09-04 4:31 ` [PATCH 5/6] Input: turbografx " Dmitry Torokhov
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Dmitry Torokhov @ 2024-09-04 4:31 UTC (permalink / raw)
To: linux-input; +Cc: Erick Archer, Christophe JAILLET, linux-kernel
Using guard notation makes the code more compact and error handling
more robust by ensuring that mutexes are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/joystick/n64joy.c | 35 +++++++++++++++------------------
1 file changed, 16 insertions(+), 19 deletions(-)
diff --git a/drivers/input/joystick/n64joy.c b/drivers/input/joystick/n64joy.c
index b0986d2195d6..c344dbc0c493 100644
--- a/drivers/input/joystick/n64joy.c
+++ b/drivers/input/joystick/n64joy.c
@@ -191,35 +191,32 @@ static void n64joy_poll(struct timer_list *t)
static int n64joy_open(struct input_dev *dev)
{
struct n64joy_priv *priv = input_get_drvdata(dev);
- int err;
-
- err = mutex_lock_interruptible(&priv->n64joy_mutex);
- if (err)
- return err;
-
- if (!priv->n64joy_opened) {
- /*
- * We could use the vblank irq, but it's not important if
- * the poll point slightly changes.
- */
- timer_setup(&priv->timer, n64joy_poll, 0);
- mod_timer(&priv->timer, jiffies + msecs_to_jiffies(16));
- }
- priv->n64joy_opened++;
+ scoped_guard(mutex_intr, &priv->n64joy_mutex) {
+ if (!priv->n64joy_opened) {
+ /*
+ * We could use the vblank irq, but it's not important
+ * if the poll point slightly changes.
+ */
+ timer_setup(&priv->timer, n64joy_poll, 0);
+ mod_timer(&priv->timer, jiffies + msecs_to_jiffies(16));
+ }
- mutex_unlock(&priv->n64joy_mutex);
- return err;
+ priv->n64joy_opened++;
+ return 0;
+ }
+
+ return -EINTR;
}
static void n64joy_close(struct input_dev *dev)
{
struct n64joy_priv *priv = input_get_drvdata(dev);
- mutex_lock(&priv->n64joy_mutex);
+ guard(mutex)(&priv->n64joy_mutex);
+
if (!--priv->n64joy_opened)
del_timer_sync(&priv->timer);
- mutex_unlock(&priv->n64joy_mutex);
}
static const u64 __initconst scandata[] ____cacheline_aligned = {
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 5/6] Input: turbografx - use guard notation when acquiring mutex
2024-09-04 4:30 [PATCH 0/6] Convert joystick drivers to use new cleanup facilities Dmitry Torokhov
` (3 preceding siblings ...)
2024-09-04 4:31 ` [PATCH 4/6] Input: n64joy - use guard notation when acquiring mutex Dmitry Torokhov
@ 2024-09-04 4:31 ` Dmitry Torokhov
2024-09-04 4:31 ` [PATCH 6/6] Input: xpad - use guard notation when acquiring mutex and spinlock Dmitry Torokhov
2024-09-04 4:43 ` [PATCH 0/6] Convert joystick drivers to use new cleanup facilities Dmitry Torokhov
6 siblings, 0 replies; 10+ messages in thread
From: Dmitry Torokhov @ 2024-09-04 4:31 UTC (permalink / raw)
To: linux-input; +Cc: Erick Archer, Christophe JAILLET, linux-kernel
Using guard notation makes the code more compact and error handling
more robust by ensuring that mutexes are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/joystick/turbografx.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/input/joystick/turbografx.c b/drivers/input/joystick/turbografx.c
index eb8455c34e67..015010a928aa 100644
--- a/drivers/input/joystick/turbografx.c
+++ b/drivers/input/joystick/turbografx.c
@@ -103,33 +103,31 @@ static void tgfx_timer(struct timer_list *t)
static int tgfx_open(struct input_dev *dev)
{
struct tgfx *tgfx = input_get_drvdata(dev);
- int err;
- err = mutex_lock_interruptible(&tgfx->sem);
- if (err)
- return err;
+ scoped_guard(mutex_intr, &tgfx->sem) {
+ if (!tgfx->used++) {
+ parport_claim(tgfx->pd);
+ parport_write_control(tgfx->pd->port, 0x04);
+ mod_timer(&tgfx->timer, jiffies + TGFX_REFRESH_TIME);
+ }
- if (!tgfx->used++) {
- parport_claim(tgfx->pd);
- parport_write_control(tgfx->pd->port, 0x04);
- mod_timer(&tgfx->timer, jiffies + TGFX_REFRESH_TIME);
+ return 0;
}
- mutex_unlock(&tgfx->sem);
- return 0;
+ return -EINTR;
}
static void tgfx_close(struct input_dev *dev)
{
struct tgfx *tgfx = input_get_drvdata(dev);
- mutex_lock(&tgfx->sem);
+ guard(mutex)(&tgfx->sem);
+
if (!--tgfx->used) {
del_timer_sync(&tgfx->timer);
parport_write_control(tgfx->pd->port, 0x00);
parport_release(tgfx->pd);
}
- mutex_unlock(&tgfx->sem);
}
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 6/6] Input: xpad - use guard notation when acquiring mutex and spinlock
2024-09-04 4:30 [PATCH 0/6] Convert joystick drivers to use new cleanup facilities Dmitry Torokhov
` (4 preceding siblings ...)
2024-09-04 4:31 ` [PATCH 5/6] Input: turbografx " Dmitry Torokhov
@ 2024-09-04 4:31 ` Dmitry Torokhov
2024-09-04 4:43 ` [PATCH 0/6] Convert joystick drivers to use new cleanup facilities Dmitry Torokhov
6 siblings, 0 replies; 10+ messages in thread
From: Dmitry Torokhov @ 2024-09-04 4:31 UTC (permalink / raw)
To: linux-input; +Cc: Erick Archer, Christophe JAILLET, linux-kernel
Using guard notation makes the code more compact and error handling
more robust by ensuring that locks are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/joystick/xpad.c | 99 ++++++++++++-----------------------
1 file changed, 34 insertions(+), 65 deletions(-)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 4eda18f4f46e..3e61df927277 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -1289,9 +1289,8 @@ static void xpad_irq_out(struct urb *urb)
struct device *dev = &xpad->intf->dev;
int status = urb->status;
int error;
- unsigned long flags;
- spin_lock_irqsave(&xpad->odata_lock, flags);
+ guard(spinlock_irqsave)(&xpad->odata_lock);
switch (status) {
case 0:
@@ -1325,8 +1324,6 @@ static void xpad_irq_out(struct urb *urb)
xpad->irq_out_active = false;
}
}
-
- spin_unlock_irqrestore(&xpad->odata_lock, flags);
}
static int xpad_init_output(struct usb_interface *intf, struct usb_xpad *xpad,
@@ -1391,10 +1388,8 @@ static int xpad_inquiry_pad_presence(struct usb_xpad *xpad)
{
struct xpad_output_packet *packet =
&xpad->out_packets[XPAD_OUT_CMD_IDX];
- unsigned long flags;
- int retval;
- spin_lock_irqsave(&xpad->odata_lock, flags);
+ guard(spinlock_irqsave)(&xpad->odata_lock);
packet->data[0] = 0x08;
packet->data[1] = 0x00;
@@ -1413,17 +1408,12 @@ static int xpad_inquiry_pad_presence(struct usb_xpad *xpad)
/* Reset the sequence so we send out presence first */
xpad->last_out_packet = -1;
- retval = xpad_try_sending_next_out_packet(xpad);
-
- spin_unlock_irqrestore(&xpad->odata_lock, flags);
-
- return retval;
+ return xpad_try_sending_next_out_packet(xpad);
}
static int xpad_start_xbox_one(struct usb_xpad *xpad)
{
- unsigned long flags;
- int retval;
+ int error;
if (usb_ifnum_to_if(xpad->udev, GIP_WIRED_INTF_AUDIO)) {
/*
@@ -1432,15 +1422,15 @@ static int xpad_start_xbox_one(struct usb_xpad *xpad)
* Controller for Series X|S (0x20d6:0x200e) to report the
* guide button.
*/
- retval = usb_set_interface(xpad->udev,
- GIP_WIRED_INTF_AUDIO, 0);
- if (retval)
+ error = usb_set_interface(xpad->udev,
+ GIP_WIRED_INTF_AUDIO, 0);
+ if (error)
dev_warn(&xpad->dev->dev,
"unable to disable audio interface: %d\n",
- retval);
+ error);
}
- spin_lock_irqsave(&xpad->odata_lock, flags);
+ guard(spinlock_irqsave)(&xpad->odata_lock);
/*
* Begin the init sequence by attempting to send a packet.
@@ -1448,16 +1438,11 @@ static int xpad_start_xbox_one(struct usb_xpad *xpad)
* sending any packets from the output ring.
*/
xpad->init_seq = 0;
- retval = xpad_try_sending_next_out_packet(xpad);
-
- spin_unlock_irqrestore(&xpad->odata_lock, flags);
-
- return retval;
+ return xpad_try_sending_next_out_packet(xpad);
}
static void xpadone_ack_mode_report(struct usb_xpad *xpad, u8 seq_num)
{
- unsigned long flags;
struct xpad_output_packet *packet =
&xpad->out_packets[XPAD_OUT_CMD_IDX];
static const u8 mode_report_ack[] = {
@@ -1465,7 +1450,7 @@ static void xpadone_ack_mode_report(struct usb_xpad *xpad, u8 seq_num)
0x00, GIP_CMD_VIRTUAL_KEY, GIP_OPT_INTERNAL, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00
};
- spin_lock_irqsave(&xpad->odata_lock, flags);
+ guard(spinlock_irqsave)(&xpad->odata_lock);
packet->len = sizeof(mode_report_ack);
memcpy(packet->data, mode_report_ack, packet->len);
@@ -1475,8 +1460,6 @@ static void xpadone_ack_mode_report(struct usb_xpad *xpad, u8 seq_num)
/* Reset the sequence so we send out the ack now */
xpad->last_out_packet = -1;
xpad_try_sending_next_out_packet(xpad);
-
- spin_unlock_irqrestore(&xpad->odata_lock, flags);
}
#ifdef CONFIG_JOYSTICK_XPAD_FF
@@ -1486,8 +1469,6 @@ static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect
struct xpad_output_packet *packet = &xpad->out_packets[XPAD_OUT_FF_IDX];
__u16 strong;
__u16 weak;
- int retval;
- unsigned long flags;
if (effect->type != FF_RUMBLE)
return 0;
@@ -1495,7 +1476,7 @@ static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect
strong = effect->u.rumble.strong_magnitude;
weak = effect->u.rumble.weak_magnitude;
- spin_lock_irqsave(&xpad->odata_lock, flags);
+ guard(spinlock_irqsave)(&xpad->odata_lock);
switch (xpad->xtype) {
case XTYPE_XBOX:
@@ -1561,15 +1542,10 @@ static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect
dev_dbg(&xpad->dev->dev,
"%s - rumble command sent to unsupported xpad type: %d\n",
__func__, xpad->xtype);
- retval = -EINVAL;
- goto out;
+ return -EINVAL;
}
- retval = xpad_try_sending_next_out_packet(xpad);
-
-out:
- spin_unlock_irqrestore(&xpad->odata_lock, flags);
- return retval;
+ return xpad_try_sending_next_out_packet(xpad);
}
static int xpad_init_ff(struct usb_xpad *xpad)
@@ -1622,11 +1598,10 @@ static void xpad_send_led_command(struct usb_xpad *xpad, int command)
{
struct xpad_output_packet *packet =
&xpad->out_packets[XPAD_OUT_LED_IDX];
- unsigned long flags;
command %= 16;
- spin_lock_irqsave(&xpad->odata_lock, flags);
+ guard(spinlock_irqsave)(&xpad->odata_lock);
switch (xpad->xtype) {
case XTYPE_XBOX360:
@@ -1656,8 +1631,6 @@ static void xpad_send_led_command(struct usb_xpad *xpad, int command)
}
xpad_try_sending_next_out_packet(xpad);
-
- spin_unlock_irqrestore(&xpad->odata_lock, flags);
}
/*
@@ -1782,11 +1755,10 @@ static void xpad_stop_input(struct usb_xpad *xpad)
static void xpad360w_poweroff_controller(struct usb_xpad *xpad)
{
- unsigned long flags;
struct xpad_output_packet *packet =
&xpad->out_packets[XPAD_OUT_CMD_IDX];
- spin_lock_irqsave(&xpad->odata_lock, flags);
+ guard(spinlock_irqsave)(&xpad->odata_lock);
packet->data[0] = 0x00;
packet->data[1] = 0x00;
@@ -1806,8 +1778,6 @@ static void xpad360w_poweroff_controller(struct usb_xpad *xpad)
/* Reset the sequence so we send out poweroff now */
xpad->last_out_packet = -1;
xpad_try_sending_next_out_packet(xpad);
-
- spin_unlock_irqrestore(&xpad->odata_lock, flags);
}
static int xpad360w_start_input(struct usb_xpad *xpad)
@@ -2231,10 +2201,10 @@ static int xpad_suspend(struct usb_interface *intf, pm_message_t message)
if (auto_poweroff && xpad->pad_present)
xpad360w_poweroff_controller(xpad);
} else {
- mutex_lock(&input->mutex);
+ guard(mutex)(&input->mutex);
+
if (input_device_enabled(input))
xpad_stop_input(xpad);
- mutex_unlock(&input->mutex);
}
xpad_stop_output(xpad);
@@ -2246,26 +2216,25 @@ static int xpad_resume(struct usb_interface *intf)
{
struct usb_xpad *xpad = usb_get_intfdata(intf);
struct input_dev *input = xpad->dev;
- int retval = 0;
- if (xpad->xtype == XTYPE_XBOX360W) {
- retval = xpad360w_start_input(xpad);
- } else {
- mutex_lock(&input->mutex);
- if (input_device_enabled(input)) {
- retval = xpad_start_input(xpad);
- } else if (xpad->xtype == XTYPE_XBOXONE) {
- /*
- * Even if there are no users, we'll send Xbox One pads
- * the startup sequence so they don't sit there and
- * blink until somebody opens the input device again.
- */
- retval = xpad_start_xbox_one(xpad);
- }
- mutex_unlock(&input->mutex);
+ if (xpad->xtype == XTYPE_XBOX360W)
+ return xpad360w_start_input(xpad);
+
+ guard(mutex)(&input->mutex);
+
+ if (input_device_enabled(input))
+ return xpad_start_input(xpad);
+
+ if (xpad->xtype == XTYPE_XBOXONE) {
+ /*
+ * Even if there are no users, we'll send Xbox One pads
+ * the startup sequence so they don't sit there and
+ * blink until somebody opens the input device again.
+ */
+ return xpad_start_xbox_one(xpad);
}
- return retval;
+ return 0;
}
static struct usb_driver xpad_driver = {
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 0/6] Convert joystick drivers to use new cleanup facilities
2024-09-04 4:30 [PATCH 0/6] Convert joystick drivers to use new cleanup facilities Dmitry Torokhov
` (5 preceding siblings ...)
2024-09-04 4:31 ` [PATCH 6/6] Input: xpad - use guard notation when acquiring mutex and spinlock Dmitry Torokhov
@ 2024-09-04 4:43 ` Dmitry Torokhov
6 siblings, 0 replies; 10+ messages in thread
From: Dmitry Torokhov @ 2024-09-04 4:43 UTC (permalink / raw)
To: linux-input; +Cc: Erick Archer, Christophe JAILLET, linux-kernel
On Tue, Sep 03, 2024 at 09:30:57PM -0700, Dmitry Torokhov wrote:
> Hi,
>
> This series converts drivers found in drivers/input/keyboard to use new
This should have read drivers/input/joystick obviously...
> __free() and guard() cleanup facilities that simplify the code and
> ensure that all resources are released appropriately.
>
> Thanks!
>
> Dmitry Torokhov (6):
> Input: db9 - use guard notation when acquiring mutex
> Input: gamecon - use guard notation when acquiring mutex
> Input: iforce - use guard notation when acquiring mutex and spinlock
> Input: n64joy - use guard notation when acquiring mutex
> Input: turbografx - use guard notation when acquiring mutex
> Input: xpad - use guard notation when acquiring mutex and spinlock
>
> drivers/input/joystick/db9.c | 30 +++---
> drivers/input/joystick/gamecon.c | 22 ++---
> drivers/input/joystick/iforce/iforce-ff.c | 48 +++++----
> .../input/joystick/iforce/iforce-packets.c | 57 +++++------
> drivers/input/joystick/iforce/iforce-serio.c | 36 +++----
> drivers/input/joystick/iforce/iforce-usb.c | 13 ++-
> drivers/input/joystick/n64joy.c | 35 +++----
> drivers/input/joystick/turbografx.c | 22 ++---
> drivers/input/joystick/xpad.c | 99 +++++++------------
> 9 files changed, 152 insertions(+), 210 deletions(-)
>
> --
> Dmitry
--
Dmitry
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/6] Input: db9 - use guard notation when acquiring mutex
2024-09-04 4:30 ` [PATCH 1/6] Input: db9 - use guard notation when acquiring mutex Dmitry Torokhov
@ 2024-11-20 18:33 ` David Lechner
2024-11-21 3:52 ` Dmitry Torokhov
0 siblings, 1 reply; 10+ messages in thread
From: David Lechner @ 2024-11-20 18:33 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: Erick Archer, Christophe JAILLET, linux-kernel
On 9/3/24 11:30 PM, Dmitry Torokhov wrote:
> Using guard notation makes the code more compact and error handling
> more robust by ensuring that mutexes are released in all code paths
> when control leaves critical section.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/joystick/db9.c | 30 ++++++++++++++----------------
> 1 file changed, 14 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/input/joystick/db9.c b/drivers/input/joystick/db9.c
> index 682a29c27832..7ac0cfc3e786 100644
> --- a/drivers/input/joystick/db9.c
> +++ b/drivers/input/joystick/db9.c
> @@ -505,24 +505,22 @@ static int db9_open(struct input_dev *dev)
> {
> struct db9 *db9 = input_get_drvdata(dev);
> struct parport *port = db9->pd->port;
> - int err;
>
> - err = mutex_lock_interruptible(&db9->mutex);
> - if (err)
> - return err;
> -
> - if (!db9->used++) {
> - parport_claim(db9->pd);
> - parport_write_data(port, 0xff);
> - if (db9_modes[db9->mode].reverse) {
> - parport_data_reverse(port);
> - parport_write_control(port, DB9_NORMAL);
> + scoped_guard(mutex_intr, &db9->mutex) {
> + if (!db9->used++) {
> + parport_claim(db9->pd);
> + parport_write_data(port, 0xff);
> + if (db9_modes[db9->mode].reverse) {
> + parport_data_reverse(port);
> + parport_write_control(port, DB9_NORMAL);
> + }
> + mod_timer(&db9->timer, jiffies + DB9_REFRESH_TIME);
> }
> - mod_timer(&db9->timer, jiffies + DB9_REFRESH_TIME);
> +
> + return 0;
> }
>
> - mutex_unlock(&db9->mutex);
> - return 0;
> + return -EINTR;
This patch and any others like it are potentially introducing a bug.
From inspecting the source code, it looks like
mutex_lock_interruptible() can return -EINTR, -EALREADY, or -EDEADLK.
Before this patch, the return value of mutex_lock_interruptible() was
passed to the caller. Now, the return value is reduced to pass/fail
and only -EINTR is returned on failure when the reason could have
been something else.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/6] Input: db9 - use guard notation when acquiring mutex
2024-11-20 18:33 ` David Lechner
@ 2024-11-21 3:52 ` Dmitry Torokhov
0 siblings, 0 replies; 10+ messages in thread
From: Dmitry Torokhov @ 2024-11-21 3:52 UTC (permalink / raw)
To: David Lechner; +Cc: linux-input, Erick Archer, Christophe JAILLET, linux-kernel
On Wed, Nov 20, 2024 at 12:33:36PM -0600, David Lechner wrote:
> On 9/3/24 11:30 PM, Dmitry Torokhov wrote:
> > Using guard notation makes the code more compact and error handling
> > more robust by ensuring that mutexes are released in all code paths
> > when control leaves critical section.
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> > drivers/input/joystick/db9.c | 30 ++++++++++++++----------------
> > 1 file changed, 14 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/input/joystick/db9.c b/drivers/input/joystick/db9.c
> > index 682a29c27832..7ac0cfc3e786 100644
> > --- a/drivers/input/joystick/db9.c
> > +++ b/drivers/input/joystick/db9.c
> > @@ -505,24 +505,22 @@ static int db9_open(struct input_dev *dev)
> > {
> > struct db9 *db9 = input_get_drvdata(dev);
> > struct parport *port = db9->pd->port;
> > - int err;
> >
> > - err = mutex_lock_interruptible(&db9->mutex);
> > - if (err)
> > - return err;
> > -
> > - if (!db9->used++) {
> > - parport_claim(db9->pd);
> > - parport_write_data(port, 0xff);
> > - if (db9_modes[db9->mode].reverse) {
> > - parport_data_reverse(port);
> > - parport_write_control(port, DB9_NORMAL);
> > + scoped_guard(mutex_intr, &db9->mutex) {
> > + if (!db9->used++) {
> > + parport_claim(db9->pd);
> > + parport_write_data(port, 0xff);
> > + if (db9_modes[db9->mode].reverse) {
> > + parport_data_reverse(port);
> > + parport_write_control(port, DB9_NORMAL);
> > + }
> > + mod_timer(&db9->timer, jiffies + DB9_REFRESH_TIME);
> > }
> > - mod_timer(&db9->timer, jiffies + DB9_REFRESH_TIME);
> > +
> > + return 0;
> > }
> >
> > - mutex_unlock(&db9->mutex);
> > - return 0;
> > + return -EINTR;
>
> This patch and any others like it are potentially introducing a bug.
>
> From inspecting the source code, it looks like
> mutex_lock_interruptible() can return -EINTR, -EALREADY, or -EDEADLK.
>
> Before this patch, the return value of mutex_lock_interruptible() was
> passed to the caller. Now, the return value is reduced to pass/fail
> and only -EINTR is returned on failure when the reason could have
> been something else.
It is documented that mutex_lock_interruptible() only returns 0 or
-EINTR. These additional errors only returned from __mutex_lock_common()
for WW mutexes.
If there is another form of scoped_cond_guard() that would make
available error code returned by the constructor of the locking
primitive we can switch to it later.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-11-21 3:52 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-04 4:30 [PATCH 0/6] Convert joystick drivers to use new cleanup facilities Dmitry Torokhov
2024-09-04 4:30 ` [PATCH 1/6] Input: db9 - use guard notation when acquiring mutex Dmitry Torokhov
2024-11-20 18:33 ` David Lechner
2024-11-21 3:52 ` Dmitry Torokhov
2024-09-04 4:30 ` [PATCH 2/6] Input: gamecon " Dmitry Torokhov
2024-09-04 4:31 ` [PATCH 3/6] Input: iforce - use guard notation when acquiring mutex and spinlock Dmitry Torokhov
2024-09-04 4:31 ` [PATCH 4/6] Input: n64joy - use guard notation when acquiring mutex Dmitry Torokhov
2024-09-04 4:31 ` [PATCH 5/6] Input: turbografx " Dmitry Torokhov
2024-09-04 4:31 ` [PATCH 6/6] Input: xpad - use guard notation when acquiring mutex and spinlock Dmitry Torokhov
2024-09-04 4:43 ` [PATCH 0/6] Convert joystick drivers to use new cleanup facilities Dmitry Torokhov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).