Input: gscps2 - supply PA-RISC keyboard keymap via device property

PATCH New Dmitry Torokhov 2 participants 2 messages Jul 27

Subsystems (3)

HID Input drivers PA-RISC architecture

lore

Share & tools

lore

Dmitry Torokhov

author maintainer

2026-07-27 00:38 UTC #0 lore raw

PATCH 1/1 Input: gscps2 - supply PA-RISC keyboard keymap via device property

Instead of hardcoding PA-RISC specific keycode tables into atkbd via compile-time inclusion, have the gscps2 PS/2 port driver attach a "linux,keymap" software node device property to the serio device when a keyboard port is registered. This allows atkbd to dynamically fetch and apply the custom keymap when probing the port using generic firmware property helpers, removing architecture-specific hacks from generic keyboard driver code.

Co-locate the keymap definitions with the serio port driver by moving hpps2atkbd.h from drivers/input/keyboard/ to drivers/input/serio/.

Assisted-by Antigravity:gemini-3.5-flash

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/keyboard/Kconfig      |  38 ----------
 drivers/input/keyboard/atkbd.c      |   8 --
 drivers/input/keyboard/hpps2atkbd.h | 110 ----------------------------
 drivers/input/serio/Kconfig         |  27 +++++++
 drivers/input/serio/gscps2.c        |  27 +++++--
 drivers/input/serio/hpps2atkbd.h    | 108 +++++++++++++++++++++++++++
 6 files changed, 154 insertions(+), 164 deletions(-)

drivers/input/keyboard/Kconfig +0 ~~-38~~

index 9d1019ba0245..d8c64f333462 100644
@@ -119,44 +119,6 @@ config KEYBOARD_ATKBD
 	  To compile this driver as a module, choose M here: the
 	  module will be called atkbd.
 
-config KEYBOARD_ATKBD_HP_KEYCODES
-	bool "Use HP keyboard scancodes"
-	depends on PARISC && KEYBOARD_ATKBD
-	default y
-	help
-	  Say Y here if you have a PA-RISC machine and want to use an AT or
-	  PS/2 keyboard, and your keyboard uses keycodes that are specific to
-	  PA-RISC keyboards.
-
-	  Say N if you use a standard keyboard.
-
-config KEYBOARD_ATKBD_RDI_KEYCODES
-	bool "Use PrecisionBook keyboard scancodes"
-	depends on KEYBOARD_ATKBD_HP_KEYCODES
-	default n
-	help
-	  If you have an RDI PrecisionBook, say Y here if you want to use its
-	  built-in keyboard (as opposed to an external keyboard).
-
-	  The PrecisionBook has five keys that conflict with those used by most
-	  AT and PS/2 keyboards. These are as follows:
-
-	    PrecisionBook    Standard AT or PS/2
-
-	    F1               F12
-	    Left Ctrl        Left Alt
-	    Caps Lock        Left Ctrl
-	    Right Ctrl       Caps Lock
-	    Left             102nd key (the key to the right of Left Shift)
-
-	  If you say N here, and use the PrecisionBook keyboard, then each key
-	  in the left-hand column will be interpreted as the corresponding key
-	  in the right-hand column.
-
-	  If you say Y here, and use an external keyboard, then each key in the
-	  right-hand column will be interpreted as the key shown in the
-	  left-hand column.
-
 config KEYBOARD_QT1050
 	tristate "Microchip AT42QT1050 Touch Sensor Chip"
 	depends on I2C

drivers/input/keyboard/atkbd.c +0 -8

