diff mbox series

[v4] i8042: enable keyboard wakeups by default when s2idle is used

Message ID 20180914083235.6332-1-drake@endlessm.com (mailing list archive)
State Accepted
Headers show
Series [v4] i8042: enable keyboard wakeups by default when s2idle is used | expand

Commit Message

Daniel Drake Sept. 14, 2018, 8:32 a.m. UTC
Previously, on typical consumer laptops, pressing a key on the keyboard
when the system is in suspend would cause it to wake up (default or
unconditional behaviour). This happens because the EC generates a SCI
interrupt in this scenario.

That is no longer true on modern laptops based on Intel WhiskeyLake,
including Acer Swift SF314-55G, Asus UX333FA, Asus UX433FN and Asus
UX533FD. We confirmed with Asus EC engineers that the "Modern Standby"
design has been modified so that the EC no longer generates a SCI
in this case; the keyboard controller itself should be used for wakeup.

In order to retain the standard behaviour of being able to use the
keyboard to wake up the system, enable serio wakeups by default on
platforms that are using s2idle.

Link: https://lkml.kernel.org/r/CAB4CAwfQ0mPMqCLp95TVjw4J0r5zKPWkSvvkK4cpZUGE--w8bQ@mail.gmail.com
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Daniel Drake <drake@endlessm.com>
---

Notes:
    v2: tweak comment as suggested by Rafael
    
    v3: fix checkpatch line length warning
    
    v4: add and use stub function pm_suspend_via_s2idle().
        Fixes !CONFIG_SUSPEND case

 drivers/input/serio/i8042.c | 29 ++++++++++++++++++++---------
 include/linux/suspend.h     |  6 ++++++
 2 files changed, 26 insertions(+), 9 deletions(-)

Comments

Dmitry Torokhov Sept. 17, 2018, 10:27 p.m. UTC | #1
On Fri, Sep 14, 2018 at 04:32:35PM +0800, Daniel Drake wrote:
> Previously, on typical consumer laptops, pressing a key on the keyboard
> when the system is in suspend would cause it to wake up (default or
> unconditional behaviour). This happens because the EC generates a SCI
> interrupt in this scenario.
> 
> That is no longer true on modern laptops based on Intel WhiskeyLake,
> including Acer Swift SF314-55G, Asus UX333FA, Asus UX433FN and Asus
> UX533FD. We confirmed with Asus EC engineers that the "Modern Standby"
> design has been modified so that the EC no longer generates a SCI
> in this case; the keyboard controller itself should be used for wakeup.
> 
> In order to retain the standard behaviour of being able to use the
> keyboard to wake up the system, enable serio wakeups by default on
> platforms that are using s2idle.
> 
> Link: https://lkml.kernel.org/r/CAB4CAwfQ0mPMqCLp95TVjw4J0r5zKPWkSvvkK4cpZUGE--w8bQ@mail.gmail.com
> Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Signed-off-by: Daniel Drake <drake@endlessm.com>

Applied, thank you.

