linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/9] gpio/omap: Some more driver cleanup and fixes
@ 2012-02-29 22:44 Tarun Kanti DebBarma
  2012-02-29 22:44 ` [PATCH v2 1/9] gpio/omap: remove saved_fallingdetect, saved_risingdetect fields Tarun Kanti DebBarma
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Tarun Kanti DebBarma @ 2012-02-29 22:44 UTC (permalink / raw)
  To: linux-arm-kernel

The cleanup is mostly getting rid of redundant fields in struct gpio_bank{}
as we already have them as part of bank->context now. Also, remove un-used
variable from gpio_irq_handler.

The fix include correction of _set_gpio_irqenable() implementation and fix
type mismatch of gpio trigger parameter.

It is baselined on top of Kevin's following series:
gpio/omap: cleanup and runtime PM conversion for v3.4
git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git for_3.4/gpio/runtime-pm-cleanup

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

Power Test: Off-mode and Retention on OMAP3430 (Suspend and Idle paths).
Functional Test: OMAP2430, OMAP3430SDP, ZOOM3, OMAP4430, OMAP4-BLAZE

v2:
- Added a new patch to update wakeup_en register in _set_gpio_wakeup()
  in addition to updating bank->context.wake_en.

- Added a new patch to remove redundant decoding of gpio offset in
  gpio_get(), _get_gpio_datain() and _get_gpio_dataout().

- Added a new patch to remove suspend/resume callbacks because the
  operations performed with the callbacks are redundant.

Tarun Kanti DebBarma (9):
  gpio/omap: remove saved_fallingdetect, saved_risingdetect fields
  gpio/omap: fix wakeup_en register update in _set_gpio_wakeup()
  gpio/omap: remove suspend_wakeup field from struct gpio_bank
  gpio/omap: remove saved_wakeup field from struct gpio_bank
  gpio/omap: get rid of retrigger variable in gpio_irq_handler
  gpio/omap: fix trigger type to unsigned
  gpio/omap: fix _set_gpio_irqenable implementation
  gpio/omap: remove redundant decoding of gpio offset
  gpio/omap: remove suspend/resume callbacks

 drivers/gpio/gpio-omap.c |  113 +++++++++++-----------------------------------
 1 files changed, 27 insertions(+), 86 deletions(-)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/9] gpio/omap: remove saved_fallingdetect, saved_risingdetect fields
  2012-02-29 22:44 [PATCH v2 0/9] gpio/omap: Some more driver cleanup and fixes Tarun Kanti DebBarma
@ 2012-02-29 22:44 ` Tarun Kanti DebBarma
  2012-02-29 22:44 ` [PATCH v2 2/9] gpio/omap: fix wakeup_en register update in _set_gpio_wakeup() Tarun Kanti DebBarma
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Tarun Kanti DebBarma @ 2012-02-29 22:44 UTC (permalink / raw)
  To: linux-arm-kernel

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

Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
 drivers/gpio/gpio-omap.c |   19 ++++++++-----------
 1 files changed, 8 insertions(+), 11 deletions(-)

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/9] gpio/omap: fix wakeup_en register update in _set_gpio_wakeup()
  2012-02-29 22:44 [PATCH v2 0/9] gpio/omap: Some more driver cleanup and fixes Tarun Kanti DebBarma
  2012-02-29 22:44 ` [PATCH v2 1/9] gpio/omap: remove saved_fallingdetect, saved_risingdetect fields Tarun Kanti DebBarma
@ 2012-02-29 22:44 ` Tarun Kanti DebBarma
  2012-02-29 22:44 ` [PATCH v2 3/9] gpio/omap: remove suspend_wakeup field from struct gpio_bank Tarun Kanti DebBarma
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Tarun Kanti DebBarma @ 2012-02-29 22:44 UTC (permalink / raw)
  To: linux-arm-kernel

There are two ways through which wakeup_en register can be programmed
using gpiolib APIs as shown below. It is seen that in the second case
in _set_gpio_wakeup(), even though bank->suspend_wakeup is updated
correctly, its value is not programmed in wakeup_en register. Fix this.

