[PATCH] Input: i8042 - Fix keyboard failure caused by S4 mouse wakeup

[PATCH] Input: i8042 - Fix keyboard failure caused by S4 mouse wakeup

[luyuantao01]

From: luyuantao <luyuantao@kylinos.cn>

There is an i8402 keyboard and mouse device on the
ThinkPad P15 laptop.When conducting a wakeup
test on S4, it was found that:

1. Using the keyboard directly can wake up S4.
2. The system failed to wake up using the mouse button first,
and when using the keyboard to wake up again, the system
cannot be woken up and can only be shut down by pressing
the power button.

This issue is that i8042_start() only enables wakeup for the
keyboard.However, because wakeup capability is set for all
devices, the device_may_wakeup() is also true for the mouse,
and the i8042_pm_suspend() enables the wakeup interrupt for the
mouse.The system can respond to mouse wakeup interrupts, but
it cannot handle them and affects keyboard interrupts.

So let the mouse also have wakeup ability to fix it

Signed-off-by: luyuantao <luyuantao@kylinos.cn>
---
 drivers/input/serio/i8042.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index 8ec4872b4471..1f9810dfb3d8 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -446,17 +446,16 @@ static int i8042_start(struct serio *serio)
 {
 	struct i8042_port *port = serio->port_data;
 
-	device_set_wakeup_capable(&serio->dev, true);
-
 	/*
-	 * On platforms using suspend-to-idle, allow the keyboard to
-	 * wake up the system from sleep by enabling keyboard wakeups
-	 * by default.  This is consistent with keyboard wakeup
-	 * behavior on many platforms using suspend-to-RAM (ACPI S3)
-	 * by default.
+	 * On platforms using suspend-to-idle
+	 * In fact, people nowadays prefer to wake up systems using
+	 * keyboards or mice.But after the previous code flow
+	 * entered S4, the mouse could not wakeup the system
+	 * and caused the keyboard to fail to wake up.
+	 * So fix it
 	 */
-	if (pm_suspend_default_s2idle() &&
-	    serio == i8042_ports[I8042_KBD_PORT_NO].serio) {
+	if (pm_suspend_default_s2idle()) {
+		device_set_wakeup_capable(&serio->dev, true);
 		device_set_wakeup_enable(&serio->dev, true);
 	}
 
-- 
2.25.1

[PATCH v2] Input: i8042 - Fix keyboard failure caused by S3 mouse wakeup

[luyuantao01]

From: luyuantao <luyuantao@kylinos.cn>

Hi Dmitry 
I'm sorry for the inconvenience caused to you. After reproducing the 
problem and conducting a thorough analysis, I found that the previous 
patch description was incorrect. Therefore, resubmit the patch

There is an i8402 keyboard and mouse device on the
ThinkPad P15 laptop.When conducting a wakeup
test on S3, it was found that:

1. Using the keyboard directly can wake up S3.
2. The system failed to wake up using the mouse button first,
and when using the keyboard to wake up again, the system
cannot be woken up and can only be shut down by pressing
the power button.

This issue is that i8042_start() only enables wakeup for the
keyboard. During the i8042_pm_suspend() phase, the aux device
will not enable irq wakeup. However, when suspend_device_irqs()
traversing irq without wakeup capability, __disable_irq() did
not truly disable aux interrupts, only setting the IRQS_SUSPEND
flag, resulting in aux interrupts still being generated.

When an interrupt is triggered, irqd_irq_isabled returns the
true execution mask irq. The mask_iopic_irq callback function
of the IR-IO-APIC chip will disable all IRQ pins, resulting
in keyboard interrupts being disabled and no longer responding

Signed-off-by: luyuantao <luyuantao@kylinos.cn>
---
 drivers/input/serio/i8042.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index 8ec4872b4471..abcd01807dc8 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -446,17 +446,15 @@ static int i8042_start(struct serio *serio)
 {
 	struct i8042_port *port = serio->port_data;
 
-	device_set_wakeup_capable(&serio->dev, true);
-
 	/*
-	 * On platforms using suspend-to-idle, allow the keyboard to
-	 * wake up the system from sleep by enabling keyboard wakeups
-	 * by default.  This is consistent with keyboard wakeup
-	 * behavior on many platforms using suspend-to-RAM (ACPI S3)
-	 * by default.
+	 * On platforms using suspend-to-idle
+	 * In fact, people nowadays prefer to wake up systems using
+	 * keyboards or mice. But after the previous code flow
+	 * entered S3, the mouse could not wakeup the system
+	 * and caused the keyboard to fail to wake up.
 	 */
-	if (pm_suspend_default_s2idle() &&
-	    serio == i8042_ports[I8042_KBD_PORT_NO].serio) {
+	if (pm_suspend_default_s2idle()) {
+		device_set_wakeup_capable(&serio->dev, true);
 		device_set_wakeup_enable(&serio->dev, true);
 	}
 
-- 
2.27.0

Re: [PATCH v2] Input: i8042 - Fix keyboard failure caused by S3 mouse wakeup

[Dmitry Torokhov]

Hi luyuantao,

On Thu, Nov 14, 2024 at 10:13:10AM +0800, luyuantao01 wrote:
> From: luyuantao <luyuantao@kylinos.cn>
> 
> Hi Dmitry 
> I'm sorry for the inconvenience caused to you. After reproducing the 
> problem and conducting a thorough analysis, I found that the previous 
> patch description was incorrect. Therefore, resubmit the patch
> 
> There is an i8402 keyboard and mouse device on the
> ThinkPad P15 laptop.When conducting a wakeup
> test on S3, it was found that:
> 
> 1. Using the keyboard directly can wake up S3.
> 2. The system failed to wake up using the mouse button first,
> and when using the keyboard to wake up again, the system
> cannot be woken up and can only be shut down by pressing
> the power button.
> 
> This issue is that i8042_start() only enables wakeup for the
> keyboard. During the i8042_pm_suspend() phase, the aux device
> will not enable irq wakeup. However, when suspend_device_irqs()
> traversing irq without wakeup capability, __disable_irq() did
> not truly disable aux interrupts, only setting the IRQS_SUSPEND
> flag, resulting in aux interrupts still being generated.
> 
> When an interrupt is triggered, irqd_irq_isabled returns the
> true execution mask irq. The mask_iopic_irq callback function
> of the IR-IO-APIC chip will disable all IRQ pins, resulting
> in keyboard interrupts being disabled and no longer responding

So this sounds like a bug in the irqchip implementation that is does not
properly handle interrupts that are wakeup capable but not enabled for
the interrupt because of policy.

The i8042 driver correctly marks both KBD and AUX interrupts as capable
of waking up the system but only enables KBD as a wakeup source for
suspend-to-idle case. If a different policy is desired on a system it
can be adjusted form userspace vis sysfs.

Thanks.

-- 
Dmitry