[coreboot] [PATCH] Split VIA cpu support code

Uwe Hermann uwe at hermann-uwe.de
Tue Mar 18 22:42:29 CET 2008


On Tue, Mar 18, 2008 at 01:43:04PM +0100, Stefan Reinauer wrote:
> Index: src/cpu/via/model_c7/Config.lb
> ===================================================================
> --- src/cpu/via/model_c7/Config.lb	(revision 0)
> +++ src/cpu/via/model_c7/Config.lb	(revision 76)
> @@ -0,0 +1,9 @@
> +dir /cpu/x86/tsc
> +dir /cpu/x86/mtrr
> +dir /cpu/x86/fpu
> +dir /cpu/x86/mmx
> +dir /cpu/x86/sse
> +dir /cpu/x86/lapic
> +dir /cpu/x86/cache
> +dir /cpu/intel/microcode
> +driver model_c7_init.o

Add the usual license header with (C) 2008 coresystems GmbH, please.


> Index: src/cpu/via/model_c7/model_c7_init.c
> ===================================================================
> --- src/cpu/via/model_c7/model_c7_init.c	(revision 0)
> +++ src/cpu/via/model_c7/model_c7_init.c	(revision 87)
> @@ -0,0 +1,227 @@
> +/*
> + * This file is part of the coreboot project.
> + * 
> + * (C) 2007-2008 coresystems GmbH
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
> + * MA 02110-1301 USA
> + */
> +
> +#include <device/device.h>
> +#include <console/console.h>
> +#include <delay.h>
> +#include <stdlib.h>
> +
> +#include <cpu/cpu.h>
> +#include <cpu/x86/mtrr.h>
> +#include <cpu/x86/msr.h>
> +#include <cpu/x86/lapic.h>
> +#include <cpu/x86/cache.h>
> +#include <cpu/x86/mtrr.h>
> +
> +#define MSR_IA32_PERF_STATUS	0x00000198
> +#define MSR_IA32_PERF_CTL	0x00000199
> +#define MSR_IA32_MISC_ENABLE	0x000001a0
> +
> +static int c7a_speed_translation[] = {

Can be 'const' also, I think.


> +//      LFM     HFM             
> +	0x0409, 0x0f13,		// 400MHz, 844mV --> 1500MHz, 1.004V    C7-M
> +	0x0409, 0x1018,		// 400MHz, 844mV --> 1600MHz, 1.084V
> +	0x0409, 0x0c18,		// 533MHz, 844mV --> 1600MHz, 1.084V
> +	0x0409, 0x121c,		// 400MHz, 844mV --> 1800MHz, 1.148V
> +	0x0409, 0x0e1c,		// 533MHz, 844mV --> 1860MHz, 1.148V
> +	0x0409, 0x141f,		// 400MHz, 844mV --> 2000MHz, 1.196V
> +	0x0409, 0x0f1f,		// 533MHz, 844mV --> 2000MHz, 1.196V
> +	0x0406, 0x0a06,		// 400MHz, 796mV --> 1000MHz, 796mV     C7-M ULV
> +	0x0406, 0x0a09,		// 400MHz, 796mV --> 1000MHz, 844mV
> +	0x0406, 0x0c09,		// 400MHz, 796mV --> 1200MHz, 844mV
> +	0x0406, 0x0f10,		// 400MHz, 796mV --> 1500MHz, 956mV
> +};
> +
> +static int c7d_speed_translation[] = {

Ditto.


> +//      LFM     HFM             
> +	0x0409, 0x1018,		// 400MHz, 844mV --> 1600MHz, 1.084V    C7-M
> +	0x0409, 0x121f,		// 400MHz, 844mV --> 1800MHz, 1.196V
> +	0x0809, 0x121f,		// 800MHz, 844mV --> 1800MHz, 1.196V
> +	0x0409, 0x141f,		// 400MHz, 844mV --> 2000MHz, 1.196V
> +	0x0809, 0x141f,		// 800MHz, 844mV --> 2000MHz, 1.196V
> +	0x0406, 0x0806,		// 400MHz, 796mV --> 800MHz, 796mV      C7-M ULV
> +	0x0406, 0x0a06,		// 400MHz, 796mV --> 1000MHz, 796mV
> +	0x0406, 0x0c09,		// 400MHz, 796mV --> 1200MHz, 844mV
> +	0x0806, 0x0c09,		// 800MHz, 796mV --> 1200MHz, 844mV
> +	0x0406, 0x0f10,		// 400MHz, 796mV --> 1500MHz, 956mV
> +	0x0806, 0x1010,		// 800MHz, 796mV --> 1600MHz, 956mV
> +};


