Re: [PATCH -tip] x86: cpu/intel.c cleanup

[Sam Ravnborg <sam@ravnborg.org>]

>  
>  #include <asm/processor.h>
> +#include <asm/topology.h>
>  #include <asm/pgtable.h>
> -#include <asm/msr.h>
> -#include <asm/uaccess.h>
> -#include <asm/ds.h>
> +#ifdef CONFIG_X86_LOCAL_APIC
> +#include <asm/mpspec.h>
> +#include <asm/apic.h>
> +#endif

If these header files are only relevant for CONFIG_X86_LOCAL_APIC
then we should move the ifdef down into the header file,
so users do not forget it.

	Sam

>  static void __cpuinit intel_workarounds(struct cpuinfo_x86 *c)
>  {
> +#ifdef CONFIG_X86_32
>  	unsigned long lo, hi;
>  
>  #ifdef CONFIG_X86_F00F_BUG
>  	/*
> -	 * All current models of Pentium and Pentium with MMX technology CPUs
> -	 * have the F0 0F bug, which lets nonprivileged users lock up the system.
> +	 * All current models of Pentium and Pentium with MMX technology
> +	 * CPUs have the F0 0F bug, which lets nonprivileged users lock
> +	 * up the system.
>  	 * Note that the workaround only should be initialized once...
>  	 */
>  	c->f00f_bug = 0;
> @@ -184,7 +207,8 @@ static void __cpuinit intel_workarounds(struct cpuinfo_x86 *c)
>  		c->f00f_bug = 1;
>  		if (!f00f_workaround_enabled) {
>  			trap_init_f00f_bug();
> -			printk(KERN_NOTICE "Intel Pentium with F0 0F bug - workaround enabled.\n");
> +			printk(KERN_NOTICE "Intel Pentium with F0 0F bug - "
> +				"workaround enabled.\n");
>  			f00f_workaround_enabled = 1;
>  		}
>  	}
> @@ -194,7 +218,7 @@ static void __cpuinit intel_workarounds(struct cpuinfo_x86 *c)
>  	 * SEP CPUID bug: Pentium Pro reports SEP but doesn't have it until
>  	 * model 3 mask 3
>  	 */
> -	if ((c->x86<<8 | c->x86_model<<4 | c->x86_mask) < 0x633)
> +	if ((c->x86 << 8 | c->x86_model << 4 | c->x86_mask) < 0x633)
>  		clear_cpu_cap(c, X86_FEATURE_SEP);
>  
>  	/*
> @@ -204,10 +228,10 @@ static void __cpuinit intel_workarounds(struct cpuinfo_x86 *c)
>  	if ((c->x86 == 15) && (c->x86_model == 1) && (c->x86_mask == 1)) {
>  		rdmsr(MSR_IA32_MISC_ENABLE, lo, hi);
>  		if ((lo & MSR_IA32_MISC_ENABLE_PREFETCH_DISABLE) == 0) {
> -			printk (KERN_INFO "CPU: C0 stepping P4 Xeon detected.\n");
> -			printk (KERN_INFO "CPU: Disabling hardware prefetching (Errata 037)\n");
> +			pr_info("CPU: C0 stepping P4 Xeon detected.\n");
> +			pr_info("CPU: Disabling hardware prefetching (Errata 037)\n");
>  			lo |= MSR_IA32_MISC_ENABLE_PREFETCH_DISABLE;
> -			wrmsr (MSR_IA32_MISC_ENABLE, lo, hi);
> +			wrmsr(MSR_IA32_MISC_ENABLE, lo, hi);
>  		}
>  	}
>  
> @@ -217,7 +241,7 @@ static void __cpuinit intel_workarounds(struct cpuinfo_x86 *c)
>  	 * integrated APIC (see 11AP erratum in "Pentium Processor
>  	 * Specification Update").
>  	 */
> -	if (cpu_has_apic && (c->x86<<8 | c->x86_model<<4) == 0x520 &&
> +	if (cpu_has_apic && (c->x86 << 8 | c->x86_model << 4) == 0x520 &&
>  	    (c->x86_mask < 0x6 || c->x86_mask == 0xb))
>  		set_cpu_cap(c, X86_FEATURE_11AP);
>  
> @@ -245,28 +269,26 @@ static void __cpuinit intel_workarounds(struct cpuinfo_x86 *c)
>  #endif
>  
>  	intel_smp_check(c);
> -}
> -#else
> -static void __cpuinit intel_workarounds(struct cpuinfo_x86 *c)
> -{
> -}
>  #endif
> +}

To me it is more obvious with the old style.
Having ifdef's inside the block is less obvious.

But I have not checked what is the common pattern.

	Sam