chip.irq_set_type()->gpio_irq_type()->_set_gpio_triggering()->set_gpio_trigger()
chip.irq_set_wake()->gpio_wake_enable()->_set_gpio_wakeup()

Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
---
 drivers/gpio/gpio-omap.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index c9369d2..895df7f 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -504,6 +504,7 @@ static int _set_gpio_wakeup(struct gpio_bank *bank, int gpio, int enable)
 	else
 		bank->suspend_wakeup &= ~gpio_bit;
 
+	__raw_writel(bank->suspend_wakeup, bank->base + bank->regs->wkup_en);
 	spin_unlock_irqrestore(&bank->lock, flags);
 
 	return 0;
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/9] gpio/omap: remove suspend_wakeup field from struct gpio_bank
  2012-02-29 22:44 [PATCH v2 0/9] gpio/omap: Some more driver cleanup and fixes Tarun Kanti DebBarma
  2012-02-29 22:44 ` [PATCH v2 1/9] gpio/omap: remove saved_fallingdetect, saved_risingdetect fields Tarun Kanti DebBarma
  2012-02-29 22:44 ` [PATCH v2 2/9] gpio/omap: fix wakeup_en register update in _set_gpio_wakeup() Tarun Kanti DebBarma
@ 2012-02-29 22:44 ` Tarun Kanti DebBarma
  2012-02-29 22:44 ` [PATCH v2 4/9] gpio/omap: remove saved_wakeup " Tarun Kanti DebBarma
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Tarun Kanti DebBarma @ 2012-02-29 22:44 UTC (permalink / raw)
  To: linux-arm-kernel

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

Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
 drivers/gpio/gpio-omap.c |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 895df7f..14c61e2 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -54,7 +54,6 @@ struct gpio_bank {
 	void __iomem *base;
 	u16 irq;
 	u16 virtual_irq_start;
-	u32 suspend_wakeup;
 	u32 saved_wakeup;
 	u32 non_wakeup_gpios;
 	u32 enabled_non_wakeup_gpios;
@@ -500,11 +499,11 @@ static int _set_gpio_wakeup(struct gpio_bank *bank, int gpio, int enable)
 
 	spin_lock_irqsave(&bank->lock, flags);
 	if (enable)
-		bank->suspend_wakeup |= gpio_bit;
+		bank->context.wake_en |= gpio_bit;
 	else
-		bank->suspend_wakeup &= ~gpio_bit;
+		bank->context.wake_en &= ~gpio_bit;
 
-	__raw_writel(bank->suspend_wakeup, bank->base + bank->regs->wkup_en);
+	__raw_writel(bank->context.wake_en, bank->base + bank->regs->wkup_en);
 	spin_unlock_irqrestore(&bank->lock, flags);
 
 	return 0;
@@ -776,7 +775,7 @@ static int omap_mpuio_suspend_noirq(struct device *dev)
 
 	spin_lock_irqsave(&bank->lock, flags);
 	bank->saved_wakeup = __raw_readl(mask_reg);
-	__raw_writel(0xffff & ~bank->suspend_wakeup, mask_reg);
+	__raw_writel(0xffff & ~bank->context.wake_en, mask_reg);
 	spin_unlock_irqrestore(&bank->lock, flags);
 
 	return 0;
@@ -1150,7 +1149,7 @@ static int omap_gpio_suspend(struct device *dev)
 	if (!bank->mod_usage || !bank->loses_context)
 		return 0;
 
-	if (!bank->regs->wkup_en || !bank->suspend_wakeup)
+	if (!bank->regs->wkup_en || !bank->context.wake_en)
 		return 0;
 
 	wakeup_enable = bank->base + bank->regs->wkup_en;
@@ -1158,7 +1157,7 @@ static int omap_gpio_suspend(struct device *dev)
 	spin_lock_irqsave(&bank->lock, flags);
 	bank->saved_wakeup = __raw_readl(wakeup_enable);
 	_gpio_rmw(base, bank->regs->wkup_en, 0xffffffff, 0);
-	_gpio_rmw(base, bank->regs->wkup_en, bank->suspend_wakeup, 1);
+	_gpio_rmw(base, bank->regs->wkup_en, bank->context.wake_en, 1);
 	spin_unlock_irqrestore(&bank->lock, flags);
 
 	return 0;
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 4/9] gpio/omap: remove saved_wakeup field from struct gpio_bank
  2012-02-29 22:44 [PATCH v2 0/9] gpio/omap: Some more driver cleanup and fixes Tarun Kanti DebBarma
                   ` (2 preceding siblings ...)
  2012-02-29 22:44 ` [PATCH v2 3/9] gpio/omap: remove suspend_wakeup field from struct gpio_bank Tarun Kanti DebBarma
