| Message ID | alpine.DEB.2.00.1210090503470.5736@utopia.booyaka.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Tuesday 09 October 2012 11:08 AM, Paul Walmsley wrote: > cc Vaibhav due to the AM33xx change > > Hi > > On Thu, 4 Oct 2012, Archit Taneja wrote: > >> I was trying out the linux-next kernel, and I noticed that DSS MODULEMODE bits >> are never cleared. >> >> In _omap4_disable_module(), there is a check: >> >> ... >> >> if (!_are_all_hardreset_lines_asserted(oh)) >> return 0; >> >> /* MODULEMODE bits cleared here */ >> ... >> ... >> ... >> >> The function _are_all_hardreset_lines_asserted() returns false if >> 'oh->rst_lines_cnt == 0', so we bail out from _omap4_disable_module() before >> clearing the MODULEMODE bits. >> >> Is this correct behavior? This would prevent all hwmods who have rst_lines_cnt >> as 0 to not get their MODULEMODE bits cleared. > > You're right, that looks bogus. What do you think about the following > patch? > The patch looks fine to me. I tried it out for DSS(with an additional patch to not represent DSS modulemode bits as a slave clock), and modulemode is getting disabled correctly now. Thanks, Archit > > - Paul > > From: Paul Walmsley <paul@pwsan.com> > Date: Mon, 8 Oct 2012 23:08:15 -0600 > Subject: [PATCH] ARM: OMAP4/AM335x: hwmod: fix disable_module regression in > hardreset handling > > Commit eb05f691290e99ee0bd1672317d6add789523c1e ("ARM: OMAP: hwmod: > partially un-reset hwmods might not be properly enabled") added code > to skip the IP block disable sequence if any hardreset lines were left > unasserted. But this did not handle the case when no hardreset lines > were associated with a module, which is the general case. In that > situation, the IP block would be skipped, which is likely to cause PM > regressions. > > So, modify _omap4_disable_module() and _am33xx_disable_module() to > only bail out early if there are any hardreset lines asserted. And > move the AM33xx test above the actual module disable code to ensure > that the behavior is consistent. > > Reported-by: Archit Taneja <a0393947@ti.com> > Cc: Omar Ramirez Luna <omar.luna@linaro.org> > Cc: Vaibhav Hiremath <hvaibhav@ti.com> > Signed-off-by: Paul Walmsley <paul@pwsan.com> > --- > arch/arm/mach-omap2/omap_hwmod.c | 31 +++++++++++++++++++++++++++---- > 1 file changed, 27 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c > index 299ca28..b969ab1 100644 > --- a/arch/arm/mach-omap2/omap_hwmod.c > +++ b/arch/arm/mach-omap2/omap_hwmod.c > @@ -1698,6 +1698,29 @@ static bool _are_all_hardreset_lines_asserted(struct omap_hwmod *oh) > } > > /** > + * _are_any_hardreset_lines_asserted - return true if any part of @oh is > + * hard-reset > + * @oh: struct omap_hwmod * > + * > + * If any hardreset lines associated with @oh are asserted, then > + * return true. Otherwise, if no hardreset lines associated with @oh > + * are asserted, or if @oh has no hardreset lines, then return false. > + * This function is used to avoid executing some parts of the IP block > + * enable/disable sequence if any hardreset line is set. > + */ > +static bool _are_any_hardreset_lines_asserted(struct omap_hwmod *oh) > +{ > + int rst_cnt = 0; > + int i; > + > + for (i = 0; i < oh->rst_lines_cnt && rst_cnt == 0; i++) > + if (_read_hardreset(oh, oh->rst_lines[i].name) > 0) > + rst_cnt++; > + > + return (rst_cnt) ? true : false; > +} > + > +/** > * _omap4_disable_module - enable CLKCTRL modulemode on OMAP4 > * @oh: struct omap_hwmod * > * > @@ -1715,7 +1738,7 @@ static int _omap4_disable_module(struct omap_hwmod *oh) > * Since integration code might still be doing something, only > * disable if all lines are under hardreset. > */ > - if (!_are_all_hardreset_lines_asserted(oh)) > + if (_are_any_hardreset_lines_asserted(oh)) > return 0; > > pr_debug("omap_hwmod: %s: %s\n", oh->name, __func__); > @@ -1749,12 +1772,12 @@ static int _am33xx_disable_module(struct omap_hwmod *oh) > > pr_debug("omap_hwmod: %s: %s\n", oh->name, __func__); > > + if (_are_any_hardreset_lines_asserted(oh)) > + return 0; > + > am33xx_cm_module_disable(oh->clkdm->cm_inst, oh->clkdm->clkdm_offs, > oh->prcm.omap4.clkctrl_offs); > > - if (_are_all_hardreset_lines_asserted(oh)) > - return 0; > - > v = _am33xx_wait_target_disable(oh); > if (v) > pr_warn("omap_hwmod: %s: _wait_target_disable failed\n", > -- 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/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c index 299ca28..b969ab1 100644 --- a/arch/arm/mach-omap2/omap_hwmod.c +++ b/arch/arm/mach-omap2/omap_hwmod.c @@ -1698,6 +1698,29 @@ static bool _are_all_hardreset_lines_asserted(struct omap_hwmod *oh) } /** + * _are_any_hardreset_lines_asserted - return true if any part of @oh is + * hard-reset + * @oh: struct omap_hwmod * + * + * If any hardreset lines associated with @oh are asserted, then + * return true. Otherwise, if no hardreset lines associated with @oh + * are asserted, or if @oh has no hardreset lines, then return false. + * This function is used to avoid executing some parts of the IP block + * enable/disable sequence if any hardreset line is set. + */ +static bool _are_any_hardreset_lines_asserted(struct omap_hwmod *oh) +{ + int rst_cnt = 0; + int i; + + for (i = 0; i < oh->rst_lines_cnt && rst_cnt == 0; i++) + if (_read_hardreset(oh, oh->rst_lines[i].name) > 0) + rst_cnt++; + + return (rst_cnt) ? true : false; +} + +/** * _omap4_disable_module - enable CLKCTRL modulemode on OMAP4 * @oh: struct omap_hwmod * * @@ -1715,7 +1738,7 @@ static int _omap4_disable_module(struct omap_hwmod *oh) * Since integration code might still be doing something, only * disable if all lines are under hardreset. */ - if (!_are_all_hardreset_lines_asserted(oh)) + if (_are_any_hardreset_lines_asserted(oh)) return 0; pr_debug("omap_hwmod: %s: %s\n", oh->name, __func__); @@ -1749,12 +1772,12 @@ static int _am33xx_disable_module(struct omap_hwmod *oh) pr_debug("omap_hwmod: %s: %s\n", oh->name, __func__); + if (_are_any_hardreset_lines_asserted(oh)) + return 0; + am33xx_cm_module_disable(oh->clkdm->cm_inst, oh->clkdm->clkdm_offs, oh->prcm.omap4.clkctrl_offs); - if (_are_all_hardreset_lines_asserted(oh)) - return 0; - v = _am33xx_wait_target_disable(oh); if (v) pr_warn("omap_hwmod: %s: _wait_target_disable failed\n",