diff mbox series

[3/6] Input: iforce - use guard notation when acquiring mutex and spinlock

Message ID 20240904043104.1030257-4-dmitry.torokhov@gmail.com (mailing list archive)
State Mainlined
Commit 63ade96711c7a0bb7226f3b029e800fafccd4b0b
Headers show
Series Convert joystick drivers to use new cleanup facilities | expand

Commit Message

Dmitry Torokhov Sept. 4, 2024, 4:31 a.m. UTC
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 mbox series

Patch

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)