> ---
> 
> Notes:
>     v2: tweak comment as suggested by Rafael
>     
>     v3: fix checkpatch line length warning
>     
>     v4: add and use stub function pm_suspend_via_s2idle().
>         Fixes !CONFIG_SUSPEND case
> 
>  drivers/input/serio/i8042.c | 29 ++++++++++++++++++++---------
>  include/linux/suspend.h     |  6 ++++++
>  2 files changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
> index b8bc71569349..95a78ccbd847 100644
> --- a/drivers/input/serio/i8042.c
> +++ b/drivers/input/serio/i8042.c
> @@ -1395,15 +1395,26 @@ static void __init i8042_register_ports(void)
>  	for (i = 0; i < I8042_NUM_PORTS; i++) {
>  		struct serio *serio = i8042_ports[i].serio;
>  
> -		if (serio) {
> -			printk(KERN_INFO "serio: %s at %#lx,%#lx irq %d\n",
> -				serio->name,
> -				(unsigned long) I8042_DATA_REG,
> -				(unsigned long) I8042_COMMAND_REG,
> -				i8042_ports[i].irq);
> -			serio_register_port(serio);
> -			device_set_wakeup_capable(&serio->dev, true);
> -		}
> +		if (!serio)
> +			continue;
> +
> +		printk(KERN_INFO "serio: %s at %#lx,%#lx irq %d\n",
> +			serio->name,
> +			(unsigned long) I8042_DATA_REG,
> +			(unsigned long) I8042_COMMAND_REG,
> +			i8042_ports[i].irq);
> +		serio_register_port(serio);
> +		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.
> +		 */
> +		if (pm_suspend_via_s2idle() && i == I8042_KBD_PORT_NO)
> +			device_set_wakeup_enable(&serio->dev, true);
>  	}
>  }
>  
> diff --git a/include/linux/suspend.h b/include/linux/suspend.h
> index 5a28ac9284f0..82eb8e6e24de 100644
> --- a/include/linux/suspend.h
> +++ b/include/linux/suspend.h
> @@ -237,6 +237,11 @@ static inline bool pm_resume_via_firmware(void)
>  	return !!(pm_suspend_global_flags & PM_SUSPEND_FLAG_FW_RESUME);
>  }
>  
> +static inline bool pm_suspend_via_s2idle(void)
> +{
> +	return mem_sleep_current == PM_SUSPEND_TO_IDLE;
> +}
> +
>  /* Suspend-to-idle state machnine. */
>  enum s2idle_states {
>  	S2IDLE_STATE_NONE,      /* Not suspended/suspending. */
> @@ -282,6 +287,7 @@ static inline void pm_set_suspend_via_firmware(void) {}
>  static inline void pm_set_resume_via_firmware(void) {}
>  static inline bool pm_suspend_via_firmware(void) { return false; }
>  static inline bool pm_resume_via_firmware(void) { return false; }
> +static inline bool pm_suspend_via_s2idle(void) { return false; }
>  
>  static inline void suspend_set_ops(const struct platform_suspend_ops *ops) {}
>  static inline int pm_suspend(suspend_state_t state) { return -ENOSYS; }
> -- 
> 2.17.1
>
diff mbox series

Patch

diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index b8bc71569349..95a78ccbd847 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -1395,15 +1395,26 @@  static void __init i8042_register_ports(void)
 	for (i = 0; i < I8042_NUM_PORTS; i++) {
 		struct serio *serio = i8042_ports[i].serio;
 
-		if (serio) {
-			printk(KERN_INFO "serio: %s at %#lx,%#lx irq %d\n",
-				serio->name,
-				(unsigned long) I8042_DATA_REG,
-				(unsigned long) I8042_COMMAND_REG,
-				i8042_ports[i].irq);
-			serio_register_port(serio);
-			device_set_wakeup_capable(&serio->dev, true);
-		}
+		if (!serio)
+			continue;
+
+		printk(KERN_INFO "serio: %s at %#lx,%#lx irq %d\n",
+			serio->name,
+			(unsigned long) I8042_DATA_REG,
+			(unsigned long) I8042_COMMAND_REG,
+			i8042_ports[i].irq);
+		serio_register_port(serio);
+		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.
+		 */
+		if (pm_suspend_via_s2idle() && i == I8042_KBD_PORT_NO)
+			device_set_wakeup_enable(&serio->dev, true);
 	}
 }
 
diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 5a28ac9284f0..82eb8e6e24de 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -237,6 +237,11 @@  static inline bool pm_resume_via_firmware(void)
 	return !!(pm_suspend_global_flags & PM_SUSPEND_FLAG_FW_RESUME);
 }
 
+static inline bool pm_suspend_via_s2idle(void)
+{
+	return mem_sleep_current == PM_SUSPEND_TO_IDLE;
+}
+
 /* Suspend-to-idle state machnine. */
 enum s2idle_states {
 	S2IDLE_STATE_NONE,      /* Not suspended/suspending. */
@@ -282,6 +287,7 @@  static inline void pm_set_suspend_via_firmware(void) {}
 static inline void pm_set_resume_via_firmware(void) {}
 static inline bool pm_suspend_via_firmware(void) { return false; }
 static inline bool pm_resume_via_firmware(void) { return false; }
+static inline bool pm_suspend_via_s2idle(void) { return false; }
 
 static inline void suspend_set_ops(const struct platform_suspend_ops *ops) {}
 static inline int pm_suspend(suspend_state_t state) { return -ENOSYS; }