Re: [PATCH v3 2/3] gpio/omap: modify wake-up register with interrupt enable.

2013-05-31 Thread Kevin Hilman
Andreas Fenkart andreas.fenk...@streamunlimited.com writes:

 OMAP4430 TRM chap. 25.4.5.2
 To reduce dynamic consumption, an efficient idle scheme is based on the
 following:
 • An efficient local autoclock gating for each module
 • The implementation of control sideband signals between the PRCM module
   and each module
 This enhanced idle control allows clocks to be activated and deactivated
 safely without requiring complex software management. The idle mode
 request, idle acknowledge, and wake-up request are sideband signals
 between the PRCM module and the general-purpose interface

 OMAP4430 TRM chap. 25.4.6.2
 There must be a correlation between the wake-up enable and interrupt
 enable register. If a GPIO pin has a wake-up configured on it, it must
 also have the corresponding interrupt enabled. Otherwise, it is possible
 there is a wake-up event, but after exiting the IDLE state, no interrupt
 is generated; the corresponding bit from the interrupt status register is
 not cleared, and the module does not acknowledge a future idle request.

 Up to now _set_gpio_triggering() is also handling the wake-up enable
 register. According the TRM this should be in sync with the interrupt
 enable. Wakeup is still enabled by default, since the module would not
 wake from idle otherwise.
 The enabled_wakeup_gpios was introduced to remember an explicit
 _set_gpio_wakeup beyond a mask/unmask cycle. Calling the flag flag
 disabled_wakeup_gpios would spare the problem of initializing it, but
 feels very unnatural to read.

There's a lot of description here, but it still fails to describe the
problem that is being solved and the motiviation for this change.  

 Wakeup functionality is completely untested, since the AM335x
 lacks a IRQWAKEN register.

So in addtion to a better description of the problem, and the changes,
this needs much broader testing, including on platforms with off-mode.

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


Re: [PATCH v3 2/3] gpio/omap: modify wake-up register with interrupt enable.

2013-05-15 Thread Santosh Shilimkar
On Wednesday 15 May 2013 02:24 AM, Andreas Fenkart wrote:
 OMAP4430 TRM chap. 25.4.5.2
 To reduce dynamic consumption, an efficient idle scheme is based on the
 following:
 • An efficient local autoclock gating for each module
 • The implementation of control sideband signals between the PRCM module
   and each module
 This enhanced idle control allows clocks to be activated and deactivated
 safely without requiring complex software management. The idle mode
 request, idle acknowledge, and wake-up request are sideband signals
 between the PRCM module and the general-purpose interface
 
 OMAP4430 TRM chap. 25.4.6.2
 There must be a correlation between the wake-up enable and interrupt
 enable register. If a GPIO pin has a wake-up configured on it, it must
 also have the corresponding interrupt enabled. Otherwise, it is possible
 there is a wake-up event, but after exiting the IDLE state, no interrupt
 is generated; the corresponding bit from the interrupt status register is
 not cleared, and the module does not acknowledge a future idle request.
 
 Up to now _set_gpio_triggering() is also handling the wake-up enable
 register. According the TRM this should be in sync with the interrupt
 enable. Wakeup is still enabled by default, since the module would not
 wake from idle otherwise.
 The enabled_wakeup_gpios was introduced to remember an explicit
 _set_gpio_wakeup beyond a mask/unmask cycle. Calling the flag flag
 disabled_wakeup_gpios would spare the problem of initializing it, but
 feels very unnatural to read.
 
 Wakeup functionality is completely untested, since the AM335x
 lacks a IRQWAKEN register.
 
 Signed-off-by: Andreas Fenkart andreas.fenk...@streamunlimited.com
 ---
I am ok with the subject patch and other two patches.
Would be good to get a tested by at least for though for OMAP4
and OMAP3. I have no access to boards for few weeks so
can't test it myself.

Other than that, for all three patches,
Acked-by: Santosh Shilimkar santosh.shilim...@ti.com

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


[PATCH v3 2/3] gpio/omap: modify wake-up register with interrupt enable.

2013-05-14 Thread Andreas Fenkart
OMAP4430 TRM chap. 25.4.5.2
To reduce dynamic consumption, an efficient idle scheme is based on the
following:
• An efficient local autoclock gating for each module
• The implementation of control sideband signals between the PRCM module
  and each module
