Fix module-mode enable sequence on OMAP4
Subsystems (6)
cover Fix module-mode enable sequence on OMAP4
Hi,
On OMAP4, the PRCM recommended sequence for enabling a module after power-on-reset is
-1- Force clkdm to SW_WKUP
-2- Configure desired module mode to "enable" or "auto" -3- Wait for the desired module idle status to be FUNC -4- Program clkdm in HW_AUTO(if supported)
This sequence applies to all older OMAPs' as well, however since they use 'autodeps', it makes sure that no clkdm is in IDLE, and hence not requiring a force SW_WKUP when a module is being enabled.
OMAP4 does not need to support autodeps, because of the dynamic dependency feature, wherein the HW takes care of waking up a clockdomain from idle and hence the module, whenever an interconnect access happens to the given module.
Implementing the sequence for OMAP4 requires some of the clockdomain handling that is currently done in clock framework to be done as part of hwmod framework since the step -3- above to "Wait for the desired module idle status to be FUNC" is done as part of hwmod framework.
This series moves the clockdomain handling into hwmod framework and implements the above sequence for OMAP4.
The above sequence is expected to be followed even for optional clocks (steps -1-, -2- and -4- only) and that makes handling it difficult as optional clocks are enabled by drivers using direct clock framework calls and hence do not go through the hwmod framework.
Hence this series also adds the handling of the sequence *only for optional clocks* in clock framework.
Lastly, with this series drivers which are yet to be adapted to PM runtime and still rely on clock calls to enable/disable the respective *main* clocks are expected to be broken. MMC is one such which even breaks boot, and hence the series has a TEMP workaround patch added which keeps l3init clockdomain always force-enabled. This TEMP patch will gate all CORE low power transitions but should atleast allow MPU low power transitions to work.
The series is tested for low power state transitions (RET/OFF) on OMAP3sdp and boot tested on OMAP4sdp. Applies on 3.0-rc2
Rajendra Nayak (8):
OMAP2+: clockdomain: Add an api to read idle mode OMAP2+: clockdomain: Add SoC support for clkdm_is_idle OMAP2+: PM: Initialise sleep_switch to a non-valid value OMAP2+: PM: idle clkdms only if already in idle
OMAP4: PM: TEMP: Prevent l3init from idling/force sleep OMAP2+: hwmod: Follow the recomended PRCM clock enable sequence
OMAP: clock: Add flags to identify optional clock nodes
OMAP: clock: Enable clockdomain only for optional clocks
arch/arm/mach-omap2/clock.c | 30 ++++++++++++------------ arch/arm/mach-omap2/clock44xx_data.c | 34 +++++++++++++++++++++++++++ arch/arm/mach-omap2/clockdomain.c | 28 +++++++++++++++++++++- arch/arm/mach-omap2/clockdomain.h | 3 ++ arch/arm/mach-omap2/clockdomain2xxx_3xxx.c | 12 +++++++++ arch/arm/mach-omap2/clockdomain44xx.c | 16 ++++++------ arch/arm/mach-omap2/clockdomains44xx_data.c | 2 +- arch/arm/mach-omap2/omap_hwmod.c | 20 +++++++++++++++- arch/arm/mach-omap2/pm.c | 6 +++- arch/arm/plat-omap/include/plat/clock.h | 1 + 10 files changed, 124 insertions(+), 28 deletions(-)
PATCH 1/8 OMAP2+: clockdomain: Add an api to read idle mode
Add a clockdomain api to check if hardware supervised idle transitions are enabled on a clockdomain.
--- arch/arm/mach-omap2/clockdomain.c | 21 +++++++++++++++++++++ arch/arm/mach-omap2/clockdomain.h | 3 +++ 2 files changed, 24 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/clockdomain.c +21 -0
@@ -795,6 +795,27 @@ void clkdm_deny_idle(struct clockdomain *clkdm) arch_clkdm->clkdm_deny_idle (clkdm); } +/** +* clkdm_is_idle - Check if the clkdm hwsup/autoidle is enabled +* @clkdm: struct clockdomain * +* +* Returns true if the clockdomain is in hardware-supervised +* idle mode, or 0 otherwise. +* +*/ +int clkdm_is_idle (struct clockdomain *clkdm) +{ +if (!clkdm) +return -EINVAL ; + +if (!arch_clkdm || !arch_clkdm->clkdm_is_idle) +return -EINVAL ; + +pr_debug ("clockdomain: reading idle state for %s\n" , clkdm->name); + +return arch_clkdm->clkdm_is_idle (clkdm); +} +/* Clockdomain-to-clock framework interface code */
arch/arm/mach-omap2/clockdomain.h +3 -0
@@ -138,6 +138,7 @@ struct clockdomain {* @clkdm_wakeup: Force a clockdomain to wakeup * @clkdm_allow_idle: Enable hw supervised idle transitions for clock domain * @clkdm_deny_idle: Disable hw supervised idle transitions for clock domain +* @clkdm_is_idle: Check if hw supervised idle transitions are enabled * @clkdm_clk_enable: Put the clkdm in right state for a clock enable * @clkdm_clk_disable: Put the clkdm in right state for a clock disable */
@@ -154,6 +155,7 @@ struct clkdm_ops {int (*clkdm_wakeup)(struct clockdomain *clkdm);void (*clkdm_allow_idle)(struct clockdomain *clkdm);void (*clkdm_deny_idle)(struct clockdomain *clkdm); +int (*clkdm_is_idle)(struct clockdomain *clkdm);int (*clkdm_clk_enable)(struct clockdomain *clkdm);int (*clkdm_clk_disable)(struct clockdomain *clkdm); };
@@ -177,6 +179,7 @@ int clkdm_clear_all_sleepdeps(struct clockdomain *clkdm);void clkdm_allow_idle (struct clockdomain *clkdm);void clkdm_deny_idle (struct clockdomain *clkdm); +int clkdm_is_idle (struct clockdomain *clkdm);int clkdm_wakeup (struct clockdomain *clkdm);int clkdm_sleep (struct clockdomain *clkdm);
Signature · 1 line
1.7.0.4
PATCH 2/8 OMAP2+: clockdomain: Add SoC support for clkdm_is_idle
Add the SoC specific implemenation for clkdm_is_idle for OMAP2/3 and OMAP4.
--- arch/arm/mach-omap2/clockdomain2xxx_3xxx.c | 12 ++++++++++++ arch/arm/mach-omap2/clockdomain44xx.c | 7 +++++++ 2 files changed, 19 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/clockdomain2xxx_3xxx.c +12 -0
@@ -13,6 +13,7 @@*/ #include <linux/types.h> +#include <linux/errno.h> #include <plat/prcm.h> #include "prm.h" #include "prm2xxx_3xxx.h"
@@ -146,6 +147,15 @@ static void omap2_clkdm_deny_idle(struct clockdomain *clkdm)_clkdm_del_autodeps (clkdm); } +static int omap2_clkdm_is_idle (struct clockdomain *clkdm) +{ +if (!clkdm->clktrctrl_mask) +return -EINVAL ; + +return omap2_cm_is_clkdm_in_hwsup (clkdm->pwrdm.ptr->prcm_offs, + clkdm->clktrctrl_mask); +} +static void _enable_hwsup (struct clockdomain *clkdm) {if (cpu_is_omap24xx ())
@@ -252,6 +262,7 @@ struct clkdm_ops omap2_clkdm_operations = { .clkdm_wakeup = omap2_clkdm_wakeup, .clkdm_allow_idle = omap2_clkdm_allow_idle, .clkdm_deny_idle = omap2_clkdm_deny_idle, + .clkdm_is_idle = omap2_clkdm_is_idle, .clkdm_clk_enable = omap2_clkdm_clk_enable, .clkdm_clk_disable = omap2_clkdm_clk_disable, };
@@ -269,6 +280,7 @@ struct clkdm_ops omap3_clkdm_operations = { .clkdm_wakeup = omap3_clkdm_wakeup, .clkdm_allow_idle = omap3_clkdm_allow_idle, .clkdm_deny_idle = omap3_clkdm_deny_idle, + .clkdm_is_idle = omap2_clkdm_is_idle, .clkdm_clk_enable = omap2_clkdm_clk_enable, .clkdm_clk_disable = omap2_clkdm_clk_disable, };
arch/arm/mach-omap2/clockdomain44xx.c +7 -0
@@ -93,6 +93,12 @@ static void omap4_clkdm_deny_idle(struct clockdomain *clkdm) clkdm->cm_inst, clkdm->clkdm_offs); } +static int omap4_clkdm_is_idle (struct clockdomain *clkdm) +{ +return omap4_cminst_is_clkdm_in_hwsup (clkdm->prcm_partition, + clkdm->cm_inst, clkdm->clkdm_offs); +} +static int omap4_clkdm_clk_enable (struct clockdomain *clkdm) {bool hwsup = false;
@@ -132,6 +138,7 @@ struct clkdm_ops omap4_clkdm_operations = { .clkdm_wakeup = omap4_clkdm_wakeup, .clkdm_allow_idle = omap4_clkdm_allow_idle, .clkdm_deny_idle = omap4_clkdm_deny_idle, + .clkdm_is_idle = omap4_clkdm_is_idle, .clkdm_clk_enable = omap4_clkdm_clk_enable, .clkdm_clk_disable = omap4_clkdm_clk_disable, };
Signature · 1 line
1.7.0.4
PATCH 3/8 OMAP2+: PM: Initialise sleep_switch to a non-valid value
sleep_switch which is initialised to 0 in omap_set_pwrdm_state happens to be a valid sleep_switch type (FORCEWAKEUP_SWITCH) which are defined as
#define FORCEWAKEUP_SWITCH 0
#define LOWPOWERSTATE_SWITCH 1
This causes the function to wrongly program some clock domains even when the Powerdomain is in ON state.
--- arch/arm/mach-omap2/pm.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/pm.c +1 -1
@@ -106,7 +106,7 @@ static void omap2_init_processor_devices(void)int omap_set_pwrdm_state (struct powerdomain *pwrdm,u32 state) {u32 cur_state; -int sleep_switch =0 ; +int sleep_switch =-1 ;int ret =0 ;if (pwrdm ==NULL ||IS_ERR (pwrdm))
Signature · 1 line
1.7.0.4
PATCH 4/8 OMAP2+: PM: idle clkdms only if already in idle
The omap_set_pwrdm_state function forces clockdomains to idle, without checking the existing idle state programmed, instead based solely on the HW capability of the clockdomain to support idle.
This is wrong and the clockdomains should be idled post a state_switch *only* if idle transitions on the clockdomain were already enabled.
--- arch/arm/mach-omap2/pm.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/pm.c +3 -1
@@ -108,6 +108,7 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)u32 cur_state;int sleep_switch =-1 ;int ret =0 ; +int hwsup =0 ;if (pwrdm ==NULL ||IS_ERR (pwrdm))return -EINVAL ;
@@ -127,6 +128,7 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state) (pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE)) { sleep_switch = LOWPOWERSTATE_SWITCH; } else { + hwsup =clkdm_is_idle (pwrdm->pwrdm_clkdms[0 ]);clkdm_wakeup (pwrdm->pwrdm_clkdms[0 ]);pwrdm_wait_transition (pwrdm); sleep_switch =FORCEWAKEUP_SWITCH ;
@@ -142,7 +144,7 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)switch (sleep_switch) {case FORCEWAKEUP_SWITCH : -if (pwrdm->pwrdm_clkdms[0 ]->flags &CLKDM_CAN_ENABLE_AUTO ) +if (hwsup)clkdm_allow_idle (pwrdm->pwrdm_clkdms[0 ]);else clkdm_sleep (pwrdm->pwrdm_clkdms[0 ]);
Signature · 1 line
1.7.0.4
PATCH 5/8 OMAP4: PM: TEMP: Prevent l3init from idling/force sleep
Since MMC driver is yet to be adapted to runtime PM and still uses direct clock calls to enable/disable module, its needed that the clockdomain (for MMC) is always kept force enabled since the next few patches move
the clockdomain handling from clock framework to hwmod framework and break MMC.
This will certainlly gate any CORE low power transitions.
--- arch/arm/mach-omap2/clockdomains44xx_data.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/clockdomains44xx_data.c +1 -1
@@ -493,7 +493,7 @@ static struct clockdomain l3_init_44xx_clkdm = { .dep_bit = OMAP4430_L3INIT_STATDEP_SHIFT, .wkdep_srcs = l3_init_wkup_sleep_deps, .sleepdep_srcs = l3_init_wkup_sleep_deps, - .flags = CLKDM_CAN_HWSUP_SWSUP, + .flags = CLKDM_CAN_FORCE_WAKEUP, .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP4430), };
Signature · 1 line
1.7.0.4
PATCH 6/8 OMAP2+: hwmod: Follow the recomended PRCM clock enable sequence
On OMAP4, the PRCM recommended sequence for enabling a module after power-on-reset is
-1- Force clkdm to SW_WKUP
-2- Configure desired module mode to "enable" or "auto" -3- Wait for the desired module idle status to be FUNC -4- Program clkdm in HW_AUTO(if supported)
This sequence applies to all older OMAPs' as well, however since they use autodeps, it makes sure that no clkdm is in IDLE, and hence not requiring a force SW_WKUP when a module is being enabled.
OMAP4 does not need to support autodeps, because of the dyanamic dependency feature, wherein the HW takes care of waking up a clockdomain from idle and hence the module, whenever an interconnect access happens to the given module.
Implementing the sequence for OMAP4 requires the clockdomain handling that is currently done in clock framework to be done as part of hwmod framework since the step -3- above to "Wait for the desired module idle status to be FUNC" is done as part of hwmod framework.
--- arch/arm/mach-omap2/clock.c | 17 +---------------- arch/arm/mach-omap2/clockdomain.c | 7 ++++++- arch/arm/mach-omap2/clockdomain44xx.c | 9 +-------- arch/arm/mach-omap2/omap_hwmod.c | 20 +++++++++++++++++++- 4 files changed, 27 insertions(+), 26 deletions(-)
arch/arm/mach-omap2/clock.c +1 -16
@@ -268,9 +268,6 @@ void omap2_clk_disable(struct clk *clk) clk->ops->disable (clk); } -if (clk->clkdm) -clkdm_clk_disable (clk->clkdm, clk); -if (clk->parent)omap2_clk_disable (clk->parent); }
@@ -308,30 +305,18 @@ int omap2_clk_enable(struct clk *clk) } } -if (clk->clkdm) { - ret =clkdm_clk_enable (clk->clkdm, clk); -if (ret) { -WARN (1 ,"clock: %s: could not enable clockdomain %s: " -"%d\n" , clk->name, clk->clkdm->name, ret); - goto oce_err2; - } - } -if (clk->ops && clk->ops->enable) {trace_clock_enable (clk->name,1 ,smp_processor_id ()); ret = clk->ops->enable (clk);if (ret) {WARN (1 ,"clock: %s: could not enable: %d\n" , clk->name, ret); - goto oce_err3; + goto oce_err2; } }return 0 ; -oce_err3: -if (clk->clkdm) -clkdm_clk_disable (clk->clkdm, clk); oce_err2:if (clk->parent)omap2_clk_disable (clk->parent);
arch/arm/mach-omap2/clockdomain.c +6 -1
@@ -846,7 +846,12 @@ int clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk)if (!arch_clkdm || !arch_clkdm->clkdm_clk_enable)return -EINVAL ; -if (atomic_inc_return (&clkdm->usecount) >1 ) +/* +* For arch's with no autodeps, clkcm_clk_enable +* should be called for every clock instance that is +* enabled, so the clkdm can be force woken up. +*/ +if ((atomic_inc_return (&clkdm->usecount) >1 ) && autodeps)return 0 ;/* Clockdomain now has one enabled downstream clock */
arch/arm/mach-omap2/clockdomain44xx.c +1 -8
@@ -101,14 +101,7 @@ static int omap4_clkdm_is_idle(struct clockdomain *clkdm)static int omap4_clkdm_clk_enable (struct clockdomain *clkdm) { -bool hwsup = false; - - hwsup =omap4_cminst_is_clkdm_in_hwsup (clkdm->prcm_partition, - clkdm->cm_inst, clkdm->clkdm_offs); - -if (!hwsup) -clkdm_wakeup (clkdm); - +clkdm_wakeup (clkdm);return 0 ; }
arch/arm/mach-omap2/omap_hwmod.c +19 -1
@@ -1223,6 +1223,7 @@ static int _reset(struct omap_hwmod *oh)static int _enable (struct omap_hwmod *oh) {int r; +int hwsup =0 ; if (oh->_state != _HWMOD_STATE_INITIALIZED && oh->_state != _HWMOD_STATE_IDLE &&
@@ -1250,10 +1251,21 @@ static int _enable(struct omap_hwmod *oh)omap_hwmod_mux (oh->mux, _HWMOD_STATE_ENABLED);_add_initiator_dep (oh, mpu_oh); +if (oh->_clk && oh->_clk->clkdm) { + hwsup =clkdm_is_idle (oh->_clk->clkdm); + r =clkdm_clk_enable (oh->_clk->clkdm, oh->_clk); +if (r) { +WARN (1 ,"clock: %s: could not enable clockdomain %s: " +"%d\n" , oh->_clk->name, oh->_clk->clkdm->name, r); +return r; + } + }_enable_clocks (oh); - r =_wait_target_ready (oh);if (!r) { +if (oh->_clk && oh->_clk->clkdm && hwsup) +clkdm_allow_idle (oh->_clk->clkdm); + oh->_state = _HWMOD_STATE_ENABLED;/* Access the sysconfig only if the target is ready */
@@ -1264,6 +1276,8 @@ static int _enable(struct omap_hwmod *oh) } }else {_disable_clocks (oh); +if (oh->_clk && oh->_clk->clkdm) +clkdm_clk_disable (oh->_clk->clkdm, oh->_clk);pr_debug ("omap_hwmod: %s: _wait_target_ready: %d\n" , oh->name, r); }
@@ -1293,6 +1307,8 @@ static int _idle(struct omap_hwmod *oh)_idle_sysc (oh);_del_initiator_dep (oh, mpu_oh);_disable_clocks (oh); +if (oh->_clk && oh->_clk->clkdm) +clkdm_clk_disable (oh->_clk->clkdm, oh->_clk);/* Mux pins for device idle if populated */ if (oh->mux && oh->mux->pads_dynamic)
@@ -1389,6 +1405,8 @@ static int _shutdown(struct omap_hwmod *oh)_del_initiator_dep (oh, mpu_oh);/* XXX what about the other system initiators here? dma, dsp */ _disable_clocks (oh); +if (oh->_clk && oh->_clk->clkdm) +clkdm_clk_disable (oh->_clk->clkdm, oh->_clk); }/* XXX Should this code also force-disable the optional clocks? */
Signature · 1 line
1.7.0.4
PATCH 7/8 OMAP: clock: Add flags to identify optional clock nodes
There is a need to identify optional clock nodes in the clock framework, so some specific sequence to enable them can be supported, which should be evident in the subsequent patches.
--- arch/arm/mach-omap2/clock44xx_data.c | 34 +++++++++++++++++++++++++++++++ arch/arm/plat-omap/include/plat/clock.h | 1 + 2 files changed, 35 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/clock44xx_data.c +34 -0
@@ -1394,6 +1394,7 @@ static struct clk bandgap_fclk = { .clkdm_name = "l4_wkup_clkdm", .parent = &sys_32k_ck, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk des3des_fck = {
@@ -1464,6 +1465,7 @@ static struct clk dss_sys_clk = { .clkdm_name = "l3_dss_clkdm", .parent = &syc_clk_div_ck, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk dss_tv_clk = {
@@ -1474,6 +1476,7 @@ static struct clk dss_tv_clk = { .clkdm_name = "l3_dss_clkdm", .parent = &extalt_clkin_ck, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk dss_dss_clk = {
@@ -1484,6 +1487,7 @@ static struct clk dss_dss_clk = { .clkdm_name = "l3_dss_clkdm", .parent = &dpll_per_m5x2_ck, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk dss_48mhz_clk = {
@@ -1494,6 +1498,7 @@ static struct clk dss_48mhz_clk = { .clkdm_name = "l3_dss_clkdm", .parent = &func_48mc_fclk, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk dss_fck = {
@@ -1577,6 +1582,7 @@ static struct clk gpio1_dbclk = { .clkdm_name = "l4_wkup_clkdm", .parent = &sys_32k_ck, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk gpio1_ick = {
@@ -1597,6 +1603,7 @@ static struct clk gpio2_dbclk = { .clkdm_name = "l4_per_clkdm", .parent = &sys_32k_ck, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk gpio2_ick = {
@@ -1617,6 +1624,7 @@ static struct clk gpio3_dbclk = { .clkdm_name = "l4_per_clkdm", .parent = &sys_32k_ck, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk gpio3_ick = {
@@ -1637,6 +1645,7 @@ static struct clk gpio4_dbclk = { .clkdm_name = "l4_per_clkdm", .parent = &sys_32k_ck, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk gpio4_ick = {
@@ -1657,6 +1666,7 @@ static struct clk gpio5_dbclk = { .clkdm_name = "l4_per_clkdm", .parent = &sys_32k_ck, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk gpio5_ick = {
@@ -1677,6 +1687,7 @@ static struct clk gpio6_dbclk = { .clkdm_name = "l4_per_clkdm", .parent = &sys_32k_ck, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk gpio6_ick = {
@@ -1809,6 +1820,7 @@ static struct clk iss_ctrlclk = { .clkdm_name = "iss_clkdm", .parent = &func_96m_fclk, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk iss_fck = {
@@ -2145,6 +2157,7 @@ static struct clk ocp2scp_usb_phy_phy_48m = { .clkdm_name = "l3_init_clkdm", .parent = &func_48m_fclk, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk ocp2scp_usb_phy_ick = {
@@ -2206,6 +2219,7 @@ static struct clk slimbus1_fclk_1 = { .clkdm_name = "abe_clkdm", .parent = &func_24m_clk, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk slimbus1_fclk_0 = {
@@ -2216,6 +2230,7 @@ static struct clk slimbus1_fclk_0 = { .clkdm_name = "abe_clkdm", .parent = &abe_24m_fclk, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk slimbus1_fclk_2 = {
@@ -2226,6 +2241,7 @@ static struct clk slimbus1_fclk_2 = { .clkdm_name = "abe_clkdm", .parent = &pad_clks_ck, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk slimbus1_slimbus_clk = {
@@ -2236,6 +2252,7 @@ static struct clk slimbus1_slimbus_clk = { .clkdm_name = "abe_clkdm", .parent = &slimbus_clk, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk slimbus1_fck = {
@@ -2256,6 +2273,7 @@ static struct clk slimbus2_fclk_1 = { .clkdm_name = "l4_per_clkdm", .parent = &per_abe_24m_fclk, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk slimbus2_fclk_0 = {
@@ -2266,6 +2284,7 @@ static struct clk slimbus2_fclk_0 = { .clkdm_name = "l4_per_clkdm", .parent = &func_24mc_fclk, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk slimbus2_slimbus_clk = {
@@ -2276,6 +2295,7 @@ static struct clk slimbus2_slimbus_clk = { .clkdm_name = "l4_per_clkdm", .parent = &pad_slimbus_core_clks_ck, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk slimbus2_fck = {
@@ -2564,6 +2584,7 @@ static struct clk usb_host_hs_utmi_p1_clk = { .clkdm_name = "l3_init_clkdm", .parent = &utmi_p1_gfclk, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static const struct clksel utmi_p2_gfclk_sel[] = {
@@ -2591,6 +2612,7 @@ static struct clk usb_host_hs_utmi_p2_clk = { .clkdm_name = "l3_init_clkdm", .parent = &utmi_p2_gfclk, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk usb_host_hs_utmi_p3_clk = {
@@ -2601,6 +2623,7 @@ static struct clk usb_host_hs_utmi_p3_clk = { .clkdm_name = "l3_init_clkdm", .parent = &init_60m_fclk, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk usb_host_hs_hsic480m_p1_clk = {
@@ -2611,6 +2634,7 @@ static struct clk usb_host_hs_hsic480m_p1_clk = { .clkdm_name = "l3_init_clkdm", .parent = &dpll_usb_m2_ck, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk usb_host_hs_hsic60m_p1_clk = {
@@ -2621,6 +2645,7 @@ static struct clk usb_host_hs_hsic60m_p1_clk = { .clkdm_name = "l3_init_clkdm", .parent = &init_60m_fclk, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk usb_host_hs_hsic60m_p2_clk = {
@@ -2631,6 +2656,7 @@ static struct clk usb_host_hs_hsic60m_p2_clk = { .clkdm_name = "l3_init_clkdm", .parent = &init_60m_fclk, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk usb_host_hs_hsic480m_p2_clk = {
@@ -2641,6 +2667,7 @@ static struct clk usb_host_hs_hsic480m_p2_clk = { .clkdm_name = "l3_init_clkdm", .parent = &dpll_usb_m2_ck, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk usb_host_hs_func48mclk = {
@@ -2651,6 +2678,7 @@ static struct clk usb_host_hs_func48mclk = { .clkdm_name = "l3_init_clkdm", .parent = &func_48mc_fclk, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk usb_host_hs_fck = {
@@ -2688,6 +2716,7 @@ static struct clk usb_otg_hs_xclk = { .clkdm_name = "l3_init_clkdm", .parent = &otg_60m_gfclk, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk usb_otg_hs_ick = {
@@ -2708,6 +2737,7 @@ static struct clk usb_phy_cm_clk32k = { .clkdm_name = "l4_ao_clkdm", .parent = &sys_32k_ck, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk usb_tll_hs_usb_ch2_clk = {
@@ -2718,6 +2748,7 @@ static struct clk usb_tll_hs_usb_ch2_clk = { .clkdm_name = "l3_init_clkdm", .parent = &init_60m_fclk, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk usb_tll_hs_usb_ch0_clk = {
@@ -2728,6 +2759,7 @@ static struct clk usb_tll_hs_usb_ch0_clk = { .clkdm_name = "l3_init_clkdm", .parent = &init_60m_fclk, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk usb_tll_hs_usb_ch1_clk = {
@@ -2738,6 +2770,7 @@ static struct clk usb_tll_hs_usb_ch1_clk = { .clkdm_name = "l3_init_clkdm", .parent = &init_60m_fclk, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk usb_tll_hs_ick = {
@@ -2781,6 +2814,7 @@ static struct clk usim_fclk = { .clkdm_name = "l4_wkup_clkdm", .parent = &usim_ck, .recalc = &followparent_recalc, + .flags = CLOCK_OPTCLK, };static struct clk usim_fck = {
arch/arm/plat-omap/include/plat/clock.h +1 -0
@@ -189,6 +189,7 @@ struct dpll_data {#define ENABLE_ON_INIT (1 << 3)/* Enable upon framework init */ #define INVERT_ENABLE (1 << 4)/* 0 enables, 1 disables */ #define CLOCK_CLKOUTX2 (1 << 5) +#define CLOCK_OPTCLK (1 << 6)/** * struct clk - OMAP struct clk
Signature · 1 line
1.7.0.4
PATCH 8/8 OMAP: clock: Enable clockdomain only for optional clocks
Optional clocks have a requirement to have the clockdomain force enabled (SW_WKUP) before the optional clock itself is enabled.
Since optional clocks are currently handled directly by drivers using the clock framework, this needs to be handled at the clock framework. This sequence is already handled in the omap_hwmod framework for the essential/main clocks.
--- arch/arm/mach-omap2/clock.c | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/clock.c +15 -0
@@ -286,6 +286,7 @@ void omap2_clk_disable(struct clk *clk)int omap2_clk_enable (struct clk *clk) {int ret; +int hwsup =0 ;pr_debug ("clock: %s: incrementing usecount\n" , clk->name);
@@ -304,6 +305,17 @@ int omap2_clk_enable(struct clk *clk) goto oce_err1; } } +/* +* TODO: This is needed here only as long as drivers use +* clock framework to enable optional clocks. For all the +* essential clocks, this sequence is handled in the +* omap_hwmod framework. +*/ +/* Enable the clockdomain, if its an optional clock */ +if ((clk->flags &CLOCK_OPTCLK ) && (clk->clkdm)) { + hwsup =clkdm_is_idle (clk->clkdm); +clkdm_wakeup (clk->clkdm); + }if (clk->ops && clk->ops->enable) {trace_clock_enable (clk->name,1 ,smp_processor_id ());
@@ -315,6 +327,9 @@ int omap2_clk_enable(struct clk *clk) } } +if ((clk->flags &CLOCK_OPTCLK ) && (clk->clkdm) && hwsup) +clkdm_allow_idle (clk->clkdm); +return 0 ; oce_err2:
Signature · 1 line
1.7.0.4
Paul Walmsley
maintainerPATCH 5/8 OMAP4: PM: TEMP: Prevent l3init from idling/force sleep
In reply to Rajendra NayakHi Rajendra,
Quoted 21 lines · Rajendra Nayak
Since MMC driver is yet to be adapted to runtime PM and still uses direct clock calls to enable/disable module, its needed that the clockdomain (for MMC) is always kept force enabled since the next few patches move
the clockdomain handling from clock framework to hwmod framework and break MMC.This will certainlly gate any CORE low power transitions.
Signed-off-by Rajendra Nayak <[email protected]>--- arch/arm/mach-omap2/clockdomains44xx_data.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)arch/arm/mach-omap2/clockdomains44xx_data.c +0
-0
arch/arm/mach-omap2/clockdomains44xx_data.c static struct clockdomain l3_init_44xx_clkdm = { · line 498
Quoted 7 lines
arch/arm/mach-omap2/clockdomains44xx_data.c +1
-1@@ -493,7 +493,7 @@ static struct clockdomain l3_init_44xx_clkdm = { .dep_bit = OMAP4430_L3INIT_STATDEP_SHIFT, .wkdep_srcs = l3_init_wkup_sleep_deps, .sleepdep_srcs = l3_init_wkup_sleep_deps, - .flags = CLKDM_CAN_HWSUP_SWSUP, + .flags = CLKDM_CAN_FORCE_WAKEUP, .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP4430), };
Since this is basically a hack to work around limitations of the current MMC driver, if we accept something like this, I think it should come with a big warning message and comment. Something like this in omap44xx_clockdomains_init():
/*
* XXX The OMAP L3 interconnect hardware is able to enter
* hardware-supervised idle. But because the OMAP HSMMC
* driver still hasn't been converted to use runtime PM, if
* the L3 is allowed to enter hwsup idle, the kernel will
* crash. Once the MMC driver is fixed (which patches have
* been posted to do, with subject line "OMAP: HSMMC: cleanup
* and runtime pm") the change to the l3_init_44xx_clkdm
* flags should be dropped. It limits the low-power state that
* the chip can enter.
*/
pr_warn("WARNING: OMAP4 low power states artificially limited, due
to unconverted HSMMC driver\n");
- Paul
PATCH 5/8 OMAP4: PM: TEMP: Prevent l3init from idling/force sleep
Quoted 5 lines · Paul Walmsley
Since this is basically a hack to work around limitations of the current MMC driver, if we accept something like this, I think it should come with a big warning message and comment. Something like this in
omap44xx_clockdomains_init():
Quoted 13 lines
/*
* XXX The OMAP L3 interconnect hardware is able to enter
* hardware-supervised idle. But because the OMAP HSMMC
* driver still hasn't been converted to use runtime PM, if
* the L3 is allowed to enter hwsup idle, the kernel will
* crash. Once the MMC driver is fixed (which patches have
* been posted to do, with subject line "OMAP: HSMMC: cleanup
* and runtime pm") the change to thel3_init_44xx_clkdm
* flags should be dropped. It limits the low-power state that
* the chip can enter.
*/
pr_warn("WARNING: OMAP4 low power states artificially limited, due
to unconverted HSMMC driver\n");
Do you really want to continue pissing Linus off with churn like this rather than pressing to get problems fixed _properly_ (eg, getting the HSMMC driver fixed) ?
Paul Walmsley
maintainerPATCH 5/8 OMAP4: PM: TEMP: Prevent l3init from idling/force sleep
Hi
Quoted 3 lines · Russell King - ARM Linux
Do you really want to continue pissing Linus off with churn like this rather than pressing to get problems fixed _properly_ (eg, getting the HSMMC driver fixed) ?
Thanks for your comments. We won't queue this patch.
- Paul
PATCH 4/8 OMAP2+: PM: idle clkdms only if already in idle
In reply to Rajendra NayakQuoted 34 lines · Rajendra Nayak
The
omap_set_pwrdm_statefunction forces clockdomains to idle, without checking the existing idle state programmed, instead based solely on the HW capability of the clockdomain to support idle.
This is wrong and the clockdomains should be idled post astate_switch*only* if idle transitions on the clockdomain were already enabled.Signed-off-by Rajendra Nayak <[email protected]>--- arch/arm/mach-omap2/pm.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)arch/arm/mach-omap2/pm.c +2
-0@@ -108,6 +108,7 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)u32 cur_state;int sleep_switch =-1 ;int ret =0 ; +int hwsup =0 ;if (pwrdm ==NULL ||IS_ERR (pwrdm))return -EINVAL ;@@ -127,6 +128,7 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state) (pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE)) { sleep_switch = LOWPOWERSTATE_SWITCH; } else { + hwsup =clkdm_is_idle (pwrdm->pwrdm_clkdms[0 ]);clkdm_wakeup (pwrdm->pwrdm_clkdms[0 ]);pwrdm_wait_transition (pwrdm); sleep_switch =FORCEWAKEUP_SWITCH ;
arch/arm/mach-omap2/pm.c int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state) · line 150
Quoted 8 lines
arch/arm/mach-omap2/pm.c +1
-1@@ -142,7 +144,7 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)switch (sleep_switch) {case FORCEWAKEUP_SWITCH : -if (pwrdm->pwrdm_clkdms[0 ]->flags &CLKDM_CAN_ENABLE_AUTO ) +if (hwsup)clkdm_allow_idle (pwrdm->pwrdm_clkdms[0 ]);else clkdm_sleep (pwrdm->pwrdm_clkdms[0 ]);
Is concurrency protection needed here? Not sure if it's expected that multiple threads would simultaneously manage the state of the same power domain, or that the associated clock domain would change state concurrently.
Todd
Quoted 6 lines
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 4/8 OMAP2+: PM: idle clkdms only if already in idle
Quoted 21 lines · Todd Poynor
Quoted 14 lines
(quoted diff) +1
-0+ int hwsup = 0;Quoted 13 lines
if (pwrdm == NULL || IS_ERR(pwrdm)) return -EINVAL; @@ -127,6 +128,7 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state) (pwrdm->flags& PWRDM_HAS_LOWPOWERSTATECHANGE)) { sleep_switch = LOWPOWERSTATE_SWITCH; } else { + hwsup = clkdm_is_idle(pwrdm->pwrdm_clkdms[0]); clkdm_wakeup(pwrdm->pwrdm_clkdms[0]); pwrdm_wait_transition(pwrdm); sleep_switch = FORCEWAKEUP_SWITCH;@@ -142,7 +144,7 @@ int
omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)Quoted 7 lines
Quoted 7 lines
switch (sleep_switch) { case FORCEWAKEUP_SWITCH: - if (pwrdm->pwrdm_clkdms[0]->flags& CLKDM_CAN_ENABLE_AUTO) + if (hwsup) clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]); else clkdm_sleep(pwrdm->pwrdm_clkdms[0]);
Quoted 4 lines
Is concurrency protection needed here? Not sure if it's expected that multiple threads would simultaneously manage the state of the same power domain, or that the associated clock domain would change state concurrently.
Yeah, maybe there is a need for a per-clkdm lock to prevent these concurrency issues. This race between omap_set_pwrdm_state and clk_enable/disable all programming clkdm state was one possibility for a race (even without this series), but with this series to move the clkdm handling into hwmod there are certainly more possibilities (across concurrent omap_hwmod_enable/idle) because the hwmod framework uses per-hwmod lock while the clock framework used a global lock. Thanks for bringing this up.
Paul/Benoit any thoughts on if a per-clkdm lock seems reasonable?
I just did a quick patch to add per-clkdm locking, it also has a couple instances of arch-specific clkdm calls making calls back to generic clkdm framework fixed, which otherwise would result in recursive locking.
----
From: Rajendra Nayak <[email protected]>
Date: Fri, 10 Jun 2011 15:42:25 +0530
Subject: [PATCH] OMAP: clockdomain: Add per clkdm lock to prevent concurrent state programming
Since the clkdm state programming is now done from within the hwmod framework (which uses a per-hwmod lock) instead of the being done from the clock framework (which used a global lock), there is now a need to have per-clkdm locking to prevent races between different hwmods/modules belonging to the same clock domain concurrently programming the clkdm state.
--- arch/arm/mach-omap2/clockdomain.c | 42 ++++++++++++++++++++++++++-- arch/arm/mach-omap2/clockdomain.h | 2 + arch/arm/mach-omap2/clockdomain2xxx_3xxx.c | 6 ++- arch/arm/mach-omap2/clockdomain44xx.c | 7 ++-- 4 files changed, 49 insertions(+), 8 deletions(-)
(quoted diff) +0 -0
b/arch/arm/mach-omap2/clockdomain.c
index b98a972..374e669 100644
arch/arm/mach-omap2/clockdomain.c +33 -3
@@ -92,6 +92,8 @@ static int _clkdm_register(struct clockdomain *clkdm)pwrdm_add_clkdm (pwrdm, clkdm); +spin_lock_init (&clkdm->lock); +pr_debug ("clockdomain: registered %s\n" , clkdm->name);return 0 ;
@@ -690,6 +692,9 @@ int clkdm_clear_all_sleepdeps(struct clockdomain *clkdm)*/ int clkdm_sleep (struct clockdomain *clkdm) { +int ret; +unsigned long flags; +if (!clkdm)return -EINVAL ;
@@ -704,7 +709,10 @@ int clkdm_sleep(struct clockdomain *clkdm)pr_debug ("clockdomain: forcing sleep on %s\n" , clkdm->name); -return arch_clkdm->clkdm_sleep (clkdm); +spin_lock_irqsave (&clkdm->lock, flags); + ret = arch_clkdm->clkdm_sleep (clkdm); +spin_unlock_irqrestore (&clkdm->lock, flags); +return ret; }/**
@@ -718,6 +726,9 @@ int clkdm_sleep(struct clockdomain *clkdm)*/ int clkdm_wakeup (struct clockdomain *clkdm) { +int ret; +unsigned long flags; +if (!clkdm)return -EINVAL ;
@@ -732,7 +743,10 @@ int clkdm_wakeup(struct clockdomain *clkdm)pr_debug ("clockdomain: forcing wakeup on %s\n" , clkdm->name); -return arch_clkdm->clkdm_wakeup (clkdm); +spin_lock_irqsave (&clkdm->lock, flags); + ret = arch_clkdm->clkdm_wakeup (clkdm); +spin_unlock_irqrestore (&clkdm->lock, flags); +return ret; }/**
@@ -747,6 +761,8 @@ int clkdm_wakeup(struct clockdomain *clkdm)*/ void clkdm_allow_idle (struct clockdomain *clkdm) { +unsigned long flags; +if (!clkdm)return ;
@@ -762,8 +778,10 @@ void clkdm_allow_idle(struct clockdomain *clkdm)pr_debug ("clockdomain: enabling automatic idle transitions for %s\n" , clkdm->name); +spin_lock_irqsave (&clkdm->lock, flags); arch_clkdm->clkdm_allow_idle (clkdm);pwrdm_clkdm_state_switch (clkdm); +spin_unlock_irqrestore (&clkdm->lock, flags); }/**
@@ -777,6 +795,8 @@ void clkdm_allow_idle(struct clockdomain *clkdm)*/ void clkdm_deny_idle (struct clockdomain *clkdm) { +unsigned long flags; +if (!clkdm)return ;
@@ -792,7 +812,9 @@ void clkdm_deny_idle(struct clockdomain *clkdm)pr_debug ("clockdomain: disabling automatic idle transitions for %s\n" , clkdm->name); +spin_lock_irqsave (&clkdm->lock, flags); arch_clkdm->clkdm_deny_idle (clkdm); +spin_unlock_irqrestore (&clkdm->lock, flags); }/**
@@ -805,6 +827,9 @@ void clkdm_deny_idle(struct clockdomain *clkdm)*/ int clkdm_is_idle (struct clockdomain *clkdm) { +int ret; +unsigned long flags; +if (!clkdm)return -EINVAL ;
@@ -813,7 +838,10 @@ int clkdm_is_idle(struct clockdomain *clkdm)pr_debug ("clockdomain: reading idle state for %s\n" , clkdm->name); -return arch_clkdm->clkdm_is_idle (clkdm); +spin_lock_irqsave (&clkdm->lock, flags); + ret = arch_clkdm->clkdm_is_idle (clkdm); +spin_unlock_irqrestore (&clkdm->lock, flags); +return ret; }
@@ -835,6 +863,8 @@ int clkdm_is_idle(struct clockdomain *clkdm)*/ int clkdm_clk_enable (struct clockdomain *clkdm,struct clk *clk) { +unsigned long flags; +/* * XXX Rewrite this code to maintain a list of enabled * downstream clocks for debugging purposes?
@@ -859,9 +889,11 @@ int clkdm_clk_enable(struct clockdomain *clkdm,
struct clk *clk)
pr_debug("clockdomain: clkdm %s: clk %s now enabled\n", clkdm->name,
clk->name);
+ spin_lock_irqsave(&clkdm->lock, flags);
arch_clkdm->clkdm_clk_enable(clkdm);
pwrdm_wait_transition(clkdm->pwrdm.ptr);
pwrdm_clkdm_state_switch(clkdm);
+ spin_unlock_irqrestore(&clkdm->lock, flags);
return 0;
}
@@ -882,6 +914,8 @@ int clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk)
*/
int clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk) {
+ unsigned long flags;
+
/*
* XXX Rewrite this code to maintain a list of enabled
* downstream clocks for debugging purposes?
@@ -908,8 +942,10 @@ int clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk)
pr_debug("clockdomain: clkdm %s: clk %s now disabled\n", clkdm->name,
clk->name);
+ spin_lock_irqsave(&clkdm->lock, flags);
arch_clkdm->clkdm_clk_disable(clkdm);
pwrdm_clkdm_state_switch(clkdm);
+ spin_unlock_irqrestore(&clkdm->lock, flags);
return 0;
}
(quoted diff) +0 -0
b/arch/arm/mach-omap2/clockdomain.h
index 085ed82..9fd4eb5 100644
arch/arm/mach-omap2/clockdomain.h +2 -0
@@ -17,6 +17,7 @@#define __ARCH_ARM_MACH_OMAP2_CLOCKDOMAIN_H#include <linux/init.h> +#include <linux/spinlock.h> #include "powerdomain.h" #include <plat/clock.h>
@@ -122,6 +123,7 @@ struct clockdomain {const struct omap_chip_id omap_chip;atomic_t usecount;struct list_head node; +spinlock_t lock; };/**
(quoted diff) +0 -0
b/arch/arm/mach-omap2/clockdomain2xxx_3xxx.c
index e0c393f..f841ac8 100644
arch/arm/mach-omap2/clockdomain2xxx_3xxx.c +0 -0
@@ -193,7 +193,8 @@ static int omap2_clkdm_clk_enable(struct clockdomain
*clkdm)
_clkdm_add_autodeps(clkdm);
_enable_hwsup(clkdm);
} else {
- clkdm_wakeup(clkdm);
+ if (clkdm->flags & CLKDM_CAN_FORCE_WAKEUP)
+ omap2_clkdm_wakeup(clkdm);
}
return 0;
@@ -215,7 +216,8 @@ static int omap2_clkdm_clk_disable(struct clockdomain *clkdm)
_clkdm_del_autodeps(clkdm); _enable_hwsup(clkdm);
} else {
- clkdm_sleep(clkdm);
+ if (clkdm->flags & CLKDM_CAN_FORCE_SLEEP)
+ omap2_clkdm_sleep(clkdm);
}
return 0;
(quoted diff) +0 -0
b/arch/arm/mach-omap2/clockdomain44xx.c
index e4b8e06..911770b 100644
arch/arm/mach-omap2/clockdomain44xx.c +0 -0
@@ -101,7 +101,8 @@ static int omap4_clkdm_is_idle(struct clockdomain
*clkdm)
static int omap4_clkdm_clk_enable(struct clockdomain *clkdm) {
- clkdm_wakeup(clkdm);
+ if (clkdm->flags & CLKDM_CAN_FORCE_WAKEUP)
+ return omap4_clkdm_wakeup(clkdm);
return 0;
}
@@ -112,8 +113,8 @@ static int omap4_clkdm_clk_disable(struct clockdomain *clkdm)
hwsup = omap4_cminst_is_clkdm_in_hwsup(clkdm->prcm_partition,
clkdm->cm_inst, clkdm->clkdm_offs);
- if (!hwsup)
- clkdm_sleep(clkdm);
+ if (!hwsup && (clkdm->flags & CLKDM_CAN_FORCE_SLEEP))
+ omap4_clkdm_sleep(clkdm);
return 0;
}
Signature · 1 line
1.7.0.4
Paul Walmsley
maintainerPATCH 4/8 OMAP2+: PM: idle clkdms only if already in idle
Hi Rajendra, Todd,
Quoted 1 line · Rajendra Nayak
Paul/Benoit any thoughts on if a per-clkdm lock seems reasonable?
Sounds okay to me.
The experimental patch that you sent didn't add the locking to the *wkdep, *sleepdep functions; I guess we'd better add it there at the same time, since some of the register access there does a read-modify-write.
It should be possible to get rid of the atomic_t usage in the clockdomain code as part of the same series.
Todd, thanks for pointing this out.
- Paul
PATCH 4/8 OMAP2+: PM: idle clkdms only if already in idle
Hi Paul,
Quoted 11 lines · Paul Walmsley
Hi Rajendra, Todd,
Quoted 1 line · Rajendra Nayak
Paul/Benoit any thoughts on if a per-clkdm lock seems reasonable?
Sounds okay to me.
The experimental patch that you sent didn't add the locking to the *wkdep, *sleepdep functions; I guess we'd better add it there at the same time, since some of the register access there does a read-modify-write.
My initial idea was to just guard the functions which program the target clockdomain state, since that's something which had a possibility of racing.
For the sleepdep/wkupdep programming, I thought they are all done from within frameworks and during pm-core init at boot and might not run into concurrency issues. But maybe it makes sense to guard those as well.
Quoted 3 lines
It should be possible to get rid of the
atomic_tusage in the clockdomain code as part of the same series.
Sure, I'll get rid of those.
Thanks,
Rajendra
Quoted 5 lines
Todd, thanks for pointing this out.
- Paul
PATCH 1/8 OMAP2+: clockdomain: Add an api to read idle mode
In reply to Rajendra NayakQuoted 3 lines · Rajendra Nayak
Add a clockdomain api to check if hardware supervised idle transitions are enabled on a clockdomain.
...
Quoted 1 line
+ *
clkdm_is_idle- Check if the clkdm hwsup/autoidle is enabled
A minor suggestion on naming: "clkdm_allows_idle" seems more accurate and a natural complement to the existing clkdm_allow_idle().
Todd
PATCH 1/8 OMAP2+: clockdomain: Add an api to read idle mode
Quoted 9 lines · Todd Poynor
Quoted 3 lines · Rajendra Nayak
Add a clockdomain api to check if hardware supervised idle transitions are enabled on a clockdomain.
...
Quoted 1 line
+ *
clkdm_is_idle- Check if the clkdm hwsup/autoidle is enabledA minor suggestion on naming: "
clkdm_allows_idle" seems more accurate and a natural complement to the existingclkdm_allow_idle().
Yeah, I agree. I'll change it in my next spin.
Thanks,
Rajendra
Quoted 3 lines
Todd