Thread (10 messages) 10 messages, 3 authors, 2024-01-16

Re: [PATCH v2] Input: atkbd - Skip ATKBD_CMD_GETID in translated mode

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2023-11-25 04:30:25
Subsystem: input (keyboard, mouse, joystick, touchscreen) drivers, the rest · Maintainers: Dmitry Torokhov, Linus Torvalds

Hi Hans,

On Wed, Nov 15, 2023 at 06:46:25PM +0100, Hans de Goede wrote:
quoted hunk ↗ jump to hunk
There have been multiple reports of keyboard issues on recent laptop models
which can be worked around by setting i8042.dumbkbd, with the downside
being this breaks the capslock LED.

It seems that these issues are caused by recent laptops getting confused by
ATKBD_CMD_GETID. Rather then adding and endless growing list of quirks for
this, just skip ATKBD_CMD_GETID alltogether on laptops in translated mode.

The main goal of sending ATKBD_CMD_GETID is to skip binding to ps/2
mice/touchpads and those are never used in translated mode.

Examples of laptop models which benefit from skipping ATKBD_CMD_GETID:

* "HP Laptop 15s-fq2xxx", "HP laptop 15s-fq4xxx" and "HP Laptop 15-dy2xxx"
  models the kbd stops working for the first 2 - 5 minutes after boot
  (waiting for EC watchdog reset?)

* On "HP Spectre x360 13-aw2xxx" atkbd fails to probe the keyboard

* At least 9 different Lenovo models have issues with ATKBD_CMD_GETID, see:
  https://github.com/yescallop/atkbd-nogetid

This has been tested on:

1. A MSI B550M PRO-VDH WIFI desktop, where the i8042 controller is not
   in translated mode when no keyboard is plugged in and with a ps/2 kbd
   a "AT Translated Set 2 keyboard" /dev/input/event# node shows up

2. A Lenovo ThinkPad X1 Yoga gen 8 (always has a translated set 2 keyboard)

Reported-by: Shang Ye <redacted>
Closes: https://lore.kernel.org/linux-input/886D6167733841AE+20231017135318.11142-1-yesh25@mail2.sysu.edu.cn/ (local)
Closes: https://github.com/yescallop/atkbd-nogetid
Reported-by: gurevitch <redacted>
Closes: https://lore.kernel.org/linux-input/2iAJTwqZV6lQs26cTb38RNYqxvsink6SRmrZ5h0cBUSuf9NT0tZTsf9fEAbbto2maavHJEOP8GA1evlKa6xjKOsaskDhtJWxjcnrgPigzVo=@gurevit.ch/ (local)
Reported-by: Egor Ignatov <redacted>
Closes: https://lore.kernel.org/all/20210609073333.8425-1-egori@altlinux.org/ (local)
Reported-by: Anton Zhilyaev <redacted>
Closes: https://lore.kernel.org/linux-input/20210201160336.16008-1-anton@cpp.in/ (local)
Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2086156
Signed-off-by: Hans de Goede <redacted>
---
Note this supersedes my previous atkbd series:
https://lore.kernel.org/linux-input/20231005201544.26983-1-hdegoede@redhat.com/ (local)
---
Changes in v2:
- Add DMI check for laptop chassis types and only skip ATKBD_CMD_GETID
  on laptops with the i8042 in translated mode
---
 drivers/input/keyboard/atkbd.c | 61 +++++++++++++++++++++++++++++++---
 1 file changed, 57 insertions(+), 4 deletions(-)
diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index c92e544c792d..5667f1e80839 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -765,6 +765,59 @@ static void atkbd_deactivate(struct atkbd *atkbd)
 			ps2dev->serio->phys);
 }
 
