Skip to contentSkip to conversation

gpio/omap: driver cleanup and fixes

PATCH Superseded Tarun Kanti DebBarma 4 participants 52 messages Aug 4, 2011 – Aug 29, 2011

Subsystems (7)

v5 of 9 Superseded Latest is v9 →

lore
Share & tools
lore
#0 lore raw

v5 cover gpio/omap: driver cleanup and fixes

This series is continuation of cleanup of OMAP GPIO driver and fixes. The cleanup include getting rid of cpu_is_* checks wherever possible, use of gpio_bank list instead of static array, use of unique platform specific value associated data member to OMAP platforms to avoid cpu_is_* checks. The series also include PM runtime support.*

Baseline: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
Branch: master
Commit: 02f8c6a (Linux 3.0)

The patch series was applied on the mainline on top of Kevin's gpio cleanup patch series taken from following location:
git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git
Branch: for_3.1/gpio-cleanup-2

Test Details:
- Compile tested for omap1_defconfig and omap2plus_defconfig.
- OMAP1710-H3: Bootup test.
- OMAP2430/SDP, OMAP3430/SDP, OMAP4430/SDP, 4430/Blaze: Functional testing.
- PM Testing on OMAP3430-SDP: retention, off_mode, system_wide suspend and gpio wakeup.

v5:
- Reduce runtime callback overhead when *_get/put_sync() called from probe() and *_gpio_request/free().

- Dynamic context save within functions where context is modified instead of saving all context within a common function.

- Removed call to mpuio_init() from omap_gpio_mod_init(). Both the functions are called once during initialization in *_gpio_probe().
Call to omap_gpio_mod_init() has been removed from omap_gpio_request() on the first access to gpio bank. One time initialization looks sufficient.

- In *_gpio_irq_handler() use *_put_sync_suspend() instead of *_put_sync().

- Removed hardcoding of OMAP16xx sysconfig register value and instead defined an associated constant.

- Removed *_get_sync() call from *_gpio_suspend() and *_put_sync() call from *_gpio_resume(). They got wrongly slipped into the code.

- Removed following redundant zero allocated initialization from mach-omap2/gpio.c
+ pdata->regs->irqctrl = 0;
+ pdata->regs->edgectrl1 = 0;
+ pdata->regs->edgectrl2 = 0;

- Removed following redundant code in gpio-omap.c
-#define bank_is_mpuio(bank) ((bank)->method == METHOD_MPUIO)

v4:
- since all accesses to registers are 4-byte aligned, removing special checks and handling of 16 and 32-bit wide bank registers and instead use 32-bit read/write access consistently.

- redundant usage of MOD_REG_BIT has been corrected and replaced with _gpio_rmw().

- omap_gpio_mod_init() function has been simplified further using _gpio_rmw().

- sysconfig register offset specific to omap16xx has been removed along with its usage.

- additional logic to skip from suspend/resume:

  if (!bank->regs->wkup_status || !bank->suspend_wakeup)
  	return 0;
  
  if (!bank->regs->wkup_status || !bank->saved_wakeup)
  	return 0;

- separated mpuio related changes into a different patch from the patch where wakeup status register related changes are done.

- Incorrect replacement of !cpu_class_is_omap2() in gpio_irq_type() corrected:
+ if (!bank->regs->leveldetect0 &&
+ (type & (IRQ_TYPE_LEVEL_LOW|IRQ_TYPE_LEVEL_HIGH)))
return -EINVAL;

v3:
- Avoid use of wkup_set and wkup_clear registers. Instead use wkup_status register for all platforms. This is because on OMAP4 it is recommended not to use them.

- Remove duplicate code in omap_gpio_mod_init() for handling the same for 32-bit and 16-bit GPIO bank widths. This is accomplished by having two functions to handle each case while assiging a common function pointer during initialization.

- Remove OMAP16xx specific one time initialization from omap_gpio_mod_init(). Move it inside omap16xx_gpio_init().

- Avoid usage of USHRT_MAX to indicate undefined values. Use 0 instead.

- In omap_gpio_suspend()/resume() functions remove code that checks if the feature is supported. Instead, assign these functions to struct platform_driver's suspend & resume function pointers for those OMAP platforms whcih support this feature.

- Remove 'suspend_support' flag because it is redundant. Instead use wkup_* registers to decode the same information.

- Restore context also when we don't know if the context is lost.

- Make omap_gpio_save_context() and omap_gpio_restore_context() static.

v2:
- Do special handling of non-wakeup GPIOs only on OMAP2420. Avoid this handling on OMAP3430.
- Isolate cleanups and fixes into separate set of patches. Keep the cleanup first followed by the fixes.
- Avoid calling omap_gpio_get_context_loss() directly and instead call it through function pointer in pdata initialized during init.
- workaround_enabled flag is not longer needed and is removed.
- Call pwrdm_post_transition() before calling omap_gpio_resume_after_idle().
- In omap2_gpio_resume_after_idle() do context restore before handling workaround.
- Use PM runtime framework.
- Modify register offset names to : wkup_status, wkup_clear, wkup_set. Also use 'base + offset' for readibility in all relevant places.
- Remove unwanted messages from commit section like TODO, etc.

Charulatha V (12):
gpio/omap: remove dependency on gpio_bank_count gpio/omap: use flag to identify wakeup domain gpio/omap: make gpio_context part of gpio_bank structure gpio/omap: fix pwrdm_post_transition call sequence gpio/omap: handle save/restore ctx in GPIO driver gpio/omap: make non-wakeup GPIO part of pdata gpio/omap: avoid cpu checks during module ena/disable gpio/omap: cleanup omap1 related macros gpio/omap: use pinctrl offset instead of macro gpio/omap: use readl in irq_handler for all access gpio/omap: remove bank->method & METHOD_* macros gpio/omap: fix bankwidth for OMAP7xx MPUIO

Tarun Kanti DebBarma (10):
gpio/omap: further cleanup using wakeup_status register gpio/omap: use level/edge detect reg offsets gpio/omap: remove hardcoded offsets in ctxt save/restore gpio/omap: cleanup set_gpio_triggering function gpio/omap: cleanup omap_gpio_mod_init function gpio/omap: use pm-runtime framework
gpio/omap: optimize suspend and resume functions gpio/omap: cleanup prepare_for_idle and resume_after_idle gpio/omap: skip operations in runtime callbacks gpio/omap: remove omap_gpio_save_context overhead

 arch/arm/mach-omap1/gpio15xx.c         |    7 +-
 arch/arm/mach-omap1/gpio16xx.c         |   46 ++-
 arch/arm/mach-omap1/gpio7xx.c          |   14 +-
 arch/arm/mach-omap2/gpio.c             |   43 ++-
 arch/arm/mach-omap2/pm34xx.c           |   21 +-
 arch/arm/plat-omap/include/plat/gpio.h |   29 +-
 drivers/gpio/gpio-omap.c               | 1054 +++++++++++++-------------------
 7 files changed, 521 insertions(+), 693 deletions(-)
#1 lore raw

PATCH v5 1/22 gpio/omap: remove dependency on gpio_bank_count

From: Charulatha V <[email protected]>

The gpio_bank_count is the count of number of GPIO devices in a SoC. Remove this dependency from the driver by using list. Also remove the dependency on array of pointers to gpio_bank struct of all GPIO devices.

---
 arch/arm/mach-omap1/gpio15xx.c         |    1 -
 arch/arm/mach-omap1/gpio16xx.c         |    2 -
 arch/arm/mach-omap1/gpio7xx.c          |    2 -
 arch/arm/mach-omap2/gpio.c             |    1 -
 arch/arm/plat-omap/include/plat/gpio.h |    3 -
 drivers/gpio/gpio-omap.c               |  163 ++++++++++++++++----------------
 6 files changed, 80 insertions(+), 92 deletions(-)
arch/arm/mach-omap1/gpio15xx.c +0 -1
index 399da4c..f8c15ea 100644
@@ -115,7 +115,6 @@ static int __init omap15xx_gpio_init(void)
 	platform_device_register(&omap15xx_mpu_gpio);
 	platform_device_register(&omap15xx_gpio);
 
-	gpio_bank_count = 2;
 	return 0;
 }
 postcore_initcall(omap15xx_gpio_init);
arch/arm/mach-omap1/gpio16xx.c +0 -2
index 0f399bd..df4bb44 100644
@@ -221,8 +221,6 @@ static int __init omap16xx_gpio_init(void)
 	for (i = 0; i < ARRAY_SIZE(omap16xx_gpio_dev); i++)
 		platform_device_register(omap16xx_gpio_dev[i]);
 
-	gpio_bank_count = ARRAY_SIZE(omap16xx_gpio_dev);
-
 	return 0;
 }
 postcore_initcall(omap16xx_gpio_init);
arch/arm/mach-omap1/gpio7xx.c +0 -2
index 5ab63ea..923eaa1 100644
@@ -282,8 +282,6 @@ static int __init omap7xx_gpio_init(void)
 	for (i = 0; i < ARRAY_SIZE(omap7xx_gpio_dev); i++)
 		platform_device_register(omap7xx_gpio_dev[i]);
 
-	gpio_bank_count = ARRAY_SIZE(omap7xx_gpio_dev);
-
 	return 0;
 }
 postcore_initcall(omap7xx_gpio_init);
arch/arm/mach-omap2/gpio.c +0 -1
index 9a46d77..f805cda 100644
@@ -119,7 +119,6 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused)
 		return PTR_ERR(od);
 	}
 
-	gpio_bank_count++;
 	return 0;
 }
 
arch/arm/plat-omap/include/plat/gpio.h +0 -3
index 91e8de3..dd330ed 100644
@@ -202,9 +202,6 @@ struct omap_gpio_platform_data {
 	struct omap_gpio_reg_offs *regs;
 };
 
-/* TODO: Analyze removing gpio_bank_count usage from driver code */
-extern int gpio_bank_count;
-
 extern void omap2_gpio_prepare_for_idle(int off_mode);
 extern void omap2_gpio_resume_after_idle(void);
 extern void omap_set_gpio_debounce(int gpio, int enable);
drivers/gpio/gpio-omap.c +80 -83

317 lines not on this page · raw message

Signature · 1 line
1.7.0.4
#2 lore raw

PATCH v5 1/22 gpio/omap: remove dependency on gpio_bank_count

Quoted 16 lines · Tarun Kanti DebBarma

From: Charulatha V<[email protected]>

The gpio_bank_count is the count of number of GPIO devices in a SoC. Remove this dependency from the driver by using list. Also remove the dependency on array of pointers to gpio_bank struct of all GPIO devices.

---
  arch/arm/mach-omap1/gpio15xx.c         |    1 -
  arch/arm/mach-omap1/gpio16xx.c         |    2 -
  arch/arm/mach-omap1/gpio7xx.c          |    2 -
  arch/arm/mach-omap2/gpio.c             |    1 -
  arch/arm/plat-omap/include/plat/gpio.h |    3 -
  drivers/gpio/gpio-omap.c               |  163 ++++++++++++++++----------------
  6 files changed, 80 insertions(+), 92 deletions(-)

[...]

Quoted 8 lines
drivers/gpio/gpio-omap.c +0 -0
index 34a7110..13d5ca4 100644
@@ -28,7 +28,10 @@
  #include<mach/gpio.h>
  #include<asm/mach/irq.h>
 

[....]

