| Message ID | 1435061394-20379-2-git-send-email-rogerq@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi On Tue, 23 Jun 2015, Roger Quadros wrote: > For some hwmods (e.g. DCAN on DRA7) we need the possibility to > disable HW_AUTO for the clockdomain while the module is active. > > To achieve this there needs to be a refcounting mechanism to > indicate whether any module in the clockdomain has requested > to disable HW_AUTO. We keep track of this in 'noidlecount'. > > Hwmod code must use clkdm_hwmod_prevent_hwauto() to prevent > HW_AUTO of the clockdomain in the future clkdm_hwmod_hwauto() calls. > > It must use clkdm_hwmod_allow_hwauto() to allow HW_AUTO in > the future clkdm_hwmod_hwauto() calls. > > Hwmod code must use clkdm_hwmod_allow_hwauto() whenever it needs > to request HW_AUTO of any clockdomain. (Typically after it has > enabled the module). How about just modifying clkdm_{allow,deny}_idle*() to do the idle-block-counting? The default state would be to allow idle, assuming that the clockdomain flags support that state, and then clkdm_deny_idle*() would increment the idle-blocking count, clkdm_allow_idle*() would decrement it. Then on the 0 -> 1 or 1 -> 0 transitions, the hardware would be reprogrammed appropiately. Anyway, seems like that would avoid races with any clkdm_{allow,deny}_idle*() users. A few other comments below: > > Signed-off-by: Roger Quadros <rogerq@ti.com> > --- > arch/arm/mach-omap2/clockdomain.c | 71 +++++++++++++++++++++++++++++++++++++++ > arch/arm/mach-omap2/clockdomain.h | 5 +++ > 2 files changed, 76 insertions(+) > > diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c > index 2da3b5e..a7190d2 100644 > --- a/arch/arm/mach-omap2/clockdomain.c > +++ b/arch/arm/mach-omap2/clockdomain.c > @@ -1212,6 +1212,77 @@ ccd_exit: > return 0; > } > > +/* > + * prevent future hwauto for this clkdm. If clkdm->usecount becomes hwauto isn't prevented. > + * It will only prevnt future hwauto but not bring it out of hwauto. > + */ If you modify clkdm_{allow,deny}_idle*(), this shouldn't be a major issue - but all of the function comments should be fixed so that they are understandable and follow kernel-doc-nano specs. > +int clkdm_hwmod_prevent_hwauto(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; > + > + if (!clkdm || !oh || !arch_clkdm || !arch_clkdm->clkdm_clk_disable) > + return -EINVAL; > + > + > + pwrdm_lock(clkdm->pwrdm.ptr); > + clkdm->noidlecount++; > + pwrdm_unlock(clkdm->pwrdm.ptr); > + > + return 0; > +} > + > +/* > + * allow future hwauto for this clkdm > + * It won't put clkdm into hwauto. use clkdm_hwmod_hwauto() for that. > + */ > +int clkdm_hwmod_allow_hwauto(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; > + > + if (!clkdm || !oh || !arch_clkdm || !arch_clkdm->clkdm_clk_disable) > + return -EINVAL; > + > + > + pwrdm_lock(clkdm->pwrdm.ptr); > + > + if (clkdm->noidlecount == 0) { > + pwrdm_unlock(clkdm->pwrdm.ptr); > + WARN_ON(1); /* underflow */ > + return -ERANGE; > + } > + > + clkdm->noidlecount--; > + pwrdm_unlock(clkdm->pwrdm.ptr); > + > + return 0; > +} > + > +/* > + * put clkdm in hwauto if we can. checks noidlecount to see if we can. > + */ > +int clkdm_hwmod_hwauto(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; > + > + if (!clkdm || !oh || !arch_clkdm || !arch_clkdm->clkdm_clk_disable) > + return -EINVAL; > + > + > + pwrdm_lock(clkdm->pwrdm.ptr); > + if (clkdm->noidlecount == 0) > + clkdm_allow_idle_nolock(clkdm); > + > + pwrdm_unlock(clkdm->pwrdm.ptr); > + > + return 0; > +} > + > /** > * clkdm_hwmod_enable - add an enabled downstream hwmod to this clkdm > * @clkdm: struct clockdomain * > diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h > index 77bab5f..8c491be 100644 > --- a/arch/arm/mach-omap2/clockdomain.h > +++ b/arch/arm/mach-omap2/clockdomain.h > @@ -114,6 +114,7 @@ struct omap_hwmod; > * @wkdep_srcs: Clockdomains that can be told to wake this powerdomain up > * @sleepdep_srcs: Clockdomains that can be told to keep this clkdm from inact > * @usecount: Usecount tracking > + * @noidlecount: Noidle count tracking. Domain won't be auto idled this is > 0. > * @node: list_head to link all clockdomains together > * > * @prcm_partition should be a macro from mach-omap2/prcm44xx.h (OMAP4 only) > @@ -138,6 +139,7 @@ struct clockdomain { > struct clkdm_dep *wkdep_srcs; > struct clkdm_dep *sleepdep_srcs; > int usecount; > + int noidlecount; > struct list_head node; > }; > > @@ -211,6 +213,9 @@ int clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk); > int clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk); > int clkdm_hwmod_enable(struct clockdomain *clkdm, struct omap_hwmod *oh); > int clkdm_hwmod_disable(struct clockdomain *clkdm, struct omap_hwmod *oh); > +int clkdm_hwmod_prevent_hwauto(struct clockdomain *clkdm, struct omap_hwmod *oh); > +int clkdm_hwmod_allow_hwauto(struct clockdomain *clkdm, struct omap_hwmod *oh); > +int clkdm_hwmod_hwauto(struct clockdomain *clkdm, struct omap_hwmod *oh); > > extern void __init omap242x_clockdomains_init(void); > extern void __init omap243x_clockdomains_init(void); > -- > 2.1.4 > - Paul -- 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 16/07/15 04:25, Paul Walmsley wrote: > Hi > > On Tue, 23 Jun 2015, Roger Quadros wrote: > >> For some hwmods (e.g. DCAN on DRA7) we need the possibility to >> disable HW_AUTO for the clockdomain while the module is active. >> >> To achieve this there needs to be a refcounting mechanism to >> indicate whether any module in the clockdomain has requested >> to disable HW_AUTO. We keep track of this in 'noidlecount'. >> >> Hwmod code must use clkdm_hwmod_prevent_hwauto() to prevent >> HW_AUTO of the clockdomain in the future clkdm_hwmod_hwauto() calls. >> >> It must use clkdm_hwmod_allow_hwauto() to allow HW_AUTO in >> the future clkdm_hwmod_hwauto() calls. >> >> Hwmod code must use clkdm_hwmod_allow_hwauto() whenever it needs >> to request HW_AUTO of any clockdomain. (Typically after it has >> enabled the module). > > How about just modifying clkdm_{allow,deny}_idle*() to do the > idle-block-counting? The default state would be to allow idle, assuming > that the clockdomain flags support that state, and then clkdm_deny_idle*() > would increment the idle-blocking count, clkdm_allow_idle*() would > decrement it. Then on the 0 -> 1 or 1 -> 0 transitions, the hardware > would be reprogrammed appropiately. That is not possible with current hwmod code as clkdm_allow_idle() and clkdm_deny_idle() are not symmetrically placed. The usual flow is clkdm_enable_hwmod(); if (hwsup) clkdm_allow_idle(); This is mainly because it is redundant to disable auto idle before enableing (SW_WKUP) the clockdomain. If we take your proposal above then we have to modify all users like so. if (hwsup) clkdm_deny_idle(); clkdm_enable_hwmod(); if (hwsup) clkdm_allow_idle(); Is this really what we want? > > Anyway, seems like that would avoid races with any > clkdm_{allow,deny}_idle*() users. > > A few other comments below: > > >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> --- >> arch/arm/mach-omap2/clockdomain.c | 71 +++++++++++++++++++++++++++++++++++++++ >> arch/arm/mach-omap2/clockdomain.h | 5 +++ >> 2 files changed, 76 insertions(+) >> >> diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c >> index 2da3b5e..a7190d2 100644 >> --- a/arch/arm/mach-omap2/clockdomain.c >> +++ b/arch/arm/mach-omap2/clockdomain.c >> @@ -1212,6 +1212,77 @@ ccd_exit: >> return 0; >> } >> >> +/* >> + * prevent future hwauto for this clkdm. If clkdm->usecount becomes hwauto isn't prevented. >> + * It will only prevnt future hwauto but not bring it out of hwauto. >> + */ > > If you modify clkdm_{allow,deny}_idle*(), this shouldn't be a major issue > - but all of the function comments should be fixed so that they are > understandable and follow kernel-doc-nano specs. OK for updating the comments. cheers, -roger > >> +int clkdm_hwmod_prevent_hwauto(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; >> + >> + if (!clkdm || !oh || !arch_clkdm || !arch_clkdm->clkdm_clk_disable) >> + return -EINVAL; >> + >> + >> + pwrdm_lock(clkdm->pwrdm.ptr); >> + clkdm->noidlecount++; >> + pwrdm_unlock(clkdm->pwrdm.ptr); >> + >> + return 0; >> +} >> + >> +/* >> + * allow future hwauto for this clkdm >> + * It won't put clkdm into hwauto. use clkdm_hwmod_hwauto() for that. >> + */ >> +int clkdm_hwmod_allow_hwauto(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; >> + >> + if (!clkdm || !oh || !arch_clkdm || !arch_clkdm->clkdm_clk_disable) >> + return -EINVAL; >> + >> + >> + pwrdm_lock(clkdm->pwrdm.ptr); >> + >> + if (clkdm->noidlecount == 0) { >> + pwrdm_unlock(clkdm->pwrdm.ptr); >> + WARN_ON(1); /* underflow */ >> + return -ERANGE; >> + } >> + >> + clkdm->noidlecount--; >> + pwrdm_unlock(clkdm->pwrdm.ptr); >> + >> + return 0; >> +} >> + >> +/* >> + * put clkdm in hwauto if we can. checks noidlecount to see if we can. >> + */ >> +int clkdm_hwmod_hwauto(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; >> + >> + if (!clkdm || !oh || !arch_clkdm || !arch_clkdm->clkdm_clk_disable) >> + return -EINVAL; >> + >> + >> + pwrdm_lock(clkdm->pwrdm.ptr); >> + if (clkdm->noidlecount == 0) >> + clkdm_allow_idle_nolock(clkdm); >> + >> + pwrdm_unlock(clkdm->pwrdm.ptr); >> + >> + return 0; >> +} >> + >> /** >> * clkdm_hwmod_enable - add an enabled downstream hwmod to this clkdm >> * @clkdm: struct clockdomain * >> diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h >> index 77bab5f..8c491be 100644 >> --- a/arch/arm/mach-omap2/clockdomain.h >> +++ b/arch/arm/mach-omap2/clockdomain.h >> @@ -114,6 +114,7 @@ struct omap_hwmod; >> * @wkdep_srcs: Clockdomains that can be told to wake this powerdomain up >> * @sleepdep_srcs: Clockdomains that can be told to keep this clkdm from inact >> * @usecount: Usecount tracking >> + * @noidlecount: Noidle count tracking. Domain won't be auto idled this is > 0. >> * @node: list_head to link all clockdomains together >> * >> * @prcm_partition should be a macro from mach-omap2/prcm44xx.h (OMAP4 only) >> @@ -138,6 +139,7 @@ struct clockdomain { >> struct clkdm_dep *wkdep_srcs; >> struct clkdm_dep *sleepdep_srcs; >> int usecount; >> + int noidlecount; >> struct list_head node; >> }; >> >> @@ -211,6 +213,9 @@ int clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk); >> int clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk); >> int clkdm_hwmod_enable(struct clockdomain *clkdm, struct omap_hwmod *oh); >> int clkdm_hwmod_disable(struct clockdomain *clkdm, struct omap_hwmod *oh); >> +int clkdm_hwmod_prevent_hwauto(struct clockdomain *clkdm, struct omap_hwmod *oh); >> +int clkdm_hwmod_allow_hwauto(struct clockdomain *clkdm, struct omap_hwmod *oh); >> +int clkdm_hwmod_hwauto(struct clockdomain *clkdm, struct omap_hwmod *oh); >> >> extern void __init omap242x_clockdomains_init(void); >> extern void __init omap243x_clockdomains_init(void); >> -- >> 2.1.4 >> > > > - Paul > -- 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
Paul, On 16/07/15 16:56, Roger Quadros wrote: > On 16/07/15 04:25, Paul Walmsley wrote: >> Hi >> >> On Tue, 23 Jun 2015, Roger Quadros wrote: >> >>> For some hwmods (e.g. DCAN on DRA7) we need the possibility to >>> disable HW_AUTO for the clockdomain while the module is active. >>> >>> To achieve this there needs to be a refcounting mechanism to >>> indicate whether any module in the clockdomain has requested >>> to disable HW_AUTO. We keep track of this in 'noidlecount'. >>> >>> Hwmod code must use clkdm_hwmod_prevent_hwauto() to prevent >>> HW_AUTO of the clockdomain in the future clkdm_hwmod_hwauto() calls. >>> >>> It must use clkdm_hwmod_allow_hwauto() to allow HW_AUTO in >>> the future clkdm_hwmod_hwauto() calls. >>> >>> Hwmod code must use clkdm_hwmod_allow_hwauto() whenever it needs >>> to request HW_AUTO of any clockdomain. (Typically after it has >>> enabled the module). >> >> How about just modifying clkdm_{allow,deny}_idle*() to do the >> idle-block-counting? The default state would be to allow idle, assuming >> that the clockdomain flags support that state, and then clkdm_deny_idle*() >> would increment the idle-blocking count, clkdm_allow_idle*() would >> decrement it. Then on the 0 -> 1 or 1 -> 0 transitions, the hardware >> would be reprogrammed appropiately. > > That is not possible with current hwmod code as clkdm_allow_idle() and clkdm_deny_idle() > are not symmetrically placed. > > The usual flow is > clkdm_enable_hwmod(); > if (hwsup) > clkdm_allow_idle(); > > This is mainly because it is redundant to disable auto idle before enableing > (SW_WKUP) the clockdomain. > > If we take your proposal above then we have to modify all users like so. > > if (hwsup) > clkdm_deny_idle(); > clkdm_enable_hwmod(); > if (hwsup) > clkdm_allow_idle(); > > Is this really what we want? Any comments on this? cheers, -roger > >> >> Anyway, seems like that would avoid races with any >> clkdm_{allow,deny}_idle*() users. >> >> A few other comments below: >> >> >>> >>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>> --- >>> arch/arm/mach-omap2/clockdomain.c | 71 +++++++++++++++++++++++++++++++++++++++ >>> arch/arm/mach-omap2/clockdomain.h | 5 +++ >>> 2 files changed, 76 insertions(+) >>> >>> diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c >>> index 2da3b5e..a7190d2 100644 >>> --- a/arch/arm/mach-omap2/clockdomain.c >>> +++ b/arch/arm/mach-omap2/clockdomain.c >>> @@ -1212,6 +1212,77 @@ ccd_exit: >>> return 0; >>> } >>> >>> +/* >>> + * prevent future hwauto for this clkdm. If clkdm->usecount becomes hwauto isn't prevented. >>> + * It will only prevnt future hwauto but not bring it out of hwauto. >>> + */ >> >> If you modify clkdm_{allow,deny}_idle*(), this shouldn't be a major issue >> - but all of the function comments should be fixed so that they are >> understandable and follow kernel-doc-nano specs. > > OK for updating the comments. > > > cheers, > -roger > >> >>> +int clkdm_hwmod_prevent_hwauto(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; >>> + >>> + if (!clkdm || !oh || !arch_clkdm || !arch_clkdm->clkdm_clk_disable) >>> + return -EINVAL; >>> + >>> + >>> + pwrdm_lock(clkdm->pwrdm.ptr); >>> + clkdm->noidlecount++; >>> + pwrdm_unlock(clkdm->pwrdm.ptr); >>> + >>> + return 0; >>> +} >>> + >>> +/* >>> + * allow future hwauto for this clkdm >>> + * It won't put clkdm into hwauto. use clkdm_hwmod_hwauto() for that. >>> + */ >>> +int clkdm_hwmod_allow_hwauto(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; >>> + >>> + if (!clkdm || !oh || !arch_clkdm || !arch_clkdm->clkdm_clk_disable) >>> + return -EINVAL; >>> + >>> + >>> + pwrdm_lock(clkdm->pwrdm.ptr); >>> + >>> + if (clkdm->noidlecount == 0) { >>> + pwrdm_unlock(clkdm->pwrdm.ptr); >>> + WARN_ON(1); /* underflow */ >>> + return -ERANGE; >>> + } >>> + >>> + clkdm->noidlecount--; >>> + pwrdm_unlock(clkdm->pwrdm.ptr); >>> + >>> + return 0; >>> +} >>> + >>> +/* >>> + * put clkdm in hwauto if we can. checks noidlecount to see if we can. >>> + */ >>> +int clkdm_hwmod_hwauto(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; >>> + >>> + if (!clkdm || !oh || !arch_clkdm || !arch_clkdm->clkdm_clk_disable) >>> + return -EINVAL; >>> + >>> + >>> + pwrdm_lock(clkdm->pwrdm.ptr); >>> + if (clkdm->noidlecount == 0) >>> + clkdm_allow_idle_nolock(clkdm); >>> + >>> + pwrdm_unlock(clkdm->pwrdm.ptr); >>> + >>> + return 0; >>> +} >>> + >>> /** >>> * clkdm_hwmod_enable - add an enabled downstream hwmod to this clkdm >>> * @clkdm: struct clockdomain * >>> diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h >>> index 77bab5f..8c491be 100644 >>> --- a/arch/arm/mach-omap2/clockdomain.h >>> +++ b/arch/arm/mach-omap2/clockdomain.h >>> @@ -114,6 +114,7 @@ struct omap_hwmod; >>> * @wkdep_srcs: Clockdomains that can be told to wake this powerdomain up >>> * @sleepdep_srcs: Clockdomains that can be told to keep this clkdm from inact >>> * @usecount: Usecount tracking >>> + * @noidlecount: Noidle count tracking. Domain won't be auto idled this is > 0. >>> * @node: list_head to link all clockdomains together >>> * >>> * @prcm_partition should be a macro from mach-omap2/prcm44xx.h (OMAP4 only) >>> @@ -138,6 +139,7 @@ struct clockdomain { >>> struct clkdm_dep *wkdep_srcs; >>> struct clkdm_dep *sleepdep_srcs; >>> int usecount; >>> + int noidlecount; >>> struct list_head node; >>> }; >>> >>> @@ -211,6 +213,9 @@ int clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk); >>> int clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk); >>> int clkdm_hwmod_enable(struct clockdomain *clkdm, struct omap_hwmod *oh); >>> int clkdm_hwmod_disable(struct clockdomain *clkdm, struct omap_hwmod *oh); >>> +int clkdm_hwmod_prevent_hwauto(struct clockdomain *clkdm, struct omap_hwmod *oh); >>> +int clkdm_hwmod_allow_hwauto(struct clockdomain *clkdm, struct omap_hwmod *oh); >>> +int clkdm_hwmod_hwauto(struct clockdomain *clkdm, struct omap_hwmod *oh); >>> >>> extern void __init omap242x_clockdomains_init(void); >>> extern void __init omap243x_clockdomains_init(void); >>> -- >>> 2.1.4 >>> >> >> >> - Paul >> > -- > 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 > -- 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 28/07/15 14:34, Roger Quadros wrote: > Paul, > > On 16/07/15 16:56, Roger Quadros wrote: >> On 16/07/15 04:25, Paul Walmsley wrote: >>> Hi >>> >>> On Tue, 23 Jun 2015, Roger Quadros wrote: >>> >>>> For some hwmods (e.g. DCAN on DRA7) we need the possibility to >>>> disable HW_AUTO for the clockdomain while the module is active. >>>> >>>> To achieve this there needs to be a refcounting mechanism to >>>> indicate whether any module in the clockdomain has requested >>>> to disable HW_AUTO. We keep track of this in 'noidlecount'. >>>> >>>> Hwmod code must use clkdm_hwmod_prevent_hwauto() to prevent >>>> HW_AUTO of the clockdomain in the future clkdm_hwmod_hwauto() calls. >>>> >>>> It must use clkdm_hwmod_allow_hwauto() to allow HW_AUTO in >>>> the future clkdm_hwmod_hwauto() calls. >>>> >>>> Hwmod code must use clkdm_hwmod_allow_hwauto() whenever it needs >>>> to request HW_AUTO of any clockdomain. (Typically after it has >>>> enabled the module). >>> >>> How about just modifying clkdm_{allow,deny}_idle*() to do the >>> idle-block-counting? The default state would be to allow idle, assuming >>> that the clockdomain flags support that state, and then clkdm_deny_idle*() >>> would increment the idle-blocking count, clkdm_allow_idle*() would >>> decrement it. Then on the 0 -> 1 or 1 -> 0 transitions, the hardware >>> would be reprogrammed appropiately. >> >> That is not possible with current hwmod code as clkdm_allow_idle() and clkdm_deny_idle() >> are not symmetrically placed. >> >> The usual flow is >> clkdm_enable_hwmod(); >> if (hwsup) >> clkdm_allow_idle(); >> >> This is mainly because it is redundant to disable auto idle before enableing >> (SW_WKUP) the clockdomain. >> >> If we take your proposal above then we have to modify all users like so. >> >> if (hwsup) >> clkdm_deny_idle(); >> clkdm_enable_hwmod(); >> if (hwsup) >> clkdm_allow_idle(); >> >> Is this really what we want? > > Any comments on this? Paul. I'm waiting on your input to rework this series if needed. Early input would be much appreciated. Thanks. cheers, -roger > >> >>> >>> Anyway, seems like that would avoid races with any >>> clkdm_{allow,deny}_idle*() users. >>> >>> A few other comments below: >>> >>> >>>> >>>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>>> --- >>>> arch/arm/mach-omap2/clockdomain.c | 71 +++++++++++++++++++++++++++++++++++++++ >>>> arch/arm/mach-omap2/clockdomain.h | 5 +++ >>>> 2 files changed, 76 insertions(+) >>>> >>>> diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c >>>> index 2da3b5e..a7190d2 100644 >>>> --- a/arch/arm/mach-omap2/clockdomain.c >>>> +++ b/arch/arm/mach-omap2/clockdomain.c >>>> @@ -1212,6 +1212,77 @@ ccd_exit: >>>> return 0; >>>> } >>>> >>>> +/* >>>> + * prevent future hwauto for this clkdm. If clkdm->usecount becomes hwauto isn't prevented. >>>> + * It will only prevnt future hwauto but not bring it out of hwauto. >>>> + */ >>> >>> If you modify clkdm_{allow,deny}_idle*(), this shouldn't be a major issue >>> - but all of the function comments should be fixed so that they are >>> understandable and follow kernel-doc-nano specs. >> >> OK for updating the comments. >> >> >> cheers, >> -roger >> >>> >>>> +int clkdm_hwmod_prevent_hwauto(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; >>>> + >>>> + if (!clkdm || !oh || !arch_clkdm || !arch_clkdm->clkdm_clk_disable) >>>> + return -EINVAL; >>>> + >>>> + >>>> + pwrdm_lock(clkdm->pwrdm.ptr); >>>> + clkdm->noidlecount++; >>>> + pwrdm_unlock(clkdm->pwrdm.ptr); >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +/* >>>> + * allow future hwauto for this clkdm >>>> + * It won't put clkdm into hwauto. use clkdm_hwmod_hwauto() for that. >>>> + */ >>>> +int clkdm_hwmod_allow_hwauto(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; >>>> + >>>> + if (!clkdm || !oh || !arch_clkdm || !arch_clkdm->clkdm_clk_disable) >>>> + return -EINVAL; >>>> + >>>> + >>>> + pwrdm_lock(clkdm->pwrdm.ptr); >>>> + >>>> + if (clkdm->noidlecount == 0) { >>>> + pwrdm_unlock(clkdm->pwrdm.ptr); >>>> + WARN_ON(1); /* underflow */ >>>> + return -ERANGE; >>>> + } >>>> + >>>> + clkdm->noidlecount--; >>>> + pwrdm_unlock(clkdm->pwrdm.ptr); >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +/* >>>> + * put clkdm in hwauto if we can. checks noidlecount to see if we can. >>>> + */ >>>> +int clkdm_hwmod_hwauto(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; >>>> + >>>> + if (!clkdm || !oh || !arch_clkdm || !arch_clkdm->clkdm_clk_disable) >>>> + return -EINVAL; >>>> + >>>> + >>>> + pwrdm_lock(clkdm->pwrdm.ptr); >>>> + if (clkdm->noidlecount == 0) >>>> + clkdm_allow_idle_nolock(clkdm); >>>> + >>>> + pwrdm_unlock(clkdm->pwrdm.ptr); >>>> + >>>> + return 0; >>>> +} >>>> + >>>> /** >>>> * clkdm_hwmod_enable - add an enabled downstream hwmod to this clkdm >>>> * @clkdm: struct clockdomain * >>>> diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h >>>> index 77bab5f..8c491be 100644 >>>> --- a/arch/arm/mach-omap2/clockdomain.h >>>> +++ b/arch/arm/mach-omap2/clockdomain.h >>>> @@ -114,6 +114,7 @@ struct omap_hwmod; >>>> * @wkdep_srcs: Clockdomains that can be told to wake this powerdomain up >>>> * @sleepdep_srcs: Clockdomains that can be told to keep this clkdm from inact >>>> * @usecount: Usecount tracking >>>> + * @noidlecount: Noidle count tracking. Domain won't be auto idled this is > 0. >>>> * @node: list_head to link all clockdomains together >>>> * >>>> * @prcm_partition should be a macro from mach-omap2/prcm44xx.h (OMAP4 only) >>>> @@ -138,6 +139,7 @@ struct clockdomain { >>>> struct clkdm_dep *wkdep_srcs; >>>> struct clkdm_dep *sleepdep_srcs; >>>> int usecount; >>>> + int noidlecount; >>>> struct list_head node; >>>> }; >>>> >>>> @@ -211,6 +213,9 @@ int clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk); >>>> int clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk); >>>> int clkdm_hwmod_enable(struct clockdomain *clkdm, struct omap_hwmod *oh); >>>> int clkdm_hwmod_disable(struct clockdomain *clkdm, struct omap_hwmod *oh); >>>> +int clkdm_hwmod_prevent_hwauto(struct clockdomain *clkdm, struct omap_hwmod *oh); >>>> +int clkdm_hwmod_allow_hwauto(struct clockdomain *clkdm, struct omap_hwmod *oh); >>>> +int clkdm_hwmod_hwauto(struct clockdomain *clkdm, struct omap_hwmod *oh); >>>> >>>> extern void __init omap242x_clockdomains_init(void); >>>> extern void __init omap243x_clockdomains_init(void); >>>> -- >>>> 2.1.4 >>>> >>> >>> >>> - Paul >>> >> -- >> 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 >> > -- > 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 > -- 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
Paul, On 16/07/15 16:56, Roger Quadros wrote: > On 16/07/15 04:25, Paul Walmsley wrote: >> Hi >> >> On Tue, 23 Jun 2015, Roger Quadros wrote: >> >>> For some hwmods (e.g. DCAN on DRA7) we need the possibility to >>> disable HW_AUTO for the clockdomain while the module is active. >>> >>> To achieve this there needs to be a refcounting mechanism to >>> indicate whether any module in the clockdomain has requested >>> to disable HW_AUTO. We keep track of this in 'noidlecount'. >>> >>> Hwmod code must use clkdm_hwmod_prevent_hwauto() to prevent >>> HW_AUTO of the clockdomain in the future clkdm_hwmod_hwauto() calls. >>> >>> It must use clkdm_hwmod_allow_hwauto() to allow HW_AUTO in >>> the future clkdm_hwmod_hwauto() calls. >>> >>> Hwmod code must use clkdm_hwmod_allow_hwauto() whenever it needs >>> to request HW_AUTO of any clockdomain. (Typically after it has >>> enabled the module). >> >> How about just modifying clkdm_{allow,deny}_idle*() to do the >> idle-block-counting? The default state would be to allow idle, assuming >> that the clockdomain flags support that state, and then clkdm_deny_idle*() >> would increment the idle-blocking count, clkdm_allow_idle*() would >> decrement it. Then on the 0 -> 1 or 1 -> 0 transitions, the hardware >> would be reprogrammed appropiately. > > That is not possible with current hwmod code as clkdm_allow_idle() and clkdm_deny_idle() > are not symmetrically placed. > > The usual flow is > clkdm_enable_hwmod(); > if (hwsup) > clkdm_allow_idle(); > > This is mainly because it is redundant to disable auto idle before enableing > (SW_WKUP) the clockdomain. > > If we take your proposal above then we have to modify all users like so. > > if (hwsup) > clkdm_deny_idle(); > clkdm_enable_hwmod(); > if (hwsup) > clkdm_allow_idle(); > > Is this really what we want? Need your view on this before I can re-spin this series. Thanks. cheers, -roger > >> >> Anyway, seems like that would avoid races with any >> clkdm_{allow,deny}_idle*() users. >> >> A few other comments below: >> >> >>> >>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>> --- >>> arch/arm/mach-omap2/clockdomain.c | 71 +++++++++++++++++++++++++++++++++++++++ >>> arch/arm/mach-omap2/clockdomain.h | 5 +++ >>> 2 files changed, 76 insertions(+) >>> >>> diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c >>> index 2da3b5e..a7190d2 100644 >>> --- a/arch/arm/mach-omap2/clockdomain.c >>> +++ b/arch/arm/mach-omap2/clockdomain.c >>> @@ -1212,6 +1212,77 @@ ccd_exit: >>> return 0; >>> } >>> >>> +/* >>> + * prevent future hwauto for this clkdm. If clkdm->usecount becomes hwauto isn't prevented. >>> + * It will only prevnt future hwauto but not bring it out of hwauto. >>> + */ >> >> If you modify clkdm_{allow,deny}_idle*(), this shouldn't be a major issue >> - but all of the function comments should be fixed so that they are >> understandable and follow kernel-doc-nano specs. > > OK for updating the comments. > > > cheers, > -roger > >> >>> +int clkdm_hwmod_prevent_hwauto(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; >>> + >>> + if (!clkdm || !oh || !arch_clkdm || !arch_clkdm->clkdm_clk_disable) >>> + return -EINVAL; >>> + >>> + >>> + pwrdm_lock(clkdm->pwrdm.ptr); >>> + clkdm->noidlecount++; >>> + pwrdm_unlock(clkdm->pwrdm.ptr); >>> + >>> + return 0; >>> +} >>> + >>> +/* >>> + * allow future hwauto for this clkdm >>> + * It won't put clkdm into hwauto. use clkdm_hwmod_hwauto() for that. >>> + */ >>> +int clkdm_hwmod_allow_hwauto(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; >>> + >>> + if (!clkdm || !oh || !arch_clkdm || !arch_clkdm->clkdm_clk_disable) >>> + return -EINVAL; >>> + >>> + >>> + pwrdm_lock(clkdm->pwrdm.ptr); >>> + >>> + if (clkdm->noidlecount == 0) { >>> + pwrdm_unlock(clkdm->pwrdm.ptr); >>> + WARN_ON(1); /* underflow */ >>> + return -ERANGE; >>> + } >>> + >>> + clkdm->noidlecount--; >>> + pwrdm_unlock(clkdm->pwrdm.ptr); >>> + >>> + return 0; >>> +} >>> + >>> +/* >>> + * put clkdm in hwauto if we can. checks noidlecount to see if we can. >>> + */ >>> +int clkdm_hwmod_hwauto(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; >>> + >>> + if (!clkdm || !oh || !arch_clkdm || !arch_clkdm->clkdm_clk_disable) >>> + return -EINVAL; >>> + >>> + >>> + pwrdm_lock(clkdm->pwrdm.ptr); >>> + if (clkdm->noidlecount == 0) >>> + clkdm_allow_idle_nolock(clkdm); >>> + >>> + pwrdm_unlock(clkdm->pwrdm.ptr); >>> + >>> + return 0; >>> +} >>> + >>> /** >>> * clkdm_hwmod_enable - add an enabled downstream hwmod to this clkdm >>> * @clkdm: struct clockdomain * >>> diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h >>> index 77bab5f..8c491be 100644 >>> --- a/arch/arm/mach-omap2/clockdomain.h >>> +++ b/arch/arm/mach-omap2/clockdomain.h >>> @@ -114,6 +114,7 @@ struct omap_hwmod; >>> * @wkdep_srcs: Clockdomains that can be told to wake this powerdomain up >>> * @sleepdep_srcs: Clockdomains that can be told to keep this clkdm from inact >>> * @usecount: Usecount tracking >>> + * @noidlecount: Noidle count tracking. Domain won't be auto idled this is > 0. >>> * @node: list_head to link all clockdomains together >>> * >>> * @prcm_partition should be a macro from mach-omap2/prcm44xx.h (OMAP4 only) >>> @@ -138,6 +139,7 @@ struct clockdomain { >>> struct clkdm_dep *wkdep_srcs; >>> struct clkdm_dep *sleepdep_srcs; >>> int usecount; >>> + int noidlecount; >>> struct list_head node; >>> }; >>> >>> @@ -211,6 +213,9 @@ int clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk); >>> int clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk); >>> int clkdm_hwmod_enable(struct clockdomain *clkdm, struct omap_hwmod *oh); >>> int clkdm_hwmod_disable(struct clockdomain *clkdm, struct omap_hwmod *oh); >>> +int clkdm_hwmod_prevent_hwauto(struct clockdomain *clkdm, struct omap_hwmod *oh); >>> +int clkdm_hwmod_allow_hwauto(struct clockdomain *clkdm, struct omap_hwmod *oh); >>> +int clkdm_hwmod_hwauto(struct clockdomain *clkdm, struct omap_hwmod *oh); >>> >>> extern void __init omap242x_clockdomains_init(void); >>> extern void __init omap243x_clockdomains_init(void); >>> -- >>> 2.1.4 >>> >> >> >> - Paul >> > -- > 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 > -- 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, On 03/09/15 10:39, Roger Quadros wrote: > On 28/07/15 14:34, Roger Quadros wrote: >> Paul, >> >> On 16/07/15 16:56, Roger Quadros wrote: >>> On 16/07/15 04:25, Paul Walmsley wrote: >>>> Hi >>>> >>>> On Tue, 23 Jun 2015, Roger Quadros wrote: >>>> >>>>> For some hwmods (e.g. DCAN on DRA7) we need the possibility to >>>>> disable HW_AUTO for the clockdomain while the module is active. >>>>> >>>>> To achieve this there needs to be a refcounting mechanism to >>>>> indicate whether any module in the clockdomain has requested >>>>> to disable HW_AUTO. We keep track of this in 'noidlecount'. >>>>> >>>>> Hwmod code must use clkdm_hwmod_prevent_hwauto() to prevent >>>>> HW_AUTO of the clockdomain in the future clkdm_hwmod_hwauto() calls. >>>>> >>>>> It must use clkdm_hwmod_allow_hwauto() to allow HW_AUTO in >>>>> the future clkdm_hwmod_hwauto() calls. >>>>> >>>>> Hwmod code must use clkdm_hwmod_allow_hwauto() whenever it needs >>>>> to request HW_AUTO of any clockdomain. (Typically after it has >>>>> enabled the module). >>>> >>>> How about just modifying clkdm_{allow,deny}_idle*() to do the >>>> idle-block-counting? The default state would be to allow idle, assuming >>>> that the clockdomain flags support that state, and then clkdm_deny_idle*() >>>> would increment the idle-blocking count, clkdm_allow_idle*() would >>>> decrement it. Then on the 0 -> 1 or 1 -> 0 transitions, the hardware >>>> would be reprogrammed appropiately. >>> >>> That is not possible with current hwmod code as clkdm_allow_idle() and clkdm_deny_idle() >>> are not symmetrically placed. >>> >>> The usual flow is >>> clkdm_enable_hwmod(); >>> if (hwsup) >>> clkdm_allow_idle(); >>> >>> This is mainly because it is redundant to disable auto idle before enableing >>> (SW_WKUP) the clockdomain. >>> >>> If we take your proposal above then we have to modify all users like so. >>> >>> if (hwsup) >>> clkdm_deny_idle(); >>> clkdm_enable_hwmod(); >>> if (hwsup) >>> clkdm_allow_idle(); >>> >>> Is this really what we want? >> >> Any comments on this? > > Paul. I'm waiting on your input to rework this series if needed. > Early input would be much appreciated. Thanks. Not sure if Paul is receiving my e-mails but I'd like others to give their opinion on how we can proceed with this. Thanks. cheers, -roger > >> >>> >>>> >>>> Anyway, seems like that would avoid races with any >>>> clkdm_{allow,deny}_idle*() users. >>>> >>>> A few other comments below: >>>> >>>> >>>>> >>>>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>>>> --- >>>>> arch/arm/mach-omap2/clockdomain.c | 71 +++++++++++++++++++++++++++++++++++++++ >>>>> arch/arm/mach-omap2/clockdomain.h | 5 +++ >>>>> 2 files changed, 76 insertions(+) >>>>> >>>>> diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c >>>>> index 2da3b5e..a7190d2 100644 >>>>> --- a/arch/arm/mach-omap2/clockdomain.c >>>>> +++ b/arch/arm/mach-omap2/clockdomain.c >>>>> @@ -1212,6 +1212,77 @@ ccd_exit: >>>>> return 0; >>>>> } >>>>> >>>>> +/* >>>>> + * prevent future hwauto for this clkdm. If clkdm->usecount becomes hwauto isn't prevented. >>>>> + * It will only prevnt future hwauto but not bring it out of hwauto. >>>>> + */ >>>> >>>> If you modify clkdm_{allow,deny}_idle*(), this shouldn't be a major issue >>>> - but all of the function comments should be fixed so that they are >>>> understandable and follow kernel-doc-nano specs. >>> >>> OK for updating the comments. >>> >>> >>> cheers, >>> -roger >>> >>>> >>>>> +int clkdm_hwmod_prevent_hwauto(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; >>>>> + >>>>> + if (!clkdm || !oh || !arch_clkdm || !arch_clkdm->clkdm_clk_disable) >>>>> + return -EINVAL; >>>>> + >>>>> + >>>>> + pwrdm_lock(clkdm->pwrdm.ptr); >>>>> + clkdm->noidlecount++; >>>>> + pwrdm_unlock(clkdm->pwrdm.ptr); >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +/* >>>>> + * allow future hwauto for this clkdm >>>>> + * It won't put clkdm into hwauto. use clkdm_hwmod_hwauto() for that. >>>>> + */ >>>>> +int clkdm_hwmod_allow_hwauto(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; >>>>> + >>>>> + if (!clkdm || !oh || !arch_clkdm || !arch_clkdm->clkdm_clk_disable) >>>>> + return -EINVAL; >>>>> + >>>>> + >>>>> + pwrdm_lock(clkdm->pwrdm.ptr); >>>>> + >>>>> + if (clkdm->noidlecount == 0) { >>>>> + pwrdm_unlock(clkdm->pwrdm.ptr); >>>>> + WARN_ON(1); /* underflow */ >>>>> + return -ERANGE; >>>>> + } >>>>> + >>>>> + clkdm->noidlecount--; >>>>> + pwrdm_unlock(clkdm->pwrdm.ptr); >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +/* >>>>> + * put clkdm in hwauto if we can. checks noidlecount to see if we can. >>>>> + */ >>>>> +int clkdm_hwmod_hwauto(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; >>>>> + >>>>> + if (!clkdm || !oh || !arch_clkdm || !arch_clkdm->clkdm_clk_disable) >>>>> + return -EINVAL; >>>>> + >>>>> + >>>>> + pwrdm_lock(clkdm->pwrdm.ptr); >>>>> + if (clkdm->noidlecount == 0) >>>>> + clkdm_allow_idle_nolock(clkdm); >>>>> + >>>>> + pwrdm_unlock(clkdm->pwrdm.ptr); >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> /** >>>>> * clkdm_hwmod_enable - add an enabled downstream hwmod to this clkdm >>>>> * @clkdm: struct clockdomain * >>>>> diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h >>>>> index 77bab5f..8c491be 100644 >>>>> --- a/arch/arm/mach-omap2/clockdomain.h >>>>> +++ b/arch/arm/mach-omap2/clockdomain.h >>>>> @@ -114,6 +114,7 @@ struct omap_hwmod; >>>>> * @wkdep_srcs: Clockdomains that can be told to wake this powerdomain up >>>>> * @sleepdep_srcs: Clockdomains that can be told to keep this clkdm from inact >>>>> * @usecount: Usecount tracking >>>>> + * @noidlecount: Noidle count tracking. Domain won't be auto idled this is > 0. >>>>> * @node: list_head to link all clockdomains together >>>>> * >>>>> * @prcm_partition should be a macro from mach-omap2/prcm44xx.h (OMAP4 only) >>>>> @@ -138,6 +139,7 @@ struct clockdomain { >>>>> struct clkdm_dep *wkdep_srcs; >>>>> struct clkdm_dep *sleepdep_srcs; >>>>> int usecount; >>>>> + int noidlecount; >>>>> struct list_head node; >>>>> }; >>>>> >>>>> @@ -211,6 +213,9 @@ int clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk); >>>>> int clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk); >>>>> int clkdm_hwmod_enable(struct clockdomain *clkdm, struct omap_hwmod *oh); >>>>> int clkdm_hwmod_disable(struct clockdomain *clkdm, struct omap_hwmod *oh); >>>>> +int clkdm_hwmod_prevent_hwauto(struct clockdomain *clkdm, struct omap_hwmod *oh); >>>>> +int clkdm_hwmod_allow_hwauto(struct clockdomain *clkdm, struct omap_hwmod *oh); >>>>> +int clkdm_hwmod_hwauto(struct clockdomain *clkdm, struct omap_hwmod *oh); >>>>> >>>>> extern void __init omap242x_clockdomains_init(void); >>>>> extern void __init omap243x_clockdomains_init(void); >>>>> -- >>>>> 2.1.4 >>>>> >>>> >>>> >>>> - Paul >>>> >>> -- >>> 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 >>> >> -- >> 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 >> > -- > 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 > -- 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
* Roger Quadros <rogerq@ti.com> [151125 02:52]: > > On 03/09/15 10:39, Roger Quadros wrote: > > On 28/07/15 14:34, Roger Quadros wrote: > >> Paul, > >> > >> On 16/07/15 16:56, Roger Quadros wrote: > >>> On 16/07/15 04:25, Paul Walmsley wrote: > >>>> Hi > >>>> > >>>> On Tue, 23 Jun 2015, Roger Quadros wrote: > >>>> > >>>>> For some hwmods (e.g. DCAN on DRA7) we need the possibility to > >>>>> disable HW_AUTO for the clockdomain while the module is active. > >>>>> > >>>>> To achieve this there needs to be a refcounting mechanism to > >>>>> indicate whether any module in the clockdomain has requested > >>>>> to disable HW_AUTO. We keep track of this in 'noidlecount'. > >>>>> > >>>>> Hwmod code must use clkdm_hwmod_prevent_hwauto() to prevent > >>>>> HW_AUTO of the clockdomain in the future clkdm_hwmod_hwauto() calls. > >>>>> > >>>>> It must use clkdm_hwmod_allow_hwauto() to allow HW_AUTO in > >>>>> the future clkdm_hwmod_hwauto() calls. > >>>>> > >>>>> Hwmod code must use clkdm_hwmod_allow_hwauto() whenever it needs > >>>>> to request HW_AUTO of any clockdomain. (Typically after it has > >>>>> enabled the module). > >>>> > >>>> How about just modifying clkdm_{allow,deny}_idle*() to do the > >>>> idle-block-counting? The default state would be to allow idle, assuming > >>>> that the clockdomain flags support that state, and then clkdm_deny_idle*() > >>>> would increment the idle-blocking count, clkdm_allow_idle*() would > >>>> decrement it. Then on the 0 -> 1 or 1 -> 0 transitions, the hardware > >>>> would be reprogrammed appropiately. > >>> > >>> That is not possible with current hwmod code as clkdm_allow_idle() and clkdm_deny_idle() > >>> are not symmetrically placed. > >>> > >>> The usual flow is > >>> clkdm_enable_hwmod(); > >>> if (hwsup) > >>> clkdm_allow_idle(); > >>> > >>> This is mainly because it is redundant to disable auto idle before enableing > >>> (SW_WKUP) the clockdomain. > >>> > >>> If we take your proposal above then we have to modify all users like so. > >>> > >>> if (hwsup) > >>> clkdm_deny_idle(); > >>> clkdm_enable_hwmod(); > >>> if (hwsup) > >>> clkdm_allow_idle(); > >>> > >>> Is this really what we want? > >> > >> Any comments on this? > > > > Paul. I'm waiting on your input to rework this series if needed. > > Early input would be much appreciated. Thanks. > > Not sure if Paul is receiving my e-mails but I'd like > others to give their opinion on how we can proceed with this. Thanks. Well in the long run we want to have a proper bus driver for each interconnect instance and use genpd. I'm afraid that solution is not going to help immediately though. Regards, Tony -- 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 2da3b5e..a7190d2 100644 --- a/arch/arm/mach-omap2/clockdomain.c +++ b/arch/arm/mach-omap2/clockdomain.c @@ -1212,6 +1212,77 @@ ccd_exit: return 0; } +/* + * prevent future hwauto for this clkdm. If clkdm->usecount becomes hwauto isn't prevented. + * It will only prevnt future hwauto but not bring it out of hwauto. + */ +int clkdm_hwmod_prevent_hwauto(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; + + if (!clkdm || !oh || !arch_clkdm || !arch_clkdm->clkdm_clk_disable) + return -EINVAL; + + + pwrdm_lock(clkdm->pwrdm.ptr); + clkdm->noidlecount++; + pwrdm_unlock(clkdm->pwrdm.ptr); + + return 0; +} + +/* + * allow future hwauto for this clkdm + * It won't put clkdm into hwauto. use clkdm_hwmod_hwauto() for that. + */ +int clkdm_hwmod_allow_hwauto(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; + + if (!clkdm || !oh || !arch_clkdm || !arch_clkdm->clkdm_clk_disable) + return -EINVAL; + + + pwrdm_lock(clkdm->pwrdm.ptr); + + if (clkdm->noidlecount == 0) { + pwrdm_unlock(clkdm->pwrdm.ptr); + WARN_ON(1); /* underflow */ + return -ERANGE; + } + + clkdm->noidlecount--; + pwrdm_unlock(clkdm->pwrdm.ptr); + + return 0; +} + +/* + * put clkdm in hwauto if we can. checks noidlecount to see if we can. + */ +int clkdm_hwmod_hwauto(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; + + if (!clkdm || !oh || !arch_clkdm || !arch_clkdm->clkdm_clk_disable) + return -EINVAL; + + + pwrdm_lock(clkdm->pwrdm.ptr); + if (clkdm->noidlecount == 0) + clkdm_allow_idle_nolock(clkdm); + + pwrdm_unlock(clkdm->pwrdm.ptr); + + return 0; +} + /** * clkdm_hwmod_enable - add an enabled downstream hwmod to this clkdm * @clkdm: struct clockdomain * diff --git a/arch/arm/mach-omap2/clockdomain.h b/arch/arm/mach-omap2/clockdomain.h index 77bab5f..8c491be 100644 --- a/arch/arm/mach-omap2/clockdomain.h +++ b/arch/arm/mach-omap2/clockdomain.h @@ -114,6 +114,7 @@ struct omap_hwmod; * @wkdep_srcs: Clockdomains that can be told to wake this powerdomain up * @sleepdep_srcs: Clockdomains that can be told to keep this clkdm from inact * @usecount: Usecount tracking + * @noidlecount: Noidle count tracking. Domain won't be auto idled this is > 0. * @node: list_head to link all clockdomains together * * @prcm_partition should be a macro from mach-omap2/prcm44xx.h (OMAP4 only) @@ -138,6 +139,7 @@ struct clockdomain { struct clkdm_dep *wkdep_srcs; struct clkdm_dep *sleepdep_srcs; int usecount; + int noidlecount; struct list_head node; }; @@ -211,6 +213,9 @@ int clkdm_clk_enable(struct clockdomain *clkdm, struct clk *clk); int clkdm_clk_disable(struct clockdomain *clkdm, struct clk *clk); int clkdm_hwmod_enable(struct clockdomain *clkdm, struct omap_hwmod *oh); int clkdm_hwmod_disable(struct clockdomain *clkdm, struct omap_hwmod *oh); +int clkdm_hwmod_prevent_hwauto(struct clockdomain *clkdm, struct omap_hwmod *oh); +int clkdm_hwmod_allow_hwauto(struct clockdomain *clkdm, struct omap_hwmod *oh); +int clkdm_hwmod_hwauto(struct clockdomain *clkdm, struct omap_hwmod *oh); extern void __init omap242x_clockdomains_init(void); extern void __init omap243x_clockdomains_init(void);
For some hwmods (e.g. DCAN on DRA7) we need the possibility to disable HW_AUTO for the clockdomain while the module is active. To achieve this there needs to be a refcounting mechanism to indicate whether any module in the clockdomain has requested to disable HW_AUTO. We keep track of this in 'noidlecount'. Hwmod code must use clkdm_hwmod_prevent_hwauto() to prevent HW_AUTO of the clockdomain in the future clkdm_hwmod_hwauto() calls. It must use clkdm_hwmod_allow_hwauto() to allow HW_AUTO in the future clkdm_hwmod_hwauto() calls. Hwmod code must use clkdm_hwmod_allow_hwauto() whenever it needs to request HW_AUTO of any clockdomain. (Typically after it has enabled the module). Signed-off-by: Roger Quadros <rogerq@ti.com> --- arch/arm/mach-omap2/clockdomain.c | 71 +++++++++++++++++++++++++++++++++++++++ arch/arm/mach-omap2/clockdomain.h | 5 +++ 2 files changed, 76 insertions(+)