[PATCH 0/7] Input: gscps2: cleanups and locking fixes

[PATCH 0/7] Input: gscps2: cleanups and locking fixes

[Dmitry Torokhov]

This series cleans up the PA-RISC GSC PS/2 driver (gscps2) and addresses
several concurrency, locking, and interrupt handling issues.

The series performs code style and structural cleanups, including removing
forward declarations, converting printk calls to dev_* logging helpers,
and simplifying enable/disable handling.

Additionally, it resolves several concurrency, locking, and interrupt
handling issues:
- Protect ps2port_list traversal with RCU and manage port registration in
  open/close rather than probe/remove to avoid use-after-free during
  device unregistration.
- Protect ring buffer indices with ps2port->lock in read and report
  helpers while releasing the lock before calling serio_interrupt().
- Assert and enforce ps2port->lock across gscps2_flush() callers.
- Return IRQ_NONE when no data was handled on shared interrupt lines to
  preserve core spurious interrupt detection.
- Serialize concurrent interrupt handlers using a try-lock to guarantee
  in-order byte delivery.
- Drop the 6 ms busy-wait and manual interrupt pump on transmit in favor
  of standard asynchronous hardware interrupt delivery.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
Dmitry Torokhov (7):
      Input: gscps2 - clean up driver code style and structure
      Input: gscps2 - use RCU for ps2port_list and manage it in open/close
      Input: gscps2 - protect buffer access in read and report helpers
      Input: gscps2 - serialize hardware and buffer access in gscps2_flush()
      Input: gscps2 - return IRQ_NONE when interrupt is not handled
      Input: gscps2 - serialize concurrent interrupt handlers
      Input: gscps2 - drop busy-wait and manual interrupt pump on transmit

 drivers/input/serio/gscps2.c | 277 ++++++++++++++++++++++---------------------
 1 file changed, 139 insertions(+), 138 deletions(-)
---
base-commit: e30626823a406725ce29bc75cb8ec467d3e1e326
change-id: 20260809-gscps2-affec5caa7f9

Thanks.

-- 
Dmitry

[PATCH 1/7] Input: gscps2 - clean up driver code style and structure

[Dmitry Torokhov]

Clean up code style issues and function ordering in the gscps2 driver:
- Reorder functions to place gscps2_interrupt() before its callers,
  allowing removal of its forward declaration.
- Change gscps2_enable() to accept a boolean parameter and remove the
  ENABLE and DISABLE macro definitions.
- Convert printk() calls to dev_dbg() and dev_warn().
- Fix operator spacing and multi-variable assignment.
- Add spinlock comment and use cpu_relax() in spin-wait loop.

Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/serio/gscps2.c | 208 +++++++++++++++++++------------------------
 1 file changed, 91 insertions(+), 117 deletions(-)

diff --git a/drivers/input/serio/gscps2.c b/drivers/input/serio/gscps2.c
index 43453ec533b2..b82c56ba8ff7 100644
--- a/drivers/input/serio/gscps2.c
+++ b/drivers/input/serio/gscps2.c
@@ -1,6 +1,4 @@
 /*
- * drivers/input/serio/gscps2.c
- *
  * Copyright (c) 2004-2006 Helge Deller <deller@gmx.de>
  * Copyright (c) 2002 Laurent Canet <canetl@esiee.fr>
  * Copyright (c) 2002 Thibaut Varene <varenet@parisc-linux.org>
@@ -37,16 +35,10 @@ MODULE_AUTHOR("Laurent Canet <canetl@esiee.fr>, Thibaut Varene <varenet@parisc-l
 MODULE_DESCRIPTION("HP GSC PS2 port driver");
 MODULE_LICENSE("GPL");
 
-#define PFX "gscps2.c: "
-
 /*
  * Driver constants
  */
 
-/* various constants */
-#define ENABLE			1
-#define DISABLE			0
-
 #define GSC_DINO_OFFSET		0x0800	/* offset for DINO controller versus LASI one */
 
 /* PS/2 IO port offsets */
@@ -77,23 +69,21 @@ MODULE_LICENSE("GPL");
 #define GSC_ID_KEYBOARD		0	/* device ID values */
 #define GSC_ID_MOUSE		1
 