Quoted 9 lines
drivers/gpio/gpio-omap.c +0 -5
@@ -1334,17 +1325,17 @@ static int workaround_enabled;
 
  void omap2_gpio_prepare_for_idle(int off_mode)
  {
-	int i, c = 0;
-	int min = 0;
-
-	if (cpu_is_omap34xx())
-		min = 1;

drivers/gpio/gpio-omap.c static int workaround_enabled; · line 1335

Quoted 10 lines
drivers/gpio/gpio-omap.c +4 -2
+	int c = 0;
+	struct gpio_bank *bank;
 
-	for (i = min; i<  gpio_bank_count; i++) {
-		struct gpio_bank *bank =&gpio_bank[i];
+	list_for_each_entry(bank,&omap_gpio_list, node) {
  		u32 l1 = 0, l2 = 0;
  		int j;
 
+		/* TODO: Do not use cpu_is_omap34xx */

Is this addressed in subsequent patches ?

drivers/gpio/gpio-omap.c static int workaround_enabled; · line 1338

Quoted 3 lines
drivers/gpio/gpio-omap.c +3 -0
+		if ((cpu_is_omap34xx())&&  (bank->id == 0))
+			continue;
+

Rest of the changes looks good to me.
After you answer above, you can add my,

Regards
Santosh

#3 lore raw

PATCH v5 1/22 gpio/omap: remove dependency on gpio_bank_count

[...]

Quoted 16 lines
Quoted 16 lines
  void omap2_gpio_prepare_for_idle(int off_mode)
  {
drivers/gpio/gpio-omap.c +3 -7
-	int i, c = 0;
-	int min = 0;
-
-	if (cpu_is_omap34xx())
-		min = 1;
+	int c = 0;
+	struct gpio_bank *bank;
 
-	for (i = min; i<  gpio_bank_count; i++) {
-		struct gpio_bank *bank =&gpio_bank[i];
+	list_for_each_entry(bank,&omap_gpio_list, node) {
  		u32 l1 = 0, l2 = 0;
  		int j;
Quoted 3 lines

Review

Quoted 1 line
(quoted diff) +1 -0
+		/* TODO: Do not use cpu_is_omap34xx */

Is this addressed in subsequent patches ?

Yes.
--
Tarun

Quoted 11 lines

drivers/gpio/gpio-omap.c static int workaround_enabled; · line 1338

Quoted 4 lines
drivers/gpio/gpio-omap.c +3 -0
+		if ((cpu_is_omap34xx())&&  (bank->id == 0))
+			continue;
+

Rest of the changes looks good to me.
After you answer above, you can add my,

Regards
Santosh

#4 lore raw

PATCH v5 2/22 gpio/omap: use flag to identify wakeup domain

In reply to Tarun Kanti DebBarma

From: Charulatha V <[email protected]>

In omap3, save/restore context is implemented for GPIO banks 2-6 as GPIO bank1 is in wakeup domain. Instead of identifying bank's power domain by bank id, use 'loses_context' flag which is filled by pwrdm_can_ever_lose_context() during dev_init.

For getting the powerdomain pointer, omap_hwmod_get_pwrdm() is used. omap_device_get_pwrdm() could not be used as the pwrdm information needs to be filled in pdata, whereas omap_device_get_pwrdm() could be used only after omap_device_build() call.

---
 arch/arm/mach-omap2/gpio.c             |    6 ++++++
 arch/arm/plat-omap/include/plat/gpio.h |    1 +
 drivers/gpio/gpio-omap.c               |   13 ++++++-------
 3 files changed, 13 insertions(+), 7 deletions(-)
arch/arm/mach-omap2/gpio.c +6 -0
index f805cda..95195a8 100644
@@ -24,6 +24,8 @@
 #include <plat/omap_hwmod.h>
 #include <plat/omap_device.h>
 
+#include "powerdomain.h"
+
 static struct omap_device_pm_latency omap_gpio_latency[] = {
 	[0] = {
 		.deactivate_func = omap_device_idle_hwmods,
@@ -39,6 +41,7 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused)
 	struct omap_gpio_dev_attr *dev_attr;
 	char *name = "omap_gpio";
 	int id;
+	struct powerdomain *pwrdm;
 
 	/*
 	 * extract the device id from name field available in the
@@ -107,6 +110,9 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused)
 		return -EINVAL;
 	}
 
+	pwrdm = omap_hwmod_get_pwrdm(oh);
+	pdata->loses_context = pwrdm_can_ever_lose_context(pwrdm);
+
 	od = omap_device_build(name, id - 1, oh, pdata,
 				sizeof(*pdata),	omap_gpio_latency,
 				ARRAY_SIZE(omap_gpio_latency),
arch/arm/plat-omap/include/plat/gpio.h +1 -0
index dd330ed..58d0bf2 100644
@@ -198,6 +198,7 @@ struct omap_gpio_platform_data {
 	int bank_width;		/* GPIO bank width */
 	int bank_stride;	/* Only needed for omap1 MPUIO */
 	bool dbck_flag;		/* dbck required or not - True for OMAP3&4 */
+	bool loses_context;	/* whether the bank would ever lose context */
 
 	struct omap_gpio_reg_offs *regs;
 };
drivers/gpio/gpio-omap.c +6 -7
index 13d5ca4..aaf07b8 100644
@@ -56,6 +56,7 @@ struct gpio_bank {
 	u32 dbck_enable_mask;
 	struct device *dev;
 	bool dbck_flag;
+	bool loses_context;
 	int stride;
 	u32 width;
 	u16 id;
@@ -1176,7 +1177,7 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
 	bank->dbck_flag = pdata->dbck_flag;
 	bank->stride = pdata->bank_stride;
 	bank->width = pdata->bank_width;
-
+	bank->loses_context = pdata->loses_context;
 	bank->regs = pdata->regs;
 
 	if (bank->regs->set_dataout && bank->regs->clr_dataout)
@@ -1332,8 +1333,7 @@ void omap2_gpio_prepare_for_idle(int off_mode)
 		u32 l1 = 0, l2 = 0;
 		int j;
 
-		/* TODO: Do not use cpu_is_omap34xx */
-		if ((cpu_is_omap34xx()) && (bank->id == 0))
+		if (!bank->loses_context)
 			continue;
 
 		for (j = 0; j < hweight_long(bank->dbck_enable_mask); j++)
@@ -1400,8 +1400,7 @@ void omap2_gpio_resume_after_idle(void)
 		u32 l = 0, gen, gen0, gen1;
 		int j;
 
-		/* TODO: Do not use cpu_is_omap34xx */
-		if ((cpu_is_omap34xx()) && (bank->id == 0))
+		if (!bank->loses_context)
 			continue;
 
 		for (j = 0; j < hweight_long(bank->dbck_enable_mask); j++)
@@ -1500,7 +1499,7 @@ void omap_gpio_save_context(void)
 	list_for_each_entry(bank, &omap_gpio_list, node) {
 		i++;
 
-		if (bank->id == 0)
+		if (!bank->loses_context)
 			continue;
 
 		gpio_context[i].irqenable1 =
@@ -1534,7 +1533,7 @@ void omap_gpio_restore_context(void)
 	list_for_each_entry(bank, &omap_gpio_list, node) {
 		i++;
 
-		if (bank->id == 0)
+		if (!bank->loses_context)
 			continue;
 
 		__raw_writel(gpio_context[i].irqenable1,
Signature · 1 line
1.7.0.4
#5 lore raw

PATCH v5 2/22 gpio/omap: use flag to identify wakeup domain

Quoted 12 lines · Tarun Kanti DebBarma

From: Charulatha V<[email protected]>

In omap3, save/restore context is implemented for GPIO banks 2-6 as GPIO bank1 is in wakeup domain. Instead of identifying bank's power domain by bank id, use 'loses_context' flag which is filled by pwrdm_can_ever_lose_context() during dev_init.

For getting the powerdomain pointer, omap_hwmod_get_pwrdm() is used. omap_device_get_pwrdm() could not be used as the pwrdm information needs to be filled in pdata, whereas omap_device_get_pwrdm() could be used only after omap_device_build() call.

Quoted 7 lines
---
  arch/arm/mach-omap2/gpio.c             |    6 ++++++
  arch/arm/plat-omap/include/plat/gpio.h |    1 +
  drivers/gpio/gpio-omap.c               |   13 ++++++-------
  3 files changed, 13 insertions(+), 7 deletions(-)

Nice change.

#6 lore raw

PATCH v5 3/22 gpio/omap: make gpio_context part of gpio_bank structure

In reply to Tarun Kanti DebBarma

From: Charulatha V <[email protected]>

Currently gpio_context array used to save gpio bank's context, is used only for OMAP3 architecture. Move gpio_context as part of gpio_bank structure so that it can be specific to each gpio bank and can be used for any OMAP architecture

---
 drivers/gpio/gpio-omap.c |   76 ++++++++++++++++++++-------------------------
 1 files changed, 34 insertions(+), 42 deletions(-)
drivers/gpio/gpio-omap.c +34 -42
index aaf07b8..2fa8b13 100644
@@ -30,6 +30,19 @@
 
 static LIST_HEAD(omap_gpio_list);
 
+struct gpio_regs {
+	u32 irqenable1;
+	u32 irqenable2;
+	u32 wake_en;
+	u32 ctrl;
+	u32 oe;
+	u32 leveldetect0;
+	u32 leveldetect1;
+	u32 risingdetect;
+	u32 fallingdetect;
+	u32 dataout;
+};
+
 struct gpio_bank {
 	struct list_head node;
 	unsigned long pbase;
@@ -43,7 +56,7 @@ struct gpio_bank {
 #endif
 	u32 non_wakeup_gpios;
 	u32 enabled_non_wakeup_gpios;
-
+	struct gpio_regs context;
 	u32 saved_datain;
 	u32 saved_fallingdetect;
 	u32 saved_risingdetect;
@@ -66,23 +79,6 @@ struct gpio_bank {
 	struct omap_gpio_reg_offs *regs;
 };
 
-#ifdef CONFIG_ARCH_OMAP3
-struct omap3_gpio_regs {
-	u32 irqenable1;
-	u32 irqenable2;
-	u32 wake_en;
-	u32 ctrl;
-	u32 oe;
-	u32 leveldetect0;
-	u32 leveldetect1;
-	u32 risingdetect;
-	u32 fallingdetect;
-	u32 dataout;
-};
-
-static struct omap3_gpio_regs gpio_context[OMAP34XX_NR_GPIOS];
-#endif
-
 #define GPIO_INDEX(bank, gpio) (gpio % bank->width)
 #define GPIO_BIT(bank, gpio) (1 << GPIO_INDEX(bank, gpio))
 
@@ -1494,33 +1490,31 @@ void omap2_gpio_resume_after_idle(void)
 void omap_gpio_save_context(void)
 {
 	struct gpio_bank *bank;
-	int i = 0;
 
 	list_for_each_entry(bank, &omap_gpio_list, node) {
-		i++;
 
 		if (!bank->loses_context)
 			continue;
 
-		gpio_context[i].irqenable1 =
+		bank->context.irqenable1 =
 			__raw_readl(bank->base + OMAP24XX_GPIO_IRQENABLE1);
-		gpio_context[i].irqenable2 =
+		bank->context.irqenable2 =
 			__raw_readl(bank->base + OMAP24XX_GPIO_IRQENABLE2);
-		gpio_context[i].wake_en =
+		bank->context.wake_en =
 			__raw_readl(bank->base + OMAP24XX_GPIO_WAKE_EN);
-		gpio_context[i].ctrl =
+		bank->context.ctrl =
 			__raw_readl(bank->base + OMAP24XX_GPIO_CTRL);
-		gpio_context[i].oe =
+		bank->context.oe =
 			__raw_readl(bank->base + OMAP24XX_GPIO_OE);
-		gpio_context[i].leveldetect0 =
+		bank->context.leveldetect0 =
 			__raw_readl(bank->base + OMAP24XX_GPIO_LEVELDETECT0);
-		gpio_context[i].leveldetect1 =
+		bank->context.leveldetect1 =
 			__raw_readl(bank->base + OMAP24XX_GPIO_LEVELDETECT1);
-		gpio_context[i].risingdetect =
+		bank->context.risingdetect =
 			__raw_readl(bank->base + OMAP24XX_GPIO_RISINGDETECT);
-		gpio_context[i].fallingdetect =
+		bank->context.fallingdetect =
 			__raw_readl(bank->base + OMAP24XX_GPIO_FALLINGDETECT);
-		gpio_context[i].dataout =
+		bank->context.dataout =
 			__raw_readl(bank->base + OMAP24XX_GPIO_DATAOUT);
 	}
 }
@@ -1528,33 +1522,31 @@ void omap_gpio_save_context(void)
 void omap_gpio_restore_context(void)
 {
 	struct gpio_bank *bank;
-	int i = 0;
 
 	list_for_each_entry(bank, &omap_gpio_list, node) {
-		i++;
 
 		if (!bank->loses_context)
 			continue;
 
-		__raw_writel(gpio_context[i].irqenable1,
+		__raw_writel(bank->context.irqenable1,
 				bank->base + OMAP24XX_GPIO_IRQENABLE1);
-		__raw_writel(gpio_context[i].irqenable2,
+		__raw_writel(bank->context.irqenable2,
 				bank->base + OMAP24XX_GPIO_IRQENABLE2);
-		__raw_writel(gpio_context[i].wake_en,
+		__raw_writel(bank->context.wake_en,
 				bank->base + OMAP24XX_GPIO_WAKE_EN);
-		__raw_writel(gpio_context[i].ctrl,
+		__raw_writel(bank->context.ctrl,
 				bank->base + OMAP24XX_GPIO_CTRL);
-		__raw_writel(gpio_context[i].oe,
+		__raw_writel(bank->context.oe,
 				bank->base + OMAP24XX_GPIO_OE);
-		__raw_writel(gpio_context[i].leveldetect0,
+		__raw_writel(bank->context.leveldetect0,
 				bank->base + OMAP24XX_GPIO_LEVELDETECT0);
-		__raw_writel(gpio_context[i].leveldetect1,
+		__raw_writel(bank->context.leveldetect1,
 				bank->base + OMAP24XX_GPIO_LEVELDETECT1);
-		__raw_writel(gpio_context[i].risingdetect,
+		__raw_writel(bank->context.risingdetect,
 				bank->base + OMAP24XX_GPIO_RISINGDETECT);
-		__raw_writel(gpio_context[i].fallingdetect,
+		__raw_writel(bank->context.fallingdetect,
 				bank->base + OMAP24XX_GPIO_FALLINGDETECT);
-		__raw_writel(gpio_context[i].dataout,
+		__raw_writel(bank->context.dataout,
 				bank->base + OMAP24XX_GPIO_DATAOUT);
 	}
 }
Signature · 1 line
1.7.0.4
#7 lore raw

PATCH v5 3/22 gpio/omap: make gpio_context part of gpio_bank structure

Quoted 8 lines · Tarun Kanti DebBarma

From: Charulatha V<[email protected]>

Currently gpio_context array used to save gpio bank's context, is used only for OMAP3 architecture. Move gpio_context as part of gpio_bank structure so that it can be specific to each gpio bank and can be used for any OMAP architecture

---

Few comments.

Quoted 12 lines
  drivers/gpio/gpio-omap.c |   76 ++++++++++++++++++++-------------------------
  1 files changed, 34 insertions(+), 42 deletions(-)
drivers/gpio/gpio-omap.c +1 -0
index aaf07b8..2fa8b13 100644
@@ -30,6 +30,19 @@
 
  static LIST_HEAD(omap_gpio_list);
 
+struct gpio_regs {

drivers/gpio/gpio-omap.c line 43

Quoted 10 lines
drivers/gpio/gpio-omap.c +10 -0
+	u32 irqenable1;
+	u32 irqenable2;
+	u32 wake_en;
+	u32 ctrl;
+	u32 oe;
+	u32 leveldetect0;
+	u32 leveldetect1;
+	u32 risingdetect;
+	u32 fallingdetect;
+	u32 dataout;

debounce and debounce_en registers are missing.
Add them please.

Quoted 46 lines
drivers/gpio/gpio-omap.c +3 -20
+};
+
  struct gpio_bank {
  	struct list_head node;
  	unsigned long pbase;
@@ -43,7 +56,7 @@ struct gpio_bank {
  #endif
  	u32 non_wakeup_gpios;
  	u32 enabled_non_wakeup_gpios;
-
+	struct gpio_regs context;
  	u32 saved_datain;
  	u32 saved_fallingdetect;
  	u32 saved_risingdetect;
@@ -66,23 +79,6 @@ struct gpio_bank {
  	struct omap_gpio_reg_offs *regs;
  };
 
-#ifdef CONFIG_ARCH_OMAP3
-struct omap3_gpio_regs {
-	u32 irqenable1;
-	u32 irqenable2;
-	u32 wake_en;
-	u32 ctrl;
-	u32 oe;
-	u32 leveldetect0;
-	u32 leveldetect1;
-	u32 risingdetect;
-	u32 fallingdetect;
-	u32 dataout;
-};
-
-static struct omap3_gpio_regs gpio_context[OMAP34XX_NR_GPIOS];
-#endif
-
  #define GPIO_INDEX(bank, gpio) (gpio % bank->width)
  #define GPIO_BIT(bank, gpio) (1<<  GPIO_INDEX(bank, gpio))
 
@@ -1494,33 +1490,31 @@ void omap2_gpio_resume_after_idle(void)
  void omap_gpio_save_context(void)
  {
  	struct gpio_bank *bank;
-	int i = 0;
 
  	list_for_each_entry(bank,&omap_gpio_list, node) {
-		i++;

drivers/gpio/gpio-omap.c void omap2_gpio_resume_after_idle(void) · line 1502

Quoted 10 lines
drivers/gpio/gpio-omap.c +2 -2
 
  		if (!bank->loses_context)
  			continue;
 
-		gpio_context[i].irqenable1 =
+		bank->context.irqenable1 =
  			__raw_readl(bank->base + OMAP24XX_GPIO_IRQENABLE1);
-		gpio_context[i].irqenable2 =
+		bank->context.irqenable2 =
  			__raw_readl(bank->base + OMAP24XX_GPIO_IRQENABLE2);

The context restore procedure should be done carefully. For instance IRQ enabled register should be restored last to avoid any spurious interrupts.

drivers/gpio/gpio-omap.c void omap2_gpio_resume_after_idle(void) · line 1508

Quoted 9 lines
drivers/gpio/gpio-omap.c +3 -3
-		gpio_context[i].wake_en =
+		bank->context.wake_en =
  			__raw_readl(bank->base + OMAP24XX_GPIO_WAKE_EN);
-		gpio_context[i].ctrl =
+		bank->context.ctrl =
  			__raw_readl(bank->base + OMAP24XX_GPIO_CTRL);
-		gpio_context[i].oe =
+		bank->context.oe =
  			__raw_readl(bank->base + OMAP24XX_GPIO_OE);

Restore dataout register before OE restore.

Quoted 7 lines
drivers/gpio/gpio-omap.c +2 -3
-		gpio_context[i].leveldetect0 =
+		bank->context.leveldetect0 =
  			__raw_readl(bank->base + OMAP24XX_GPIO_LEVELDETECT0);
-		gpio_context[i].leveldetect1 =
+		bank->context.leveldetect1 =
  			__raw_readl(bank->base + OMAP24XX_GPIO_LEVELDETECT1);
-		gpio_context[i].risingdetect =

drivers/gpio/gpio-omap.c void omap2_gpio_resume_after_idle(void) · line 1520

Quoted 10 lines
drivers/gpio/gpio-omap.c +3 -2
+		bank->context.risingdetect =
  			__raw_readl(bank->base + OMAP24XX_GPIO_RISINGDETECT);
-		gpio_context[i].fallingdetect =
+		bank->context.fallingdetect =
  			__raw_readl(bank->base + OMAP24XX_GPIO_FALLINGDETECT);
-		gpio_context[i].dataout =
+		bank->context.dataout =
  			__raw_readl(bank->base + OMAP24XX_GPIO_DATAOUT);
  	}
  }

Regards
Santosh

#8 lore raw

PATCH v5 3/22 gpio/omap: make gpio_context part of gpio_bank structure

Quoted 11 lines · Santosh
Quoted 8 lines · Tarun Kanti DebBarma

From: Charulatha V<[email protected]>

Currently gpio_context array used to save gpio bank's context, is used only for OMAP3 architecture. Move gpio_context as part of gpio_bank structure so that it can be specific to each gpio bank and can be used for any OMAP architecture

---

Few comments.

[...]

Quoted 12 lines
Quoted 12 lines
drivers/gpio/gpio-omap.c +0 -2
@@ -1494,33 +1490,31 @@ void omap2_gpio_resume_after_idle(void)
  void omap_gpio_save_context(void)
  {
  	struct gpio_bank *bank;
-	int i = 0;
 
  	list_for_each_entry(bank,&omap_gpio_list, node) {
-		i++;
 
  		if (!bank->loses_context)
  			continue;
 
Quoted 10 lines

drivers/gpio/gpio-omap.c void omap2_gpio_resume_after_idle(void) · line 1502

Quoted 6 lines
drivers/gpio/gpio-omap.c +2 -2
-		gpio_context[i].irqenable1 =
+		bank->context.irqenable1 =
  			__raw_readl(bank->base + OMAP24XX_GPIO_IRQENABLE1);
-		gpio_context[i].irqenable2 =
+		bank->context.irqenable2 =
  			__raw_readl(bank->base + OMAP24XX_GPIO_IRQENABLE2);

The context restore procedure should be done carefully. For instance IRQ enabled register should be restored last to avoid any spurious interrupts.

For the sake of clean, easy-to-review patches, this kind of functional change should be a separate patch.

The goal of $SUBJECT patch is simply to move the context struct into the bank struct, not change the order of the save restore.

Any changing of the order of save/restore should be in a dedicated patch with a descriptive changelog since that is changing behavior of the code.

Kevin

#9 lore raw

PATCH v5 3/22 gpio/omap: make gpio_context part of gpio_bank structure

[...]

Quoted 24 lines
Quoted 13 lines
Quoted 13 lines
drivers/gpio/gpio-omap.c +0 -2
@@ -1494,33 +1490,31 @@ void omap2_gpio_resume_after_idle(void)
  void omap_gpio_save_context(void)
  {
  	struct gpio_bank *bank;
-	int i = 0;
 
  	list_for_each_entry(bank,&omap_gpio_list, node) {
-		i++;
 
  		if (!bank->loses_context)
  			continue;
 
Quoted 10 lines

drivers/gpio/gpio-omap.c void omap2_gpio_resume_after_idle(void) · line 1502

Quoted 6 lines
drivers/gpio/gpio-omap.c +2 -2
-		gpio_context[i].irqenable1 =
+		bank->context.irqenable1 =
  			__raw_readl(bank->base + OMAP24XX_GPIO_IRQENABLE1);
-		gpio_context[i].irqenable2 =
+		bank->context.irqenable2 =
  			__raw_readl(bank->base + OMAP24XX_GPIO_IRQENABLE2);

The context restore procedure should be done carefully. For instance IRQ enabled register should be restored last to avoid any spurious interrupts.

Quoted 8 lines

For the sake of clean, easy-to-review patches, this kind of functional change should be a separate patch.

The goal of $SUBJECT patch is simply to move the context struct into the bank struct, not change the order of the save restore.

Any changing of the order of save/restore should be in a dedicated patch with a descriptive changelog since that is changing behavior of the code.

Ok.

Quoted 2 lines

Kevin

#10 lore raw

PATCH v5 3/22 gpio/omap: make gpio_context part of gpio_bank structure

In reply to Kevin Hilman
Quoted 39 lines · Kevin Hilman
Quoted 11 lines · Santosh
Quoted 8 lines · Tarun Kanti DebBarma

From: Charulatha V<[email protected]>

Currently gpio_context array used to save gpio bank's context, is used only for OMAP3 architecture. Move gpio_context as part of gpio_bank structure so that it can be specific to each gpio bank and can be used for any OMAP architecture

---

Few comments.

[...]

Quoted 12 lines
Quoted 12 lines
drivers/gpio/gpio-omap.c +0 -2
@@ -1494,33 +1490,31 @@ void omap2_gpio_resume_after_idle(void)
   void omap_gpio_save_context(void)
   {
   	struct gpio_bank *bank;
-	int i = 0;
 
   	list_for_each_entry(bank,&omap_gpio_list, node) {
-		i++;
 
   		if (!bank->loses_context)
   			continue;
 
Quoted 10 lines

drivers/gpio/gpio-omap.c void omap2_gpio_resume_after_idle(void) · line 1502

Quoted 6 lines
drivers/gpio/gpio-omap.c +2 -2
-		gpio_context[i].irqenable1 =
+		bank->context.irqenable1 =
   			__raw_readl(bank->base + OMAP24XX_GPIO_IRQENABLE1);
-		gpio_context[i].irqenable2 =
+		bank->context.irqenable2 =
   			__raw_readl(bank->base + OMAP24XX_GPIO_IRQENABLE2);

The context restore procedure should be done carefully. For instance IRQ enabled register should be restored last to avoid any spurious interrupts.

Quoted 9 lines

For the sake of clean, easy-to-review patches, this kind of functional change should be a separate patch.

The goal of $SUBJECT patch is simply to move the context struct into the bank struct, not change the order of the save restore.

Any changing of the order of save/restore should be in a dedicated patch with a descriptive changelog since that is changing behavior of the code.

Agree.

Regards
Santosh

#11 lore raw

PATCH v5 4/22 gpio/omap: fix pwrdm_post_transition call sequence

In reply to Tarun Kanti DebBarma

From: Charulatha V <[email protected]>

The context lost count is modified in omap_sram_idle() path when pwrdm_post_transition() is called. But pwrdm_post_transition() is called only after omap_gpio_resume_after_idle() is called. Correct this so that context lost count is modified before calling omap_gpio_resume_after_idle().

This would be useful when OMAP GPIO save/restore context is called by the OMAP GPIO driver itself.

---
 arch/arm/mach-omap2/pm34xx.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)
arch/arm/mach-omap2/pm34xx.c +4 -3
index c155c9d..51d5a5c 100644
@@ -375,7 +375,6 @@ void omap_sram_idle(void)
 		printk(KERN_ERR "Invalid mpu state in sram_idle\n");
 		return;
 	}
-	pwrdm_pre_transition();
 
 	/* NEON control */
 	if (pwrdm_read_pwrst(neon_pwrdm) == PWRDM_POWER_ON)
@@ -398,6 +397,8 @@ void omap_sram_idle(void)
 			if (!console_trylock())
 				goto console_still_active;
 
+	pwrdm_pre_transition();
+
 	/* PER */
 	if (per_next_state < PWRDM_POWER_ON) {
 		per_going_off = (per_next_state == PWRDM_POWER_OFF) ? 1 : 0;
@@ -467,6 +468,8 @@ void omap_sram_idle(void)
 	}
 	omap3_intc_resume_idle();
 
+	pwrdm_post_transition();
+
 	/* PER */
 	if (per_next_state < PWRDM_POWER_ON) {
 		per_prev_state = pwrdm_read_prev_pwrst(per_pwrdm);
@@ -490,8 +493,6 @@ console_still_active:
 		omap3_disable_io_chain();
 	}
 
-	pwrdm_post_transition();
-
 	clkdm_allow_idle(mpu_pwrdm->pwrdm_clkdms[0]);
 }
 
Signature · 1 line
1.7.0.4
#12 lore raw

PATCH v5 4/22 gpio/omap: fix pwrdm_post_transition call sequence

Quoted 7 lines · Tarun Kanti DebBarma

From: Charulatha V<[email protected]>

The context lost count is modified in omap_sram_idle() path when pwrdm_post_transition() is called. But pwrdm_post_transition() is called only after omap_gpio_resume_after_idle() is called. Correct this so that context lost count is modified before calling omap_gpio_resume_after_idle().

Quoted 5 lines

This would be useful when OMAP GPIO save/restore context is called by the OMAP GPIO driver itself.

---

Looks good to me.

regards
Santosh

#13 lore raw

PATCH v5 5/22 gpio/omap: handle save/restore ctx in GPIO driver

In reply to Tarun Kanti DebBarma

From: Charulatha V <[email protected]>

Modify omap_gpio_prepare_for_idle() & omap_gpio_resume_after_idle() functions to handle save context & restore context respectively in the OMAP GPIO driver itself instead of calling these functions from pm specific files. For this, in gpio_prepare_for_idle(), call *_get_context_loss_count() and in gpio_resume_after_idle() call it again. If the count is different, do restore context. The workaround_enabled flag is no more required and is removed.

---
 arch/arm/mach-omap2/gpio.c             |   12 +++
 arch/arm/mach-omap2/pm34xx.c           |   14 ----
 arch/arm/plat-omap/include/plat/gpio.h |    5 +-
 drivers/gpio/gpio-omap.c               |  131 ++++++++++++++------------------
 4 files changed, 73 insertions(+), 89 deletions(-)
arch/arm/mach-omap2/gpio.c +12 -0
index 95195a8..2e65377 100644
@@ -23,6 +23,7 @@
 
 #include <plat/omap_hwmod.h>
 #include <plat/omap_device.h>
+#include <plat/omap-pm.h>
 
 #include "powerdomain.h"
 
@@ -34,6 +35,16 @@ static struct omap_device_pm_latency omap_gpio_latency[] = {
 	},
 };
 
+#ifdef CONFIG_PM
+static int omap_gpio_get_context_loss(struct device *dev)
+{
+	return omap_pm_get_dev_context_loss_count(dev);
+}
+
+#else
+#define omap_gpio_get_context_loss NULL
+#endif
+
 static int omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused)
 {
 	struct omap_device *od;
@@ -63,6 +74,7 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused)
 	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_gpio_get_context_loss;
 
 	pdata->regs = kzalloc(sizeof(struct omap_gpio_reg_offs), GFP_KERNEL);
 	if (!pdata) {
arch/arm/mach-omap2/pm34xx.c +0 -14
index 51d5a5c..0aaa32c 100644
@@ -91,16 +91,6 @@ static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
 static struct powerdomain *core_pwrdm, *per_pwrdm;
 static struct powerdomain *cam_pwrdm;
 
-static inline void omap3_per_save_context(void)
-{
-	omap_gpio_save_context();
-}
-
-static inline void omap3_per_restore_context(void)
-{
-	omap_gpio_restore_context();
-}
-
 static void omap3_enable_io_chain(void)
 {
 	int timeout = 0;
@@ -405,8 +395,6 @@ void omap_sram_idle(void)
 		omap_uart_prepare_idle(2);
 		omap_uart_prepare_idle(3);
 		omap2_gpio_prepare_for_idle(per_going_off);
-		if (per_next_state == PWRDM_POWER_OFF)
-				omap3_per_save_context();
 	}
 
 	/* CORE */
@@ -474,8 +462,6 @@ void omap_sram_idle(void)
 	if (per_next_state < PWRDM_POWER_ON) {
 		per_prev_state = pwrdm_read_prev_pwrst(per_pwrdm);
 		omap2_gpio_resume_after_idle();
-		if (per_prev_state == PWRDM_POWER_OFF)
-			omap3_per_restore_context();
 		omap_uart_resume_idle(2);
 		omap_uart_resume_idle(3);
 	}
arch/arm/plat-omap/include/plat/gpio.h +3 -2
index 58d0bf2..f7798b5 100644
@@ -201,14 +201,15 @@ struct omap_gpio_platform_data {
 	bool loses_context;	/* whether the bank would ever lose context */
 
 	struct omap_gpio_reg_offs *regs;
+
+	/* Return context loss count due to PM states changing */
+	int (*get_context_loss_count)(struct device *dev);
 };
 
 extern void omap2_gpio_prepare_for_idle(int off_mode);
 extern void omap2_gpio_resume_after_idle(void);
 extern void omap_set_gpio_debounce(int gpio, int enable);
 extern void omap_set_gpio_debounce_time(int gpio, int enable);
-extern void omap_gpio_save_context(void);
-extern void omap_gpio_restore_context(void);
 /*-------------------------------------------------------------------------*/
 
 /* Wrappers for "new style" GPIO calls, using the new infrastructure
drivers/gpio/gpio-omap.c +58 -73

192 lines not on this page · raw message

Signature · 1 line
1.7.0.4
#14 lore raw

PATCH v5 5/22 gpio/omap: handle save/restore ctx in GPIO driver

Quoted 9 lines · Tarun Kanti DebBarma

From: Charulatha V<[email protected]>

Modify omap_gpio_prepare_for_idle()& omap_gpio_resume_after_idle() functions to handle save context& restore context respectively in the OMAP GPIO driver itself instead of calling these functions from pm specific files. For this, in gpio_prepare_for_idle(), call *_get_context_loss_count() and in gpio_resume_after_idle() call it again. If the count is different, do restore context. The workaround_enabled flag is no more required and is removed.

Quoted 3 lines

---

"ctx" ?
Please change it to 'context' is subject and rest of the code.

Quoted 19 lines
  arch/arm/mach-omap2/gpio.c             |   12 +++
  arch/arm/mach-omap2/pm34xx.c           |   14 ----
  arch/arm/plat-omap/include/plat/gpio.h |    5 +-
  drivers/gpio/gpio-omap.c               |  131 ++++++++++++++------------------
  4 files changed, 73 insertions(+), 89 deletions(-)
arch/arm/mach-omap2/gpio.c +1 -0
index 95195a8..2e65377 100644
@@ -23,6 +23,7 @@
 
  #include<plat/omap_hwmod.h>
  #include<plat/omap_device.h>
+#include<plat/omap-pm.h>
 
  #include "powerdomain.h"
 

arch/arm/mach-omap2/gpio.c static struct omap_device_pm_latency omap_gpio_latency[] = { · line 42

Quoted 8 lines
arch/arm/mach-omap2/gpio.c +5 -0
@@ -34,6 +35,16 @@ static struct omap_device_pm_latency omap_gpio_latency[] = {
  	},
  };
 
+#ifdef CONFIG_PM
+static int omap_gpio_get_context_loss(struct device *dev)
+{
+	return omap_pm_get_dev_context_loss_count(dev);
+}

Do you really need this wrapper. Use the funtion directly instead.

Rest looks fine to me

#15 lore raw

PATCH v5 5/22 gpio/omap: handle save/restore ctx in GPIO driver

Quoted 45 lines · Santosh
Quoted 13 lines · Tarun Kanti DebBarma

From: Charulatha V<[email protected]>

Modify omap_gpio_prepare_for_idle()& omap_gpio_resume_after_idle() functions
to handle save context& restore context respectively in the OMAP GPIO driver
itself instead of calling these functions from pm specific files. For this, in gpio_prepare_for_idle(), call *_get_context_loss_count() and in
gpio_resume_after_idle() call it again. If the count is different, do restore
context. The workaround_enabled flag is no more required and is removed.

Quoted 3 lines

---

"ctx" ?
Please change it to 'context' is subject and rest of the code.

Quoted 23 lines

arch/arm/mach-omap2/gpio.c | 12 +++ arch/arm/mach-omap2/pm34xx.c | 14 ---- arch/arm/plat-omap/include/plat/gpio.h | 5 +- drivers/gpio/gpio-omap.c | 131 ++++++++++++++------------------

4 files changed, 73 insertions(+), 89 deletions(-)
arch/arm/mach-omap2/gpio.c +1 -0
index 95195a8..2e65377 100644
@@ -23,6 +23,7 @@
 
 #include<plat/omap_hwmod.h>
 #include<plat/omap_device.h>
+#include<plat/omap-pm.h>
 
 #include "powerdomain.h"
 
@@ -34,6 +35,16 @@ static struct omap_device_pm_latency
 omap_gpio_latency[] = {
 },
 };
 
Quoted 8 lines

arch/arm/mach-omap2/gpio.c static struct omap_device_pm_latency omap_gpio_latency[] = { · line 42

Quoted 5 lines
arch/arm/mach-omap2/gpio.c +5 -0
+#ifdef CONFIG_PM
+static int omap_gpio_get_context_loss(struct device *dev)
+{
+ return omap_pm_get_dev_context_loss_count(dev);
+}

Do you really need this wrapper. Use the funtion directly instead.

Rest looks fine to me

One more thing I missed is, the dataout register save/restore As per your patches, it's OMAP version dependent to use set_dataout vs dataout. Please handle it as part of save/restore code.

Regards
Santosh

#16 lore raw

PATCH v5 6/22 gpio/omap: make non-wakeup GPIO part of pdata

In reply to Tarun Kanti DebBarma

From: Charulatha V <[email protected]>

Non-wakeup GPIOs are available only in OMAP2. Avoid cpu_is checks by making non_wakeup_gpios as part of pdata.

---
 arch/arm/mach-omap2/gpio.c             |    8 ++++++++
 arch/arm/plat-omap/include/plat/gpio.h |    1 +
 drivers/gpio/gpio-omap.c               |    8 +-------
 3 files changed, 10 insertions(+), 7 deletions(-)
arch/arm/mach-omap2/gpio.c +8 -0
index 2e65377..4952a9d 100644
@@ -84,6 +84,14 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused)
 
 	switch (oh->class->rev) {
 	case 0:
+		if (id == 1)
+			/* non-wakeup GPIO pins for OMAP2 Bank1 */
+			pdata->non_wakeup_gpios = 0xe203ffc0;
+		else if (id == 2)
+			/* non-wakeup GPIO pins for OMAP2 Bank2 */
+			pdata->non_wakeup_gpios = 0x08700040;
+		/* fall through */
+
 	case 1:
 		pdata->bank_type = METHOD_GPIO_24XX;
 		pdata->regs->revision = OMAP24XX_GPIO_REVISION;
arch/arm/plat-omap/include/plat/gpio.h +1 -0
index f7798b5..ce417cd 100644
@@ -199,6 +199,7 @@ struct omap_gpio_platform_data {
 	int bank_stride;	/* Only needed for omap1 MPUIO */
 	bool dbck_flag;		/* dbck required or not - True for OMAP3&4 */
 	bool loses_context;	/* whether the bank would ever lose context */
+	u32 non_wakeup_gpios;
 
 	struct omap_gpio_reg_offs *regs;
 
drivers/gpio/gpio-omap.c +1 -7
index 61c0da9..35ab3e9 100644
@@ -1021,13 +1021,6 @@ static void omap_gpio_mod_init(struct gpio_bank *bank)
 
 			/* Initialize interface clk ungated, module enabled */
 			__raw_writel(0, bank->base + OMAP24XX_GPIO_CTRL);
-		} else if (cpu_is_omap24xx()) {
-			static const u32 non_wakeup_gpios[] = {
-				0xe203ffc0, 0x08700040
-			};
-			if (bank->id < ARRAY_SIZE(non_wakeup_gpios))
-				bank->non_wakeup_gpios =
-						non_wakeup_gpios[bank->id];
 		}
 	} else if (cpu_class_is_omap1()) {
 		if (bank_is_mpuio(bank)) {
@@ -1175,6 +1168,7 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
 	bank->dbck_flag = pdata->dbck_flag;
 	bank->stride = pdata->bank_stride;
 	bank->width = pdata->bank_width;
+	bank->non_wakeup_gpios = pdata->non_wakeup_gpios;
 	bank->loses_context = pdata->loses_context;
 	bank->get_context_loss_count = pdata->get_context_loss_count;
 	bank->regs = pdata->regs;
Signature · 1 line
1.7.0.4
#18 lore raw

PATCH v5 7/22 gpio/omap: avoid cpu checks during module ena/disable

In reply to Tarun Kanti DebBarma

From: Charulatha V <[email protected]>

Remove cpu-is checks while enabling/disabling OMAP GPIO module during a gpio request/free.

---
 arch/arm/mach-omap2/gpio.c             |    2 +
 arch/arm/plat-omap/include/plat/gpio.h |    1 +
 drivers/gpio/gpio-omap.c               |   53 ++++++++++++++------------------
 3 files changed, 26 insertions(+), 30 deletions(-)
arch/arm/mach-omap2/gpio.c +2 -0
index 4952a9d..cdbc728 100644
@@ -107,6 +107,7 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused)
 		pdata->regs->clr_irqenable = OMAP24XX_GPIO_CLEARIRQENABLE1;
 		pdata->regs->debounce = OMAP24XX_GPIO_DEBOUNCE_VAL;
 		pdata->regs->debounce_en = OMAP24XX_GPIO_DEBOUNCE_EN;
+		pdata->regs->ctrl = OMAP24XX_GPIO_CTRL;
 		break;
 	case 2:
 		pdata->bank_type = METHOD_GPIO_44XX;
@@ -123,6 +124,7 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused)
 		pdata->regs->clr_irqenable = OMAP4_GPIO_IRQSTATUSCLR0;
 		pdata->regs->debounce = OMAP4_GPIO_DEBOUNCINGTIME;
 		pdata->regs->debounce_en = OMAP4_GPIO_DEBOUNCENABLE;
+		pdata->regs->ctrl = OMAP4_GPIO_CTRL;
 		break;
 	default:
 		WARN(1, "Invalid gpio bank_type\n");
arch/arm/plat-omap/include/plat/gpio.h +1 -0
index ce417cd..cf41743 100644
@@ -188,6 +188,7 @@ struct omap_gpio_reg_offs {
 	u16 clr_irqenable;
 	u16 debounce;
 	u16 debounce_en;
+	u16 ctrl;
 
 	bool irqenable_inv;
 };
drivers/gpio/gpio-omap.c +23 -30

73 lines not on this page · raw message

Signature · 1 line
1.7.0.4
#20 lore raw

PATCH v5 8/22 gpio/omap: further cleanup using wakeup_status register

In reply to Tarun Kanti DebBarma

Wakeup istatus register offset initialized according to OMAP versions during device registration. Use this to avoid version checks. Starting with OMAP4, legacy registers should not be used in combination with the updated regsiters. Use wkup_status register consistently for all SoCs wherever applicable.

---
 arch/arm/mach-omap1/gpio16xx.c         |    1 +
 arch/arm/mach-omap2/gpio.c             |    2 +
 arch/arm/plat-omap/include/plat/gpio.h |    1 +
 drivers/gpio/gpio-omap.c               |  109 ++++++--------------------------
 4 files changed, 24 insertions(+), 89 deletions(-)
arch/arm/mach-omap1/gpio16xx.c +1 -0
index df4bb44..ed9f285 100644
@@ -89,6 +89,7 @@ static struct omap_gpio_reg_offs omap16xx_gpio_regs = {
 	.irqenable	= OMAP1610_GPIO_IRQENABLE1,
 	.set_irqenable	= OMAP1610_GPIO_SET_IRQENABLE1,
 	.clr_irqenable	= OMAP1610_GPIO_CLEAR_IRQENABLE1,
+	.wkup_status	= OMAP1610_GPIO_WAKEUPENABLE,
 };
 
 static struct __initdata omap_gpio_platform_data omap16xx_gpio1_config = {
arch/arm/mach-omap2/gpio.c +2 -0
index cdbc728..fd05cea 100644
@@ -108,6 +108,7 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused)
 		pdata->regs->debounce = OMAP24XX_GPIO_DEBOUNCE_VAL;
 		pdata->regs->debounce_en = OMAP24XX_GPIO_DEBOUNCE_EN;
 		pdata->regs->ctrl = OMAP24XX_GPIO_CTRL;
+		pdata->regs->wkup_status = OMAP24XX_GPIO_WAKE_EN;
 		break;
 	case 2:
 		pdata->bank_type = METHOD_GPIO_44XX;
@@ -125,6 +126,7 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused)
 		pdata->regs->debounce = OMAP4_GPIO_DEBOUNCINGTIME;
 		pdata->regs->debounce_en = OMAP4_GPIO_DEBOUNCENABLE;
 		pdata->regs->ctrl = OMAP4_GPIO_CTRL;
+		pdata->regs->wkup_status = OMAP4_GPIO_IRQWAKEN0;
 		break;
 	default:
 		WARN(1, "Invalid gpio bank_type\n");
arch/arm/plat-omap/include/plat/gpio.h +1 -0
index cf41743..817997d 100644
@@ -189,6 +189,7 @@ struct omap_gpio_reg_offs {
 	u16 debounce;
 	u16 debounce_en;
 	u16 ctrl;
+	u16 wkup_status;
 
 	bool irqenable_inv;
 };
drivers/gpio/gpio-omap.c +20 -89

173 lines not on this page · raw message

Signature · 1 line
1.7.0.4
#21 lore raw

PATCH v5 8/22 gpio/omap: further cleanup using wakeup_status register

Quoted 1 line · Tarun Kanti DebBarma

Wakeup istatus register offset initialized according to OMAP versions

/s/ istatus / status

Quoted 19 lines

during device registration. Use this to avoid version checks. Starting with OMAP4, legacy registers should not be used in combination with the updated regsiters. Use wkup_status register consistently for all SoCs wherever applicable.

---
  arch/arm/mach-omap1/gpio16xx.c         |    1 +
  arch/arm/mach-omap2/gpio.c             |    2 +
  arch/arm/plat-omap/include/plat/gpio.h |    1 +
  drivers/gpio/gpio-omap.c               |  109 ++++++--------------------------
  4 files changed, 24 insertions(+), 89 deletions(-)
arch/arm/mach-omap1/gpio16xx.c +0 -0
index df4bb44..ed9f285 100644

arch/arm/mach-omap1/gpio16xx.c static struct omap_gpio_reg_offs omap16xx_gpio_regs = { · line 92

Quoted 4 lines
arch/arm/mach-omap1/gpio16xx.c +1 -0
@@ -89,6 +89,7 @@ static struct omap_gpio_reg_offs omap16xx_gpio_regs = {
  	.irqenable	= OMAP1610_GPIO_IRQENABLE1,
  	.set_irqenable	= OMAP1610_GPIO_SET_IRQENABLE1,
  	.clr_irqenable	= OMAP1610_GPIO_CLEAR_IRQENABLE1,
+	.wkup_status	= OMAP1610_GPIO_WAKEUPENABLE,

The register doesn't seems to be the status register. wkup_en is more appropriate.

Regards
Santosh

#22 lore raw

PATCH v5 9/22 gpio/omap: cleanup omap1 related macros

In reply to Tarun Kanti DebBarma

From: Charulatha V <[email protected]>

mpuio_init() function is defined under #ifdefs. It is required only in case of MPUIO bank type and only when PM operations are supported by it. This is applicable only in case of OMAP16xx SoC's MPUIO GPIO bank type. For all the other cases it is a dummy function. Hence clean up the same and remove all the OMAP SoC specific #ifdefs.

bank_is_mpuio() is defined as a check to identify if the bank type is MPUIO. It is not required to define it separately as zero for OMAP2PLUS. Remove this.

---
 drivers/gpio/gpio-omap.c |   19 -------------------
 1 files changed, 0 insertions(+), 19 deletions(-)
drivers/gpio/gpio-omap.c +0 -19
index 996e54b..e841ac1 100644
@@ -778,14 +778,8 @@ static struct irq_chip gpio_irq_chip = {
 
 /*---------------------------------------------------------------------*/
 
-#ifdef CONFIG_ARCH_OMAP1
-
 #define bank_is_mpuio(bank)	((bank)->method == METHOD_MPUIO)
 
-#ifdef CONFIG_ARCH_OMAP16XX
-
-#include <linux/platform_device.h>
-
 static int omap_mpuio_suspend_noirq(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
@@ -847,17 +841,6 @@ static inline void mpuio_init(struct gpio_bank *bank)
 		(void) platform_device_register(&omap_mpuio_device);
 }
 
-#else
-static inline void mpuio_init(struct gpio_bank *bank) {}
-#endif	/* 16xx */
-
-#else
-
-#define bank_is_mpuio(bank)	0
-static inline void mpuio_init(struct gpio_bank *bank) {}
-
-#endif
-
 /*---------------------------------------------------------------------*/
 
 /* REVISIT these are stupid implementations!  replace by ones that
@@ -1078,10 +1061,8 @@ static void __devinit omap_gpio_chip_init(struct gpio_bank *bank)
 	bank->chip.to_irq = gpio_2irq;
 	if (bank_is_mpuio(bank)) {
 		bank->chip.label = "mpuio";
-#ifdef CONFIG_ARCH_OMAP16XX
 		if (bank->regs->wkup_status)
 			bank->chip.dev = &omap_mpuio_device.dev;
-#endif
 		bank->chip.base = OMAP_MPUIO(0);
 	} else {
 		bank->chip.label = "gpio";
Signature · 1 line
1.7.0.4
#23 lore raw

PATCH v5 9/22 gpio/omap: cleanup omap1 related macros

Quoted 8 lines · Tarun Kanti DebBarma

From: Charulatha V<[email protected]>

mpuio_init() function is defined under #ifdefs. It is required only in case of MPUIO bank type and only when PM operations are supported by it. This is applicable only in case of OMAP16xx SoC's MPUIO GPIO bank type. For all the other cases it is a dummy function. Hence clean up the same and remove all the OMAP SoC specific #ifdefs.

Quoted 4 lines

bank_is_mpuio() is defined as a check to identify if the bank type is MPUIO. It is not required to define it separately as zero for OMAP2PLUS. Remove this.

Looks good though it can be cleaned up in the patch where 'bank_is_mpuio()' introduced.

Regards
Santosh

#24 lore raw

PATCH v5 10/22 gpio/omap: use level/edge detect reg offsets

In reply to Tarun Kanti DebBarma

By adding level and edge detection register offsets and then initializing them correctly according to OMAP versions during device registrations we can now remove lot of revision checks in these functions.

---
 arch/arm/mach-omap2/gpio.c             |    8 ++
 arch/arm/plat-omap/include/plat/gpio.h |    4 +
 drivers/gpio/gpio-omap.c               |  118 ++++++++++----------------------
 3 files changed, 48 insertions(+), 82 deletions(-)
arch/arm/mach-omap2/gpio.c +8 -0
index fd05cea..8745c3a 100644
@@ -109,6 +109,10 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused)
 		pdata->regs->debounce_en = OMAP24XX_GPIO_DEBOUNCE_EN;
 		pdata->regs->ctrl = OMAP24XX_GPIO_CTRL;
 		pdata->regs->wkup_status = OMAP24XX_GPIO_WAKE_EN;
+		pdata->regs->leveldetect0 = OMAP24XX_GPIO_LEVELDETECT0;
+		pdata->regs->leveldetect1 = OMAP24XX_GPIO_LEVELDETECT1;
+		pdata->regs->risingdetect = OMAP24XX_GPIO_RISINGDETECT;
+		pdata->regs->fallingdetect = OMAP24XX_GPIO_FALLINGDETECT;
 		break;
 	case 2:
 		pdata->bank_type = METHOD_GPIO_44XX;
@@ -127,6 +131,10 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused)
 		pdata->regs->debounce_en = OMAP4_GPIO_DEBOUNCENABLE;
 		pdata->regs->ctrl = OMAP4_GPIO_CTRL;
 		pdata->regs->wkup_status = OMAP4_GPIO_IRQWAKEN0;
+		pdata->regs->leveldetect0 = OMAP4_GPIO_LEVELDETECT0;
+		pdata->regs->leveldetect1 = OMAP4_GPIO_LEVELDETECT1;
+		pdata->regs->risingdetect = OMAP4_GPIO_RISINGDETECT;
+		pdata->regs->fallingdetect = OMAP4_GPIO_FALLINGDETECT;
 		break;
 	default:
 		WARN(1, "Invalid gpio bank_type\n");
arch/arm/plat-omap/include/plat/gpio.h +4 -0
index 817997d..7d12fe8 100644
@@ -190,6 +190,10 @@ struct omap_gpio_reg_offs {
 	u16 debounce_en;
 	u16 ctrl;
 	u16 wkup_status;
+	u16 leveldetect0;
+	u16 leveldetect1;
+	u16 risingdetect;
+	u16 fallingdetect;
 
 	bool irqenable_inv;
 };
drivers/gpio/gpio-omap.c +36 -82

168 lines not on this page · raw message

Signature · 1 line
1.7.0.4
#25 lore raw

PATCH v5 10/22 gpio/omap: use level/edge detect reg offsets

Quoted 7 lines · Tarun Kanti DebBarma

By adding level and edge detection register offsets and then initializing them correctly according to OMAP versions during device registrations we can now remove lot of revision checks in these functions.

---

Looks good.

Regards
Santosh

#26 lore raw

PATCH v5 11/22 gpio/omap: remove hardcoded offsets in ctxt save/restore

In reply to Tarun Kanti DebBarma

It is not required to use hard-coded offsets any more in context save and restore functions and instead use the generic offsets which have been correctly initialized during device registration.

---
 arch/arm/mach-omap2/gpio.c             |    2 +
 arch/arm/plat-omap/include/plat/gpio.h |    1 +
 drivers/gpio/gpio-omap.c               |   41 ++++++++++++++-----------------
 3 files changed, 22 insertions(+), 22 deletions(-)
arch/arm/mach-omap2/gpio.c +2 -0
index 8745c3a..0ee9a55 100644
@@ -103,6 +103,7 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused)
 		pdata->regs->irqstatus = OMAP24XX_GPIO_IRQSTATUS1;
 		pdata->regs->irqstatus2 = OMAP24XX_GPIO_IRQSTATUS2;
 		pdata->regs->irqenable = OMAP24XX_GPIO_IRQENABLE1;
+		pdata->regs->irqenable2 = OMAP24XX_GPIO_IRQENABLE2;
 		pdata->regs->set_irqenable = OMAP24XX_GPIO_SETIRQENABLE1;
 		pdata->regs->clr_irqenable = OMAP24XX_GPIO_CLEARIRQENABLE1;
 		pdata->regs->debounce = OMAP24XX_GPIO_DEBOUNCE_VAL;
@@ -125,6 +126,7 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused)
 		pdata->regs->irqstatus = OMAP4_GPIO_IRQSTATUS0;
 		pdata->regs->irqstatus2 = OMAP4_GPIO_IRQSTATUS1;
 		pdata->regs->irqenable = OMAP4_GPIO_IRQSTATUSSET0;
+		pdata->regs->irqenable2 = OMAP4_GPIO_IRQSTATUSSET1;
 		pdata->regs->set_irqenable = OMAP4_GPIO_IRQSTATUSSET0;
 		pdata->regs->clr_irqenable = OMAP4_GPIO_IRQSTATUSCLR0;
 		pdata->regs->debounce = OMAP4_GPIO_DEBOUNCINGTIME;
arch/arm/plat-omap/include/plat/gpio.h +1 -0
index 7d12fe8..0ad4f49 100644
@@ -184,6 +184,7 @@ struct omap_gpio_reg_offs {
 	u16 irqstatus;
 	u16 irqstatus2;
 	u16 irqenable;
+	u16 irqenable2;
 	u16 set_irqenable;
 	u16 clr_irqenable;
 	u16 debounce;
drivers/gpio/gpio-omap.c +19 -22

64 lines not on this page · raw message

Signature · 1 line
1.7.0.4
#27 lore raw

PATCH v5 11/22 gpio/omap: remove hardcoded offsets in ctxt save/restore

Quoted 7 lines · Tarun Kanti DebBarma

It is not required to use hard-coded offsets any more in context save and restore functions and instead use the generic offsets which have been correctly initialized during device registration.

---

$SUB
/s /ctx /context please

Quoted 45 lines
  arch/arm/mach-omap2/gpio.c             |    2 +
  arch/arm/plat-omap/include/plat/gpio.h |    1 +
  drivers/gpio/gpio-omap.c               |   41 ++++++++++++++-----------------
  3 files changed, 22 insertions(+), 22 deletions(-)
arch/arm/mach-omap2/gpio.c +2 -0
index 8745c3a..0ee9a55 100644
@@ -103,6 +103,7 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused)
  		pdata->regs->irqstatus = OMAP24XX_GPIO_IRQSTATUS1;
  		pdata->regs->irqstatus2 = OMAP24XX_GPIO_IRQSTATUS2;
  		pdata->regs->irqenable = OMAP24XX_GPIO_IRQENABLE1;
+		pdata->regs->irqenable2 = OMAP24XX_GPIO_IRQENABLE2;
  		pdata->regs->set_irqenable = OMAP24XX_GPIO_SETIRQENABLE1;
  		pdata->regs->clr_irqenable = OMAP24XX_GPIO_CLEARIRQENABLE1;
  		pdata->regs->debounce = OMAP24XX_GPIO_DEBOUNCE_VAL;
@@ -125,6 +126,7 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused)
  		pdata->regs->irqstatus = OMAP4_GPIO_IRQSTATUS0;
  		pdata->regs->irqstatus2 = OMAP4_GPIO_IRQSTATUS1;
  		pdata->regs->irqenable = OMAP4_GPIO_IRQSTATUSSET0;
+		pdata->regs->irqenable2 = OMAP4_GPIO_IRQSTATUSSET1;
  		pdata->regs->set_irqenable = OMAP4_GPIO_IRQSTATUSSET0;
  		pdata->regs->clr_irqenable = OMAP4_GPIO_IRQSTATUSCLR0;
  		pdata->regs->debounce = OMAP4_GPIO_DEBOUNCINGTIME;
arch/arm/plat-omap/include/plat/gpio.h +1 -0
index 7d12fe8..0ad4f49 100644
@@ -184,6 +184,7 @@ struct omap_gpio_reg_offs {
  	u16 irqstatus;
  	u16 irqstatus2;
  	u16 irqenable;
+	u16 irqenable2;
  	u16 set_irqenable;
  	u16 clr_irqenable;
  	u16 debounce;
drivers/gpio/gpio-omap.c +0 -0
index be561be..e7c9fe5 100644
@@ -1348,45 +1348,42 @@ void omap2_gpio_resume_after_idle(void)
  static void omap_gpio_save_context(struct gpio_bank *bank)
  {
  	bank->context.irqenable1 =

drivers/gpio/gpio-omap.c void omap2_gpio_resume_after_idle(void) · line 1355

Quoted 10 lines
drivers/gpio/gpio-omap.c +3 -5
-		__raw_readl(bank->base + OMAP24XX_GPIO_IRQENABLE1);
+			__raw_readl(bank->base + bank->regs->irqenable);
  	bank->context.irqenable2 =
-		__raw_readl(bank->base + OMAP24XX_GPIO_IRQENABLE2);
+			__raw_readl(bank->base + bank->regs->irqenable2);
  	bank->context.wake_en =
-		__raw_readl(bank->base + OMAP24XX_GPIO_WAKE_EN);
-	bank->context.ctrl = __raw_readl(bank->base + OMAP24XX_GPIO_CTRL);
-	bank->context.oe = __raw_readl(bank->base + OMAP24XX_GPIO_OE);
+			__raw_readl(bank->base + bank->regs->wkup_status);

Not related to this patch but saving the status register just useless. You should rather save/restore 'wkup_set' register.

Can you please fix this in relevant patch. O.w this patch looks ok to me.

Regards
Santosh

#28 lore raw

PATCH v5 12/22 gpio/omap: cleanup set_gpio_triggering function

In reply to Tarun Kanti DebBarma

Getting rid of ifdefs within the function by adding register offset intctrl and associating OMAPXXXX_GPIO_INT_CONTROL in respective SoC specific files. Also, use wkup_status register consistently instead of referring to wakeup clear and wakeup set register offsets.

---
 arch/arm/mach-omap1/gpio15xx.c         |    2 +
 arch/arm/mach-omap1/gpio16xx.c         |    3 +
 arch/arm/mach-omap1/gpio7xx.c          |    2 +
 arch/arm/plat-omap/include/plat/gpio.h |    3 +
 drivers/gpio/gpio-omap.c               |  159 ++++++++-----------------------
 5 files changed, 51 insertions(+), 118 deletions(-)
arch/arm/mach-omap1/gpio15xx.c +2 -0
index f8c15ea..2adfece 100644
@@ -42,6 +42,7 @@ static struct omap_gpio_reg_offs omap15xx_mpuio_regs = {
 	.irqstatus	= OMAP_MPUIO_GPIO_INT,
 	.irqenable	= OMAP_MPUIO_GPIO_MASKIT,
 	.irqenable_inv	= true,
+	.irqctrl	= OMAP_MPUIO_GPIO_INT_EDGE,
 };
 
 static struct __initdata omap_gpio_platform_data omap15xx_mpu_gpio_config = {
@@ -83,6 +84,7 @@ static struct omap_gpio_reg_offs omap15xx_gpio_regs = {
 	.irqstatus	= OMAP1510_GPIO_INT_STATUS,
 	.irqenable	= OMAP1510_GPIO_INT_MASK,
 	.irqenable_inv	= true,
+	.irqctrl	= OMAP1510_GPIO_INT_CONTROL,
 };
 
 static struct __initdata omap_gpio_platform_data omap15xx_gpio_config = {
arch/arm/mach-omap1/gpio16xx.c +3 -0
index ed9f285..f619805 100644
@@ -45,6 +45,7 @@ static struct omap_gpio_reg_offs omap16xx_mpuio_regs = {
 	.irqstatus	= OMAP_MPUIO_GPIO_INT,
 	.irqenable	= OMAP_MPUIO_GPIO_MASKIT,
 	.irqenable_inv	= true,
+	.irqctrl	= OMAP_MPUIO_GPIO_INT_EDGE,
 };
 
 static struct __initdata omap_gpio_platform_data omap16xx_mpu_gpio_config = {
@@ -90,6 +91,8 @@ static struct omap_gpio_reg_offs omap16xx_gpio_regs = {
 	.set_irqenable	= OMAP1610_GPIO_SET_IRQENABLE1,
 	.clr_irqenable	= OMAP1610_GPIO_CLEAR_IRQENABLE1,
 	.wkup_status	= OMAP1610_GPIO_WAKEUPENABLE,
+	.edgectrl1	= OMAP1610_GPIO_EDGE_CTRL1,
+	.edgectrl2	= OMAP1610_GPIO_EDGE_CTRL2,
 };
 
 static struct __initdata omap_gpio_platform_data omap16xx_gpio1_config = {
arch/arm/mach-omap1/gpio7xx.c +2 -0
index 923eaa1..cb083c5 100644
@@ -47,6 +47,7 @@ static struct omap_gpio_reg_offs omap7xx_mpuio_regs = {
 	.irqstatus	= OMAP_MPUIO_GPIO_INT / 2,
 	.irqenable	= OMAP_MPUIO_GPIO_MASKIT / 2,
 	.irqenable_inv	= true,
+	.irqctrl	= OMAP_MPUIO_GPIO_INT_EDGE / 2,
 };
 
 static struct __initdata omap_gpio_platform_data omap7xx_mpu_gpio_config = {
@@ -88,6 +89,7 @@ static struct omap_gpio_reg_offs omap7xx_gpio_regs = {
 	.irqstatus	= OMAP7XX_GPIO_INT_STATUS,
 	.irqenable	= OMAP7XX_GPIO_INT_MASK,
 	.irqenable_inv	= true,
+	.irqctrl	= OMAP7XX_GPIO_INT_CONTROL,
 };
 
 static struct __initdata omap_gpio_platform_data omap7xx_gpio1_config = {
arch/arm/plat-omap/include/plat/gpio.h +3 -0
index 0ad4f49..4f584de 100644
@@ -195,6 +195,9 @@ struct omap_gpio_reg_offs {
 	u16 leveldetect1;
 	u16 risingdetect;
 	u16 fallingdetect;
+	u16 irqctrl;
+	u16 edgectrl1;
+	u16 edgectrl2;
 
 	bool irqenable_inv;
 };
drivers/gpio/gpio-omap.c +41 -118

230 lines not on this page · raw message

Signature · 1 line
1.7.0.4
#29 lore raw

PATCH v5 12/22 gpio/omap: cleanup set_gpio_triggering function

Quoted 8 lines · Tarun Kanti DebBarma

Getting rid of ifdefs within the function by adding register offset intctrl and associating OMAPXXXX_GPIO_INT_CONTROL in respective SoC specific files. Also, use wkup_status register consistently instead of referring to wakeup clear and wakeup set register offsets.

---

Good. Some comments.

Quoted 53 lines
  arch/arm/mach-omap1/gpio15xx.c         |    2 +
  arch/arm/mach-omap1/gpio16xx.c         |    3 +
  arch/arm/mach-omap1/gpio7xx.c          |    2 +
  arch/arm/plat-omap/include/plat/gpio.h |    3 +
  drivers/gpio/gpio-omap.c               |  159 ++++++++-----------------------
  5 files changed, 51 insertions(+), 118 deletions(-)
arch/arm/mach-omap1/gpio15xx.c +2 -0
index f8c15ea..2adfece 100644
@@ -42,6 +42,7 @@ static struct omap_gpio_reg_offs omap15xx_mpuio_regs = {
  	.irqstatus	= OMAP_MPUIO_GPIO_INT,
  	.irqenable	= OMAP_MPUIO_GPIO_MASKIT,
  	.irqenable_inv	= true,
+	.irqctrl	= OMAP_MPUIO_GPIO_INT_EDGE,
  };
 
  static struct __initdata omap_gpio_platform_data omap15xx_mpu_gpio_config = {
@@ -83,6 +84,7 @@ static struct omap_gpio_reg_offs omap15xx_gpio_regs = {
  	.irqstatus	= OMAP1510_GPIO_INT_STATUS,
  	.irqenable	= OMAP1510_GPIO_INT_MASK,
  	.irqenable_inv	= true,
+	.irqctrl	= OMAP1510_GPIO_INT_CONTROL,
  };
 
  static struct __initdata omap_gpio_platform_data omap15xx_gpio_config = {
arch/arm/mach-omap1/gpio16xx.c +3 -0
index ed9f285..f619805 100644
@@ -45,6 +45,7 @@ static struct omap_gpio_reg_offs omap16xx_mpuio_regs = {
  	.irqstatus	= OMAP_MPUIO_GPIO_INT,
  	.irqenable	= OMAP_MPUIO_GPIO_MASKIT,
  	.irqenable_inv	= true,
+	.irqctrl	= OMAP_MPUIO_GPIO_INT_EDGE,
  };
 
  static struct __initdata omap_gpio_platform_data omap16xx_mpu_gpio_config = {
@@ -90,6 +91,8 @@ static struct omap_gpio_reg_offs omap16xx_gpio_regs = {
  	.set_irqenable	= OMAP1610_GPIO_SET_IRQENABLE1,
  	.clr_irqenable	= OMAP1610_GPIO_CLEAR_IRQENABLE1,
  	.wkup_status	= OMAP1610_GPIO_WAKEUPENABLE,
+	.edgectrl1	= OMAP1610_GPIO_EDGE_CTRL1,
+	.edgectrl2	= OMAP1610_GPIO_EDGE_CTRL2,
  };
 
  static struct __initdata omap_gpio_platform_data omap16xx_gpio1_config = {
arch/arm/mach-omap1/gpio7xx.c +0 -0
index 923eaa1..cb083c5 100644

arch/arm/mach-omap1/gpio7xx.c static struct omap_gpio_reg_offs omap7xx_mpuio_regs = { · line 50

Quoted 4 lines
arch/arm/mach-omap1/gpio7xx.c +1 -0
@@ -47,6 +47,7 @@ static struct omap_gpio_reg_offs omap7xx_mpuio_regs = {
  	.irqstatus	= OMAP_MPUIO_GPIO_INT / 2,
  	.irqenable	= OMAP_MPUIO_GPIO_MASKIT / 2,
  	.irqenable_inv	= true,
+	.irqctrl	= OMAP_MPUIO_GPIO_INT_EDGE / 2,

Shift operator would have been better here.

[...]

Quoted 4 lines
drivers/gpio/gpio-omap.c +0 -0
index e7c9fe5..21cb0d4 100644

[..]

drivers/gpio/gpio-omap.c static void _set_gpio_debounce(struct gpio_bank *bank, unsigned gpio,

Quoted 4 lines
drivers/gpio/gpio-omap.c +4 -0
+			trigger != 0);
+
+	/* This part needs to be executed always for OMAP34xx */
+	if (cpu_is_omap34xx() || (bank->non_wakeup_gpios&  gpio_bit)) {

Why ? You might want handle this special case with some flag instead.

Quoted 11 lines

/*
* Log the edge gpio and manually trigger the IRQ
* after resume if the input level changes

drivers/gpio/gpio-omap.c +0 -1
@@ -261,7 +236,6 @@ static inline void set_24xx_gpio_triggering(struct gpio_bank *bank, int gpio,
  		__raw_readl(bank->base + bank->regs->leveldetect0) |
  		__raw_readl(bank->base + bank->regs->leveldetect1);
  }
-#endif
 
  #ifdef CONFIG_ARCH_OMAP1
  /*

[...]

drivers/gpio/gpio-omap.c static int _set_gpio_triggering(struct gpio_bank *bank, int gpio, int trigger) · line 305

Quoted 6 lines
drivers/gpio/gpio-omap.c +3 -3
-#endif
-	default:
-		goto bad;
+
+		_gpio_rmw(base, bank->regs->wkup_status, 1<<  gpio, trigger);
+

Avoid this extra line.

Rest of the changes looks good to me.

Regards
Santosh

#30 lore raw

PATCH v5 13/22 gpio/omap: cleanup omap_gpio_mod_init function

In reply to Tarun Kanti DebBarma

With register offsets now defined for respective OMAP versions we can get rid of cpu_class_* checks. This function now has common initialization code for all OMAP versions. Initialization specific to OMAP16xx has been moved within omap16xx_gpio_init().

---
 arch/arm/mach-omap1/gpio16xx.c |   34 ++++++++++++++++++-
 drivers/gpio/gpio-omap.c       |   71 +++++++++++-----------------------------
 2 files changed, 52 insertions(+), 53 deletions(-)
arch/arm/mach-omap1/gpio16xx.c +33 -1
index f619805..61b1998 100644
@@ -24,6 +24,8 @@
 #define OMAP1610_GPIO4_BASE		0xfffbbc00
 #define OMAP1_MPUIO_VBASE		OMAP1_MPUIO_BASE
 
+#define SYSCONFIG_WORD			0x14 /* smart idle, enable wakeup */
+
 /* mpu gpio */
 static struct __initdata resource omap16xx_mpu_gpio_resources[] = {
 	{
@@ -218,12 +220,42 @@ static struct __initdata platform_device * omap16xx_gpio_dev[] = {
 static int __init omap16xx_gpio_init(void)
 {
 	int i;
+	void __iomem *base;
+	struct resource *res;
+	struct platform_device *pdev;
+	struct omap_gpio_platform_data *pdata;
 
 	if (!cpu_is_omap16xx())
 		return -EINVAL;
 
-	for (i = 0; i < ARRAY_SIZE(omap16xx_gpio_dev); i++)
+	for (i = 0; i < ARRAY_SIZE(omap16xx_gpio_dev); i++) {
+		pdev = omap16xx_gpio_dev[i];
+		pdata = pdev->dev.platform_data;
+
+		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+		if (unlikely(!res)) {
+			dev_err(&pdev->dev, "Invalid mem resource.\n");
+			return -ENODEV;
+		}
+
+		base = ioremap(res->start, resource_size(res));
+		if (unlikely(!base)) {
+			dev_err(&pdev->dev, "ioremap failed.\n");
+			return -ENOMEM;
+		}
+
+		__raw_writel(SYSCONFIG_WORD, base + OMAP1610_GPIO_SYSCONFIG);
+		iounmap(base);
+
+		/*
+		 * Enable system clock for GPIO module.
+		 * The CAM_CLK_CTRL *is* really the right place.
+		 */
+		omap_writel(omap_readl(ULPD_CAM_CLK_CTRL) | 0x04,
+					ULPD_CAM_CLK_CTRL);
+
 		platform_device_register(omap16xx_gpio_dev[i]);
+	}
 
 	return 0;
 }
drivers/gpio/gpio-omap.c +19 -52

87 lines not on this page · raw message

Signature · 1 line
1.7.0.4
#31 lore raw

PATCH v5 13/22 gpio/omap: cleanup omap_gpio_mod_init function

Quoted 12 lines · Tarun Kanti DebBarma

With register offsets now defined for respective OMAP versions we can get rid of cpu_class_* checks. This function now has common initialization code for all OMAP versions. Initialization specific to OMAP16xx has been moved within omap16xx_gpio_init().

---
  arch/arm/mach-omap1/gpio16xx.c |   34 ++++++++++++++++++-
  drivers/gpio/gpio-omap.c       |   71 +++++++++++-----------------------------
  2 files changed, 52 insertions(+), 53 deletions(-)

[...]

Quoted 4 lines
drivers/gpio/gpio-omap.c +0 -0
index 21cb0d4..f6855e5 100644

[...]

drivers/gpio/gpio-omap.c static int __devinit omap_gpio_probe(struct platform_device *pdev) · line 1041

Quoted 7 lines
drivers/gpio/gpio-omap.c +3 -0
@@ -1074,6 +1036,11 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
  	pm_runtime_enable(bank->dev);
  	pm_runtime_get_sync(bank->dev);
 
+	if (bank_is_mpuio(bank)) {
+		if (bank->regs->wkup_status)
+			mpuio_init(bank);

The above check looks strange. What are you trying here ?

Regards
Santosh

#32 lore raw

PATCH v5 14/22 gpio/omap: use pinctrl offset instead of macro

In reply to Tarun Kanti DebBarma

From: Charulatha V <[email protected]>

Use regs->pinctrl field instead of using the macro OMAP1510_GPIO_PIN_CONTROL

---
 arch/arm/mach-omap1/gpio15xx.c         |    1 +
 arch/arm/plat-omap/include/plat/gpio.h |    1 +
 drivers/gpio/gpio-omap.c               |    8 +++-----
 3 files changed, 5 insertions(+), 5 deletions(-)
arch/arm/mach-omap1/gpio15xx.c +1 -0
index 2adfece..950e467 100644
@@ -85,6 +85,7 @@ static struct omap_gpio_reg_offs omap15xx_gpio_regs = {
 	.irqenable	= OMAP1510_GPIO_INT_MASK,
 	.irqenable_inv	= true,
 	.irqctrl	= OMAP1510_GPIO_INT_CONTROL,
+	.pinctrl	= OMAP1510_GPIO_PIN_CONTROL,
 };
 
 static struct __initdata omap_gpio_platform_data omap15xx_gpio_config = {
arch/arm/plat-omap/include/plat/gpio.h +1 -0
index 4f584de..3a05c58 100644
@@ -198,6 +198,7 @@ struct omap_gpio_reg_offs {
 	u16 irqctrl;
 	u16 edgectrl1;
 	u16 edgectrl2;
+	u16 pinctrl;
 
 	bool irqenable_inv;
 };
drivers/gpio/gpio-omap.c +3 -5
index f6855e5..3d18cdf 100644
@@ -486,15 +486,13 @@ static int omap_gpio_request(struct gpio_chip *chip, unsigned offset)
 	 */
 	_set_gpio_triggering(bank, offset, IRQ_TYPE_NONE);
 
-#ifdef CONFIG_ARCH_OMAP15XX
-	if (bank->method == METHOD_GPIO_1510) {
-		void __iomem *reg;
+	if (bank->regs->pinctrl) {
+		void __iomem *reg = bank->base + bank->regs->pinctrl;
 
 		/* Claim the pin for MPU */
-		reg = bank->base + OMAP1510_GPIO_PIN_CONTROL;
 		__raw_writel(__raw_readl(reg) | (1 << offset), reg);
 	}
-#endif
+
 	if (bank->regs->ctrl && !bank->mod_usage) {
 		void __iomem *reg = bank->base + bank->regs->ctrl;
 		u32 ctrl;
Signature · 1 line
1.7.0.4
#34 lore raw

PATCH v5 15/22 gpio/omap: use readl in irq_handler for all access

In reply to Tarun Kanti DebBarma

From: Charulatha V <[email protected]>

Even when bank->width is 16, all the OMAP1 registers are 4-byte aligned, so just use a 4-byte read. The 'enabled' mask is already taking care to mask for bank width.

---
 drivers/gpio/gpio-omap.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)
drivers/gpio/gpio-omap.c +0 -2
index 3d18cdf..ba20e42 100644
@@ -572,8 +572,6 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
 		enabled = _get_gpio_irqbank_mask(bank);
 		isr_saved = isr = __raw_readl(isr_reg) & enabled;
 
-		if (cpu_is_omap15xx() && (bank->method == METHOD_MPUIO))
-			isr &= 0x0000ffff;
 
 		if (bank->level_mask)
 			level_mask = bank->level_mask & enabled;
Signature · 1 line
1.7.0.4
#35 lore raw

PATCH v5 15/22 gpio/omap: use readl in irq_handler for all access

Quoted 8 lines · Tarun Kanti DebBarma

From: Charulatha V<[email protected]>

Even when bank->width is 16, all the OMAP1 registers are 4-byte aligned, so just use a 4-byte read. The 'enabled' mask is already taking care to mask for bank width.

---

Patch $SUBJECT and the change don't match.

May be you want to say remove un-necessary bit masking since the register are 4 byte aligned and readl would work as is.

Quoted 16 lines
  drivers/gpio/gpio-omap.c |    2 --
  1 files changed, 0 insertions(+), 2 deletions(-)
drivers/gpio/gpio-omap.c +0 -2
index 3d18cdf..ba20e42 100644
@@ -572,8 +572,6 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
  		enabled = _get_gpio_irqbank_mask(bank);
  		isr_saved = isr = __raw_readl(isr_reg)&  enabled;
 
-		if (cpu_is_omap15xx()&&  (bank->method == METHOD_MPUIO))
-			isr&= 0x0000ffff;
 
  		if (bank->level_mask)
  			level_mask = bank->level_mask&  enabled;
#36 lore raw

PATCH v5 16/22 gpio/omap: remove bank->method & METHOD_* macros

In reply to Tarun Kanti DebBarma

From: Charulatha V <[email protected]>

The only bank->type (method) used in the OMAP GPIO driver is MPUIO type as they need to be handled separately. Identify the same using a flag and remove all METHOD_* macros.

---
 arch/arm/mach-omap1/gpio15xx.c         |    3 +--
 arch/arm/mach-omap1/gpio16xx.c         |    6 +-----
 arch/arm/mach-omap1/gpio7xx.c          |    8 +-------
 arch/arm/mach-omap2/gpio.c             |    2 --
 arch/arm/plat-omap/include/plat/gpio.h |    8 +-------
 drivers/gpio/gpio-omap.c               |   19 ++++++-------------
 6 files changed, 10 insertions(+), 36 deletions(-)
arch/arm/mach-omap1/gpio15xx.c +1 -2
index 950e467..634903e 100644
@@ -47,7 +47,7 @@ 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,
-	.bank_type		= METHOD_MPUIO,
+	.is_mpuio		= true,
 	.bank_width		= 16,
 	.bank_stride		= 1,
 	.regs			= &omap15xx_mpuio_regs,
@@ -90,7 +90,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_type		= METHOD_GPIO_1510,
 	.bank_width		= 16,
 	.regs                   = &omap15xx_gpio_regs,
 };
arch/arm/mach-omap1/gpio16xx.c +1 -5
index 61b1998..227e0e8 100644
@@ -52,7 +52,7 @@ 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,
-	.bank_type		= METHOD_MPUIO,
+	.is_mpuio		= true,
 	.bank_width		= 16,
 	.bank_stride		= 1,
 	.regs                   = &omap16xx_mpuio_regs,
@@ -99,7 +99,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_type		= METHOD_GPIO_1610,
 	.bank_width		= 16,
 	.regs                   = &omap16xx_gpio_regs,
 };
@@ -129,7 +128,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_type		= METHOD_GPIO_1610,
 	.bank_width		= 16,
 	.regs                   = &omap16xx_gpio_regs,
 };
@@ -159,7 +157,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_type		= METHOD_GPIO_1610,
 	.bank_width		= 16,
 	.regs                   = &omap16xx_gpio_regs,
 };
@@ -189,7 +186,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_type		= METHOD_GPIO_1610,
 	.bank_width		= 16,
 	.regs                   = &omap16xx_gpio_regs,
 };
arch/arm/mach-omap1/gpio7xx.c +1 -7
index cb083c5..8d25052 100644
@@ -52,8 +52,8 @@ 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,
-	.bank_type		= METHOD_MPUIO,
 	.bank_width		= 32,
+	.is_mpuio		= true,
 	.bank_stride		= 2,
 	.regs                   = &omap7xx_mpuio_regs,
 };
@@ -94,7 +94,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_type		= METHOD_GPIO_7XX,
 	.bank_width		= 32,
 	.regs			= &omap7xx_gpio_regs,
 };
@@ -124,7 +123,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_type		= METHOD_GPIO_7XX,
 	.bank_width		= 32,
 	.regs			= &omap7xx_gpio_regs,
 };
@@ -154,7 +152,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_type		= METHOD_GPIO_7XX,
 	.bank_width		= 32,
 	.regs			= &omap7xx_gpio_regs,
 };
@@ -184,7 +181,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_type		= METHOD_GPIO_7XX,
 	.bank_width		= 32,
 	.regs			= &omap7xx_gpio_regs,
 };
@@ -214,7 +210,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_type		= METHOD_GPIO_7XX,
 	.bank_width		= 32,
 	.regs			= &omap7xx_gpio_regs,
 };
@@ -244,7 +239,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_type		= METHOD_GPIO_7XX,
 	.bank_width		= 32,
 	.regs			= &omap7xx_gpio_regs,
 };
arch/arm/mach-omap2/gpio.c +0 -2
index 0ee9a55..3bbed93 100644
@@ -93,7 +93,6 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused)
 		/* fall through */
 
 	case 1:
-		pdata->bank_type = METHOD_GPIO_24XX;
 		pdata->regs->revision = OMAP24XX_GPIO_REVISION;
 		pdata->regs->direction = OMAP24XX_GPIO_OE;
 		pdata->regs->datain = OMAP24XX_GPIO_DATAIN;
@@ -116,7 +115,6 @@ static int omap2_gpio_dev_init(struct omap_hwmod *oh, void *unused)
 		pdata->regs->fallingdetect = OMAP24XX_GPIO_FALLINGDETECT;
 		break;
 	case 2:
-		pdata->bank_type = METHOD_GPIO_44XX;
 		pdata->regs->revision = OMAP4_GPIO_REVISION;
 		pdata->regs->direction = OMAP4_GPIO_OE;
 		pdata->regs->datain = OMAP4_GPIO_DATAIN;
arch/arm/plat-omap/include/plat/gpio.h +1 -7
index 3a05c58..acb5e2c 100644
@@ -162,13 +162,6 @@
 				 IH_MPUIO_BASE + ((nr) & 0x0f) : \
 				 IH_GPIO_BASE + (nr))
 
-#define METHOD_MPUIO		0
-#define METHOD_GPIO_1510	1
-#define METHOD_GPIO_1610	2
-#define METHOD_GPIO_7XX		3
-#define METHOD_GPIO_24XX	5
-#define METHOD_GPIO_44XX	6
-
 struct omap_gpio_dev_attr {
 	int bank_width;		/* GPIO bank width */
 	bool dbck_flag;		/* dbck required or not - True for OMAP3&4 */
@@ -210,6 +203,7 @@ struct omap_gpio_platform_data {
 	int bank_stride;	/* Only needed for omap1 MPUIO */
 	bool dbck_flag;		/* dbck required or not - True for OMAP3&4 */
 	bool loses_context;	/* whether the bank would ever lose context */
+	bool is_mpuio;		/* whether the bank is of type MPUIO */
 	u32 non_wakeup_gpios;
 
 	struct omap_gpio_reg_offs *regs;
drivers/gpio/gpio-omap.c +6 -13
index ba20e42..0545d8f 100644
@@ -49,7 +49,6 @@ struct gpio_bank {
 	void __iomem *base;
 	u16 irq;
 	u16 virtual_irq_start;
-	int method;
 	u32 suspend_wakeup;
 	u32 saved_wakeup;
 	u32 non_wakeup_gpios;
@@ -66,6 +65,7 @@ struct gpio_bank {
 	u32 mod_usage;
 	u32 dbck_enable_mask;
 	struct device *dev;
+	bool is_mpuio;
 	bool dbck_flag;
 	bool loses_context;
 	int stride;
@@ -690,8 +690,6 @@ static struct irq_chip gpio_irq_chip = {
 
 /*---------------------------------------------------------------------*/
 
-#define bank_is_mpuio(bank)	((bank)->method == METHOD_MPUIO)
-
 static int omap_mpuio_suspend_noirq(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
@@ -755,10 +753,6 @@ static inline void mpuio_init(struct gpio_bank *bank)
 
 /*---------------------------------------------------------------------*/
 
-/* REVISIT these are stupid implementations!  replace by ones that
- * don't switch on METHOD_* and which mostly avoid spinlocks
- */
-
 static int gpio_input(struct gpio_chip *chip, unsigned offset)
 {
 	struct gpio_bank *bank;
@@ -879,7 +873,7 @@ static void omap_gpio_mod_init(struct gpio_bank *bank)
 	if (bank->width == 16)
 		l = 0xffff;
 
-	if (bank_is_mpuio(bank)) {
+	if (bank->is_mpuio) {
 		__raw_writel(l, bank->base + bank->regs->irqenable);
 		return;
 	}
@@ -920,7 +914,6 @@ static void __devinit omap_gpio_chip_init(struct gpio_bank *bank)
 	int j;
 	static int gpio;
 
-	bank->mod_usage = 0;
 	/*
 	 * REVISIT eventually switch from OMAP-specific gpio structs
 	 * over to the generic ones
@@ -933,7 +926,7 @@ static void __devinit omap_gpio_chip_init(struct gpio_bank *bank)
 	bank->chip.set_debounce = gpio_debounce;
 	bank->chip.set = gpio_set;
 	bank->chip.to_irq = gpio_2irq;
-	if (bank_is_mpuio(bank)) {
+	if (bank->is_mpuio) {
 		bank->chip.label = "mpuio";
 		if (bank->regs->wkup_status)
 			bank->chip.dev = &omap_mpuio_device.dev;
@@ -951,7 +944,7 @@ static void __devinit omap_gpio_chip_init(struct gpio_bank *bank)
 		     j < bank->virtual_irq_start + bank->width; j++) {
 		irq_set_lockdep_class(j, &gpio_lock_class);
 		irq_set_chip_data(j, bank);
-		if (bank_is_mpuio(bank)) {
+		if (bank->is_mpuio) {
 			omap_mpuio_alloc_gc(bank, j, bank->width);
 		} else {
 			irq_set_chip(j, &gpio_irq_chip);
@@ -995,11 +988,11 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
 
 	pdata = pdev->dev.platform_data;
 	bank->virtual_irq_start = pdata->virtual_irq_start;
-	bank->method = pdata->bank_type;
 	bank->dev = &pdev->dev;
 	bank->dbck_flag = pdata->dbck_flag;
 	bank->stride = pdata->bank_stride;
 	bank->width = pdata->bank_width;
+	bank->is_mpuio = pdata->is_mpuio;
 	bank->non_wakeup_gpios = pdata->non_wakeup_gpios;
 	bank->loses_context = pdata->loses_context;
 	bank->get_context_loss_count = pdata->get_context_loss_count;
@@ -1032,7 +1025,7 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
 	pm_runtime_enable(bank->dev);
 	pm_runtime_get_sync(bank->dev);
 
-	if (bank_is_mpuio(bank)) {
+	if (bank->is_mpuio) {
 		if (bank->regs->wkup_status)
 			mpuio_init(bank);
 	}
Signature · 1 line
1.7.0.4
#37 lore raw

PATCH v5 16/22 gpio/omap: remove bank->method & METHOD_* macros

Quoted 9 lines · Tarun Kanti DebBarma

From: Charulatha V<[email protected]>

The only bank->type (method) used in the OMAP GPIO driver is MPUIO type as they need to be handled separately. Identify the same using a flag and remove all METHOD_* macros.

---

Looks good.

Regards
Santosh

#38 lore raw

PATCH v5 17/22 gpio/omap: fix bankwidth for OMAP7xx MPUIO

In reply to Tarun Kanti DebBarma

From: Charulatha V <[email protected]>

In all OMAP1 SoCs, the MPUIO bank width is 16 bits. But, in OMAP7xx, it is wrongly initialised to 32. Fix this.

---
 arch/arm/mach-omap1/gpio7xx.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/mach-omap1/gpio7xx.c +1 -1
index 8d25052..3ca9600 100644
@@ -52,8 +52,8 @@ 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,
-	.bank_width		= 32,
 	.is_mpuio		= true,
+	.bank_width		= 16,
 	.bank_stride		= 2,
 	.regs                   = &omap7xx_mpuio_regs,
 };
Signature · 1 line
1.7.0.4
#40 lore raw

PATCH v5 18/22 gpio/omap: use pm-runtime framework

In reply to Tarun Kanti DebBarma

Call runtime pm APIs pm_runtime_get_sync() and pm_runtime_put_sync() for enabling/disabling clocks appropriately. Remove syscore_ops and instead use dev_pm_ops now.

---
 drivers/gpio/gpio-omap.c |   89 ++++++++++++++++++++++++++++++++++-----------
 1 files changed, 67 insertions(+), 22 deletions(-)
drivers/gpio/gpio-omap.c +67 -22
index 0545d8f..bca6dcd 100644
@@ -79,6 +79,8 @@ struct gpio_bank {
 	struct omap_gpio_reg_offs *regs;
 };
 
+static void omap_gpio_mod_init(struct gpio_bank *bank);
+
 #define GPIO_INDEX(bank, gpio) (gpio % bank->width)
 #define GPIO_BIT(bank, gpio) (1 << GPIO_INDEX(bank, gpio))
 #define GPIO_MOD_CTRL_BIT	BIT(0)
@@ -481,6 +483,19 @@ static int omap_gpio_request(struct gpio_chip *chip, unsigned offset)
 
 	spin_lock_irqsave(&bank->lock, flags);
 
+	/*
+	 * If this is the first gpio_request for the bank,
+	 * enable the bank module.
+	 */
+	if (!bank->mod_usage)
+		if (IS_ERR_VALUE(pm_runtime_get_sync(bank->dev) < 0)) {
+			dev_err(bank->dev, "%s: GPIO bank %d "
+					"pm_runtime_get_sync failed\n",
+					__func__, bank->id);
+			spin_unlock_irqrestore(&bank->lock, flags);
+			return -EINVAL;
+		}
+
 	/* Set trigger to none. You need to enable the desired trigger with
 	 * request_irq() or set_irq_type().
 	 */
@@ -535,6 +550,19 @@ static void omap_gpio_free(struct gpio_chip *chip, unsigned offset)
 	}
 
 	_reset_gpio(bank, bank->chip.base + offset);
+
+	/*
+	 * If this is the last gpio to be freed in the bank,
+	 * disable the bank module.
+	 */
+	if (!bank->mod_usage) {
+		if (IS_ERR_VALUE(pm_runtime_put_sync(bank->dev) < 0)) {
+			dev_err(bank->dev, "%s: GPIO bank %d "
+					"pm_runtime_put_sync failed\n",
+					__func__, bank->id);
+		}
+	}
+
 	spin_unlock_irqrestore(&bank->lock, flags);
 }
 
@@ -561,6 +589,7 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
 
 	bank = irq_get_handler_data(irq);
 	isr_reg = bank->base + bank->regs->irqstatus;
+	pm_runtime_get_sync(bank->dev);
 
 	if (WARN_ON(!isr_reg))
 		goto exit;
@@ -622,6 +651,7 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
 exit:
 	if (!unmasked)
 		chained_irq_exit(chip, desc);
+	pm_runtime_put_sync_suspend(bank->dev);
 }
 
 static void gpio_irq_shutdown(struct irq_data *d)
@@ -1023,7 +1053,13 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
 	}
 
 	pm_runtime_enable(bank->dev);
-	pm_runtime_get_sync(bank->dev);
+	pm_runtime_irq_safe(bank->dev);
+	if (IS_ERR_VALUE(pm_runtime_get_sync(bank->dev) < 0)) {
+		dev_err(bank->dev, "%s: GPIO bank %d pm_runtime_get_sync "
+				"failed\n", __func__, bank->id);
+		iounmap(bank->base);
+		return -EINVAL;
+	}
 
 	if (bank->is_mpuio) {
 		if (bank->regs->wkup_status)
@@ -1034,6 +1070,13 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
 	omap_gpio_chip_init(bank);
 	omap_gpio_show_rev(bank);
 
+	if (IS_ERR_VALUE(pm_runtime_put_sync(bank->dev) < 0)) {
+		dev_err(bank->dev, "%s: GPIO bank %d pm_runtime_put_sync "
+				"failed\n", __func__, bank->id);
+		iounmap(bank->base);
+		return -EINVAL;
+	}
+
 	list_add_tail(&bank->node, &omap_gpio_list);
 
 	return ret;
@@ -1044,7 +1087,7 @@ err_exit:
 	return ret;
 }
 
-static int omap_gpio_suspend(void)
+static int omap_gpio_suspend(struct device *dev)
 {
 	struct gpio_bank *bank;
 
@@ -1063,12 +1106,13 @@ static int omap_gpio_suspend(void)
 		_gpio_rmw(base, bank->regs->wkup_status,
 					bank->suspend_wakeup, 1);
 		spin_unlock_irqrestore(&bank->lock, flags);
+		pm_runtime_put_sync(dev);
 	}
 
 	return 0;
 }
 
-static void omap_gpio_resume(void)
+static int omap_gpio_resume(struct device *dev)
 {
 	struct gpio_bank *bank;
 
@@ -1077,18 +1121,16 @@ static void omap_gpio_resume(void)
 		unsigned long flags;
 
 		if (!bank->regs->wkup_status)
-			return;
+			return 0;
 
+		pm_runtime_get_sync(dev);
 		spin_lock_irqsave(&bank->lock, flags);
 		_gpio_rmw(base, bank->regs->wkup_status, bank->saved_wakeup, 1);
 		spin_unlock_irqrestore(&bank->lock, flags);
 	}
-}
 
-static struct syscore_ops omap_gpio_syscore_ops = {
-	.suspend	= omap_gpio_suspend,
-	.resume		= omap_gpio_resume,
-};
+	return 0;
+}
 
 #ifdef CONFIG_ARCH_OMAP2PLUS
 
@@ -1112,6 +1154,11 @@ void omap2_gpio_prepare_for_idle(int off_mode)
 		if (!off_mode)
 			continue;
 
+		if (IS_ERR_VALUE(pm_runtime_put_sync(bank->dev) < 0))
+			dev_err(bank->dev, "%s: GPIO bank %d "
+					"pm_runtime_put_sync failed\n",
+					__func__, bank->id);
+
 		/* If going to OFF, remove triggering for all
 		 * non-wakeup GPIOs.  Otherwise spurious IRQs will be
 		 * generated.  See OMAP2420 Errata item 1.101. */
@@ -1152,6 +1199,11 @@ void omap2_gpio_resume_after_idle(void)
 		if (!bank->loses_context)
 			continue;
 
+		if (IS_ERR_VALUE(pm_runtime_get_sync(bank->dev) < 0))
+			dev_err(bank->dev, "%s: GPIO bank %d "
+					"pm_runtime_get_sync failed\n",
+					__func__, bank->id);
+
 		for (j = 0; j < hweight_long(bank->dbck_enable_mask); j++)
 			clk_enable(bank->dbck);
 
@@ -1266,10 +1318,16 @@ static void omap_gpio_restore_context(struct gpio_bank *bank)
 }
 #endif
 
+static const struct dev_pm_ops gpio_pm_ops = {
+	.suspend		= omap_gpio_suspend,
+	.resume			= omap_gpio_resume,
+};
+
 static struct platform_driver omap_gpio_driver = {
 	.probe		= omap_gpio_probe,
 	.driver		= {
 		.name	= "omap_gpio",
+		.pm	= &gpio_pm_ops,
 	},
 };
 
@@ -1283,16 +1341,3 @@ static int __init omap_gpio_drv_reg(void)
 	return platform_driver_register(&omap_gpio_driver);
 }
 postcore_initcall(omap_gpio_drv_reg);
-
-static int __init omap_gpio_sysinit(void)
-{
-
-#if defined(CONFIG_ARCH_OMAP16XX) || defined(CONFIG_ARCH_OMAP2PLUS)
-	if (cpu_is_omap16xx() || cpu_class_is_omap2())
-		register_syscore_ops(&omap_gpio_syscore_ops);
-#endif
-
-	return 0;
-}
-
-arch_initcall(omap_gpio_sysinit);
Signature · 1 line
1.7.0.4
#41 lore raw

PATCH v5 18/22 gpio/omap: use pm-runtime framework

+ Rajendra and Benoit to comment on optional clock handling.

Quoted 168 lines · Tarun Kanti DebBarma

Call runtime pm APIs pm_runtime_get_sync() and pm_runtime_put_sync() for enabling/disabling clocks appropriately. Remove syscore_ops and instead use dev_pm_ops now.

---
  drivers/gpio/gpio-omap.c |   89 ++++++++++++++++++++++++++++++++++-----------
  1 files changed, 67 insertions(+), 22 deletions(-)
drivers/gpio/gpio-omap.c +56 -9
index 0545d8f..bca6dcd 100644
@@ -79,6 +79,8 @@ struct gpio_bank {
  	struct omap_gpio_reg_offs *regs;
  };
 
+static void omap_gpio_mod_init(struct gpio_bank *bank);
+
  #define GPIO_INDEX(bank, gpio) (gpio % bank->width)
  #define GPIO_BIT(bank, gpio) (1<<  GPIO_INDEX(bank, gpio))
  #define GPIO_MOD_CTRL_BIT	BIT(0)
@@ -481,6 +483,19 @@ static int omap_gpio_request(struct gpio_chip *chip, unsigned offset)
 
  	spin_lock_irqsave(&bank->lock, flags);
 
+	/*
+	 * If this is the first gpio_request for the bank,
+	 * enable the bank module.
+	 */
+	if (!bank->mod_usage)
+		if (IS_ERR_VALUE(pm_runtime_get_sync(bank->dev)<  0)) {
+			dev_err(bank->dev, "%s: GPIO bank %d "
+					"pm_runtime_get_sync failed\n",
+					__func__, bank->id);
+			spin_unlock_irqrestore(&bank->lock, flags);
+			return -EINVAL;
+		}
+
  	/* Set trigger to none. You need to enable the desired trigger with
  	 * request_irq() or set_irq_type().
  	 */
@@ -535,6 +550,19 @@ static void omap_gpio_free(struct gpio_chip *chip, unsigned offset)
  	}
 
  	_reset_gpio(bank, bank->chip.base + offset);
+
+	/*
+	 * If this is the last gpio to be freed in the bank,
+	 * disable the bank module.
+	 */
+	if (!bank->mod_usage) {
+		if (IS_ERR_VALUE(pm_runtime_put_sync(bank->dev)<  0)) {
+			dev_err(bank->dev, "%s: GPIO bank %d "
+					"pm_runtime_put_sync failed\n",
+					__func__, bank->id);
+		}
+	}
+
  	spin_unlock_irqrestore(&bank->lock, flags);
  }
 
@@ -561,6 +589,7 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
 
  	bank = irq_get_handler_data(irq);
  	isr_reg = bank->base + bank->regs->irqstatus;
+	pm_runtime_get_sync(bank->dev);
 
  	if (WARN_ON(!isr_reg))
  		goto exit;
@@ -622,6 +651,7 @@ static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
  exit:
  	if (!unmasked)
  		chained_irq_exit(chip, desc);
+	pm_runtime_put_sync_suspend(bank->dev);
  }
 
  static void gpio_irq_shutdown(struct irq_data *d)
@@ -1023,7 +1053,13 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
  	}
 
  	pm_runtime_enable(bank->dev);
-	pm_runtime_get_sync(bank->dev);
+	pm_runtime_irq_safe(bank->dev);
+	if (IS_ERR_VALUE(pm_runtime_get_sync(bank->dev)<  0)) {
+		dev_err(bank->dev, "%s: GPIO bank %d pm_runtime_get_sync "
+				"failed\n", __func__, bank->id);
+		iounmap(bank->base);
+		return -EINVAL;
+	}
 
  	if (bank->is_mpuio) {
  		if (bank->regs->wkup_status)
@@ -1034,6 +1070,13 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
  	omap_gpio_chip_init(bank);
  	omap_gpio_show_rev(bank);
 
+	if (IS_ERR_VALUE(pm_runtime_put_sync(bank->dev)<  0)) {
+		dev_err(bank->dev, "%s: GPIO bank %d pm_runtime_put_sync "
+				"failed\n", __func__, bank->id);
+		iounmap(bank->base);
+		return -EINVAL;
+	}
+
  	list_add_tail(&bank->node,&omap_gpio_list);
 
  	return ret;
@@ -1044,7 +1087,7 @@ err_exit:
  	return ret;
  }
 
-static int omap_gpio_suspend(void)
+static int omap_gpio_suspend(struct device *dev)
  {
  	struct gpio_bank *bank;
 
@@ -1063,12 +1106,13 @@ static int omap_gpio_suspend(void)
  		_gpio_rmw(base, bank->regs->wkup_status,
  					bank->suspend_wakeup, 1);
  		spin_unlock_irqrestore(&bank->lock, flags);
+		pm_runtime_put_sync(dev);
  	}
 
  	return 0;
  }
 
-static void omap_gpio_resume(void)
+static int omap_gpio_resume(struct device *dev)
  {
  	struct gpio_bank *bank;
 
@@ -1077,18 +1121,16 @@ static void omap_gpio_resume(void)
  		unsigned long flags;
 
  		if (!bank->regs->wkup_status)
-			return;
+			return 0;
 
+		pm_runtime_get_sync(dev);
  		spin_lock_irqsave(&bank->lock, flags);
  		_gpio_rmw(base, bank->regs->wkup_status, bank->saved_wakeup, 1);
  		spin_unlock_irqrestore(&bank->lock, flags);
  	}
-}
 
-static struct syscore_ops omap_gpio_syscore_ops = {
-	.suspend	= omap_gpio_suspend,
-	.resume		= omap_gpio_resume,
-};
+	return 0;
+}
 
  #ifdef CONFIG_ARCH_OMAP2PLUS
 
@@ -1112,6 +1154,11 @@ void omap2_gpio_prepare_for_idle(int off_mode)
  		if (!off_mode)
  			continue;
 
+		if (IS_ERR_VALUE(pm_runtime_put_sync(bank->dev)<  0))
+			dev_err(bank->dev, "%s: GPIO bank %d "
+					"pm_runtime_put_sync failed\n",
+					__func__, bank->id);
+
  		/* If going to OFF, remove triggering for all
  		 * non-wakeup GPIOs.  Otherwise spurious IRQs will be
  		 * generated.  See OMAP2420 Errata item 1.101. */

drivers/gpio/gpio-omap.c void omap2_gpio_resume_after_idle(void) · line 1208

Quoted 10 lines
drivers/gpio/gpio-omap.c +5 -0
@@ -1152,6 +1199,11 @@ void omap2_gpio_resume_after_idle(void)
  		if (!bank->loses_context)
  			continue;
 
+		if (IS_ERR_VALUE(pm_runtime_get_sync(bank->dev)<  0))
+			dev_err(bank->dev, "%s: GPIO bank %d "
+					"pm_runtime_get_sync failed\n",
+					__func__, bank->id);
+
  		for (j = 0; j<  hweight_long(bank->dbck_enable_mask); j++)
  			clk_enable(bank->dbck);

Optional clock handling also should been addressed using runtime hooks, isn't it ?

We still seems to use clock framework here?

Regards
Santosh

#42 lore raw

PATCH v5 18/22 gpio/omap: use pm-runtime framework

Quoted 182 lines · Santosh

+ Rajendra and Benoit to comment on optional clock handling.

Quoted 178 lines · Tarun Kanti DebBarma

Call runtime pm APIs pm_runtime_get_sync() and pm_runtime_put_sync() for enabling/disabling clocks appropriately. Remove syscore_ops and instead use dev_pm_ops now.

---
drivers/gpio/gpio-omap.c | 89
++++++++++++++++++++++++++++++++++-----------

1 files changed, 67 insertions(+), 22 deletions(-)
drivers/gpio/gpio-omap.c +15 -0
index 0545d8f..bca6dcd 100644
@@ -79,6 +79,8 @@ struct gpio_bank {
 struct omap_gpio_reg_offs *regs;
 };
 
+static void omap_gpio_mod_init(struct gpio_bank *bank);
+
 #define GPIO_INDEX(bank, gpio) (gpio % bank->width)
 #define GPIO_BIT(bank, gpio) (1<< GPIO_INDEX(bank, gpio))
 #define GPIO_MOD_CTRL_BIT BIT(0)
@@ -481,6 +483,19 @@ static int omap_gpio_request(struct gpio_chip
 *chip, unsigned offset)
 
 spin_lock_irqsave(&bank->lock, flags);
 
+ /*
+ * If this is the first gpio_request for the bank,
+ * enable the bank module.
+ */
+ if (!bank->mod_usage)
+ if (IS_ERR_VALUE(pm_runtime_get_sync(bank->dev)< 0)) {
+ dev_err(bank->dev, "%s: GPIO bank %d "
+ "pm_runtime_get_sync failed\n",
+ __func__, bank->id);
+ spin_unlock_irqrestore(&bank->lock, flags);
+ return -EINVAL;
+ }
+
 /* Set trigger to none. You need to enable the desired trigger with
 * request_irq() or set_irq_type().

*/

drivers/gpio/gpio-omap.c +14 -0
@@ -535,6 +550,19 @@ static void omap_gpio_free(struct gpio_chip
 *chip, unsigned offset)
 }
 
 _reset_gpio(bank, bank->chip.base + offset);
+
+ /*
+ * If this is the last gpio to be freed in the bank,
+ * disable the bank module.
+ */
+ if (!bank->mod_usage) {
+ if (IS_ERR_VALUE(pm_runtime_put_sync(bank->dev)< 0)) {
+ dev_err(bank->dev, "%s: GPIO bank %d "
+ "pm_runtime_put_sync failed\n",
+ __func__, bank->id);
+ }
+ }
+
 spin_unlock_irqrestore(&bank->lock, flags);
 }
@@ -561,6 +589,7 @@ static void gpio_irq_handler(unsigned int irq,
 struct irq_desc *desc)
 
 bank = irq_get_handler_data(irq);
 isr_reg = bank->base + bank->regs->irqstatus;
+ pm_runtime_get_sync(bank->dev);
 
 if (WARN_ON(!isr_reg))

goto exit;

drivers/gpio/gpio-omap.c +0 -0
@@ -622,6 +651,7 @@ static void gpio_irq_handler(unsigned int irq,
 struct irq_desc *desc)

exit:
if (!unmasked)
chained_irq_exit(chip, desc);
+ pm_runtime_put_sync_suspend(bank->dev);
}

static void gpio_irq_shutdown(struct irq_data *d)

drivers/gpio/gpio-omap.c +7 -1
@@ -1023,7 +1053,13 @@ static int __devinit omap_gpio_probe(struct
 platform_device *pdev)
 }
 
 pm_runtime_enable(bank->dev);
- pm_runtime_get_sync(bank->dev);
+ pm_runtime_irq_safe(bank->dev);
+ if (IS_ERR_VALUE(pm_runtime_get_sync(bank->dev)< 0)) {
+ dev_err(bank->dev, "%s: GPIO bank %d pm_runtime_get_sync "
+ "failed\n", __func__, bank->id);
+ iounmap(bank->base);
+ return -EINVAL;
+ }
 
 if (bank->is_mpuio) {

if (bank->regs->wkup_status)

drivers/gpio/gpio-omap.c +7 -0
@@ -1034,6 +1070,13 @@ static int __devinit omap_gpio_probe(struct
 platform_device *pdev)
 omap_gpio_chip_init(bank);
 omap_gpio_show_rev(bank);
 
+ if (IS_ERR_VALUE(pm_runtime_put_sync(bank->dev)< 0)) {
+ dev_err(bank->dev, "%s: GPIO bank %d pm_runtime_put_sync "
+ "failed\n", __func__, bank->id);
+ iounmap(bank->base);
+ return -EINVAL;
+ }
+
 list_add_tail(&bank->node,&omap_gpio_list);
 

return ret;

drivers/gpio/gpio-omap.c +12 -8
@@ -1044,7 +1087,7 @@ err_exit:
 return ret;
 }
 
-static int omap_gpio_suspend(void)
+static int omap_gpio_suspend(struct device *dev)
 {
 struct gpio_bank *bank;
 
@@ -1063,12 +1106,13 @@ static int omap_gpio_suspend(void)
 _gpio_rmw(base, bank->regs->wkup_status,
 bank->suspend_wakeup, 1);
 spin_unlock_irqrestore(&bank->lock, flags);
+ pm_runtime_put_sync(dev);
 }
 
 return 0;
 }
 
-static void omap_gpio_resume(void)
+static int omap_gpio_resume(struct device *dev)
 {
 struct gpio_bank *bank;
 
@@ -1077,18 +1121,16 @@ static void omap_gpio_resume(void)
 unsigned long flags;
 
 if (!bank->regs->wkup_status)
- return;
+ return 0;
 
+ pm_runtime_get_sync(dev);
 spin_lock_irqsave(&bank->lock, flags);
 _gpio_rmw(base, bank->regs->wkup_status, bank->saved_wakeup, 1);
 spin_unlock_irqrestore(&bank->lock, flags);
 }
-}
 
-static struct syscore_ops omap_gpio_syscore_ops = {
- .suspend = omap_gpio_suspend,
- .resume = omap_gpio_resume,
-};
+ return 0;
+}
 
 #ifdef CONFIG_ARCH_OMAP2PLUS
 
@@ -1112,6 +1154,11 @@ void omap2_gpio_prepare_for_idle(int off_mode)
 if (!off_mode)
 continue;
 
+ if (IS_ERR_VALUE(pm_runtime_put_sync(bank->dev)< 0))
+ dev_err(bank->dev, "%s: GPIO bank %d "
+ "pm_runtime_put_sync failed\n",
+ __func__, bank->id);
+
 /* If going to OFF, remove triggering for all
 * non-wakeup GPIOs. Otherwise spurious IRQs will be
 * generated. See OMAP2420 Errata item 1.101. */
@@ -1152,6 +1199,11 @@ void omap2_gpio_resume_after_idle(void)
 if (!bank->loses_context)
 continue;
 
Quoted 9 lines

drivers/gpio/gpio-omap.c void omap2_gpio_resume_after_idle(void) · line 1208

Quoted 7 lines
drivers/gpio/gpio-omap.c +5 -0
+ if (IS_ERR_VALUE(pm_runtime_get_sync(bank->dev)< 0))
+ dev_err(bank->dev, "%s: GPIO bank %d "
+ "pm_runtime_get_sync failed\n",
+ __func__, bank->id);
+
 for (j = 0; j< hweight_long(bank->dbck_enable_mask); j++)
 clk_enable(bank->dbck);

Optional clock handling also should been addressed using runtime hooks, isn't it ?

Whats handled by runtime hooks is the device;s *main* clocks, which are needed for the device to become accessible.
Anything else *optional* which the device would need optionally based on the various modes it can operate etc are still controlled by the driver by enabling/disabling them as and when needed using the clock framework.

Quoted 7 lines

We still seems to use clock framework here?

Regards
Santosh

#43 lore raw

PATCH v5 18/22 gpio/omap: use pm-runtime framework

Quoted 5 lines · Rajendra Nayak
Quoted 4 lines · Santosh

+ Rajendra and Benoit to comment on optional clock handling.

On Thursday 04 August 2011 04:34 PM, Tarun Kanti DebBarma wrote:

[....]

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

+ if (IS_ERR_VALUE(pm_runtime_get_sync(bank->dev)< 0))
+ dev_err(bank->dev, "%s: GPIO bank %d "
+ "pm_runtime_get_sync failed\n",
+ __func__, bank->id);
+
for (j = 0; j< hweight_long(bank->dbck_enable_mask); j++) clk_enable(bank->dbck);

Quoted 9 lines
Quoted 2 lines

Optional clock handling also should been addressed using runtime hooks, isn't it ?

Whats handled by runtime hooks is the device;s *main* clocks, which are needed for the device to become accessible.
Anything else *optional* which the device would need optionally based on the various modes it can operate etc are still controlled by the driver by enabling/disabling them as and when needed using the clock framework.

Thanks Rajendra for clarification. In summary, we continue to use clock framework for the optional clock handling from the drivers.

Regards
Santosh

#44 lore raw

PATCH v5 19/22 gpio/omap: optimize suspend and resume functions

In reply to Tarun Kanti DebBarma

There is no need to operate on all the banks every time the function is called. Just operate on the current bank passed by the framework.

---
 drivers/gpio/gpio-omap.c |   54 +++++++++++++++++++++------------------------
 1 files changed, 25 insertions(+), 29 deletions(-)
drivers/gpio/gpio-omap.c +25 -29
index e71cfcc..ce93898 100644
@@ -1051,6 +1051,8 @@ static int __devinit omap_gpio_probe(struct platform_device *pdev)
 		goto err_free;
 	}
 
+	platform_set_drvdata(pdev, bank);
+
 	pm_runtime_enable(bank->dev);
 	pm_runtime_irq_safe(bank->dev);
 	if (IS_ERR_VALUE(pm_runtime_get_sync(bank->dev) < 0)) {
@@ -1088,45 +1090,39 @@ err_exit:
 
 static int omap_gpio_suspend(struct device *dev)
 {
-	struct gpio_bank *bank;
-
-	list_for_each_entry(bank, &omap_gpio_list, node) {
-		void __iomem *base = bank->base;
-		void __iomem *wake_status;
-		unsigned long flags;
-
-		if (!bank->regs->wkup_status)
-			return 0;
+	struct platform_device *pdev = to_platform_device(dev);
+	struct gpio_bank *bank = platform_get_drvdata(pdev);
+	void __iomem *base = bank->base;
+	void __iomem *wake_status;
+	unsigned long flags;
 
-		wake_status = bank->base + bank->regs->wkup_status;
+	if (!bank->regs->wkup_status || !bank->suspend_wakeup)
+		return 0;
 
-		spin_lock_irqsave(&bank->lock, flags);
-		bank->saved_wakeup = __raw_readl(wake_status);
-		_gpio_rmw(base, bank->regs->wkup_status,
-					bank->suspend_wakeup, 1);
-		spin_unlock_irqrestore(&bank->lock, flags);
-		pm_runtime_put_sync(dev);
-	}
+	wake_status = bank->base + bank->regs->wkup_status;
 
+	spin_lock_irqsave(&bank->lock, flags);
+	bank->saved_wakeup = __raw_readl(wake_status);
+	_gpio_rmw(base, bank->regs->wkup_status, bank->suspend_wakeup, 1);
+	spin_unlock_irqrestore(&bank->lock, flags);
+	pm_runtime_put_sync(dev);
 	return 0;
 }
 
 static int omap_gpio_resume(struct device *dev)
 {
-	struct gpio_bank *bank;
-
-	list_for_each_entry(bank, &omap_gpio_list, node) {
-		void __iomem *base = bank->base;
-		unsigned long flags;
+	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->regs->wkup_status)
-			return 0;
+	if (!bank->regs->wkup_status || !bank->saved_wakeup)
+		return 0;
 
-		pm_runtime_get_sync(dev);
-		spin_lock_irqsave(&bank->lock, flags);
-		_gpio_rmw(base, bank->regs->wkup_status, bank->saved_wakeup, 1);
-		spin_unlock_irqrestore(&bank->lock, flags);
-	}
+	pm_runtime_get_sync(dev);
+	spin_lock_irqsave(&bank->lock, flags);
+	_gpio_rmw(base, bank->regs->wkup_status, bank->saved_wakeup, 1);
+	spin_unlock_irqrestore(&bank->lock, flags);
 
 	return 0;
 }
Signature · 1 line
1.7.0.4
#46 lore raw

PATCH v5 20/22 gpio/omap: cleanup prepare_for_idle and resume_after_idle

In reply to Tarun Kanti DebBarma

Simplify omap2_gpio_prepare_for_idle() and omap2_gpio_resume_after_idle() by moving most of the stuff to *_runtime_suspend() and *_runtime_resume().

---
 drivers/gpio/gpio-omap.c |  230 ++++++++++++++++++++++++----------------------
 1 files changed, 121 insertions(+), 109 deletions(-)
drivers/gpio/gpio-omap.c +121 -109
index ce93898..ba743f5 100644
@@ -1132,142 +1132,149 @@ static int omap_gpio_resume(struct device *dev)
 static void omap_gpio_save_context(struct gpio_bank *bank);
 static void omap_gpio_restore_context(struct gpio_bank *bank);
 
-void omap2_gpio_prepare_for_idle(int off_mode)
+static int omap_gpio_runtime_suspend(struct device *dev)
 {
-	struct gpio_bank *bank;
+	struct platform_device *pdev = to_platform_device(dev);
+	struct gpio_bank *bank = platform_get_drvdata(pdev);
+	u32 l1 = 0, l2 = 0;
+	int j;
 
-	list_for_each_entry(bank, &omap_gpio_list, node) {
-		u32 l1 = 0, l2 = 0;
-		int j;
+	for (j = 0; j < hweight_long(bank->dbck_enable_mask); j++)
+		clk_disable(bank->dbck);
 
-		if (!bank->loses_context)
-			continue;
+	/* If going to OFF, remove triggering for all
+	 * non-wakeup GPIOs.  Otherwise spurious IRQs will be
+	 * generated.  See OMAP2420 Errata item 1.101. */
+	if (!(bank->enabled_non_wakeup_gpios))
+		goto save_gpio_ctx;
 
-		for (j = 0; j < hweight_long(bank->dbck_enable_mask); j++)
-			clk_disable(bank->dbck);
+	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);
 
-		if (!off_mode)
-			continue;
+	bank->saved_fallingdetect = l1;
+	bank->saved_risingdetect = l2;
+	l1 &= ~bank->enabled_non_wakeup_gpios;
+	l2 &= ~bank->enabled_non_wakeup_gpios;
 
-		if (IS_ERR_VALUE(pm_runtime_put_sync(bank->dev) < 0))
-			dev_err(bank->dev, "%s: GPIO bank %d "
-					"pm_runtime_put_sync failed\n",
-					__func__, bank->id);
+	__raw_writel(l1, bank->base + bank->regs->fallingdetect);
+	__raw_writel(l2, bank->base + bank->regs->risingdetect);
+
+save_gpio_ctx:
+	if (bank->get_context_loss_count)
+		bank->ctx_loss_count = bank->get_context_loss_count(bank->dev);
+	omap_gpio_save_context(bank);
+
+	return 0;
+}
+
+static int omap_gpio_runtime_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct gpio_bank *bank = platform_get_drvdata(pdev);
+	u32 ctx_lost_cnt_after;
+	u32 l = 0, gen, gen0, gen1;
+	int j;
+
+	for (j = 0; j < hweight_long(bank->dbck_enable_mask); j++)
+		clk_enable(bank->dbck);
 
-		/* If going to OFF, remove triggering for all
-		 * non-wakeup GPIOs.  Otherwise spurious IRQs will be
-		 * generated.  See OMAP2420 Errata item 1.101. */
-		if (!(bank->enabled_non_wakeup_gpios))
-			goto save_gpio_ctx;
+	if (bank->get_context_loss_count) {
+		ctx_lost_cnt_after =
+			bank->get_context_loss_count(bank->dev);
+		if (ctx_lost_cnt_after != bank->ctx_loss_count ||
+						!ctx_lost_cnt_after)
+			omap_gpio_restore_context(bank);
+		else
+			return 0;
+	}
 
-		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);
+	if (!(bank->enabled_non_wakeup_gpios))
+		return 0;
 
-		bank->saved_fallingdetect = l1;
-		bank->saved_risingdetect = l2;
-		l1 &= ~bank->enabled_non_wakeup_gpios;
-		l2 &= ~bank->enabled_non_wakeup_gpios;
+	__raw_writel(bank->saved_fallingdetect,
+			bank->base + bank->regs->fallingdetect);
+	__raw_writel(bank->saved_risingdetect,
+			bank->base + bank->regs->risingdetect);
+	l = __raw_readl(bank->base + bank->regs->datain);
 
-		__raw_writel(l1, bank->base + bank->regs->fallingdetect);
-		__raw_writel(l2, bank->base + bank->regs->risingdetect);
+	/* Check if any of the non-wakeup interrupt GPIOs have changed
+	 * state.  If so, generate an IRQ by software.  This is
+	 * horribly racy, but it's the best we can do to work around
+	 * this silicon bug. */
+	l ^= bank->saved_datain;
+	l &= bank->enabled_non_wakeup_gpios;
 
-save_gpio_ctx:
-		if (bank->get_context_loss_count)
-			bank->ctx_loss_count =
-				bank->get_context_loss_count(bank->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 &= bank->saved_datain;
+	gen1 = l & bank->saved_risingdetect;
+	gen1 &= ~(bank->saved_datain);
+
+	/* FIXME: Consider GPIO IRQs with level detections properly! */
+	gen = l & (~(bank->saved_fallingdetect) &
+			~(bank->saved_risingdetect));
+	/* Consider all GPIO IRQs needed to be updated */
+	gen |= gen0 | gen1;
+
+	if (gen) {
+		u32 old0, old1;
+
+		old0 = __raw_readl(bank->base + bank->regs->leveldetect0);
+		old1 = __raw_readl(bank->base + bank->regs->leveldetect1);
+
+		__raw_writel(old0, bank->base + bank->regs->leveldetect0);
+		__raw_writel(old1, bank->base + bank->regs->leveldetect1);
+		if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
+			old0 |= gen;
+			old1 |= gen;
+		}
 
-		omap_gpio_save_context(bank);
+		if (cpu_is_omap44xx()) {
+			old0 |= l;
+			old1 |= l;
+		}
+		__raw_writel(old0, bank->base + bank->regs->leveldetect0);
+		__raw_writel(old1, bank->base + bank->regs->leveldetect1);
 	}
+
+	return 0;
 }
 
-void omap2_gpio_resume_after_idle(void)
+void omap2_gpio_prepare_for_idle(int off_mode)
 {
 	struct gpio_bank *bank;
 
-	list_for_each_entry(bank, &omap_gpio_list, node) {
-		u32 ctx_lost_cnt_after;
-		u32 l = 0, gen, gen0, gen1;
-		int j;
+	if (!off_mode)
+		return;
 
-		if (!bank->loses_context)
+	list_for_each_entry(bank, &omap_gpio_list, node) {
+		if (!bank->mod_usage || !bank->loses_context)
 			continue;
 
-		if (IS_ERR_VALUE(pm_runtime_get_sync(bank->dev) < 0))
+		if (IS_ERR_VALUE(pm_runtime_put_sync(bank->dev) < 0))
 			dev_err(bank->dev, "%s: GPIO bank %d "
-					"pm_runtime_get_sync failed\n",
+					"pm_runtime_put_sync failed\n",
 					__func__, bank->id);
+	}
+}
 
-		for (j = 0; j < hweight_long(bank->dbck_enable_mask); j++)
-			clk_enable(bank->dbck);
-
-		if (bank->get_context_loss_count) {
-			ctx_lost_cnt_after =
-				bank->get_context_loss_count(bank->dev);
-			if (ctx_lost_cnt_after != bank->ctx_loss_count ||
-				!ctx_lost_cnt_after)
-				omap_gpio_restore_context(bank);
-		}
+void omap2_gpio_resume_after_idle(void)
+{
+	struct gpio_bank *bank;
 
-		if (!(bank->enabled_non_wakeup_gpios))
+	list_for_each_entry(bank, &omap_gpio_list, node) {
+		if (!bank->mod_usage || !bank->loses_context)
 			continue;
 
-		__raw_writel(bank->saved_fallingdetect,
-				bank->base + bank->regs->fallingdetect);
-		__raw_writel(bank->saved_risingdetect,
-				bank->base + bank->regs->risingdetect);
-		l = __raw_readl(bank->base + bank->regs->datain);
-
-		/* Check if any of the non-wakeup interrupt GPIOs have changed
-		 * state.  If so, generate an IRQ by software.  This is
-		 * horribly racy, but it's the best we can do to work around
-		 * this silicon bug. */
-		l ^= bank->saved_datain;
-		l &= bank->enabled_non_wakeup_gpios;
-
-		/*
-		 * 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 &= bank->saved_datain;
-
-		gen1 = l & bank->saved_risingdetect;
-		gen1 &= ~(bank->saved_datain);
-
-		/* FIXME: Consider GPIO IRQs with level detections properly! */
-		gen = l & (~(bank->saved_fallingdetect) &
-				~(bank->saved_risingdetect));
-		/* Consider all GPIO IRQs needed to be updated */
-		gen |= gen0 | gen1;
-
-		if (gen) {
-			u32 old0, old1;
-
-			old0 = __raw_readl(bank->base +
-						bank->regs->leveldetect0);
-			old1 = __raw_readl(bank->base +
-						bank->regs->leveldetect1);
-
-			__raw_writel(old0, bank->base +
-						bank->regs->leveldetect0);
-			__raw_writel(old1, bank->base +
-						bank->regs->leveldetect1);
-			if (cpu_is_omap24xx() || cpu_is_omap34xx()) {
-				old0 |= gen;
-				old1 |= gen;
-			}
-
-			if (cpu_is_omap44xx()) {
-				old0 |= l;
-				old1 |= l;
-			}
-			__raw_writel(old0, bank->base +
-						bank->regs->leveldetect0);
-			__raw_writel(old1, bank->base +
-						bank->regs->leveldetect1);
-		}
+		if (IS_ERR_VALUE(pm_runtime_get_sync(bank->dev) < 0))
+			dev_err(bank->dev, "%s: GPIO bank %d "
+					"pm_runtime_get_sync failed\n",
+					__func__, bank->id);
 	}
 }
 
@@ -1311,9 +1318,14 @@ static void omap_gpio_restore_context(struct gpio_bank *bank)
 				bank->base + bank->regs->fallingdetect);
 	__raw_writel(bank->context.dataout, bank->base + bank->regs->dataout);
 }
+#else
+#define omap_gpio_runtime_suspend NULL
+#define omap_gpio_runtime_resume NULL
 #endif
 
 static const struct dev_pm_ops gpio_pm_ops = {
+	.runtime_suspend	= omap_gpio_runtime_suspend,
+	.runtime_resume		= omap_gpio_runtime_resume,
 	.suspend		= omap_gpio_suspend,
 	.resume			= omap_gpio_resume,
 };
Signature · 1 line
1.7.0.4
#47 lore raw

PATCH v5 20/22 gpio/omap: cleanup prepare_for_idle and resume_after_idle

Quoted 1 line · Tarun Kanti DebBarma

Simplify omap2_gpio_prepare_for_idle() and omap2_gpio_resume_after_idle()

/s /Simplify /Cleanup

Quoted 107 lines

by moving most of the stuff to *_runtime_suspend() and *_runtime_resume().

---
  drivers/gpio/gpio-omap.c |  230 ++++++++++++++++++++++++----------------------
  1 files changed, 121 insertions(+), 109 deletions(-)
drivers/gpio/gpio-omap.c +54 -28
index ce93898..ba743f5 100644
@@ -1132,142 +1132,149 @@ static int omap_gpio_resume(struct device *dev)
  static void omap_gpio_save_context(struct gpio_bank *bank);
  static void omap_gpio_restore_context(struct gpio_bank *bank);
 
-void omap2_gpio_prepare_for_idle(int off_mode)
+static int omap_gpio_runtime_suspend(struct device *dev)
  {
-	struct gpio_bank *bank;
+	struct platform_device *pdev = to_platform_device(dev);
+	struct gpio_bank *bank = platform_get_drvdata(pdev);
+	u32 l1 = 0, l2 = 0;
+	int j;
 
-	list_for_each_entry(bank,&omap_gpio_list, node) {
-		u32 l1 = 0, l2 = 0;
-		int j;
+	for (j = 0; j<  hweight_long(bank->dbck_enable_mask); j++)
+		clk_disable(bank->dbck);
 
-		if (!bank->loses_context)
-			continue;
+	/* If going to OFF, remove triggering for all
+	 * non-wakeup GPIOs.  Otherwise spurious IRQs will be
+	 * generated.  See OMAP2420 Errata item 1.101. */
+	if (!(bank->enabled_non_wakeup_gpios))
+		goto save_gpio_ctx;
 
-		for (j = 0; j<  hweight_long(bank->dbck_enable_mask); j++)
-			clk_disable(bank->dbck);
+	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);
 
-		if (!off_mode)
-			continue;
+	bank->saved_fallingdetect = l1;
+	bank->saved_risingdetect = l2;
+	l1&= ~bank->enabled_non_wakeup_gpios;
+	l2&= ~bank->enabled_non_wakeup_gpios;
 
-		if (IS_ERR_VALUE(pm_runtime_put_sync(bank->dev)<  0))
-			dev_err(bank->dev, "%s: GPIO bank %d "
-					"pm_runtime_put_sync failed\n",
-					__func__, bank->id);
+	__raw_writel(l1, bank->base + bank->regs->fallingdetect);
+	__raw_writel(l2, bank->base + bank->regs->risingdetect);
+
+save_gpio_ctx:
+	if (bank->get_context_loss_count)
+		bank->ctx_loss_count = bank->get_context_loss_count(bank->dev);
+	omap_gpio_save_context(bank);
+
+	return 0;
+}
+
+static int omap_gpio_runtime_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct gpio_bank *bank = platform_get_drvdata(pdev);
+	u32 ctx_lost_cnt_after;
+	u32 l = 0, gen, gen0, gen1;
+	int j;
+
+	for (j = 0; j<  hweight_long(bank->dbck_enable_mask); j++)
+		clk_enable(bank->dbck);
 
-		/* If going to OFF, remove triggering for all
-		 * non-wakeup GPIOs.  Otherwise spurious IRQs will be
-		 * generated.  See OMAP2420 Errata item 1.101. */
-		if (!(bank->enabled_non_wakeup_gpios))
-			goto save_gpio_ctx;
+	if (bank->get_context_loss_count) {
+		ctx_lost_cnt_after =
+			bank->get_context_loss_count(bank->dev);
+		if (ctx_lost_cnt_after != bank->ctx_loss_count ||
+						!ctx_lost_cnt_after)
+			omap_gpio_restore_context(bank);
+		else
+			return 0;
+	}
 
-		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);
+	if (!(bank->enabled_non_wakeup_gpios))
+		return 0;
 
