Cleanups for powernow-k8
Hi!
powernow-k8 uses strange kind of comments, and is way too verbose. Also powernow-k7 should just shut up when it is monolithic kernel and cpu is not k7.
[Oh and look at this:
@@ -637,6 +629,7 @@
}
if ((numps <= 1) || (batps <= 1)) {
+ /* FIXME: Is this right? I can see one state on battery, two states total as an usefull config */
printk(KERN_ERR PFX "only 1 p-state to transition\n"); return -ENODEV;
}
the test probably should be numps <= 1 only, but it does not matter as we force numps = batps]
Please apply,
Pavel
arch/i386/kernel/cpu/cpufreq/powernow-k7.c renamed from linux/arch/i386/kernel/cpu/cpufreq/powernow-k7.c +3 -8
@@ -91,18 +91,13 @@struct cpuinfo_x86 *c = cpu_data;unsigned int maxei, eax, ebx, ecx, edx; -if (c->x86_vendor !=X86_VENDOR_AMD ) { - printk (KERN_INFO PFX "AMD processor not detected.\n"); -return 0 ; - } - -if (c->x86 !=6 ) { +if ((c->x86_vendor !=X86_VENDOR_AMD ) || (c->x86 !=6 )) { +#ifdef MODULE printk (KERN_INFO PFX "This module only works with AMD K7 CPUs\n"); +#endif return 0 ; } - printk (KERN_INFO PFX "AMD K7 CPU detected.\n"); -if ((c->x86_model ==6 ) && (c->x86_mask ==0 )) { printk (KERN_INFO PFX "K7 660[A0] core detected, enabling errata workarounds\n"); have_a0 =1 ;
arch/i386/kernel/cpu/cpufreq/powernow-k8.c renamed from linux/arch/i386/kernel/cpu/cpufreq/powernow-k8.c +41 -49
170 lines not on this page · raw message
Signature · 2 lines
When do you have a heart between your knees? [Johanka's followup: and *two* hearts?]
Quoted 1 line · Pavel Machek
powernow-k8 uses strange kind of comments
comments part I kinda agree with, though its not critical..
Quoted 1 line
and is way too verbose.
I agree that something like that output belongs more in x86info, or a standalone tool, but I think Paul wanted to keep debugging stuff there for the time being. Maybe silence it, and have it enabled with a 'debug' module param ? Paul ?
Quoted 2 lines
Also powernow-k7 should just shut up when it is monolithic kernel and cpu is not k7.
Agreed, applied.
Quoted 10 lines
(quoted diff) +1
-0@@ -637,6 +629,7 @@ } if ((numps <= 1) || (batps <= 1)) { + /* FIXME: Is this right? I can see one state on battery, two states total as an usefull config */ printk(KERN_ERR PFX "only 1 p-state to transition\n"); return -ENODEV; }the test probably should be numps <= 1 only, but it does not matter as we force numps = batps]
1 state on battery sounds odd. Buggy BIOS ?
Dave
Hi!
Quoted 6 lines
Quoted 1 line
and is way too verbose.
I agree that something like that output belongs more in x86info, or a standalone tool, but I think Paul wanted to keep debugging stuff there for the time being. Maybe silence it, and have it enabled with a 'debug' module param ? Paul ?
I made sure all the info is still there in dmesg, just on less lines.
Quoted 12 lines
Quoted 10 lines
(quoted diff) +1
-0@@ -637,6 +629,7 @@ } if ((numps <= 1) || (batps <= 1)) { + /* FIXME: Is this right? I can see one state on battery, two states total as an usefull config */ printk(KERN_ERR PFX "only 1 p-state to transition\n"); return -ENODEV; }the test probably should be numps <= 1 only, but it does not matter as we force numps = batps]
1 state on battery sounds odd. Buggy BIOS ?
No if you have 1 state on battery but two on AC power. That way you *have* to be low power on battery, but can select low or high on AC... Ugly hardware, but Intel done this before, and it kind-of makes sense.
Pavel
Signature · 1 line
When do you have heart between your knees?