Ranjith Lohithakshan <ranjithl@xxxxxx> writes: > 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. 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 -- 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