Re: [PATCH 1/3] ARM: OMAP2+: clockdomain: Add mechanism for disabling HW_AUTO

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@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
>>
> --
> 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
> 
--
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



[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux