diff options
author | Mattia Dongili <malattia@linux.it> | 2006-07-05 17:12:20 -0400 |
---|---|---|
committer | Dave Jones <davej@redhat.com> | 2006-07-31 18:37:05 -0400 |
commit | 9c9a43ed2734081124407c779b36a4761c41139b (patch) | |
tree | b32e4d83e840c46f8ef760bda594d7a02e1c41c9 | |
parent | 49b1e3ea19b1c95c2f012b8331ffb3b169e4c042 (diff) |
[CPUFREQ] return error when failing to set minfreq
I just stumbled on this bug/feature, this is how to reproduce it:
# echo 450000 > /sys/devices/system/cpu/cpu0/cpufreq/scaling_min_freq
# echo 450000 > /sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
# echo powersave > /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor
# cpufreq-info -p
450000 450000 powersave
# echo 1800000 > /sys/devices/system/cpu/cpu0/cpufreq/scaling_min_freq ; echo $?
0
# cpufreq-info -p
450000 450000 powersave
Here it is. The kernel refuses to set a min_freq higher than the
max_freq but it allows a max_freq lower than min_freq (lowering min_freq
also).
This behaviour is pretty straightforward (but undocumented) and it
doesn't return an error altough failing to accomplish the requested
action (set min_freq).
The problem (IMO) is basically that userspace is not allowed to set a
full policy atomically while the kernel always does that thus it must
enforce an ordering on operations.
The attached patch returns -EINVAL if trying to increase frequencies
starting from scaling_min_freq and documents the correct ordering of writes.
Signed-off-by: Mattia Dongili <malattia@linux.it>
Signed-off-by: Dominik Brodowski <linux at dominikbrodowski.net>
Signed-off-by: Dave Jones <davej@redhat.com>
--
-rw-r--r-- | Documentation/cpu-freq/user-guide.txt | 5 | ||||
-rw-r--r-- | drivers/cpufreq/cpufreq.c | 5 |
2 files changed, 9 insertions, 1 deletions
diff --git a/Documentation/cpu-freq/user-guide.txt b/Documentation/cpu-freq/user-guide.txt index 7fedc00c3d30..555c8cf3650a 100644 --- a/Documentation/cpu-freq/user-guide.txt +++ b/Documentation/cpu-freq/user-guide.txt | |||
@@ -153,10 +153,13 @@ scaling_governor, and by "echoing" the name of another | |||
153 | that some governors won't load - they only | 153 | that some governors won't load - they only |
154 | work on some specific architectures or | 154 | work on some specific architectures or |
155 | processors. | 155 | processors. |
156 | scaling_min_freq and | 156 | scaling_min_freq and |
157 | scaling_max_freq show the current "policy limits" (in | 157 | scaling_max_freq show the current "policy limits" (in |
158 | kHz). By echoing new values into these | 158 | kHz). By echoing new values into these |
159 | files, you can change these limits. | 159 | files, you can change these limits. |
160 | NOTE: when setting a policy you need to | ||
161 | first set scaling_max_freq, then | ||
162 | scaling_min_freq. | ||
160 | 163 | ||
161 | 164 | ||
162 | If you have selected the "userspace" governor which allows you to | 165 | If you have selected the "userspace" governor which allows you to |
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index bc1088d9b379..ad996c772c8b 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c | |||
@@ -1343,6 +1343,11 @@ static int __cpufreq_set_policy(struct cpufreq_policy *data, struct cpufreq_poli | |||
1343 | 1343 | ||
1344 | memcpy(&policy->cpuinfo, &data->cpuinfo, sizeof(struct cpufreq_cpuinfo)); | 1344 | memcpy(&policy->cpuinfo, &data->cpuinfo, sizeof(struct cpufreq_cpuinfo)); |
1345 | 1345 | ||
1346 | if (policy->min > data->min && policy->min > policy->max) { | ||
1347 | ret = -EINVAL; | ||
1348 | goto error_out; | ||
1349 | } | ||
1350 | |||
1346 | /* verify the cpu speed can be set within this limit */ | 1351 | /* verify the cpu speed can be set within this limit */ |
1347 | ret = cpufreq_driver->verify(policy); | 1352 | ret = cpufreq_driver->verify(policy); |
1348 | if (ret) | 1353 | if (ret) |