>-----Original Message-----
>From: ext Kevin Hilman [mailto:khil...@deeprootsystems.com] 
>Sent: 20 October, 2009 20:48
>To: Kristo Tero (Nokia-D/Tampere)
>Cc: linux-omap@vger.kernel.org
>Subject: Re: [PATCH 16/17] OMAP3: PM: Write voltage and clock 
>setup times dynamically in idle loop
>
>Tero Kristo <tero.kri...@nokia.com> writes:
>
>> From: Tero Kristo <tero.kri...@nokia.com>
>>
>> It is suggested by TI (SWPA152 February 2009) to write clksetup,
>> voltsetup_time1, voltsetup_time2, voltsetup2 dynamically in 
>idle loop.
>
>Can you summarize the reasons why?

Basically this optimizes the clksetup / voltsetup times according to the sleep 
mode. Currently the settings are too high in both retention and off-mode, 
because the hardware appears to use for example VOLTSETUP2 even if we are not 
in off-mode, adding extra delay to wakeup. Also, CLKSETUP is too high for 
retention mode because oscillator is not actually shut-off here.

However, now that I think about it, it might be better to change this in a way 
that it is user configurable whether we want to change the settings or not, 
maybe add new items in to the prm_setup struct for alternate settings for ret / 
off and only use these if available. Some boards might actually switch 
oscillator off in retention mode which would require higher setup time.

>
>> Signed-off-by: Jouni Hogander <jouni.hogan...@nokia.com>
>> ---
>>  arch/arm/mach-omap2/pm34xx.c |   36 
>+++++++++++++++++++++++++-----------
>>  1 files changed, 25 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/pm34xx.c 
>b/arch/arm/mach-omap2/pm34xx.c
>> index f492142..ae83121 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -575,6 +575,30 @@ void omap_sram_idle(void)
>>      if (regset_save_on_suspend)
>>              pm_dbg_regset_save(1);
>>  
>> +    /* Write voltage setup times which are changed dynamically */
>
>AFAICT, we only set these values at init time and they are never
>changed.  Are there some forthcoming patches that change these
>dynamically?

Following bit of the code actually changes them dynamically, you can see that 
e.g. CLKSETUP time is either prm_setup.clksetup or 0x1 depending on the sleep 
mode. But as previously said, I think these should probably be added as new 
items to the prm_setup struct.

>
>Kevin
>
>> +    if (core_next_state == PWRDM_POWER_OFF) {
>> +            prm_write_mod_reg(0, OMAP3430_GR_MOD,
>> +                            OMAP3_PRM_VOLTSETUP1_OFFSET);
>> +            prm_write_mod_reg(prm_setup.voltsetup2, OMAP3430_GR_MOD,
>> +                            OMAP3_PRM_VOLTSETUP2_OFFSET);
>> +            prm_write_mod_reg(prm_setup.clksetup, OMAP3430_GR_MOD,
>> +                            OMAP3_PRM_CLKSETUP_OFFSET);
>> +    } else {
>> +            prm_write_mod_reg((prm_setup.voltsetup_time2 <<
>> +                            OMAP3430_SETUP_TIME2_SHIFT) |
>> +                            (prm_setup.voltsetup_time1 <<
>> +                            OMAP3430_SETUP_TIME1_SHIFT),
>> +                            OMAP3430_GR_MOD, 
>OMAP3_PRM_VOLTSETUP1_OFFSET);
>> +            prm_write_mod_reg(0, OMAP3430_GR_MOD,
>> +                            OMAP3_PRM_VOLTSETUP2_OFFSET);
>> +            /*
>> +             * Use static 1 as only HF_CLKOUT is turned off.
>> +             * Value taken from application note SWPA152
>> +             */
>> +            prm_write_mod_reg(0x1, OMAP3430_GR_MOD,
>> +                            OMAP3_PRM_CLKSETUP_OFFSET);
>> +    }
>> +
>>      /*
>>       * omap3_arm_context is the location where ARM registers
>>       * get saved. The restore path then reads from this
>> @@ -1379,19 +1403,9 @@ static void __init configure_vc(void)
>>                        OMAP3430_GR_MOD,
>>                        OMAP3_PRM_VC_I2C_CFG_OFFSET);
>>  
>> -    /* Write setup times */
>> -    prm_write_mod_reg(prm_setup.clksetup, OMAP3430_GR_MOD,
>> -                    OMAP3_PRM_CLKSETUP_OFFSET);
>> -    prm_write_mod_reg((prm_setup.voltsetup_time2 <<
>> -                    OMAP3430_SETUP_TIME2_SHIFT) |
>> -                    (prm_setup.voltsetup_time1 <<
>> -                    OMAP3430_SETUP_TIME1_SHIFT),
>> -                    OMAP3430_GR_MOD, OMAP3_PRM_VOLTSETUP1_OFFSET);
>> -
>> +    /* Write static setup times */
>>      prm_write_mod_reg(prm_setup.voltoffset, OMAP3430_GR_MOD,
>>                      OMAP3_PRM_VOLTOFFSET_OFFSET);
>> -    prm_write_mod_reg(prm_setup.voltsetup2, OMAP3430_GR_MOD,
>> -                    OMAP3_PRM_VOLTSETUP2_OFFSET);
>>  
>>      pm_dbg_regset_init(1);
>>      pm_dbg_regset_init(2);
>> -- 
>> 1.5.4.3
>>
>> --
>> 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
>--
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

Reply via email to