[PATCH] OMAP3EVM: Update pad configuration for wakeup enabled pads

[PATCH] OMAP3EVM: Update pad configuration for wakeup enabled pads

[Ranjith Lohithakshan]

OMAP3530 TRM section 7.4.4.4.2 requires OFFOUTENABLE to be set (active low)
if wakeup capabilities are enabled on a pad. During OFF mode testing
on OMAP3530 EVM, it was observed that the device was not residing in
the OFF state. The device enters into the OFF state and immediately exits
from that state as if an IO wakeup event has occured. The issue was traced
down to the pad configuration of wkaeup enabled pad's.

Also, the pad configuration is included only if the respective drivers are
enabled in the defconfig.

Signed-off-by: Ranjith Lohithakshan <ranjithl@ti.com>
---
 arch/arm/mach-omap2/board-omap3evm.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
index 017bb2f..ce66ef0 100644
--- a/arch/arm/mach-omap2/board-omap3evm.c
+++ b/arch/arm/mach-omap2/board-omap3evm.c
@@ -650,12 +650,16 @@ static struct ehci_hcd_omap_platform_data ehci_pdata __initdata = {
 
 #ifdef CONFIG_OMAP_MUX
 static struct omap_board_mux board_mux[] __initdata = {
+#ifdef CONFIG_KEYBOARD_TWL4030
 	OMAP3_MUX(SYS_NIRQ, OMAP_MUX_MODE0 | OMAP_PIN_INPUT_PULLUP |
-				OMAP_PIN_OFF_INPUT_PULLUP |
+				OMAP_PIN_OFF_INPUT_PULLUP | OMAP_PIN_OFF_OUTPUT_LOW |
 				OMAP_PIN_OFF_WAKEUPENABLE),
+#endif
+#ifdef CONFIG_TOUCHSCREEN_ADS7846
 	OMAP3_MUX(MCSPI1_CS1, OMAP_MUX_MODE4 | OMAP_PIN_INPUT_PULLUP |
-				OMAP_PIN_OFF_INPUT_PULLUP |
+				OMAP_PIN_OFF_INPUT_PULLUP | OMAP_PIN_OFF_OUTPUT_LOW |
 				OMAP_PIN_OFF_WAKEUPENABLE),
+#endif
 	{ .reg_offset = OMAP_MUX_TERMINATOR },
 };
 #else
-- 
1.6.2.4

Re: [PATCH] OMAP3EVM: Update pad configuration for wakeup enabled pads

[Kevin Hilman]

Ranjith Lohithakshan <ranjithl@ti.com> writes:

> OMAP3530 TRM section 7.4.4.4.2 requires OFFOUTENABLE to be set (active low)
> if wakeup capabilities are enabled on a pad. During OFF mode testing
> on OMAP3530 EVM, it was observed that the device was not residing in
> the OFF state. The device enters into the OFF state and immediately exits
> from that state as if an IO wakeup event has occured. The issue was traced
> down to the pad configuration of wkaeup enabled pad's.

Nice.

> Also, the pad configuration is included only if the respective drivers are
> enabled in the defconfig.

Hmm, do you really want this?  If you don't have the driver enabled,
you have to rely on the bootloader settings of these pads which may
also be wrong and trigger an IO wakeup as well.

Kevin


> Signed-off-by: Ranjith Lohithakshan <ranjithl@ti.com>
> ---
>  arch/arm/mach-omap2/board-omap3evm.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
> index 017bb2f..ce66ef0 100644
> --- a/arch/arm/mach-omap2/board-omap3evm.c
> +++ b/arch/arm/mach-omap2/board-omap3evm.c
> @@ -650,12 +650,16 @@ static struct ehci_hcd_omap_platform_data ehci_pdata __initdata = {
>  
>  #ifdef CONFIG_OMAP_MUX
>  static struct omap_board_mux board_mux[] __initdata = {
> +#ifdef CONFIG_KEYBOARD_TWL4030
>  	OMAP3_MUX(SYS_NIRQ, OMAP_MUX_MODE0 | OMAP_PIN_INPUT_PULLUP |
> -				OMAP_PIN_OFF_INPUT_PULLUP |
> +				OMAP_PIN_OFF_INPUT_PULLUP | OMAP_PIN_OFF_OUTPUT_LOW |
>  				OMAP_PIN_OFF_WAKEUPENABLE),
> +#endif
> +#ifdef CONFIG_TOUCHSCREEN_ADS7846
>  	OMAP3_MUX(MCSPI1_CS1, OMAP_MUX_MODE4 | OMAP_PIN_INPUT_PULLUP |
> -				OMAP_PIN_OFF_INPUT_PULLUP |
> +				OMAP_PIN_OFF_INPUT_PULLUP | OMAP_PIN_OFF_OUTPUT_LOW |
>  				OMAP_PIN_OFF_WAKEUPENABLE),
> +#endif
>  	{ .reg_offset = OMAP_MUX_TERMINATOR },
>  };
>  #else
> -- 
> 1.6.2.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] OMAP3EVM: Update pad configuration for wakeup enabled pads

[Ranjith Lohithakshan]

Kevin,

On Tue, 27-Apr-10 8:46 PM +0530, Kevin Hilman wrote:
> Ranjith Lohithakshan <ranjithl@ti.com> writes:
> 
>> OMAP3530 TRM section 7.4.4.4.2 requires OFFOUTENABLE to be set (active low)
>> if wakeup capabilities are enabled on a pad. During OFF mode testing
>> on OMAP3530 EVM, it was observed that the device was not residing in
>> the OFF state. The device enters into the OFF state and immediately exits
>> from that state as if an IO wakeup event has occured. The issue was traced
>> down to the pad configuration of wkaeup enabled pad's.
> 
> Nice.
> 
>> Also, the pad configuration is included only if the respective drivers are
>> enabled in the defconfig.
> 
> Hmm, do you really want this?  If you don't have the driver enabled,
> you have to rely on the bootloader settings of these pads which may
> also be wrong and trigger an IO wakeup as well.

The thought process was that, for example, if keypad is not enabled in a
system configuration you probably don't want to see a wakeup occurring
if someone presses a key stroke. I understand the concern that you have
raised regarding bootloader mis-configurations. My impression was that
the bootloaders typically set the mux modes and pull up's/downs and dont
really program or enable the wakeup capability. But we cannot depend on
that thumb rule. What is your recommendation?

> 
>> Signed-off-by: Ranjith Lohithakshan <ranjithl@ti.com>
>> ---
>>  arch/arm/mach-omap2/board-omap3evm.c |    8 ++++++--
>>  1 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
>> index 017bb2f..ce66ef0 100644
>> --- a/arch/arm/mach-omap2/board-omap3evm.c
>> +++ b/arch/arm/mach-omap2/board-omap3evm.c
>> @@ -650,12 +650,16 @@ static struct ehci_hcd_omap_platform_data ehci_pdata __initdata = {
>>  
>>  #ifdef CONFIG_OMAP_MUX
>>  static struct omap_board_mux board_mux[] __initdata = {
>> +#ifdef CONFIG_KEYBOARD_TWL4030
>>  	OMAP3_MUX(SYS_NIRQ, OMAP_MUX_MODE0 | OMAP_PIN_INPUT_PULLUP |
>> -				OMAP_PIN_OFF_INPUT_PULLUP |
>> +				OMAP_PIN_OFF_INPUT_PULLUP | OMAP_PIN_OFF_OUTPUT_LOW |
>>  				OMAP_PIN_OFF_WAKEUPENABLE),
>> +#endif
>> +#ifdef CONFIG_TOUCHSCREEN_ADS7846
>>  	OMAP3_MUX(MCSPI1_CS1, OMAP_MUX_MODE4 | OMAP_PIN_INPUT_PULLUP |
>> -				OMAP_PIN_OFF_INPUT_PULLUP |
>> +				OMAP_PIN_OFF_INPUT_PULLUP | OMAP_PIN_OFF_OUTPUT_LOW |
>>  				OMAP_PIN_OFF_WAKEUPENABLE),
>> +#endif
>>  	{ .reg_offset = OMAP_MUX_TERMINATOR },
>>  };
>>  #else
>> -- 
>> 1.6.2.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] OMAP3EVM: Update pad configuration for wakeup enabled pads

[Kevin Hilman]

Ranjith Lohithakshan <ranjithl@ti.com> writes:

> Kevin,
>
> On Tue, 27-Apr-10 8:46 PM +0530, Kevin Hilman wrote:
>> Ranjith Lohithakshan <ranjithl@ti.com> writes:
>> 
>>> OMAP3530 TRM section 7.4.4.4.2 requires OFFOUTENABLE to be set (active low)
>>> if wakeup capabilities are enabled on a pad. During OFF mode testing
>>> on OMAP3530 EVM, it was observed that the device was not residing in
>>> the OFF state. The device enters into the OFF state and immediately exits
>>> from that state as if an IO wakeup event has occured. The issue was traced
>>> down to the pad configuration of wkaeup enabled pad's.
>> 
>> Nice.
>> 
>>> Also, the pad configuration is included only if the respective drivers are
>>> enabled in the defconfig.
>> 
>> Hmm, do you really want this?  If you don't have the driver enabled,
>> you have to rely on the bootloader settings of these pads which may
>> also be wrong and trigger an IO wakeup as well.
>
> The thought process was that, for example, if keypad is not enabled
> in a system configuration you probably don't want to see a wakeup
> occurring if someone presses a key stroke. I understand the concern
> that you have raised regarding bootloader mis-configurations. My
> impression was that the bootloaders typically set the mux modes and
> pull up's/downs and dont really program or enable the wakeup
> capability.  But we cannot depend on that thumb rule.

Unfortunately, Bootloaders don't "typically" do anything.  They are
routinely hacked/patched and cannot be trusted at all.

> What is your recommendation?

First, I suggest you fix the OFFOUTENABLE bug in a single patch
without introducing the #ifdefs.  Then, address the enable/disable of
the wakeups in a separate patch.

Next, ideally wakeups should not be configured a this level of board
code.  There are APIs for that: enable_irq_wake()/disable_irq_wake()

For GPIOs (like the touchscreen), you really need to enable wakeups
using existing APIs, either in the driver or in board init code and be
sure there is an interrupt handler.  Please see the 'Known Problems'
section of the OMAP PM wiki[2] where it talks about GPIO wakeups.
Below[2] is an test patch I've used.

For MPU IRQs, the IRQ wake APIs don't quite work as expected.  You
can look at board-3430sdp.c to see how wakeups are enabled there.  
If you want to make that conditional on the T2 driver being installed,
it could be part of the T2 init process.

Kevin

[1] http://elinux.org/OMAP_Power_Management#Known_Problems

[2]
commit a12a1b43c75795fa106d1222730591354209c037
Author: Kevin Hilman <khilman@deeprootsystems.com>
Date:   Wed Sep 9 11:58:20 2009 -0700

    OMAP3: PM: Enable touchscreen GPIO wakeups on SDP and omap3evm
    
    For the GPIO wakeup to work, the GPIO IRQ must be configured as a
    wakeup IRQ.
    
    In addition, a corresponding interrupt handler must be used and enabled
    so that after coming out of idle/suspend the interrupt will be cleared.
    Otherwise, a pending GPIO IRQ will prevent future idle request. (c.f.
    GPIO 'Interrupt and Wakeup' section of the TRM, specifically the
    subsection 'Involved Configuration Registers'.)
    
    Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>

diff --git a/arch/arm/mach-omap2/board-3430sdp.c b/arch/arm/mach-omap2/board-3430sdp.c
index c1742d0..96f921e 100644
--- a/arch/arm/mach-omap2/board-3430sdp.c
+++ b/arch/arm/mach-omap2/board-3430sdp.c
@@ -24,6 +24,8 @@
 #include <linux/regulator/machine.h>
 #include <linux/io.h>
 #include <linux/gpio.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
 
 #include <mach/hardware.h>
 #include <asm/mach-types.h>
@@ -140,6 +142,11 @@ static struct twl4030_keypad_data sdp3430_kp_data = {
 
 static int ts_gpio;	/* Needed for ads7846_get_pendown_state */
 
+static irqreturn_t dummy_interrupt_handler (int irq, void *dev_id)
+{
+	return IRQ_HANDLED;
+}
+
 /**
  * @brief ads7846_dev_init : Requests & sets GPIO line for pen-irq
  *
@@ -147,6 +154,8 @@ static int ts_gpio;	/* Needed for ads7846_get_pendown_state */
  */
 static void ads7846_dev_init(void)
 {
+	int ret;
+
 	if (gpio_request(ts_gpio, "ADS7846 pendown") < 0) {
 		printk(KERN_ERR "can't get ads746 pen down GPIO\n");
 		return;
@@ -156,6 +165,12 @@ static void ads7846_dev_init(void)
 
 	omap_set_gpio_debounce(ts_gpio, 1);
 	omap_set_gpio_debounce_time(ts_gpio, 0xa);
+
+	ret = request_irq(gpio_to_irq(ts_gpio),
+			  (irq_handler_t)dummy_interrupt_handler,
+			  IRQF_TRIGGER_FALLING,
+			  "ads7846-dummy", NULL);
+	enable_irq_wake(gpio_to_irq(ts_gpio));
 }
 
 static int ads7846_get_pendown_state(void)
diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
index 9ac1eb2..1647440 100644
--- a/arch/arm/mach-omap2/board-omap3evm.c
+++ b/arch/arm/mach-omap2/board-omap3evm.c
@@ -574,8 +574,15 @@ static int __init omap3_evm_i2c_init(void)
 	return 0;
 }
 
+static irqreturn_t dummy_interrupt_handler (int irq, void *dev_id)
+{
+	return IRQ_HANDLED;
+}
+
 static void ads7846_dev_init(void)
 {
+	int ret;
+
 	if (gpio_request(OMAP3_EVM_TS_GPIO, "ADS7846 pendown") < 0)
 		printk(KERN_ERR "can't get ads7846 pen down GPIO\n");
 
@@ -583,6 +590,12 @@ static void ads7846_dev_init(void)
 
 	omap_set_gpio_debounce(OMAP3_EVM_TS_GPIO, 1);
 	omap_set_gpio_debounce_time(OMAP3_EVM_TS_GPIO, 0xa);
+
+	ret = request_irq(gpio_to_irq(OMAP3_EVM_TS_GPIO),
+			  (irq_handler_t)dummy_interrupt_handler,
+			  IRQF_TRIGGER_FALLING,
+			  "ads7846-dummy", NULL);
+	enable_irq_wake(gpio_to_irq(OMAP3_EVM_TS_GPIO));
 }
 
 static int ads7846_get_pendown_state(void)

Re: [PATCH] OMAP3EVM: Update pad configuration for wakeup enabled pads

[Ranjith Lohithakshan]

On Wed, 28-Apr-10 11:53 PM +0530, Kevin Hilman wrote:
> Ranjith Lohithakshan <ranjithl@ti.com> writes:
> 
>> Kevin,
>>
>> On Tue, 27-Apr-10 8:46 PM +0530, Kevin Hilman wrote:
>>> Ranjith Lohithakshan <ranjithl@ti.com> writes:
>>>
>>>> OMAP3530 TRM section 7.4.4.4.2 requires OFFOUTENABLE to be set (active low)
>>>> if wakeup capabilities are enabled on a pad. During OFF mode testing
>>>> on OMAP3530 EVM, it was observed that the device was not residing in
>>>> the OFF state. The device enters into the OFF state and immediately exits
>>>> from that state as if an IO wakeup event has occured. The issue was traced
>>>> down to the pad configuration of wkaeup enabled pad's.
>>> Nice.
>>>
>>>> Also, the pad configuration is included only if the respective drivers are
>>>> enabled in the defconfig.
>>> Hmm, do you really want this?  If you don't have the driver enabled,
>>> you have to rely on the bootloader settings of these pads which may
>>> also be wrong and trigger an IO wakeup as well.
>> The thought process was that, for example, if keypad is not enabled
>> in a system configuration you probably don't want to see a wakeup
>> occurring if someone presses a key stroke. I understand the concern
>> that you have raised regarding bootloader mis-configurations. My
>> impression was that the bootloaders typically set the mux modes and
>> pull up's/downs and dont really program or enable the wakeup
>> capability.  But we cannot depend on that thumb rule.
> 
> Unfortunately, Bootloaders don't "typically" do anything.  They are
> routinely hacked/patched and cannot be trusted at all.
> 
>> What is your recommendation?
> 
> First, I suggest you fix the OFFOUTENABLE bug in a single patch
> without introducing the #ifdefs.  Then, address the enable/disable of
> the wakeups in a separate patch.

I will do that.

> Next, ideally wakeups should not be configured a this level of board
> code.  There are APIs for that: enable_irq_wake()/disable_irq_wake()
> 
> For GPIOs (like the touchscreen), you really need to enable wakeups
> using existing APIs, either in the driver or in board init code and be
> sure there is an interrupt handler.  Please see the 'Known Problems'
> section of the OMAP PM wiki[2] where it talks about GPIO wakeups.
> Below[2] is an test patch I've used.

I have pushed a patch on ads7846 to linux-input some time ago adding
wakeup support.

fdba2bb : Input: ads7846 - add wakeup support

There is now a wakeup flag added to the ads7846 platform data which can
enabled at the board level. Once this is set , the driver will do an
enable_irq_wake. The patch is now accepted and in mainline. I will
remove the wakeup mux configuration from the board file and instead will
just set the wakeup flag in the ads7846 platform data.

The keypad uses a pin in the non-gpio mode. Is enable_irq_wake supported
for non-gpio mode?

> For MPU IRQs, the IRQ wake APIs don't quite work as expected.  You
> can look at board-3430sdp.c to see how wakeups are enabled there.  
> If you want to make that conditional on the T2 driver being installed,
> it could be part of the T2 init process.
> 

Thanks for the pointers.

> 
> [1] http://elinux.org/OMAP_Power_Management#Known_Problems
> 
> [2]
> commit a12a1b43c75795fa106d1222730591354209c037
> Author: Kevin Hilman <khilman@deeprootsystems.com>
> Date:   Wed Sep 9 11:58:20 2009 -0700
> 
>     OMAP3: PM: Enable touchscreen GPIO wakeups on SDP and omap3evm
>     
>     For the GPIO wakeup to work, the GPIO IRQ must be configured as a
>     wakeup IRQ.
>     
>     In addition, a corresponding interrupt handler must be used and enabled
>     so that after coming out of idle/suspend the interrupt will be cleared.
>     Otherwise, a pending GPIO IRQ will prevent future idle request. (c.f.
>     GPIO 'Interrupt and Wakeup' section of the TRM, specifically the
>     subsection 'Involved Configuration Registers'.)
>     
>     Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
> 
> diff --git a/arch/arm/mach-omap2/board-3430sdp.c b/arch/arm/mach-omap2/board-3430sdp.c
> index c1742d0..96f921e 100644
> --- a/arch/arm/mach-omap2/board-3430sdp.c
> +++ b/arch/arm/mach-omap2/board-3430sdp.c
> @@ -24,6 +24,8 @@
>  #include <linux/regulator/machine.h>
>  #include <linux/io.h>
>  #include <linux/gpio.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
>  
>  #include <mach/hardware.h>
>  #include <asm/mach-types.h>
> @@ -140,6 +142,11 @@ static struct twl4030_keypad_data sdp3430_kp_data = {
>  
>  static int ts_gpio;	/* Needed for ads7846_get_pendown_state */
>  
> +static irqreturn_t dummy_interrupt_handler (int irq, void *dev_id)
> +{
> +	return IRQ_HANDLED;
> +}
> +
>  /**
>   * @brief ads7846_dev_init : Requests & sets GPIO line for pen-irq
>   *
> @@ -147,6 +154,8 @@ static int ts_gpio;	/* Needed for ads7846_get_pendown_state */
>   */
>  static void ads7846_dev_init(void)
>  {
> +	int ret;
> +
>  	if (gpio_request(ts_gpio, "ADS7846 pendown") < 0) {
>  		printk(KERN_ERR "can't get ads746 pen down GPIO\n");
>  		return;
> @@ -156,6 +165,12 @@ static void ads7846_dev_init(void)
>  
>  	omap_set_gpio_debounce(ts_gpio, 1);
>  	omap_set_gpio_debounce_time(ts_gpio, 0xa);
> +
> +	ret = request_irq(gpio_to_irq(ts_gpio),
> +			  (irq_handler_t)dummy_interrupt_handler,
> +			  IRQF_TRIGGER_FALLING,
> +			  "ads7846-dummy", NULL);
> +	enable_irq_wake(gpio_to_irq(ts_gpio));
>  }
>  
>  static int ads7846_get_pendown_state(void)
> diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach-omap2/board-omap3evm.c
> index 9ac1eb2..1647440 100644
> --- a/arch/arm/mach-omap2/board-omap3evm.c
> +++ b/arch/arm/mach-omap2/board-omap3evm.c
> @@ -574,8 +574,15 @@ static int __init omap3_evm_i2c_init(void)
>  	return 0;
>  }
>  
> +static irqreturn_t dummy_interrupt_handler (int irq, void *dev_id)
> +{
> +	return IRQ_HANDLED;
> +}
> +
>  static void ads7846_dev_init(void)
>  {
> +	int ret;
> +
>  	if (gpio_request(OMAP3_EVM_TS_GPIO, "ADS7846 pendown") < 0)
>  		printk(KERN_ERR "can't get ads7846 pen down GPIO\n");
>  
> @@ -583,6 +590,12 @@ static void ads7846_dev_init(void)
>  
>  	omap_set_gpio_debounce(OMAP3_EVM_TS_GPIO, 1);
>  	omap_set_gpio_debounce_time(OMAP3_EVM_TS_GPIO, 0xa);
> +
> +	ret = request_irq(gpio_to_irq(OMAP3_EVM_TS_GPIO),
> +			  (irq_handler_t)dummy_interrupt_handler,
> +			  IRQF_TRIGGER_FALLING,
> +			  "ads7846-dummy", NULL);
> +	enable_irq_wake(gpio_to_irq(OMAP3_EVM_TS_GPIO));
>  }
>  
>  static int ads7846_get_pendown_state(void)
> 
> 
> 

Re: [PATCH] OMAP3EVM: Update pad configuration for wakeup enabled pads

[Kevin Hilman]

Ranjith Lohithakshan <ranjithl@ti.com> writes:

> On Wed, 28-Apr-10 11:53 PM +0530, Kevin Hilman wrote:
>> Ranjith Lohithakshan <ranjithl@ti.com> writes:
>> 
>>> Kevin,
>>>
>>> On Tue, 27-Apr-10 8:46 PM +0530, Kevin Hilman wrote:
>>>> Ranjith Lohithakshan <ranjithl@ti.com> writes:
>>>>
>>>>> OMAP3530 TRM section 7.4.4.4.2 requires OFFOUTENABLE to be set (active low)
>>>>> if wakeup capabilities are enabled on a pad. During OFF mode testing
>>>>> on OMAP3530 EVM, it was observed that the device was not residing in
>>>>> the OFF state. The device enters into the OFF state and immediately exits
>>>>> from that state as if an IO wakeup event has occured. The issue was traced
>>>>> down to the pad configuration of wkaeup enabled pad's.
>>>> Nice.
>>>>
>>>>> Also, the pad configuration is included only if the respective drivers are
>>>>> enabled in the defconfig.
>>>> Hmm, do you really want this?  If you don't have the driver enabled,
>>>> you have to rely on the bootloader settings of these pads which may
>>>> also be wrong and trigger an IO wakeup as well.
>>> The thought process was that, for example, if keypad is not enabled
>>> in a system configuration you probably don't want to see a wakeup
>>> occurring if someone presses a key stroke. I understand the concern
>>> that you have raised regarding bootloader mis-configurations. My
>>> impression was that the bootloaders typically set the mux modes and
>>> pull up's/downs and dont really program or enable the wakeup
>>> capability.  But we cannot depend on that thumb rule.
>> 
>> Unfortunately, Bootloaders don't "typically" do anything.  They are
>> routinely hacked/patched and cannot be trusted at all.
>> 
>>> What is your recommendation?
>> 
>> First, I suggest you fix the OFFOUTENABLE bug in a single patch
>> without introducing the #ifdefs.  Then, address the enable/disable of
>> the wakeups in a separate patch.
>
> I will do that.
>
>> Next, ideally wakeups should not be configured a this level of board
>> code.  There are APIs for that: enable_irq_wake()/disable_irq_wake()
>> 
>> For GPIOs (like the touchscreen), you really need to enable wakeups
>> using existing APIs, either in the driver or in board init code and be
>> sure there is an interrupt handler.  Please see the 'Known Problems'
>> section of the OMAP PM wiki[2] where it talks about GPIO wakeups.
>> Below[2] is an test patch I've used.
>
> I have pushed a patch on ads7846 to linux-input some time ago adding
> wakeup support.
>
> fdba2bb : Input: ads7846 - add wakeup support
>
> There is now a wakeup flag added to the ads7846 platform data which can
> enabled at the board level. Once this is set , the driver will do an
> enable_irq_wake. The patch is now accepted and in mainline. I will
> remove the wakeup mux configuration from the board file and instead will
> just set the wakeup flag in the ads7846 platform data.

Brilliant!  you're several steps ahead of me.  Would you mind
submitting a patch to l-o to enable that for SDP and omap3evm so I
can drop my suggested patch?

> The keypad uses a pin in the non-gpio mode. Is enable_irq_wake supported
> for non-gpio mode?

Not currently, as enable_irq_wake() ends up calling into the
irq_chip's set_wake() method.  This then calls into the OMAP GPIO
layer as plat-omap/gpio.c:gpio_wake_enable()

For now, the keypad could just use the mux API at runtime like SDP does
to enable wakeups when needed.

Kevin

Re: [PATCH] OMAP3EVM: Update pad configuration for wakeup enabled pads

[Ranjith Lohithakshan]

On Thu, 29-Apr-10 7:41 PM +0530, Kevin Hilman wrote:
> Ranjith Lohithakshan <ranjithl@ti.com> writes:
> 
>> On Wed, 28-Apr-10 11:53 PM +0530, Kevin Hilman wrote:
>>> Ranjith Lohithakshan <ranjithl@ti.com> writes:
>>>
>>>> Kevin,
>>>>
>>>> On Tue, 27-Apr-10 8:46 PM +0530, Kevin Hilman wrote:
>>>>> Ranjith Lohithakshan <ranjithl@ti.com> writes:
>>>>>
>>>>>> OMAP3530 TRM section 7.4.4.4.2 requires OFFOUTENABLE to be set (active low)
>>>>>> if wakeup capabilities are enabled on a pad. During OFF mode testing
>>>>>> on OMAP3530 EVM, it was observed that the device was not residing in
>>>>>> the OFF state. The device enters into the OFF state and immediately exits
>>>>>> from that state as if an IO wakeup event has occured. The issue was traced
>>>>>> down to the pad configuration of wkaeup enabled pad's.
>>>>> Nice.
>>>>>
>>>>>> Also, the pad configuration is included only if the respective drivers are
>>>>>> enabled in the defconfig.
>>>>> Hmm, do you really want this?  If you don't have the driver enabled,
>>>>> you have to rely on the bootloader settings of these pads which may
>>>>> also be wrong and trigger an IO wakeup as well.
>>>> The thought process was that, for example, if keypad is not enabled
>>>> in a system configuration you probably don't want to see a wakeup
>>>> occurring if someone presses a key stroke. I understand the concern
>>>> that you have raised regarding bootloader mis-configurations. My
>>>> impression was that the bootloaders typically set the mux modes and
>>>> pull up's/downs and dont really program or enable the wakeup
>>>> capability.  But we cannot depend on that thumb rule.
>>> Unfortunately, Bootloaders don't "typically" do anything.  They are
>>> routinely hacked/patched and cannot be trusted at all.
>>>
>>>> What is your recommendation?
>>> First, I suggest you fix the OFFOUTENABLE bug in a single patch
>>> without introducing the #ifdefs.  Then, address the enable/disable of
>>> the wakeups in a separate patch.
>> I will do that.
>>
>>> Next, ideally wakeups should not be configured a this level of board
>>> code.  There are APIs for that: enable_irq_wake()/disable_irq_wake()
>>>
>>> For GPIOs (like the touchscreen), you really need to enable wakeups
>>> using existing APIs, either in the driver or in board init code and be
>>> sure there is an interrupt handler.  Please see the 'Known Problems'
>>> section of the OMAP PM wiki[2] where it talks about GPIO wakeups.
>>> Below[2] is an test patch I've used.
>> I have pushed a patch on ads7846 to linux-input some time ago adding
>> wakeup support.
>>
>> fdba2bb : Input: ads7846 - add wakeup support
>>
>> There is now a wakeup flag added to the ads7846 platform data which can
>> enabled at the board level. Once this is set , the driver will do an
>> enable_irq_wake. The patch is now accepted and in mainline. I will
>> remove the wakeup mux configuration from the board file and instead will
>> just set the wakeup flag in the ads7846 platform data.

I will submit a patch with the necessary changes.

> Brilliant!  you're several steps ahead of me.  Would you mind
> submitting a patch to l-o to enable that for SDP and omap3evm so I
> can drop my suggested patch?
> 
>> The keypad uses a pin in the non-gpio mode. Is enable_irq_wake supported
>> for non-gpio mode?
> 
> Not currently, as enable_irq_wake() ends up calling into the
> irq_chip's set_wake() method.  This then calls into the OMAP GPIO
> layer as plat-omap/gpio.c:gpio_wake_enable()
> 
> For now, the keypad could just use the mux API at runtime like SDP does
> to enable wakeups when needed.
> 
> Kevin