[PATCH v4 RESEND 0/7] gpio/omap: Some more driver fixes

[PATCH v4 RESEND 0/7] gpio/omap: Some more driver fixes

[Tarun Kanti DebBarma]

This series excludes the cleanup patches as suggested by Kevin from
the previously posted series.

The fixes include correction of _set_gpio_irqenable() implementation,
missing wakeup_en register update in set_gpio_wakeup(), type mismatch
of gpio trigger parameter in set_gpio_trigger(), incorrect dataout
register update in set_gpio_dataout_() and few corrections in context
save logic.

It is baselined on top of:
git://git.secretlab.ca/git/linux-2.6.git gpio/next
Commit: 81b279d80a63628e580c71a31d30a8c3b3047ad4

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

Power Test:
Off-mode and Retention on OMAP3430 (Suspend and Idle paths).
Also confirmed that dataout register content preserved over
off-mode.

Functional Test:
OMAP2430, OMAP3430SDP, ZOOM3, OMAP4430, OMAP4-BLAZE

v4:
a) Implemented all comments on v3 which are mostly related to
avoiding unnecessary register read while updating the context.

b) Folded:
gpio/omap: fix dataout register overwrite in _set_gpio_dataout
into:
gpio/omap: fix missing dataout context save in _set_gpio_dataout_reg

v3:
- Added 4 more additional patches to the previous series
 which are all bug fixes.

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 (7):
  gpio/omap: fix wakeup_en register update in _set_gpio_wakeup()
  gpio/omap: fix trigger type to unsigned
  gpio/omap: fix _set_gpio_irqenable implementation
  gpio/omap: fix missing dataout context save in _set_gpio_dataout_reg
  gpio/omap: fix incorrect context restore logic in omap_gpio_runtime_*
  gpio/omap: fix incorrect update to context.irqenable1
  gpio/omap: fix redundant decoding of gpio offset

 drivers/gpio/gpio-omap.c |   47 ++++++++++++++++++++++++---------------------
 1 files changed, 25 insertions(+), 22 deletions(-)

[PATCH v4 RESEND 1/7] gpio/omap: fix wakeup_en register update in _set_gpio_wakeup()

[Tarun Kanti DebBarma]

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.

irq_set_type()->gpio_irq_type()->_set_gpio_triggering()->set_gpio_trigger()
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 7cbad85..1a144ac 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -511,6 +511,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

[PATCH v4 RESEND 2/7] gpio/omap: fix trigger type to unsigned

[Tarun Kanti DebBarma]

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 1a144ac..2042857 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -245,7 +245,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;
@@ -327,7 +327,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

[PATCH v4 RESEND 3/7] gpio/omap: fix _set_gpio_irqenable implementation

[Tarun Kanti DebBarma]

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 2042857..8901d57 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -484,7 +484,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

[PATCH v4 RESEND 4/7] gpio/omap: fix missing dataout context save in _set_gpio_dataout_reg

[Tarun Kanti DebBarma]

There are two functions, _set_gpio_dataout_reg() and _set_gpio_dataout_mask()
which writes to dataout register and the dataout context must be saved.
It is missing in the first function, _set_gpio_dataout_reg(). Fix this.

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

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 8901d57..bbe9648 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -120,10 +120,13 @@ static void _set_gpio_dataout_reg(struct gpio_bank *bank, int gpio, int enable)
 	void __iomem *reg = bank->base;
 	u32 l = GPIO_BIT(bank, gpio);
 
-	if (enable)
+	if (enable) {
 		reg += bank->regs->set_dataout;
-	else
+		bank->context.dataout |= l;
+	} else {
 		reg += bank->regs->clr_dataout;
+		bank->context.dataout &= ~l;
+	}
 
 	__raw_writel(l, reg);
 }
-- 
1.7.0.4

[PATCH v4 RESEND 5/7] gpio/omap: fix incorrect context restore logic in omap_gpio_runtime_*

[Tarun Kanti DebBarma]

In omap_gpio_runtime_suspend/resume() the context save/restore should
be independent of bank->enabled_non_wakeup_gpios. This was preventing
context restore of GPIO lines which are not wakeup enabled.

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

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index bbe9648..bcb1061 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1247,9 +1247,6 @@ static int omap_gpio_runtime_suspend(struct device *dev)
 	 * non-wakeup GPIOs.  Otherwise spurious IRQs will be
 	 * generated.  See OMAP2420 Errata item 1.101.
 	 */