-		bank->saved_fallingdetect = l1;
-		bank->saved_risingdetect = l2;
-		l1&= ~bank->enabled_non_wakeup_gpios;
-		l2&= ~bank->enabled_non_wakeup_gpios;
+	__raw_writel(bank->saved_fallingdetect,
+			bank->base + bank->regs->fallingdetect);

drivers/gpio/gpio-omap.c static int omap_gpio_resume(struct device *dev) · line 1205

Quoted 10 lines
drivers/gpio/gpio-omap.c +7 -2
+	__raw_writel(bank->saved_risingdetect,
+			bank->base + bank->regs->risingdetect);
+	l = __raw_readl(bank->base + bank->regs->datain);
 
-		__raw_writel(l1, bank->base + bank->regs->fallingdetect);
-		__raw_writel(l2, bank->base + bank->regs->risingdetect);
+	/* Check if any of the non-wakeup interrupt GPIOs have changed
+	 * state.  If so, generate an IRQ by software.  This is
+	 * horribly racy, but it's the best we can do to work around
+	 * this silicon bug. */

Multi-line comment.

drivers/gpio/gpio-omap.c static int omap_gpio_resume(struct device *dev) · line 1207

Quoted 2 lines
drivers/gpio/gpio-omap.c +2 -0
+	l ^= bank->saved_datain;
+	l&= bank->enabled_non_wakeup_gpios;