@ 2012-02-29 22:44 ` Tarun Kanti DebBarma
  2012-02-29 22:44 ` [PATCH v2 5/9] gpio/omap: get rid of retrigger variable in gpio_irq_handler Tarun Kanti DebBarma
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Tarun Kanti DebBarma @ 2012-02-29 22:44 UTC (permalink / raw)
  To: linux-arm-kernel

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

Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
 drivers/gpio/gpio-omap.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 14c61e2..3765654 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -54,7 +54,6 @@ struct gpio_bank {
 	void __iomem *base;
 	u16 irq;
 	u16 virtual_irq_start;
-	u32 saved_wakeup;
 	u32 non_wakeup_gpios;
 	u32 enabled_non_wakeup_gpios;
 	struct gpio_regs context;
@@ -774,7 +773,6 @@ static int omap_mpuio_suspend_noirq(struct device *dev)
 	unsigned long		flags;
 
 	spin_lock_irqsave(&bank->lock, flags);
-	bank->saved_wakeup = __raw_readl(mask_reg);
 	__raw_writel(0xffff & ~bank->context.wake_en, mask_reg);
 	spin_unlock_irqrestore(&bank->lock, flags);
 
@@ -790,7 +788,7 @@ static int omap_mpuio_resume_noirq(struct device *dev)
 	unsigned long		flags;
 
 	spin_lock_irqsave(&bank->lock, flags);
-	__raw_writel(bank->saved_wakeup, mask_reg);
+	__raw_writel(bank->context.wake_en, mask_reg);
 	spin_unlock_irqrestore(&bank->lock, flags);
 
 	return 0;
@@ -1143,7 +1141,6 @@ static int omap_gpio_suspend(struct device *dev)
 	struct platform_device *pdev = to_platform_device(dev);
 	struct gpio_bank *bank = platform_get_drvdata(pdev);
 	void __iomem *base = bank->base;
-	void __iomem *wakeup_enable;
 	unsigned long flags;
 
 	if (!bank->mod_usage || !bank->loses_context)
@@ -1152,10 +1149,7 @@ static int omap_gpio_suspend(struct device *dev)
 	if (!bank->regs->wkup_en || !bank->context.wake_en)
 		return 0;
 
-	wakeup_enable = bank->base + bank->regs->wkup_en;
-
 	spin_lock_irqsave(&bank->lock, flags);
-	bank->saved_wakeup = __raw_readl(wakeup_enable);
 	_gpio_rmw(base, bank->regs->wkup_en, 0xffffffff, 0);
 	_gpio_rmw(base, bank->regs->wkup_en, bank->context.wake_en, 1);
 	spin_unlock_irqrestore(&bank->lock, flags);
@@ -1173,12 +1167,12 @@ static int omap_gpio_resume(struct device *dev)
 	if (!bank->mod_usage || !bank->loses_context)
 		return 0;
 
-	if (!bank->regs->wkup_en || !bank->saved_wakeup)
+	if (!bank->regs->wkup_en || !bank->context.wake_en)
 		return 0;
 
 	spin_lock_irqsave(&bank->lock, flags);
 	_gpio_rmw(base, bank->regs->wkup_en, 0xffffffff, 0);
-	_gpio_rmw(base, bank->regs->wkup_en, bank->saved_wakeup, 1);
+	_gpio_rmw(base, bank->regs->wkup_en, bank->context.wake_en, 1);
 	spin_unlock_irqrestore(&bank->lock, flags);
 
 	return 0;
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 5/9] gpio/omap: get rid of retrigger variable in gpio_irq_handler
  2012-02-29 22:44 [PATCH v2 0/9] gpio/omap: Some more driver cleanup and fixes Tarun Kanti DebBarma
                   ` (3 preceding siblings ...)
  2012-02-29 22:44 ` [PATCH v2 4/9] gpio/omap: remove saved_wakeup " Tarun Kanti DebBarma
