linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: linux-input@vger.kernel.org
Cc: "Pali Rohár" <pali@kernel.org>, "Helge Deller" <deller@gmx.de>,
	"K. Y. Srinivasan" <kys@microsoft.com>,
	"Wei Liu" <wei.liu@kernel.org>,
	"Dexuan Cui" <decui@microsoft.com>,
	"Samuel Holland" <samuel@sholland.org>,
	"Lyude Paul" <thatslyude@gmail.com>,
	"Michal Simek" <michal.simek@amd.com>,
	"Hans de Goede" <hdegoede@redhat.com>,
	linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-sunxi@lists.linux.dev
Subject: [PATCH 13/24] Input: i8042 - use guard notation when acquiring spinlock
Date: Wed,  4 Sep 2024 21:17:18 -0700	[thread overview]
Message-ID: <20240905041732.2034348-14-dmitry.torokhov@gmail.com> (raw)
In-Reply-To: <20240905041732.2034348-1-dmitry.torokhov@gmail.com>

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/serio/i8042.c | 204 +++++++++++++++---------------------
 1 file changed, 82 insertions(+), 122 deletions(-)

diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index 674cd155ec8f..86916fe3925f 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -197,42 +197,26 @@ EXPORT_SYMBOL(i8042_unlock_chip);
 int i8042_install_filter(bool (*filter)(unsigned char data, unsigned char str,
 					struct serio *serio))
 {
-	unsigned long flags;
-	int ret = 0;
+	guard(spinlock_irqsave)(&i8042_lock);
 
-	spin_lock_irqsave(&i8042_lock, flags);
-
-	if (i8042_platform_filter) {
-		ret = -EBUSY;
-		goto out;
-	}
+	if (i8042_platform_filter)
+		return -EBUSY;
 
 	i8042_platform_filter = filter;
-
-out:
-	spin_unlock_irqrestore(&i8042_lock, flags);
-	return ret;
+	return 0;
 }
 EXPORT_SYMBOL(i8042_install_filter);
 
 int i8042_remove_filter(bool (*filter)(unsigned char data, unsigned char str,
 				       struct serio *port))
 {
-	unsigned long flags;
-	int ret = 0;
-
-	spin_lock_irqsave(&i8042_lock, flags);
+	guard(spinlock_irqsave)(&i8042_lock);
 
-	if (i8042_platform_filter != filter) {
-		ret = -EINVAL;
-		goto out;
-	}
+	if (i8042_platform_filter != filter)
+		return -EINVAL;
 
 	i8042_platform_filter = NULL;
-
-out:
-	spin_unlock_irqrestore(&i8042_lock, flags);
-	return ret;
+	return 0;
 }
 EXPORT_SYMBOL(i8042_remove_filter);
 
@@ -271,28 +255,22 @@ static int i8042_wait_write(void)
 
 static int i8042_flush(void)
 {
-	unsigned long flags;
 	unsigned char data, str;
 	int count = 0;
-	int retval = 0;
 
-	spin_lock_irqsave(&i8042_lock, flags);
+	guard(spinlock_irqsave)(&i8042_lock);
 
 	while ((str = i8042_read_status()) & I8042_STR_OBF) {
-		if (count++ < I8042_BUFFER_SIZE) {
-			udelay(50);
-			data = i8042_read_data();
-			dbg("%02x <- i8042 (flush, %s)\n",
-			    data, str & I8042_STR_AUXDATA ? "aux" : "kbd");
-		} else {
-			retval = -EIO;
-			break;
-		}
-	}
+		if (count++ >= I8042_BUFFER_SIZE)
+			return -EIO;
 
-	spin_unlock_irqrestore(&i8042_lock, flags);
+		udelay(50);
+		data = i8042_read_data();
+		dbg("%02x <- i8042 (flush, %s)\n",
+		    data, str & I8042_STR_AUXDATA ? "aux" : "kbd");
+	}
 