Space please around operator.

drivers/gpio/gpio-omap.c static int omap_gpio_resume(struct device *dev)

Quoted 13 lines
drivers/gpio/gpio-omap.c +8 -4
-save_gpio_ctx:
-		if (bank->get_context_loss_count)
-			bank->ctx_loss_count =
-				bank->get_context_loss_count(bank->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&= bank->saved_datain;
+	gen1 = l&  bank->saved_risingdetect;
+	gen1&= ~(bank->saved_datain);

Ditto

Rest looks fine.

regards
Santosh

#48 lore raw

PATCH v5 21/22 gpio/omap: skip operations in runtime callbacks

In reply to Tarun Kanti DebBarma

Most operations within runtime callbacks should be skipped when *_runtime_get_sync() and *_runtime_put_sync() are called in probe(), *_gpio_request() and *_gpio_free(). We just need clock enable/disable.

---
 drivers/gpio/gpio-omap.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
drivers/gpio/gpio-omap.c +6 -0
index ba743f5..eae955a 100644
@@ -1142,6 +1142,9 @@ static int omap_gpio_runtime_suspend(struct device *dev)
 	for (j = 0; j < hweight_long(bank->dbck_enable_mask); j++)
 		clk_disable(bank->dbck);
 
+	if (!bank->mod_usage)
+		return 0;
+
 	/* If going to OFF, remove triggering for all
 	 * non-wakeup GPIOs.  Otherwise spurious IRQs will be
 	 * generated.  See OMAP2420 Errata item 1.101. */
@@ -1180,6 +1183,9 @@ static int omap_gpio_runtime_resume(struct device *dev)
 	for (j = 0; j < hweight_long(bank->dbck_enable_mask); j++)
 		clk_enable(bank->dbck);
 
+	if (!bank->mod_usage)
+		return 0;
+
 	if (bank->get_context_loss_count) {
 		ctx_lost_cnt_after =
 			bank->get_context_loss_count(bank->dev);
Signature · 1 line
1.7.0.4
#50 lore raw

PATCH v5 22/22 gpio/omap: remove omap_gpio_save_context overhead

In reply to Tarun Kanti DebBarma

Context is now saved dynamically in respective functions whenever and whichever registers are modified. This avoid overhead of saving all registers context in the runtime callback.

---
 drivers/gpio/gpio-omap.c |   66 ++++++++++++++++++++++++++++------------------
 1 files changed, 40 insertions(+), 26 deletions(-)
drivers/gpio/gpio-omap.c +40 -26
index eae955a..ee1726d 100644
@@ -97,6 +97,7 @@ static void _set_gpio_direction(struct gpio_bank *bank, int gpio, int is_input)
 	else
 		l &= ~(1 << gpio);
 	__raw_writel(l, reg);
+	bank->context.oe = l;
 }
 
 
@@ -127,6 +128,7 @@ static void _set_gpio_dataout_mask(struct gpio_bank *bank, int gpio, int enable)
 	else
 		l &= ~gpio_bit;
 	__raw_writel(l, reg);
+	bank->context.dataout = l;
 }
 
 static int _get_gpio_datain(struct gpio_bank *bank, int gpio)