@ 2012-02-29 22:44 ` Tarun Kanti DebBarma
  2012-02-29 22:44 ` [PATCH v2 6/9] gpio/omap: fix trigger type to unsigned Tarun Kanti DebBarma
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Tarun Kanti DebBarma @ 2012-02-29 22:44 UTC (permalink / raw)
  To: linux-arm-kernel

This local variable is just assigned zero and then OR'ed
with isr. It does not appear to serve any purpose and so
removing it.

Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
 drivers/gpio/gpio-omap.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 6/9] gpio/omap: fix trigger type to unsigned
  2012-02-29 22:44 [PATCH v2 0/9] gpio/omap: Some more driver cleanup and fixes Tarun Kanti DebBarma
                   ` (4 preceding siblings ...)
  2012-02-29 22:44 ` [PATCH v2 5/9] gpio/omap: get rid of retrigger variable in gpio_irq_handler Tarun Kanti DebBarma
@ 2012-02-29 22:44 ` Tarun Kanti DebBarma
  2012-02-29 22:44 ` [PATCH v2 7/9] gpio/omap: fix _set_gpio_irqenable implementation Tarun Kanti DebBarma
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Tarun Kanti DebBarma @ 2012-02-29 22:44 UTC (permalink / raw)
  To: linux-arm-kernel

The GPIO trigger parameter is of type unsigned.
enum {
        IRQ_TYPE_NONE           = 0x00000000,
        IRQ_TYPE_EDGE_RISING    = 0x00000001,
        IRQ_TYPE_EDGE_FALLING   = 0x00000002,
        IRQ_TYPE_EDGE_BOTH      = (IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING),
        IRQ_TYPE_LEVEL_HIGH     = 0x00000004,
        IRQ_TYPE_LEVEL_LOW      = 0x00000008,
        IRQ_TYPE_LEVEL_MASK     = (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_LEVEL_HIGH),
        IRQ_TYPE_SENSE_MASK     = 0x0000000f,