-	if (!(bank->enabled_non_wakeup_gpios))
-		goto update_gpio_context_count;
-
 	bank->saved_datain = __raw_readl(bank->base +
 						bank->regs->datain);
 	l1 = __raw_readl(bank->base + bank->regs->fallingdetect);
@@ -1298,7 +1295,7 @@ static int omap_gpio_runtime_resume(struct device *dev)
 	__raw_writel(bank->context.risingdetect,
 		     bank->base + bank->regs->risingdetect);
 
-	if (!bank->enabled_non_wakeup_gpios || !bank->workaround_enabled) {
+	if (!bank->workaround_enabled) {
 		spin_unlock_irqrestore(&bank->lock, flags);
 		return 0;
 	}
-- 
1.7.0.4

[PATCH v4 RESEND 6/7] gpio/omap: fix incorrect update to context.irqenable1

[Tarun Kanti DebBarma]

In _enable_gpio_irqbank() when bank->regs->set_irqenable is valid,
gpio_mask can be directly set by writing to set_irqenable register
without overwriting current value. In order to ensure the same is
stored in context.irqenable1, we must read from regs->irqenable
instead of overwriting it with gpio_mask.
The overwriting makes sense only in the second case where irqenable
is explicitly read and updated with new gpio_mask before writing it
back. However, for consistency reading regs->irqenable into the
bank->context.irqenable1 takes care of both the scenarios.

        if (bank->regs->set_irqenable) {
                reg += bank->regs->set_irqenable;
                l = gpio_mask;
        } else {
                reg += bank->regs->irqenable;
                l = __raw_readl(reg);
                if (bank->regs->irqenable_inv)
                        l &= ~gpio_mask;
                else
                        l |= gpio_mask;
        }

Make the same change for _disable_gpio_irqbank().

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

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index bcb1061..6c17e58 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -451,6 +451,7 @@ static void _enable_gpio_irqbank(struct gpio_bank *bank, int gpio_mask)
 	if (bank->regs->set_irqenable) {
 		reg += bank->regs->set_irqenable;
 		l = gpio_mask;
+		bank->context.irqenable1 |= gpio_mask;
 	} else {
 		reg += bank->regs->irqenable;
 		l = __raw_readl(reg);
@@ -458,10 +459,10 @@ static void _enable_gpio_irqbank(struct gpio_bank *bank, int gpio_mask)
 			l &= ~gpio_mask;
 		else
 			l |= gpio_mask;
+		bank->context.irqenable1 = l;
 	}
 
 	__raw_writel(l, reg);
-	bank->context.irqenable1 = l;
 }
 
 static void _disable_gpio_irqbank(struct gpio_bank *bank, int gpio_mask)
@@ -472,6 +473,7 @@ static void _disable_gpio_irqbank(struct gpio_bank *bank, int gpio_mask)
 	if (bank->regs->clr_irqenable) {
 		reg += bank->regs->clr_irqenable;
 		l = gpio_mask;
+		bank->context.irqenable1 &= ~gpio_mask;
 	} else {
 		reg += bank->regs->irqenable;
 		l = __raw_readl(reg);
@@ -479,10 +481,10 @@ static void _disable_gpio_irqbank(struct gpio_bank *bank, int gpio_mask)
 			l |= gpio_mask;
 		else
 			l &= ~gpio_mask;
+		bank->context.irqenable1 = l;
 	}
 
 	__raw_writel(l, reg);
-	bank->context.irqenable1 = l;
 }
 
 static inline void _set_gpio_irqenable(struct gpio_bank *bank, int gpio, int enable)
-- 
1.7.0.4

[PATCH v4 RESEND 7/7] gpio/omap: fix redundant decoding of gpio offset

[Tarun Kanti DebBarma]

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:

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>
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@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 6c17e58..1adc2ec 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -147,18 +147,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)
@@ -865,19 +865,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

[PATCH v4 RESEND 0/7] gpio/omap: Some more driver fixes

[Grant Likely]

