Linux Input/HID development
 help / color / mirror / Atom feed
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: "James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>,
	 Helge Deller <deller@gmx.de>
Cc: linux-kernel@vger.kernel.org, linux-input@vger.kernel.org,
	 linux-parisc@vger.kernel.org
Subject: [PATCH 1/7] Input: gscps2 - clean up driver code style and structure
Date: Sun, 30 Aug 2026 13:52:46 -0700	[thread overview]
Message-ID: <20260830-gscps2-v1-1-c733d4cae7f9@gmail.com> (raw)
In-Reply-To: <20260830-gscps2-v1-0-c733d4cae7f9@gmail.com>

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


  reply	other threads:[~2026-08-30 20:52 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-30 20:52 [PATCH 0/7] Input: gscps2: cleanups and locking fixes Dmitry Torokhov
2026-08-30 20:52 ` Dmitry Torokhov [this message]
2026-08-30 21:04   ` [PATCH 1/7] Input: gscps2 - clean up driver code style and structure sashiko-bot
2026-08-30 20:52 ` [PATCH 2/7] Input: gscps2 - use RCU for ps2port_list and manage it in open/close Dmitry Torokhov
2026-08-30 20:52 ` [PATCH 3/7] Input: gscps2 - protect buffer access in read and report helpers Dmitry Torokhov
2026-08-30 21:05   ` sashiko-bot
2026-08-30 20:52 ` [PATCH 4/7] Input: gscps2 - serialize hardware and buffer access in gscps2_flush() Dmitry Torokhov
2026-08-30 21:02   ` sashiko-bot
2026-08-30 20:52 ` [PATCH 5/7] Input: gscps2 - return IRQ_NONE when interrupt is not handled Dmitry Torokhov
2026-08-30 21:05   ` sashiko-bot
2026-08-30 20:52 ` [PATCH 6/7] Input: gscps2 - serialize concurrent interrupt handlers Dmitry Torokhov
2026-08-30 21:07   ` sashiko-bot
2026-08-30 20:52 ` [PATCH 7/7] Input: gscps2 - drop busy-wait and manual interrupt pump on transmit Dmitry Torokhov
2026-08-30 21:05   ` sashiko-bot

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=20260830-gscps2-v1-1-c733d4cae7f9@gmail.com \
    --to=dmitry.torokhov@gmail.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=deller@gmx.de \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-parisc@vger.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