linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Input: enable i8042-level wakeup control
@ 2011-02-10 17:16 Daniel Drake
  2011-03-10 20:36 ` Andres Salomon
  2011-03-11  9:01 ` Andres Salomon
  0 siblings, 2 replies; 7+ messages in thread
From: Daniel Drake @ 2011-02-10 17:16 UTC (permalink / raw)
  To: dmitry.torokhov, dtor; +Cc: linux-input, rjw, dilinger

The OLPC XO laptop is able to use the PS/2 controller as a wakeup source.
When used as a wakeup source, the key press/mouse motion must not be lost.

Add infrastructure to allow controllers to be marked as wakeup-capable,
and avoid doing power control/reset on the i8042/serio devices when running
in this mode.

Signed-off-by: Daniel Drake <dsd@laptop.org>
---
 drivers/input/serio/i8042-io.h        |    4 +++
 drivers/input/serio/i8042-ip22io.h    |    4 +++
 drivers/input/serio/i8042-jazzio.h    |    4 +++
 drivers/input/serio/i8042-ppcio.h     |    4 +++
 drivers/input/serio/i8042-snirm.h     |    4 +++
 drivers/input/serio/i8042-sparcio.h   |    4 +++
 drivers/input/serio/i8042-x86ia64io.h |    4 +++
 drivers/input/serio/i8042.c           |   37 +++++++++++++++++++++++++++++---
 drivers/input/serio/serio.c           |   28 +++++++++++++++++++++---
 9 files changed, 85 insertions(+), 8 deletions(-)

A followup patch will come soon, hooking this into OLPC's embedded controller:
http://dev.laptop.org/~dsd/20110114/0015-i8042-Enable-OLPC-s-EC-based-i8042-wakeup-control.patch

diff --git a/drivers/input/serio/i8042-io.h b/drivers/input/serio/i8042-io.h
index 5d48bb6..296633c 100644
--- a/drivers/input/serio/i8042-io.h
+++ b/drivers/input/serio/i8042-io.h
@@ -92,4 +92,8 @@ static inline void i8042_platform_exit(void)
 #endif
 }
 
+static inline void i8042_platform_suspend(struct device *dev, bool may_wakeup)
+{
+}
+
 #endif /* _I8042_IO_H */
diff --git a/drivers/input/serio/i8042-ip22io.h b/drivers/input/serio/i8042-ip22io.h
index ee1ad27..c5b76a4 100644
--- a/drivers/input/serio/i8042-ip22io.h
+++ b/drivers/input/serio/i8042-ip22io.h
@@ -73,4 +73,8 @@ static inline void i8042_platform_exit(void)
 #endif
 }
 
+static inline void i8042_platform_suspend(struct device *dev, bool may_wakeup)
+{
+}
+
 #endif /* _I8042_IP22_H */
diff --git a/drivers/input/serio/i8042-jazzio.h b/drivers/input/serio/i8042-jazzio.h
index 13fd710..a11913a 100644
--- a/drivers/input/serio/i8042-jazzio.h
+++ b/drivers/input/serio/i8042-jazzio.h
@@ -66,4 +66,8 @@ static inline void i8042_platform_exit(void)
 #endif
 }
 
+static inline void i8042_platform_suspend(struct device *dev, bool may_wakeup)
+{
+}
+
 #endif /* _I8042_JAZZ_H */
diff --git a/drivers/input/serio/i8042-ppcio.h b/drivers/input/serio/i8042-ppcio.h
index f708c75..c9f4292 100644
--- a/drivers/input/serio/i8042-ppcio.h
+++ b/drivers/input/serio/i8042-ppcio.h
@@ -52,6 +52,10 @@ static inline void i8042_platform_exit(void)
 {
 }
 
+static inline void i8042_platform_suspend(struct device *dev, bool may_wakeup)
+{
+}
+
 #else
 
 #include "i8042-io.h"
diff --git a/drivers/input/serio/i8042-snirm.h b/drivers/input/serio/i8042-snirm.h
index 409a934..96d034f 100644
--- a/drivers/input/serio/i8042-snirm.h
+++ b/drivers/input/serio/i8042-snirm.h
@@ -72,4 +72,8 @@ static inline void i8042_platform_exit(void)
 
 }
 
+static inline void i8042_platform_suspend(struct device *dev, bool may_wakeup)
+{
+}
+
 #endif /* _I8042_SNIRM_H */
diff --git a/drivers/input/serio/i8042-sparcio.h b/drivers/input/serio/i8042-sparcio.h
index c5cc450..a6c1f74 100644
--- a/drivers/input/serio/i8042-sparcio.h
+++ b/drivers/input/serio/i8042-sparcio.h
@@ -154,4 +154,8 @@ static inline void i8042_platform_exit(void)
 }
 #endif /* !CONFIG_PCI */
 
+static inline void i8042_platform_suspend(struct device *dev, bool may_wakeup)
+{
+}
+
 #endif /* _I8042_SPARCIO_H */
diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h
index bb9f5d3..76b2e58 100644
--- a/drivers/input/serio/i8042-x86ia64io.h
+++ b/drivers/input/serio/i8042-x86ia64io.h
@@ -875,6 +875,10 @@ static inline int i8042_pnp_init(void) { return 0; }
 static inline void i8042_pnp_exit(void) { }
 #endif
 
+static inline void i8042_platform_suspend(struct device *dev, bool may_wakeup)
+{
+}
+
 static int __init i8042_platform_init(void)
 {
 	int retval;
diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index ac4c936..fb9dee6 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -79,6 +79,10 @@ module_param_named(nopnp, i8042_nopnp, bool, 0);
 MODULE_PARM_DESC(nopnp, "Do not use PNP to detect controller settings");
 #endif
 
+static bool i8042_enable_wakeup;
+module_param_named(enable_wakeup, i8042_enable_wakeup, bool, 0);
+MODULE_PARM_DESC(enable_wakeup, "Enable i8042 as wakeup-capable device");
+
 #define DEBUG
 #ifdef DEBUG
 static bool i8042_debug;
@@ -1081,10 +1085,17 @@ static void i8042_dritek_enable(void)
  * before suspending.
  */
 
-static int i8042_controller_resume(bool force_reset)
+static int i8042_controller_resume(bool force_reset, bool soft_resume)
 {
 	int error;
 
+	/*
+	 * If device is selected as a wakeup source, it was not powered down
+	 * or reset during suspend, so we have very little to do.
+	 */
+	if (soft_resume)
+		goto soft;
+
 	error = i8042_controller_check();
 	if (error)
 		return error;
@@ -1128,6 +1139,7 @@ static int i8042_controller_resume(bool force_reset)
 	if (i8042_ports[I8042_KBD_PORT_NO].serio)
 		i8042_enable_kbd_port();
 
+soft:
 	i8042_interrupt(0, NULL);
 
 	return 0;
@@ -1145,6 +1157,20 @@ static int i8042_pm_reset(struct device *dev)
 	return 0;
 }
 
+static int i8042_pm_suspend(struct device *dev)
+{
+	i8042_platform_suspend(dev, device_may_wakeup(dev));
+
+	/*
+	 * If device is selected as a wakeup source, don't powerdown or reset
+	 * during suspend.
+	 */
+	if (device_may_wakeup(dev))
+		return 0;
+
+	return i8042_pm_reset(dev);
+}
+
 static int i8042_pm_resume(struct device *dev)
 {
 	/*
@@ -1152,7 +1178,7 @@ static int i8042_pm_resume(struct device *dev)
 	 * to bring it in a sane state. (In case of S2D we expect
 	 * BIOS to reset the controller for us.)
 	 */
-	return i8042_controller_resume(true);
+	return i8042_controller_resume(true, device_may_wakeup(dev));
 }
 
 static int i8042_pm_thaw(struct device *dev)
@@ -1164,11 +1190,11 @@ static int i8042_pm_thaw(struct device *dev)
 
 static int i8042_pm_restore(struct device *dev)
 {
-	return i8042_controller_resume(false);
+	return i8042_controller_resume(false, false);
 }
 
 static const struct dev_pm_ops i8042_pm_ops = {
-	.suspend	= i8042_pm_reset,
+	.suspend	= i8042_pm_suspend,
 	.resume		= i8042_pm_resume,
 	.thaw		= i8042_pm_thaw,
 	.poweroff	= i8042_pm_reset,
@@ -1402,6 +1428,9 @@ static int __init i8042_probe(struct platform_device *dev)
 		i8042_dritek_enable();
 #endif
 
+	if (i8042_enable_wakeup)
+		device_set_wakeup_capable(&dev->dev, true);
+
 	if (!i8042_noaux) {
 		error = i8042_setup_aux();
 		if (error && error != -ENODEV && error != -EBUSY)
diff --git a/drivers/input/serio/serio.c b/drivers/input/serio/serio.c
index db5b0bc..7480be5 100644
--- a/drivers/input/serio/serio.c
+++ b/drivers/input/serio/serio.c
@@ -928,7 +928,7 @@ static int serio_uevent(struct device *dev, struct kobj_uevent_env *env)
 #endif /* CONFIG_HOTPLUG */
 
 #ifdef CONFIG_PM
-static int serio_suspend(struct device *dev)
+static int serio_poweroff(struct device *dev)
 {
 	struct serio *serio = to_serio_port(dev);
 
@@ -937,7 +937,17 @@ static int serio_suspend(struct device *dev)
 	return 0;
 }
 
-static int serio_resume(struct device *dev)
+static int serio_suspend(struct device *dev)
+{
+	/* If parent controller is configured as a wakeup source, don't
+	 * power off. */
+	if (device_may_wakeup(dev->parent))
+		return 0;
+
+	return serio_poweroff(dev);
+}
+
+static int serio_restore(struct device *dev)
 {
 	struct serio *serio = to_serio_port(dev);
 
@@ -950,11 +960,21 @@ static int serio_resume(struct device *dev)
 	return 0;
 }
 
+static int serio_resume(struct device *dev)
+{
+	/* If parent controller is configured as a wakeup source, we didn't
+	 * power off during suspend, and hence have nothing to do. */
+	if (device_may_wakeup(dev->parent))
+		return 0;
+
+	return serio_restore(dev);
+}
+
 static const struct dev_pm_ops serio_pm_ops = {
 	.suspend	= serio_suspend,
 	.resume		= serio_resume,
-	.poweroff	= serio_suspend,
-	.restore	= serio_resume,
+	.poweroff	= serio_poweroff,
+	.restore	= serio_restore,
 };
 #endif /* CONFIG_PM */
 
-- 
1.7.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] Input: enable i8042-level wakeup control
  2011-02-10 17:16 [PATCH] Input: enable i8042-level wakeup control Daniel Drake
@ 2011-03-10 20:36 ` Andres Salomon
  2011-03-11  8:31   ` Dmitry Torokhov
  2011-03-11  9:01 ` Andres Salomon
  1 sibling, 1 reply; 7+ messages in thread
From: Andres Salomon @ 2011-03-10 20:36 UTC (permalink / raw)
  To: dmitry.torokhov; +Cc: Daniel Drake, dtor, linux-input, rjw

I never saw any discussion on this, and it doesn't look like it's in
any git trees (afaict). Dmitry, what are your thoughts on the patch?


On Thu, 10 Feb 2011 17:16:07 +0000 (GMT)
Daniel Drake <dsd@laptop.org> wrote:

> The OLPC XO laptop is able to use the PS/2 controller as a wakeup
> source. When used as a wakeup source, the key press/mouse motion
> must not be lost.
> 
> Add infrastructure to allow controllers to be marked as
> wakeup-capable, and avoid doing power control/reset on the
> i8042/serio devices when running in this mode.
> 
> Signed-off-by: Daniel Drake <dsd@laptop.org>
> ---
>  drivers/input/serio/i8042-io.h        |    4 +++
>  drivers/input/serio/i8042-ip22io.h    |    4 +++
>  drivers/input/serio/i8042-jazzio.h    |    4 +++
>  drivers/input/serio/i8042-ppcio.h     |    4 +++
>  drivers/input/serio/i8042-snirm.h     |    4 +++
>  drivers/input/serio/i8042-sparcio.h   |    4 +++
>  drivers/input/serio/i8042-x86ia64io.h |    4 +++
>  drivers/input/serio/i8042.c           |   37
> +++++++++++++++++++++++++++++---
> drivers/input/serio/serio.c           |   28 +++++++++++++++++++++---
> 9 files changed, 85 insertions(+), 8 deletions(-)
> 
> A followup patch will come soon, hooking this into OLPC's embedded
> controller:
> http://dev.laptop.org/~dsd/20110114/0015-i8042-Enable-OLPC-s-EC-based-i8042-wakeup-control.patch
> 
> diff --git a/drivers/input/serio/i8042-io.h
> b/drivers/input/serio/i8042-io.h index 5d48bb6..296633c 100644
> --- a/drivers/input/serio/i8042-io.h
> +++ b/drivers/input/serio/i8042-io.h
> @@ -92,4 +92,8 @@ static inline void i8042_platform_exit(void)
>  #endif
>  }
>  
> +static inline void i8042_platform_suspend(struct device *dev, bool
> may_wakeup) +{
> +}
> +
>  #endif /* _I8042_IO_H */
> diff --git a/drivers/input/serio/i8042-ip22io.h
> b/drivers/input/serio/i8042-ip22io.h index ee1ad27..c5b76a4 100644
> --- a/drivers/input/serio/i8042-ip22io.h
> +++ b/drivers/input/serio/i8042-ip22io.h
> @@ -73,4 +73,8 @@ static inline void i8042_platform_exit(void)
>  #endif
>  }
>  
> +static inline void i8042_platform_suspend(struct device *dev, bool
> may_wakeup) +{
> +}
> +
>  #endif /* _I8042_IP22_H */
> diff --git a/drivers/input/serio/i8042-jazzio.h
> b/drivers/input/serio/i8042-jazzio.h index 13fd710..a11913a 100644
> --- a/drivers/input/serio/i8042-jazzio.h
> +++ b/drivers/input/serio/i8042-jazzio.h
> @@ -66,4 +66,8 @@ static inline void i8042_platform_exit(void)
>  #endif
>  }
>  
> +static inline void i8042_platform_suspend(struct device *dev, bool
> may_wakeup) +{
> +}
> +
>  #endif /* _I8042_JAZZ_H */
> diff --git a/drivers/input/serio/i8042-ppcio.h
> b/drivers/input/serio/i8042-ppcio.h index f708c75..c9f4292 100644
> --- a/drivers/input/serio/i8042-ppcio.h
> +++ b/drivers/input/serio/i8042-ppcio.h
> @@ -52,6 +52,10 @@ static inline void i8042_platform_exit(void)
>  {
>  }
>  
> +static inline void i8042_platform_suspend(struct device *dev, bool
> may_wakeup) +{
> +}
> +
>  #else
>  
>  #include "i8042-io.h"
> diff --git a/drivers/input/serio/i8042-snirm.h
> b/drivers/input/serio/i8042-snirm.h index 409a934..96d034f 100644
> --- a/drivers/input/serio/i8042-snirm.h
> +++ b/drivers/input/serio/i8042-snirm.h
> @@ -72,4 +72,8 @@ static inline void i8042_platform_exit(void)
>  
>  }
>  
> +static inline void i8042_platform_suspend(struct device *dev, bool
> may_wakeup) +{
> +}
> +
>  #endif /* _I8042_SNIRM_H */
> diff --git a/drivers/input/serio/i8042-sparcio.h
> b/drivers/input/serio/i8042-sparcio.h index c5cc450..a6c1f74 100644
> --- a/drivers/input/serio/i8042-sparcio.h
> +++ b/drivers/input/serio/i8042-sparcio.h
> @@ -154,4 +154,8 @@ static inline void i8042_platform_exit(void)
>  }
>  #endif /* !CONFIG_PCI */
>  
> +static inline void i8042_platform_suspend(struct device *dev, bool
> may_wakeup) +{
> +}
> +
>  #endif /* _I8042_SPARCIO_H */
> diff --git a/drivers/input/serio/i8042-x86ia64io.h
> b/drivers/input/serio/i8042-x86ia64io.h index bb9f5d3..76b2e58 100644
> --- a/drivers/input/serio/i8042-x86ia64io.h
> +++ b/drivers/input/serio/i8042-x86ia64io.h
> @@ -875,6 +875,10 @@ static inline int i8042_pnp_init(void) { return
> 0; } static inline void i8042_pnp_exit(void) { }
>  #endif
>  
> +static inline void i8042_platform_suspend(struct device *dev, bool
> may_wakeup) +{
> +}
> +
>  static int __init i8042_platform_init(void)
>  {
>  	int retval;
> diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
> index ac4c936..fb9dee6 100644
> --- a/drivers/input/serio/i8042.c
> +++ b/drivers/input/serio/i8042.c
> @@ -79,6 +79,10 @@ module_param_named(nopnp, i8042_nopnp, bool, 0);
>  MODULE_PARM_DESC(nopnp, "Do not use PNP to detect controller
> settings"); #endif
>  
> +static bool i8042_enable_wakeup;
> +module_param_named(enable_wakeup, i8042_enable_wakeup, bool, 0);
> +MODULE_PARM_DESC(enable_wakeup, "Enable i8042 as wakeup-capable
> device"); +
>  #define DEBUG
>  #ifdef DEBUG
>  static bool i8042_debug;
> @@ -1081,10 +1085,17 @@ static void i8042_dritek_enable(void)
>   * before suspending.
>   */
>  
> -static int i8042_controller_resume(bool force_reset)
> +static int i8042_controller_resume(bool force_reset, bool
> soft_resume) {
>  	int error;
>  
> +	/*
> +	 * If device is selected as a wakeup source, it was not
> powered down
> +	 * or reset during suspend, so we have very little to do.
> +	 */
> +	if (soft_resume)
> +		goto soft;
> +
>  	error = i8042_controller_check();
>  	if (error)
>  		return error;
> @@ -1128,6 +1139,7 @@ static int i8042_controller_resume(bool
> force_reset) if (i8042_ports[I8042_KBD_PORT_NO].serio)
>  		i8042_enable_kbd_port();
>  
> +soft:
>  	i8042_interrupt(0, NULL);
>  
>  	return 0;
> @@ -1145,6 +1157,20 @@ static int i8042_pm_reset(struct device *dev)
>  	return 0;
>  }
>  
> +static int i8042_pm_suspend(struct device *dev)
> +{
> +	i8042_platform_suspend(dev, device_may_wakeup(dev));
> +
> +	/*
> +	 * If device is selected as a wakeup source, don't powerdown
> or reset
> +	 * during suspend.
> +	 */
> +	if (device_may_wakeup(dev))
> +		return 0;
> +
> +	return i8042_pm_reset(dev);
> +}
> +
>  static int i8042_pm_resume(struct device *dev)
>  {
>  	/*
> @@ -1152,7 +1178,7 @@ static int i8042_pm_resume(struct device *dev)
>  	 * to bring it in a sane state. (In case of S2D we expect
>  	 * BIOS to reset the controller for us.)
>  	 */
> -	return i8042_controller_resume(true);
> +	return i8042_controller_resume(true, device_may_wakeup(dev));
>  }
>  
>  static int i8042_pm_thaw(struct device *dev)
> @@ -1164,11 +1190,11 @@ static int i8042_pm_thaw(struct device *dev)
>  
>  static int i8042_pm_restore(struct device *dev)
>  {
> -	return i8042_controller_resume(false);
> +	return i8042_controller_resume(false, false);
>  }
>  
>  static const struct dev_pm_ops i8042_pm_ops = {
> -	.suspend	= i8042_pm_reset,
> +	.suspend	= i8042_pm_suspend,
>  	.resume		= i8042_pm_resume,
>  	.thaw		= i8042_pm_thaw,
>  	.poweroff	= i8042_pm_reset,
> @@ -1402,6 +1428,9 @@ static int __init i8042_probe(struct
> platform_device *dev) i8042_dritek_enable();
>  #endif
>  
> +	if (i8042_enable_wakeup)
> +		device_set_wakeup_capable(&dev->dev, true);
> +
>  	if (!i8042_noaux) {
>  		error = i8042_setup_aux();
>  		if (error && error != -ENODEV && error != -EBUSY)
> diff --git a/drivers/input/serio/serio.c b/drivers/input/serio/serio.c
> index db5b0bc..7480be5 100644
> --- a/drivers/input/serio/serio.c
> +++ b/drivers/input/serio/serio.c
> @@ -928,7 +928,7 @@ static int serio_uevent(struct device *dev,
> struct kobj_uevent_env *env) #endif /* CONFIG_HOTPLUG */
>  
>  #ifdef CONFIG_PM
> -static int serio_suspend(struct device *dev)
> +static int serio_poweroff(struct device *dev)
>  {
>  	struct serio *serio = to_serio_port(dev);
>  
> @@ -937,7 +937,17 @@ static int serio_suspend(struct device *dev)
>  	return 0;
>  }
>  
> -static int serio_resume(struct device *dev)
> +static int serio_suspend(struct device *dev)
> +{
> +	/* If parent controller is configured as a wakeup source,
> don't
> +	 * power off. */
> +	if (device_may_wakeup(dev->parent))
> +		return 0;
> +
> +	return serio_poweroff(dev);
> +}
> +
> +static int serio_restore(struct device *dev)
>  {
>  	struct serio *serio = to_serio_port(dev);
>  
> @@ -950,11 +960,21 @@ static int serio_resume(struct device *dev)
>  	return 0;
>  }
>  
> +static int serio_resume(struct device *dev)
> +{
> +	/* If parent controller is configured as a wakeup source, we
> didn't
> +	 * power off during suspend, and hence have nothing to do. */
> +	if (device_may_wakeup(dev->parent))
> +		return 0;
> +
> +	return serio_restore(dev);
> +}
> +
>  static const struct dev_pm_ops serio_pm_ops = {
>  	.suspend	= serio_suspend,
>  	.resume		= serio_resume,
> -	.poweroff	= serio_suspend,
> -	.restore	= serio_resume,
> +	.poweroff	= serio_poweroff,
> +	.restore	= serio_restore,
>  };
>  #endif /* CONFIG_PM */
>  


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Input: enable i8042-level wakeup control
  2011-03-10 20:36 ` Andres Salomon
@ 2011-03-11  8:31   ` Dmitry Torokhov
  2011-03-11  8:42     ` Daniel Drake
  0 siblings, 1 reply; 7+ messages in thread
From: Dmitry Torokhov @ 2011-03-11  8:31 UTC (permalink / raw)
  To: Andres Salomon; +Cc: Daniel Drake, linux-input, rjw

Hi Andres,

On Thu, Mar 10, 2011 at 08:36:02PM +0000, Andres Salomon wrote:
> I never saw any discussion on this, and it doesn't look like it's in
> any git trees (afaict). Dmitry, what are your thoughts on the patch?
> 

I must say I am very wary about this patch. I know that there are a few
boxes that will get very confused if we do not reset their
mice/touchpads to the bare PS/2 protocol and I also do not believe that
we can rely on devices keeping their protocol settings after s2r and
especially s2d even if they are marked as wakeup sources.

I understand that that may be the case with OLPC but not with other
boxes...

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Input: enable i8042-level wakeup control
  2011-03-11  8:31   ` Dmitry Torokhov
@ 2011-03-11  8:42     ` Daniel Drake
  0 siblings, 0 replies; 7+ messages in thread
From: Daniel Drake @ 2011-03-11  8:42 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Andres Salomon, linux-input, rjw

On 11 March 2011 08:31, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> I must say I am very wary about this patch. I know that there are a few
> boxes that will get very confused if we do not reset their
> mice/touchpads to the bare PS/2 protocol and I also do not believe that
> we can rely on devices keeping their protocol settings after s2r and
> especially s2d even if they are marked as wakeup sources.
>
> I understand that that may be the case with OLPC but not with other
> boxes...

I agree. Thats why the whole thing is hidden behind
i8042_enable_wakeup - so that it only happens on OLPC and not other
platforms. I don't see i8042-based wakeup being possible in any other
scenario.

If you prefer I could drop the module_param access to that variable so
that it can't even be enabled except when appropriate platforms are
detected.

Daniel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Input: enable i8042-level wakeup control
  2011-02-10 17:16 [PATCH] Input: enable i8042-level wakeup control Daniel Drake
  2011-03-10 20:36 ` Andres Salomon
@ 2011-03-11  9:01 ` Andres Salomon
  2011-03-11  9:33   ` Daniel Drake
  1 sibling, 1 reply; 7+ messages in thread
From: Andres Salomon @ 2011-03-11  9:01 UTC (permalink / raw)
  To: Daniel Drake; +Cc: dmitry.torokhov, dtor, linux-input, rjw

On Thu, 10 Feb 2011 17:16:07 +0000 (GMT)
Daniel Drake <dsd@laptop.org> wrote:

> The OLPC XO laptop is able to use the PS/2 controller as a wakeup
> source. When used as a wakeup source, the key press/mouse motion must
> not be lost.
> 
> Add infrastructure to allow controllers to be marked as
> wakeup-capable, and avoid doing power control/reset on the
> i8042/serio devices when running in this mode.
> 
> Signed-off-by: Daniel Drake <dsd@laptop.org>
> ---
>  drivers/input/serio/i8042-io.h        |    4 +++
>  drivers/input/serio/i8042-ip22io.h    |    4 +++
>  drivers/input/serio/i8042-jazzio.h    |    4 +++
>  drivers/input/serio/i8042-ppcio.h     |    4 +++
>  drivers/input/serio/i8042-snirm.h     |    4 +++
>  drivers/input/serio/i8042-sparcio.h   |    4 +++
>  drivers/input/serio/i8042-x86ia64io.h |    4 +++
>  drivers/input/serio/i8042.c           |   37
> +++++++++++++++++++++++++++++---
> drivers/input/serio/serio.c           |   28 +++++++++++++++++++++---
> 9 files changed, 85 insertions(+), 8 deletions(-)
> 
> A followup patch will come soon, hooking this into OLPC's embedded
> controller:
> http://dev.laptop.org/~dsd/20110114/0015-i8042-Enable-OLPC-s-EC-based-i8042-wakeup-control.patch
> 
> diff --git a/drivers/input/serio/i8042-io.h
> b/drivers/input/serio/i8042-io.h index 5d48bb6..296633c 100644
> --- a/drivers/input/serio/i8042-io.h
> +++ b/drivers/input/serio/i8042-io.h
> @@ -92,4 +92,8 @@ static inline void i8042_platform_exit(void)
>  #endif
>  }
>  
> +static inline void i8042_platform_suspend(struct device *dev, bool
> may_wakeup) +{
> +}
> +
>  #endif /* _I8042_IO_H */
> diff --git a/drivers/input/serio/i8042-ip22io.h
> b/drivers/input/serio/i8042-ip22io.h index ee1ad27..c5b76a4 100644
> --- a/drivers/input/serio/i8042-ip22io.h
> +++ b/drivers/input/serio/i8042-ip22io.h
> @@ -73,4 +73,8 @@ static inline void i8042_platform_exit(void)
>  #endif
>  }
>  
> +static inline void i8042_platform_suspend(struct device *dev, bool
> may_wakeup) +{
> +}
> +
>  #endif /* _I8042_IP22_H */
> diff --git a/drivers/input/serio/i8042-jazzio.h
> b/drivers/input/serio/i8042-jazzio.h index 13fd710..a11913a 100644
> --- a/drivers/input/serio/i8042-jazzio.h
> +++ b/drivers/input/serio/i8042-jazzio.h
> @@ -66,4 +66,8 @@ static inline void i8042_platform_exit(void)
>  #endif
>  }
>  
> +static inline void i8042_platform_suspend(struct device *dev, bool
> may_wakeup) +{
> +}
> +
>  #endif /* _I8042_JAZZ_H */
> diff --git a/drivers/input/serio/i8042-ppcio.h
> b/drivers/input/serio/i8042-ppcio.h index f708c75..c9f4292 100644
> --- a/drivers/input/serio/i8042-ppcio.h
> +++ b/drivers/input/serio/i8042-ppcio.h
> @@ -52,6 +52,10 @@ static inline void i8042_platform_exit(void)
>  {
>  }
>  
> +static inline void i8042_platform_suspend(struct device *dev, bool
> may_wakeup) +{
> +}
> +
>  #else
>  
>  #include "i8042-io.h"
> diff --git a/drivers/input/serio/i8042-snirm.h
> b/drivers/input/serio/i8042-snirm.h index 409a934..96d034f 100644
> --- a/drivers/input/serio/i8042-snirm.h
> +++ b/drivers/input/serio/i8042-snirm.h
> @@ -72,4 +72,8 @@ static inline void i8042_platform_exit(void)
>  
>  }
>  
> +static inline void i8042_platform_suspend(struct device *dev, bool
> may_wakeup) +{
> +}
> +
>  #endif /* _I8042_SNIRM_H */
> diff --git a/drivers/input/serio/i8042-sparcio.h
> b/drivers/input/serio/i8042-sparcio.h index c5cc450..a6c1f74 100644
> --- a/drivers/input/serio/i8042-sparcio.h
> +++ b/drivers/input/serio/i8042-sparcio.h
> @@ -154,4 +154,8 @@ static inline void i8042_platform_exit(void)
>  }
>  #endif /* !CONFIG_PCI */
>  
> +static inline void i8042_platform_suspend(struct device *dev, bool
> may_wakeup) +{
> +}
> +
>  #endif /* _I8042_SPARCIO_H */
> diff --git a/drivers/input/serio/i8042-x86ia64io.h
> b/drivers/input/serio/i8042-x86ia64io.h index bb9f5d3..76b2e58 100644
> --- a/drivers/input/serio/i8042-x86ia64io.h
> +++ b/drivers/input/serio/i8042-x86ia64io.h
> @@ -875,6 +875,10 @@ static inline int i8042_pnp_init(void) { return
> 0; } static inline void i8042_pnp_exit(void) { }
>  #endif
>  
> +static inline void i8042_platform_suspend(struct device *dev, bool
> may_wakeup) +{
> +}
> +
>  static int __init i8042_platform_init(void)
>  {
>  	int retval;
> diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
> index ac4c936..fb9dee6 100644
> --- a/drivers/input/serio/i8042.c
> +++ b/drivers/input/serio/i8042.c
> @@ -79,6 +79,10 @@ module_param_named(nopnp, i8042_nopnp, bool, 0);
>  MODULE_PARM_DESC(nopnp, "Do not use PNP to detect controller
> settings"); #endif
>  
> +static bool i8042_enable_wakeup;
> +module_param_named(enable_wakeup, i8042_enable_wakeup, bool, 0);
> +MODULE_PARM_DESC(enable_wakeup, "Enable i8042 as wakeup-capable
> device"); +


If we're going to default to disabling this (which we should), it would
be nice to have a quirk for OLPC (probably checking DMI or something)
that enables it.


>  #define DEBUG
>  #ifdef DEBUG
>  static bool i8042_debug;
> @@ -1081,10 +1085,17 @@ static void i8042_dritek_enable(void)
>   * before suspending.
>   */
>  
> -static int i8042_controller_resume(bool force_reset)
> +static int i8042_controller_resume(bool force_reset, bool
> soft_resume) {
>  	int error;
>  
> +	/*
> +	 * If device is selected as a wakeup source, it was not
> powered down
> +	 * or reset during suspend, so we have very little to do.
> +	 */
> +	if (soft_resume)
> +		goto soft;
> +
>  	error = i8042_controller_check();
>  	if (error)
>  		return error;
> @@ -1128,6 +1139,7 @@ static int i8042_controller_resume(bool
> force_reset) if (i8042_ports[I8042_KBD_PORT_NO].serio)
>  		i8042_enable_kbd_port();
>  
> +soft:
>  	i8042_interrupt(0, NULL);
>  
>  	return 0;
> @@ -1145,6 +1157,20 @@ static int i8042_pm_reset(struct device *dev)
>  	return 0;
>  }
>  
> +static int i8042_pm_suspend(struct device *dev)
> +{
> +	i8042_platform_suspend(dev, device_may_wakeup(dev));


What exactly is the purpose of this?   I see this allows various
architectures to provide an override here, but it seems like this
functionality wasn't previously present (they all just used
i8042_pm_reset).  Do you have a later patch that makes use of this or
something?



> +
> +	/*
> +	 * If device is selected as a wakeup source, don't powerdown
> or reset
> +	 * during suspend.
> +	 */
> +	if (device_may_wakeup(dev))
> +		return 0;
> +
> +	return i8042_pm_reset(dev);
> +}
> +
>  static int i8042_pm_resume(struct device *dev)
>  {
>  	/*
> @@ -1152,7 +1178,7 @@ static int i8042_pm_resume(struct device *dev)
>  	 * to bring it in a sane state. (In case of S2D we expect
>  	 * BIOS to reset the controller for us.)
>  	 */
> -	return i8042_controller_resume(true);
> +	return i8042_controller_resume(true, device_may_wakeup(dev));
>  }
>  
>  static int i8042_pm_thaw(struct device *dev)
> @@ -1164,11 +1190,11 @@ static int i8042_pm_thaw(struct device *dev)
>  
>  static int i8042_pm_restore(struct device *dev)
>  {
> -	return i8042_controller_resume(false);
> +	return i8042_controller_resume(false, false);
>  }
>  
>  static const struct dev_pm_ops i8042_pm_ops = {
> -	.suspend	= i8042_pm_reset,
> +	.suspend	= i8042_pm_suspend,
>  	.resume		= i8042_pm_resume,
>  	.thaw		= i8042_pm_thaw,
>  	.poweroff	= i8042_pm_reset,
> @@ -1402,6 +1428,9 @@ static int __init i8042_probe(struct
> platform_device *dev) i8042_dritek_enable();
>  #endif
>  
> +	if (i8042_enable_wakeup)
> +		device_set_wakeup_capable(&dev->dev, true);
> +
>  	if (!i8042_noaux) {
>  		error = i8042_setup_aux();
>  		if (error && error != -ENODEV && error != -EBUSY)
> diff --git a/drivers/input/serio/serio.c b/drivers/input/serio/serio.c
> index db5b0bc..7480be5 100644
> --- a/drivers/input/serio/serio.c
> +++ b/drivers/input/serio/serio.c
> @@ -928,7 +928,7 @@ static int serio_uevent(struct device *dev,
> struct kobj_uevent_env *env) #endif /* CONFIG_HOTPLUG */
>  
>  #ifdef CONFIG_PM
> -static int serio_suspend(struct device *dev)
> +static int serio_poweroff(struct device *dev)
>  {
>  	struct serio *serio = to_serio_port(dev);
>  
> @@ -937,7 +937,17 @@ static int serio_suspend(struct device *dev)
>  	return 0;
>  }
>  
> -static int serio_resume(struct device *dev)
> +static int serio_suspend(struct device *dev)
> +{
> +	/* If parent controller is configured as a wakeup source,
> don't
> +	 * power off. */
> +	if (device_may_wakeup(dev->parent))
> +		return 0;
> +
> +	return serio_poweroff(dev);
> +}
> +
> +static int serio_restore(struct device *dev)
>  {
>  	struct serio *serio = to_serio_port(dev);
>  
> @@ -950,11 +960,21 @@ static int serio_resume(struct device *dev)
>  	return 0;
>  }
>  
> +static int serio_resume(struct device *dev)
> +{
> +	/* If parent controller is configured as a wakeup source, we
> didn't
> +	 * power off during suspend, and hence have nothing to do. */
> +	if (device_may_wakeup(dev->parent))
> +		return 0;
> +
> +	return serio_restore(dev);
> +}
> +
>  static const struct dev_pm_ops serio_pm_ops = {
>  	.suspend	= serio_suspend,
>  	.resume		= serio_resume,
> -	.poweroff	= serio_suspend,
> -	.restore	= serio_resume,
> +	.poweroff	= serio_poweroff,
> +	.restore	= serio_restore,
>  };
>  #endif /* CONFIG_PM */
>  

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Input: enable i8042-level wakeup control
  2011-03-11  9:01 ` Andres Salomon
@ 2011-03-11  9:33   ` Daniel Drake
  2011-03-11  9:41     ` Andres Salomon
  0 siblings, 1 reply; 7+ messages in thread
From: Daniel Drake @ 2011-03-11  9:33 UTC (permalink / raw)
  To: Andres Salomon; +Cc: dmitry.torokhov, dtor, linux-input, rjw

On 11 March 2011 09:01, Andres Salomon <dilinger@queued.net> wrote:
> What exactly is the purpose of this?   I see this allows various
> architectures to provide an override here, but it seems like this
> functionality wasn't previously present (they all just used
> i8042_pm_reset).  Do you have a later patch that makes use of this or
> something?

Yes, it was mentioned in the original submission:
http://dev.laptop.org/~dsd/20110114/0015-i8042-Enable-OLPC-s-EC-based-i8042-wakeup-control.patch

Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Input: enable i8042-level wakeup control
  2011-03-11  9:33   ` Daniel Drake
@ 2011-03-11  9:41     ` Andres Salomon
  0 siblings, 0 replies; 7+ messages in thread
From: Andres Salomon @ 2011-03-11  9:41 UTC (permalink / raw)
  To: Daniel Drake; +Cc: dmitry.torokhov, dtor, linux-input, rjw

On Fri, 11 Mar 2011 09:33:42 +0000
Daniel Drake <dsd@laptop.org> wrote:

> On 11 March 2011 09:01, Andres Salomon <dilinger@queued.net> wrote:
> > What exactly is the purpose of this?   I see this allows various
> > architectures to provide an override here, but it seems like this
> > functionality wasn't previously present (they all just used
> > i8042_pm_reset).  Do you have a later patch that makes use of this
> > or something?
> 
> Yes, it was mentioned in the original submission:
> http://dev.laptop.org/~dsd/20110114/0015-i8042-Enable-OLPC-s-EC-based-i8042-wakeup-control.patch
> 
> Daniel

Ah, right, and that handles enabling for OLPC as well.  Looks good to
me.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2011-03-11  9:41 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-02-10 17:16 [PATCH] Input: enable i8042-level wakeup control Daniel Drake
2011-03-10 20:36 ` Andres Salomon
2011-03-11  8:31   ` Dmitry Torokhov
2011-03-11  8:42     ` Daniel Drake
2011-03-11  9:01 ` Andres Salomon
2011-03-11  9:33   ` Daniel Drake
2011-03-11  9:41     ` Andres Salomon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).