[PATCH 0/8] gpio/omap: remaining cleanups and fix

[PATCH 0/8] gpio/omap: remaining cleanups and fix

[Tarun Kanti DebBarma]

Here are the remaining cleanup patches. There are broadly
two categories of cleanups.

Cat-1: Those missed while introducing new feature like SPARSE_IRQ
       handling and DT support; use edge/level handlers from
       generic IRQ framework.

Cat-2: Removal of redundant fields from struct gpio_bank{} as a
       result of they being already covered by members within
       context field of struct gpio_bank{}.

Reference: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
Commit: 66f75a5d028beaf67c931435fdc3e7823125730c (Linux 3.4-rc4)

Series is available for reference here:
git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev.git for_3.5/gpio_more_cleanup_fixes

Test info:
OMAP2+ platforms: OMAP2430SDP, OMAP3430SDP, OMAP4430SDP
OMAP1: Bootup test on OMAP1710SDP.

Cc: Kevin Hilman <khilman@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Cousson, Benoit <b-cousson@ti.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Tarun Kanti DebBarma (8):
  gpio/omap: remove virtual_irq_start variable
  gpio/omap: remove saved_fallingdetect, saved_risingdetect
  gpio/omap: remove suspend_wakeup field from struct gpio_bank
  gpio/omap: remove saved_wakeup field from struct gpio_bank
  gpio/omap: remove retrigger variable in gpio_irq_handler
  gpio/omap: remove suspend/resume callbacks
  gpio/omap: remove cpu_is_omapxxxx() checks from *_runtime_resume()
  gpio/omap: fix missing check in *_runtime_suspend()

 arch/arm/mach-omap1/gpio15xx.c         |    2 -
 arch/arm/mach-omap1/gpio16xx.c         |    5 --
 arch/arm/mach-omap1/gpio7xx.c          |    7 --
 arch/arm/mach-omap2/gpio.c             |    3 +-
 arch/arm/plat-omap/include/plat/gpio.h |    3 +-
 drivers/gpio/gpio-omap.c               |  103 +++++++-------------------------
 6 files changed, 27 insertions(+), 96 deletions(-)

[PATCH 1/8] gpio/omap: remove virtual_irq_start variable

[Tarun Kanti DebBarma]

This cleanup got missed while implementing following:
25db711 gpio/omap: Fix IRQ handling for SPARSE_IRQ
384ebe1 gpio/omap: Add DT support to GPIO driver

Cc: Kevin Hilman <khilman@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Cousson, Benoit <b-cousson@ti.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
---
 arch/arm/mach-omap1/gpio15xx.c         |    2 --
 arch/arm/mach-omap1/gpio16xx.c         |    5 -----
 arch/arm/mach-omap1/gpio7xx.c          |    7 -------
 arch/arm/mach-omap2/gpio.c             |    1 -
 arch/arm/plat-omap/include/plat/gpio.h |    1 -
 5 files changed, 0 insertions(+), 16 deletions(-)