-#ifndef CONFIG_SERIO_GSCPS2_RDI_KEYCODES
-# define CONFLICT(x, y) x
-#else
-# define CONFLICT(x, y) y
-#endif
+#define RDI_CONFLICT(x, y) \
+	((x) * !IS_ENABLED(CONFIG_SERIO_GSCPS2_RDI_KEYCODES) + \
+	 (y) * IS_ENABLED(CONFIG_SERIO_GSCPS2_RDI_KEYCODES))
 
 /*
  * Sadly RDI (Tadpole) decided to ship a different keyboard layout
  * than HP for their PS/2 laptop keyboard which leads to conflicting
  * keycodes between a normal HP PS/2 keyboard and a RDI PrecisionBook.
- *                       HP:            RDI:
+ *                           HP:                RDI:
  */
-#define C_07	CONFLICT(KEY_F12,	KEY_F1)
-#define C_11	CONFLICT(KEY_LEFTALT,	KEY_LEFTCTRL)
-#define C_14	CONFLICT(KEY_LEFTCTRL,	KEY_CAPSLOCK)
-#define C_58	CONFLICT(KEY_CAPSLOCK,	KEY_RIGHTCTRL)
-#define C_61	CONFLICT(KEY_102ND,	KEY_LEFT)
+#define C_07	RDI_CONFLICT(KEY_F12,		KEY_F1)
+#define C_11	RDI_CONFLICT(KEY_LEFTALT,	KEY_LEFTCTRL)
+#define C_14	RDI_CONFLICT(KEY_LEFTCTRL,	KEY_CAPSLOCK)
+#define C_58	RDI_CONFLICT(KEY_CAPSLOCK,	KEY_RIGHTCTRL)
+#define C_61	RDI_CONFLICT(KEY_102ND,		KEY_LEFT)
 
 /*
  * Special keycode value recognized by atkbd (ATKBD_KEY_NULL) to silently
@@ -188,8 +178,6 @@ static const struct software_node gscps2_keyboard_node = {
 	.properties = gscps2_props,
 };
 
-static irqreturn_t gscps2_interrupt(int irq, void *dev);
-
 #define BUFFER_SIZE 0x0f
 
 /* GSC PS/2 port device struct */
@@ -197,33 +185,34 @@ struct gscps2port {
 	struct list_head node;
 	struct parisc_device *padev;
 	struct serio *port;
-	spinlock_t lock;
+	spinlock_t lock; /* serializes HW access */
 	char __iomem *addr;
 	u8 act, append; /* position in buffer[] */
 	struct {
 		u8 data;
 		u8 str;
-	} buffer[BUFFER_SIZE+1];
+	} buffer[BUFFER_SIZE + 1];
 	int id;
 };
 
+static LIST_HEAD(ps2port_list);
+
 /*
  * Various HW level routines
  */
 
-#define gscps2_readb_input(x)		readb((x)+GSC_RCVDATA)
-#define gscps2_readb_control(x)		readb((x)+GSC_CONTROL)
-#define gscps2_readb_status(x)		readb((x)+GSC_STATUS)
-#define gscps2_writeb_control(x, y)	writeb((x), (y)+GSC_CONTROL)
-
+#define gscps2_readb_input(x)		readb((x) + GSC_RCVDATA)
+#define gscps2_readb_control(x)		readb((x) + GSC_CONTROL)
+#define gscps2_readb_status(x)		readb((x) + GSC_STATUS)
+#define gscps2_writeb_control(x, y)	writeb((x), (y) + GSC_CONTROL)
 
 /*
  * wait_TBE() - wait for Transmit Buffer Empty
  */
