| Message ID | 1309191103-8817-9-git-send-email-b-cousson@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Benoit Cousson |
| Headers | show |
Benoit Cousson <b-cousson@ti.com> writes: > From: Rajendra Nayak <rnayak@ti.com> > > On OMAP4, the PRCM recommended sequence for enabling > a module after power-on-reset is: > -1- Force clkdm to SW_WKUP > -2- Enabling the clocks > -3- Configure desired module mode to "enable" or "auto" > -4- Wait for the desired module idle status to be FUNC > -5- 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. OK, I found the problem that prevents booting on OMAP3... > 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 -4- above to "Wait for the desired > module idle status to be FUNC" is done as part of > hwmod framework. > > Signed-off-by: Rajendra Nayak <rnayak@ti.com> > [b-cousson@ti.com: Adapt it to the new clkdm hwmod attribute and API] > Signed-off-by: Benoit Cousson <b-cousson@ti.com> > Cc: Paul Walmsley <paul@pwsan.com> > --- > arch/arm/mach-omap2/clock.c | 17 +---------------- > arch/arm/mach-omap2/omap_hwmod.c | 26 +++++++++++++++++++++++++- > 2 files changed, 26 insertions(+), 17 deletions(-) > > diff --git a/arch/arm/mach-omap2/clock.c b/arch/arm/mach-omap2/clock.c > index 180299e..2828d29 100644 > --- a/arch/arm/mach-omap2/clock.c > +++ b/arch/arm/mach-omap2/clock.c > @@ -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; > } > } The clkdm enable/disable in this part of the patch is still required on OMAP2/3, since the clkdm is still attached to the clock and not to the hwmod. Just backing out the mach-omap2/clock.c changes from this patch allows this series to boot fine on OMAP3. Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 6/28/2011 7:16 PM, Hilman, Kevin wrote: > Benoit Cousson<b-cousson@ti.com> writes: > >> From: Rajendra Nayak<rnayak@ti.com> >> >> On OMAP4, the PRCM recommended sequence for enabling >> a module after power-on-reset is: >> -1- Force clkdm to SW_WKUP >> -2- Enabling the clocks >> -3- Configure desired module mode to "enable" or "auto" >> -4- Wait for the desired module idle status to be FUNC >> -5- 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. > > OK, I found the problem that prevents booting on OMAP3... Cool... thanks for that. >> 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 -4- above to "Wait for the desired >> module idle status to be FUNC" is done as part of >> hwmod framework. >> >> Signed-off-by: Rajendra Nayak<rnayak@ti.com> >> [b-cousson@ti.com: Adapt it to the new clkdm hwmod attribute and API] >> Signed-off-by: Benoit Cousson<b-cousson@ti.com> >> Cc: Paul Walmsley<paul@pwsan.com> >> --- >> arch/arm/mach-omap2/clock.c | 17 +---------------- >> arch/arm/mach-omap2/omap_hwmod.c | 26 +++++++++++++++++++++++++- >> 2 files changed, 26 insertions(+), 17 deletions(-) >> >> diff --git a/arch/arm/mach-omap2/clock.c b/arch/arm/mach-omap2/clock.c >> index 180299e..2828d29 100644 >> --- a/arch/arm/mach-omap2/clock.c >> +++ b/arch/arm/mach-omap2/clock.c >> @@ -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; >> } >> } > > The clkdm enable/disable in this part of the patch is still required on > OMAP2/3, since the clkdm is still attached to the clock and not to the > hwmod. OK, but I guess that in case of OMAP4, we are probably incrementing twice the usage counter... but that should not hurt, because we will decrement twice as well. Rajendra, Does that seems OK to you? Thanks, Benoit -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 6/28/2011 2:33 PM, Cousson, Benoit wrote: > On 6/28/2011 7:16 PM, Hilman, Kevin wrote: >> Benoit Cousson<b-cousson@ti.com> writes: >> >>> From: Rajendra Nayak<rnayak@ti.com> >>> >>> On OMAP4, the PRCM recommended sequence for enabling >>> a module after power-on-reset is: >>> -1- Force clkdm to SW_WKUP >>> -2- Enabling the clocks >>> -3- Configure desired module mode to "enable" or "auto" >>> -4- Wait for the desired module idle status to be FUNC >>> -5- 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. >> >> OK, I found the problem that prevents booting on OMAP3... > > Cool... thanks for that. > >>> 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 -4- above to "Wait for the desired >>> module idle status to be FUNC" is done as part of >>> hwmod framework. >>> >>> Signed-off-by: Rajendra Nayak<rnayak@ti.com> >>> [b-cousson@ti.com: Adapt it to the new clkdm hwmod attribute and API] >>> Signed-off-by: Benoit Cousson<b-cousson@ti.com> >>> Cc: Paul Walmsley<paul@pwsan.com> >>> --- >>> arch/arm/mach-omap2/clock.c | 17 +---------------- >>> arch/arm/mach-omap2/omap_hwmod.c | 26 +++++++++++++++++++++++++- >>> 2 files changed, 26 insertions(+), 17 deletions(-) >>> >>> diff --git a/arch/arm/mach-omap2/clock.c b/arch/arm/mach-omap2/clock.c >>> index 180299e..2828d29 100644 >>> --- a/arch/arm/mach-omap2/clock.c >>> +++ b/arch/arm/mach-omap2/clock.c >>> @@ -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; >>> } >>> } >> >> The clkdm enable/disable in this part of the patch is still required on >> OMAP2/3, since the clkdm is still attached to the clock and not to the >> hwmod. > > OK, but I guess that in case of OMAP4, we are probably incrementing > twice the usage counter... but that should not hurt, because we will > decrement twice as well. > > Rajendra, > Does that seems OK to you? When I did the initial patch, all clkdm's were still associated with clocks, and removing this from clock framework made sense and also worked on OMAP3/2, because I had clkdm_clk_enable/disable being called from hwmod framework. Now that we have clkdm_hwmod_enable/disable being called from hwmod framework, and clkdm's are associated with hwmod, I guess its best to do this for all OMAP's. Keeping this association of clkdm with clocks (for OMAP3/2) and the above piece of code because that association exits looks messy. > > Thanks, > Benoit -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/mach-omap2/clock.c b/arch/arm/mach-omap2/clock.c index 180299e..2828d29 100644 --- a/arch/arm/mach-omap2/clock.c +++ b/arch/arm/mach-omap2/clock.c @@ -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); diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c index 3eef106..3538805 100644 --- a/arch/arm/mach-omap2/omap_hwmod.c +++ b/arch/arm/mach-omap2/omap_hwmod.c @@ -1360,6 +1360,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 && @@ -1378,6 +1379,19 @@ static int _enable(struct omap_hwmod *oh) omap_hwmod_mux(oh->mux, _HWMOD_STATE_ENABLED); _add_initiator_dep(oh, mpu_oh); + + /* + * A clockdomain must be in SW_SUP before enabling completely the + * module. The clockdomain can be set in HW_AUTO only when the module + * become ready. + */ + hwsup = clkdm_allows_idle(oh->clkdm); + r = clkdm_hwmod_enable(oh->clkdm, oh); + if (r) { + WARN(1, "omap_hwmod: %s: could not enable clockdomain %s: %d\n", + oh->name, oh->clkdm->name, r); + return r; + } _enable_clocks(oh); _enable_module(oh); @@ -1392,6 +1406,13 @@ static int _enable(struct omap_hwmod *oh) r = _wait_target_ready(oh); if (!r) { + /* + * Set the clockdomain to HW_AUTO only if the target is ready, + * assuming that the previous state was HW_AUTO + */ + if (hwsup) + clkdm_allow_idle(oh->clkdm); + oh->_state = _HWMOD_STATE_ENABLED; /* Access the sysconfig only if the target is ready */ @@ -1402,7 +1423,8 @@ static int _enable(struct omap_hwmod *oh) } } else { _disable_clocks(oh); - pr_debug("omap_hwmod: %s: _wait_target_ready: %d\n", + clkdm_hwmod_disable(oh->clkdm, oh); + pr_debug("omap_hwmod: %s: _wait_target_ready failed: %d\n", oh->name, r); } @@ -1444,6 +1466,7 @@ static int _idle(struct omap_hwmod *oh) * transition to complete properly. */ _disable_clocks(oh); + clkdm_hwmod_disable(oh->clkdm, oh); /* Mux pins for device idle if populated */ if (oh->mux && oh->mux->pads_dynamic) @@ -1541,6 +1564,7 @@ static int _shutdown(struct omap_hwmod *oh) pr_debug("omap_hwmod: %s: _wait_target_disable failed\n", oh->name); _disable_clocks(oh); + clkdm_hwmod_disable(oh->clkdm, oh); } /* XXX Should this code also force-disable the optional clocks? */