[PATCH] i8042 / PM: Allow PC keyboard to wake up from suspend-to-idle

[PATCH] i8042 / PM: Allow PC keyboard to wake up from suspend-to-idle

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

If they keyboard interrupt is registered, mark the i8042 platform
device as wakeup-capable and check the user space wakeup setting in
i8042_pm_suspend() and i8042_pm_resume() to enable or disable,
respectively, the keyboard interrupt to wake up the system.

This makes it possible to use the PC keyboard to wake up the system
from suspend-to-idle after writing "enabled" to the i8042 device's
power/wakeup sysfs attribute.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/input/serio/i8042.c |    6 ++++++
 1 file changed, 6 insertions(+)

Index: linux-pm/drivers/input/serio/i8042.c
===================================================================
--- linux-pm.orig/drivers/input/serio/i8042.c
+++ linux-pm/drivers/input/serio/i8042.c
@@ -1163,12 +1163,17 @@ static int i8042_controller_resume(bool
 static int i8042_pm_suspend(struct device *dev)
 {
 	i8042_controller_reset(true);
+	if (device_may_wakeup(&i8042_platform_device->dev))
+		enable_irq_wake(I8042_KBD_IRQ);
 
 	return 0;
 }
 
 static int i8042_pm_resume(struct device *dev)
 {
+	if (device_may_wakeup(&i8042_platform_device->dev))
+		disable_irq_wake(I8042_KBD_IRQ);
+
 	/*
 	 * On resume from S2R we always try to reset the controller
 	 * to bring it in a sane state. (In case of S2D we expect
@@ -1406,6 +1411,7 @@ static int __init i8042_setup_kbd(void)
 	if (error)
 		goto err_free_irq;
 
+	device_set_wakeup_capable(&i8042_platform_device->dev, true);
 	i8042_kbd_irq_registered = true;
 	return 0;
 

Re: [PATCH] i8042 / PM: Allow PC keyboard to wake up from suspend-to-idle

[Dmitry Torokhov]

Hi Rafael,

On Mon, Mar 09, 2015 at 04:19:50PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> If they keyboard interrupt is registered, mark the i8042 platform
> device as wakeup-capable and check the user space wakeup setting in
> i8042_pm_suspend() and i8042_pm_resume() to enable or disable,
> respectively, the keyboard interrupt to wake up the system.
> 
> This makes it possible to use the PC keyboard to wake up the system
> from suspend-to-idle after writing "enabled" to the i8042 device's
> power/wakeup sysfs attribute.

Why do we do that for KBD but not AUX port? Should we mark individual
serio port be wakeup capable and not the whole i8042.

Also, why exactly is this needed? My laptops seem to resume just fine
from keyboard activity without this patch...

Thanks.

> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/input/serio/i8042.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> Index: linux-pm/drivers/input/serio/i8042.c
> ===================================================================
> --- linux-pm.orig/drivers/input/serio/i8042.c
> +++ linux-pm/drivers/input/serio/i8042.c
> @@ -1163,12 +1163,17 @@ static int i8042_controller_resume(bool
>  static int i8042_pm_suspend(struct device *dev)
>  {
>  	i8042_controller_reset(true);
> +	if (device_may_wakeup(&i8042_platform_device->dev))
> +		enable_irq_wake(I8042_KBD_IRQ);
>  
>  	return 0;
>  }
>  
>  static int i8042_pm_resume(struct device *dev)
>  {
> +	if (device_may_wakeup(&i8042_platform_device->dev))
> +		disable_irq_wake(I8042_KBD_IRQ);
> +
>  	/*
>  	 * On resume from S2R we always try to reset the controller
>  	 * to bring it in a sane state. (In case of S2D we expect
> @@ -1406,6 +1411,7 @@ static int __init i8042_setup_kbd(void)
>  	if (error)
>  		goto err_free_irq;
>  
> +	device_set_wakeup_capable(&i8042_platform_device->dev, true);
>  	i8042_kbd_irq_registered = true;
>  	return 0;
>  
> 

-- 
Dmitry

Re: [PATCH] i8042 / PM: Allow PC keyboard to wake up from suspend-to-idle

[Rafael J. Wysocki]

On Monday, March 09, 2015 11:00:04 AM Dmitry Torokhov wrote:
> Hi Rafael,
> 
> On Mon, Mar 09, 2015 at 04:19:50PM +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > 
> > If they keyboard interrupt is registered, mark the i8042 platform
> > device as wakeup-capable and check the user space wakeup setting in
> > i8042_pm_suspend() and i8042_pm_resume() to enable or disable,
> > respectively, the keyboard interrupt to wake up the system.
> > 
> > This makes it possible to use the PC keyboard to wake up the system
> > from suspend-to-idle after writing "enabled" to the i8042 device's
> > power/wakeup sysfs attribute.
> 
> Why do we do that for KBD but not AUX port? Should we mark individual
> serio port be wakeup capable and not the whole i8042.

We can do that, but only after the port serio device has been registered.

I guess I can add code for that to i8042_register_ports().  Let me try that.

> Also, why exactly is this needed? My laptops seem to resume just fine
> from keyboard activity without this patch...

Mine don't.  At least not from suspend-to-idle ("echo freeze > /sys/power/state").

Whether or not the PC keyboard wakes them up from suspend-to-RAM depends on the
BIOS.  That only works for one of my laptops, but this patch doesn't help here.

Rafael

Re: [PATCH] i8042 / PM: Allow PC keyboard to wake up from suspend-to-idle

[Rafael J. Wysocki]

On Monday, March 09, 2015 11:41:12 PM Rafael J. Wysocki wrote:
> On Monday, March 09, 2015 11:00:04 AM Dmitry Torokhov wrote:
> > Hi Rafael,
> > 
> > On Mon, Mar 09, 2015 at 04:19:50PM +0100, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > 
> > > If they keyboard interrupt is registered, mark the i8042 platform
> > > device as wakeup-capable and check the user space wakeup setting in
> > > i8042_pm_suspend() and i8042_pm_resume() to enable or disable,
> > > respectively, the keyboard interrupt to wake up the system.
> > > 
> > > This makes it possible to use the PC keyboard to wake up the system
> > > from suspend-to-idle after writing "enabled" to the i8042 device's
> > > power/wakeup sysfs attribute.
> > 
> > Why do we do that for KBD but not AUX port? Should we mark individual
> > serio port be wakeup capable and not the whole i8042.
> 
> We can do that, but only after the port serio device has been registered.
> 
> I guess I can add code for that to i8042_register_ports().  Let me try that.

Yeah, that works too.  And you're right that there's no reason to do that for
keyboard only.

Patch below.

Rafael


---
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Subject: i8042 / PM: Allow i8042 ports to wake up from suspend-to-idle

While registering serio device for i8042, mark them as wakeup-capable
and check their user space wakeup settings in i8042_pm_suspend() and
i8042_pm_resume() to enable or disable, respectively, their interrupts
to wake up the system.

This makes it possible to use the PC keyboard to wake up the system
from suspend-to-idle, among other things, after writing "enabled" to
the keyboard serio device's power/wakeup sysfs attribute.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/input/serio/i8042.c |   28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

Index: linux-pm/drivers/input/serio/i8042.c
===================================================================
--- linux-pm.orig/drivers/input/serio/i8042.c
+++ linux-pm/drivers/input/serio/i8042.c
@@ -1162,13 +1162,32 @@ static int i8042_controller_resume(bool
 
 static int i8042_pm_suspend(struct device *dev)
 {
+	int i;
+
 	i8042_controller_reset(true);
 
+	/* Set up serio interrupts for system wakeup. */
+	for (i = 0; i < I8042_NUM_PORTS; i++) {
+		struct serio *serio = i8042_ports[i].serio;
+
+		if (serio && device_may_wakeup(&serio->dev))
+			enable_irq_wake(i8042_ports[i].irq);
+	}
+
 	return 0;
 }
 
 static int i8042_pm_resume(struct device *dev)
 {
+	int i;
+
+	for (i = 0; i < I8042_NUM_PORTS; i++) {
+		struct serio *serio = i8042_ports[i].serio;
+
+		if (serio && device_may_wakeup(&serio->dev))
+			disable_irq_wake(i8042_ports[i].irq);
+	}
+
 	/*
 	 * On resume from S2R we always try to reset the controller
 	 * to bring it in a sane state. (In case of S2D we expect
@@ -1300,13 +1319,16 @@ static void __init i8042_register_ports(
 	int i;
 
 	for (i = 0; i < I8042_NUM_PORTS; i++) {
-		if (i8042_ports[i].serio) {
+		struct serio *serio = i8042_ports[i].serio;
+
+		if (serio) {
 			printk(KERN_INFO "serio: %s at %#lx,%#lx irq %d\n",
-				i8042_ports[i].serio->name,
+				serio->name,
 				(unsigned long) I8042_DATA_REG,
 				(unsigned long) I8042_COMMAND_REG,
 				i8042_ports[i].irq);
-			serio_register_port(i8042_ports[i].serio);
+			serio_register_port(serio);
+			device_set_wakeup_capable(&serio->dev, true);
 		}
 	}
 }

Re: [PATCH] i8042 / PM: Allow PC keyboard to wake up from suspend-to-idle

[Dmitry Torokhov]

On Tue, Mar 10, 2015 at 12:08:43AM +0100, Rafael J. Wysocki wrote:
> On Monday, March 09, 2015 11:41:12 PM Rafael J. Wysocki wrote:
> > On Monday, March 09, 2015 11:00:04 AM Dmitry Torokhov wrote:
> > > Hi Rafael,
> > > 
> > > On Mon, Mar 09, 2015 at 04:19:50PM +0100, Rafael J. Wysocki wrote:
> > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > > 
> > > > If they keyboard interrupt is registered, mark the i8042 platform
> > > > device as wakeup-capable and check the user space wakeup setting in
> > > > i8042_pm_suspend() and i8042_pm_resume() to enable or disable,
> > > > respectively, the keyboard interrupt to wake up the system.
> > > > 
> > > > This makes it possible to use the PC keyboard to wake up the system
> > > > from suspend-to-idle after writing "enabled" to the i8042 device's
> > > > power/wakeup sysfs attribute.
> > > 
> > > Why do we do that for KBD but not AUX port? Should we mark individual
> > > serio port be wakeup capable and not the whole i8042.
> > 
> > We can do that, but only after the port serio device has been registered.
> > 
> > I guess I can add code for that to i8042_register_ports().  Let me try that.
> 
> Yeah, that works too.  And you're right that there's no reason to do that for
> keyboard only.
> 
> Patch below.

Applied, thank you.

> 
> Rafael
> 
> 
> ---
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Subject: i8042 / PM: Allow i8042 ports to wake up from suspend-to-idle
> 
> While registering serio device for i8042, mark them as wakeup-capable
> and check their user space wakeup settings in i8042_pm_suspend() and
> i8042_pm_resume() to enable or disable, respectively, their interrupts
> to wake up the system.
> 
> This makes it possible to use the PC keyboard to wake up the system
> from suspend-to-idle, among other things, after writing "enabled" to
> the keyboard serio device's power/wakeup sysfs attribute.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/input/serio/i8042.c |   28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> Index: linux-pm/drivers/input/serio/i8042.c
> ===================================================================
> --- linux-pm.orig/drivers/input/serio/i8042.c
> +++ linux-pm/drivers/input/serio/i8042.c
> @@ -1162,13 +1162,32 @@ static int i8042_controller_resume(bool
>  
>  static int i8042_pm_suspend(struct device *dev)
>  {
> +	int i;
> +
>  	i8042_controller_reset(true);
>  
> +	/* Set up serio interrupts for system wakeup. */
> +	for (i = 0; i < I8042_NUM_PORTS; i++) {
> +		struct serio *serio = i8042_ports[i].serio;
> +
> +		if (serio && device_may_wakeup(&serio->dev))
> +			enable_irq_wake(i8042_ports[i].irq);
> +	}
> +
>  	return 0;
>  }
>  
>  static int i8042_pm_resume(struct device *dev)
>  {
> +	int i;
> +
> +	for (i = 0; i < I8042_NUM_PORTS; i++) {
> +		struct serio *serio = i8042_ports[i].serio;
> +
> +		if (serio && device_may_wakeup(&serio->dev))
> +			disable_irq_wake(i8042_ports[i].irq);
> +	}
> +
>  	/*
>  	 * On resume from S2R we always try to reset the controller
>  	 * to bring it in a sane state. (In case of S2D we expect
> @@ -1300,13 +1319,16 @@ static void __init i8042_register_ports(
>  	int i;
>  
>  	for (i = 0; i < I8042_NUM_PORTS; i++) {
> -		if (i8042_ports[i].serio) {
> +		struct serio *serio = i8042_ports[i].serio;
> +
> +		if (serio) {
>  			printk(KERN_INFO "serio: %s at %#lx,%#lx irq %d\n",
> -				i8042_ports[i].serio->name,
> +				serio->name,
>  				(unsigned long) I8042_DATA_REG,
>  				(unsigned long) I8042_COMMAND_REG,
>  				i8042_ports[i].irq);
> -			serio_register_port(i8042_ports[i].serio);
> +			serio_register_port(serio);
> +			device_set_wakeup_capable(&serio->dev, true);
>  		}
>  	}
>  }
> 

-- 
Dmitry

Re: [PATCH] i8042 / PM: Allow PC keyboard to wake up from suspend-to-idle

[Pavel Machek]

On Tue 2015-03-10 00:08:43, Rafael J. Wysocki wrote:
> On Monday, March 09, 2015 11:41:12 PM Rafael J. Wysocki wrote:
> > On Monday, March 09, 2015 11:00:04 AM Dmitry Torokhov wrote:
> > > Hi Rafael,
> > > 
> > > On Mon, Mar 09, 2015 at 04:19:50PM +0100, Rafael J. Wysocki wrote:
> > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > > 
> > > > If they keyboard interrupt is registered, mark the i8042 platform
> > > > device as wakeup-capable and check the user space wakeup setting in
> > > > i8042_pm_suspend() and i8042_pm_resume() to enable or disable,
> > > > respectively, the keyboard interrupt to wake up the system.
> > > > 
> > > > This makes it possible to use the PC keyboard to wake up the system
> > > > from suspend-to-idle after writing "enabled" to the i8042 device's
> > > > power/wakeup sysfs attribute.
> > > 
> > > Why do we do that for KBD but not AUX port? Should we mark individual
> > > serio port be wakeup capable and not the whole i8042.
> > 
> > We can do that, but only after the port serio device has been registered.
> > 
> > I guess I can add code for that to i8042_register_ports().  Let me try that.
> 
> Yeah, that works too.  And you're right that there's no reason to do that for
> keyboard only.

Actually, are you sure?

Resuming on mouse click is reasonable, resuming on mouse move is not,
as table vibrations are likely to cause that.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] i8042 / PM: Allow PC keyboard to wake up from suspend-to-idle

[Rafael J. Wysocki]

On Monday, March 16, 2015 10:11:08 PM Pavel Machek wrote:
> On Tue 2015-03-10 00:08:43, Rafael J. Wysocki wrote:
> > On Monday, March 09, 2015 11:41:12 PM Rafael J. Wysocki wrote:
> > > On Monday, March 09, 2015 11:00:04 AM Dmitry Torokhov wrote:
> > > > Hi Rafael,
> > > > 
> > > > On Mon, Mar 09, 2015 at 04:19:50PM +0100, Rafael J. Wysocki wrote:
> > > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > > > 
> > > > > If they keyboard interrupt is registered, mark the i8042 platform
> > > > > device as wakeup-capable and check the user space wakeup setting in
> > > > > i8042_pm_suspend() and i8042_pm_resume() to enable or disable,
> > > > > respectively, the keyboard interrupt to wake up the system.
> > > > > 
> > > > > This makes it possible to use the PC keyboard to wake up the system
> > > > > from suspend-to-idle after writing "enabled" to the i8042 device's
> > > > > power/wakeup sysfs attribute.
> > > > 
> > > > Why do we do that for KBD but not AUX port? Should we mark individual
> > > > serio port be wakeup capable and not the whole i8042.
> > > 
> > > We can do that, but only after the port serio device has been registered.
> > > 
> > > I guess I can add code for that to i8042_register_ports().  Let me try that.
> > 
> > Yeah, that works too.  And you're right that there's no reason to do that for
> > keyboard only.
> 
> Actually, are you sure?
> 
> Resuming on mouse click is reasonable, resuming on mouse move is not,
> as table vibrations are likely to cause that.

This is disabled by default, so user space has to enable it anyway if it wants
to.

Rafael

Re: [PATCH] i8042 / PM: Allow PC keyboard to wake up from suspend-to-idle

[Pavel Machek]

> > > Yeah, that works too.  And you're right that there's no reason to do that for
> > > keyboard only.
> > 
> > Actually, are you sure?
> > 
> > Resuming on mouse click is reasonable, resuming on mouse move is not,
> > as table vibrations are likely to cause that.
> 
> This is disabled by default, so user space has to enable it anyway if it wants
> to.

Good. (But still, for suspend-to-RAM, only mouse _click_ will resume
the machine, as the optical sensor is powered down, no?)

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] i8042 / PM: Allow PC keyboard to wake up from suspend-to-idle

[Rafael J. Wysocki]

On Tuesday, March 17, 2015 09:50:33 AM Pavel Machek wrote:
> 
> > > > Yeah, that works too.  And you're right that there's no reason to do that for
> > > > keyboard only.
> > > 
> > > Actually, are you sure?
> > > 
> > > Resuming on mouse click is reasonable, resuming on mouse move is not,
> > > as table vibrations are likely to cause that.
> > 
> > This is disabled by default, so user space has to enable it anyway if it wants
> > to.
> 
> Good. (But still, for suspend-to-RAM, only mouse _click_ will resume
> the machine, as the optical sensor is powered down, no?)

I'm not sure really.  I have mice that resume from suspend-to-RAM on a movement
too, but those are USB.

Rafael