-
 static int wait_TBE(char __iomem *addr)
 {
 	int timeout = 25000; /* device is expected to react within 250 msec */
+
 	while (gscps2_readb_status(addr) & GSC_STAT_TBNE) {
 		if (!--timeout)
 			return 0;	/* This should not happen */
@@ -232,88 +221,17 @@ static int wait_TBE(char __iomem *addr)
 	return 1;
 }
 
-
 /*
  * gscps2_flush() - flush the receive buffer
  */
-
 static void gscps2_flush(struct gscps2port *ps2port)
 {
 	while (gscps2_readb_status(ps2port->addr) & GSC_STAT_RBNE)
 		gscps2_readb_input(ps2port->addr);
-	ps2port->act = ps2port->append = 0;
-}
-
-/*
- * gscps2_writeb_output() - write a byte to the port
- *
- * returns 1 on success, 0 on error
- */
-
-static inline int gscps2_writeb_output(struct gscps2port *ps2port, u8 data)
-{
-	char __iomem *addr = ps2port->addr;
-
-	if (!wait_TBE(addr)) {
-		printk(KERN_DEBUG PFX "timeout - could not write byte %#x\n", data);
-		return 0;
-	}
-
-	while (gscps2_readb_status(addr) & GSC_STAT_RBNE)
-		/* wait */;
-
-	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);
-
-	/* make sure any received data is returned as fast as possible */
-	/* this is important e.g. when we set the LEDs on the keyboard */
-	gscps2_interrupt(0, NULL);
-
-	return 1;
-}
-
-
-/*
- * gscps2_enable() - enables or disables the port
- */
-
-static void gscps2_enable(struct gscps2port *ps2port, int enable)
-{
-	u8 data;
-
-	/* now enable/disable the port */
-	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);
-}
-
-/*
- * gscps2_reset() - resets the PS/2 port
- */
-
-static void gscps2_reset(struct gscps2port *ps2port)
-{
-	/* reset the interface */
-	guard(spinlock_irqsave)(&ps2port->lock);
-	gscps2_flush(ps2port);
-	writeb(0xff, ps2port->addr + GSC_RESET);
-	gscps2_flush(ps2port);
+	ps2port->act = 0;
+	ps2port->append = 0;
 }
 
-static LIST_HEAD(ps2port_list);
-
 static void gscps2_read_data(struct gscps2port *ps2port)
 {
 	u8 status;
@@ -348,8 +266,8 @@ static bool gscps2_report_data(struct gscps2port *ps2port)
 		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 );
+		rxflags = ((status & GSC_STAT_TERR) ? SERIO_TIMEOUT : 0) |
+			  ((status & GSC_STAT_PERR) ? SERIO_PARITY  : 0);
 
 		serio_interrupt(ps2port->port, data, rxflags);
 	}
@@ -370,7 +288,6 @@ static bool gscps2_report_data(struct gscps2port *ps2port)
  * the data as fast as possible and handle the reporting to the upper layer
  * later.
  */
-
 static irqreturn_t gscps2_interrupt(int irq, void *dev)
 {
 	struct gscps2port *ps2port;
@@ -392,17 +309,79 @@ static irqreturn_t gscps2_interrupt(int irq, void *dev)
 	return IRQ_HANDLED;
 }
 
+/*
+ * gscps2_writeb_output() - write a byte to the port
+ *
+ * returns 1 on success, 0 on error
+ */
+static inline int gscps2_writeb_output(struct gscps2port *ps2port, u8 data)
+{
+	char __iomem *addr = ps2port->addr;
+
+	if (!wait_TBE(addr)) {
+		dev_dbg(&ps2port->padev->dev, "timeout - could not write byte %#x\n", data);
+		return 0;
+	}
+
+	while (gscps2_readb_status(addr) & GSC_STAT_RBNE)
+		cpu_relax();
+
+	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);
+
+	/* make sure any received data is returned as fast as possible */
+	/* this is important e.g. when we set the LEDs on the keyboard */
+	gscps2_interrupt(0, NULL);
+
+	return 1;
+}
 
 /*
- * gscps2_write() - send a byte out through the aux interface.
+ * gscps2_enable() - enables or disables the port
  */