diff --git a/arch/arm/mach-omap1/gpio15xx.c b/arch/arm/mach-omap1/gpio15xx.c
index 634903e..ebef15e 100644
--- a/arch/arm/mach-omap1/gpio15xx.c
+++ b/arch/arm/mach-omap1/gpio15xx.c
@@ -46,7 +46,6 @@ static struct omap_gpio_reg_offs omap15xx_mpuio_regs = {
 };
 
 static struct __initdata omap_gpio_platform_data omap15xx_mpu_gpio_config = {
-	.virtual_irq_start	= IH_MPUIO_BASE,
 	.is_mpuio		= true,
 	.bank_width		= 16,
 	.bank_stride		= 1,
@@ -89,7 +88,6 @@ static struct omap_gpio_reg_offs omap15xx_gpio_regs = {
 };
 
 static struct __initdata omap_gpio_platform_data omap15xx_gpio_config = {
-	.virtual_irq_start	= IH_GPIO_BASE,
 	.bank_width		= 16,
 	.regs                   = &omap15xx_gpio_regs,
 };
diff --git a/arch/arm/mach-omap1/gpio16xx.c b/arch/arm/mach-omap1/gpio16xx.c
index 1fb3b9a..2a48cd2 100644
--- a/arch/arm/mach-omap1/gpio16xx.c
+++ b/arch/arm/mach-omap1/gpio16xx.c
@@ -52,7 +52,6 @@ static struct omap_gpio_reg_offs omap16xx_mpuio_regs = {
 };
 
 static struct __initdata omap_gpio_platform_data omap16xx_mpu_gpio_config = {
-	.virtual_irq_start	= IH_MPUIO_BASE,
 	.is_mpuio		= true,
 	.bank_width		= 16,
 	.bank_stride		= 1,
@@ -99,7 +98,6 @@ static struct omap_gpio_reg_offs omap16xx_gpio_regs = {
 };
 
 static struct __initdata omap_gpio_platform_data omap16xx_gpio1_config = {
-	.virtual_irq_start	= IH_GPIO_BASE,
 	.bank_width		= 16,
 	.regs                   = &omap16xx_gpio_regs,
 };
@@ -128,7 +126,6 @@ static struct __initdata resource omap16xx_gpio2_resources[] = {
 };
 
 static struct __initdata omap_gpio_platform_data omap16xx_gpio2_config = {
-	.virtual_irq_start	= IH_GPIO_BASE + 16,
 	.bank_width		= 16,
 	.regs                   = &omap16xx_gpio_regs,
 };
@@ -157,7 +154,6 @@ static struct __initdata resource omap16xx_gpio3_resources[] = {
 };
 
 static struct __initdata omap_gpio_platform_data omap16xx_gpio3_config = {
-	.virtual_irq_start	= IH_GPIO_BASE + 32,
 	.bank_width		= 16,
 	.regs                   = &omap16xx_gpio_regs,
 };
@@ -186,7 +182,6 @@ static struct __initdata resource omap16xx_gpio4_resources[] = {
 };
 
 static struct __initdata omap_gpio_platform_data omap16xx_gpio4_config = {
-	.virtual_irq_start	= IH_GPIO_BASE + 48,
 	.bank_width		= 16,
 	.regs                   = &omap16xx_gpio_regs,
 };
diff --git a/arch/arm/mach-omap1/gpio7xx.c b/arch/arm/mach-omap1/gpio7xx.c
index 4771d6b..acf12b7 100644
--- a/arch/arm/mach-omap1/gpio7xx.c
+++ b/arch/arm/mach-omap1/gpio7xx.c
@@ -51,7 +51,6 @@ static struct omap_gpio_reg_offs omap7xx_mpuio_regs = {
 };
 
 static struct __initdata omap_gpio_platform_data omap7xx_mpu_gpio_config = {
-	.virtual_irq_start	= IH_MPUIO_BASE,
 	.is_mpuio		= true,
 	.bank_width		= 16,
 	.bank_stride		= 2,
@@ -93,7 +92,6 @@ static struct omap_gpio_reg_offs omap7xx_gpio_regs = {
 };
 
 static struct __initdata omap_gpio_platform_data omap7xx_gpio1_config = {
-	.virtual_irq_start	= IH_GPIO_BASE,
 	.bank_width		= 32,
 	.regs			= &omap7xx_gpio_regs,
 };
@@ -122,7 +120,6 @@ static struct __initdata resource omap7xx_gpio2_resources[] = {
 };
 
 static struct __initdata omap_gpio_platform_data omap7xx_gpio2_config = {
-	.virtual_irq_start	= IH_GPIO_BASE + 32,
 	.bank_width		= 32,
 	.regs			= &omap7xx_gpio_regs,
 };
@@ -151,7 +148,6 @@ static struct __initdata resource omap7xx_gpio3_resources[] = {
 };
 
 static struct __initdata omap_gpio_platform_data omap7xx_gpio3_config = {
-	.virtual_irq_start	= IH_GPIO_BASE + 64,
 	.bank_width		= 32,
 	.regs			= &omap7xx_gpio_regs,
 };
@@ -180,7 +176,6 @@ static struct __initdata resource omap7xx_gpio4_resources[] = {
 };
 
 static struct __initdata omap_gpio_platform_data omap7xx_gpio4_config = {
-	.virtual_irq_start	= IH_GPIO_BASE + 96,
 	.bank_width		= 32,
 	.regs			= &omap7xx_gpio_regs,
 };
@@ -209,7 +204,6 @@ static struct __initdata resource omap7xx_gpio5_resources[] = {
 };
 
 static struct __initdata omap_gpio_platform_data omap7xx_gpio5_config = {
-	.virtual_irq_start	= IH_GPIO_BASE + 128,
 	.bank_width		= 32,
 	.regs			= &omap7xx_gpio_regs,
 };
@@ -238,7 +232,6 @@ static struct __initdata resource omap7xx_gpio6_resources[] = {
 };
 
 static struct __initdata omap_gpio_platform_data omap7xx_gpio6_config = {
-	.virtual_irq_start	= IH_GPIO_BASE + 160,
 	.bank_width		= 32,
 	.regs			= &omap7xx_gpio_regs,
 };
diff --git a/arch/arm/mach-omap2/gpio.c b/arch/arm/mach-omap2/gpio.c
index 2f994e5..86f91a6 100644
--- a/arch/arm/mach-omap2/gpio.c
+++ b/arch/arm/mach-omap2/gpio.c
@@ -55,7 +55,6 @@ static int __init omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused)
 	dev_attr = (struct omap_gpio_dev_attr *)oh->dev_attr;
 	pdata->bank_width = dev_attr->bank_width;
 	pdata->dbck_flag = dev_attr->dbck_flag;
-	pdata->virtual_irq_start = IH_GPIO_BASE + 32 * (id - 1);
 	pdata->get_context_loss_count = omap_pm_get_dev_context_loss_count;
 	pdata->regs = kzalloc(sizeof(struct omap_gpio_reg_offs), GFP_KERNEL);
 	if (!pdata) {
diff --git a/arch/arm/plat-omap/include/plat/gpio.h b/arch/arm/plat-omap/include/plat/gpio.h
index 2f6e992..d903e7d 100644
--- a/arch/arm/plat-omap/include/plat/gpio.h
+++ b/arch/arm/plat-omap/include/plat/gpio.h
@@ -193,7 +193,6 @@ struct omap_gpio_reg_offs {
 };
 
 struct omap_gpio_platform_data {
-	u16 virtual_irq_start;
 	int bank_type;
 	int bank_width;		/* GPIO bank width */
 	int bank_stride;	/* Only needed for omap1 MPUIO */
-- 
1.7.0.4

[PATCH 1/8] gpio/omap: remove virtual_irq_start variable

[Santosh Shilimkar]

On Friday 27 April 2012 07:43 PM, Tarun Kanti DebBarma wrote:
> This cleanup got missed while implementing following:
> 25db711 gpio/omap: Fix IRQ handling for SPARSE_IRQ
> 384ebe1 gpio/omap: Add DT support to GPIO driver
> 
> Cc: Kevin Hilman <khilman@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Cousson, Benoit <b-cousson@ti.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
> ---
>  arch/arm/mach-omap1/gpio15xx.c         |    2 --
>  arch/arm/mach-omap1/gpio16xx.c         |    5 -----
>  arch/arm/mach-omap1/gpio7xx.c          |    7 -------
>  arch/arm/mach-omap2/gpio.c             |    1 -
>  arch/arm/plat-omap/include/plat/gpio.h |    1 -
>  5 files changed, 0 insertions(+), 16 deletions(-)
> 
Looks good.
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

[PATCH 2/8] gpio/omap: remove saved_fallingdetect, saved_risingdetect

[Tarun Kanti DebBarma]

Since we already have context.fallingdetect and context.risingdetect
there is no more need to have these additional fields. Also, getting
rid of extra reads associated with them.

Cc: Kevin Hilman <khilman@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Cousson, Benoit <b-cousson@ti.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
 drivers/gpio/gpio-omap.c |   19 ++++++++-----------
 1 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 1adc2ec..1751f89 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -63,8 +63,6 @@ struct gpio_bank {
 	u32 enabled_non_wakeup_gpios;
 	struct gpio_regs context;
 	u32 saved_datain;
-	u32 saved_fallingdetect;
-	u32 saved_risingdetect;
 	u32 level_mask;
 	u32 toggle_mask;
 	spinlock_t lock;
@@ -1247,11 +1245,9 @@ static int omap_gpio_runtime_suspend(struct device *dev)
 	 */
 	bank->saved_datain = __raw_readl(bank->base +
 						bank->regs->datain);
-	l1 = __raw_readl(bank->base + bank->regs->fallingdetect);
-	l2 = __raw_readl(bank->base + bank->regs->risingdetect);
+	l1 = bank->context.fallingdetect;
+	l2 = bank->context.risingdetect;
 
-	bank->saved_fallingdetect = l1;
-	bank->saved_risingdetect = l2;
 	l1 &= ~bank->enabled_non_wakeup_gpios;
 	l2 &= ~bank->enabled_non_wakeup_gpios;
 
@@ -1310,9 +1306,9 @@ static int omap_gpio_runtime_resume(struct device *dev)
 		}
 	}
 
-	__raw_writel(bank->saved_fallingdetect,
+	__raw_writel(bank->context.fallingdetect,
 			bank->base + bank->regs->fallingdetect);
-	__raw_writel(bank->saved_risingdetect,
+	__raw_writel(bank->context.risingdetect,
 			bank->base + bank->regs->risingdetect);
 	l = __raw_readl(bank->base + bank->regs->datain);
 
@@ -1329,14 +1325,15 @@ static int omap_gpio_runtime_resume(struct device *dev)
 	 * No need to generate IRQs for the rising edge for gpio IRQs
 	 * configured with falling edge only; and vice versa.
 	 */
-	gen0 = l & bank->saved_fallingdetect;
+	gen0 = l & bank->context.fallingdetect;
 	gen0 &= bank->saved_datain;
 
-	gen1 = l & bank->saved_risingdetect;
+	gen1 = l & bank->context.risingdetect;
 	gen1 &= ~(bank->saved_datain);
 
 	/* FIXME: Consider GPIO IRQs with level detections properly! */
-	gen = l & (~(bank->saved_fallingdetect) & ~(bank->saved_risingdetect));
+	gen = l & (~(bank->context.fallingdetect) &
+					 ~(bank->context.risingdetect));
 	/* Consider all GPIO IRQs needed to be updated */
 	gen |= gen0 | gen1;
 
-- 
1.7.0.4

[PATCH 3/8] gpio/omap: remove suspend_wakeup field from struct gpio_bank

[Tarun Kanti DebBarma]

Since we already have bank->context.wake_en to keep track
of gpios which are wakeup enabled, there is no need to have
this field any more.

Cc: Kevin Hilman <khilman@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Cousson, Benoit <b-cousson@ti.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
 drivers/gpio/gpio-omap.c |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 1751f89..7b00256 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -57,7 +57,6 @@ struct gpio_bank {
 	u16 irq;
 	int irq_base;
 	struct irq_domain *domain;
-	u32 suspend_wakeup;
 	u32 saved_wakeup;
 	u32 non_wakeup_gpios;
 	u32 enabled_non_wakeup_gpios;
@@ -514,11 +513,11 @@ static int _set_gpio_wakeup(struct gpio_bank *bank, int gpio, int enable)
 
 	spin_lock_irqsave(&bank->lock, flags);
 	if (enable)
-		bank->suspend_wakeup |= gpio_bit;
+		bank->context.wake_en |= gpio_bit;
 	else
-		bank->suspend_wakeup &= ~gpio_bit;
+		bank->context.wake_en &= ~gpio_bit;
 
-	__raw_writel(bank->suspend_wakeup, bank->base + bank->regs->wkup_en);
+	__raw_writel(bank->context.wake_en, bank->base + bank->regs->wkup_en);
 	spin_unlock_irqrestore(&bank->lock, flags);
 
 	return 0;
@@ -788,7 +787,7 @@ static int omap_mpuio_suspend_noirq(struct device *dev)
 
 	spin_lock_irqsave(&bank->lock, flags);
 	bank->saved_wakeup = __raw_readl(mask_reg);
-	__raw_writel(0xffff & ~bank->suspend_wakeup, mask_reg);
+	__raw_writel(0xffff & ~bank->context.wake_en, mask_reg);
 	spin_unlock_irqrestore(&bank->lock, flags);
 
 	return 0;
@@ -1165,7 +1164,7 @@ static int omap_gpio_suspend(struct device *dev)
 	if (!bank->mod_usage || !bank->loses_context)
 		return 0;
 
-	if (!bank->regs->wkup_en || !bank->suspend_wakeup)
+	if (!bank->regs->wkup_en || !bank->context.wake_en)
 		return 0;
 
 	wakeup_enable = bank->base + bank->regs->wkup_en;
@@ -1173,7 +1172,7 @@ static int omap_gpio_suspend(struct device *dev)
 	spin_lock_irqsave(&bank->lock, flags);
 	bank->saved_wakeup = __raw_readl(wakeup_enable);
 	_gpio_rmw(base, bank->regs->wkup_en, 0xffffffff, 0);
-	_gpio_rmw(base, bank->regs->wkup_en, bank->suspend_wakeup, 1);
+	_gpio_rmw(base, bank->regs->wkup_en, bank->context.wake_en, 1);
 	spin_unlock_irqrestore(&bank->lock, flags);
 
 	return 0;
-- 
1.7.0.4

[PATCH 4/8] gpio/omap: remove saved_wakeup field from struct gpio_bank

[Tarun Kanti DebBarma]

There is no more need to have saved_wakeup because bank->context.wake_en
already holds that value. So getting rid of read/write operation associated
with this field.

Cc: Kevin Hilman <khilman@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Cousson, Benoit <b-cousson@ti.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
 drivers/gpio/gpio-omap.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 7b00256..cb9f6d9 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -57,7 +57,6 @@ struct gpio_bank {
 	u16 irq;
 	int irq_base;
 	struct irq_domain *domain;
-	u32 saved_wakeup;
 	u32 non_wakeup_gpios;
 	u32 enabled_non_wakeup_gpios;
 	struct gpio_regs context;
@@ -786,7 +785,6 @@ static int omap_mpuio_suspend_noirq(struct device *dev)
 	unsigned long		flags;
 
 	spin_lock_irqsave(&bank->lock, flags);
-	bank->saved_wakeup = __raw_readl(mask_reg);
 	__raw_writel(0xffff & ~bank->context.wake_en, mask_reg);
 	spin_unlock_irqrestore(&bank->lock, flags);
 
@@ -802,7 +800,7 @@ static int omap_mpuio_resume_noirq(struct device *dev)
 	unsigned long		flags;
 
 	spin_lock_irqsave(&bank->lock, flags);
-	__raw_writel(bank->saved_wakeup, mask_reg);
+	__raw_writel(bank->context.wake_en, mask_reg);
 	spin_unlock_irqrestore(&bank->lock, flags);
 
 	return 0;
@@ -1158,7 +1156,6 @@ static int omap_gpio_suspend(struct device *dev)
 	struct platform_device *pdev = to_platform_device(dev);
 	struct gpio_bank *bank = platform_get_drvdata(pdev);
 	void __iomem *base = bank->base;
-	void __iomem *wakeup_enable;
 	unsigned long flags;
 
 	if (!bank->mod_usage || !bank->loses_context)
@@ -1167,10 +1164,7 @@ static int omap_gpio_suspend(struct device *dev)
 	if (!bank->regs->wkup_en || !bank->context.wake_en)
 		return 0;
 
-	wakeup_enable = bank->base + bank->regs->wkup_en;
-
 	spin_lock_irqsave(&bank->lock, flags);
-	bank->saved_wakeup = __raw_readl(wakeup_enable);
 	_gpio_rmw(base, bank->regs->wkup_en, 0xffffffff, 0);
 	_gpio_rmw(base, bank->regs->wkup_en, bank->context.wake_en, 1);
 	spin_unlock_irqrestore(&bank->lock, flags);
@@ -1188,12 +1182,12 @@ static int omap_gpio_resume(struct device *dev)
 	if (!bank->mod_usage || !bank->loses_context)
 		return 0;
 
-	if (!bank->regs->wkup_en || !bank->saved_wakeup)
+	if (!bank->regs->wkup_en || !bank->context.wake_en)
 		return 0;
 
 	spin_lock_irqsave(&bank->lock, flags);
 	_gpio_rmw(base, bank->regs->wkup_en, 0xffffffff, 0);
-	_gpio_rmw(base, bank->regs->wkup_en, bank->saved_wakeup, 1);
+	_gpio_rmw(base, bank->regs->wkup_en, bank->context.wake_en, 1);
 	spin_unlock_irqrestore(&bank->lock, flags);
 
 	return 0;
-- 
1.7.0.4

[PATCH 5/8] gpio/omap: remove retrigger variable in gpio_irq_handler

[Tarun Kanti DebBarma]

commit 672e302e3c (ARM: OMAP: use edge/level handlers from generic IRQ
framework) removed retrigger support in favor of using generic IRQ
framework.  This patch cleans up some unused remnants of that removal.

Cc: Kevin Hilman <khilman@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Cousson, Benoit <b-cousson@ti.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
 drivers/gpio/gpio-omap.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index cb9f6d9..7533564 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -636,7 +636,6 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
 	u32 isr;
 	unsigned int gpio_irq, gpio_index;
 	struct gpio_bank *bank;
-	u32 retrigger = 0;
 	int unmasked = 0;
 	struct irq_chip *chip = irq_desc_get_chip(desc);
 
@@ -673,8 +672,6 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
 			chained_irq_exit(chip, desc);
 		}
 
-		isr |= retrigger;
-		retrigger = 0;
 		if (!isr)
 			break;
 
-- 
1.7.0.4

[PATCH 6/8] gpio/omap: remove suspend/resume callbacks

[Tarun Kanti DebBarma]

Both omap_gpio_suspend() and omap_gpio_resume() does programming
of wakeup_en register.
_gpio_rmw(base, bank->regs->wkup_en, 0xffffffff, 0);
_gpio_rmw(base, bank->regs->wkup_en, bank->context.wake_en, 1);

This is redundant in omap_gpio_suspend() because wakeup_en
register automatically gets initialized in _set_gpio_wakeup()
and set_gpio_trigger() while being called either from
chip.irq_set_wake() or chip.irq_set_type().

This is also redundant in omap_gpio_resume() because wakeup_en
register is programmed in omap_gpio_restore_context() called
which is called from runtime resume callback.

Cc: Kevin Hilman <khilman@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Cousson, Benoit <b-cousson@ti.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 drivers/gpio/gpio-omap.c |   47 ----------------------------------------------
 1 files changed, 0 insertions(+), 47 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 7533564..ae62c62 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1147,50 +1147,6 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
 
 #ifdef CONFIG_ARCH_OMAP2PLUS
 
-#if defined(CONFIG_PM_SLEEP)
-static int omap_gpio_suspend(struct device *dev)
-{
-	struct platform_device *pdev = to_platform_device(dev);
-	struct gpio_bank *bank = platform_get_drvdata(pdev);
-	void __iomem *base = bank->base;
-	unsigned long flags;
-
-	if (!bank->mod_usage || !bank->loses_context)
-		return 0;
-
-	if (!bank->regs->wkup_en || !bank->context.wake_en)
-		return 0;
-
-	spin_lock_irqsave(&bank->lock, flags);
-	_gpio_rmw(base, bank->regs->wkup_en, 0xffffffff, 0);
-	_gpio_rmw(base, bank->regs->wkup_en, bank->context.wake_en, 1);
-	spin_unlock_irqrestore(&bank->lock, flags);
-
-	return 0;
-}
-
-static int omap_gpio_resume(struct device *dev)
-{
-	struct platform_device *pdev = to_platform_device(dev);
-	struct gpio_bank *bank = platform_get_drvdata(pdev);
-	void __iomem *base = bank->base;
-	unsigned long flags;
-
-	if (!bank->mod_usage || !bank->loses_context)
-		return 0;
-
-	if (!bank->regs->wkup_en || !bank->context.wake_en)
-		return 0;
-
-	spin_lock_irqsave(&bank->lock, flags);
-	_gpio_rmw(base, bank->regs->wkup_en, 0xffffffff, 0);
-	_gpio_rmw(base, bank->regs->wkup_en, bank->context.wake_en, 1);
-	spin_unlock_irqrestore(&bank->lock, flags);
-
-	return 0;
-}
-#endif /* CONFIG_PM_SLEEP */
-
 #if defined(CONFIG_PM_RUNTIME)
 static void omap_gpio_restore_context(struct gpio_bank *bank);
 
@@ -1419,14 +1375,11 @@ static void omap_gpio_restore_context(struct gpio_bank *bank)
 }
 #endif /* CONFIG_PM_RUNTIME */
 #else
-#define omap_gpio_suspend NULL
-#define omap_gpio_resume NULL
 #define omap_gpio_runtime_suspend NULL
 #define omap_gpio_runtime_resume NULL
 #endif
 
 static const struct dev_pm_ops gpio_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(omap_gpio_suspend, omap_gpio_resume)
 	SET_RUNTIME_PM_OPS(omap_gpio_runtime_suspend, omap_gpio_runtime_resume,
 									NULL)
 };
-- 
1.7.0.4

[PATCH 7/8] gpio/omap: remove cpu_is_omapxxxx() checks from *_runtime_resume()

[Tarun Kanti DebBarma]

Add register offsets for GPIO_IRQSTATUS_RAW_0, GPIO_IRQSTATUS_RAW_0
which are present on OMAP4+ processors. Now we can distinguish
conditions applicable to OMAP4,5 and those specific to OMAP24xx
and OMAP3xxx.

Cc: Kevin Hilman <khilman@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Cousson, Benoit <b-cousson@ti.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
---
 arch/arm/mach-omap2/gpio.c             |    2 ++
 arch/arm/plat-omap/include/plat/gpio.h |    2 ++
 drivers/gpio/gpio-omap.c               |    4 ++--
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/gpio.c b/arch/arm/mach-omap2/gpio.c
index 86f91a6..a144e22 100644
--- a/arch/arm/mach-omap2/gpio.c
+++ b/arch/arm/mach-omap2/gpio.c
@@ -101,6 +101,8 @@ static int __init omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused)
 		pdata->regs->dataout = OMAP4_GPIO_DATAOUT;
 		pdata->regs->set_dataout = OMAP4_GPIO_SETDATAOUT;
 		pdata->regs->clr_dataout = OMAP4_GPIO_CLEARDATAOUT;
+		pdata->regs->irqstatus_raw0 = OMAP4_GPIO_IRQSTATUSRAW0;
+		pdata->regs->irqstatus_raw1 = OMAP4_GPIO_IRQSTATUSRAW1;
 		pdata->regs->irqstatus = OMAP4_GPIO_IRQSTATUS0;
 		pdata->regs->irqstatus2 = OMAP4_GPIO_IRQSTATUS1;
 		pdata->regs->irqenable = OMAP4_GPIO_IRQSTATUSSET0;
diff --git a/arch/arm/plat-omap/include/plat/gpio.h b/arch/arm/plat-omap/include/plat/gpio.h
index d903e7d..50fb7cc 100644
--- a/arch/arm/plat-omap/include/plat/gpio.h
+++ b/arch/arm/plat-omap/include/plat/gpio.h
@@ -172,6 +172,8 @@ struct omap_gpio_reg_offs {
 	u16 clr_dataout;
 	u16 irqstatus;
 	u16 irqstatus2;
+	u16 irqstatus_raw0;
+	u16 irqstatus_raw1;
 	u16 irqenable;
 	u16 irqenable2;
 	u16 set_irqenable;
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index ae62c62..d238f84 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1289,14 +1289,14 @@ static int omap_gpio_runtime_resume(struct device *dev)
 		old0 = __raw_readl(bank->base + bank->regs->leveldetect0);
 		old1 = __raw_readl(bank->base + bank->regs->leveldetect1);
 
-		if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
+		if (!bank->regs->irqstatus_raw0) {
 			__raw_writel(old0 | gen, bank->base +
 						bank->regs->leveldetect0);
 			__raw_writel(old1 | gen, bank->base +
 						bank->regs->leveldetect1);
 		}
 
-		if (cpu_is_omap44xx()) {
+		if (bank->regs->irqstatus_raw0) {
 			__raw_writel(old0 | l, bank->base +
 						bank->regs->leveldetect0);
 			__raw_writel(old1 | l, bank->base +
-- 
1.7.0.4

[PATCH 7/8] gpio/omap: remove cpu_is_omapxxxx() checks from *_runtime_resume()

[Santosh Shilimkar]

On Friday 27 April 2012 07:43 PM, Tarun Kanti DebBarma wrote:
> Add register offsets for GPIO_IRQSTATUS_RAW_0, GPIO_IRQSTATUS_RAW_0
> which are present on OMAP4+ processors. Now we can distinguish
> conditions applicable to OMAP4,5 and those specific to OMAP24xx
> and OMAP3xxx.
> 
> Cc: Kevin Hilman <khilman@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Cousson, Benoit <b-cousson@ti.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
> ---
>  arch/arm/mach-omap2/gpio.c             |    2 ++
>  arch/arm/plat-omap/include/plat/gpio.h |    2 ++
>  drivers/gpio/gpio-omap.c               |    4 ++--
>  3 files changed, 6 insertions(+), 2 deletions(-)
> 
Looks like this was the last cpu_is_*() in gpio driver.
Thanks for cleaning up this.

Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

Regards
Santosh

[PATCH 8/8] gpio/omap: fix missing check in *_runtime_suspend()

[Tarun Kanti DebBarma]

We do checking for bank->enabled_non_wakeup_gpios in order
to skip redundant operations. Somehow, the check got missed
while doing the cleanup series.

Just to make sure that we do context restore correctly in
*_runtime_resume(), the bank->workaround_enabled check is
moved after context restore. Otherwise, it would prevent
context restore when bank->enabled_non_wakeup_gpios is 0.

Cc: Kevin Hilman <khilman@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Cousson, Benoit <b-cousson@ti.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
---
 drivers/gpio/gpio-omap.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index d238f84..59a4af1 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1160,6 +1160,9 @@ static int omap_gpio_runtime_suspend(struct device *dev)
 
 	spin_lock_irqsave(&bank->lock, flags);
 
+	if (!bank->enabled_non_wakeup_gpios)
+		goto update_gpio_context_count;
+
 	/*
 	 * Only edges can generate a wakeup event to the PRCM.
 	 *
@@ -1235,11 +1238,6 @@ static int omap_gpio_runtime_resume(struct device *dev)
 	__raw_writel(bank->context.risingdetect,
 		     bank->base + bank->regs->risingdetect);
 
-	if (!bank->workaround_enabled) {
-		spin_unlock_irqrestore(&bank->lock, flags);
-		return 0;
-	}
-
 	if (bank->get_context_loss_count) {
 		context_lost_cnt_after =
 			bank->get_context_loss_count(bank->dev);
@@ -1252,6 +1250,11 @@ static int omap_gpio_runtime_resume(struct device *dev)
 		}
 	}
 
+	if (!bank->workaround_enabled) {
+		spin_unlock_irqrestore(&bank->lock, flags);
+		return 0;
+	}
+
 	__raw_writel(bank->context.fallingdetect,
 			bank->base + bank->regs->fallingdetect);
 	__raw_writel(bank->context.risingdetect,
-- 
1.7.0.4

[PATCH 8/8] gpio/omap: fix missing check in *_runtime_suspend()

[Santosh Shilimkar]

On Friday 27 April 2012 07:43 PM, Tarun Kanti DebBarma wrote:
> We do checking for bank->enabled_non_wakeup_gpios in order
> to skip redundant operations. Somehow, the check got missed
> while doing the cleanup series.
> 
> Just to make sure that we do context restore correctly in
> *_runtime_resume(), the bank->workaround_enabled check is
> moved after context restore. Otherwise, it would prevent
> context restore when bank->enabled_non_wakeup_gpios is 0.
> 
> Cc: Kevin Hilman <khilman@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Cousson, Benoit <b-cousson@ti.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
> ---
Looks good.
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

[PATCH 8/8] gpio/omap: fix missing check in *_runtime_suspend()

[Kevin Hilman]

Tarun, Santosh,

Tarun Kanti DebBarma <tarun.kanti@ti.com> writes:

> We do checking for bank->enabled_non_wakeup_gpios in order
> to skip redundant operations. Somehow, the check got missed
> while doing the cleanup series.
>
> Just to make sure that we do context restore correctly in
> *_runtime_resume(), the bank->workaround_enabled check is
> moved after context restore. Otherwise, it would prevent
> context restore when bank->enabled_non_wakeup_gpios is 0.
>
> Cc: Kevin Hilman <khilman@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Cousson, Benoit <b-cousson@ti.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>

I just noticed that this patch has caused some strange problems, notably
with the GPIO IRQ used by smsc911x NIC (Overo, Zoom3, 2430SDP, etc. etc.)

The patch itself is OK, but it has exposed a bug in other parts of the
context restore path that was previously hidden.

We seem to have been finding lots of GPIO bugs by just testing the
network chips with GPIO IRQs.  Can I suggest that a platform with a GPIO
IRQ NIC be added to the test platforms you're using? 

> ---
>  drivers/gpio/gpio-omap.c |   13 ++++++++-----
>  1 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index d238f84..59a4af1 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -1160,6 +1160,9 @@ static int omap_gpio_runtime_suspend(struct device *dev)
>  
>  	spin_lock_irqsave(&bank->lock, flags);
>  
> +	if (!bank->enabled_non_wakeup_gpios)
> +		goto update_gpio_context_count;
> +
>  	/*
>  	 * Only edges can generate a wakeup event to the PRCM.
>  	 *
> @@ -1235,11 +1238,6 @@ static int omap_gpio_runtime_resume(struct device *dev)
>  	__raw_writel(bank->context.risingdetect,
>  		     bank->base + bank->regs->risingdetect);
>  
> -	if (!bank->workaround_enabled) {
> -		spin_unlock_irqrestore(&bank->lock, flags);
> -		return 0;
> -	}
> -

My moving this below, you exposed some buggy code that can now get
executed in non-OFF mode, wherease before $SUBJECT patch, it would never
be exectued in non-OFF mode.

>  	if (bank->get_context_loss_count) {
>  		context_lost_cnt_after =
>  			bank->get_context_loss_count(bank->dev);

...right below this line, we have:

		if (context_lost_cnt_after != bank->context_loss_count ||
						!context_lost_cnt_after) {
			omap_gpio_restore_context(bank);

While we've never hit off-mode, context_lost_cnt_after will always be
zero.  However, becasue of the !context_lost_cnt_after check there, we
will *always* restore the bank context, even though context has never
been lost.

I have no idea why that !context_lost_cnt_after check is there, but
clearly it is wrong.  Now that you moved the 'workraound_enabled' check
below this section, as long as off-mode is disabled, the some GPIO
context will be restored here on *every* runtime PM transition.

The patch below fixes the problem.

Grant, after Tarun/Santosh have a chance to review/ack this, can you
still get this in for 3.5?  If not, getting it into -rc should be fine.

Kevin


>From 83874df8e0dd78a3609f5ba81d608df7217fd212 Mon Sep 17 00:00:00 2001
From: Kevin Hilman <khilman@ti.com>
Date: Thu, 17 May 2012 14:52:56 -0700
Subject: [PATCH] gpio/omap: fix broken context restore for non-OFF mode
 transitions

The fix in commit 1b12870 (gpio/omap: fix missing check in
*_runtime_suspend()) exposed another bug in the context restore path.

Currently, the per-bank context restore happens whenever the context
loss count is different in runtime suspend and runtime resume *and*
whenever the per-bank contex_loss_count == 0:

	if (context_lost_cnt_after != bank->context_loss_count ||
					!context_lost_cnt_after) {
		omap_gpio_restore_context(bank);

Restoring context when the context_lost_cnt_after == 0 is clearly
wrong, since this will be true until the first off-mode transition
(which could be never, if off-mode is never enabled.)  This check
causes the context to be restored on *every* runtime PM transition.

Before commit 1b12870 (gpio/omap: fix missing check in
*_runtime_suspend()), this code was never executed in non-OFF mode, so
there were never spurious context restores happening.  After that
change though, spurious context restores could happen.

To fix, simply remove the !context_lost_cnt_after check. It is not
needed.

This bug was found when noticing that the smc911x NIC on 3530/Overo
was not working, and git bisect tracked it down to this patch.  It
seems that the spurious context restore was causing the smsc911x to
not be properly probed on this platform.

Cc: Tarun Kanti DebBarma <tarun.kanti@ti.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Signed-off-by: Kevin Hilman <khilman@ti.com>
---
 drivers/gpio/gpio-omap.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 9b71f04..b570a6a 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1238,8 +1238,7 @@ static int omap_gpio_runtime_resume(struct device *dev)
 	if (bank->get_context_loss_count) {
 		context_lost_cnt_after =
 			bank->get_context_loss_count(bank->dev);
-		if (context_lost_cnt_after != bank->context_loss_count ||
-						!context_lost_cnt_after) {
+		if (context_lost_cnt_after != bank->context_loss_count) {
 			omap_gpio_restore_context(bank);
 		} else {
 			spin_unlock_irqrestore(&bank->lock, flags);
-- 
1.7.9.2

[PATCH 8/8] gpio/omap: fix missing check in *_runtime_suspend()

[Tony Lindgren]

* Kevin Hilman <khilman@ti.com> [120517 15:29]:
> 
> I just noticed that this patch has caused some strange problems, notably
> with the GPIO IRQ used by smsc911x NIC (Overo, Zoom3, 2430SDP, etc. etc.)
> 
> The patch itself is OK, but it has exposed a bug in other parts of the
> context restore path that was previously hidden.
> 
> We seem to have been finding lots of GPIO bugs by just testing the
> network chips with GPIO IRQs.  Can I suggest that a platform with a GPIO
> IRQ NIC be added to the test platforms you're using? 

Yes considering the breakage it's pretty obvious the original series was
never properly tested on omaps.

Regarding this fix, using gpio/next + this patch fixes nfsroot for 2430sdp,
and gets zoom3 nfsroot booting going a bit better.

However, at least on zoom3 nfsroot now takes several _minutes_ to get to
login: with gpio/next + this patch. The system is totally unusable.

It seems that the GPIO interrupt wake-up events are not properly working
now?

Reverting the "gpio/omap: fix missing check in *_runtime_suspend()"
patch seems to fix the issue.

> Subject: [PATCH] gpio/omap: fix broken context restore for non-OFF mode
>  transitions
> 
> The fix in commit 1b12870 (gpio/omap: fix missing check in
> *_runtime_suspend()) exposed another bug in the context restore path.

Kevin, looks like commit 1b12870 does not exist in gpio/next?

Regards,

Tony

[PATCH 8/8] gpio/omap: fix missing check in *_runtime_suspend()

[Kevin Hilman]

Tony Lindgren <tony@atomide.com> writes:

> * Kevin Hilman <khilman@ti.com> [120517 15:29]:
>> 
>> I just noticed that this patch has caused some strange problems, notably
>> with the GPIO IRQ used by smsc911x NIC (Overo, Zoom3, 2430SDP, etc. etc.)
>> 
>> The patch itself is OK, but it has exposed a bug in other parts of the
>> context restore path that was previously hidden.
>> 
>> We seem to have been finding lots of GPIO bugs by just testing the
>> network chips with GPIO IRQs.  Can I suggest that a platform with a GPIO
>> IRQ NIC be added to the test platforms you're using? 
>
> Yes considering the breakage it's pretty obvious the original series was
> never properly tested on omaps.

Agreed.

> Regarding this fix, using gpio/next + this patch fixes nfsroot for 2430sdp,
> and gets zoom3 nfsroot booting going a bit better.
>
> However, at least on zoom3 nfsroot now takes several _minutes_ to get to
> login: with gpio/next + this patch. The system is totally unusable.
>
> It seems that the GPIO interrupt wake-up events are not properly working
> now?
>
> Reverting the "gpio/omap: fix missing check in *_runtime_suspend()"
> patch seems to fix the issue.

Argh, then $SUBJECT patch here has caused brokeness in multiple ways.
It managed to break both runtime suspend and runtime resume at the same
time. :(

The change added by this patch to runtime_suspend effectively disables
the fix I did in 68942edb09 (gpio/omap: fix wakeups on level-triggered GPIOs)
causing the sluggish network problems to reappear, since that GPIO IRQ
is no longer causing wakeups.

Simple fix is below, which just moves the check added in $SUBJECT patch
below the workaround for the edge/level fix.  Can you confirm it works
on Zoom3 (applies on gpio/next + my previous fix.)

I didn't notice the same problem on Overo, but I guess it's because
Overo had some enabled non-wakeup GPIOs in the same bank, so it didn't
bypass the level-triggered IRQ fix.

>> Subject: [PATCH] gpio/omap: fix broken context restore for non-OFF mode
>>  transitions
>> 
>> The fix in commit 1b12870 (gpio/omap: fix missing check in
>> *_runtime_suspend()) exposed another bug in the context restore path.
>
> Kevin, looks like commit 1b12870 does not exist in gpio/next?

Will update the changelog.

Because of this new problem, I have to add the patch below too, so
I'll get them both queued up for Grant

In the mean time, they're availble in my for_3.5/fixes/gpio-2 branch.

Kevin

[1]
>From afb4f0836dc3c432aa999fc98a80bf75e1481e04 Mon Sep 17 00:00:00 2001
From: Kevin Hilman <khilman@ti.com>
Date: Thu, 17 May 2012 16:42:16 -0700
Subject: [PATCH] gpio/omap: (re)fix wakeups on level-triggered GPIOs

commit 1b1287032 (gpio/omap: fix missing check in *_runtime_suspend())
broke wakeups on level-triggered GPIOs by adding the enabled
non-wakeup GPIO check before the workaround that enables wakeups
on level-triggered IRQs, effectively disabling that workaround.

To fix, move the enabled non-wakeup GPIO check after the
level-triggered IRQ workaround.

Reported-by: Tony Lindgren <tony@atomide.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Tarun Kanti DebBarma <tarun.kanti@ti.com>
Signed-off-by: Kevin Hilman <khilman@ti.com>
---
 drivers/gpio/gpio-omap.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index b570a6a..c4ed172 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1157,9 +1157,6 @@ static int omap_gpio_runtime_suspend(struct device *dev)
 
 	spin_lock_irqsave(&bank->lock, flags);
 
-	if (!bank->enabled_non_wakeup_gpios)
-		goto update_gpio_context_count;
-
 	/*
 	 * Only edges can generate a wakeup event to the PRCM.
 	 *
@@ -1180,6 +1177,9 @@ static int omap_gpio_runtime_suspend(struct device *dev)
 		__raw_writel(wake_hi | bank->context.risingdetect,
 			     bank->base + bank->regs->risingdetect);
 
+	if (!bank->enabled_non_wakeup_gpios)
+		goto update_gpio_context_count;
+
 	if (bank->power_mode != OFF_MODE) {
 		bank->power_mode = 0;
 		goto update_gpio_context_count;
-- 
1.7.9.2

[PATCH 8/8] gpio/omap: fix missing check in *_runtime_suspend()

[Tony Lindgren]

* Kevin Hilman <khilman@ti.com> [120517 17:00]:
> 
> Argh, then $SUBJECT patch here has caused brokeness in multiple ways.
> It managed to break both runtime suspend and runtime resume at the same
> time. :(
> 
> The change added by this patch to runtime_suspend effectively disables
> the fix I did in 68942edb09 (gpio/omap: fix wakeups on level-triggered GPIOs)
> causing the sluggish network problems to reappear, since that GPIO IRQ
> is no longer causing wakeups.
>
> Simple fix is below, which just moves the check added in $SUBJECT patch
> below the workaround for the edge/level fix.  Can you confirm it works
> on Zoom3 (applies on gpio/next + my previous fix.)

Thanks yes having both of your fixes applied fixes the issues I was
seeing, so for both:

Tested-by: Tony Lindgren <tony@atomide.com>
 
> I didn't notice the same problem on Overo, but I guess it's because
> Overo had some enabled non-wakeup GPIOs in the same bank, so it didn't
> bypass the level-triggered IRQ fix.

Makes sense for why it only appears on some boards.
 
> In the mean time, they're availble in my for_3.5/fixes/gpio-2 branch.

I'll merge that also into l-o master for some more testing before
the merge window.

Regards,

Tony

[PATCH 8/8] gpio/omap: fix missing check in *_runtime_suspend()

[DebBarma, Tarun Kanti]

On Fri, May 18, 2012 at 5:26 AM, Kevin Hilman <khilman@ti.com> wrote:
> Tony Lindgren <tony@atomide.com> writes:
>
>> * Kevin Hilman <khilman@ti.com> [120517 15:29]:
>>>
>>> I just noticed that this patch has caused some strange problems, notably
>>> with the GPIO IRQ used by smsc911x NIC (Overo, Zoom3, 2430SDP, etc. etc.)
>>>
>>> The patch itself is OK, but it has exposed a bug in other parts of the
>>> context restore path that was previously hidden.
>>>
>>> We seem to have been finding lots of GPIO bugs by just testing the
>>> network chips with GPIO IRQs. ?Can I suggest that a platform with a GPIO
>>> IRQ NIC be added to the test platforms you're using?
>>
>> Yes considering the breakage it's pretty obvious the original series was
>> never properly tested on omaps.
>
> Agreed.
>
>> Regarding this fix, using gpio/next + this patch fixes nfsroot for 2430sdp,
>> and gets zoom3 nfsroot booting going a bit better.
>>
>> However, at least on zoom3 nfsroot now takes several _minutes_ to get to
>> login: with gpio/next + this patch. The system is totally unusable.
>>
>> It seems that the GPIO interrupt wake-up events are not properly working
>> now?
>>
>> Reverting the "gpio/omap: fix missing check in *_runtime_suspend()"
>> patch seems to fix the issue.
>
> Argh, then $SUBJECT patch here has caused brokeness in multiple ways.
> It managed to break both runtime suspend and runtime resume at the same
> time. :(
>
> The change added by this patch to runtime_suspend effectively disables
> the fix I did in 68942edb09 (gpio/omap: fix wakeups on level-triggered GPIOs)
> causing the sluggish network problems to reappear, since that GPIO IRQ
> is no longer causing wakeups.
>
> Simple fix is below, which just moves the check added in $SUBJECT patch
> below the workaround for the edge/level fix. ?Can you confirm it works
> on Zoom3 (applies on gpio/next + my previous fix.)
>
> I didn't notice the same problem on Overo, but I guess it's because
> Overo had some enabled non-wakeup GPIOs in the same bank, so it didn't
> bypass the level-triggered IRQ fix.
>
>>> Subject: [PATCH] gpio/omap: fix broken context restore for non-OFF mode
>>> ?transitions
>>>
>>> The fix in commit 1b12870 (gpio/omap: fix missing check in
>>> *_runtime_suspend()) exposed another bug in the context restore path.
>>
>> Kevin, looks like commit 1b12870 does not exist in gpio/next?
>
> Will update the changelog.
>
> Because of this new problem, I have to add the patch below too, so
> I'll get them both queued up for Grant
>
> In the mean time, they're availble in my for_3.5/fixes/gpio-2 branch.
>
> Kevin
>
> [1]
> From afb4f0836dc3c432aa999fc98a80bf75e1481e04 Mon Sep 17 00:00:00 2001
> From: Kevin Hilman <khilman@ti.com>
> Date: Thu, 17 May 2012 16:42:16 -0700
> Subject: [PATCH] gpio/omap: (re)fix wakeups on level-triggered GPIOs
>
> commit 1b1287032 (gpio/omap: fix missing check in *_runtime_suspend())
> broke wakeups on level-triggered GPIOs by adding the enabled
> non-wakeup GPIO check before the workaround that enables wakeups
> on level-triggered IRQs, effectively disabling that workaround.
>
> To fix, move the enabled non-wakeup GPIO check after the
> level-triggered IRQ workaround.
>
> Reported-by: Tony Lindgren <tony@atomide.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Tarun Kanti DebBarma <tarun.kanti@ti.com>
> Signed-off-by: Kevin Hilman <khilman@ti.com>

Acked-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>

> ---
> ?drivers/gpio/gpio-omap.c | ? ?6 +++---
> ?1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index b570a6a..c4ed172 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -1157,9 +1157,6 @@ static int omap_gpio_runtime_suspend(struct device *dev)
>
> ? ? ? ?spin_lock_irqsave(&bank->lock, flags);
>
> - ? ? ? if (!bank->enabled_non_wakeup_gpios)
> - ? ? ? ? ? ? ? goto update_gpio_context_count;
> -
> ? ? ? ?/*
> ? ? ? ? * Only edges can generate a wakeup event to the PRCM.
> ? ? ? ? *
> @@ -1180,6 +1177,9 @@ static int omap_gpio_runtime_suspend(struct device *dev)
> ? ? ? ? ? ? ? ?__raw_writel(wake_hi | bank->context.risingdetect,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? bank->base + bank->regs->risingdetect);
>
> + ? ? ? if (!bank->enabled_non_wakeup_gpios)
> + ? ? ? ? ? ? ? goto update_gpio_context_count;
> +
> ? ? ? ?if (bank->power_mode != OFF_MODE) {
> ? ? ? ? ? ? ? ?bank->power_mode = 0;
> ? ? ? ? ? ? ? ?goto update_gpio_context_count;
> --
> 1.7.9.2
>

[PATCH 8/8] gpio/omap: fix missing check in *_runtime_suspend()

[Shilimkar, Santosh]

On Fri, May 18, 2012 at 5:26 AM, Kevin Hilman <khilman@ti.com> wrote:
> Tony Lindgren <tony@atomide.com> writes:
>
>> * Kevin Hilman <khilman@ti.com> [120517 15:29]:
>>>
>>> I just noticed that this patch has caused some strange problems, notably
>>> with the GPIO IRQ used by smsc911x NIC (Overo, Zoom3, 2430SDP, etc. etc.)
>>>
>>> The patch itself is OK, but it has exposed a bug in other parts of the
>>> context restore path that was previously hidden.
>>>
>>> We seem to have been finding lots of GPIO bugs by just testing the
>>> network chips with GPIO IRQs. ?Can I suggest that a platform with a GPIO
>>> IRQ NIC be added to the test platforms you're using?
>>
>> Yes considering the breakage it's pretty obvious the original series was
>> never properly tested on omaps.
>
> Agreed.
>
Actually OMAP4 network interface as well uses the GPIO as a interrupt line but
that didn't show the issue. But I agree more and more test scenario's are needed
for infrastructure components like GPIO/DMA/TImer because of their multiple
types of usages.

>> Regarding this fix, using gpio/next + this patch fixes nfsroot for 2430sdp,
>> and gets zoom3 nfsroot booting going a bit better.
>>
>> However, at least on zoom3 nfsroot now takes several _minutes_ to get to
>> login: with gpio/next + this patch. The system is totally unusable.
>>
>> It seems that the GPIO interrupt wake-up events are not properly working
>> now?
>>
>> Reverting the "gpio/omap: fix missing check in *_runtime_suspend()"
>> patch seems to fix the issue.
>
> Argh, then $SUBJECT patch here has caused brokeness in multiple ways.
> It managed to break both runtime suspend and runtime resume at the same
> time. :(
>
> The change added by this patch to runtime_suspend effectively disables
> the fix I did in 68942edb09 (gpio/omap: fix wakeups on level-triggered GPIOs)
> causing the sluggish network problems to reappear, since that GPIO IRQ
> is no longer causing wakeups.
>
That's pretty bad.

> Simple fix is below, which just moves the check added in $SUBJECT patch
> below the workaround for the edge/level fix. ?Can you confirm it works
> on Zoom3 (applies on gpio/next + my previous fix.)
>
> I didn't notice the same problem on Overo, but I guess it's because
> Overo had some enabled non-wakeup GPIOs in the same bank, so it didn't
> bypass the level-triggered IRQ fix.
>
>>> Subject: [PATCH] gpio/omap: fix broken context restore for non-OFF mode
>>> ?transitions
>>>
>>> The fix in commit 1b12870 (gpio/omap: fix missing check in
>>> *_runtime_suspend()) exposed another bug in the context restore path.
>>
>> Kevin, looks like commit 1b12870 does not exist in gpio/next?
>
> Will update the changelog.
>
> Because of this new problem, I have to add the patch below too, so
> I'll get them both queued up for Grant
>
> In the mean time, they're availble in my for_3.5/fixes/gpio-2 branch.
>
> Kevin
>
> [1]
> From afb4f0836dc3c432aa999fc98a80bf75e1481e04 Mon Sep 17 00:00:00 2001
> From: Kevin Hilman <khilman@ti.com>
> Date: Thu, 17 May 2012 16:42:16 -0700
> Subject: [PATCH] gpio/omap: (re)fix wakeups on level-triggered GPIOs
>
> commit 1b1287032 (gpio/omap: fix missing check in *_runtime_suspend())
> broke wakeups on level-triggered GPIOs by adding the enabled
> non-wakeup GPIO check before the workaround that enables wakeups
> on level-triggered IRQs, effectively disabling that workaround.
>
> To fix, move the enabled non-wakeup GPIO check after the
> level-triggered IRQ workaround.
>
> Reported-by: Tony Lindgren <tony@atomide.com>
>
> Cc: Tarun Kanti DebBarma <tarun.kanti@ti.com>
> Signed-off-by: Kevin Hilman <khilman@ti.com>
> ---
Thanks for the Fix Kevin.
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

[PATCH 8/8] gpio/omap: fix missing check in *_runtime_suspend()

[Grant Likely]

On Thu, 17 May 2012 15:21:07 -0700, Kevin Hilman <khilman@ti.com> wrote:
> Tarun, Santosh,
> 
> Tarun Kanti DebBarma <tarun.kanti@ti.com> writes:
> 
> > We do checking for bank->enabled_non_wakeup_gpios in order
> > to skip redundant operations. Somehow, the check got missed
> > while doing the cleanup series.
> >
> > Just to make sure that we do context restore correctly in
> > *_runtime_resume(), the bank->workaround_enabled check is
> > moved after context restore. Otherwise, it would prevent
> > context restore when bank->enabled_non_wakeup_gpios is 0.
> >
> > Cc: Kevin Hilman <khilman@ti.com>
> > Cc: Tony Lindgren <tony@atomide.com>
> > Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> > Cc: Cousson, Benoit <b-cousson@ti.com>
> > Cc: Grant Likely <grant.likely@secretlab.ca>
> > Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
> 
> I just noticed that this patch has caused some strange problems, notably
> with the GPIO IRQ used by smsc911x NIC (Overo, Zoom3, 2430SDP, etc. etc.)
> 
> The patch itself is OK, but it has exposed a bug in other parts of the
> context restore path that was previously hidden.
> 
> We seem to have been finding lots of GPIO bugs by just testing the
> network chips with GPIO IRQs.  Can I suggest that a platform with a GPIO
> IRQ NIC be added to the test platforms you're using? 
> 
> > ---
> >  drivers/gpio/gpio-omap.c |   13 ++++++++-----
> >  1 files changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> > index d238f84..59a4af1 100644
> > --- a/drivers/gpio/gpio-omap.c
> > +++ b/drivers/gpio/gpio-omap.c
> > @@ -1160,6 +1160,9 @@ static int omap_gpio_runtime_suspend(struct device *dev)
> >  
> >  	spin_lock_irqsave(&bank->lock, flags);
> >  
> > +	if (!bank->enabled_non_wakeup_gpios)
> > +		goto update_gpio_context_count;
> > +
> >  	/*
> >  	 * Only edges can generate a wakeup event to the PRCM.
> >  	 *
> > @@ -1235,11 +1238,6 @@ static int omap_gpio_runtime_resume(struct device *dev)
> >  	__raw_writel(bank->context.risingdetect,
> >  		     bank->base + bank->regs->risingdetect);
> >  
> > -	if (!bank->workaround_enabled) {
> > -		spin_unlock_irqrestore(&bank->lock, flags);
> > -		return 0;
> > -	}
> > -
> 
> My moving this below, you exposed some buggy code that can now get
> executed in non-OFF mode, wherease before $SUBJECT patch, it would never
> be exectued in non-OFF mode.
> 
> >  	if (bank->get_context_loss_count) {
> >  		context_lost_cnt_after =
> >  			bank->get_context_loss_count(bank->dev);
> 
> ...right below this line, we have:
> 
> 		if (context_lost_cnt_after != bank->context_loss_count ||
> 						!context_lost_cnt_after) {
> 			omap_gpio_restore_context(bank);
> 
> While we've never hit off-mode, context_lost_cnt_after will always be
> zero.  However, becasue of the !context_lost_cnt_after check there, we
> will *always* restore the bank context, even though context has never
> been lost.
> 
> I have no idea why that !context_lost_cnt_after check is there, but
> clearly it is wrong.  Now that you moved the 'workraound_enabled' check
> below this section, as long as off-mode is disabled, the some GPIO
> context will be restored here on *every* runtime PM transition.
> 
> The patch below fixes the problem.
> 
> Grant, after Tarun/Santosh have a chance to review/ack this, can you
> still get this in for 3.5?  If not, getting it into -rc should be fine.

Yes.  Do you have any other omap patches?  Do you want to send me a
pull req?

g.

> > Kevin
> 
> 
> From 83874df8e0dd78a3609f5ba81d608df7217fd212 Mon Sep 17 00:00:00 2001
> From: Kevin Hilman <khilman@ti.com>
> Date: Thu, 17 May 2012 14:52:56 -0700
> Subject: [PATCH] gpio/omap: fix broken context restore for non-OFF mode
>  transitions
> 
> The fix in commit 1b12870 (gpio/omap: fix missing check in
> *_runtime_suspend()) exposed another bug in the context restore path.
> 
> Currently, the per-bank context restore happens whenever the context
> loss count is different in runtime suspend and runtime resume *and*
> whenever the per-bank contex_loss_count == 0:
> 
> 	if (context_lost_cnt_after != bank->context_loss_count ||
> 					!context_lost_cnt_after) {
> 		omap_gpio_restore_context(bank);
> 
> Restoring context when the context_lost_cnt_after == 0 is clearly
> wrong, since this will be true until the first off-mode transition
> (which could be never, if off-mode is never enabled.)  This check
> causes the context to be restored on *every* runtime PM transition.
> 
> Before commit 1b12870 (gpio/omap: fix missing check in
> *_runtime_suspend()), this code was never executed in non-OFF mode, so
> there were never spurious context restores happening.  After that
> change though, spurious context restores could happen.
> 
> To fix, simply remove the !context_lost_cnt_after check. It is not
> needed.
> 
> This bug was found when noticing that the smc911x NIC on 3530/Overo
> was not working, and git bisect tracked it down to this patch.  It
> seems that the spurious context restore was causing the smsc911x to
> not be properly probed on this platform.
> 
> Cc: Tarun Kanti DebBarma <tarun.kanti@ti.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Kevin Hilman <khilman@ti.com>
> ---
>  drivers/gpio/gpio-omap.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index 9b71f04..b570a6a 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -1238,8 +1238,7 @@ static int omap_gpio_runtime_resume(struct device *dev)
>  	if (bank->get_context_loss_count) {
>  		context_lost_cnt_after =
>  			bank->get_context_loss_count(bank->dev);
> -		if (context_lost_cnt_after != bank->context_loss_count ||
> -						!context_lost_cnt_after) {
> +		if (context_lost_cnt_after != bank->context_loss_count) {
>  			omap_gpio_restore_context(bank);
>  		} else {
>  			spin_unlock_irqrestore(&bank->lock, flags);
> -- 
> 1.7.9.2
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

[PATCH 8/8] gpio/omap: fix missing check in *_runtime_suspend()

[Kevin Hilman]

Grant Likely <grant.likely@secretlab.ca> writes:

>> Grant, after Tarun/Santosh have a chance to review/ack this, can you
>> still get this in for 3.5?  If not, getting it into -rc should be fine.
>
> Yes.  Do you have any other omap patches?  Do you want to send me a
> pull req?

We just found one more fix needed.

I'm collecting the acks/tested-bys and will have a branch for you
shortly.

Kevin

[PATCH 8/8] gpio/omap: fix missing check in *_runtime_suspend()

[DebBarma, Tarun Kanti]

On Fri, May 18, 2012 at 3:51 AM, Kevin Hilman <khilman@ti.com> wrote:
> Tarun, Santosh,
>
> Tarun Kanti DebBarma <tarun.kanti@ti.com> writes:
>
>> We do checking for bank->enabled_non_wakeup_gpios in order
>> to skip redundant operations. Somehow, the check got missed
>> while doing the cleanup series.
>>
>> Just to make sure that we do context restore correctly in
>> *_runtime_resume(), the bank->workaround_enabled check is
>> moved after context restore. Otherwise, it would prevent
>> context restore when bank->enabled_non_wakeup_gpios is 0.
>>
>> Cc: Kevin Hilman <khilman@ti.com>
>> Cc: Tony Lindgren <tony@atomide.com>
>> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
>> Cc: Cousson, Benoit <b-cousson@ti.com>
>> Cc: Grant Likely <grant.likely@secretlab.ca>
>> Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
>
> I just noticed that this patch has caused some strange problems, notably
> with the GPIO IRQ used by smsc911x NIC (Overo, Zoom3, 2430SDP, etc. etc.)
>
> The patch itself is OK, but it has exposed a bug in other parts of the
> context restore path that was previously hidden.
>
> We seem to have been finding lots of GPIO bugs by just testing the
> network chips with GPIO IRQs. ?Can I suggest that a platform with a GPIO
> IRQ NIC be added to the test platforms you're using?
>
>> ---
>> ?drivers/gpio/gpio-omap.c | ? 13 ++++++++-----
>> ?1 files changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
>> index d238f84..59a4af1 100644
>> --- a/drivers/gpio/gpio-omap.c
>> +++ b/drivers/gpio/gpio-omap.c
>> @@ -1160,6 +1160,9 @@ static int omap_gpio_runtime_suspend(struct device *dev)
>>
>> ? ? ? spin_lock_irqsave(&bank->lock, flags);
>>
>> + ? ? if (!bank->enabled_non_wakeup_gpios)
>> + ? ? ? ? ? ? goto update_gpio_context_count;
>> +
>> ? ? ? /*
>> ? ? ? ?* Only edges can generate a wakeup event to the PRCM.
>> ? ? ? ?*
>> @@ -1235,11 +1238,6 @@ static int omap_gpio_runtime_resume(struct device *dev)
>> ? ? ? __raw_writel(bank->context.risingdetect,
>> ? ? ? ? ? ? ? ? ? ?bank->base + bank->regs->risingdetect);
>>
>> - ? ? if (!bank->workaround_enabled) {
>> - ? ? ? ? ? ? spin_unlock_irqrestore(&bank->lock, flags);
>> - ? ? ? ? ? ? return 0;
>> - ? ? }
>> -
>
> My moving this below, you exposed some buggy code that can now get
> executed in non-OFF mode, wherease before $SUBJECT patch, it would never
> be exectued in non-OFF mode.
>
>> ? ? ? if (bank->get_context_loss_count) {
>> ? ? ? ? ? ? ? context_lost_cnt_after =
>> ? ? ? ? ? ? ? ? ? ? ? bank->get_context_loss_count(bank->dev);
>
> ...right below this line, we have:
>
> ? ? ? ? ? ? ? ?if (context_lost_cnt_after != bank->context_loss_count ||
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?!context_lost_cnt_after) {
> ? ? ? ? ? ? ? ? ? ? ? ?omap_gpio_restore_context(bank);
>
> While we've never hit off-mode, context_lost_cnt_after will always be
> zero. ?However, becasue of the !context_lost_cnt_after check there, we
> will *always* restore the bank context, even though context has never
> been lost.
>
> I have no idea why that !context_lost_cnt_after check is there, but
> clearly it is wrong. ?Now that you moved the 'workraound_enabled' check
> below this section, as long as off-mode is disabled, the some GPIO
> context will be restored here on *every* runtime PM transition.
>
> The patch below fixes the problem.
>
> Grant, after Tarun/Santosh have a chance to review/ack this, can you
> still get this in for 3.5? ?If not, getting it into -rc should be fine.
>
> Kevin
>
>
> From 83874df8e0dd78a3609f5ba81d608df7217fd212 Mon Sep 17 00:00:00 2001
> From: Kevin Hilman <khilman@ti.com>
> Date: Thu, 17 May 2012 14:52:56 -0700
> Subject: [PATCH] gpio/omap: fix broken context restore for non-OFF mode
> ?transitions
>
> The fix in commit 1b12870 (gpio/omap: fix missing check in
> *_runtime_suspend()) exposed another bug in the context restore path.
>
> Currently, the per-bank context restore happens whenever the context
> loss count is different in runtime suspend and runtime resume *and*
> whenever the per-bank contex_loss_count == 0:
>
> ? ? ? ?if (context_lost_cnt_after != bank->context_loss_count ||
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?!context_lost_cnt_after) {
> ? ? ? ? ? ? ? ?omap_gpio_restore_context(bank);
>
> Restoring context when the context_lost_cnt_after == 0 is clearly
> wrong, since this will be true until the first off-mode transition
> (which could be never, if off-mode is never enabled.) ?This check
> causes the context to be restored on *every* runtime PM transition.
>
> Before commit 1b12870 (gpio/omap: fix missing check in
> *_runtime_suspend()), this code was never executed in non-OFF mode, so
> there were never spurious context restores happening. ?After that
> change though, spurious context restores could happen.
>
> To fix, simply remove the !context_lost_cnt_after check. It is not
> needed.
>
> This bug was found when noticing that the smc911x NIC on 3530/Overo
> was not working, and git bisect tracked it down to this patch. ?It
> seems that the spurious context restore was causing the smsc911x to
> not be properly probed on this platform.
>
> Cc: Tarun Kanti DebBarma <tarun.kanti@ti.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Kevin Hilman <khilman@ti.com>

Acked-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>

> ---
> ?drivers/gpio/gpio-omap.c | ? ?3 +--
> ?1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index 9b71f04..b570a6a 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -1238,8 +1238,7 @@ static int omap_gpio_runtime_resume(struct device *dev)
> ? ? ? ?if (bank->get_context_loss_count) {
> ? ? ? ? ? ? ? ?context_lost_cnt_after =
> ? ? ? ? ? ? ? ? ? ? ? ?bank->get_context_loss_count(bank->dev);
> - ? ? ? ? ? ? ? if (context_lost_cnt_after != bank->context_loss_count ||
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? !context_lost_cnt_after) {
> + ? ? ? ? ? ? ? if (context_lost_cnt_after != bank->context_loss_count) {
> ? ? ? ? ? ? ? ? ? ? ? ?omap_gpio_restore_context(bank);
> ? ? ? ? ? ? ? ?} else {
> ? ? ? ? ? ? ? ? ? ? ? ?spin_unlock_irqrestore(&bank->lock, flags);
> --
> 1.7.9.2
>

[PATCH 8/8] gpio/omap: fix missing check in *_runtime_suspend()

[DebBarma, Tarun Kanti]

On Fri, May 18, 2012 at 3:51 AM, Kevin Hilman <khilman@ti.com> wrote:
> Tarun, Santosh,
>
> Tarun Kanti DebBarma <tarun.kanti@ti.com> writes:
>
>> We do checking for bank->enabled_non_wakeup_gpios in order
>> to skip redundant operations. Somehow, the check got missed
>> while doing the cleanup series.
>>
>> Just to make sure that we do context restore correctly in
>> *_runtime_resume(), the bank->workaround_enabled check is
>> moved after context restore. Otherwise, it would prevent
>> context restore when bank->enabled_non_wakeup_gpios is 0.
>>
>> Cc: Kevin Hilman <khilman@ti.com>
>> Cc: Tony Lindgren <tony@atomide.com>
>> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
>> Cc: Cousson, Benoit <b-cousson@ti.com>
>> Cc: Grant Likely <grant.likely@secretlab.ca>
>> Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
>
> I just noticed that this patch has caused some strange problems, notably
> with the GPIO IRQ used by smsc911x NIC (Overo, Zoom3, 2430SDP, etc. etc.)
>
> The patch itself is OK, but it has exposed a bug in other parts of the
> context restore path that was previously hidden.
>
> We seem to have been finding lots of GPIO bugs by just testing the
> network chips with GPIO IRQs. ?Can I suggest that a platform with a GPIO
> IRQ NIC be added to the test platforms you're using?

Able to reproduce the problem. Tested on Zoom3 by enabling NFS.
Verified the time difference for the filesystem to boot. Without your
fixes it takes
around 60 seconds while with the fix it takes less than 7 seconds.

Tested-by:  Tarun Kanti DebBarma <tarun.kanti@ti.com>

>
>> ---
>> ?drivers/gpio/gpio-omap.c | ? 13 ++++++++-----
>> ?1 files changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
>> index d238f84..59a4af1 100644
>> --- a/drivers/gpio/gpio-omap.c
>> +++ b/drivers/gpio/gpio-omap.c
>> @@ -1160,6 +1160,9 @@ static int omap_gpio_runtime_suspend(struct device *dev)
>>
>> ? ? ? spin_lock_irqsave(&bank->lock, flags);
>>
>> + ? ? if (!bank->enabled_non_wakeup_gpios)
>> + ? ? ? ? ? ? goto update_gpio_context_count;
>> +
>> ? ? ? /*
>> ? ? ? ?* Only edges can generate a wakeup event to the PRCM.
>> ? ? ? ?*
>> @@ -1235,11 +1238,6 @@ static int omap_gpio_runtime_resume(struct device *dev)
>> ? ? ? __raw_writel(bank->context.risingdetect,
>> ? ? ? ? ? ? ? ? ? ?bank->base + bank->regs->risingdetect);
>>
>> - ? ? if (!bank->workaround_enabled) {
>> - ? ? ? ? ? ? spin_unlock_irqrestore(&bank->lock, flags);
>> - ? ? ? ? ? ? return 0;
>> - ? ? }
>> -
>
> My moving this below, you exposed some buggy code that can now get
> executed in non-OFF mode, wherease before $SUBJECT patch, it would never
> be exectued in non-OFF mode.
>
>> ? ? ? if (bank->get_context_loss_count) {
>> ? ? ? ? ? ? ? context_lost_cnt_after =
>> ? ? ? ? ? ? ? ? ? ? ? bank->get_context_loss_count(bank->dev);
>
> ...right below this line, we have:
>
> ? ? ? ? ? ? ? ?if (context_lost_cnt_after != bank->context_loss_count ||
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?!context_lost_cnt_after) {
> ? ? ? ? ? ? ? ? ? ? ? ?omap_gpio_restore_context(bank);
>
> While we've never hit off-mode, context_lost_cnt_after will always be
> zero. ?However, becasue of the !context_lost_cnt_after check there, we
> will *always* restore the bank context, even though context has never
> been lost.
>
> I have no idea why that !context_lost_cnt_after check is there, but
> clearly it is wrong. ?Now that you moved the 'workraound_enabled' check
> below this section, as long as off-mode is disabled, the some GPIO
> context will be restored here on *every* runtime PM transition.
>
> The patch below fixes the problem.
>
> Grant, after Tarun/Santosh have a chance to review/ack this, can you
> still get this in for 3.5? ?If not, getting it into -rc should be fine.
>
> Kevin
>
>
> From 83874df8e0dd78a3609f5ba81d608df7217fd212 Mon Sep 17 00:00:00 2001
> From: Kevin Hilman <khilman@ti.com>
> Date: Thu, 17 May 2012 14:52:56 -0700
> Subject: [PATCH] gpio/omap: fix broken context restore for non-OFF mode
> ?transitions
>
> The fix in commit 1b12870 (gpio/omap: fix missing check in
> *_runtime_suspend()) exposed another bug in the context restore path.
>
> Currently, the per-bank context restore happens whenever the context
> loss count is different in runtime suspend and runtime resume *and*
> whenever the per-bank contex_loss_count == 0:
>
> ? ? ? ?if (context_lost_cnt_after != bank->context_loss_count ||
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?!context_lost_cnt_after) {
> ? ? ? ? ? ? ? ?omap_gpio_restore_context(bank);
>
> Restoring context when the context_lost_cnt_after == 0 is clearly
> wrong, since this will be true until the first off-mode transition
> (which could be never, if off-mode is never enabled.) ?This check
> causes the context to be restored on *every* runtime PM transition.
>
> Before commit 1b12870 (gpio/omap: fix missing check in
> *_runtime_suspend()), this code was never executed in non-OFF mode, so
> there were never spurious context restores happening. ?After that
> change though, spurious context restores could happen.
>
> To fix, simply remove the !context_lost_cnt_after check. It is not
> needed.
>
> This bug was found when noticing that the smc911x NIC on 3530/Overo
> was not working, and git bisect tracked it down to this patch. ?It
> seems that the spurious context restore was causing the smsc911x to
> not be properly probed on this platform.
>
> Cc: Tarun Kanti DebBarma <tarun.kanti@ti.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Kevin Hilman <khilman@ti.com>
> ---
> ?drivers/gpio/gpio-omap.c | ? ?3 +--
> ?1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index 9b71f04..b570a6a 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -1238,8 +1238,7 @@ static int omap_gpio_runtime_resume(struct device *dev)
> ? ? ? ?if (bank->get_context_loss_count) {
> ? ? ? ? ? ? ? ?context_lost_cnt_after =
> ? ? ? ? ? ? ? ? ? ? ? ?bank->get_context_loss_count(bank->dev);
> - ? ? ? ? ? ? ? if (context_lost_cnt_after != bank->context_loss_count ||
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? !context_lost_cnt_after) {
> + ? ? ? ? ? ? ? if (context_lost_cnt_after != bank->context_loss_count) {
> ? ? ? ? ? ? ? ? ? ? ? ?omap_gpio_restore_context(bank);
> ? ? ? ? ? ? ? ?} else {
> ? ? ? ? ? ? ? ? ? ? ? ?spin_unlock_irqrestore(&bank->lock, flags);
> --
> 1.7.9.2
>

[PATCH 0/8] gpio/omap: remaining cleanups and fix

[Grant Likely]

On Fri, 27 Apr 2012 19:43:30 +0530, Tarun Kanti DebBarma <tarun.kanti@ti.com> wrote:
> Here are the remaining cleanup patches. There are broadly
> two categories of cleanups.
> 
> Cat-1: Those missed while introducing new feature like SPARSE_IRQ
>        handling and DT support; use edge/level handlers from
>        generic IRQ framework.
> 
> Cat-2: Removal of redundant fields from struct gpio_bank{} as a
>        result of they being already covered by members within
>        context field of struct gpio_bank{}.
> 
> Reference: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
> Commit: 66f75a5d028beaf67c931435fdc3e7823125730c (Linux 3.4-rc4)
> 
> Series is available for reference here:
> git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev.git for_3.5/gpio_more_cleanup_fixes
> 
> Test info:
> OMAP2+ platforms: OMAP2430SDP, OMAP3430SDP, OMAP4430SDP
> OMAP1: Bootup test on OMAP1710SDP.
> 
> Cc: Kevin Hilman <khilman@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Cousson, Benoit <b-cousson@ti.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Tarun Kanti DebBarma (8):
>   gpio/omap: remove virtual_irq_start variable
>   gpio/omap: remove saved_fallingdetect, saved_risingdetect
>   gpio/omap: remove suspend_wakeup field from struct gpio_bank
>   gpio/omap: remove saved_wakeup field from struct gpio_bank
>   gpio/omap: remove retrigger variable in gpio_irq_handler
>   gpio/omap: remove suspend/resume callbacks
>   gpio/omap: remove cpu_is_omapxxxx() checks from *_runtime_resume()
>   gpio/omap: fix missing check in *_runtime_suspend()

The changes look fine to me.  Which branch should this series be
merged through?  It can either go via arm-soc or my gpio/next branch.

g.

[PATCH 0/8] gpio/omap: remaining cleanups and fix

[Kevin Hilman]

Grant Likely <grant.likely@secretlab.ca> writes:

> On Fri, 27 Apr 2012 19:43:30 +0530, Tarun Kanti DebBarma <tarun.kanti@ti.com> wrote:
>> Here are the remaining cleanup patches. There are broadly
>> two categories of cleanups.
>> 
>> Cat-1: Those missed while introducing new feature like SPARSE_IRQ
>>        handling and DT support; use edge/level handlers from
>>        generic IRQ framework.
>> 
>> Cat-2: Removal of redundant fields from struct gpio_bank{} as a
>>        result of they being already covered by members within
>>        context field of struct gpio_bank{}.
>> 
>> Reference: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
>> Commit: 66f75a5d028beaf67c931435fdc3e7823125730c (Linux 3.4-rc4)
>> 
>> Series is available for reference here:
>> git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev.git for_3.5/gpio_more_cleanup_fixes
>> 
>> Test info:
>> OMAP2+ platforms: OMAP2430SDP, OMAP3430SDP, OMAP4430SDP
>> OMAP1: Bootup test on OMAP1710SDP.
>> 
>> Cc: Kevin Hilman <khilman@ti.com>
>> Cc: Tony Lindgren <tony@atomide.com>
>> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
>> Cc: Cousson, Benoit <b-cousson@ti.com>
>> Cc: Grant Likely <grant.likely@secretlab.ca>
>> Tarun Kanti DebBarma (8):
>>   gpio/omap: remove virtual_irq_start variable
>>   gpio/omap: remove saved_fallingdetect, saved_risingdetect
>>   gpio/omap: remove suspend_wakeup field from struct gpio_bank
>>   gpio/omap: remove saved_wakeup field from struct gpio_bank
>>   gpio/omap: remove retrigger variable in gpio_irq_handler
>>   gpio/omap: remove suspend/resume callbacks
>>   gpio/omap: remove cpu_is_omapxxxx() checks from *_runtime_resume()
>>   gpio/omap: fix missing check in *_runtime_suspend()
>
> The changes look fine to me.  Which branch should this series be
> merged through?  It can either go via arm-soc or my gpio/next branch.

This needs a little more review/testing on OMAP.

Expect a pull request from me when it's ready and you can take it
through gpio/nex.

Thanks,

Kevin

[PATCH 0/8] gpio/omap: remaining cleanups and fix

[Grant Likely]

On Fri, Apr 27, 2012 at 3:57 PM, Kevin Hilman <khilman@ti.com> wrote:
> Grant Likely <grant.likely@secretlab.ca> writes:
>
>> On Fri, 27 Apr 2012 19:43:30 +0530, Tarun Kanti DebBarma <tarun.kanti@ti.com> wrote:
>>> Here are the remaining cleanup patches. There are broadly
>>> two categories of cleanups.
>>>
>>> Cat-1: Those missed while introducing new feature like SPARSE_IRQ
>>> ? ? ? ?handling and DT support; use edge/level handlers from
>>> ? ? ? ?generic IRQ framework.
>>>
>>> Cat-2: Removal of redundant fields from struct gpio_bank{} as a
>>> ? ? ? ?result of they being already covered by members within
>>> ? ? ? ?context field of struct gpio_bank{}.
>>>
>>> Reference: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
>>> Commit: 66f75a5d028beaf67c931435fdc3e7823125730c (Linux 3.4-rc4)
>>>
>>> Series is available for reference here:
>>> git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev.git for_3.5/gpio_more_cleanup_fixes
>>>
>>> Test info:
>>> OMAP2+ platforms: OMAP2430SDP, OMAP3430SDP, OMAP4430SDP
>>> OMAP1: Bootup test on OMAP1710SDP.
>>>
>>> Cc: Kevin Hilman <khilman@ti.com>
>>> Cc: Tony Lindgren <tony@atomide.com>
>>> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
>>> Cc: Cousson, Benoit <b-cousson@ti.com>
>>> Cc: Grant Likely <grant.likely@secretlab.ca>
>>> Tarun Kanti DebBarma (8):
>>> ? gpio/omap: remove virtual_irq_start variable
>>> ? gpio/omap: remove saved_fallingdetect, saved_risingdetect
>>> ? gpio/omap: remove suspend_wakeup field from struct gpio_bank
>>> ? gpio/omap: remove saved_wakeup field from struct gpio_bank
>>> ? gpio/omap: remove retrigger variable in gpio_irq_handler
>>> ? gpio/omap: remove suspend/resume callbacks
>>> ? gpio/omap: remove cpu_is_omapxxxx() checks from *_runtime_resume()
>>> ? gpio/omap: fix missing check in *_runtime_suspend()
>>
>> The changes look fine to me. ?Which branch should this series be
>> merged through? ?It can either go via arm-soc or my gpio/next branch.
>
> This needs a little more review/testing on OMAP.
>
> Expect a pull request from me when it's ready and you can take it
> through gpio/nex.

Okay.

g.

[PATCH 0/8] gpio/omap: remaining cleanups and fix

[Santosh Shilimkar]

Tarun,

On Friday 27 April 2012 07:43 PM, Tarun Kanti DebBarma wrote:
> Here are the remaining cleanup patches. There are broadly
> two categories of cleanups.
> 
> Cat-1: Those missed while introducing new feature like SPARSE_IRQ
>        handling and DT support; use edge/level handlers from
>        generic IRQ framework.
> 
> Cat-2: Removal of redundant fields from struct gpio_bank{} as a
>        result of they being already covered by members within
>        context field of struct gpio_bank{}.
> 
> Reference: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
> Commit: 66f75a5d028beaf67c931435fdc3e7823125730c (Linux 3.4-rc4)
> 
> Series is available for reference here:
> git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev.git for_3.5/gpio_more_cleanup_fixes
> 
> Test info:
> OMAP2+ platforms: OMAP2430SDP, OMAP3430SDP, OMAP4430SDP
> OMAP1: Bootup test on OMAP1710SDP.
> 
> Cc: Kevin Hilman <khilman@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Cousson, Benoit <b-cousson@ti.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Tarun Kanti DebBarma (8):
>   gpio/omap: remove virtual_irq_start variable
>   gpio/omap: remove saved_fallingdetect, saved_risingdetect
>   gpio/omap: remove suspend_wakeup field from struct gpio_bank
>   gpio/omap: remove saved_wakeup field from struct gpio_bank
>   gpio/omap: remove retrigger variable in gpio_irq_handler
>   gpio/omap: remove suspend/resume callbacks
>   gpio/omap: remove cpu_is_omapxxxx() checks from *_runtime_resume()
>   gpio/omap: fix missing check in *_runtime_suspend()
> 
I have reviewed the remaining 3 patches from this series.
Thanks.

Regards
santosh

[PATCH 0/8] gpio/omap: remaining cleanups and fix

[Raja, Govindraj]

Hi Tarun,

On Fri, Apr 27, 2012 at 7:43 PM, Tarun Kanti DebBarma
<tarun.kanti@ti.com> wrote:
> Here are the remaining cleanup patches. There are broadly
> two categories of cleanups.
>
> Cat-1: Those missed while introducing new feature like SPARSE_IRQ
> ? ? ? handling and DT support; use edge/level handlers from
> ? ? ? generic IRQ framework.
>
> Cat-2: Removal of redundant fields from struct gpio_bank{} as a
> ? ? ? result of they being already covered by members within
> ? ? ? context field of struct gpio_bank{}.
>
> Reference: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
> Commit: 66f75a5d028beaf67c931435fdc3e7823125730c (Linux 3.4-rc4)
>
> Series is available for reference here:
> git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev.git for_3.5/gpio_more_cleanup_fixes


I tested this series from your tree on Beagle-XM retention, offmode,
suspend to mem and resume with and and without enabling offmode
from sysfs was checked and found retention count and off mode
count was getting incremented.

Also ran few test cases available from here[1] to request and release gpio,
test cases passed.

Feel Free to add:

Tested-by: Govindraj.R <govindraj.raja@ti.com>

--
Thanks
Govindraj.R

[1]: http://gitorious.org/omap-ddt/omap-ddt/trees/master/gpio/test_code/utils/gpio_mods

[PATCH 0/8] gpio/omap: remaining cleanups and fix

[Kevin Hilman]

Hi Govindraj,

"Raja, Govindraj" <govindraj.raja@ti.com> writes:

[...]

> I tested this series from your tree on Beagle-XM retention, offmode,
> suspend to mem and resume with and and without enabling offmode
> from sysfs was checked and found retention count and off mode
> count was getting incremented.
>
> Also ran few test cases available from here[1] to request and release gpio,
> test cases passed.
>
> Feel Free to add:
>
> Tested-by: Govindraj.R <govindraj.raja@ti.com>

Awesome!

Many thanks for the testing.  It *greatly* helps when maintainers can
see more testing by folks other than the author.

Thank you!

Kevin