[PATCH v3 12/13] gpio/omap: fix incorrect context restore logic in omap_gpio_runtime_resume
From: DebBarma, Tarun Kanti <hidden>
Date: 2012-03-09 09:25:18
Also in:
linux-omap
On Thu, Mar 8, 2012 at 12:49 PM, Shilimkar, Santosh [off-list ref] wrote:
On Thu, Mar 8, 2012 at 4:58 AM, DebBarma, Tarun Kanti [off-list ref] wrote:quoted
On Wed, Mar 7, 2012 at 5:37 PM, Santosh Shilimkar [off-list ref] wrote:quoted
On Wednesday 07 March 2012 12:16 PM, Tarun Kanti DebBarma wrote:quoted
In omap_gpio_runtime_resume() the context restore should be independent of bank->enabled_non_wakeup_gpios. This was preventing context restore of GPIO lines which are not wakeup enabled. Reported-by: Govindraj Raja <redacted> Signed-off-by: Tarun Kanti DebBarma <redacted> --- ?drivers/gpio/gpio-omap.c | ? ?2 +- ?1 files changed, 1 insertions(+), 1 deletions(-)diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index 2e8e476..ccfbae0 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c@@ -1227,7 +1227,7 @@ static int omap_gpio_runtime_resume(struct device *dev)? ? ? __raw_writel(bank->context.risingdetect, ? ? ? ? ? ? ? ? ? ?bank->base + bank->regs->risingdetect); - ? ? if (!bank->enabled_non_wakeup_gpios || !bank->workaround_enabled) { + ? ? if (!bank->workaround_enabled) {This doesn't seem to be right. Don't you want to avoid GPIO restore for banks which are in always on domain. Infact the purpose of "enabled_non_wakeup_gpios" is exactly that ? Isn't it. What am I missing ?The bank->enabled_non_wakeup_gpios is set whenever a gpio line is programmed as edge trigger as shown below. (This is not meant to distinguish between gpios in WKUP domain vs those in PER domain). The context restore should happen irrespective of whether the trigger type is edge or level. In fact context restore was not happening for a gpio line because of this condition while testing suspend/resume. [...] ? ? ? ? ? ? ? ?if (trigger & IRQ_TYPE_EDGE_BOTH) ? ? ? ? ? ? ? ? ? ? ? ?bank->enabled_non_wakeup_gpios |= gpio_bit; ? ? ? ? ? ? ? ?else ? ? ? ? ? ? ? ? ? ? ? ?bank->enabled_non_wakeup_gpios &= ~gpio_bit; [...]Make sense now. Thanks for clarification Tarun. You can add mine.. Reviewed-by: Santosh Shilimkar <redacted>
Thanks. I have missed removing the same change from omap_gpio_runtime_suspend().
@@ -1178,9 +1178,6 @@ static int omap_gpio_runtime_suspend(struct device *dev) * non-wakeup GPIOs. Otherwise spurious IRQs will be * generated. See OMAP2420 Errata item 1.101. */ - if (!(bank->enabled_non_wakeup_gpios)) - goto update_gpio_context_count; -
FYI, I tested the change on OMAP5 code-base which did not have the above change. Anyways, I have updated the change in: git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev for_3.4/gpio_further_cleanup_fixes -- Tarun
Regards Santosh