+#ifdef CONFIG_X86
+static const struct dmi_system_id atkbd_dmi_laptop_table[] = {
+	{
+		.matches = {
+			DMI_EXACT_MATCH(DMI_CHASSIS_TYPE, "8"), /* Portable */
+		},
+	},
+	{
+		.matches = {
+			DMI_EXACT_MATCH(DMI_CHASSIS_TYPE, "9"), /* Laptop */
+		},
+	},
+	{
+		.matches = {
+			DMI_EXACT_MATCH(DMI_CHASSIS_TYPE, "10"), /* Notebook */
+		},
+	},
+	{
+		.matches = {
+			DMI_EXACT_MATCH(DMI_CHASSIS_TYPE, "14"), /* Sub-Notebook */
+		},
+	},
+	{
+		.matches = {
+			DMI_EXACT_MATCH(DMI_CHASSIS_TYPE, "31"), /* Convertible */
+		},
+	},
+	{
+		.matches = {
+			DMI_EXACT_MATCH(DMI_CHASSIS_TYPE, "32"), /* Detachable */
+		},
+	},
+	{ }
+};
Thank you for making the changes, however I wonder if we should
open-code check for the chassis type, as DMI can be quite bloated and
here we are dealing with exactly one field. Something like this:
diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 5667f1e80839..786f00f6b7fd 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -766,39 +766,24 @@ static void atkbd_deactivate(struct atkbd *atkbd)
 }
 
 #ifdef CONFIG_X86
-static const struct dmi_system_id atkbd_dmi_laptop_table[] = {
-	{
-		.matches = {
-			DMI_EXACT_MATCH(DMI_CHASSIS_TYPE, "8"), /* Portable */
-		},
-	},
-	{
-		.matches = {
-			DMI_EXACT_MATCH(DMI_CHASSIS_TYPE, "9"), /* Laptop */
-		},
-	},
-	{
-		.matches = {
-			DMI_EXACT_MATCH(DMI_CHASSIS_TYPE, "10"), /* Notebook */
-		},
-	},
-	{
-		.matches = {
-			DMI_EXACT_MATCH(DMI_CHASSIS_TYPE, "14"), /* Sub-Notebook */
-		},
-	},
-	{
-		.matches = {
-			DMI_EXACT_MATCH(DMI_CHASSIS_TYPE, "31"), /* Convertible */
-		},
-	},
-	{
-		.matches = {
-			DMI_EXACT_MATCH(DMI_CHASSIS_TYPE, "32"), /* Detachable */
-		},
-	},
-	{ }
-};
+static bool atkbd_is_portable_device(void)
+{
+	static const char * const chassis_types[] = {
+		"8",	/* Portable */
+		"9",	/* Laptop */
+		"10",	/* Notebook */
+		"14",	/* Sub-Notebook */
+		"31",	/* Convertible */
+		"32",	/* Detachable */
+	};
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(chassis_types); i++)
+		if (dmi_match(DMI_CHASSIS_TYPE, chassis_types[i]))
+			return true;
+
+	return false;
+}
 
 /*
  * On many modern laptops ATKBD_CMD_GETID may cause problems, on these laptops
@@ -812,7 +797,7 @@ static const struct dmi_system_id atkbd_dmi_laptop_table[] = {
  */
 static bool atkbd_skip_getid(struct atkbd *atkbd)
 {
-	return atkbd->translated && dmi_check_system(atkbd_dmi_laptop_table);
+	return atkbd->translated && atkbd_is_portable_device();
 }
 #else
 static inline bool atkbd_skip_getid(struct atkbd *atkbd) { return false; }

It gives me slightly smaller output:

dtor@dtor-ws:~/kernel/work (master *)$ size drivers/input/keyboard/atkbd.o.old
   text    data     bss     dec     hex filename
  28407    1512      37   29956    7504 drivers/input/keyboard/atkbd.o.old
dtor@dtor-ws:~/kernel/work (master *)$ size drivers/input/keyboard/atkbd.o
   text    data     bss     dec     hex filename
  26107    1512      37   27656    6c08 drivers/input/keyboard/atkbd.o

Please let me know if this works for you and I can combine on my end.

Thanks.

-- 
Dmitry
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help