@@ -216,9 +218,21 @@ static inline void set_gpio_trigger(struct gpio_bank *bank, int gpio,
 	_gpio_rmw(base, bank->regs->fallingdetect, gpio_bit,
 		  trigger & IRQ_TYPE_EDGE_FALLING);
 
-	if (likely(!(bank->non_wakeup_gpios & gpio_bit)))
+	bank->context.leveldetect0 =
+			__raw_readl(bank->base + bank->regs->leveldetect0);
+	bank->context.leveldetect1 =
+			__raw_readl(bank->base + bank->regs->leveldetect1);
+	bank->context.risingdetect =
+			__raw_readl(bank->base + bank->regs->risingdetect);
+	bank->context.fallingdetect =
+			__raw_readl(bank->base + bank->regs->fallingdetect);
+
+	if (likely(!(bank->non_wakeup_gpios & gpio_bit))) {
 		_gpio_rmw(base, bank->regs->wkup_status, gpio_bit,
 			trigger != 0);
+		bank->context.wake_en =
+			__raw_readl(bank->base + bank->regs->wkup_status);
+	}
 
 	/* This part needs to be executed always for OMAP34xx */
 	if (cpu_is_omap34xx() || (bank->non_wakeup_gpios & gpio_bit)) {
@@ -304,6 +318,8 @@ static int _set_gpio_triggering(struct gpio_bank *bank, int gpio, int trigger)
 			l |= 1 << (gpio << 1);
 
 		_gpio_rmw(base, bank->regs->wkup_status, 1 << gpio, trigger);
+		bank->context.wake_en =
+			__raw_readl(bank->base + bank->regs->wkup_status);
 
 		__raw_writel(l, reg);
 	}
@@ -398,6 +414,7 @@ static void _enable_gpio_irqbank(struct gpio_bank *bank, int gpio_mask)
 	}
 
 	__raw_writel(l, reg);
