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

[Ingo Molnar]

* Jaswinder Singh Rajput <jaswinder@xxxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/