Skip to content

Commit 7dc406b

Browse files
committed
Input: i8042 - use guard notation when acquiring spinlock
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. Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Dmitry Torokhov <[email protected]>
1 parent c374a0c commit 7dc406b

File tree

1 file changed

+82
-122
lines changed

1 file changed

+82
-122
lines changed

drivers/input/serio/i8042.c

Lines changed: 82 additions & 122 deletions
Original file line numberDiff line numberDiff line change
@@ -197,42 +197,26 @@ EXPORT_SYMBOL(i8042_unlock_chip);
197197
int i8042_install_filter(bool (*filter)(unsigned char data, unsigned char str,
198198
struct serio *serio))
199199
{
200-
unsigned long flags;
201-
int ret = 0;
200+
guard(spinlock_irqsave)(&i8042_lock);
202201

203-
spin_lock_irqsave(&i8042_lock, flags);
204-
205-
if (i8042_platform_filter) {
206-
ret = -EBUSY;
207-
goto out;
208-
}
202+
if (i8042_platform_filter)
203+
return -EBUSY;
209204

210205
i8042_platform_filter = filter;
211-
212-
out:
213-
spin_unlock_irqrestore(&i8042_lock, flags);
214-
return ret;
206+
return 0;
215207
}
216208
EXPORT_SYMBOL(i8042_install_filter);
217209

218210
int i8042_remove_filter(bool (*filter)(unsigned char data, unsigned char str,
219211
struct serio *port))
220212
{
221-
unsigned long flags;
222-
int ret = 0;
223-
224-
spin_lock_irqsave(&i8042_lock, flags);
213+
guard(spinlock_irqsave)(&i8042_lock);
225214

226-
if (i8042_platform_filter != filter) {
227-
ret = -EINVAL;
228-
goto out;
229-
}
215+
if (i8042_platform_filter != filter)
216+
return -EINVAL;
230217

231218
i8042_platform_filter = NULL;
232-
233-
out:
234-
spin_unlock_irqrestore(&i8042_lock, flags);
235-
return ret;
219+
return 0;
236220
}
237221
EXPORT_SYMBOL(i8042_remove_filter);
238222

@@ -271,28 +255,22 @@ static int i8042_wait_write(void)
271255

272256
static int i8042_flush(void)
273257
{
274-
unsigned long flags;
275258
unsigned char data, str;
276259
int count = 0;
277-
int retval = 0;
278260

279-
spin_lock_irqsave(&i8042_lock, flags);
261+
guard(spinlock_irqsave)(&i8042_lock);
280262

281263
while ((str = i8042_read_status()) & I8042_STR_OBF) {
282-
if (count++ < I8042_BUFFER_SIZE) {
283-
udelay(50);
284-
data = i8042_read_data();
285-
dbg("%02x <- i8042 (flush, %s)\n",
286-
data, str & I8042_STR_AUXDATA ? "aux" : "kbd");
287-
} else {
288-
retval = -EIO;
289-
break;
290-
}
291-
}
264+
if (count++ >= I8042_BUFFER_SIZE)
265+
return -EIO;
292266

293-
spin_unlock_irqrestore(&i8042_lock, flags);
267+
udelay(50);
268+
data = i8042_read_data();
269+
dbg("%02x <- i8042 (flush, %s)\n",
270+
data, str & I8042_STR_AUXDATA ? "aux" : "kbd");
271+
}
294272

295-
return retval;
273+
return 0;
296274
}
297275