+	bank->context.irqenable1 = l;
 }
 
 static void _disable_gpio_irqbank(struct gpio_bank *bank, int gpio_mask)
@@ -418,6 +435,7 @@ static void _disable_gpio_irqbank(struct gpio_bank *bank, int gpio_mask)
 	}
 
 	__raw_writel(l, reg);
+	bank->context.irqenable1 = l;
 }
 
 static inline void _set_gpio_irqenable(struct gpio_bank *bank, int gpio, int enable)
@@ -515,6 +533,7 @@ static int omap_gpio_request(struct gpio_chip *chip, unsigned offset)
 		/* Module is enabled, clocks are not gated */
 		ctrl &= ~GPIO_MOD_CTRL_BIT;
 		__raw_writel(ctrl, reg);
+		bank->context.ctrl = ctrl;
 	}
 
 	bank->mod_usage |= 1 << offset;
@@ -532,9 +551,12 @@ static void omap_gpio_free(struct gpio_chip *chip, unsigned offset)
 
 	spin_lock_irqsave(&bank->lock, flags);
 
-	if (bank->regs->wkup_status)
+	if (bank->regs->wkup_status) {
 		/* Disable wake-up during idle for dynamic tick */
 		_gpio_rmw(base, bank->regs->wkup_status, 1 << offset, 0);
+		bank->context.wake_en =
+			__raw_readl(bank->base + bank->regs->wkup_status);
+	}
 
 	bank->mod_usage &= ~(1 << offset);
 
