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(¶m, 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(¶m, 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
next prev 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).