[PATCH v2 1/2] ARM: omap4: gpio: fix setting IRQWAKEN bits

[PATCH v2 1/2] ARM: omap4: gpio: fix setting IRQWAKEN bits

[Colin Cross]

Setting the IRQWAKEN bit was overwriting previous IRQWAKEN bits,
causing only the last bit set to take effect, resulting in lost
wakeups when the GPIO controller is in idle.

Replace direct writes to IRQWAKEN with MOD_REG_BIT calls to
perform a read-modify-write on the register.

Signed-off-by: Colin Cross <ccross@android.com>
---
 drivers/gpio/gpio-omap.c |   12 ++----------
 1 files changed, 2 insertions(+), 10 deletions(-)

Santosh, I left your Acked-by off this patch because I changed
the register access to match the OMAP4 TRM.

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 6c51191..8ba6957 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -432,7 +432,6 @@ static inline void set_24xx_gpio_triggering(struct gpio_bank *bank, int gpio,
 {
 	void __iomem *base = bank->base;
 	u32 gpio_bit = 1 << gpio;
-	u32 val;
 
 	if (cpu_is_omap44xx()) {
 		MOD_REG_BIT(OMAP4_GPIO_LEVELDETECT0, gpio_bit,
@@ -455,15 +454,8 @@ static inline void set_24xx_gpio_triggering(struct gpio_bank *bank, int gpio,
 	}
 	if (likely(!(bank->non_wakeup_gpios & gpio_bit))) {
 		if (cpu_is_omap44xx()) {
-			if (trigger != 0)
-				__raw_writel(1 << gpio, bank->base+
-						OMAP4_GPIO_IRQWAKEN0);
-			else {
-				val = __raw_readl(bank->base +
-							OMAP4_GPIO_IRQWAKEN0);
-				__raw_writel(val & (~(1 << gpio)), bank->base +
-							 OMAP4_GPIO_IRQWAKEN0);
-			}
+			MOD_REG_BIT(OMAP4_GPIO_IRQWAKEN0, gpio_bit,
+				trigger != 0);
 		} else {
 			/*
 			 * GPIO wakeup request can only be generated on edge
-- 
1.7.4.1

[PATCH v2 2/2] ARM: omap4: gpio: add locking around calls to _set_gpio_triggering

[Colin Cross]

_set_gpio_triggering uses read-modify-write on bank registers,
lock bank->lock around all calls to it to prevent register
corruption if two cpus access gpios in the same bank at the
same time.

Signed-off-by: Colin Cross <ccross@android.com>
---
 drivers/gpio/gpio-omap.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 8ba6957..b14bfd4 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1126,8 +1126,11 @@ static void gpio_irq_shutdown(struct irq_data *d)
 {
 	unsigned int gpio = d->irq - IH_GPIO_BASE;
 	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
+	unsigned long flags;
 
+	spin_lock_irqsave(&bank->lock, flags);
 	_reset_gpio(bank, gpio);
+	spin_unlock_irqrestore(&bank->lock, flags);
 }
 
 static void gpio_ack_irq(struct irq_data *d)
@@ -1142,9 +1145,12 @@ static void gpio_mask_irq(struct irq_data *d)
 {
 	unsigned int gpio = d->irq - IH_GPIO_BASE;
 	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
+	unsigned long flags;
 
+	spin_lock_irqsave(&bank->lock, flags);
 	_set_gpio_irqenable(bank, gpio, 0);
 	_set_gpio_triggering(bank, get_gpio_index(gpio), IRQ_TYPE_NONE);
+	spin_unlock_irqrestore(&bank->lock, flags);
 }
 
 static void gpio_unmask_irq(struct irq_data *d)
@@ -1153,7 +1159,9 @@ static void gpio_unmask_irq(struct irq_data *d)
 	struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
 	unsigned int irq_mask = 1 << get_gpio_index(gpio);
 	u32 trigger = irqd_get_trigger_type(d);
+	unsigned long flags;
 
+	spin_lock_irqsave(&bank->lock, flags);
 	if (trigger)
 		_set_gpio_triggering(bank, get_gpio_index(gpio), trigger);
 
@@ -1165,6 +1173,7 @@ static void gpio_unmask_irq(struct irq_data *d)
 	}
 
 	_set_gpio_irqenable(bank, gpio, 1);
+	spin_unlock_irqrestore(&bank->lock, flags);
 }
 
 static struct irq_chip gpio_irq_chip = {
-- 
1.7.4.1

Re: [PATCH v2 1/2] ARM: omap4: gpio: fix setting IRQWAKEN bits

[Kevin Hilman]

Hi Colin,

Colin Cross <ccross@android.com> writes:

> Setting the IRQWAKEN bit was overwriting previous IRQWAKEN bits,
> causing only the last bit set to take effect, resulting in lost
> wakeups when the GPIO controller is in idle.
>
> Replace direct writes to IRQWAKEN with MOD_REG_BIT calls to
> perform a read-modify-write on the register.
>
> Signed-off-by: Colin Cross <ccross@android.com>

Thanks, I'll queue these both for Grant for the 3.0-rc fixes along with
another GPIO fix for a section mismatch I have queued.

Minor: now that this driver has moved to drivers, I changed the subject
prefixes slightly.  They now read:

GPIO: OMAP: fix setting IRQWAKEN bits for OMAP4
GPIO: OMAP: add locking around calls to _set_gpio_triggering

Kevin

Re: [PATCH v2 1/2] ARM: omap4: gpio: fix setting IRQWAKEN bits

[Grant Likely]

On Mon, Jun 6, 2011 at 5:18 PM, Kevin Hilman <khilman@ti.com> wrote:
> Hi Colin,
>
> Colin Cross <ccross@android.com> writes:
>
>> Setting the IRQWAKEN bit was overwriting previous IRQWAKEN bits,
>> causing only the last bit set to take effect, resulting in lost
>> wakeups when the GPIO controller is in idle.
>>
>> Replace direct writes to IRQWAKEN with MOD_REG_BIT calls to
>> perform a read-modify-write on the register.
>>
>> Signed-off-by: Colin Cross <ccross@android.com>
>
> Thanks, I'll queue these both for Grant for the 3.0-rc fixes along with
> another GPIO fix for a section mismatch I have queued.
>
> Minor: now that this driver has moved to drivers, I changed the subject
> prefixes slightly.  They now read:
>
> GPIO: OMAP: fix setting IRQWAKEN bits for OMAP4
> GPIO: OMAP: add locking around calls to _set_gpio_triggering

If you're collecting fixes for Linus anyway, go ahead and add my a-b
line and include these two to send on to Linus

Acked-by: Grant Likely <grant.likely@secretlab.ca>

Or, if they are only gpio fixes, then I'd also be happy to get a git
pull req for the changes.  :-)

g.

Re: [PATCH v2 1/2] ARM: omap4: gpio: fix setting IRQWAKEN bits

[Kevin Hilman]

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

> On Mon, Jun 6, 2011 at 5:18 PM, Kevin Hilman <khilman@ti.com> wrote:
>> Hi Colin,
>>
>> Colin Cross <ccross@android.com> writes:
>>
>>> Setting the IRQWAKEN bit was overwriting previous IRQWAKEN bits,
>>> causing only the last bit set to take effect, resulting in lost
>>> wakeups when the GPIO controller is in idle.
>>>
>>> Replace direct writes to IRQWAKEN with MOD_REG_BIT calls to
>>> perform a read-modify-write on the register.
>>>
>>> Signed-off-by: Colin Cross <ccross@android.com>
>>
>> Thanks, I'll queue these both for Grant for the 3.0-rc fixes along with
>> another GPIO fix for a section mismatch I have queued.
>>
>> Minor: now that this driver has moved to drivers, I changed the subject
>> prefixes slightly.  They now read:
>>
>> GPIO: OMAP: fix setting IRQWAKEN bits for OMAP4
>> GPIO: OMAP: add locking around calls to _set_gpio_triggering
>
> If you're collecting fixes for Linus anyway, go ahead and add my a-b
> line and include these two to send on to Linus
>
> Acked-by: Grant Likely <grant.likely@secretlab.ca>
>
> Or, if they are only gpio fixes, then I'd also be happy to get a git
> pull req for the changes.  :-)
>

I have a GPIO-only queue, so I'll have a pull req for you today.

Kevin

Re: [PATCH v2 1/2] ARM: omap4: gpio: fix setting IRQWAKEN bits

[Grant Likely]

On Tue, Jun 7, 2011 at 10:04 AM, Kevin Hilman <khilman@ti.com> wrote:
> Grant Likely <grant.likely@secretlab.ca> writes:
>
>> On Mon, Jun 6, 2011 at 5:18 PM, Kevin Hilman <khilman@ti.com> wrote:
>>> Hi Colin,
>>>
>>> Colin Cross <ccross@android.com> writes:
>>>
>>>> Setting the IRQWAKEN bit was overwriting previous IRQWAKEN bits,
>>>> causing only the last bit set to take effect, resulting in lost
>>>> wakeups when the GPIO controller is in idle.
>>>>
>>>> Replace direct writes to IRQWAKEN with MOD_REG_BIT calls to
>>>> perform a read-modify-write on the register.
>>>>
>>>> Signed-off-by: Colin Cross <ccross@android.com>
>>>
>>> Thanks, I'll queue these both for Grant for the 3.0-rc fixes along with
>>> another GPIO fix for a section mismatch I have queued.
>>>
>>> Minor: now that this driver has moved to drivers, I changed the subject
>>> prefixes slightly.  They now read:
>>>
>>> GPIO: OMAP: fix setting IRQWAKEN bits for OMAP4
>>> GPIO: OMAP: add locking around calls to _set_gpio_triggering
>>
>> If you're collecting fixes for Linus anyway, go ahead and add my a-b
>> line and include these two to send on to Linus
>>
>> Acked-by: Grant Likely <grant.likely@secretlab.ca>
>>
>> Or, if they are only gpio fixes, then I'd also be happy to get a git
>> pull req for the changes.  :-)
>>
>
> I have a GPIO-only queue, so I'll have a pull req for you today.

thx.

g.