        IRQ_TYPE_PROBE          = 0x00000010,
...
};
Even though gpio_irq_type(struct irq_data *d, unsigned type) has the right type
of parameter, the subsequent called functions set_gpio_triggering() and
set_gpio_trigger() wrongly makes it signed integer. Fix this.

Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
 drivers/gpio/gpio-omap.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index de5fe8f..bb994db 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -234,7 +234,7 @@ static void _set_gpio_debounce(struct gpio_bank *bank, unsigned gpio,
 }
 
 static inline void set_gpio_trigger(struct gpio_bank *bank, int gpio,
-						int trigger)
+						unsigned trigger)
 {
 	void __iomem *base = bank->base;
 	u32 gpio_bit = 1 << gpio;
@@ -316,7 +316,8 @@ static void _toggle_gpio_edge_triggering(struct gpio_bank *bank, int gpio)
 static void _toggle_gpio_edge_triggering(struct gpio_bank *bank, int gpio) {}
 #endif
 
-static int _set_gpio_triggering(struct gpio_bank *bank, int gpio, int trigger)
+static int _set_gpio_triggering(struct gpio_bank *bank, int gpio,
+							unsigned trigger)
 {
 	void __iomem *reg = bank->base;
 	void __iomem *base = bank->base;
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 7/9] gpio/omap: fix _set_gpio_irqenable implementation
  2012-02-29 22:44 [PATCH v2 0/9] gpio/omap: Some more driver cleanup and fixes Tarun Kanti DebBarma
                   ` (5 preceding siblings ...)
  2012-02-29 22:44 ` [PATCH v2 6/9] gpio/omap: fix trigger type to unsigned Tarun Kanti DebBarma
@ 2012-02-29 22:44 ` Tarun Kanti DebBarma
  2012-02-29 22:44 ` [PATCH v2 8/9] gpio/omap: remove redundant decoding of gpio offset Tarun Kanti DebBarma
  2012-02-29 22:44 ` [PATCH v2 9/9] gpio/omap: remove suspend/resume callbacks Tarun Kanti DebBarma
  8 siblings, 0 replies; 10+ messages in thread
From: Tarun Kanti DebBarma @ 2012-02-29 22:44 UTC (permalink / raw)
  To: linux-arm-kernel

This function should be capable of both enabling and disabling interrupts
based upon the *enable* parameter. Right now the function only enables
the interrupt and *enable* is not used at all. So add the interrupt
disable capability also using the parameter.

Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
 drivers/gpio/gpio-omap.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index bb994db..19f8f44 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -475,7 +475,10 @@ static void _disable_gpio_irqbank(struct gpio_bank *bank, int gpio_mask)
 
 static inline void _set_gpio_irqenable(struct gpio_bank *bank, int gpio, int enable)
 {
-	_enable_gpio_irqbank(bank, GPIO_BIT(bank, gpio));
+	if (enable)
+		_enable_gpio_irqbank(bank, GPIO_BIT(bank, gpio));
+	else
+		_disable_gpio_irqbank(bank, GPIO_BIT(bank, gpio));
 }
 
 /*
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 8/9] gpio/omap: remove redundant decoding of gpio offset
  2012-02-29 22:44 [PATCH v2 0/9] gpio/omap: Some more driver cleanup and fixes Tarun Kanti DebBarma
                   ` (6 preceding siblings ...)
  2012-02-29 22:44 ` [PATCH v2 7/9] gpio/omap: fix _set_gpio_irqenable implementation Tarun Kanti DebBarma
@ 2012-02-29 22:44 ` Tarun Kanti DebBarma
  2012-02-29 22:44 ` [PATCH v2 9/9] gpio/omap: remove suspend/resume callbacks Tarun Kanti DebBarma
  8 siblings, 0 replies; 10+ messages in thread
From: Tarun Kanti DebBarma @ 2012-02-29 22:44 UTC (permalink / raw)
  To: linux-arm-kernel

In gpio_get(), _get_gpio_datain() and _get_gpio_dataout() get rid of
un-necessary operation to compute gpio mask. The gpio offset passed
to gpio_get() is sufficient to do that.

Here is Russell's original comment:
Can someone explain to me this:
#define GPIO_INDEX(bank, gpio) (gpio % bank->width)
#define GPIO_BIT(bank, gpio) (1 << GPIO_INDEX(bank, gpio))

static int _get_gpio_datain(struct gpio_bank *bank, int gpio)
{
       void __iomem *reg = bank->base + bank->regs->datain;

       return (__raw_readl(reg) & GPIO_BIT(bank, gpio)) != 0;
}

static int gpio_get(struct gpio_chip *chip, unsigned offset)
{
       struct gpio_bank *bank = container_of(chip, struct gpio_bank, chip);
       void __iomem *reg = bank->base;
       int gpio = chip->base + offset;
       u32 mask = GPIO_BIT(bank, gpio);

       if (gpio_is_input(bank, mask))
               return _get_gpio_datain(bank, gpio);
       else
               return _get_gpio_dataout(bank, gpio);
}

Given that bank->width on OMAP is either 32 or 16, and GPIO numbers for
any GPIO chip are always aligned to 32 or 16, why does this code bother
adding the chips base gpio number and then modulo the width?

Surely this means if - for argument sake - you registered a GPIO chip
with 8 lines followed by one with 16 lines, GPIO0..7 would be chip 0
bit 0..7, GPIO8..15 would be chip 1 bit 8..15, GPIO16..23 would be
chip 1 bit 0..7.

However, if you registered a GPIO chip with 16 lines first, it would
mean GPIO0..15 would be chip 0 bit 0..15, and GPIO16..31 would be
chip 1 bit 0..15.

Surely this kind of behaviour is not intended?

Is there a reason why the bitmask can't just be (1 << offset) where
offset is passed into these functions as GPIO number - chip->base ?

Reported-by: Russell King - ARM Linux <linux@arm.linux.org.uk>
Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
---
 drivers/gpio/gpio-omap.c |   18 +++++++-----------
 1 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 19f8f44..186ce92 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -133,18 +133,18 @@ static void _set_gpio_dataout_mask(struct gpio_bank *bank, int gpio, int enable)
 	bank->context.dataout = l;
 }
 
-static int _get_gpio_datain(struct gpio_bank *bank, int gpio)
+static int _get_gpio_datain(struct gpio_bank *bank, int offset)
 {
 	void __iomem *reg = bank->base + bank->regs->datain;
 
-	return (__raw_readl(reg) & GPIO_BIT(bank, gpio)) != 0;
+	return (__raw_readl(reg) & (1 << offset)) != 0;
 }
 
-static int _get_gpio_dataout(struct gpio_bank *bank, int gpio)
+static int _get_gpio_dataout(struct gpio_bank *bank, int offset)
 {
 	void __iomem *reg = bank->base + bank->regs->dataout;
 
-	return (__raw_readl(reg) & GPIO_BIT(bank, gpio)) != 0;
+	return (__raw_readl(reg) & (1 << offset)) != 0;
 }
 
 static inline void _gpio_rmw(void __iomem *base, u32 reg, u32 mask, bool set)
@@ -849,19 +849,15 @@ static int gpio_is_input(struct gpio_bank *bank, int mask)
 static int gpio_get(struct gpio_chip *chip, unsigned offset)
 {
 	struct gpio_bank *bank;
-	void __iomem *reg;
-	int gpio;
 	u32 mask;
 
-	gpio = chip->base + offset;
 	bank = container_of(chip, struct gpio_bank, chip);
-	reg = bank->base;
-	mask = GPIO_BIT(bank, gpio);
+	mask = (1 << offset);
 
 	if (gpio_is_input(bank, mask))
-		return _get_gpio_datain(bank, gpio);
+		return _get_gpio_datain(bank, offset);
 	else
-		return _get_gpio_dataout(bank, gpio);
+		return _get_gpio_dataout(bank, offset);
 }
 
 static int gpio_output(struct gpio_chip *chip, unsigned offset, int value)
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 9/9] gpio/omap: remove suspend/resume callbacks
  2012-02-29 22:44 [PATCH v2 0/9] gpio/omap: Some more driver cleanup and fixes Tarun Kanti DebBarma
                   ` (7 preceding siblings ...)
  2012-02-29 22:44 ` [PATCH v2 8/9] gpio/omap: remove redundant decoding of gpio offset Tarun Kanti DebBarma
@ 2012-02-29 22:44 ` Tarun Kanti DebBarma
  8 siblings, 0 replies; 10+ messages in thread
From: Tarun Kanti DebBarma @ 2012-02-29 22:44 UTC (permalink / raw)
  To: linux-arm-kernel

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

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

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

Signed-off-by: Tarun Kanti DebBarma <tarun.kanti@ti.com>
---
 drivers/gpio/gpio-omap.c |   47 ----------------------------------------------
 1 files changed, 0 insertions(+), 47 deletions(-)

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2012-02-29 22:44 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-29 22:44 [PATCH v2 0/9] gpio/omap: Some more driver cleanup and fixes Tarun Kanti DebBarma
2012-02-29 22:44 ` [PATCH v2 1/9] gpio/omap: remove saved_fallingdetect, saved_risingdetect fields Tarun Kanti DebBarma
2012-02-29 22:44 ` [PATCH v2 2/9] gpio/omap: fix wakeup_en register update in _set_gpio_wakeup() Tarun Kanti DebBarma
2012-02-29 22:44 ` [PATCH v2 3/9] gpio/omap: remove suspend_wakeup field from struct gpio_bank Tarun Kanti DebBarma
2012-02-29 22:44 ` [PATCH v2 4/9] gpio/omap: remove saved_wakeup " Tarun Kanti DebBarma
2012-02-29 22:44 ` [PATCH v2 5/9] gpio/omap: get rid of retrigger variable in gpio_irq_handler Tarun Kanti DebBarma
2012-02-29 22:44 ` [PATCH v2 6/9] gpio/omap: fix trigger type to unsigned Tarun Kanti DebBarma
2012-02-29 22:44 ` [PATCH v2 7/9] gpio/omap: fix _set_gpio_irqenable implementation Tarun Kanti DebBarma
2012-02-29 22:44 ` [PATCH v2 8/9] gpio/omap: remove redundant decoding of gpio offset Tarun Kanti DebBarma
2012-02-29 22:44 ` [PATCH v2 9/9] gpio/omap: remove suspend/resume callbacks Tarun Kanti DebBarma

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).