[PATCHv4 6/8] ARM: OMAP4: PM: support ret_logic/mem_off_counters

[Rajendra Nayak]

On Tuesday 08 May 2012 02:39 PM, Tero Kristo wrote:
> On Tue, 2012-05-08 at 14:27 +0530, Rajendra Nayak wrote:
>> On Tuesday 08 May 2012 02:06 PM, Tero Kristo wrote:
>>> On Mon, 2012-05-07 at 17:19 -0700, Kevin Hilman wrote:
>>>> Tero Kristo<t-kristo at ti.com>   writes:
>>>>
>>>>> From: Axel Haslam<axelhaslam at gmail.com>
>>>>>
>>>>> On OMAP4, there is no support to read previous logic state
>>>>> or previous memory state achieved when a power domain transitions
>>>>> to RET. Instead there are module level context registers.
>>>>>
>>>>> In order to support the powerdomain level logic/mem_off_counters
>>>>> on OMAP4, instead use the previous power state achieved (RET) and
>>>>> the *programmed* logic/mem RET state to derive if a powerdomain lost
>>>>> logic or did not.
>>>>
>>>> OK, but this also changes the behavior for OMAP3 as well, right?  I
>>>> don't see in the changelog how this affects OMAP3 and whether it is
>>>> still expected to work on OMAP3.  When changing common code like this,
>>>> the changelog should describe the impacts on to all affected SoCs.
>>>>
>>>> As suggested by Vaibhav Bedia, now might be the right time to add this
>>>> function to the SoC specific function pointers (struct pwrdm_ops.)
>>>>
>>>> Doing that, the existing function could be used for OMAP3 (and OMAP4 if
>>>> the changelog describes that it can/should be used for both.)
>>>>
>>>> Then, when AM33xx support is added, it will be obvious where they will
>>>> need to plugin support for that SoC.
>>>
>>> How about the following patch? It will add a couple of redundant
>>> read_prev_pwrst calls, but works in the same way as the original patch,
>>> without touching the generic code. Also, as there have been talks about
>>> adding caching for some of the pwrdm registers (especially the
>>> prev_pwrst), this might not be that big of an issue.
>>>
>>> If this looks good to you, I'll re-post the set with this patch.
>>>
>>> -Tero
>>>
>>>
>>> diff --git a/arch/arm/mach-omap2/powerdomain44xx.c
>>> b/arch/arm/mach-omap2/powerdomain44xx.c
>>> index 9bfb8a0..3d5e8d4 100644
>>> --- a/arch/arm/mach-omap2/powerdomain44xx.c
>>> +++ b/arch/arm/mach-omap2/powerdomain44xx.c
>>> @@ -155,6 +155,14 @@ static int omap4_pwrdm_read_logic_retst(struct
>>> powerdomain *pwrdm)
>>>    	return v;
>>>    }
>>>
>>> +static int omap4_pwrdm_read_prev_logic_pwrst(struct powerdomain *pwrdm)
>>> +{
>>> +	if (omap4_pwrdm_read_prev_pwrst(pwrdm) != PWRDM_POWER_RET)
>>> +		return PWRDM_POWER_ON;
>>> +
>>> +	return omap4_pwrdm_read_logic_retst(pwrdm);
>>
>> Looks good to me. Do these ever get called with target state programmed
>> to OFF?
>
> At least with current kernel code, no. But you are right, it might be
> better to change these to check against>  PWRDM_POWER_RET.

But in case of target power state being programmed to OFF, the logic
retst might not be programmed at all, so in case of target state being
OFF, returning the logic retst programmed might as well return
PWRDM_POWER_ON itself, right?

>
> -Tero
>
>>
>> regards,
>> Rajendra
>>
>>> +}
>>> +
>>>    static int omap4_pwrdm_read_mem_pwrst(struct powerdomain *pwrdm, u8
>>> bank)
>>>    {
>>>    	u32 m, v;
>>> @@ -183,6 +191,14 @@ static int omap4_pwrdm_read_mem_retst(struct
>>> powerdomain *pwrdm, u8 bank)
>>>    	return v;
>>>    }
>>>
>>> +static int omap4_pwrdm_read_prev_mem_pwrst(struct powerdomain *pwrdm,
>>> u8 bank)
>>> +{
>>> +	if (omap4_pwrdm_read_prev_pwrst(pwrdm) != PWRDM_POWER_RET)
>>> +		return PWRDM_POWER_ON;
>>> +
>>> +	return omap4_pwrdm_read_mem_retst(pwrdm, bank);
>>> +}
>>> +
>>>    static int omap4_pwrdm_wait_transition(struct powerdomain *pwrdm)
>>>    {
>>>    	u32 c = 0;
>>> @@ -256,9 +272,11 @@ struct pwrdm_ops omap4_pwrdm_operations = {
>>>    	.pwrdm_clear_all_prev_pwrst	= omap4_pwrdm_clear_all_prev_pwrst,
>>>    	.pwrdm_set_logic_retst	= omap4_pwrdm_set_logic_retst,
>>>    	.pwrdm_read_logic_pwrst	= omap4_pwrdm_read_logic_pwrst,
>>> +	.pwrdm_read_prev_logic_pwrst	= omap4_pwrdm_read_prev_logic_pwrst,
>>>    	.pwrdm_read_logic_retst	= omap4_pwrdm_read_logic_retst,
>>>    	.pwrdm_read_mem_pwrst	= omap4_pwrdm_read_mem_pwrst,
>>>    	.pwrdm_read_mem_retst	= omap4_pwrdm_read_mem_retst,
>>> +	.pwrdm_read_prev_mem_pwrst	= omap4_pwrdm_read_prev_mem_pwrst,
>>>    	.pwrdm_set_mem_onst	= omap4_pwrdm_set_mem_onst,
>>>    	.pwrdm_set_mem_retst	= omap4_pwrdm_set_mem_retst,
>>>    	.pwrdm_wait_transition	= omap4_pwrdm_wait_transition,
>>>
>>>
>>
>
>