This enhanced idle control allows clocks to be activated and deactivated
safely without requiring complex software management. The idle mode
request, idle acknowledge, and wake-up request are sideband signals
between the PRCM module and the general-purpose interface

OMAP4430 TRM chap. 25.4.6.2
There must be a correlation between the wake-up enable and interrupt
enable register. If a GPIO pin has a wake-up configured on it, it must
also have the corresponding interrupt enabled. Otherwise, it is possible
there is a wake-up event, but after exiting the IDLE state, no interrupt
is generated; the corresponding bit from the interrupt status register is
not cleared, and the module does not acknowledge a future idle request.

Up to now _set_gpio_triggering() is also handling the wake-up enable
register. According the TRM this should be in sync with the interrupt
enable. Wakeup is still enabled by default, since the module would not
wake from idle otherwise.
The enabled_wakeup_gpios was introduced to remember an explicit
_set_gpio_wakeup beyond a mask/unmask cycle. Calling the flag flag
disabled_wakeup_gpios would spare the problem of initializing it, but
feels very unnatural to read.

Wakeup functionality is completely untested, since the AM335x
lacks a IRQWAKEN register.

Signed-off-by: Andreas Fenkart andreas.fenk...@streamunlimited.com
---
 drivers/gpio/gpio-omap.c |   68 ++
 1 file changed, 45 insertions(+), 23 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 082919e..44a93be 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -57,6 +57,7 @@ struct gpio_bank {
struct irq_domain *domain;
u32 non_wakeup_gpios;
u32 enabled_non_wakeup_gpios;
+   u32 enabled_wakeup_gpios;
struct gpio_regs context;
u32 saved_datain;
u32 level_mask;
@@ -287,12 +288,6 @@ 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.wake_en =
-   _gpio_rmw(base, bank-regs-wkup_en, gpio_bit,
- trigger != 0);
-   }
-
/* This part needs to be executed always for OMAP{34xx, 44xx} */
if (!bank-regs-irqctrl) {
/* On omap24xx proceed only when valid GPIO bit is set */
@@ -350,12 +345,13 @@ static int _set_gpio_triggering(struct gpio_bank *bank, 
int gpio,
unsigned trigger)
 {
void __iomem *reg = bank-base;
-   void __iomem *base = bank-base;
u32 l = 0;
 
-   if (bank-regs-leveldetect0  bank-regs-wkup_en) {
+   if (bank-regs-leveldetect0) {
+   /* edge both flanks simultaneously / plus level */
set_gpio_trigger(bank, gpio, trigger);
} else if (bank-regs-irqctrl) {
+   /* edge single flank */
reg += bank-regs-irqctrl;
 
l = __raw_readl(reg);
@@ -370,6 +366,7 @@ static int _set_gpio_triggering(struct gpio_bank *bank, int 
gpio,
 
__raw_writel(l, reg);
} else if (bank-regs-edgectrl1) {
+   /* edge both flanks simultaneously */
if (gpio  0x08)
reg += bank-regs-edgectrl2;
else
@@ -382,11 +379,6 @@ static int _set_gpio_triggering(struct gpio_bank *bank, 
int gpio,
l |= 2  (gpio  1);
if (trigger  IRQ_TYPE_EDGE_FALLING)
l |= 1  (gpio  1);
-
-   /* Enable wake-up during idle for dynamic tick */
-   bank-context.wake_en =
-   _gpio_rmw(base, bank-regs-wkup_en, 1  gpio,
- trigger);
__raw_writel(l, reg);
}
return 0;
@@ -485,10 +477,19 @@ 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)
 {
+   u32 gpio_bit = GPIO_BIT(bank, gpio);
+   void __iomem *base = bank-base;
+
if (enable)
-   _enable_gpio_irqbank(bank, GPIO_BIT(bank, gpio));
+   _enable_gpio_irqbank(bank, gpio_bit);
else
-   _disable_gpio_irqbank(bank, GPIO_BIT(bank, gpio));
+   _disable_gpio_irqbank(bank, gpio_bit);
+
+   if (bank-enabled_wakeup_gpios  gpio_bit) {
+   /* Enable wake-up during idle for dynamic tick */
+   bank-context.wake_en =
+