index 8cb4dc6fb165..3509b58e6492 100644
@@ -73,13 +73,6 @@ MODULE_PARM_DESC(terminal, "Enable break codes on an IBM Terminal keyboard conne
 #define ATKBD_KEYMAP_SIZE	512
 
 static const unsigned short atkbd_set2_keycode[ATKBD_KEYMAP_SIZE] = {
-#ifdef CONFIG_KEYBOARD_ATKBD_HP_KEYCODES
-
-/* XXX: need a more general approach */
-
-#include "hpps2atkbd.h"	/* include the keyboard scancodes */
-
-#else
 	  0, 67, 65, 63, 61, 59, 60, 88,183, 68, 66, 64, 62, 15, 41,117,
 	184, 56, 42, 93, 29, 16,  2,  0,185,  0, 44, 31, 30, 17,  3,  0,
 	186, 46, 45, 32, 18,  5,  4, 95,187, 57, 47, 33, 20, 19,  6,183,

@@ -99,7 +92,6 @@ static const unsigned short atkbd_set2_keycode[ATKBD_KEYMAP_SIZE] = {
 	110,111,108,112,106,103,  0,119,  0,118,109,  0, 99,104,119,  0,
 
 	  0,  0,  0, 65, 99,
-#endif
 };
 
 static const unsigned short atkbd_set3_keycode[ATKBD_KEYMAP_SIZE] = {

drivers/input/keyboard/hpps2atkbd.h deleted +0 ~~-110~~

110 lines not on this page · raw message

drivers/input/serio/Kconfig +27 -0

index bacab1f58400..087f0203fff9 100644
@@ -106,6 +106,33 @@ config SERIO_GSCPS2
 	  To compile this driver as a module, choose M here: the
 	  module will be called gscps2.
 
+config SERIO_GSCPS2_RDI_KEYCODES
+	bool "Use PrecisionBook keyboard scancodes"
+	depends on SERIO_GSCPS2
+	default n
+	help
+	  If you have an RDI PrecisionBook, say Y here if you want to use its
+	  built-in keyboard (as opposed to an external keyboard).
+
+	  The PrecisionBook has five keys that conflict with those used by most
+	  AT and PS/2 keyboards. These are as follows:
+
+	    PrecisionBook    Standard AT or PS/2
+
+	    F1               F12
+	    Left Ctrl        Left Alt
+	    Caps Lock        Left Ctrl
+	    Right Ctrl       Caps Lock
+	    Left             102nd key (the key to the right of Left Shift)
+
+	  If you say N here, and use the PrecisionBook keyboard, then each key
+	  in the left-hand column will be interpreted as the corresponding key
+	  in the right-hand column.
+
+	  If you say Y here, and use an external keyboard, then each key in the
+	  right-hand column will be interpreted as the key shown in the
+	  left-hand column.
+
 config HP_SDC
 	tristate "HP System Device Controller i8042 Support"
 	depends on (GSC || HP300) && SERIO

drivers/input/serio/gscps2.c +19 -8

index bf9b993f5733..f4a4afde35c9 100644
@@ -22,20 +22,19 @@
  *                 was usable/enabled ?)
  */
 
-#include <linux/init.h>
-#include <linux/module.h>
-#include <linux/slab.h>
-#include <linux/serio.h>
+#include <linux/delay.h>
 #include <linux/input.h>
 #include <linux/interrupt.h>
-#include <linux/spinlock.h>
-#include <linux/delay.h>
+#include <linux/io.h>
 #include <linux/ioport.h>
+#include <linux/property.h>
+#include <linux/serio.h>
 
 #include <asm/irq.h>
-#include <asm/io.h>
 #include <asm/parisc-device.h>
 
+#include "hpps2atkbd.h"
+
 MODULE_AUTHOR("Laurent Canet <[email protected]>, Thibaut Varene <[email protected]>, Helge Deller <[email protected]>");
 MODULE_DESCRIPTION("HP GSC PS2 port driver");
 MODULE_LICENSE("GPL");

@@ -398,6 +397,17 @@ static int __init gscps2_probe(struct parisc_device *dev)
 		goto fail;
 #endif
 
+	if (ps2port->id == GSC_ID_KEYBOARD) {
+		ret = device_create_managed_software_node(&serio->dev,
+							  gscps2_props, NULL);
+		if (ret) {
+			dev_err(&dev->dev,
+				"failed to add software node for keyboard: %d\n",
+				ret);
+			goto fail;
+		}
+	}
+
 	pr_info("serio: %s port at 0x%08lx irq %d @ %s\n",
 		ps2port->port->name,
 		hpa,

@@ -415,7 +425,9 @@ static int __init gscps2_probe(struct parisc_device *dev)
 
 fail_miserably:
 	iounmap(ps2port->addr);
+#if 0
 	release_mem_region(dev->hpa.start, GSC_STATUS + 4);
+#endif
 
 fail_nomem:
 	kfree(ps2port);

@@ -477,4 +489,3 @@ static void __exit gscps2_exit(void)
 
 module_init(gscps2_init);
 module_exit(gscps2_exit);
-

drivers/input/serio/hpps2atkbd.h new file +108 -0

108 lines not on this page · raw message

Signature · 5 lines

2.55.0.229.g6434b31f56-goog


-- 
Dmitry

[email protected]

AI review

2026-07-27 00:55 UTC #1 lore raw

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [Medium] Missing keycode mapping for PA-RISC keyboard scancode 0x92 leads to spurious dmesg warnings.
- [Medium] The managed software node allocated for the keyboard serio device leaks if port registration is canceled before execution.

Pre-existing issues:
- [High] Lockless modifications to ps2port_list race with lockless traversal in the shared hardirq handler, leading to potential list corruption and panics.
- [High] gscps2_remove() frees the serio port before unregistering the interrupt handler, creating a use-after-free vulnerability.
- [High] The interrupt handler unconditionally returns IRQ_HANDLED for a shared IRQ line, defeating spurious interrupt detection. --

--- Patch [1]: [PATCH] Input: gscps2 - supply PA-RISC keyboard keymap via device property --- commit 66edd58351136502cfbfeb7361cae9d5adeebf48
Author: Dmitry Torokhov <[email protected]>

Input: gscps2 - supply PA-RISC keyboard keymap via device property

This patch refactors the PA-RISC keyboard driver to supply the keymap via a software node device property instead of hardcoding it in the generic atkbd driver. It moves hpps2atkbd.h to the serio subsystem and attaches the property during port registration.

drivers/input/serio/gscps2.c +0 -0

[ ... ]

Review static int __init gscps2_probe(struct parisc_device *dev) · line 402

Quoted 7 lines

(quoted diff) +3 -0

@@ -398,6 +397,17 @@ static int __init gscps2_probe(struct parisc_device *dev)
 		goto fail;
 #endif
 
+	if (ps2port->id == GSC_ID_KEYBOARD) {
+		ret = device_create_managed_software_node(&serio->dev,
+							  gscps2_props, NULL);

[Severity: Medium]
Does this leak the software node if driver unbinding races with the asynchronous port registration?

serio_register_port() queues port registration asynchronously on a workqueue. If the driver is unbound and calls serio_unregister_port() before the workqueue executes device_add(), device_is_registered() evaluates to false in serio_destroy_port(), bypassing device_del(). Because device_del() is skipped, the software node is never removed.

Quoted 11 lines

(quoted diff) +8 -0

+		if (ret) {
+			dev_err(&dev->dev,
+				"failed to add software node for keyboard: %d\n",
+				ret);
+			goto fail;
+		}
+	}
+
 	pr_info("serio: %s port at 0x%08lx irq %d @ %s\n",
 		ps2port->port->name,
 		hpa,

Review

Quoted 4 lines

(quoted diff) +0 -0

 		ps2port->padev->irq,
 		ps2port->port->phys);
 
 	serio_register_port(ps2port->port);

[Severity: High]
This is a pre-existing issue, but does gscps2_remove() create a use-after-free window?

In the driver removal path, serio_unregister_port() is called before free_irq() and before the port is removed from ps2port_list. If a shared interrupt fires in this window, gscps2_interrupt() will execute, traverse the list, and dereference the freed ps2port->port pointer.

Quoted 1 line

list_add_tail(&ps2port->node, &ps2port_list);

[Severity: High]
This is a pre-existing issue, but can this race with the interrupt handler and corrupt the list?

The global ps2port_list is modified here locklessly. Concurrently, the shared interrupt handler gscps2_interrupt() iterates over this list using list_for_each_entry() without any locks. Since the interrupt is registered as IRQF_SHARED earlier in this function, an interrupt can trigger on another CPU while the list is being modified.

[Severity: High]
This is also a pre-existing issue, but does gscps2_interrupt() defeat spurious interrupt detection?

It appears the interrupt handler unconditionally returns IRQ_HANDLED regardless of whether it actually serviced any pending data. Because this IRQ is requested with IRQF_SHARED, this prevents the generic IRQ subsystem from detecting unhandled states and disabling the IRQ line if another device sharing this line asserts an interrupt but its driver fails to clear it.

drivers/input/serio/hpps2atkbd.h new file +0 -0

new file mode 100644

[ ... ]

Review

Quoted 6 lines

(quoted diff) +6 -0

+	KEYMAP_ENTRY(0x79, KEY_KPPLUS),		KEYMAP_ENTRY(0x7a, KEY_KP3),
+	KEYMAP_ENTRY(0x7b, KEY_KPMINUS),	KEYMAP_ENTRY(0x7c, KEY_KPASTERISK),
+	KEYMAP_ENTRY(0x7d, KEY_KP9),		KEYMAP_ENTRY(0x7e, KEY_SCROLLLOCK),
+	KEYMAP_ENTRY(0x7f, KEY_102ND),		KEYMAP_ENTRY(0x91, KEY_RIGHTALT),
+	KEYMAP_ENTRY(0x94, KEY_RIGHTCTRL),	KEYMAP_ENTRY(0x9d, KEY_CAPSLOCK),
+	KEYMAP_ENTRY(0x9f, KEY_LEFTMETA),	KEYMAP_ENTRY(0xa7, KEY_RIGHTMETA),

[Severity: Medium]
Is the mapping for scancode 0x92 intentionally omitted here?

The previous PA-RISC keycode table explicitly mapped scancode 0x92 to 255 (ATKBD_KEY_NULL), which atkbd ignores silently. With the new gscps2_keymap property array, scancode 0x92 defaults to 0 (ATKBD_KEY_UNKNOWN).

If the physical keyboard emits this scancode, atkbd will process it and flood the kernel logs with "Unknown key %s ..." warnings.

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1