+static void gscps2_enable(struct gscps2port *ps2port, bool enable)
+{
+	u8 data;
 
+	/* now enable/disable the port */
+	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);
+}
+
+/*
+ * gscps2_reset() - resets the PS/2 port
+ */
+static void gscps2_reset(struct gscps2port *ps2port)
+{
+	/* reset the interface */
+	guard(spinlock_irqsave)(&ps2port->lock);
+	gscps2_flush(ps2port);
+	writeb(0xff, ps2port->addr + GSC_RESET);
+	gscps2_flush(ps2port);
+}
+
+/*
+ * gscps2_write() - send a byte out through the aux interface.
+ */
 static int gscps2_write(struct serio *port, unsigned char data)
 {
 	struct gscps2port *ps2port = port->port_data;
 
 	if (!gscps2_writeb_output(ps2port, data)) {
-		printk(KERN_DEBUG PFX "sending byte %#x failed.\n", data);
+		dev_dbg(&ps2port->padev->dev, "sending byte %#x failed.\n", data);
 		return -1;
 	}
 	return 0;
@@ -412,7 +391,6 @@ static int gscps2_write(struct serio *port, unsigned char data)
  * gscps2_open() is called when a port is opened by the higher layer.
  * It resets and enables the port.
  */
-
 static int gscps2_open(struct serio *port)
 {
 	struct gscps2port *ps2port = port->port_data;
@@ -420,7 +398,7 @@ static int gscps2_open(struct serio *port)
 	gscps2_reset(ps2port);
 
 	/* enable it */
-	gscps2_enable(ps2port, ENABLE);
+	gscps2_enable(ps2port, true);
 
 	gscps2_interrupt(0, NULL);
 
@@ -430,11 +408,11 @@ static int gscps2_open(struct serio *port)
 /*
  * gscps2_close() disables the port
  */
-
 static void gscps2_close(struct serio *port)
 {
 	struct gscps2port *ps2port = port->port_data;
-	gscps2_enable(ps2port, DISABLE);
+
+	gscps2_enable(ps2port, false);
 }
 
 /**
@@ -443,7 +421,6 @@ static void gscps2_close(struct serio *port)
  *
  * @return: success/error report
  */
-
 static int __init gscps2_probe(struct parisc_device *dev)
 {
 	struct gscps2port *ps2port;
@@ -494,8 +471,8 @@ static int __init gscps2_probe(struct parisc_device *dev)
 		goto fail_miserably;
 
 	if (ps2port->id != GSC_ID_KEYBOARD && ps2port->id != GSC_ID_MOUSE) {
-		printk(KERN_WARNING PFX "Unsupported PS/2 port at 0x%08lx (id=%d) ignored\n",
-				hpa, ps2port->id);
+		dev_warn(&dev->dev, "Unsupported PS/2 port at 0x%08lx (id=%d) ignored\n",
+			 hpa, ps2port->id);
 		ret = -ENODEV;
 		goto fail;
 	}
@@ -552,7 +529,6 @@ static int __init gscps2_probe(struct parisc_device *dev)
  *
  * @return: success/error report
  */
-
 static void __exit gscps2_remove(struct parisc_device *dev)
 {
 	struct gscps2port *ps2port = dev_get_drvdata(&dev->dev);
@@ -572,7 +548,6 @@ static void __exit gscps2_remove(struct parisc_device *dev)
 	kfree(ps2port);
 }
 
-
 static const struct parisc_device_id gscps2_device_tbl[] __initconst = {
 	{ HPHW_FIO, HVERSION_REV_ANY_ID, HVERSION_ANY_ID, 0x00084 }, /* LASI PS/2 */
 #ifdef DINO_TESTED
@@ -610,6 +585,5 @@ static void __exit gscps2_exit(void)
 	software_node_unregister(&gscps2_keyboard_node);
 }
 
-
 module_init(gscps2_init);
 module_exit(gscps2_exit);

-- 
2.55.0.897.gb25b4bd76c-goog

[PATCH 2/7] Input: gscps2 - use RCU for ps2port_list and manage it in open/close

[Dmitry Torokhov]

Managing ps2port_list in gscps2_probe() and gscps2_remove() had two
issues:

- in gscps2_remove(), serio_unregister_port() frees the serio port, but
  because the port remained in ps2port_list until later in remove, a
  shared interrupt firing on another CPU could traverse ps2port_list and
  dereference the freed serio port

- ps2port_list additions and deletions in probe/remove raced locklessly
  against list traversals in gscps2_interrupt().

Convert ps2port_list traversal in gscps2_interrupt() to use RCU, and
move list management to gscps2_open() and gscps2_close(). When
serio_unregister_port() runs during device removal, serio_close() is
invoked, cleanly taking the port out of ps2port_list before the serio
structure is destroyed, while maintaining active hardware communication
during child driver disconnect.

Reported-by: sashiko-bot@kernel.org
Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/serio/gscps2.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/input/serio/gscps2.c b/drivers/input/serio/gscps2.c
index b82c56ba8ff7..5b6e311f8a02 100644
--- a/drivers/input/serio/gscps2.c
+++ b/drivers/input/serio/gscps2.c
@@ -26,6 +26,7 @@
 #include <linux/io.h>
 #include <linux/ioport.h>
 #include <linux/property.h>
+#include <linux/rcupdate.h>
 #include <linux/serio.h>
 
 #include <asm/irq.h>
@@ -195,6 +196,7 @@ struct gscps2port {
 	int id;
 };
 
+static DEFINE_SPINLOCK(ps2port_list_lock);
 static LIST_HEAD(ps2port_list);
 
 /*
@@ -292,14 +294,16 @@ static irqreturn_t gscps2_interrupt(int irq, void *dev)
 {
 	struct gscps2port *ps2port;
 
-	list_for_each_entry(ps2port, &ps2port_list, node) {
+	guard(rcu)();
+
+	list_for_each_entry_rcu(ps2port, &ps2port_list, node) {
 		guard(spinlock_irqsave)(&ps2port->lock);
 
 		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) {
+	list_for_each_entry_rcu(ps2port, &ps2port_list, node) {
 		if (gscps2_report_data(ps2port)) {
 			/* More data ready - break early to restart interrupt */
 			break;
@@ -397,6 +401,9 @@ static int gscps2_open(struct serio *port)
 
 	gscps2_reset(ps2port);
 
+	scoped_guard(spinlock_irqsave, &ps2port_list_lock)
+		list_add_tail_rcu(&ps2port->node, &ps2port_list);
+
 	/* enable it */
 	gscps2_enable(ps2port, true);
 
@@ -413,6 +420,11 @@ static void gscps2_close(struct serio *port)
 	struct gscps2port *ps2port = port->port_data;
 
 	gscps2_enable(ps2port, false);
+
+	scoped_guard(spinlock_irqsave, &ps2port_list_lock)
+		list_del_rcu(&ps2port->node);
+
+	synchronize_rcu();
 }
 
 /**
@@ -446,6 +458,7 @@ static int __init gscps2_probe(struct parisc_device *dev)
 
 	ps2port->port = serio;
 	ps2port->padev = dev;
+	INIT_LIST_HEAD(&ps2port->node);
 	ps2port->addr = ioremap(hpa, GSC_STATUS + 4);
 	if (!ps2port->addr) {
 		ret = -ENOMEM;
@@ -501,8 +514,6 @@ static int __init gscps2_probe(struct parisc_device *dev)
 
 	serio_register_port(ps2port->port);
 
-	list_add_tail(&ps2port->node, &ps2port_list);
-
 	return 0;
 
 fail:
@@ -539,12 +550,10 @@ static void __exit gscps2_remove(struct parisc_device *dev)
 	serio_unregister_port(ps2port->port);
 	free_irq(dev->irq, ps2port);
 	gscps2_flush(ps2port);
-	list_del(&ps2port->node);
 	iounmap(ps2port->addr);
 #if 0
 	release_mem_region(dev->hpa, GSC_STATUS + 4);
 #endif
-	dev_set_drvdata(&dev->dev, NULL);
 	kfree(ps2port);
 }
 

-- 
2.55.0.897.gb25b4bd76c-goog

[PATCH 3/7] Input: gscps2 - protect buffer access in read and report helpers

[Dmitry Torokhov]

In gscps2_report_data(), the ring buffer consumer index ps2port->act was
read and updated locklessly. When gscps2_interrupt() was called from
process context (such as during port write or open) concurrently with a
hardware interrupt running on another CPU, two execution contexts could
execute gscps2_report_data() simultaneously for the same port, racing on
ps2port->act and leading to duplicate, skipped, or out-of-order bytes.

Protect buffer access by taking ps2port->lock inside gscps2_read_data()
and gscps2_report_data(). In gscps2_report_data(), acquire ps2port->lock
only when popping entries from the ring buffer and release it before
calling serio_interrupt() to avoid recursive deadlocks if the input
driver synchronously sends a command back via serio_write().

Reported-by: sashiko-bot@kernel.org
Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/serio/gscps2.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/input/serio/gscps2.c b/drivers/input/serio/gscps2.c
index 5b6e311f8a02..fef6fffb6f86 100644
--- a/drivers/input/serio/gscps2.c
+++ b/drivers/input/serio/gscps2.c
@@ -238,6 +238,8 @@ static void gscps2_read_data(struct gscps2port *ps2port)
 {
 	u8 status;
 
+	guard(spinlock_irqsave)(&ps2port->lock);
+
 	do {
 		status = gscps2_readb_status(ps2port->addr);
 		if (!(status & GSC_STAT_RBNE))
@@ -255,7 +257,7 @@ static bool gscps2_report_data(struct gscps2port *ps2port)
 	unsigned int rxflags;
 	u8 data, status;
 
-	while (ps2port->act != ps2port->append) {
+	while (true) {
 		/*
 		 * Did new data arrived while we read existing data ?
 		 * If yes, exit now and let the new irq handler start
@@ -264,17 +266,20 @@ static bool gscps2_report_data(struct gscps2port *ps2port)
 		if (gscps2_readb_status(ps2port->addr) & GSC_STAT_CMPINTR)
 			return true;
 
-		status = ps2port->buffer[ps2port->act].str;
-		data   = ps2port->buffer[ps2port->act].data;
+		scoped_guard(spinlock_irqsave, &ps2port->lock) {
+			if (ps2port->act == ps2port->append)
+				return false;
+
+			status = ps2port->buffer[ps2port->act].str;
+			data   = ps2port->buffer[ps2port->act].data;
+			ps2port->act = (ps2port->act + 1) & BUFFER_SIZE;
+		}
 
-		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;
 }
 
 /**
@@ -296,11 +301,8 @@ static irqreturn_t gscps2_interrupt(int irq, void *dev)
 
 	guard(rcu)();
 
-	list_for_each_entry_rcu(ps2port, &ps2port_list, node) {
-		guard(spinlock_irqsave)(&ps2port->lock);
-
+	list_for_each_entry_rcu(ps2port, &ps2port_list, node)
 		gscps2_read_data(ps2port);
-	}
 
 	/* all data was read from the ports - now report the data to upper layer */
 	list_for_each_entry_rcu(ps2port, &ps2port_list, node) {

-- 
2.55.0.897.gb25b4bd76c-goog

[PATCH 4/7] Input: gscps2 - serialize hardware and buffer access in gscps2_flush()

[Dmitry Torokhov]

gscps2_flush() reads from hardware registers and resets the ring buffer
indices ps2port->act and ps2port->append. In gscps2_enable(), the
trailing gscps2_flush() was called without holding ps2port->lock, racing
with concurrent hardware interrupts and buffer access.

Assert that ps2port->lock is held in gscps2_flush() with
lockdep_assert_held(), and ensure all callers acquire ps2port->lock so
that multi-step hardware sequences remain fully serialized without
unprotected windows.

Reported-by: sashiko-bot@kernel.org
Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/serio/gscps2.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/input/serio/gscps2.c b/drivers/input/serio/gscps2.c
index fef6fffb6f86..36c25db9ff7f 100644
--- a/drivers/input/serio/gscps2.c
+++ b/drivers/input/serio/gscps2.c
@@ -228,6 +228,8 @@ static int wait_TBE(char __iomem *addr)
  */
 static void gscps2_flush(struct gscps2port *ps2port)
 {
+	lockdep_assert_held(&ps2port->lock);
+
 	while (gscps2_readb_status(ps2port->addr) & GSC_STAT_RBNE)
 		gscps2_readb_input(ps2port->addr);
 	ps2port->act = 0;
@@ -364,7 +366,8 @@ static void gscps2_enable(struct gscps2port *ps2port, bool enable)
 	}
 
 	wait_TBE(ps2port->addr);
-	gscps2_flush(ps2port);
+	scoped_guard(spinlock_irqsave, &ps2port->lock)
+		gscps2_flush(ps2port);
 }
 
 /*
@@ -551,7 +554,8 @@ static void __exit gscps2_remove(struct parisc_device *dev)
 
 	serio_unregister_port(ps2port->port);
 	free_irq(dev->irq, ps2port);
-	gscps2_flush(ps2port);
+	scoped_guard(spinlock_irqsave, &ps2port->lock)
+		gscps2_flush(ps2port);
 	iounmap(ps2port->addr);
 #if 0
 	release_mem_region(dev->hpa, GSC_STATUS + 4);

-- 
2.55.0.897.gb25b4bd76c-goog

[PATCH 5/7] Input: gscps2 - return IRQ_NONE when interrupt is not handled

[Dmitry Torokhov]

gscps2_interrupt() is registered with IRQF_SHARED. Unconditionally
returning IRQ_HANDLED when no data was pending on any GSC PS/2 port
masks unhandled interrupts on the shared interrupt line and prevents the
kernel core spurious interrupt detector from identifying runaway
interrupt storms.

Have gscps2_read_data() return whether any bytes were read, accumulate
the handled status in gscps2_interrupt(), and return IRQ_RETVAL(handled).

Reported-by: sashiko-bot@kernel.org
Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/serio/gscps2.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/input/serio/gscps2.c b/drivers/input/serio/gscps2.c
index 36c25db9ff7f..907fb1537595 100644
--- a/drivers/input/serio/gscps2.c
+++ b/drivers/input/serio/gscps2.c
@@ -236,8 +236,9 @@ static void gscps2_flush(struct gscps2port *ps2port)
 	ps2port->append = 0;
 }
 
-static void gscps2_read_data(struct gscps2port *ps2port)
+static bool gscps2_read_data(struct gscps2port *ps2port)
 {
+	bool read_any = false;
 	u8 status;
 
 	guard(spinlock_irqsave)(&ps2port->lock);
@@ -247,11 +248,14 @@ static void gscps2_read_data(struct gscps2port *ps2port)
 		if (!(status & GSC_STAT_RBNE))
 			break;
 
+		read_any = true;
 		ps2port->buffer[ps2port->append].str = status;
 		ps2port->buffer[ps2port->append].data =
 				gscps2_readb_input(ps2port->addr);
 		ps2port->append = (ps2port->append + 1) & BUFFER_SIZE;
 	} while (true);
+
+	return read_any;
 }
 
 static bool gscps2_report_data(struct gscps2port *ps2port)
@@ -300,11 +304,14 @@ static bool gscps2_report_data(struct gscps2port *ps2port)
 static irqreturn_t gscps2_interrupt(int irq, void *dev)
 {
 	struct gscps2port *ps2port;
+	bool handled = false;
 
 	guard(rcu)();
 
-	list_for_each_entry_rcu(ps2port, &ps2port_list, node)
-		gscps2_read_data(ps2port);
+	list_for_each_entry_rcu(ps2port, &ps2port_list, node) {
+		if (gscps2_read_data(ps2port))
+			handled = true;
+	}
 
 	/* all data was read from the ports - now report the data to upper layer */
 	list_for_each_entry_rcu(ps2port, &ps2port_list, node) {
@@ -314,7 +321,7 @@ static irqreturn_t gscps2_interrupt(int irq, void *dev)
 		}
 	}
 
-	return IRQ_HANDLED;
+	return IRQ_RETVAL(handled);
 }
 
 /*

-- 
2.55.0.897.gb25b4bd76c-goog

[PATCH 6/7] Input: gscps2 - serialize concurrent interrupt handlers

[Dmitry Torokhov]

gscps2_interrupt() may be invoked concurrently from a hardware interrupt
on one CPU and from process context via gscps2_writeb_output() on another
CPU. In gscps2_report_data(), ps2port->lock is released before calling
serio_interrupt() to avoid recursive deadlocks. However, if two execution
contexts run gscps2_report_data() concurrently for the same port, they
could race to acquire serio->lock inside serio_interrupt(), potentially
delivering multi-byte scancodes out of order.

Serialize execution of gscps2_interrupt() using gscps2_interrupt_lock
with ACQUIRE(spinlock_irqsave_try). Using spin_trylock prevents
overlapping executions and guarantees in-order packet delivery without
risking recursive deadlocks if an input driver synchronously sends a
command back via serio_write(). If the lock cannot be acquired, return
IRQ_NONE to preserve spurious interrupt detection on shared IRQ lines.

Reported-by: sashiko-bot@kernel.org
Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/serio/gscps2.c | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/drivers/input/serio/gscps2.c b/drivers/input/serio/gscps2.c
index 907fb1537595..2afd53a163ff 100644
--- a/drivers/input/serio/gscps2.c
+++ b/drivers/input/serio/gscps2.c
@@ -288,6 +288,8 @@ static bool gscps2_report_data(struct gscps2port *ps2port)
 	}
 }
 
+static DEFINE_SPINLOCK(gscps2_interrupt_lock);
+
 /**
  * gscps2_interrupt() - Interruption service routine
  * @irq: interrupt number which triggered (unused)
@@ -305,21 +307,31 @@ static irqreturn_t gscps2_interrupt(int irq, void *dev)
 {
 	struct gscps2port *ps2port;
 	bool handled = false;
+	bool more_data;
+
+	ACQUIRE(spinlock_irqsave_try, lock)(&gscps2_interrupt_lock);
+	if (ACQUIRE_ERR(spinlock_irqsave_try, &lock))
+		return IRQ_NONE;
 
 	guard(rcu)();
 
-	list_for_each_entry_rcu(ps2port, &ps2port_list, node) {
-		if (gscps2_read_data(ps2port))
-			handled = true;
-	}
+	do {
+		more_data = false;
 
-	/* all data was read from the ports - now report the data to upper layer */
-	list_for_each_entry_rcu(ps2port, &ps2port_list, node) {
-		if (gscps2_report_data(ps2port)) {
-			/* More data ready - break early to restart interrupt */
-			break;
+		list_for_each_entry_rcu(ps2port, &ps2port_list, node) {
+			if (gscps2_read_data(ps2port))
+				handled = true;
 		}
-	}
+
+		/* all data was read from the ports - now report the data to upper layer */
+		list_for_each_entry_rcu(ps2port, &ps2port_list, node) {
+			if (gscps2_report_data(ps2port)) {
+				/* More data ready - restart loop to read new data */
+				more_data = true;
+				break;
+			}
+		}
+	} while (more_data);
 
 	return IRQ_RETVAL(handled);
 }

