diff mbox

OMAP2/3 Avoid GPIO pending irq status been set after irq_disable

Message ID B00E06E2766C2744B022DE9BAF3C59D532D196@zmy16exm69.ds.mot.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Wang Sawsd-A24013 June 1, 2009, 11:24 p.m. UTC 
This patch adds irq_enable and irq_disable for OMAP GPIO IRQ chip, and
also only enable WAKEUPEN
When GPIO edge detection is enabled, also clear the gpio event
triggering configurations to avoid
Pending interrupt status which is generated regardless of the IRQEN and
WKUPEN bit, the pending
IRQ status may prevent GPIO module acknowledge IDLE request and prevent
PER and thus RETENTION.

This is only applied for OMAP2/3 GPIO IRQs.

Signed-off-by: Chunqiu Wang <cqwang@motorola.com>
---
 arch/arm/plat-omap/gpio.c |   44
++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 42 insertions(+), 2 deletions(-)

 {
 	void __iomem *base = bank->base;
@@ -597,7 +597,12 @@ static inline void set_24xx_gpio_triggering(struct
gpio_bank *bank, int gpio,
 		trigger & IRQ_TYPE_EDGE_FALLING);
 
 	if (likely(!(bank->non_wakeup_gpios & gpio_bit))) {
-		if (trigger != 0)
+		/*
+		 * GPIO wakeup request can only be generated on edge
+		 * transitions, see OMAP34xx_ES3.1_TRM_V_Q G25.5.3.1
+		 * wake-up request note for detail
+		 */
+		if ((trigger & IRQ_TYPE_EDGE_BOTH) != 0)
 			__raw_writel(1 << gpio, bank->base
 					+ OMAP24XX_GPIO_SETWKUENA);
 		else
@@ -1133,6 +1138,39 @@ static void gpio_ack_irq(unsigned int irq)
 	_clear_gpio_irqstatus(bank, gpio);
 }
 
+static void gpio_enable_irq(unsigned int irq)
+{
+	unsigned int gpio = irq - IH_GPIO_BASE;
+	struct gpio_bank *bank = get_irq_chip_data(irq);
+	int trigger = irq_desc[irq].status & IRQ_TYPE_SENSE_MASK;
+
+#if defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX)
+	set_24xx_gpio_triggering(bank, get_gpio_index(gpio), trigger);
+#endif
+	_set_gpio_irqenable(bank, gpio, 1);
+}
+
+static void gpio_disable_irq(unsigned int irq)
+{
+	unsigned int gpio = irq - IH_GPIO_BASE;
+	struct gpio_bank *bank = get_irq_chip_data(irq);
+
+	_set_gpio_irqenable(bank, gpio, 0);
+#if defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX)
+	/*
+	 * Clear the event detect setting and IRQ status to prevent
+	 * IRQ staus been set or pending there While IRQ is disabled,
+	 * otherwise GPIO module will not acknowledge the IDLE request
+	 * from PER power domain, this may prevent System wide retention
+	 * See OMAP34xx_ES3.1_TRM_V_Q G25.5.3.1 GPIO interrupt and
wakeup
+	 * enable note for detail
+	 */
+	set_24xx_gpio_triggering(bank, get_gpio_index(gpio),
IRQ_TYPE_NONE);
+	_clear_gpio_irqstatus(bank, gpio);
+#endif
+
+}
+
 static void gpio_mask_irq(unsigned int irq)
 {
 	unsigned int gpio = irq - IH_GPIO_BASE;
@@ -1160,6 +1198,8 @@ static void gpio_unmask_irq(unsigned int irq)
 static struct irq_chip gpio_irq_chip = {
 	.name		= "GPIO",
 	.shutdown	= gpio_irq_shutdown,
+	.enable		= gpio_enable_irq,
+	.disable	= gpio_disable_irq,
 	.ack		= gpio_ack_irq,
 	.mask		= gpio_mask_irq,
 	.unmask		= gpio_unmask_irq,

Comments

Tony Lindgren Aug. 5, 2009, 12:33 p.m. UTC | #1 
Hi,

Sorry for the long delay on replying to this one.

* Wang Sawsd-A24013 <cqwang@motorola.com> [090602 02:25]:
> This patch adds irq_enable and irq_disable for OMAP GPIO IRQ chip, and
> also only enable WAKEUPEN
> When GPIO edge detection is enabled, also clear the gpio event
> triggering configurations to avoid
> Pending interrupt status which is generated regardless of the IRQEN and
> WKUPEN bit, the pending
> IRQ status may prevent GPIO module acknowledge IDLE request and prevent
> PER and thus RETENTION.
> 
> This is only applied for OMAP2/3 GPIO IRQs.
> 
> Signed-off-by: Chunqiu Wang <cqwang@motorola.com>
> ---
>  arch/arm/plat-omap/gpio.c |   44
> ++++++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 42 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c
> index bd04b40..19e0461 100644
> --- a/arch/arm/plat-omap/gpio.c
> +++ b/arch/arm/plat-omap/gpio.c
> @@ -581,7 +581,7 @@ void omap_set_gpio_debounce_time(int gpio, int
> enc_time)
>  EXPORT_SYMBOL(omap_set_gpio_debounce_time);
>  
>  #if defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX)
> -static inline void set_24xx_gpio_triggering(struct gpio_bank *bank, int
> gpio,
> +static void set_24xx_gpio_triggering(struct gpio_bank *bank, int gpio,
>  						int trigger)
>  {
>  	void __iomem *base = bank->base;
> @@ -597,7 +597,12 @@ static inline void set_24xx_gpio_triggering(struct
> gpio_bank *bank, int gpio,
>  		trigger & IRQ_TYPE_EDGE_FALLING);
>  
>  	if (likely(!(bank->non_wakeup_gpios & gpio_bit))) {
> -		if (trigger != 0)
> +		/*
> +		 * GPIO wakeup request can only be generated on edge
> +		 * transitions, see OMAP34xx_ES3.1_TRM_V_Q G25.5.3.1
> +		 * wake-up request note for detail
> +		 */
> +		if ((trigger & IRQ_TYPE_EDGE_BOTH) != 0)
>  			__raw_writel(1 << gpio, bank->base
>  					+ OMAP24XX_GPIO_SETWKUENA);
>  		else

To me it looks like the right way to deal with the level gpio
would be to change it temporarily to be edge for the duration of
idle, then change it back to be level after returning from idle.


> @@ -1133,6 +1138,39 @@ static void gpio_ack_irq(unsigned int irq)
>  	_clear_gpio_irqstatus(bank, gpio);
>  }
>  
> +static void gpio_enable_irq(unsigned int irq)
> +{
> +	unsigned int gpio = irq - IH_GPIO_BASE;
> +	struct gpio_bank *bank = get_irq_chip_data(irq);
> +	int trigger = irq_desc[irq].status & IRQ_TYPE_SENSE_MASK;
> +
> +#if defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX)
> +	set_24xx_gpio_triggering(bank, get_gpio_index(gpio), trigger);
> +#endif
> +	_set_gpio_irqenable(bank, gpio, 1);
> +}
> +
> +static void gpio_disable_irq(unsigned int irq)
> +{
> +	unsigned int gpio = irq - IH_GPIO_BASE;
> +	struct gpio_bank *bank = get_irq_chip_data(irq);
> +
> +	_set_gpio_irqenable(bank, gpio, 0);
> +#if defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX)
> +	/*
> +	 * Clear the event detect setting and IRQ status to prevent
> +	 * IRQ staus been set or pending there While IRQ is disabled,
> +	 * otherwise GPIO module will not acknowledge the IDLE request
> +	 * from PER power domain, this may prevent System wide retention
> +	 * See OMAP34xx_ES3.1_TRM_V_Q G25.5.3.1 GPIO interrupt and
> wakeup
> +	 * enable note for detail
> +	 */
> +	set_24xx_gpio_triggering(bank, get_gpio_index(gpio),
> IRQ_TYPE_NONE);
> +	_clear_gpio_irqstatus(bank, gpio);
> +#endif
> +
> +}
> +
>  static void gpio_mask_irq(unsigned int irq)
>  {
>  	unsigned int gpio = irq - IH_GPIO_BASE;
> @@ -1160,6 +1198,8 @@ static void gpio_unmask_irq(unsigned int irq)
>  static struct irq_chip gpio_irq_chip = {
>  	.name		= "GPIO",
>  	.shutdown	= gpio_irq_shutdown,
> +	.enable		= gpio_enable_irq,
> +	.disable	= gpio_disable_irq,
>  	.ack		= gpio_ack_irq,
>  	.mask		= gpio_mask_irq,
>  	.unmask		= gpio_unmask_irq,

Also, this irq_enable/disable should be a separate patch.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kevin Hilman Aug. 5, 2009, 2:36 p.m. UTC | #2 
Tony Lindgren <tony@atomide.com> writes:

> Hi,
>
> Sorry for the long delay on replying to this one.

Tony, this one has been superceded, and the irq_enable/disable stuff no
longer is needed due to the patch

  OMAP: GPIO: clear/restore level/edge detect settings on mask/unmask

from my PM fixes queue.

An updated version (currently in the PM branch) is here:
http://git.kernel.org/?p=linux/kernel/git/khilman/linux-omap-pm.git;a=commit;h=c80a909697ad931ffad9d30ed321469fa699b208

but...

> * Wang Sawsd-A24013 <cqwang@motorola.com> [090602 02:25]:
>> This patch adds irq_enable and irq_disable for OMAP GPIO IRQ chip, and
>> also only enable WAKEUPEN
>> When GPIO edge detection is enabled, also clear the gpio event
>> triggering configurations to avoid
>> Pending interrupt status which is generated regardless of the IRQEN and
>> WKUPEN bit, the pending
>> IRQ status may prevent GPIO module acknowledge IDLE request and prevent
>> PER and thus RETENTION.
>> 
>> This is only applied for OMAP2/3 GPIO IRQs.
>> 
>> Signed-off-by: Chunqiu Wang <cqwang@motorola.com>
>> ---
>>  arch/arm/plat-omap/gpio.c |   44
>> ++++++++++++++++++++++++++++++++++++++++++--
>>  1 files changed, 42 insertions(+), 2 deletions(-)
>> 
>> diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c
>> index bd04b40..19e0461 100644
>> --- a/arch/arm/plat-omap/gpio.c
>> +++ b/arch/arm/plat-omap/gpio.c
>> @@ -581,7 +581,7 @@ void omap_set_gpio_debounce_time(int gpio, int
>> enc_time)
>>  EXPORT_SYMBOL(omap_set_gpio_debounce_time);
>>  
>>  #if defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX)
>> -static inline void set_24xx_gpio_triggering(struct gpio_bank *bank, int
>> gpio,
>> +static void set_24xx_gpio_triggering(struct gpio_bank *bank, int gpio,
>>  						int trigger)
>>  {
>>  	void __iomem *base = bank->base;
>> @@ -597,7 +597,12 @@ static inline void set_24xx_gpio_triggering(struct
>> gpio_bank *bank, int gpio,
>>  		trigger & IRQ_TYPE_EDGE_FALLING);
>>  
>>  	if (likely(!(bank->non_wakeup_gpios & gpio_bit))) {
>> -		if (trigger != 0)
>> +		/*
>> +		 * GPIO wakeup request can only be generated on edge
>> +		 * transitions, see OMAP34xx_ES3.1_TRM_V_Q G25.5.3.1
>> +		 * wake-up request note for detail
>> +		 */
>> +		if ((trigger & IRQ_TYPE_EDGE_BOTH) != 0)
>>  			__raw_writel(1 << gpio, bank->base
>>  					+ OMAP24XX_GPIO_SETWKUENA);
>>  		else
>
> To me it looks like the right way to deal with the level gpio
> would be to change it temporarily to be edge for the duration of
> idle, then change it back to be level after returning from idle.

Not sure about that.  That would have to be experimented with to 
see if there are any other side effects.

In any case, that kind of change would be independent of this change
as this change just fixes a bug so wake-enables are only set on
edge-triggered GPIOs.

Kevin

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tony Lindgren Aug. 5, 2009, 3:11 p.m. UTC | #3 
* Kevin Hilman <khilman@deeprootsystems.com> [090805 17:36]:
> Tony Lindgren <tony@atomide.com> writes:
> 
> > Hi,
> >
> > Sorry for the long delay on replying to this one.
> 
> Tony, this one has been superceded, and the irq_enable/disable stuff no
> longer is needed due to the patch
> 
>   OMAP: GPIO: clear/restore level/edge detect settings on mask/unmask
> 
> from my PM fixes queue.
> 
> An updated version (currently in the PM branch) is here:
> http://git.kernel.org/?p=linux/kernel/git/khilman/linux-omap-pm.git;a=commit;h=c80a909697ad931ffad9d30ed321469fa699b208

OK, thanks.


> 
> but...
> 

<snip>

> > To me it looks like the right way to deal with the level gpio
> > would be to change it temporarily to be edge for the duration of
> > idle, then change it back to be level after returning from idle.
> 
> Not sure about that.  That would have to be experimented with to 
> see if there are any other side effects.
> 
> In any case, that kind of change would be independent of this change
> as this change just fixes a bug so wake-enables are only set on
> edge-triggered GPIOs.

OK. Just FYI, I think I experimented on that for 2420 at some point
and it worked, but that was several years ago.

Tony

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c
index bd04b40..19e0461 100644
--- a/arch/arm/plat-omap/gpio.c
+++ b/arch/arm/plat-omap/gpio.c
@@ -581,7 +581,7 @@  void omap_set_gpio_debounce_time(int gpio, int
enc_time)
 EXPORT_SYMBOL(omap_set_gpio_debounce_time);
 
 #if defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX)
-static inline void set_24xx_gpio_triggering(struct gpio_bank *bank, int
gpio,
+static void set_24xx_gpio_triggering(struct gpio_bank *bank, int gpio,
 						int trigger)