>-----Original Message-----
>From: Kevin Hilman [mailto:[email protected]]
>Sent: Wednesday, September 22, 2010 7:55 PM
>To: Kalliguddi, Hema
>Cc: [email protected]; Varadarajan, Charulatha;
>Basak, Partha; Tero Kristo
>Subject: Re: [PATCH 2/2] OMAP2+: GPIO: move late PM out of
>interrupts-disabled idle path
>
>"Kalliguddi, Hema" <[email protected]> writes:
>
>>>-----Original Message-----
>>>From: [email protected]
>>>[mailto:[email protected]] On Behalf Of Kevin Hilman
>>>Sent: Tuesday, September 14, 2010 4:33 AM
>>>To: [email protected]
>>>Cc: Varadarajan, Charulatha; Basak, Partha; Tero Kristo
>>>Subject: [PATCH 2/2] OMAP2+: GPIO: move late PM out of
>>>interrupts-disabled idle path
>>>
>>>From: Kevin Hilman <[email protected]>
>>>
>>>Currently, we wait until late in the idle path where interrupts are
>>>disabled to do runtime-PM-like management for certain special-case
>>>devices like GPIO.
>>>
>>>As a prerequiste to moving GPIO to the new runtime PM framework, move
>>>this runtime-PM-like code out of the late idle path into new device
>>>idle and resume functions that can be called before interrupts are
>>>disabled by CPUidle and/or suspend.
>>>
>>>In addition, move all the GPIO-specific logic into the GPIO core
>>>instead of keeping GPIO-specific knowledge of power-states, context
>>>saving etc. in the PM core.
>>>
>>>Also, call the new device-idle and -resume methods from CPUidle and
>>>static suspend path.
>>>
>>>Signed-off-by: Kevin Hilman <[email protected]>
>>>---
>>> arch/arm/mach-omap2/cpuidle34xx.c | 4 ++
>>> arch/arm/mach-omap2/pm.h | 2 +
>>> arch/arm/mach-omap2/pm24xx.c | 2 +-
>>> arch/arm/mach-omap2/pm34xx.c | 38 +++++++++------------
>>> arch/arm/plat-omap/gpio.c | 57
>>>++++++++++++++++++++++++--------
>>> arch/arm/plat-omap/include/plat/gpio.h | 4 +--
>>> 6 files changed, 67 insertions(+), 40 deletions(-)
>>>
>>>diff --git a/arch/arm/mach-omap2/cpuidle34xx.c
>>>b/arch/arm/mach-omap2/cpuidle34xx.c
>>>index 0923b82..681d823 100644
>>>--- a/arch/arm/mach-omap2/cpuidle34xx.c
>>>+++ b/arch/arm/mach-omap2/cpuidle34xx.c
>>>@@ -274,9 +274,13 @@ static int omap3_enter_idle_bm(struct
>>>cpuidle_device *dev,
>>> pwrdm_set_next_pwrst(per_pd, per_next_state);
>>>
>>> select_state:
>>>+ omap3_device_idle();
>>>+
>>> dev->last_state = new_state;
>>> ret = omap3_enter_idle(dev, new_state);
>>>
>>>+ omap3_device_resume();
>>>+
>>> /* Restore original PER state if it was modified */
>>> if (per_next_state != per_saved_state)
>>> pwrdm_set_next_pwrst(per_pd, per_saved_state);
>>>diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
>>>index 3de6ece..33cae4b 100644
>>>--- a/arch/arm/mach-omap2/pm.h
>>>+++ b/arch/arm/mach-omap2/pm.h
>>>@@ -22,6 +22,8 @@ extern void omap_sram_idle(void);
>>> extern int omap3_can_sleep(void);
>>> extern int set_pwrdm_state(struct powerdomain *pwrdm, u32 state);
>>> extern int omap3_idle_init(void);
>>>+extern void omap3_device_idle(void);
>>>+extern void omap3_device_resume(void);
>>>
>>> struct cpuidle_params {
>>> u8 valid;
>>>diff --git a/arch/arm/mach-omap2/pm24xx.c
>>>b/arch/arm/mach-omap2/pm24xx.c
>>>index 6aeedea..7bbf0db 100644
>>>--- a/arch/arm/mach-omap2/pm24xx.c
>>>+++ b/arch/arm/mach-omap2/pm24xx.c
>>>@@ -106,7 +106,7 @@ static void omap2_enter_full_retention(void)
>>> l = omap_ctrl_readl(OMAP2_CONTROL_DEVCONF0) |
>>>OMAP24XX_USBSTANDBYCTRL;
>>> omap_ctrl_writel(l, OMAP2_CONTROL_DEVCONF0);
>>>
>>>- omap2_gpio_prepare_for_idle(PWRDM_POWER_RET);
>>>+ omap2_gpio_prepare_for_idle();
>>>
>>> if (omap2_pm_debug) {
>>> omap2_pm_dump(0, 0, 0);
>>>diff --git a/arch/arm/mach-omap2/pm34xx.c
>>>b/arch/arm/mach-omap2/pm34xx.c
>>>index bb2ba1e..9e590d9 100644
>>>--- a/arch/arm/mach-omap2/pm34xx.c
>>>+++ b/arch/arm/mach-omap2/pm34xx.c
>>>@@ -74,16 +74,6 @@ static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
>>> static struct powerdomain *core_pwrdm, *per_pwrdm;
>>> static struct powerdomain *cam_pwrdm;
>>>
>>>-static inline void omap3_per_save_context(void)
>>>-{
>>>- omap_gpio_save_context();
>>>-}
>>>-
>>>-static inline void omap3_per_restore_context(void)
>>>-{
>>>- omap_gpio_restore_context();
>>>-}
>>>-
>>> static void omap3_enable_io_chain(void)
>>> {
>>> int timeout = 0;
>>>@@ -332,6 +322,16 @@ static void restore_table_entry(void)
>>> restore_control_register(control_reg_value);
>>> }
>>>
>>>+void omap3_device_idle(void)
>>>+{
>>>+ omap2_gpio_prepare_for_idle();
>>>+}
>>>+
>>
>> Is not it is a good idea to pass the new_state as the
>parameter for the driver calls?
>> It will reduce each individual drivers reading the PRCM
>register to findout the next state.
>> This might save some time in the idle loop.
>
>I chose not to pass the parameters on purpose. All of the intelligence
>for device idle (including which powerdomain state to check)
>should live
>in the driver code.
>
OK agree.
>If reading the PRCM registers is becoming a problem, it will
>be a simple
>patch to patch the powerdomain core to cache the current value of it's
>next state so at least reads of next_state will be fast.
>
This is good idea, with this we can avoid n number of PRCM register reads.
Today it may not be a problem as there are few hooks in the idle path
But if there are more number of driver calls, then it might be useful patch.
~Hema
>Kevin
>
>>>+void omap3_device_resume(void)
>>>+{
>>>+ omap2_gpio_resume_after_idle();
>>>+}
>>>+
>>
>> Here we will need to pass the next_state as well as
>previous_state as parameters. If we agree to
>> pass the parameters.
>>
>> ~Hema
>>
>>> void omap_sram_idle(void)
>>> {
>>> /* Variable to tell what needs to be saved and restored
>>>@@ -344,7 +344,7 @@ void omap_sram_idle(void)
>>> int mpu_next_state = PWRDM_POWER_ON;
>>> int per_next_state = PWRDM_POWER_ON;
>>> int core_next_state = PWRDM_POWER_ON;
>>>- int core_prev_state, per_prev_state;
>>>+ int core_prev_state;
>>> u32 sdrc_pwr = 0;
>>>
>>> if (!_omap_sram_idle)
>>>@@ -387,12 +387,9 @@ void omap_sram_idle(void)
>>> }
>>>
>>> /* PER */
>>>- if (per_next_state < PWRDM_POWER_ON) {
>>>+ if (per_next_state < PWRDM_POWER_ON)
>>> omap_uart_prepare_idle(2);
>>>- omap2_gpio_prepare_for_idle(per_next_state);
>>>- if (per_next_state == PWRDM_POWER_OFF)
>>>- omap3_per_save_context();
>>>- }
>>>+
>>>
>>> /* CORE */
>>> if (core_next_state < PWRDM_POWER_ON) {
>>>@@ -454,13 +451,8 @@ void omap_sram_idle(void)
>>> omap3_intc_resume_idle();
>>>
>>> /* PER */
>>>- if (per_next_state < PWRDM_POWER_ON) {
>>>- per_prev_state = pwrdm_read_prev_pwrst(per_pwrdm);
>>>- omap2_gpio_resume_after_idle();
>>>- if (per_prev_state == PWRDM_POWER_OFF)
>>>- omap3_per_restore_context();
>>>+ if (per_next_state < PWRDM_POWER_ON)
>>> omap_uart_resume_idle(2);
>>>- }
>>>
>>> /* Disable IO-PAD and IO-CHAIN wakeup */
>>> if (omap3_has_io_wakeup() &&
>>>@@ -570,6 +562,7 @@ static void omap2_pm_wakeup_on_timer(u32
>>>seconds, u32 milliseconds)
>>> static int omap3_pm_prepare(void)
>>> {
>>> disable_hlt();
>>>+ omap3_device_idle();
>>> return 0;
>>> }
>>>
>>>@@ -637,6 +630,7 @@ static int omap3_pm_enter(suspend_state_t unused)
>>>
>>> static void omap3_pm_finish(void)
>>> {
>>>+ omap3_device_resume();
>>> enable_hlt();
>>> }
>>>
>>>diff --git a/arch/arm/plat-omap/gpio.c b/arch/arm/plat-omap/gpio.c
>>>index 7951eef..b0467c1 100644
>>>--- a/arch/arm/plat-omap/gpio.c
>>>+++ b/arch/arm/plat-omap/gpio.c
>>>@@ -29,6 +29,8 @@
>>> #include <asm/mach/irq.h>
>>> #include <plat/powerdomain.h>
>>>
>>>+static struct powerdomain *per_pwrdm;
>>>+
>>> /*
>>> * OMAP1510 GPIO registers
>>> */
>>>@@ -207,6 +209,9 @@ struct gpio_bank {
>>> u32 dbck_enable_mask;
>>> };
>>>
>>>+static void omap3_gpio_restore_context(void);
>>>+static void omap3_gpio_save_context(void);
>>>+
>>> #define METHOD_MPUIO 0
>>> #define METHOD_GPIO_1510 1
>>> #define METHOD_GPIO_1610 2
>>>@@ -1778,6 +1783,8 @@ static int __init _omap_gpio_init(void)
>>> }
>>> #endif
>>>
>>>+ if (cpu_class_is_omap2())
>>>+ per_pwrdm = pwrdm_lookup("per_pwrdm");
>>>
>>> #ifdef CONFIG_ARCH_OMAP15XX
>>> if (cpu_is_omap15xx()) {
>>>@@ -2074,14 +2081,22 @@ static struct sys_device omap_gpio_device = {
>>>
>>> static int workaround_enabled;
>>>
>>>-void omap2_gpio_prepare_for_idle(int power_state)
>>>+void omap2_gpio_prepare_for_idle(void)
>>> {
>>>- int i, c = 0;
>>>- int min = 0;
>>>+ int i, c = 0, min = 0;
>>>+ int per_next_state;
>>>+
>>>+ if (!per_pwrdm)
>>>+ return;
>>>+
>>>+ per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
>>>+ if (per_next_state >= PWRDM_POWER_INACTIVE)
>>>+ return;
>>>
>>> if (cpu_is_omap34xx())
>>> min = 1;
>>>
>>>+ workaround_enabled = 0;
>>> for (i = min; i < gpio_bank_count; i++) {
>>> struct gpio_bank *bank = &gpio_bank[i];
>>> u32 l1, l2;
>>>@@ -2089,7 +2104,7 @@ void omap2_gpio_prepare_for_idle(int
>power_state)
>>> if (bank->dbck_enable_mask)
>>> clk_disable(bank->dbck);
>>>
>>>- if (power_state > PWRDM_POWER_OFF)
>>>+ if (per_next_state > PWRDM_POWER_OFF)
>>> continue;
>>>
>>> /* If going to OFF, remove triggering for all
>>>@@ -2135,20 +2150,35 @@ void omap2_gpio_prepare_for_idle(int
>>>power_state)
>>>
>>> c++;
>>> }
>>>- if (!c) {
>>>- workaround_enabled = 0;
>>>- return;
>>>- }
>>>- workaround_enabled = 1;
>>>+ if (c)
>>>+ workaround_enabled = 1;
>>>+
>>>+ if (cpu_is_omap34xx() && (per_next_state == PWRDM_POWER_OFF))
>>>+ omap3_gpio_save_context();
>>> }
>>>
>>> void omap2_gpio_resume_after_idle(void)
>>> {
>>>- int i;
>>>- int min = 0;
>>>+ int i, min = 0;
>>>+ int per_next_state;
>>>+
>>>+ if (!per_pwrdm)
>>>+ return;
>>>+
>>>+ per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
>>>+ if (per_next_state >= PWRDM_POWER_INACTIVE)
>>>+ return;
>>>+
>>>+ if (cpu_is_omap34xx() && (per_next_state == PWRDM_POWER_OFF)) {
>>>+ int per_prev_state = pwrdm_read_prev_pwrst(per_pwrdm);
>>>+
>>>+ if (per_prev_state == PWRDM_POWER_OFF)
>>>+ omap3_gpio_restore_context();
>>>+ }
>>>
>>> if (cpu_is_omap34xx())
>>> min = 1;
>>>+
>>> for (i = min; i < gpio_bank_count; i++) {
>>> struct gpio_bank *bank = &gpio_bank[i];
>>> u32 l, gen, gen0, gen1;
>>>@@ -2235,14 +2265,13 @@ void omap2_gpio_resume_after_idle(void)
>>> }
>>> }
>>> }
>>>-
>>> }
>>>
>>> #endif
>>>
>>> #ifdef CONFIG_ARCH_OMAP3
>>> /* save the registers of bank 2-6 */
>>>-void omap_gpio_save_context(void)
>>>+static void omap3_gpio_save_context(void)
>>> {
>>> int i;
>>>
>>>@@ -2275,7 +2304,7 @@ void omap_gpio_save_context(void)
>>> }
>>>
>>> /* restore the required registers of bank 2-6 */
>>>-void omap_gpio_restore_context(void)
>>>+static void omap3_gpio_restore_context(void)
>>> {
>>> int i;
>>>
>>>diff --git a/arch/arm/plat-omap/include/plat/gpio.h
>>>b/arch/arm/plat-omap/include/plat/gpio.h
>>>index de1c604..c81ef94 100644
>>>--- a/arch/arm/plat-omap/include/plat/gpio.h
>>>+++ b/arch/arm/plat-omap/include/plat/gpio.h
>>>@@ -72,12 +72,10 @@
>>> IH_GPIO_BASE + (nr))
>>>
>>> extern int omap_gpio_init(void); /* Call from board init only */
>>>-extern void omap2_gpio_prepare_for_idle(int power_state);
>>>+extern void omap2_gpio_prepare_for_idle(void);
>>> extern void omap2_gpio_resume_after_idle(void);
>>> extern void omap_set_gpio_debounce(int gpio, int enable);
>>> extern void omap_set_gpio_debounce_time(int gpio, int enable);
>>>-extern void omap_gpio_save_context(void);
>>>-extern void omap_gpio_restore_context(void);
>>>
>>>/*-------------------------------------------------------------
>>>------------*/
>>>
>>> /* Wrappers for "new style" GPIO calls, using the new infrastructure
>>>--
>>>1.7.2.1
>>>
>>>--
>>>To unsubscribe from this list: send the line "unsubscribe
>>>linux-omap" in
>>>the body of a message to [email protected]
>>>More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
