| Message ID | 20180904034134.15381-1-drake@endlessm.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | i8042: enable keyboard wakeups by default when s2idle is used | expand |
On Tuesday, September 4, 2018 5:41:34 AM CEST 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 WhiskyLake, > 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 > Signed-off-by: Daniel Drake <drake@endlessm.com> > --- > drivers/input/serio/i8042.c | 31 ++++++++++++++++++++++--------- > 1 file changed, 22 insertions(+), 9 deletions(-) > > diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c > index a8cd9072481c..7229609a9397 100644 > --- a/drivers/input/serio/i8042.c > +++ b/drivers/input/serio/i8042.c > @@ -1392,15 +1392,28 @@ 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); > + > + /* > + * Modern laptops (as of 2018) typically no longer have the > + * behaviour where pressing a key in suspend causes a SCI > + * interrupt which unconditionally wakes up the system. This behavior was not universally present even before 2018 AFAICS. > + * > + * On platforms that use s2idle, retain the previous keyboard > + * wakeup behaviour by enabling wakeups by default. > + */ So I would say: "On platforms using suspend-to-idle by default, make the keyboard wake up the system from seel by enabling keyboard wakeups by default. That is consistent with keyboard wakeup behavior on many platforms using suspend-to-RAM (ACPI S3) by default." > + if (mem_sleep_current == PM_SUSPEND_TO_IDLE > + && i == I8042_KBD_PORT_NO) > + device_set_wakeup_enable(&serio->dev, true); > } > } > > Apart from the above it looks OK to me. Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c index a8cd9072481c..7229609a9397 100644 --- a/drivers/input/serio/i8042.c +++ b/drivers/input/serio/i8042.c @@ -1392,15 +1392,28 @@ 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); + + /* + * Modern laptops (as of 2018) typically no longer have the + * behaviour where pressing a key in suspend causes a SCI + * interrupt which unconditionally wakes up the system. + * + * On platforms that use s2idle, retain the previous keyboard + * wakeup behaviour by enabling wakeups by default. + */ + if (mem_sleep_current == PM_SUSPEND_TO_IDLE + && i == I8042_KBD_PORT_NO) + device_set_wakeup_enable(&serio->dev, true); } }
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 WhiskyLake, 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 Signed-off-by: Daniel Drake <drake@endlessm.com> --- drivers/input/serio/i8042.c | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-)