Hi Paul,
On 1/21/24 15:26, Paul Menzel wrote:
> Dear Hans,
>
>
> As always thank you very much for taking the time to reply.
>
>
> Am 20.01.24 um 21:26 schrieb Hans de Goede:
>
>> On 1/18/24 13:57, Paul Menzel wrote:
>>> #regzbot introduced v6.6.11..v6.7
>
>>> There seems to be a regression in Linux 6.7 on the Dell XPS 13 9360 (Intel i7-7500U).
>>>
>>> [ 0.000000] DMI: Dell Inc. XPS 13 9360/0596KF, BIOS 2.21.0 06/02/2022
>>>
>>> The PS/2 keyboard goes missing after S3 resume¹. The problem does not happen with Linux 6.6.11.
>>
>> Thank you for reporting this.
>>
>> Can you try adding "i8042.dumbkbd=1" to your kernel commandline?
>>
>> This should at least lead to the device not disappearing from
>>
>> "sudo libinput list-devices"
>>
>> The next question is if the keyboard will still actually
>> work after suspend/resume with "i8042.dumbkbd=1". If it
>> stays in the list, but no longer works then there is
>> a problem with the i8042 controller; or interrupt
>> delivery to the i8042 controller.
>>
>> If "i8042.dumbkbd=1" somehow fully fixes things, then I guess
>> my atkbd driver fix for other laptop keyboards is somehow
>> causing issues for yours.
>
> Just a quick feedback, that booting with `i8042.dumbkbd=1` seems to fix the issue.
>
>> If "i8042.dumbkbd=1" fully fixes things, can you try building
>> your own 6.7.0 kernel with commit 936e4d49ecbc:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=936e4d49ecbc8c404790504386e1422b599dec39
>>
>> reverted?
>
> I am going to try that as soon as possible.
Assuming this was not some one time glitch with 6.7.0,
I have prepared a patch hopefully fixing this (1) as well
as a follow up fix to address another potential issue which
I have noticed.
Can you please give a 6.7.0 (2) kernel with the 2 attached
patches added a try ?
I know building kernels can be a bit of work / takes time,
sorry. If you are short on time I would prefer testing these 2
patches and see if they fix things over trying a plain revert.
Regards,
Hans
1) Assuming it is caused by this commit in the first place,
which seems likely
2) 6.8-rc1 has a follow up patch which is squashed into the
first patch here, so these patches will only apply cleanly
to 6.7.0 .
>>> [ 1.435071] i8042: PNP: PS/2 Controller [PNP0303:PS2K,PNP0f13:PS2M] at 0x60,0x64 irq 1,12
>>> [ 1.435409] i8042: Warning: Keylock active
>>> [ 1.437624] serio: i8042 KBD port at 0x60,0x64 irq 1
>>> [ 1.437631] serio: i8042 AUX port at 0x60,0x64 irq 12
>>> […]
>>> [ 1.439743] input: AT Translated Set 2 keyboard as /devices/platform/i8042/serio0/input/input0
>>>
>>> $ sudo libinput list-devices
>>> […]
>>> Device: AT Translated Set 2 keyboard
>>> Kernel: /dev/input/event0
>>> Group: 15
>>> Seat: seat0, default
>>> Capabilities: keyboard
>>> Tap-to-click: n/a
>>> Tap-and-drag: n/a
>>> Tap drag lock: n/a
>>> Left-handed: n/a
>>> Nat.scrolling: n/a
>>> Middle emulation: n/a
>>> Calibration: n/a
>>> Scroll methods: none
>>> Click methods: none
>>> Disable-w-typing: n/a
>>> Disable-w-trackpointing: n/a
>>> Accel profiles: n/a
>>> Rotation: 0.0
>>>
>>> `libinput list-devices` does not list the device after resuming
>>> from S3. Some of the function keys, like brightness and airplane
>>> mode keys, still work, as the events are probably transmitted over
>>> the embedded controller or some other mechanism. An external USB
>>> keyboard also still works.
>>>
>>> I haven’t had time to further analyze this, but wanted to report
>>> it. No idea
>>>
>>>
>>> Kind regards,
>>>
>>> Paul
>>>
>>>
>>> ¹ s2idle is not working correctly on the device, in the sense, that
>>> energy usage is very high in that state, and the full battery is at
>>> 20 % after leaving it for eight hours.
>
From 067f89e34264617dd22b0cb850c7996c93152cd1 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@xxxxxxxxxx>
Date: Tue, 16 Jan 2024 21:43:25 +0100
Subject: [PATCH 1/2] Input: atkbd - Skip ATKBD_CMD_SETLEDS when skipping
ATKBD_CMD_GETID
After commit 936e4d49ecbc ("Input: atkbd - skip ATKBD_CMD_GETID in
translated mode") the keyboard on a Dell XPS 13 9360 has stopped
working after a suspend/resume.
The problem appears to be that atkbd_probe() fails when called
from atkbd_reconnect() on resume, which on systems where
ATKBD_CMD_GETID is skipped can only happen by ATKBD_CMD_SETLEDS
failing. ATKBD_CMD_SETLEDS failing because ATKBD_CMD_GETID was
skipped is weird, but apparently that is what is happening.
Fix this by also skipping ATKBD_CMD_SETLEDS when skipping
ATKBD_CMD_GETID.
Fixes: 936e4d49ecbc ("Input: atkbd - skip ATKBD_CMD_GETID in translated mode")
Reported-by: Paul Menzel <pmenzel@xxxxxxxxxxxxx>
Closes: https://lore.kernel.org/linux-input/0aa4a61f-c939-46fe-a572-08022e8931c7@xxxxxxxxxxxxx/
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
drivers/input/keyboard/atkbd.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 786f00f6b7fd..c229bd6b3f7f 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -791,9 +791,9 @@ static bool atkbd_is_portable_device(void)
* not work. So in this case simply assume a keyboard is connected to avoid
* confusing some laptop keyboards.
*
- * Skipping ATKBD_CMD_GETID ends up using a fake keyboard id. Using a fake id is
- * ok in translated mode, only atkbd_select_set() checks atkbd->id and in
- * translated mode that is a no-op.
+ * Skipping ATKBD_CMD_GETID ends up using a fake keyboard id. Using the standard
+ * 0xab83 id is ok in translated mode, only atkbd_select_set() checks atkbd->id
+ * and in translated mode that is a no-op.
*/
static bool atkbd_skip_getid(struct atkbd *atkbd)
{
@@ -824,6 +824,11 @@ static int atkbd_probe(struct atkbd *atkbd)
"keyboard reset failed on %s\n",
ps2dev->serio->phys);
+ if (atkbd_skip_getid(atkbd)) {
+ atkbd->id = 0xab83;
+ return 0;
+ }
+
/*
* Then we check the keyboard ID. We should get 0xab83 under normal conditions.
* Some keyboards report different values, but the first byte is always 0xab or
@@ -832,10 +837,10 @@ static int atkbd_probe(struct atkbd *atkbd)
*/
param[0] = param[1] = 0xa5; /* initialize with invalid values */
- if (atkbd_skip_getid(atkbd) || ps2_command(ps2dev, param, ATKBD_CMD_GETID)) {
+ if (ps2_command(ps2dev, param, ATKBD_CMD_GETID)) {
/*
- * If the get ID command was skipped or failed, we check if we can at least set
+ * If the get ID command failed, we check if we can at least set
* the LEDs on the keyboard. This should work on every keyboard out there.
* It also turns the LEDs off, which we want anyway.
*/
--
2.43.0
From bb6435baea52f66e7aaca43520bdb1f71a181a5e Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@xxxxxxxxxx>
Date: Mon, 22 Jan 2024 14:32:16 +0100
Subject: [PATCH 2/2] Input: atkbd - Do not skip atkbd_deactivate() when
skipping ATKBD_CMD_GETID
After commit 936e4d49ecbc ("Input: atkbd - skip ATKBD_CMD_GETID in
translated mode") not only the getid command is skipped, but also
the de-activating of the keyboard at the end of atkbd_probe(), potentially
re-introducing the problem fixed by commit be2d7e4233a4 ("Input: atkbd -
fix multi-byte scancode handling on reconnect").
Make sure multi-byte scancode handling on reconnect is still handled
correctly by not skipping the atkbd_deactivate() call.
Fixes: 936e4d49ecbc ("Input: atkbd - skip ATKBD_CMD_GETID in translated mode")
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
drivers/input/keyboard/atkbd.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index c229bd6b3f7f..7f67f9f2946b 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -826,7 +826,7 @@ static int atkbd_probe(struct atkbd *atkbd)
if (atkbd_skip_getid(atkbd)) {
atkbd->id = 0xab83;
- return 0;
+ goto deactivate_kbd;
}
/*
@@ -863,6 +863,7 @@ static int atkbd_probe(struct atkbd *atkbd)
return -1;
}
+deactivate_kbd:
/*
* Make sure nothing is coming from the keyboard and disturbs our
* internal state.
--
2.43.0
