Re: [PATCH 1/2] CPUFREQ: Enable acpi-cpufreq driver for VIA/CentaurCPUs

[Linus Torvalds]

On Mon, 8 Jun 2009, Harald Welte wrote:
>
> The VIA/Centaur C7, C7-M and Nano CPU's all support ACPI based cpu p-states
> using a MSR interface.  The Linux driver just never made use of it, since in
> addition to the check for the EST flag it also checked if the vendor is Intel.
> 
> Signed-off-by: Harald Welte <HaraldWelte@xxxxxxxxxxx>
> ---
>  arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> index 208ecf6..ee03585 100644
> --- a/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> +++ b/arch/x86/kernel/cpu/cpufreq/acpi-cpufreq.c
> @@ -90,7 +90,8 @@ static int check_est_cpu(unsigned int cpuid)
>  {
>  	struct cpuinfo_x86 *cpu = &cpu_data(cpuid);
>  
> -	if (cpu->x86_vendor != X86_VENDOR_INTEL ||
> +	if ((cpu->x86_vendor != X86_VENDOR_INTEL &&
> +	     cpu->x86_vendor != X86_VENDOR_CENTAUR) ||
>  	    !cpu_has(cpu, X86_FEATURE_EST))

Hmm. This all really should be just

	static int check_est_cpu(unsigned int cpuid)
	{
		struct cpuinfo_x86 *cpu = &cpu_data(cpuid);
		return cpu_has(cpu, X86_FEATURE_EST);
	}

I suspect, with no vendor tests. That's the whole _point_ of CPU features, 
after all.

If some vendor claims EST but doesn't actually support the EST interfaces, 
we should just have fixups to clear the bit in the per-vendor cpuinfo 
code, not in some random driver.

The only thing that makes me nervous about this is how close to 2.6.30 we 
are. I'd be happier if this was resolved by doing this as a patch 
post-2.6.30, and then adding 'stable@xxxxxxxxxx' as a Cc: tag, and 
backporting it to 2.6.30.1 if no problems appear. 

It's not like this is a regression, I think.

Does that sound like a reasonable plan?

		Linus
--
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/