Thread (6 messages) 6 messages, 5 authors, 2012-09-03

Re: [PATCH 5/5][RFC] cpuidle : add cpuidle_register_states function

From: Daniel Lezcano <hidden>
Date: 2012-08-31 21:21:16

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:
quoted
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 <redacted>
---
 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


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help