Re: [RFC][PATCH 1/7] ARM: OMAP4: cpuidle - Remove unused valid field

2012-03-21 Thread Jean Pihet 
On Wed, Mar 21, 2012 at 11:03 AM, Santosh Shilimkar
 wrote:
> On Wednesday 21 March 2012 03:16 PM, Daniel Lezcano wrote:
>> On 03/21/2012 10:41 AM, Shilimkar, Santosh wrote:
>>> On Wed, Mar 21, 2012 at 2:57 PM, Daniel Lezcano
>>>   wrote:
 The 'valid' field is never used in the code, let's remove it.

 Signed-off-by: Daniel Lezcano
 ---
>>> It is used during the registration. This field has been very useful for
>>> debug when need to disable a C-state etc.
>>> So unless and until there is a strong reason, i would like to retain it.
>>
>> IMO if it used for debug purpose, it should be moved to the debug code
>> and if the debug code is not upstream, then that 'valid' should not be
>> here but in the out-of-tree code.
>>
> When I said debug, I mean CPUIDLE debug and not any special debug code.
>
>> By the way, this may be a debate for nothing because a patchset is on
>> the way to disable C-states from sysfs.
>>
> I see but sysfs won't solve that because you want to disable certain
> C-state so that CPUIDLE driver don't use that state.
This will solve the problem, the only difference is that you need the
user space to switch the disable knob from sysfs.
>From the kernel space, for debug, you can set the .disable value to 1
in the cpuidle_driver->states struct.

>
> Let say if the C4 which is OSWR is broken. In such cases, just
> setting valid flag let you disable it.
>
> Again I don't have strong objection to this change.
>
> Regards
> santosh

Regards,
Jean

>
> ___
> linaro-dev mailing list
> linaro-...@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC][PATCH 1/7] ARM: OMAP4: cpuidle - Remove unused valid field

2012-03-21 Thread Santosh Shilimkar 
On Wednesday 21 March 2012 03:16 PM, Daniel Lezcano wrote:
> On 03/21/2012 10:41 AM, Shilimkar, Santosh wrote:
>> On Wed, Mar 21, 2012 at 2:57 PM, Daniel Lezcano
>>   wrote:
>>> The 'valid' field is never used in the code, let's remove it.
>>>
>>> Signed-off-by: Daniel Lezcano
>>> ---
>> It is used during the registration. This field has been very useful for
>> debug when need to disable a C-state etc.
>> So unless and until there is a strong reason, i would like to retain it.
> 
> IMO if it used for debug purpose, it should be moved to the debug code
> and if the debug code is not upstream, then that 'valid' should not be
> here but in the out-of-tree code.
>
When I said debug, I mean CPUIDLE debug and not any special debug code.

> By the way, this may be a debate for nothing because a patchset is on
> the way to disable C-states from sysfs.
> 
I see but sysfs won't solve that because you want to disable certain
C-state so that CPUIDLE driver don't use that state.

Let say if the C4 which is OSWR is broken. In such cases, just
setting valid flag let you disable it.

Again I don't have strong objection to this change.

Regards
santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC][PATCH 1/7] ARM: OMAP4: cpuidle - Remove unused valid field

2012-03-21 Thread Daniel Lezcano 

On 03/21/2012 10:41 AM, Shilimkar, Santosh wrote:

On Wed, Mar 21, 2012 at 2:57 PM, Daniel Lezcano
  wrote:

The 'valid' field is never used in the code, let's remove it.

Signed-off-by: Daniel Lezcano
---

It is used during the registration. This field has been very useful for
debug when need to disable a C-state etc.
So unless and until there is a strong reason, i would like to retain it.


IMO if it used for debug purpose, it should be moved to the debug code 
and if the debug code is not upstream, then that 'valid' should not be 
here but in the out-of-tree code.


By the way, this may be a debate for nothing because a patchset is on 
the way to disable C-states from sysfs.


--
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC][PATCH 1/7] ARM: OMAP4: cpuidle - Remove unused valid field

2012-03-21 Thread Shilimkar, Santosh 
On Wed, Mar 21, 2012 at 2:57 PM, Daniel Lezcano
 wrote:
> The 'valid' field is never used in the code, let's remove it.
>
> Signed-off-by: Daniel Lezcano 
> ---
It is used during the registration. This field has been very useful for
debug when need to disable a C-state etc.
So unless and until there is a strong reason, i would like to retain it.

Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC][PATCH 1/7] ARM: OMAP4: cpuidle - Remove unused valid field

2012-03-21 Thread Daniel Lezcano 
The 'valid' field is never used in the code, let's remove it.

Signed-off-by: Daniel Lezcano 
---
 arch/arm/mach-omap2/cpuidle44xx.c |9 +++--
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle44xx.c 
b/arch/arm/mach-omap2/cpuidle44xx.c
index cfdbb86..1210229 100644
--- a/arch/arm/mach-omap2/cpuidle44xx.c
+++ b/arch/arm/mach-omap2/cpuidle44xx.c
@@ -29,16 +29,15 @@ struct omap4_idle_statedata {
u32 cpu_state;
u32 mpu_logic_state;
u32 mpu_state;
-   u8 valid;
 };
 
 static struct cpuidle_params cpuidle_params_table[] = {
/* C1 - CPU0 ON + CPU1 ON + MPU ON */
-   {.exit_latency = 2 + 2 , .target_residency = 5, .valid = 1},
+   {.exit_latency = 2 + 2 , .target_residency = 5 },
/* C2- CPU0 OFF + CPU1 OFF + MPU CSWR */
-   {.exit_latency = 328 + 440 , .target_residency = 960, .valid = 1},
+   {.exit_latency = 328 + 440 , .target_residency = 960 },
/* C3 - CPU0 OFF + CPU1 OFF + MPU OSWR */
-   {.exit_latency = 460 + 518 , .target_residency = 1100, .valid = 1},
+   {.exit_latency = 460 + 518 , .target_residency = 1100 },
 };
 
 #define OMAP4_NUM_STATES ARRAY_SIZE(cpuidle_params_table)
@@ -171,7 +170,6 @@ static inline struct omap4_idle_statedata 
*_fill_cstate_usage(
struct omap4_idle_statedata *cx = &omap4_idle_data[idx];
struct cpuidle_state_usage *state_usage = &dev->states_usage[idx];
 
-   cx->valid   = cpuidle_params_table[idx].valid;
cpuidle_set_statedata(state_usage, cx);
 
return cx;
@@ -207,7 +205,6 @@ int __init omap4_idle_init(void)
_fill_cstate(drv, 0, "MPUSS ON");
drv->safe_state_index = 0;
cx = _fill_cstate_usage(dev, 0);
-   cx->valid = 1;  /* C1 is always valid */
cx->cpu_state = PWRDM_POWER_ON;
cx->mpu_state = PWRDM_POWER_ON;
cx->mpu_logic_state = PWRDM_POWER_RET;
-- 
1.7.5.4

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html