On 08/10/2012 07:17 PM, Lorenzo Pieralisi wrote:
Hi Daniel,
thanks for this patchset.
On Wed, Jul 25, 2012 at 11:46:02AM +0100, Daniel Lezcano wrote:
The tegra3 and big.LITTLE architecture have different cpu latencies. This API allows to specify a different cpu latency for a specific cpu.
With the previous patches, we use the per cpuidle device states pointer, this function overrides this pointer.
Signed-off-by: Daniel Lezcano daniel.lezcano@linaro.org
drivers/cpuidle/cpuidle.c | 17 +++++++++++++++++ include/linux/cpuidle.h | 10 +++++++--- 2 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c index 199878a..3b21b68 100644 --- a/drivers/cpuidle/cpuidle.c +++ b/drivers/cpuidle/cpuidle.c @@ -456,6 +456,23 @@ void cpuidle_unregister_device(struct cpuidle_device *dev) EXPORT_SYMBOL_GPL(cpuidle_unregister_device); +int cpuidle_register_states(struct cpuidle_device *dev,
struct cpuidle_state *states,int state_count)+{
- if (!dev || !states)
return -EINVAL;- if (state_count <= 0)
return -EINVAL;- dev->states = states;
- dev->state_count = state_count;
Is this function supposed to be called after cpuidle_device registration ? I think so since at registration time the dev->states pointers are all initialized to point to the driver state array, which is global and not really what we want.
Unless this function is called on the cpu that requires swapping the state pointer, I think it is unsafe to register a different state pointer without a minimal level of locking (or disabling idle and renabling idle) since the update of dev->states and dev->state_count is not atomic. Maybe it is implicit but it should be documented somehow to define cpuidle_register_states(...) proper usage.
Hi Lorenzo,
Yes, you are right. I will add the cpuidle lock.
Thanks !
-- Daniel