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

Linus Torvalds
2009-06-08T18:36:17Z

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@viatech.com>
> ---
> 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@kernel.org' 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