@@ -546,6 +568,7 @@ static void omap_gpio_free(struct gpio_chip *chip, unsigned offset)
 		/* Module is disabled, clocks are gated */
 		ctrl |= GPIO_MOD_CTRL_BIT;
 		__raw_writel(ctrl, reg);
+		bank->context.ctrl = ctrl;
 	}
 
 	_reset_gpio(bank, bank->chip.base + offset);
@@ -912,6 +935,9 @@ static void omap_gpio_mod_init(struct gpio_bank *bank)
 					bank->regs->irqenable_inv == false);
 	_gpio_rmw(base, bank->regs->irqenable, l, bank->regs->debounce_en != 0);
 	_gpio_rmw(base, bank->regs->irqenable, l, bank->regs->ctrl != 0);
+
+	bank->context.irqenable1 =
+			__raw_readl(bank->base + bank->regs->irqenable);
 }
 
 static __init void
@@ -1104,6 +1130,8 @@ static int omap_gpio_suspend(struct device *dev)
 	spin_lock_irqsave(&bank->lock, flags);
 	bank->saved_wakeup = __raw_readl(wake_status);
 	_gpio_rmw(base, bank->regs->wkup_status, bank->suspend_wakeup, 1);
+	bank->context.wake_en =
+		__raw_readl(bank->base + bank->regs->wkup_status);
 	spin_unlock_irqrestore(&bank->lock, flags);
 	pm_runtime_put_sync(dev);
 	return 0;
