>-----Original Message-----
>From: ext Kevin Hilman [mailto:khil...@deeprootsystems.com] 
>Sent: 21 October, 2009 17:15
>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.kri...@nokia.com> writes:
>
>>  
>>
>>>-----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. 
>
>Doh, I see now.
>
>> But as previously said, I think these should probably be added as
>> new items to the prm_setup struct.
>
>Yes, I would rather see the prm_setup struct extended so these can be
>passed in by board code.

I'll change the patch accordingly.


>>>
>>>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