-	return retval;
+	return 0;
 }
 
 /*
@@ -349,17 +327,12 @@ static int __i8042_command(unsigned char *param, int command)
 
 int i8042_command(unsigned char *param, int command)
 {
-	unsigned long flags;
-	int retval;
-
 	if (!i8042_present)
 		return -1;
 
-	spin_lock_irqsave(&i8042_lock, flags);
-	retval = __i8042_command(param, command);
-	spin_unlock_irqrestore(&i8042_lock, flags);
+	guard(spinlock_irqsave)(&i8042_lock);
 
-	return retval;
+	return __i8042_command(param, command);
 }
 EXPORT_SYMBOL(i8042_command);
 
@@ -369,19 +342,18 @@ EXPORT_SYMBOL(i8042_command);
 
 static int i8042_kbd_write(struct serio *port, unsigned char c)
 {
-	unsigned long flags;
-	int retval = 0;
+	int error;
 
-	spin_lock_irqsave(&i8042_lock, flags);
+	guard(spinlock_irqsave)(&i8042_lock);
 
-	if (!(retval = i8042_wait_write())) {
-		dbg("%02x -> i8042 (kbd-data)\n", c);
-		i8042_write_data(c);
-	}
+	error = i8042_wait_write();
+	if (error)
+		return error;
 
-	spin_unlock_irqrestore(&i8042_lock, flags);
+	dbg("%02x -> i8042 (kbd-data)\n", c);
+	i8042_write_data(c);
 
-	return retval;
+	return 0;
 }
 
 /*
@@ -460,9 +432,8 @@ static int i8042_start(struct serio *serio)
 		device_set_wakeup_enable(&serio->dev, true);
 	}
 
-	spin_lock_irq(&i8042_lock);
+	guard(spinlock_irq)(&i8042_lock);
 	port->exists = true;
-	spin_unlock_irq(&i8042_lock);
 
 	return 0;
 }
@@ -476,10 +447,10 @@ static void i8042_stop(struct serio *serio)
 {
 	struct i8042_port *port = serio->port_data;
 
-	spin_lock_irq(&i8042_lock);
-	port->exists = false;
-	port->serio = NULL;
-	spin_unlock_irq(&i8042_lock);
+	scoped_guard(spinlock_irq, &i8042_lock) {
+		port->exists = false;
+		port->serio = NULL;
+	}
 
 	/*
 	 * We need to make sure that interrupt handler finishes using
@@ -583,45 +554,41 @@ static bool i8042_handle_data(int irq)
 {
 	struct i8042_port *port;
 	struct serio *serio;
-	unsigned long flags;
 	unsigned char str, data;
 	unsigned int dfl;
 	unsigned int port_no;
 	bool filtered;
 
-	spin_lock_irqsave(&i8042_lock, flags);
-
-	str = i8042_read_status();
-	if (unlikely(~str & I8042_STR_OBF)) {
-		spin_unlock_irqrestore(&i8042_lock, flags);
-		return false;
-	}
-
-	data = i8042_read_data();
+	scoped_guard(spinlock_irqsave, &i8042_lock) {
+		str = i8042_read_status();
+		if (unlikely(~str & I8042_STR_OBF))
+			return false;
 
-	if (i8042_mux_present && (str & I8042_STR_AUXDATA)) {
-		port_no = i8042_handle_mux(str, &data, &dfl);
-	} else {
+		data = i8042_read_data();
 
-		dfl = (str & I8042_STR_PARITY) ? SERIO_PARITY : 0;
-		if ((str & I8042_STR_TIMEOUT) && !i8042_notimeout)
-			dfl |= SERIO_TIMEOUT;
+		if (i8042_mux_present && (str & I8042_STR_AUXDATA)) {
+			port_no = i8042_handle_mux(str, &data, &dfl);
+		} else {
 
-		port_no = (str & I8042_STR_AUXDATA) ?
-				I8042_AUX_PORT_NO : I8042_KBD_PORT_NO;
-	}
+			dfl = (str & I8042_STR_PARITY) ? SERIO_PARITY : 0;
+			if ((str & I8042_STR_TIMEOUT) && !i8042_notimeout)
+				dfl |= SERIO_TIMEOUT;
 
-	port = &i8042_ports[port_no];
-	serio = port->exists ? port->serio : NULL;
+			port_no = (str & I8042_STR_AUXDATA) ?
+					I8042_AUX_PORT_NO : I8042_KBD_PORT_NO;
+		}
 
-	filter_dbg(port->driver_bound, data, "<- i8042 (interrupt, %d, %d%s%s)\n",
-		   port_no, irq,
-		   dfl & SERIO_PARITY ? ", bad parity" : "",
-		   dfl & SERIO_TIMEOUT ? ", timeout" : "");
+		port = &i8042_ports[port_no];
+		serio = port->exists ? port->serio : NULL;
 
-	filtered = i8042_filter(data, str, serio);
+		filter_dbg(port->driver_bound,
+			   data, "<- i8042 (interrupt, %d, %d%s%s)\n",
+			   port_no, irq,
+			   dfl & SERIO_PARITY ? ", bad parity" : "",
+			   dfl & SERIO_TIMEOUT ? ", timeout" : "");
 
-	spin_unlock_irqrestore(&i8042_lock, flags);
+		filtered = i8042_filter(data, str, serio);
+	}
 
 	if (likely(serio && !filtered))
 		serio_interrupt(serio, data, dfl);
@@ -779,24 +746,22 @@ static bool i8042_irq_being_tested;
 
 static irqreturn_t i8042_aux_test_irq(int irq, void *dev_id)
 {
-	unsigned long flags;
 	unsigned char str, data;
-	int ret = 0;
 
-	spin_lock_irqsave(&i8042_lock, flags);
+	guard(spinlock_irqsave)(&i8042_lock);
+
 	str = i8042_read_status();
-	if (str & I8042_STR_OBF) {
-		data = i8042_read_data();
-		dbg("%02x <- i8042 (aux_test_irq, %s)\n",
-		    data, str & I8042_STR_AUXDATA ? "aux" : "kbd");
-		if (i8042_irq_being_tested &&
-		    data == 0xa5 && (str & I8042_STR_AUXDATA))
-			complete(&i8042_aux_irq_delivered);
-		ret = 1;
-	}
-	spin_unlock_irqrestore(&i8042_lock, flags);
+	if (!(str & I8042_STR_OBF))
+		return IRQ_NONE;
+
+	data = i8042_read_data();
+	dbg("%02x <- i8042 (aux_test_irq, %s)\n",
+	    data, str & I8042_STR_AUXDATA ? "aux" : "kbd");
+
+	if (i8042_irq_being_tested && data == 0xa5 && (str & I8042_STR_AUXDATA))
+		complete(&i8042_aux_irq_delivered);
 
-	return IRQ_RETVAL(ret);
+	return IRQ_HANDLED;
 }
 
 /*
@@ -837,7 +802,6 @@ static int i8042_check_aux(void)
 	int retval = -1;
 	bool irq_registered = false;
 	bool aux_loop_broken = false;
-	unsigned long flags;
 	unsigned char param;
 
 /*
@@ -921,18 +885,15 @@ static int i8042_check_aux(void)
 	if (i8042_enable_aux_port())
 		goto out;
 
-	spin_lock_irqsave(&i8042_lock, flags);
-
-	init_completion(&i8042_aux_irq_delivered);
-	i8042_irq_being_tested = true;
-
-	param = 0xa5;
-	retval = __i8042_command(&param, I8042_CMD_AUX_LOOP & 0xf0ff);
-
-	spin_unlock_irqrestore(&i8042_lock, flags);
+	scoped_guard(spinlock_irqsave, &i8042_lock) {
+		init_completion(&i8042_aux_irq_delivered);
+		i8042_irq_being_tested = true;
 
-	if (retval)
-		goto out;
+		param = 0xa5;
+		retval = __i8042_command(&param, I8042_CMD_AUX_LOOP & 0xf0ff);
+		if (retval)
+			goto out;
+	}
 
 	if (wait_for_completion_timeout(&i8042_aux_irq_delivered,
 					msecs_to_jiffies(250)) == 0) {
@@ -1020,7 +981,6 @@ static int i8042_controller_selftest(void)
 
 static int i8042_controller_init(void)
 {
-	unsigned long flags;
 	int n = 0;
 	unsigned char ctr[2];
 
@@ -1057,14 +1017,14 @@ static int i8042_controller_init(void)
  * Handle keylock.
  */
 