> +static void model_c7_init(device_t dev)
> +{
> +	u8 brand;
> +	struct cpuinfo_x86 c;
> +	msr_t msr;
> +
> +	get_fms(&c, dev->device);
> +
> +	printk_info("Detected VIA ");
> +
> +	switch (c.x86_model) {
> +	case 10:
> +		msr = rdmsr(0x1153);

This should probably be a #define for better readability.


> +		brand = (((msr.lo >> 2) ^ msr.lo) >> 18) & 3;
> +		printk_info("Model A ");
> +		break;
> +	case 13:
> +		msr = rdmsr(0x1154);

Ditto.


> +	/* Gear up */
> +	set_c7_speed(c.x86_model);
> +
> +	/* Turn on cache */

That's L1 cache but not L2, correct? If so, please mention "L1" in the
comment explicitly to avoid misunderstandings.


> +	x86_enable_cache();
> +
> +	/* Set up Memory Type Range Registers */
> +	x86_setup_mtrrs(36);
> +	x86_mtrr_check();
> +
> +	/* Enable the local cpu apics */
> +	setup_lapic();
> +};
> +
> +static struct device_operations cpu_dev_ops = {
> +	.init = model_c7_init,
> +};
> +
> +/* Look in arch/i386/lib/cpu.c:cpu_initialize. If there is no CPU with an exact
> + * ID, the cpu mask (stepping) is masked out and the check is repeated. This
> + * allows us to keep the table significantly smaller.
> + */
> +
> +static struct cpu_device_id cpu_table[] = {
> +	{X86_VENDOR_CENTAUR, 0x06A0},	// VIA C7 Esther
> +	{X86_VENDOR_CENTAUR, 0x06D0},	// VIA C7-M
> +	{0, 0},
> +};
> +
> +static const struct cpu_driver driver __cpu_driver = {
> +	.ops = &cpu_dev_ops,
> +	.id_table = cpu_table,
> +};
> Index: src/cpu/via/model_c3/Config.lb
> ===================================================================
> --- src/cpu/via/model_c3/Config.lb	(revision 0)
> +++ src/cpu/via/model_c3/Config.lb	(revision 76)

Add license header, see above.


> @@ -0,0 +1,9 @@
> +dir /cpu/x86/tsc
> +dir /cpu/x86/mtrr
> +dir /cpu/x86/fpu
> +dir /cpu/x86/mmx
> +dir /cpu/x86/sse
> +dir /cpu/x86/lapic
> +dir /cpu/x86/cache
> +dir /cpu/intel/microcode
> +driver model_c3_init.o
> Index: src/cpu/via/model_c3/model_c3_init.c
> ===================================================================
> --- src/cpu/via/model_c3/model_c3_init.c	(revision 0)
> +++ src/cpu/via/model_c3/model_c3_init.c	(revision 76)


Add license header, see above. This is trivial enough IMHO to just
make it (C) 2008 coresystems, no need for extra history research
in svn logs.


> @@ -0,0 +1,39 @@
> +#include <console/console.h>
> +#include <device/device.h>
> +#include <device/device.h>
> +#include <device/pci.h>
> +#include <string.h>
> +#include <cpu/cpu.h>
> +#include <cpu/x86/mtrr.h>
> +#include <cpu/x86/msr.h>
> +#include <cpu/x86/lapic.h>
> +#include <cpu/intel/microcode.h>
> +#include <cpu/x86/cache.h>
> +#include <cpu/x86/mtrr.h>
> +
> +static void model_c3_init(device_t dev)
> +{
> +	/* Turn on caching if we haven't already */
> +	x86_enable_cache();

L1 cache? See above.


> +	x86_setup_mtrrs(36);
> +	x86_mtrr_check();
> +
> +	/* Enable the local cpu apics */
> +	setup_lapic();
> +};
> +
> +static struct device_operations cpu_dev_ops = {
> +	.init     = model_c3_init,
> +};
> +
> +static struct cpu_device_id cpu_table[] = {
> +	{ X86_VENDOR_CENTAUR, 0x0670 },		// VIA C3 Samual 2 + Ezra
> +	{ X86_VENDOR_CENTAUR, 0x0680 },		// VIA C3 Ezra-T
> +	{ X86_VENDOR_CENTAUR, 0x0690 },		// VIA C3 Nehemiah
> +	{ 0, 0 },
> +};
> +
> +static const struct cpu_driver driver __cpu_driver = {
> +	.ops      = &cpu_dev_ops,
> +	.id_table = cpu_table,
> +};


> Index: src/mainboard/via/epia-m/Config.lb
> ===================================================================
> --- src/mainboard/via/epia-m/Config.lb	(revision 60)
> +++ src/mainboard/via/epia-m/Config.lb	(working copy)
> @@ -134,7 +134,7 @@
>  chip northbridge/via/vt8623
>  
>  	device apic_cluster 0 on
> -		chip cpu/via/model_centaur
> +		chip cpu/via/model_c3
>  			device apic 0 on  end 
>  		end
>  	end
> Index: src/mainboard/via/epia/Config.lb
> ===================================================================
> --- src/mainboard/via/epia/Config.lb	(revision 60)
> +++ src/mainboard/via/epia/Config.lb	(working copy)
> @@ -184,7 +184,7 @@
>  	end
>  
>          device apic_cluster 0 on
> -                chip cpu/via/model_centaur 
> +                chip cpu/via/model_c3
>                          device apic 0 on end
>                  end
>          end

Is the patch tested on hardware, ideally some C3 and C7 and
epia/epia-m system? It should not break any boards, of course.
Is it abuild-tested, too? I didn't check if it compiles...

Patch looks good otherwise, with the above changes I'll ack (I cannot
test on hardware, though).


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org




More information about the coreboot mailing list