-- 
2.55.0.897.gb25b4bd76c-goog

[PATCH 7/7] Input: gscps2 - drop busy-wait and manual interrupt pump on transmit

[Dmitry Torokhov]

In gscps2_writeb_output(), after writing data to GSC_XMTDATA, the driver
explicitly executed mdelay(6) and manually called gscps2_interrupt() as a
polling mechanism to accelerate command responses (such as keyboard ACK
or LED updates).

On PA-RISC, the PS/2 controller asserts a level interrupt to the system
ASIC whenever received data arrives in hardware, and the input/serio
subsystem handles command responses asynchronously via completions.
Busy-waiting for 6 ms on every transmitted byte introduces significant
unnecessary latency during multi-byte command sequences and complicates
interrupt handler locking.

Remove mdelay(6) and the manual invocation of gscps2_interrupt() from
gscps2_writeb_output(), relying on normal hardware interrupt delivery.

Reported-by: sashiko-bot@kernel.org
Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/serio/gscps2.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/input/serio/gscps2.c b/drivers/input/serio/gscps2.c
index 2afd53a163ff..985539f1d088 100644
--- a/drivers/input/serio/gscps2.c
+++ b/drivers/input/serio/gscps2.c
@@ -356,13 +356,6 @@ static inline int gscps2_writeb_output(struct gscps2port *ps2port, u8 data)
 	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);
-
-	/* make sure any received data is returned as fast as possible */
-	/* this is important e.g. when we set the LEDs on the keyboard */
-	gscps2_interrupt(0, NULL);
-
 	return 1;
 }
 

-- 
2.55.0.897.gb25b4bd76c-goog