On Tue, 20 Mar 2012 16:23:12 +0530, Tarun Kanti DebBarma <tarun.kanti@ti.com> wrote:
> This series excludes the cleanup patches as suggested by Kevin from
> the previously posted series.
> 
> The fixes include correction of _set_gpio_irqenable() implementation,
> missing wakeup_en register update in set_gpio_wakeup(), type mismatch
> of gpio trigger parameter in set_gpio_trigger(), incorrect dataout
> register update in set_gpio_dataout_() and few corrections in context
> save logic.
> 
> It is baselined on top of:
> git://git.secretlab.ca/git/linux-2.6.git gpio/next
> Commit: 81b279d80a63628e580c71a31d30a8c3b3047ad4
> 
> Series is available here for reference:
> git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev for_3.4/gpio_more_fixes

Merged; thanks

g.

> 
> Power Test:
> Off-mode and Retention on OMAP3430 (Suspend and Idle paths).
> Also confirmed that dataout register content preserved over
> off-mode.
> 
> Functional Test:
> OMAP2430, OMAP3430SDP, ZOOM3, OMAP4430, OMAP4-BLAZE
> 
> v4:
> a) Implemented all comments on v3 which are mostly related to
> avoiding unnecessary register read while updating the context.
> 
> b) Folded:
> gpio/omap: fix dataout register overwrite in _set_gpio_dataout
> into:
> gpio/omap: fix missing dataout context save in _set_gpio_dataout_reg
> 
> v3:
> - Added 4 more additional patches to the previous series
>  which are all bug fixes.
> 
> 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 (7):
>   gpio/omap: fix wakeup_en register update in _set_gpio_wakeup()
>   gpio/omap: fix trigger type to unsigned
>   gpio/omap: fix _set_gpio_irqenable implementation
>   gpio/omap: fix missing dataout context save in _set_gpio_dataout_reg
>   gpio/omap: fix incorrect context restore logic in omap_gpio_runtime_*
>   gpio/omap: fix incorrect update to context.irqenable1
>   gpio/omap: fix redundant decoding of gpio offset
> 
>  drivers/gpio/gpio-omap.c |   47 ++++++++++++++++++++++++---------------------
>  1 files changed, 25 insertions(+), 22 deletions(-)
> 

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

[PATCH v4 RESEND 0/7] gpio/omap: Some more driver fixes

[Kevin Hilman]

Hi Grant,

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

> On Tue, 20 Mar 2012 16:23:12 +0530, Tarun Kanti DebBarma <tarun.kanti@ti.com> wrote:
>> This series excludes the cleanup patches as suggested by Kevin from
>> the previously posted series.
>> 
>> The fixes include correction of _set_gpio_irqenable() implementation,
>> missing wakeup_en register update in set_gpio_wakeup(), type mismatch
>> of gpio trigger parameter in set_gpio_trigger(), incorrect dataout
>> register update in set_gpio_dataout_() and few corrections in context
>> save logic.
>> 
>> It is baselined on top of:
>> git://git.secretlab.ca/git/linux-2.6.git gpio/next
>> Commit: 81b279d80a63628e580c71a31d30a8c3b3047ad4
>> 
>> Series is available here for reference:
>> git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev for_3.4/gpio_more_fixes
>
> Merged; thanks

Can you hold off slightly before merging this.

I haven't been through v4 yet or tested the fixes alone now that they're
separated from the other cleanups.

Expect a pull request from me when ready.

Thanks,

Kevin

[PATCH v4 RESEND 6/7] gpio/omap: fix incorrect update to context.irqenable1

[Kevin Hilman]

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

> In _enable_gpio_irqbank() when bank->regs->set_irqenable is valid,
> gpio_mask can be directly set by writing to set_irqenable register
> without overwriting current value. In order to ensure the same is
> stored in context.irqenable1, we must read from regs->irqenable
> instead of overwriting it with gpio_mask.

> The overwriting makes sense only in the second case where irqenable
> is explicitly read and updated with new gpio_mask before writing it
> back. However, for consistency reading regs->irqenable into the
> bank->context.irqenable1 takes care of both the scenarios.

...except that the code doesn't do this anymore.

I like the newer version (I hope so, since I suggested it :), but please
update the changlog to describe what the code is actually doing.

Thanks,

Kevin

[PATCH v4 RESEND 6/7] gpio/omap: fix incorrect update to context.irqenable1

[DebBarma, Tarun Kanti]

