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@xxxxxx>
>> ---
>> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
