<Tero.Kristo@xxxxxxxxx> writes: > > >>-----Original Message----- >>From: ext Kevin Hilman [mailto:khilman@xxxxxxxxxxxxxxxxxxx] >>Sent: 20 October, 2009 20:48 >>To: Kristo Tero (Nokia-D/Tampere) >>Cc: linux-omap@xxxxxxxxxxxxxxx >>Subject: Re: [PATCH 16/17] OMAP3: PM: Write voltage and clock >>setup times dynamically in idle loop >> >>Tero Kristo <tero.kristo@xxxxxxxxx> writes: >> >>> From: Tero Kristo <tero.kristo@xxxxxxxxx> >>> >>> 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.hogander@xxxxxxxxx> >>> --- >>> 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. Kevin >> >>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 majordomo@xxxxxxxxxxxxxxx >>> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html