Re: [PATCH 5/10 -tip] x86: check_powernow() for K8 and later user of Advanced Power Management features

Ingo Molnar
2009-05-12T17:50:33Z

* Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:

> - eax = cpuid_eax(CPUID_GET_MAX_CAPABILITIES);
> - if (eax < CPUID_FREQ_VOLT_CAPABILITIES) {
> + /* Advanced Power Management capabilities */
> + if (c->x86_capability[9]) {
> printk(KERN_INFO PFX
> "No frequency change capabilities detected\n");
> goto out;
> }

How is the new check equivalent to the old one? It isnt and this is
a bug.

Also, open-coding x86_capability[9] like that is quite unclean. Were
we ever to reorder those bits internally, this could would break.

But i see what you are trying to do. A better method might be to add
a new helper:

+#define X86_FEATURE_TS (9*32+ 0) /* Temperatue sensor */
+#define X86_FEATURE_FID (9*32+ 1) /* Frequency ID control */
+#define X86_FEATURE_VID (9*32+ 2) /* Voltage ID control */
+#define X86_FEATURE_TTP (9*32+ 3) /* Thermal trip */
+#define X86_FEATURE_HTC (9*32+ 4) /* Hardware thermal control */
+#define X86_FEATURE_STC (9*32+ 5) /* Software thermal control */
+#define X86_FEATURE_100MHZSTEPS (9*32+ 6) /* 100 MHz multiplier control */
+#define X86_FEATURE_HWPSTATE (9*32+ 7) /* Hardware P-State control */
+#define X86_FEATURE_CONSTANT_TSC (9*32+ 8) /* Constant rate TSC ticks */

... to represent the 'is any of these set' property.

[ btw., there's a typo in the X86_FEATURE_TS comment above ]

Ingo