On Tue, Mar 20, 2012 at 11:31 PM, Kevin Hilman <khilman@ti.com> wrote:
> Tarun Kanti DebBarma <tarun.kanti@ti.com> writes:
>
>> In _enable_gpio_irqbank() when bank->regs->set_irqenable is valid,
>> gpio_mask can be directly set by writing to set_irqenable register
>> without overwriting current value. In order to ensure the same is
>> stored in context.irqenable1, we must read from regs->irqenable
>> instead of overwriting it with gpio_mask.
>
>> The overwriting makes sense only in the second case where irqenable
>> is explicitly read and updated with new gpio_mask before writing it
>> back. However, for consistency reading regs->irqenable into the
>> bank->context.irqenable1 takes care of both the scenarios.
>
> ...except that the code doesn't do this anymore.
Yes.
>
> I like the newer version (I hope so, since I suggested it :), but please
> update the changlog to describe what the code is actually doing.
Sure.
--
Tarun
>
> Thanks,
>
> Kevin

[PATCH v4 RESEND 6/7] gpio/omap: fix incorrect update to context.irqenable1

[DebBarma, Tarun Kanti]

On Wed, Mar 21, 2012 at 8:10 AM, DebBarma, Tarun Kanti
<tarun.kanti@ti.com> wrote:
> On Tue, Mar 20, 2012 at 11:31 PM, Kevin Hilman <khilman@ti.com> wrote:
>> Tarun Kanti DebBarma <tarun.kanti@ti.com> writes:
>>
>>> In _enable_gpio_irqbank() when bank->regs->set_irqenable is valid,
>>> gpio_mask can be directly set by writing to set_irqenable register
>>> without overwriting current value. In order to ensure the same is
>>> stored in context.irqenable1, we must read from regs->irqenable
>>> instead of overwriting it with gpio_mask.
>>
>>> The overwriting makes sense only in the second case where irqenable
>>> is explicitly read and updated with new gpio_mask before writing it
>>> back. However, for consistency reading regs->irqenable into the
>>> bank->context.irqenable1 takes care of both the scenarios.
>>
>> ...except that the code doesn't do this anymore.
> Yes.
>>
>> I like the newer version (I hope so, since I suggested it :), but please
>> update the changlog to describe what the code is actually doing.
> Sure.
I have updated the change log here:
git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev
for_3.4/gpio_more_fixes
--
Tarun
>>
>> Thanks,
>>
>> Kevin

[PATCH v4 RESEND 6/7] gpio/omap: fix incorrect update to context.irqenable1

[Kevin Hilman]

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

> On Wed, Mar 21, 2012 at 8:10 AM, DebBarma, Tarun Kanti
> <tarun.kanti@ti.com> wrote:
>> On Tue, Mar 20, 2012 at 11:31 PM, Kevin Hilman <khilman@ti.com> wrote:
>>> Tarun Kanti DebBarma <tarun.kanti@ti.com> writes:
>>>
>>>> In _enable_gpio_irqbank() when bank->regs->set_irqenable is valid,
>>>> gpio_mask can be directly set by writing to set_irqenable register
>>>> without overwriting current value. In order to ensure the same is
>>>> stored in context.irqenable1, we must read from regs->irqenable
>>>> instead of overwriting it with gpio_mask.
>>>
>>>> The overwriting makes sense only in the second case where irqenable
>>>> is explicitly read and updated with new gpio_mask before writing it
>>>> back. However, for consistency reading regs->irqenable into the
>>>> bank->context.irqenable1 takes care of both the scenarios.
>>>
>>> ...except that the code doesn't do this anymore.
>> Yes.
>>>
>>> I like the newer version (I hope so, since I suggested it :), but please
>>> update the changlog to describe what the code is actually doing.
>> Sure.
> I have updated the change log here:
> git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev
> for_3.4/gpio_more_fixes

Please also post and updated version of the patch for review.

Kevin

[PATCH v4 RESEND 0/7] gpio/omap: Some more driver fixes

[Grant Likely]

On Tue, 20 Mar 2012 10:46:57 -0700, Kevin Hilman <khilman@ti.com> wrote:
> Hi Grant,
> 
> Grant Likely <grant.likely@secretlab.ca> writes:
> 
> > On Tue, 20 Mar 2012 16:23:12 +0530, Tarun Kanti DebBarma <tarun.kanti@ti.com> wrote:
> >> This series excludes the cleanup patches as suggested by Kevin from
> >> the previously posted series.
> >> 
> >> The fixes include correction of _set_gpio_irqenable() implementation,
> >> missing wakeup_en register update in set_gpio_wakeup(), type mismatch
> >> of gpio trigger parameter in set_gpio_trigger(), incorrect dataout
> >> register update in set_gpio_dataout_() and few corrections in context
> >> save logic.
> >> 
> >> It is baselined on top of:
> >> git://git.secretlab.ca/git/linux-2.6.git gpio/next
> >> Commit: 81b279d80a63628e580c71a31d30a8c3b3047ad4
> >> 
> >> Series is available here for reference:
> >> git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev for_3.4/gpio_more_fixes
> >
> > Merged; thanks
> 
> Can you hold off slightly before merging this.
> 
> I haven't been through v4 yet or tested the fixes alone now that they're
> separated from the other cleanups.
> 
> Expect a pull request from me when ready.