298276
/*
@@ -349,17 +327,12 @@ static int __i8042_command(unsigned char *param, int command)
349327

350328
int i8042_command(unsigned char *param, int command)
351329
{
352-
unsigned long flags;
353-
int retval;
354-
355330
if (!i8042_present)
356331
return -1;
357332

358-
spin_lock_irqsave(&i8042_lock, flags);
359-
retval = __i8042_command(param, command);
360-
spin_unlock_irqrestore(&i8042_lock, flags);
333+
guard(spinlock_irqsave)(&i8042_lock);
361334

362-
return retval;
335+
return __i8042_command(param, command);
363336
}
364337
EXPORT_SYMBOL(i8042_command);
365338

@@ -369,19 +342,18 @@ EXPORT_SYMBOL(i8042_command);
369342

370343
static int i8042_kbd_write(struct serio *port, unsigned char c)
371344
{
372-
unsigned long flags;
373-
int retval = 0;
345+
int error;
374346

375-
spin_lock_irqsave(&i8042_lock, flags);
347+
guard(spinlock_irqsave)(&i8042_lock);
376348

377-
if (!(retval = i8042_wait_write())) {
378-
dbg("%02x -> i8042 (kbd-data)\n", c);
379-
i8042_write_data(c);
380-
}
349+
error = i8042_wait_write();
350+
if (error)
351+
return error;
381352

382-
spin_unlock_irqrestore(&i8042_lock, flags);
353+
dbg("%02x -> i8042 (kbd-data)\n", c);
354+
i8042_write_data(c);
383355

384-
return retval;
356+
return 0;
385357
}
386358

387359
/*
@@ -460,9 +432,8 @@ static int i8042_start(struct serio *serio)
460432
device_set_wakeup_enable(&serio->dev, true);
461433
}
462434

463-
spin_lock_irq(&i8042_lock);
435+
guard(spinlock_irq)(&i8042_lock);
464436
port->exists = true;
465-
spin_unlock_irq(&i8042_lock);
466437

467438
return 0;
468439
}
@@ -476,10 +447,10 @@ static void i8042_stop(struct serio *serio)
476447
{
477448
struct i8042_port *port = serio->port_data;
478449

479-
spin_lock_irq(&i8042_lock);
480-
port->exists = false;
481-
port->serio = NULL;
482-
spin_unlock_irq(&i8042_lock);
450+
scoped_guard(spinlock_irq, &i8042_lock) {
451+
port->exists = false;
452+
port->serio = NULL;
453+
}
483454

484455
/*
485456
* We need to make sure that interrupt handler finishes using
@@ -583,45 +554,41 @@ static bool i8042_handle_data(int irq)
583554
{
584555
struct i8042_port *port;
585556
struct serio *serio;
586-
unsigned long flags;
587557
unsigned char str, data;
588558
unsigned int dfl;
589559
unsigned int port_no;
590560
bool filtered;
591561

592-
spin_lock_irqsave(&i8042_lock, flags);
593-
594-
str = i8042_read_status();
595-
if (unlikely(~str & I8042_STR_OBF)) {
596-
spin_unlock_irqrestore(&i8042_lock, flags);
597-
return false;
598-
}
599-
600-
data = i8042_read_data();
562+
scoped_guard(spinlock_irqsave, &i8042_lock) {
563+
str = i8042_read_status();
564+
if (unlikely(~str & I8042_STR_OBF))
565+
return false;
601566

602-
if (i8042_mux_present && (str & I8042_STR_AUXDATA)) {
603-
port_no = i8042_handle_mux(str, &data, &dfl);
604-
} else {
567+
data = i8042_read_data();
605568

606-
dfl = (str & I8042_STR_PARITY) ? SERIO_PARITY : 0;
607-
if ((str & I8042_STR_TIMEOUT) && !i8042_notimeout)
608-
dfl |= SERIO_TIMEOUT;
569+
if (i8042_mux_present && (str & I8042_STR_AUXDATA)) {
570+
port_no = i8042_handle_mux(str, &data, &dfl);
571+
} else {
609572

610-
port_no = (str & I8042_STR_AUXDATA) ?
611-
I8042_AUX_PORT_NO : I8042_KBD_PORT_NO;
612-
}
573+
dfl = (str & I8042_STR_PARITY) ? SERIO_PARITY : 0;
574+
if ((str & I8042_STR_TIMEOUT) && !i8042_notimeout)
575+
dfl |= SERIO_TIMEOUT;
613576

614-
port = &i8042_ports[port_no];
615-
serio = port->exists ? port->serio : NULL;
577+
port_no = (str & I8042_STR_AUXDATA) ?
578+
I8042_AUX_PORT_NO : I8042_KBD_PORT_NO;
579+
}
616580

617-
filter_dbg(port->driver_bound, data, "<- i8042 (interrupt, %d, %d%s%s)\n",
618-
port_no, irq,
619-
dfl & SERIO_PARITY ? ", bad parity" : "",
620-
dfl & SERIO_TIMEOUT ? ", timeout" : "");
581+
port = &i8042_ports[port_no];
582+
serio = port->exists ? port->serio : NULL;
621583

622-
filtered = i8042_filter(data, str, serio);
584+
filter_dbg(port->driver_bound,
585+
data, "<- i8042 (interrupt, %d, %d%s%s)\n",
586+
port_no, irq,
587+
dfl & SERIO_PARITY ? ", bad parity" : "",
588+
dfl & SERIO_TIMEOUT ? ", timeout" : "");
623589

624-
spin_unlock_irqrestore(&i8042_lock, flags);
590+
filtered = i8042_filter(data, str, serio);
591+
}
625592

626593
if (likely(serio && !filtered))
627594
serio_interrupt(serio, data, dfl);
@@ -779,24 +746,22 @@ static bool i8042_irq_being_tested;
779746

780747
static irqreturn_t i8042_aux_test_irq(int irq, void *dev_id)
781748
{
782-
unsigned long flags;
783749
unsigned char str, data;
784-
int ret = 0;
785750

786-
spin_lock_irqsave(&i8042_lock, flags);
751+
guard(spinlock_irqsave)(&i8042_lock);
752+
787753
str = i8042_read_status();
788-
if (str & I8042_STR_OBF) {
789-
data = i8042_read_data();
790-
dbg("%02x <- i8042 (aux_test_irq, %s)\n",
791-
data, str & I8042_STR_AUXDATA ? "aux" : "kbd");
792-
if (i8042_irq_being_tested &&
793-
data == 0xa5 && (str & I8042_STR_AUXDATA))
794-
complete(&i8042_aux_irq_delivered);
795-
ret = 1;
796-
}
797-
spin_unlock_irqrestore(&i8042_lock, flags);
754+
if (!(str & I8042_STR_OBF))
755+
return IRQ_NONE;
756+
757+
data = i8042_read_data();
758+
dbg("%02x <- i8042 (aux_test_irq, %s)\n",
759+
data, str & I8042_STR_AUXDATA ? "aux" : "kbd");
760+
761+
if (i8042_irq_being_tested && data == 0xa5 && (str & I8042_STR_AUXDATA))
762+
complete(&i8042_aux_irq_delivered);
798763

799-
return IRQ_RETVAL(ret);
764+
return IRQ_HANDLED;
800765
}
801766

802767
/*
@@ -837,7 +802,6 @@ static int i8042_check_aux(void)
837802
int retval = -1;
838803
bool irq_registered = false;
839804
bool aux_loop_broken = false;
840-
unsigned long flags;
841805
unsigned char param;
842806

843807
/*
@@ -921,18 +885,15 @@ static int i8042_check_aux(void)
921885
if (i8042_enable_aux_port())
922886
goto out;
923887

924-
spin_lock_irqsave(&i8042_lock, flags);
925-
926-
init_completion(&i8042_aux_irq_delivered);
927-
i8042_irq_being_tested = true;
928-
929-
param = 0xa5;
930-
retval = __i8042_command(&param, I8042_CMD_AUX_LOOP & 0xf0ff);
931-
932-
spin_unlock_irqrestore(&i8042_lock, flags);
888+
scoped_guard(spinlock_irqsave, &i8042_lock) {
889+
init_completion(&i8042_aux_irq_delivered);
890+
i8042_irq_being_tested = true;
933891

934-
if (retval)
935-
goto out;
892+
param = 0xa5;
893+
retval = __i8042_command(&param, I8042_CMD_AUX_LOOP & 0xf0ff);
894+
if (retval)
895+
goto out;
896+
}
936897

937898
if (wait_for_completion_timeout(&i8042_aux_irq_delivered,
938899
msecs_to_jiffies(250)) == 0) {
@@ -1020,7 +981,6 @@ static int i8042_controller_selftest(void)
1020981

1021982
static int i8042_controller_init(void)
1022983
{
1023-
unsigned long flags;
1024984
int n = 0;
1025985
unsigned char ctr[2];
1026986

@@ -1057,14 +1017,14 @@ static int i8042_controller_init(void)
10571017
* Handle keylock.
10581018
*/
10591019

1060-
spin_lock_irqsave(&i8042_lock, flags);
1061-
if (~i8042_read_status() & I8042_STR_KEYLOCK) {
1062-
if (i8042_unlock)
1063-
i8042_ctr |= I8042_CTR_IGNKEYLOCK;
1064-
else
1065-
pr_warn("Warning: Keylock active\n");
1020+
scoped_guard(spinlock_irqsave, &i8042_lock) {
1021+
if (~i8042_read_status() & I8042_STR_KEYLOCK) {
1022+
if (i8042_unlock)
1023+
i8042_ctr |= I8042_CTR_IGNKEYLOCK;
1024+
else
1025+
pr_warn("Warning: Keylock active\n");
1026+
}
10661027
}
1067-
spin_unlock_irqrestore(&i8042_lock, flags);
10681028

10691029
/*
10701030
* If the chip is configured into nontranslated mode by the BIOS, don't

0 commit comments

Comments
 (0)