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 10/24] Input: gscps2 - use guard notation when acquiring spinlock
Date: Wed,  4 Sep 2024 21:17:15 -0700	[thread overview]
Message-ID: <20240905041732.2034348-11-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/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


  parent reply	other threads:[~2024-09-05  4:17 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 ` Dmitry Torokhov [this message]
2024-09-05  4:17 ` [PATCH 11/24] Input: hyperv-keyboard - use guard notation when acquiring spinlock Dmitry Torokhov
2024-09-05  4:17 ` [PATCH 12/24] Input: i8042 - tease apart interrupt handler Dmitry Torokhov
2024-09-05  4:17 ` [PATCH 13/24] Input: i8042 - use guard notation when acquiring spinlock Dmitry Torokhov
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-11-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).