Okay.  It's in linux-next at the moment, but I'll pull it back out.

g.

[PATCH v4 RESEND 6/7] gpio/omap: fix incorrect update to context.irqenable1

[DebBarma, Tarun Kanti]

On Wed, Mar 21, 2012 at 7:36 PM, Kevin Hilman <khilman@ti.com> wrote:
> "DebBarma, Tarun Kanti" <tarun.kanti@ti.com> writes:
>
>> On Wed, Mar 21, 2012 at 8:10 AM, DebBarma, Tarun Kanti
>> <tarun.kanti@ti.com> wrote:
>>> On Tue, Mar 20, 2012 at 11:31 PM, Kevin Hilman <khilman@ti.com> wrote:
>>>> Tarun Kanti DebBarma <tarun.kanti@ti.com> writes:
>>>>
>>>>> In _enable_gpio_irqbank() when bank->regs->set_irqenable is valid,
>>>>> gpio_mask can be directly set by writing to set_irqenable register
>>>>> without overwriting current value. In order to ensure the same is
>>>>> stored in context.irqenable1, we must read from regs->irqenable
>>>>> instead of overwriting it with gpio_mask.
>>>>
>>>>> The overwriting makes sense only in the second case where irqenable
>>>>> is explicitly read and updated with new gpio_mask before writing it
>>>>> back. However, for consistency reading regs->irqenable into the
>>>>> bank->context.irqenable1 takes care of both the scenarios.
>>>>
>>>> ...except that the code doesn't do this anymore.
>>> Yes.
>>>>
>>>> I like the newer version (I hope so, since I suggested it :), but please
>>>> update the changlog to describe what the code is actually doing.
>>> Sure.
>> I have updated the change log here:
>> git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev
>> for_3.4/gpio_more_fixes
>
> Please also post and updated version of the patch for review.
Sure.
--
Tarun
>
> Kevin

[PATCH v4 RESEND 0/7] gpio/omap: Some more driver fixes

[Kevin Hilman]

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

> On Tue, 20 Mar 2012 10:46:57 -0700, Kevin Hilman <khilman@ti.com> wrote:
>> Hi Grant,
>> 
>> Grant Likely <grant.likely@secretlab.ca> writes:
>> 
>> > On Tue, 20 Mar 2012 16:23:12 +0530, Tarun Kanti DebBarma <tarun.kanti@ti.com> wrote:
>> >> This series excludes the cleanup patches as suggested by Kevin from
>> >> the previously posted series.
>> >> 
>> >> The fixes include correction of _set_gpio_irqenable() implementation,
>> >> missing wakeup_en register update in set_gpio_wakeup(), type mismatch
>> >> of gpio trigger parameter in set_gpio_trigger(), incorrect dataout
>> >> register update in set_gpio_dataout_() and few corrections in context
>> >> save logic.
>> >> 
>> >> It is baselined on top of:
>> >> git://git.secretlab.ca/git/linux-2.6.git gpio/next
>> >> Commit: 81b279d80a63628e580c71a31d30a8c3b3047ad4
>> >> 
>> >> Series is available here for reference:
>> >> git://gitorious.org/~tarunkanti/omap-sw-develoment/tarunkantis-linux-omap-dev for_3.4/gpio_more_fixes
>> >
>> > Merged; thanks
>> 
>> Can you hold off slightly before merging this.
>> 
>> I haven't been through v4 yet or tested the fixes alone now that they're
>> separated from the other cleanups.
>> 
>> Expect a pull request from me when ready.
>
> Okay.  It's in linux-next at the moment, but I'll pull it back out.

Thanks.

I've been through this series now, pull request coming shortly.
Hopefully it can still make it for v3.4 since this fixes some
regressions introduced by the previous series. :(

Kevin