On Wed, 28-Apr-10 11:53 PM +0530, Kevin Hilman wrote: > Ranjith Lohithakshan <ranjithl@xxxxxx> writes: > >> Kevin, >> >> On Tue, 27-Apr-10 8:46 PM +0530, Kevin Hilman wrote: >>> Ranjith Lohithakshan <ranjithl@xxxxxx> 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@xxxxxxxxxxxxxxxxxxx> > 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@xxxxxxxxxxxxxxxxxxx> > > 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) > > > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
