[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240905041732.2034348-11-dmitry.torokhov@gmail.com>
Date: Wed, 4 Sep 2024 21:17:15 -0700
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: linux-input@...r.kernel.org
Cc: Pali Rohár <pali@...nel.org>,
Helge Deller <deller@....de>,
"K. Y. Srinivasan" <kys@...rosoft.com>,
Wei Liu <wei.liu@...nel.org>,
Dexuan Cui <decui@...rosoft.com>,
Samuel Holland <samuel@...lland.org>,
Lyude Paul <thatslyude@...il.com>,
Michal Simek <michal.simek@....com>,
Hans de Goede <hdegoede@...hat.com>,
linux-kernel@...r.kernel.org,
linux-hyperv@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
linux-sunxi@...ts.linux.dev
Subject: [PATCH 10/24] Input: gscps2 - 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.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
---
drivers/input/serio/gscps2.c | 114 +++++++++++++++++++----------------
1 file changed, 62 insertions(+), 52 deletions(-)
diff --git a/drivers/input/serio/gscps2.c b/drivers/input/serio/gscps2.c
index d94c01eb3fc9..cf0603d1a113 100644
--- a/drivers/input/serio/gscps2.c
+++ b/drivers/input/serio/gscps2.c
@@ -145,7 +145,6 @@ static void gscps2_flush(struct gscps2port *ps2port)
static inline int gscps2_writeb_output(struct gscps2port *ps2port, u8 data)
{
- unsigned long flags;
char __iomem *addr = ps2port->addr;
if (!wait_TBE(addr)) {
@@ -156,9 +155,8 @@ static inline int gscps2_writeb_output(struct gscps2port *ps2port, u8 data)
while (gscps2_readb_status(addr) & GSC_STAT_RBNE)
/* wait */;
- spin_lock_irqsave(&ps2port->lock, flags);
- writeb(data, addr+GSC_XMTDATA);
- spin_unlock_irqrestore(&ps2port->lock, flags);
+ scoped_guard(spinlock_irqsave, &ps2port->lock)
+ writeb(data, addr+GSC_XMTDATA);
/* this is ugly, but due to timing of the port it seems to be necessary. */
mdelay(6);
@@ -177,19 +175,19 @@ static inline int gscps2_writeb_output(struct gscps2port *ps2port, u8 data)
static void gscps2_enable(struct gscps2port *ps2port, int enable)
{
- unsigned long flags;
u8 data;
/* now enable/disable the port */
- spin_lock_irqsave(&ps2port->lock, flags);
- gscps2_flush(ps2port);
- data = gscps2_readb_control(ps2port->addr);
- if (enable)
- data |= GSC_CTRL_ENBL;
- else
- data &= ~GSC_CTRL_ENBL;
- gscps2_writeb_control(data, ps2port->addr);
- spin_unlock_irqrestore(&ps2port->lock, flags);
+ scoped_guard(spinlock_irqsave, &ps2port->lock) {
+ gscps2_flush(ps2port);
+ data = gscps2_readb_control(ps2port->addr);
+ if (enable)
+ data |= GSC_CTRL_ENBL;
+ else
+ data &= ~GSC_CTRL_ENBL;
+ gscps2_writeb_control(data, ps2port->addr);
+ }
+
wait_TBE(ps2port->addr);
gscps2_flush(ps2port);
}
@@ -203,15 +201,56 @@ static void gscps2_reset(struct gscps2port *ps2port)
unsigned long flags;
/* reset the interface */
- spin_lock_irqsave(&ps2port->lock, flags);
+ guard(spinlock_irqsave)(&ps2port->lock);
gscps2_flush(ps2port);
writeb(0xff, ps2port->addr + GSC_RESET);
gscps2_flush(ps2port);
- spin_unlock_irqrestore(&ps2port->lock, flags);
}
static LIST_HEAD(ps2port_list);
+static void gscps2_read_data(struct gscps2port *ps2port)
+{
+ u8 status;
+
+ do {
+ status = gscps2_readb_status(ps2port->addr);
+ if (!(status & GSC_STAT_RBNE))
+ break;
+
+ ps2port->buffer[ps2port->append].ste = status;
+ ps2port->buffer[ps2port->append].data =
+ gscps2_readb_input(ps2port->addr);
+ } while (true);
+}
+
+static bool gscps2_report_data(struct gscps2port *ps2port)
+{
+ unsigned int rxflags;
+ u8 data, status;
+
+ while (ps2port->act != ps2port->append) {
+ /*
+ * Did new data arrived while we read existing data ?
+ * If yes, exit now and let the new irq handler start
+ * over again.
+ */
+ if (gscps2_readb_status(ps2port->addr) & GSC_STAT_CMPINTR)
+ return true;
+
+ status = ps2port->buffer[ps2port->act].str;
+ data = ps2port->buffer[ps2port->act].data;
+
+ ps2port->act = (ps2port->act + 1) & BUFFER_SIZE;
+ rxflags = ((status & GSC_STAT_TERR) ? SERIO_TIMEOUT : 0 ) |
+ ((status & GSC_STAT_PERR) ? SERIO_PARITY : 0 );
+
+ serio_interrupt(ps2port->port, data, rxflags);
+ }
+
+ return false;
+}
+
/**
* gscps2_interrupt() - Interruption service routine
*
@@ -229,47 +268,18 @@ static irqreturn_t gscps2_interrupt(int irq, void *dev)
struct gscps2port *ps2port;
list_for_each_entry(ps2port, &ps2port_list, node) {
+ guard(spinlock_irqsave)(&ps2port->lock);
- unsigned long flags;
- spin_lock_irqsave(&ps2port->lock, flags);
-
- while ( (ps2port->buffer[ps2port->append].str =
- gscps2_readb_status(ps2port->addr)) & GSC_STAT_RBNE ) {
- ps2port->buffer[ps2port->append].data =
- gscps2_readb_input(ps2port->addr);
- ps2port->append = ((ps2port->append+1) & BUFFER_SIZE);
- }
-
- spin_unlock_irqrestore(&ps2port->lock, flags);
-
+ gscps2_read_data(ps2port);
} /* list_for_each_entry */
/* all data was read from the ports - now report the data to upper layer */
-
list_for_each_entry(ps2port, &ps2port_list, node) {
-
- while (ps2port->act != ps2port->append) {
-
- unsigned int rxflags;
- u8 data, status;
-
- /* Did new data arrived while we read existing data ?
- If yes, exit now and let the new irq handler start over again */
- if (gscps2_readb_status(ps2port->addr) & GSC_STAT_CMPINTR)
- return IRQ_HANDLED;
-
- status = ps2port->buffer[ps2port->act].str;
- data = ps2port->buffer[ps2port->act].data;
-
- ps2port->act = ((ps2port->act+1) & BUFFER_SIZE);
- rxflags = ((status & GSC_STAT_TERR) ? SERIO_TIMEOUT : 0 ) |
- ((status & GSC_STAT_PERR) ? SERIO_PARITY : 0 );
-
- serio_interrupt(ps2port->port, data, rxflags);
-
- } /* while() */
-
- } /* list_for_each_entry */
+ if (gscps2_report_data(ps2port)) {
+ /* More data ready - break early to restart interrupt */
+ break;
+ }
+ }
return IRQ_HANDLED;
}
--
2.46.0.469.g59c65b2a67-goog
Powered by blists - more mailing lists