Thread (26 messages) 26 messages, 2 authors, 2012-03-09
DORMANTno replies

[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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help