| Message ID | 501761E8.3030307@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Jon, On Tuesday 31 July 2012 10:11 AM, Jon Hunter wrote: > Hi Paul, Rajendra, > > On 07/27/2012 12:43 AM, Rajendra Nayak wrote: >> On Friday 27 July 2012 02:34 AM, Paul Walmsley wrote: >>> >>> Commit 4da71ae6 ("OMAP: clockdomain: Arch specific funcs for >>> clkdm_clk_enable/disable") called the OMAP2xxx-specific functions for >>> clockdomain wakeup and sleep. This would probably have broken >>> software-supervised clockdomain wakeup and sleep on OMAP3. >> >> Its strange this went unnoticed for so long. Thanks for this fix and >> sorry about introducing the bug in the first place. > > Any chance that's because of the following code? I needed to > remove this to get the EMU clock domain to turn off on OMAP3 > whilst testing PMU. No, this doesn't seem right. We still have clockdomains for omap2/3 controlled using clkdm_clk_enable/disable functions called from within clk framework, and not clkdm_hwmod_enable/disable from within hwmod framework. Besides you removing these checks only in clkdm_hwmod_disable (and keeping them in clkdm_hwmod_enable) tells me its just hiding some usecounting issues you were having with clkdm_clk_enable/disable. Btw, on OMAP2/3 as long as a domain has interface clocks which are explicitly enabled and autoidled, the clkdm usecount never ends up going to 0, which is probably what you are hit with too. regards, Rajendra > > Cheers > Jon > > commit a0307bd539ecef976793679a1c4ff0d47b22c4bd > Author: Jon Hunter<jon-hunter@ti.com> > Date: Mon Jul 30 18:04:06 2012 -0500 > > ARM: OMAP2/3: Allow HWMOD to disable clock domains > > Currently when HWMOD attempts to disable a clock domain on OMAP2/3 devices we > will return from the function clkdm_hwmod_disable() without actually disabling > the clock domain. Per the comment this is deliberate because initially HWMOD > OMAP2/3 devices did not support clock domains. However, clock domains are now > supported by HWMOD for these devices and so allow HWMOD to disable the clock > domains. > > XXX - Tested on OMAP3430 beagle board, but needs more testing on OMAP2/3 > > diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c > index 011186f..8f7a941 100644 > --- a/arch/arm/mach-omap2/clockdomain.c > +++ b/arch/arm/mach-omap2/clockdomain.c > @@ -1075,10 +1075,6 @@ int clkdm_hwmod_enable(struct clockdomain *clkdm, struct omap_hwmod *oh) > */ > int clkdm_hwmod_disable(struct clockdomain *clkdm, struct omap_hwmod *oh) > { > - /* The clkdm attribute does not exist yet prior OMAP4 */ > - if (cpu_is_omap24xx() || cpu_is_omap34xx()) > - return 0; > - > /* > * XXX Rewrite this code to maintain a list of enabled > * downstream hwmods for debugging purposes? > -- 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
Hi Rajendra, On 07/31/2012 12:41 AM, Rajendra Nayak wrote: > Hi Jon, > > On Tuesday 31 July 2012 10:11 AM, Jon Hunter wrote: >> Hi Paul, Rajendra, >> >> On 07/27/2012 12:43 AM, Rajendra Nayak wrote: >>> On Friday 27 July 2012 02:34 AM, Paul Walmsley wrote: >>>> >>>> Commit 4da71ae6 ("OMAP: clockdomain: Arch specific funcs for >>>> clkdm_clk_enable/disable") called the OMAP2xxx-specific functions for >>>> clockdomain wakeup and sleep. This would probably have broken >>>> software-supervised clockdomain wakeup and sleep on OMAP3. >>> >>> Its strange this went unnoticed for so long. Thanks for this fix and >>> sorry about introducing the bug in the first place. >> >> Any chance that's because of the following code? I needed to >> remove this to get the EMU clock domain to turn off on OMAP3 >> whilst testing PMU. > > No, this doesn't seem right. We still have clockdomains for omap2/3 > controlled using clkdm_clk_enable/disable functions called from > within clk framework, and not clkdm_hwmod_enable/disable from > within hwmod framework. > Besides you removing these checks only in clkdm_hwmod_disable (and > keeping them in clkdm_hwmod_enable) tells me its just hiding some > usecounting issues you were having with clkdm_clk_enable/disable. Yes you are right. I was focused in the disable side because the EMU CD is staying on. > Btw, on OMAP2/3 as long as a domain has interface clocks which are > explicitly enabled and autoidled, the clkdm usecount never ends up > going to 0, which is probably what you are hit with too. Yes so after further debugging, this is not a problem and not the cause of my problem either. Sorry for the noise. Cheers Jon -- 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/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c index 011186f..8f7a941 100644 --- a/arch/arm/mach-omap2/clockdomain.c +++ b/arch/arm/mach-omap2/clockdomain.c @@ -1075,10 +1075,6 @@ int clkdm_hwmod_enable(struct clockdomain *clkdm, struct omap_hwmod *oh) */ int clkdm_hwmod_disable(struct clockdomain *clkdm, struct omap_hwmod *oh) { - /* The clkdm attribute does not exist yet prior OMAP4 */ - if (cpu_is_omap24xx() || cpu_is_omap34xx()) - return 0; - /* * XXX Rewrite this code to maintain a list of enabled * downstream hwmods for debugging purposes?