* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.