@@ -1122,6 +1150,8 @@ static int omap_gpio_resume(struct device *dev)
 	pm_runtime_get_sync(dev);
 	spin_lock_irqsave(&bank->lock, flags);
 	_gpio_rmw(base, bank->regs->wkup_status, bank->saved_wakeup, 1);
+	bank->context.wake_en =
+		__raw_readl(bank->base + bank->regs->wkup_status);
 	spin_unlock_irqrestore(&bank->lock, flags);
 
 	return 0;
@@ -1129,7 +1159,6 @@ static int omap_gpio_resume(struct device *dev)
 
 #ifdef CONFIG_ARCH_OMAP2PLUS
 
-static void omap_gpio_save_context(struct gpio_bank *bank);
 static void omap_gpio_restore_context(struct gpio_bank *bank);
 
 static int omap_gpio_runtime_suspend(struct device *dev)
@@ -1149,7 +1178,7 @@ 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 save_gpio_ctx;
+		goto update_gpio_ctx_cnt;
 
 	bank->saved_datain = __raw_readl(bank->base +
 						bank->regs->datain);
@@ -1163,11 +1192,12 @@ static int omap_gpio_runtime_suspend(struct device *dev)
 
 	__raw_writel(l1, bank->base + bank->regs->fallingdetect);
 	__raw_writel(l2, bank->base + bank->regs->risingdetect);
+	bank->context.fallingdetect = l1;
+	bank->context.risingdetect = l2;
 
-save_gpio_ctx:
+update_gpio_ctx_cnt:
 	if (bank->get_context_loss_count)
 		bank->ctx_loss_count = bank->get_context_loss_count(bank->dev);
-	omap_gpio_save_context(bank);
 
 	return 0;
 }
@@ -1203,6 +1233,8 @@ static int omap_gpio_runtime_resume(struct device *dev)
 			bank->base + bank->regs->fallingdetect);
 	__raw_writel(bank->saved_risingdetect,
 			bank->base + bank->regs->risingdetect);
+	bank->context.fallingdetect = bank->saved_fallingdetect;
+	bank->context.risingdetect = bank->saved_risingdetect;
 	l = __raw_readl(bank->base + bank->regs->datain);
 
 	/* Check if any of the non-wakeup interrupt GPIOs have changed
@@ -1246,6 +1278,8 @@ static int omap_gpio_runtime_resume(struct device *dev)
 		}
 		__raw_writel(old0, bank->base + bank->regs->leveldetect0);
 		__raw_writel(old1, bank->base + bank->regs->leveldetect1);
+		bank->context.leveldetect0 = old0;
+		bank->context.leveldetect1 = old1;
 	}
 
 	return 0;
@@ -1284,26 +1318,6 @@ void omap2_gpio_resume_after_idle(void)
 	}
 }
 
-static void omap_gpio_save_context(struct gpio_bank *bank)
-{
-	bank->context.irqenable1 =
-			__raw_readl(bank->base + bank->regs->irqenable);
-	bank->context.irqenable2 =
-			__raw_readl(bank->base + bank->regs->irqenable2);
-	bank->context.wake_en =
-			__raw_readl(bank->base + bank->regs->wkup_status);
-	bank->context.ctrl = __raw_readl(bank->base + bank->regs->ctrl);
-	bank->context.oe = __raw_readl(bank->base + bank->regs->direction);
-	bank->context.leveldetect0 =
-			__raw_readl(bank->base + bank->regs->leveldetect0);
-	bank->context.leveldetect1 =
-			__raw_readl(bank->base + bank->regs->leveldetect1);
-	bank->context.risingdetect =
-			__raw_readl(bank->base + bank->regs->risingdetect);
-	bank->context.fallingdetect =
-	bank->context.dataout = __raw_readl(bank->base + bank->regs->dataout);
-}
-
 static void omap_gpio_restore_context(struct gpio_bank *bank)
 {
 	__raw_writel(bank->context.irqenable1,
Signature · 1 line
1.7.0.4