-	spin_lock_irqsave(&i8042_lock, flags);
-	if (~i8042_read_status() & I8042_STR_KEYLOCK) {
-		if (i8042_unlock)
-			i8042_ctr |= I8042_CTR_IGNKEYLOCK;
-		else
-			pr_warn("Warning: Keylock active\n");
+	scoped_guard(spinlock_irqsave, &i8042_lock) {
+		if (~i8042_read_status() & I8042_STR_KEYLOCK) {
+			if (i8042_unlock)
+				i8042_ctr |= I8042_CTR_IGNKEYLOCK;
+			else
+				pr_warn("Warning: Keylock active\n");
+		}
 	}
-	spin_unlock_irqrestore(&i8042_lock, flags);
 
 /*
  * If the chip is configured into nontranslated mode by the BIOS, don't
-- 
2.46.0.469.g59c65b2a67-goog


  parent reply	other threads:[~2024-09-05  4:18 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-05  4:17 [PATCH 00/24] Convert serio-related drivers to use new cleanup facilities Dmitry Torokhov
2024-09-05  4:17 ` [PATCH 01/24] Input: serio - define serio_pause_rx guard to pause and resume serio ports Dmitry Torokhov
2024-09-05  4:17 ` [PATCH 02/24] Input: libps2 - use guard notation when temporarily pausing " Dmitry Torokhov
2024-09-05  4:17 ` [PATCH 03/24] Input: alps - use guard notation when pausing serio port Dmitry Torokhov
2024-09-05 15:46   ` Pali Rohár
2024-09-05  4:17 ` [PATCH 04/24] Input: byd " Dmitry Torokhov
2024-09-05  4:17 ` [PATCH 05/24] Input: synaptics " Dmitry Torokhov
2024-09-05  4:17 ` [PATCH 06/24] Input: atkbd " Dmitry Torokhov
2024-09-05  4:17 ` [PATCH 07/24] Input: sunkbd " Dmitry Torokhov
2024-09-05  4:17 ` [PATCH 08/24] Input: synaptics-rmi4 - use guard notation when pausing serio port in F03 Dmitry Torokhov
2024-09-05  4:17 ` [PATCH 09/24] Input: elo - use guard notation when pausing serio port Dmitry Torokhov
2024-09-05  4:17 ` [PATCH 10/24] Input: gscps2 - use guard notation when acquiring spinlock Dmitry Torokhov
2024-09-05  4:17 ` [PATCH 11/24] Input: hyperv-keyboard " Dmitry Torokhov
2024-09-05  4:17 ` [PATCH 12/24] Input: i8042 - tease apart interrupt handler Dmitry Torokhov
2024-09-05  4:17 ` Dmitry Torokhov [this message]
2024-09-05  4:17 ` [PATCH 14/24] Input: ps2-gpio - use guard notation when acquiring mutex Dmitry Torokhov
2024-09-05  4:17 ` [PATCH 15/24] Input: ps2mult - use guard notation when acquiring spinlock Dmitry Torokhov
2024-09-05  4:17 ` [PATCH 16/24] Input: q40kbd " Dmitry Torokhov
2024-09-05  4:17 ` [PATCH 17/24] Input: sa1111ps2 " Dmitry Torokhov
2024-09-05  4:17 ` [PATCH 18/24] Input: serport " Dmitry Torokhov
2024-09-05  4:17 ` [PATCH 19/24] Input: serio - use guard notation when acquiring mutexes and spinlocks Dmitry Torokhov
2024-09-05  4:17 ` [PATCH 20/24] Input: serio_raw - use guard notation for locks and other resources Dmitry Torokhov
2024-10-21 20:41   ` Kees Bakker
2024-09-05  4:17 ` [PATCH 21/24] Input: serio-raw - fix potential serio port name truncation Dmitry Torokhov
2024-09-05  4:17 ` [PATCH 22/24] Input: sun4i-ps2 - use guard notation when acquiring spinlock Dmitry Torokhov
2024-09-05  4:17 ` [PATCH 23/24] Input: userio - switch to using cleanup functions Dmitry Torokhov
2024-09-05  4:17 ` [PATCH 24/24] Input: xilinx_ps2 - use guard notation when acquiring spinlock Dmitry Torokhov

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=20240905041732.2034348-14-dmitry.torokhov@gmail.com \
    --to=dmitry.torokhov@gmail.com \
    --cc=decui@microsoft.com \
    --cc=deller@gmx.de \
    --cc=hdegoede@redhat.com \
    --cc=kys@microsoft.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-hyperv@vger.kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sunxi@lists.linux.dev \
    --cc=michal.simek@amd.com \
    --cc=pali@kernel.org \
    --cc=samuel@sholland.org \
    --cc=thatslyude@gmail.com \
    --cc=wei.liu@kernel.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;
as well as URLs for NNTP newsgroup(s).