epia-cn patch

[aaron lwe]

> Do you have the dsdt.dsl source? Where did it come from?

dsdt.c comes from disassembling the dsl file. the dsl file is
extracted from memory when booted with factory bios. I'll add that
file if necessary.
> Index: src/mainboard/via/epia-cn/fadt.c
> ===================================================================
> --- src/mainboard/via/epia-cn/fadt.c  (revision 0)
> +++ src/mainboard/via/epia-cn/fadt.c  (revision 0)
> @@ -0,0 +1,154 @@
> +/*
> + * ACPI - create the Fixed ACPI Description Tables (FADT)
> + * Copyright (C) 2004 Nick Barker <[e-mail]>
> Did you change anything in this file? Then I think you should add your copyright, possibly even remove Nick's.

yes, I changed the values.
> +void acpi_create_fadt(acpi_fadt_t *fadt,acpi_facs_t *facs,void *dsdt){
> +     acpi_header_t *header=&(fadt->header);
> +
> +     memset((void *)fadt,0,sizeof(acpi_fadt_t));
> +     memcpy(header->signature,"FACP",4);
> +     header->length = 244;
> +     header->revision = 1;
> +     memcpy(header->oem_id,"CN700 ",6);
> +     memcpy(header->oem_table_id,"AWRDACPI",8);
> +     memcpy(header->asl_compiler_id,"AWRD",4);
> We are not Award. What is this about?

These values are extracted from factory bios FACP table, so I just keep them.
> +     header->asl_compiler_revision=0;
> +
> +     fadt->firmware_ctrl=facs;
> +     fadt->dsdt= dsdt;
> +     fadt->preferred_pm_profile=0;
> +     fadt->sci_int=9;
> +     fadt->smi_cmd = 0x0;
> +     fadt->acpi_enable = 0x0;
> +     fadt->acpi_disable = 0x0;
> +     fadt->s4bios_req = 0x0;
> +     fadt->pstate_cnt = 0x0;
> +
> +     fadt->pm1a_evt_blk = 0x400;
> +     fadt->pm1b_evt_blk = 0x0;
> +     fadt->pm1a_cnt_blk = 0x404;
> +     fadt->pm1b_cnt_blk = 0x0;
> +     fadt->pm2_cnt_blk = 0x0;
> +     fadt->pm_tmr_blk = 0x408;
> +     fadt->gpe0_blk = 0x420;
> +     fadt->gpe1_blk = 0x0;
> Where do all these magic values come from?

By extracting FACP table of the factory BIOS, these values are all in
that table.
> -/**
> - * Configure the bus between the cpu and the northbridge. This might be able to
> - * be moved to post-ram code in the future. For the most part, these registers
> - * should not be messed around with. These are too complex to explain short of
> - * copying the datasheets into the comments, but most of these values are from
> - * the BIOS Porting Guide, so they should work on any board. If they don't,
> - * try the values from your factory BIOS.
> - *
> - * TODO: Changing the DRAM frequency doesn't work (hard lockup)
> - *
> - * @param dev The northbridge's CPU Host Interface (D0F2)
> - */
>  static void c7_cpu_setup(device_t dev)
>  {
> Hey, please don't remove doxygen. :)
> Oh...my bad ;-)
> Index: src/northbridge/via/cn700/northbridge.c
> ===================================================================
> --- src/northbridge/via/cn700/northbridge.c   (revision 3300)
> +++ src/northbridge/via/cn700/northbridge.c   (working copy)
> @@ -1,6 +1,8 @@
>  /*
>   * This file is part of the coreboot project.
>   *
> + * Copyright (C) 2008 VIA Technologies, Inc.
> + * Copyright (C) 2008 Aaron Lwe <[e-mail]>
> How is VIA involved in this?
> I'm a VIA employee, so I think I should add VIA here, sorry not making
> this clear.
> Thanks a lot for the patch! I guess Corey and Bari will have a go at
> it but I'll try to test as well.
> Thanks. If dram didn't work right, try to change the values of D0F2
> registers in c7_cpu_setup to be the same as the facotry BIOS set.

-Aaron

[Carl-Daniel Hailfinger]

Hi Aaron,

On 13.05.2008 15:48, aaron lwe wrote:
> > Do you have the dsdt.dsl source? Where did it come from?
>
> dsdt.c comes from disassembling the dsl file. the dsl file is
> extracted from memory when booted with factory bios. I'll add that
> file if necessary.

There could be a problem. Some BIOS vendors claim all ACPI tables belong
to them, not to the mainboard vendor.
> > Index: src/mainboard/via/epia-cn/fadt.c
> > ===================================================================
> > --- src/mainboard/via/epia-cn/fadt.c  (revision 0)
> > +++ src/mainboard/via/epia-cn/fadt.c  (revision 0)
> > @@ -0,0 +1,154 @@
> > +/*
> > + * ACPI - create the Fixed ACPI Description Tables (FADT)
> > + * Copyright (C) 2004 Nick Barker <[e-mail]>
> > Did you change anything in this file? Then I think you should add your copyright, possibly even remove Nick's.
>
> yes, I changed the values.
> > +void acpi_create_fadt(acpi_fadt_t *fadt,acpi_facs_t *facs,void *dsdt){
> > +     acpi_header_t *header=&(fadt->header);
> > +
> > +     memset((void *)fadt,0,sizeof(acpi_fadt_t));
> > +     memcpy(header->signature,"FACP",4);
> > +     header->length = 244;
> > +     header->revision = 1;
> > +     memcpy(header->oem_id,"CN700 ",6);
> > +     memcpy(header->oem_table_id,"AWRDACPI",8);
> > +     memcpy(header->asl_compiler_id,"AWRD",4);
> > We are not Award. What is this about?
>
> These values are extracted from factory bios FACP table, so I just keep them.

Please see above. We have some code to create our own FACP tables, maybe
we should use that?
> > +     header->asl_compiler_revision=0;
> > +
> > +     fadt->firmware_ctrl=facs;
> > +     fadt->dsdt= dsdt;
> > +     fadt->preferred_pm_profile=0;
> > +     fadt->sci_int=9;
> > +     fadt->smi_cmd = 0x0;
> > +     fadt->acpi_enable = 0x0;
> > +     fadt->acpi_disable = 0x0;
> > +     fadt->s4bios_req = 0x0;
> > +     fadt->pstate_cnt = 0x0;
> > +
> > +     fadt->pm1a_evt_blk = 0x400;
> > +     fadt->pm1b_evt_blk = 0x0;
> > +     fadt->pm1a_cnt_blk = 0x404;
> > +     fadt->pm1b_cnt_blk = 0x0;
> > +     fadt->pm2_cnt_blk = 0x0;
> > +     fadt->pm_tmr_blk = 0x408;
> > +     fadt->gpe0_blk = 0x420;
> > +     fadt->gpe1_blk = 0x0;
> > Where do all these magic values come from?
>
> By extracting FACP table of the factory BIOS, these values are all in
> that table.

See above.
> > Index: src/northbridge/via/cn700/northbridge.c
> > ===================================================================
> > --- src/northbridge/via/cn700/northbridge.c   (revision 3300)
> > +++ src/northbridge/via/cn700/northbridge.c   (working copy)
> > @@ -1,6 +1,8 @@
> >  /*
> >   * This file is part of the coreboot project.
> >   *
> > + * Copyright (C) 2008 VIA Technologies, Inc.
> > + * Copyright (C) 2008 Aaron Lwe <[e-mail]>
> > How is VIA involved in this?
>
> I'm a VIA employee, so I think I should add VIA here, sorry not making
> this clear.

Wow, this is really GREAT! Thanks to you and to VIA for working on CN
support.


I think we can merge this (without the ACPI stuff) once one of the other
people working on CN support have commented.


Regards,
Carl-Daniel

[Peter Stuge]

On Tue, May 13, 2008 at 09:48:58PM +0800, aaron lwe wrote:
> > Do you have the dsdt.dsl source? Where did it come from?
>
> dsdt.c comes from disassembling the dsl file. the dsl file is
> extracted from memory when booted with factory bios. I'll add that
> file if necessary.

I don't think it is critical at the moment, but I would like to have
the actual source file in the repository rather than a compiled
version.
> > +     memcpy(header->oem_table_id,"AWRDACPI",8);
> > +     memcpy(header->asl_compiler_id,"AWRD",4);
> > We are not Award. What is this about?
>
> These values are extracted from factory bios FACP table, so I just
> keep them.

We should at least change them before commit.
> > +     fadt->gpe0_blk = 0x420;
> > +     fadt->gpe1_blk = 0x0;
> > Where do all these magic values come from?
>
> By extracting FACP table of the factory BIOS, these values are all
> in that table.

..
> > Hey, please don't remove doxygen. :)
> > Oh...my bad ;-)

Easy fix. :)
> > + * Copyright (C) 2008 VIA Technologies, Inc.
> > + * Copyright (C) 2008 Aaron Lwe <[e-mail]>
> > How is VIA involved in this?
>
> I'm a VIA employee, so I think I should add VIA here, sorry not
> making this clear.

That is excellent! Welcome aboard. I completely missed this fact.

I do believe that the ACPI table situation needs some attention.

Are you sure that you/VIA can freely distribute source code for the
ACPI tables from Award's BIOS?

I understand that they may be good or even neccessary for testing
purposes, but I doubt that we will be able to use them verbatim.

Or am I wrong?
> > Thanks a lot for the patch! I guess Corey and Bari will have a go
> > at it but I'll try to test as well.
>
> Thanks. If dram didn't work right, try to change the values of D0F2
> registers in c7_cpu_setup to be the same as the facotry BIOS set.

Ok. Thanks for that pointer!


//Peter

[aaron lwe]

Hi all,

I've cleaned up the code, adding back the comments and use defined
macro to replace magic values. I've also removed acpi related files
since I'm not sure of its ownership. Thank you all for your comments.

-Aaron

[Carl-Daniel Hailfinger]

Hi Aaron,

it seems you forgot the Signed-off-by statement. That is required for
every new version of a submitted patch. You don't have to repost the
patch, it is sufficient to reply to your patch with the Signed-off-by line.

On 14.05.2008 05:40, aaron lwe wrote:
> I've cleaned up the code, adding back the comments and use defined
> macro to replace magic values. I've also removed acpi related files
> since I'm not sure of its ownership. Thank you all for your comments.

I really like the new code. It looks nice and clean. From a coding style
of view and the general structure, this patch is
Acked-by: Carl-Daniel Hailfinger <[e-mail]>
if you add your Signed-off-by.

We should wait with the commit until Bari and maybe Rudolf have reviewed
your code. They are a lot more familiar with the code.

Regards,
Carl-Daniel

[aaron lwe]

On Wed, May 14, 2008 at 11:40 AM, aaron lwe <[e-mail]> wrote:
> Hi all,
>
> I've cleaned up the code, adding back the comments and use defined
>  macro to replace magic values. I've also removed acpi related files
>  since I'm not sure of its ownership. Thank you all for your comments.
>
> -Aaron

Signed-off-by: Aaron Lwe <[e-mail]>

Thanks.

[Uwe Hermann]

Hi,

On Wed, May 14, 2008 at 11:40:58AM +0800, aaron lwe wrote:
> Index: src/include/device/pci_ids.h
> ===================================================================
> --- src/include/device/pci_ids.h	(revision 3305)
> +++ src/include/device/pci_ids.h	(working copy)
> @@ -1160,6 +1160,7 @@
>  #define PCI_DEVICE_ID_VIA_K8T890CE_BR	0xb188
>  #define PCI_DEVICE_ID_VIA_VT6420_SATA	0x3149
>  #define PCI_DEVICE_ID_VIA_VT8237R_LPC	0x3227
> +#define PCI_DEVICE_ID_VIA_VT8237R_SATA	0x3149
>  #define PCI_DEVICE_ID_VIA_CN700_AGP	0x0314
>  #define PCI_DEVICE_ID_VIA_CN700_ERR	0x1314
>  #define PCI_DEVICE_ID_VIA_CN700_HOST	0x2314
> Index: src/mainboard/via/epia-cn/Config.lb
> ===================================================================
> --- src/mainboard/via/epia-cn/Config.lb	(revision 0)
> +++ src/mainboard/via/epia-cn/Config.lb	(revision 0)
> @@ -0,0 +1,218 @@
> +##
> +## This file is part of the coreboot project.
> +##

Missing copyright owner, please add

  Copyright (C) 2008 VIA Technologies, Inc.
> +## 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
> +##
> +
> +##
> +## Compute the location and size of where this firmware image
> +## (coreboot plus bootloader) will live in the boot rom chip.
> +##
> +if USE_FALLBACK_IMAGE
> +	default ROM_SECTION_SIZE   = FALLBACK_SIZE
> +	default ROM_SECTION_OFFSET = ( ROM_SIZE - FALLBACK_SIZE )
> +else
> +	default ROM_SECTION_SIZE   = ( ROM_SIZE - FALLBACK_SIZE )
> +	default ROM_SECTION_OFFSET = 0
> +end
> +
> +##
> +## Compute the start location and size size of
> +## The coreboot bootloader.
> +##
> +default PAYLOAD_SIZE            = ( ROM_SECTION_SIZE - ROM_IMAGE_SIZE )
> +default CONFIG_ROM_PAYLOAD_START = (0xffffffff - ROM_SIZE + ROM_SECTION_OFFSET + 1)
> +
> +##
> +## Compute where this copy of coreboot will start in the boot rom
> +##
> +default _ROMBASE      = ( CONFIG_ROM_PAYLOAD_START + PAYLOAD_SIZE )
> +
> +##
> +## Compute a range of ROM that can cached to speed up coreboot,
> +## execution speed.
> +##
> +## XIP_ROM_SIZE must be a power of 2.
> +## XIP_ROM_BASE must be a multiple of XIP_ROM_SIZE
> +##
> +default XIP_ROM_SIZE=0x10000
> +default XIP_ROM_BASE = ( _ROMBASE + ROM_IMAGE_SIZE - XIP_ROM_SIZE )
> +
> +##
> +## Set all of the defaults for an x86 architecture
> +##
> +
> +arch i386 end
> +
> +##
> +## Build the objects we have code for in this directory.
> +##
> +
> +driver mainboard.o
> +if HAVE_PIRQ_TABLE object irq_tables.o end
> +
> +if HAVE_MP_TABLE object mptable.o end
> +
> +if HAVE_ACPI_TABLES
> +	object fadt.o
> +	object dsdt.o
> +	object acpi_tables.o
> +end
> +
> +##
> +## Romcc output
> +##
> +makerule ./failover.E
> +	depends "$(MAINBOARD)/failover.c ./romcc" 
> +	action "./romcc -E -O --label-prefix=failover -I$(TOP)/src -I. $(CPPFLAGS) $(MAINBOARD)/failover.c -o $@"
> +end
> +
> +makerule ./failover.inc
> +	depends "$(MAINBOARD)/failover.c ./romcc"
> +	action "./romcc    -O --label-prefix=failover -I$(TOP)/src -I. $(CPPFLAGS) $(MAINBOARD)/failover.c -o $@"
> +end
> +
> +makerule ./auto.E 
> +	depends	"$(MAINBOARD)/auto.c option_table.h ./romcc" 
> +	action	"./romcc -E -mcpu=c3 -O -I$(TOP)/src -I. $(CPPFLAGS) $(MAINBOARD)/auto.c -o $@"
> +end
> +makerule ./auto.inc 
> +	depends "$(MAINBOARD)/auto.c option_table.h ./romcc"
> +	action	"./romcc    -mcpu=c3 -O -I$(TOP)/src -I. $(CPPFLAGS) $(MAINBOARD)/auto.c -o $@"
> +end
> +
> +##
> +## Build our 16 bit and 32 bit coreboot entry code
> +##
> +mainboardinit cpu/x86/16bit/entry16.inc
> +mainboardinit cpu/x86/32bit/entry32.inc
> +ldscript /cpu/x86/16bit/entry16.lds
> +ldscript /cpu/x86/32bit/entry32.lds
> +
> +##
> +## Build our reset vector (This is where coreboot is entered)
> +##
> +if USE_FALLBACK_IMAGE 
> +	mainboardinit cpu/x86/16bit/reset16.inc 
> +	ldscript /cpu/x86/16bit/reset16.lds 
> +else
> +	mainboardinit cpu/x86/32bit/reset32.inc 
> +	ldscript /cpu/x86/32bit/reset32.lds 
> +end
> +
> +### Should this be in the northbridge code?
> +mainboardinit arch/i386/lib/cpu_reset.inc
> +
> +##
> +## Include an id string (For safe flashing)
> +##
> +mainboardinit arch/i386/lib/id.inc
> +ldscript /arch/i386/lib/id.lds
> +
> +###
> +### This is the early phase of coreboot startup 
> +### Things are delicate and we test to see if we should
> +### failover to another image.
> +###
> +if USE_FALLBACK_IMAGE
> +	ldscript /arch/i386/lib/failover.lds 
> +	mainboardinit ./failover.inc
> +end
> +
> +###
> +### O.k. We aren't just an intermediary anymore!
> +###
> +
> +##
> +## Setup RAM
> +##
> +mainboardinit cpu/x86/fpu/enable_fpu.inc
> +mainboardinit cpu/x86/mmx/enable_mmx.inc
> +mainboardinit ./auto.inc
> +mainboardinit cpu/x86/mmx/disable_mmx.inc
> +
> +##
> +## Include the secondary Configuration files 
> +##
> +dir /pc80
> +config chip.h
> +
> +chip northbridge/via/cn700
> +	device pci_domain 0 on
> +		device pci 0.0 on end # AGP Bridge
> +		device pci 0.1 on end # Error Reporting
> +		device pci 0.2 on end # Host Bus Control
> +		device pci 0.3 on end # Memory Controller
> +		device pci 0.4 on end # Power Management
> +		device pci 0.7 on end # V-Link Controller
> +		device pci 1.0 on # PCI Bridge
> +#			chip drivers/pci/onboard
> +#				device pci 0.0 on end
> +#				register "rom_address" = "0xfff80000" #512k image
> +#			end # Onboard Video
> +		end # PCI Bridge
> +		
> +		chip southbridge/via/vt8237r
> +			#both IDE channels
> +			register "ide0_enable" = "1"
> +			register "ide1_enable" = "1"
> +			#both cables are 80pin

The comment is not correct, you set both to "0".
> +			register "ide0_80pin_cable" = "0"
> +			register "ide1_80pin_cable" = "0"
> +			device pci f.0 on end # IDE
> +			register "fn_ctrl_lo" = "0x8a"
> +			register "fn_ctrl_hi" = "0x9d"
> +			device pci 10.0 on end # USB 1.1
> +			device pci 10.1 on end # USB 1.1
> +			device pci 10.2 on end # USB 1.1
> +			device pci 10.3 on end # USB 1.1
> +			device pci 11.0 on      # Southbridge LPC
> +				chip superio/via/vt1211
> +					device pnp 2e.0 off	# Floppy
> +						io 0x60 = 0x3f0
> +						irq 0x70 = 6
> +						drq 0x74 = 2
> +					end
> +					device pnp 2e.1 on	# Parallel Port
> +						io 0x60 = 0x378
> +						irq 0x70 = 7
> +						drq 0x74 = 3
> +					end
> +					device pnp 2e.2 on	# COM1
> +						io 0x60 = 0x3f8
> +						irq 0x70 = 4
> +					end
> +					#device pnp 2e.3 on	# COM2
> +					#	io 0x60 = 0x2f8
> +					#	irq 0x70 = 3

No COM2 port at all? Or why are the lines commented?
> +					#end
> +					device pnp 2e.b on	# HWM
> +						io 0x60 = 0xec00
> +					end
> +				end #superio
> +			end # pci 11.0
> +			# 1-4 non existant
> +			device pci 11.5 on end # AC97 Audio
> +			#device pci 11.6 off end # AC97 Modem
> +			device pci 12.0 on end  # Ethernet
> +		end # vt8237
> +	end # pci domain 0
> +
> +	device apic_cluster 0 on
> +		chip cpu/via/model_c7
> +			device apic 0 on end 
> +		end
> +	end
> +end # cn700
> Index: src/mainboard/via/epia-cn/irq_tables.c
> ===================================================================
> --- src/mainboard/via/epia-cn/irq_tables.c	(revision 0)
> +++ src/mainboard/via/epia-cn/irq_tables.c	(revision 0)
> @@ -0,0 +1,48 @@
> +/* This file was generated by getpir.c, do not modify!
> + * (but if you do, please run checkpir on it to verify)
> + *
> + * Contains the IRQ Routing Table dumped directly from your
> + * memory, which BIOS sets up.
> + *
> + * Documentation at: http://www.microsoft.com/whdc/archive/pciirq.mspx
> + */

Drop this, add the usual GPL header instead please.
> +
> +#ifdef GETPIR
> +#include "pirq_routing.h"
> +#else
> +#include <arch/pirq_routing.h>
> +#endif

Drop, should be
  #include <arch/pirq_routing.h>
only.
> +
> +const struct irq_routing_table intel_irq_routing_table = {
> +	PIRQ_SIGNATURE,  /* u32 signature */
> +	PIRQ_VERSION,    /* u16 version   */
> +	32+16*10,	 /* There can be total 10 devices on the bus */

Replace 10 with IRQ_SLOT_COUNT, which should be set to 10 in Options.lb.
> +	0x00,		 /* Where the interrupt router lies (bus) */
> +	(0x11<<3)|0x0,   /* Where the interrupt router lies (dev) */
> +	0x828,		 /* IRQs devoted exclusively to PCI usage */
> +	0x1106,		 /* Vendor */
> +	0x596,		 /* Device */
> +	0,		 /* Crap (miniport) */
> +	{ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, /* u8 rfu[11] */
> +	0x3e,		 /* u8 checksum. This has to be set to some
> +			    value that would give 0 after the sum of all
> +			    bytes for this structure (including checksum) */
> +	{
> +		/* bus,     dev|fn,   {link, bitmap}, {link, bitmap}, {link, bitmap}, {link, bitmap},  slot, rfu */
> +		{0x00,(0x08<<3)|0x0, {{0x01, 0xdef8}, {0x02, 0xdef8}, {0x03, 0xdef8}, {0x05, 0x0def8}}, 0x1, 0x0},
> +		{0x00,(0x09<<3)|0x0, {{0x02, 0xdef8}, {0x03, 0xdef8}, {0x05, 0xdef8}, {0x01, 0x0def8}}, 0x2, 0x0},
> +		{0x00,(0x0a<<3)|0x0, {{0x03, 0xdef8}, {0x05, 0xdef8}, {0x01, 0xdef8}, {0x02, 0x0def8}}, 0x3, 0x0},
> +		{0x00,(0x0b<<3)|0x0, {{0x05, 0xdef8}, {0x01, 0xdef8}, {0x02, 0xdef8}, {0x03, 0x0def8}}, 0x4, 0x0},
> +		{0x00,(0x0c<<3)|0x0, {{0x01, 0xdef8}, {0x02, 0xdef8}, {0x03, 0xdef8}, {0x05, 0x0def8}}, 0x5, 0x0},
> +		{0x00,(0x11<<3)|0x0, {{0x00, 0xdef8}, {0x00, 0xdef8}, {0x03, 0xdef8}, {0x05, 0x0def8}}, 0x0, 0x0},
> +		{0x00,(0x0f<<3)|0x0, {{0x01, 0xdef8}, {0x02, 0xdef8}, {0x03, 0xdef8}, {0x05, 0x0def8}}, 0x0, 0x0},
> +		{0x00,(0x01<<3)|0x0, {{0x01, 0xdef8}, {0x02, 0xdef8}, {0x03, 0xdef8}, {0x05, 0x0def8}}, 0x0, 0x0},
> +		{0x00,(0x10<<3)|0x0, {{0x01, 0xdef8}, {0x02, 0xdef8}, {0x03, 0xdef8}, {0x05, 0x0def8}}, 0x0, 0x0},
> +		{0x00,(0x12<<3)|0x0, {{0x01, 0xdef8}, {0x00, 0xdef8}, {0x00, 0xdef8}, {0x00, 0x0def8}}, 0x0, 0x0},
> +	}
> +};
> +
> +unsigned long write_pirq_routing_table(unsigned long addr)
> +{
> +        return copy_pirq_routing_table(addr);
> +}
> Index: src/mainboard/via/epia-cn/Options.lb
> ===================================================================
> --- src/mainboard/via/epia-cn/Options.lb	(revision 0)
> +++ src/mainboard/via/epia-cn/Options.lb	(revision 0)

Add the usual GPL header, please (including copyright line and year), e.g.

/*
 * This file is part of the coreboot project.
 *
 * Copyright (C) 2008 VIA Technologies, Inc.
 *
 * 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
 */

Same for all other files which don't have a GPL header.
> @@ -0,0 +1,151 @@
> +uses HAVE_MP_TABLE
> +uses HAVE_PIRQ_TABLE
> +uses USE_FALLBACK_IMAGE
> +uses HAVE_FALLBACK_BOOT
> +uses HAVE_HARD_RESET
> +uses HAVE_OPTION_TABLE
> +uses USE_OPTION_TABLE
> +uses CONFIG_ROM_PAYLOAD
> +uses IRQ_SLOT_COUNT
> +uses MAINBOARD
> +uses MAINBOARD_VENDOR
> +uses MAINBOARD_PART_NUMBER
> +uses COREBOOT_EXTRA_VERSION
> +uses ARCH
> +uses FALLBACK_SIZE
> +uses STACK_SIZE
> +uses HEAP_SIZE
> +uses ROM_SIZE
> +uses ROM_SECTION_SIZE
> +uses ROM_IMAGE_SIZE
> +uses ROM_SECTION_SIZE
> +uses ROM_SECTION_OFFSET
> +uses CONFIG_ROM_PAYLOAD_START
> +uses CONFIG_COMPRESSED_PAYLOAD_NRV2B
> +uses CONFIG_COMPRESSED_PAYLOAD_LZMA
> +uses PAYLOAD_SIZE
> +uses _ROMBASE
> +uses _RAMBASE
> +uses XIP_ROM_SIZE
> +uses XIP_ROM_BASE
> +uses HAVE_MP_TABLE
> +uses HAVE_ACPI_TABLES
> +uses CROSS_COMPILE
> +uses CC
> +uses HOSTCC
> +uses OBJCOPY
> +uses DEFAULT_CONSOLE_LOGLEVEL
> +uses MAXIMUM_CONSOLE_LOGLEVEL
> +uses CONFIG_CONSOLE_SERIAL8250
> +uses CONFIG_UDELAY_TSC
> +uses CONFIG_TSC_X86RDTSC_CALIBRATE_WITH_TIMER2
> +uses CONFIG_PCI_ROM_RUN
> +uses CONFIG_CONSOLE_VGA
> +uses CONFIG_MAX_PCI_BUSES 
> +uses TTYS0_BAUD
> +uses CONFIG_CHIP_NAME
> +uses CONFIG_VIDEO_MB
> +uses CONFIG_IOAPIC
> +
> +# default IOAPIC
> +default CONFIG_IOAPIC = 0
> +
> +# default MB size
> +default CONFIG_VIDEO_MB = 32
> +
> +## ROM_SIZE is the size of boot ROM that this board will use.
> +default ROM_SIZE  = 512*1024
> +
> +###
> +### Build options
> +###
> +default CONFIG_PCI_ROM_RUN=0
> +default CONFIG_CONSOLE_VGA=0
> +
> +default CONFIG_CHIP_NAME=1
> +
> +##
> +## Build code for the fallback boot
> +##
> +default HAVE_FALLBACK_BOOT=1
> +
> +##
> +## no MP table
> +##
> +default HAVE_MP_TABLE=0
> +
> +##
> +## Use TSC for udelay.
> +##
> +default CONFIG_UDELAY_TSC=1
> +default CONFIG_TSC_X86RDTSC_CALIBRATE_WITH_TIMER2=1
> +
> +##
> +## Build code to reset the motherboard from coreboot
> +##
> +default HAVE_HARD_RESET=0
> +
> +##
> +## Build code to export a programmable irq routing table
> +##
> +default HAVE_PIRQ_TABLE=1
> +default IRQ_SLOT_COUNT=10
> +
> +
> +##
> +## Build code to load acpi tables
> +##
> +default HAVE_ACPI_TABLES=0
> +
> +
> +##
> +## Build code to export a CMOS option table
> +##
> +default HAVE_OPTION_TABLE=1
> +
> +###
> +### coreboot layout values
> +###
> +
> +## ROM_IMAGE_SIZE is the amount of space to allow coreboot to occupy.
> +default ROM_IMAGE_SIZE = 128*1024
> +default FALLBACK_SIZE = 512*1024
> +
> +##
> +## Use a small 8K stack
> +##
> +default STACK_SIZE=0x2000
> +
> +##
> +## Use a small 16K heap
> +##
> +default HEAP_SIZE=0x4000
> +
> +##
> +## Only use the option table in a normal image
> +##
> +#default USE_OPTION_TABLE = !USE_FALLBACK_IMAGE
> +default USE_OPTION_TABLE = 0
> +
> +default _RAMBASE = 0x00004000
> +
> +default CONFIG_ROM_PAYLOAD     = 1
> +
> +##
> +## The default compiler
> +##
> +default CROSS_COMPILE=""
> +default CC="$(CROSS_COMPILE)gcc -m32"
> +default HOSTCC="gcc"
> +
> +##
> +## Set this to the max PCI bus number you 
> +## would ever use for PCI config IO.
> +## Setting this number very high will make 
> +## pci_locate_device take a long time when
> +## it can't find a device.
> +##
> +default CONFIG_MAX_PCI_BUSES = 5	 
> +
> +end
> +
> Index: src/mainboard/via/epia-cn/failover.c
> ===================================================================
> --- src/mainboard/via/epia-cn/failover.c	(revision 0)
> +++ src/mainboard/via/epia-cn/failover.c	(revision 0)

Please check if you can use the common arch/i386/lib/failover.c file instead
of adding another clone (but please test on hardware).

You'll need a small fix in Config.lb for that, something like:

Before:
   	depends "$(MAINBOARD)/failover.c ./romcc" 
   	action "./romcc    -O --label-prefix=failover -I$(TOP)/src -I. $(CPPFLAGS) $(MAINBOARD)/failover.c -o $@"

After:
        depends "$(MAINBOARD)/../../../arch/i386/lib/failover.c ./romcc"
        action "./romcc    -O --label-prefix=failover -I$(TOP)/src -I. $(CPPFLAGS) $(MAINBOARD)/../../../arch/i386/lib/failover.c -o $@"
> @@ -0,0 +1,34 @@
> +#define ASSEMBLY 1
> +#include <stdint.h>
> +#include <device/pci_def.h>
> +#include <device/pci_ids.h>
> +#include <arch/io.h>
> +#include "arch/romcc_io.h"
> +#include "pc80/mc146818rtc_early.c"
> +
> +static unsigned long main(unsigned long bist)
> +{
> +#if 0
> +	/* This is the primary cpu how should I boot? */
> +	if (do_normal_boot()) {
> +		goto normal_image;
> +	}
> +	else {
> +		goto fallback_image;
> +	}
> + normal_image:
> +	asm volatile ("jmp __normal_image" 
> +		: /* outputs */ 
> +		: "a" (bist) /* inputs */
> +		: /* clobbers */
> +		);
> + cpu_reset:
> +	asm volatile ("jmp __cpu_reset"
> +		: /* outputs */ 
> +		: "a"(bist) /* inputs */
> +		: /* clobbers */
> +		);
> + fallback_image:
> +#endif
> +	return bist;
> +}
> Index: src/mainboard/via/epia-cn/chip.h
> ===================================================================
> --- src/mainboard/via/epia-cn/chip.h	(revision 0)
> +++ src/mainboard/via/epia-cn/chip.h	(revision 0)
> @@ -0,0 +1,5 @@
> +extern struct chip_operations mainboard_via_epia_cn_ops;
> +
> +struct mainboard_via_epia_cn_config {
> +	int nothing;
> +};

Trivial file, but still add the usual license header please.
> Index: src/mainboard/via/epia-cn/auto.c
> ===================================================================
> --- src/mainboard/via/epia-cn/auto.c	(revision 0)
> +++ src/mainboard/via/epia-cn/auto.c	(revision 0)
> @@ -0,0 +1,160 @@
> +/*
> + * This file is part of the coreboot project.
> + *
> + * Copyright (C) 2008 VIA Technologies, Inc.
> + * Copyright (C) 2008 Aaron Lwe <[e-mail]>

Are you sure both is the case? If you're doing this during work hours at/for
VIA the code is very likely (C) VIA (unless your contract says otherwise),
if you do the work at home in your spare time it's likely (C) Aaron Lwe.

In the first case use something like this:

  Copyright (C) 2008 VIA Technologies, Inc.
  (Written by Aaron Lwe <[e-mail]> for VIA)
> + *
> + * 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
> + */
> +
> +#define ASSEMBLY 1
> +
> +#include <stdint.h>
> +#include <device/pci_def.h>
> +#include <device/pci_ids.h>
> +#if 0
> +#include <cpu/x86/lapic.h>
> +#endif

Drop?
> +#include <arch/io.h>
> +#include <device/pnp_def.h>
> +#include <arch/romcc_io.h>
> +#include <arch/hlt.h>
> +#include "pc80/serial.c"
> +#include "arch/i386/lib/console.c"
> +#include "ram/ramtest.c"
> +#include "northbridge/via/cn700/raminit.h"
> +#include "cpu/x86/mtrr/earlymtrr.c"
> +#include "cpu/x86/bist.h"
> +
> +/*
> + */
> +void udelay(int usecs) 
> +{
> +	int i;
> +	for(i = 0; i < usecs; i++)
> +		outb(i&0xff, 0x80);
> +}

Drop? There's a delay elsewhere in svn, forgot the exact location.
> +
> +#include "lib/delay.c"
> +#include "cpu/x86/lapic/boot_cpu.c"
> +
> +#include "southbridge/via/vt8237r/vt8237r_early_smbus.c"
> +
> +#include "southbridge/via/vt8235/vt8235_early_serial.c"
> +static void memreset_setup(void)
> +{
> +}
> +
> +static inline int spd_read_byte(unsigned device, unsigned address)
> +{
> +	return smbus_read_byte(device, address);
> +}
> +
> +#include "northbridge/via/cn700/raminit.c"
> +
> +static void enable_mainboard_devices(void) 
> +{
> +	device_t dev;
> +	u8 reg;
> + 
> +       /*
> +	* disable sata and move ide device from 00:0f.1 to 00:0f.0
> +	*/

Please also add a comment _why_ this is done.
> +	dev = pci_locate_device(PCI_ID(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_VT8237R_SATA), 0);
> +	if (dev != PCI_DEV_INVALID) {
> +		// enable backdoor

Eh?
> +		reg = pci_read_config8(dev, 0xd1);
> +		reg |= 0x08;
> +		pci_write_config8(dev, 0xd1, reg);
> +		// enable pata
> +		reg = pci_read_config8(dev, 0x49);
> +		reg |= 0x80;
> +		pci_write_config8(dev, 0x49, reg);
> +	}
> +	else
> +		print_debug("no sata device \r\n");
> +
> +	/* disable sata in VT8237R_LPC */
> +	dev = pci_locate_device(PCI_ID(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_VT8237R_LPC), 0);
> +	if (dev == PCI_DEV_INVALID)
> +		die("Southbridge not found!!!\r\n");
> +	reg = pci_read_config8(dev, 0x50);
> +	reg |= 0x08;
> +	pci_write_config8(dev, 0x50, reg);
> +}
> +
> +static const struct mem_controller ctrl = {
> +	.d0f0 = 0x0000,
> +	.d0f2 = 0x2000,
> +	.d0f3 = 0x3000,
> +	.d0f4 = 0x4000,
> +	.d0f7 = 0x7000,
> +	.d1f0 = 0x8000,
> +	.channel0 = { 0x50 },
> +};
> +
> +static void main(unsigned long bist)
> +{
> +	unsigned long x;
> +	device_t dev;
> +	
> +	/* multifunction for northbridge */

Please elaborate, the comment is a bit unclear.
> +	pci_write_config8(ctrl.d0f0, 0x4f, 0x01);
> +	
> +	enable_vt8235_serial();
> +	uart_init();
> +	console_init();
> +
> +	print_spew("In auto.c:main()\r\n");
> +	
> +	enable_smbus();
> +	smbus_fixup(&ctrl);
> +
> +	if (bist == 0) {
> +		print_debug("doing early_mtrr\r\n");
> +		early_mtrr_init();
> +	}
> +
> +	/* Halt if there was a built in self test failure */
> +	report_bist_failure(bist);
> +
> +	print_debug(" Enabling mainboard devices\r\n");
> +	enable_mainboard_devices();
> +
> +	ddr_ram_setup(&ctrl);
> +
> +	/* Check all of memory */
> +#if 0
> +	ram_check(0x0, 0x1000);
> +#endif
> +#if 0
> +	static const struct {
> +		unsigned long lo, hi;
> +	} check_addrs[] = {
> +		/* Check 16MB of memory @ 0*/
> +		{ 0x000fff00, 0x001000ff },
> +#if TOTAL_CPUS > 1
> +		/* Check 16MB of memory @ 2GB */
> +		{ 0x80000000, 0x81000000 },
> +#endif
> +	};
> +	int i;
> +	for(i = 0; i < sizeof(check_addrs)/sizeof(check_addrs[0]); i++) {
> +		ram_check(check_addrs[i].lo, check_addrs[i].hi);
> +	}
> +#endif

I'd drop the whole block, if needed leave a single line
(or two) of commented ram_check() call(s), but the above is
a bit too verbose IMO.
> +
> +	print_spew("Leaving auto.c:main()\r\n");
> +}
> Index: src/mainboard/via/epia-cn/cmos.layout
> ===================================================================
> --- src/mainboard/via/epia-cn/cmos.layout	(revision 0)
> +++ src/mainboard/via/epia-cn/cmos.layout	(revision 0)
> @@ -0,0 +1,74 @@
> +entries
> +
> +#start-bit length  config config-ID    name
> +#0            8       r       0        seconds
> +#8            8       r       0        alarm_seconds
> +#16           8       r       0        minutes
> +#24           8       r       0        alarm_minutes
> +#32           8       r       0        hours
> +#40           8       r       0        alarm_hours
> +#48           8       r       0        day_of_week
> +#56           8       r       0        day_of_month
> +#64           8       r       0        month
> +#72           8       r       0        year
> +#80           4       r       0        rate_select
> +#84           3       r       0        REF_Clock
> +#87           1       r       0        UIP
> +#88           1       r       0        auto_switch_DST
> +#89           1       r       0        24_hour_mode
> +#90           1       r       0        binary_values_enable
> +#91           1       r       0        square-wave_out_enable
> +#92           1       r       0        update_finished_enable
> +#93           1       r       0        alarm_interrupt_enable
> +#94           1       r       0        periodic_interrupt_enable
> +#95           1       r       0        disable_clock_updates
> +#96         288       r       0        temporary_filler
> +0          384       r       0        reserved_memory
> +384          1       e       4        boot_option
> +385          1       e       4        last_boot
> +386          1       e       1        ECC_memory
> +388          4       r       0        reboot_bits
> +392          3       e       5        baud_rate
> +400          1       e       1        power_on_after_fail
> +412          4       e       6        debug_level
> +416          4       e       7        boot_first
> +420          4       e       7        boot_second
> +424          4       e       7        boot_third
> +428          4       h       0        boot_index
> +432	     8       h       0        boot_countdown
> +1008         16      h       0        check_sum
> +
> +enumerations
> +
> +#ID value   text
> +1     0     Disable
> +1     1     Enable
> +2     0     Enable
> +2     1     Disable
> +4     0     Fallback
> +4     1     Normal
> +5     0     115200
> +5     1     57600
> +5     2     38400
> +5     3     19200
> +5     4     9600
> +5     5     4800
> +5     6     2400
> +5     7     1200
> +6     6     Notice
> +6     7     Info
> +6     8     Debug
> +6     9     Spew
> +7     0     Network
> +7     1     HDD
> +7     2     Floppy
> +7     8     Fallback_Network
> +7     9     Fallback_HDD
> +7     10    Fallback_Floppy
> +#7     3     ROM
> +
> +checksums
> +
> +checksum 392 1007 1008
> +
> +
> Index: src/mainboard/via/epia-cn/mainboard.c
> ===================================================================
> --- src/mainboard/via/epia-cn/mainboard.c	(revision 0)
> +++ src/mainboard/via/epia-cn/mainboard.c	(revision 0)

Add the usual license header, please.
> @@ -0,0 +1,10 @@
> +#include <device/device.h>
> +#include <device/pci.h>
> +#include <device/pci_ids.h>
> +#include <device/pci_ops.h>

These three are not needed, I think (but please test).
> +#include "chip.h"
> +
> +struct chip_operations mainboard_via_epia_cn_ops = {
> +	CHIP_NAME("VIA EPIA-CN Mainboard")
> +};
> +
> Index: src/northbridge/via/cn700/raminit.c
> ===================================================================
> --- src/northbridge/via/cn700/raminit.c	(revision 3305)
> +++ src/northbridge/via/cn700/raminit.c	(working copy)
> @@ -1,6 +1,8 @@
>  /*
>   * This file is part of the coreboot project.
>   *
> + * Copyright (C) 2008 VIA Technologies, Inc.
> + * Copyright (C) 2008 Aaron Lwe <[e-mail]>

See above, are both correct?
> * Copyright (C) 2007 Corey Osgood <[e-mail]>
>   *
>   * This program is free software; you can redistribute it and/or modify
> @@ -55,11 +57,8 @@
>   PRINT_DEBUG_MEM(" to 0x");
>   PRINT_DEBUG_MEM_HEX32(0 + addr_offset);
>   PRINT_DEBUG_MEM("\r\n");
> -
> -	read32(0 + addr_offset);
>  }
>
> -
>  /**
>   * Configure the bus between the cpu and the northbridge. This might be able to 
>   * be moved to post-ram code in the future. For the most part, these registers
> @@ -83,7 +82,7 @@
>   /* Arbitration */
>   pci_write_config8(dev, 0x53, 0x88);
>   /* Miscellaneous Control */
> -	pci_write_config8(dev, 0x54, 0x10);
> +	pci_write_config8(dev, 0x54, 0x1e);
>   pci_write_config8(dev, 0x55, 0x16);
>   /* Write Policy */
>   pci_write_config8(dev, 0x56, 0x01);
> @@ -93,9 +92,8 @@
>    *	010 : 166MHz	011 : 200MHz
>    *	100 : 266MHz	101 : 333MHz
>    *	110/111 : Reserved */
> -	//pci_write_config8(dev, 0x57, 0x60);//set 200MHz dram clock
>   /* CPU Miscellaneous Control */
> -	pci_write_config8(dev, 0x59, 0x60);
> +	pci_write_config8(dev, 0x59, 0x44);	
>   /* Write Policy */
>   pci_write_config8(dev, 0x5d, 0xb2);
>   /* Bandwidth Timer */
> @@ -113,16 +111,16 @@
>   pci_write_config8(dev, 0x65, 0x0f);
>   /* Read Line Burst DRDY# Timing Control */
>   pci_write_config8(dev, 0x66, 0xff);
> -	pci_write_config8(dev, 0x67, 0x70);
> +	pci_write_config8(dev, 0x67, 0x30);
>
> /* Host Bus IO Circuit (See datasheet) */
>   /* Host Address Pullup/down Driving */
> -	pci_write_config8(dev, 0x70, 0x33);
> -	pci_write_config8(dev, 0x71, 0x00);
> -	pci_write_config8(dev, 0x72, 0x33);
> -	pci_write_config8(dev, 0x73, 0x00);
> +	pci_write_config8(dev, 0x70, 0x11);
> +	pci_write_config8(dev, 0x71, 0x11);
> +	pci_write_config8(dev, 0x72, 0x11);
> +	pci_write_config8(dev, 0x73, 0x11);
>   /* Miscellaneous Control */
> -	pci_write_config8(dev, 0x74, 0x00);
> +	pci_write_config8(dev, 0x74, 0x35);
>   /* AGTL+ I/O Circuit */
>   pci_write_config8(dev, 0x75, 0x28);
>   /* AGTL+ Compensation Status */
> @@ -135,206 +133,297 @@
>   pci_write_config8(dev, 0x79, 0xaa);
>   /* Address Strobe Input Delay Control */
>   pci_write_config8(dev, 0x7a, 0x24);
> -	/* Address CKG Rising/Falling Time Control */
> -	pci_write_config8(dev, 0x7b, 0x00);
> +	// Address CKG Rising/Falling Time Control
> +	pci_write_config8(dev, 0x7b, 0xaa);
>   /* Address CKG Clock Rising/Falling Time Control */
>   pci_write_config8(dev, 0x7c, 0x00);
>   /* Undefined (can't remember why I did this) */
> -	pci_write_config8(dev, 0x7d, 0x6d);	
> +	pci_write_config8(dev, 0x7d, 0x6d);
> +	pci_write_config8(dev, 0x7e, 0x00);
> +	pci_write_config8(dev, 0x7f, 0x00);
> +	pci_write_config8(dev, 0x80, 0x1b);
> +	pci_write_config8(dev, 0x81, 0x0a);
> +	pci_write_config8(dev, 0x82, 0x0a);
> +	pci_write_config8(dev, 0x83, 0x0a);
>  }
>
> /**
> + * Set up dram size according to spd data. Eventually, DRAM timings should be 
> + * done in a similar manner.
> + *
> + * @param ctrl The northbridge devices and spd addresses.
> + */
> +static void sdram_set_size(const struct mem_controller *ctrl)
> +{
> +	u8 density, ranks, result, col;
> +
> +	ranks = spd_read_byte(ctrl->channel0[0], SPD_NUM_DIMM_BANKS);
> +	ranks = (ranks & 0x07) + 1;
> +	density = spd_read_byte(ctrl->channel0[0], SPD_DENSITY_OF_EACH_ROW_ON_MODULE);
> +	switch (density)
> +	{
> +		case 0x80:
> +			result = 0x08;	/* 512MB / 64MB = 0x08 */
> +			break;
> +		case 0x40:
> +			result = 0x04;
> +			break;
> +		case 0x20:
> +			result = 0x02;
> +			break;
> +		case 0x10:
> +			result = 0xff;	/* 16GB */
> +			break;
> +		case 0x08:
> +			result = 0xff;	/* 8GB */
> +			break;
> +		case 0x04:
> +			result = 0xff;	/* 4GB */
> +			break;
> +		case 0x02:
> +			result = 0x20;	/* 2GB */
> +			break;
> +		case 0x01:
> +			result = 0x10;	/* 1GB */
> +			break;
> +	}
> +
> +	if (result == 0xff)
> +		die("dram module size too big, not supported by cn700\r\n");
> +
> +	pci_write_config8(ctrl->d0f3, 0x40, result);
> +	pci_write_config8(ctrl->d0f3, 0x48, 0x00);
> +	if (ranks == 2) {
> +		pci_write_config8(ctrl->d0f3, 0x41, result * ranks);
> +		pci_write_config8(ctrl->d0f3, 0x49, result);
> +	}
> +	/* size mirror */
> +	pci_write_config8(ctrl->d0f7, 0xe5, (result * ranks) << 2);
> +	pci_write_config8(ctrl->d0f7, 0x57, (result * ranks) << 2);
> +	/* Low Top Address */
> +	pci_write_config8(ctrl->d0f3, 0x84, 0x00);
> +	pci_write_config8(ctrl->d0f3, 0x85, (result * ranks) << 2);
> +	pci_write_config8(ctrl->d0f3, 0x88, (result * ranks) << 2);
> +
> +	/* Physical-Virtual Mapping */
> +	if (ranks == 2)
> +		pci_write_config8(ctrl->d0f3, 0x54, 1 << 7 | 0 << 4 | 1 << 3 | 1 << 0);
> +	if (ranks == 1)
> +		pci_write_config8(ctrl->d0f3, 0x54, 1 << 7 | 0 << 4);
> +	pci_write_config8(ctrl->d0f3, 0x55, 0x00);
> +	/* virtual rank interleave, disable */
> +	pci_write_config32(ctrl->d0f3, 0x58, 0x00);
> +
> +	/* MA Map Type */
> +	result = spd_read_byte(ctrl->channel0[0], SPD_NUM_BANKS_PER_SDRAM);
> +	if (result == 8) {
> +		col = spd_read_byte(ctrl->channel0[0], SPD_NUM_COLUMNS);
> +		switch (col)
> +		{
> +			case 10:
> +				pci_write_config8(ctrl->d0f3, 0x50, 0xa0);
> +				break;
> +			case 11:
> +				pci_write_config8(ctrl->d0f3, 0x50, 0xc0);
> +				break;
> +			case 12:
> +				pci_write_config8(ctrl->d0f3, 0x50, 0xe0);
> +				break;
> +		}
> +	}
> +	else if (result == 4) {
> +		col = spd_read_byte(ctrl->channel0[0], SPD_NUM_COLUMNS);
> +		switch (col)
> +		{
> +			case 9:
> +				pci_write_config8(ctrl->d0f3, 0x50, 0x00);
> +				break;
> +			case 10:
> +				pci_write_config8(ctrl->d0f3, 0x50, 0x20);
> +				break;
> +			case 11:
> +				pci_write_config8(ctrl->d0f3, 0x50, 0x40);
> +				break;
> +			case 12:
> +				pci_write_config8(ctrl->d0f3, 0x50, 0x60);
> +				break;
> +		}
> +	}

Minor issue (I'd leave that until after the initial commit): the above can
likely be converted to simpler and shorter code using a table/struct for
the values. Same goes for the 'switch (density)' above.
> +	pci_write_config8(ctrl->d0f3, 0x51, 0x00);
> +}
> +
> +/**
>   * Set up various ram and other control registers statically. Some of these may 
>   * not be needed, other should be done with spd info, but that's a project for
>   * the future
>   */
>  static void sdram_set_registers(const struct mem_controller *ctrl)
>  {
> +	u8 reg;
> +
> +	/* Set WR=5 */
> +	pci_write_config8(ctrl->d0f3, 0x61, 0xe0);
> +	/* Set CAS=4 */
> +	pci_write_config8(ctrl->d0f3, 0x62, 0xfa);
> +	/* dram timing-3 */
> +	pci_write_config8(ctrl->d0f3, 0x63, 0xca);
> +	/* dram timing-4 */
> +	pci_write_config8(ctrl->d0f3, 0x64, 0xcc);
> +	/* DIMM command / Address Selection */
> +	pci_write_config8(ctrl->d0f3, 0x67, 0x00);
> +	/* Disable cross bank/multi page mode */
> +	pci_write_config8(ctrl->d0f3, 0x69, 0x00);
> +	/* disable refresh now */
> +	pci_write_config8(ctrl->d0f3, 0x6a, 0x00);
> +
> +	/* frequency 100MHZ */
> +	pci_write_config8(ctrl->d0f3, 0x90, 0x00);
> +	pci_write_config8(ctrl->d0f2, 0x57, 0x18);
> +	/* Allow manual dll reset */
> +	pci_write_config8(ctrl->d0f3, 0x6b, 0x10);
> +
> +	/* Bank/Rank Interleave Address Select */
> +	pci_write_config8(ctrl->d0f3, 0x52, 0x33);
> +	pci_write_config8(ctrl->d0f3, 0x53, 0x3f);
> +	
> +	/* Set to DDR2 sdram, BL=8 (0xc8, 0xc0 for bl=4) */
> +	pci_write_config8(ctrl->d0f3, 0x6c, 0xc8);
> +
> +	/* DRAM Bus Turn-Around Setting */
> +	pci_write_config8(ctrl->d0f3, 0x60, 0x03);
> +	/* DRAM Arbitration Control */
> +	pci_write_config8(ctrl->d0f3, 0x66, 0x80);
> +	/* DQS Tuning: testing on a couple different boards has shown this is
> +	 * static, or close enough that it can be. Which is good, because the
> + 	 * tuning function used too many registers
> +	 */
> +	/* DQS Output Delay for CHannel A */
> +	pci_write_config8(ctrl->d0f3, 0x70, 0x00);
> +	/* MD Output Delay for Channel A */
> +	pci_write_config8(ctrl->d0f3, 0x71, 0x01);
> +	pci_write_config8(ctrl->d0f3, 0x73, 0x01);
> +
> +	/* dram arbitration timer */
> +	pci_write_config8(ctrl->d0f3, 0x65, 0xd9);
> +
> +	/* dram signal timing control */
> +	pci_write_config8(ctrl->d0f3, 0x74, 0x01);
> +	pci_write_config8(ctrl->d0f3, 0x75, 0x01);
> +	pci_write_config8(ctrl->d0f3, 0x76, 0x06);
> +	pci_write_config8(ctrl->d0f3, 0x77, 0x92);
> +	pci_write_config8(ctrl->d0f3, 0x78, 0x83);
> +	pci_write_config8(ctrl->d0f3, 0x79, 0x83);
> +	pci_write_config8(ctrl->d0f3, 0x7a, 0x00);
> +	pci_write_config8(ctrl->d0f3, 0x7b, 0x10);
> +
> +	/* dram clocking control */
> +	pci_write_config8(ctrl->d0f3, 0x91, 0x01);
> +	/* CS/CKE Clock Phase Control */
> +	pci_write_config8(ctrl->d0f3, 0x92, 0x02);
> +	/* SCMD/MA Clock Phase Control */
> +	pci_write_config8(ctrl->d0f3, 0x93, 0x02);
> +	/* DCLKO Feedback Mode Output Control */
> +	pci_write_config8(ctrl->d0f3, 0x94, 0x00);
> +	pci_write_config8(ctrl->d0f3, 0x9d, 0x0f);
> +	
> +	/* SDRAM ODT Control */
> +	pci_write_config8(ctrl->d0f3, 0xda, 0x80);
> +	/* Channel A DQ/DQS CKG Output Delay Control */
> +	pci_write_config8(ctrl->d0f3, 0xdc, 0x54);
> +	/* Channel A DQ/DQS CKG Output Delay Control */
> +	pci_write_config8(ctrl->d0f3, 0xdd, 0x55);
> +	/* odt lookup table */
> +	pci_write_config8(ctrl->d0f3, 0xd8, 0x01);
> +	pci_write_config8(ctrl->d0f3, 0xd9, 0x0a);
> +
> +	/* ddr sdram control */
> +	pci_write_config8(ctrl->d0f3, 0x6d, 0xc0);
> +	pci_write_config8(ctrl->d0f3, 0x6f, 0x41);
> +
>   /* DQ/DQS Strength Control */
> -	pci_write_config8(ctrl->d0f3, 0xd0, 0x88);
> -	pci_write_config8(ctrl->d0f3, 0xd1, 0x8b);
> -	pci_write_config8(ctrl->d0f3, 0xd2, 0x89);
> -	/* SMM and APIC Decoding */
> -	pci_write_config8(ctrl->d0f3, 0x86, 0x2d);
> -	
> -	/* Driving selection */
> -	/* DQ / DQS ODT Driving and Range Select */
> +	pci_write_config8(ctrl->d0f3, 0xd0, 0xaa);
> +
> +	/* Compensation Control */
> +	pci_write_config8(ctrl->d0f3, 0xd3, 0x01); /*enable auto compensation*/
> +	/* ODT (some are set with driving select above) */
> +	pci_write_config8(ctrl->d0f3, 0xd4, 0x80);
>   pci_write_config8(ctrl->d0f3, 0xd5, 0x8a);
>   /* Memory Pads Driving and Range Select */
>   pci_write_config8(ctrl->d0f3, 0xd6, 0xaa);
> -	/* DRAM Driving – Group DQS */
> +
>   pci_write_config8(ctrl->d0f3, 0xe0, 0xee);
> -	/* DRAM Driving – Group DQ (DQ, MPD, DQM) */
> -	pci_write_config8(ctrl->d0f3, 0xe2, 0xac);//ba
> -	/* DRAM Driving – Group CS */
> +	pci_write_config8(ctrl->d0f3, 0xe2, 0xac);
>   pci_write_config8(ctrl->d0f3, 0xe4, 0x66);
> -	/* DRAM Driving – Group MA */
> +	pci_write_config8(ctrl->d0f3, 0xe6, 0x33);
>   pci_write_config8(ctrl->d0f3, 0xe8, 0x86);
> -	/* DRAM Driving – Group MCLK */
> -	pci_write_config8(ctrl->d0f3, 0xe6, 0xaa);
> -
> -	/* ODT (some are set with driving select above) */
> -	/* Memory Pad ODT Pullup / Pulldown Control */
> -	pci_write_config8(ctrl->d0f3, 0xd4, 0x0a);
> -	/* Memory Ranks ODT Lookup Table */
> -	pci_write_config8(ctrl->d0f3, 0xd8, 0x00);//was 1
> -	/* Compensation Control */
> -	pci_write_config8(ctrl->d0f3, 0xd3, 0x89);//enable auto compensation
> -	
> -	/* MCLKO Phase Control */
> -	pci_write_config8(ctrl->d0f3, 0x91, 0x02);
> -	/* CS/CKE Clock Phase Control */
> -	pci_write_config8(ctrl->d0f3, 0x92, 0x06);
> -	/* SCMD/MA Clock Phase Control */
> -	pci_write_config8(ctrl->d0f3, 0x93, 0x07);
> -	/* Channel A DQS Input Capture Range Control */
> -	pci_write_config8(ctrl->d0f3, 0x78, 0x83);
> -	/* DQS Input Capture Range Control */
> -	/* Set in accordance with the BIOS update note */
> -	pci_write_config8(ctrl->d0f3, 0x7a, 0x00);
> -	/* DQS Input Delay Offset Control */
> -	pci_write_config8(ctrl->d0f3, 0x7c, 0x00);
> -	/* SDRAM ODT Control */
> -	pci_write_config8(ctrl->d0f3, 0xda, 0x80);
> -	/* DQ/DQS CKG Output Delay Control - I */
> -	pci_write_config8(ctrl->d0f3, 0xdc, 0xff);
> -	/* DQ/DQS CKG Output Delay Control - II */
> -	pci_write_config8(ctrl->d0f3, 0xdd, 0xff);
>   /* DQS / DQ CKG Duty Cycle Control */
> -	pci_write_config8(ctrl->d0f3, 0xec, 0x88);
> +	pci_write_config8(ctrl->d0f3, 0xec, 0x00);
>   /* MCLK Output Duty Control */
>   pci_write_config8(ctrl->d0f3, 0xee, 0x00);
> -	pci_write_config8(ctrl->d0f3, 0xed, 0x10);
>   /* DQS CKG Input Delay Control */
>   pci_write_config8(ctrl->d0f3, 0xef, 0x10);
> -
> -	pci_write_config8(ctrl->d0f3, 0x77, 0x9d);
> -	pci_write_config8(ctrl->d0f3, 0x79, 0x83);
> -	pci_write_config16(ctrl->d0f3, 0x88, 0x0020);
>
> -	pci_write_config8(ctrl->d0f4, 0xa7, 0x80);
> +	/* dram duty control */
> +	pci_write_config8(ctrl->d0f3, 0xed, 0x10);
>
> -	/* VLink Control */
> -	pci_write_config8(ctrl->d0f7, 0xb0, 0x05);
> -	pci_write_config8(ctrl->d0f7, 0xb1, 0x01);
> -	
> -	/* Memory base */
> -	pci_write_config16(ctrl->d1f0, 0x20, 0xfb00);
> -	/* Memory limit */
> -	pci_write_config16(ctrl->d1f0, 0x22, 0xfcf0);
> -	/* Prefetch memory base */
> -	pci_write_config16(ctrl->d1f0, 0x24, 0xf400);
> -	/* Prefetch memory limit */
> -	pci_write_config16(ctrl->d1f0, 0x26, 0xf7f0);
> -	/* PCI to PCI bridge control */
> -	pci_write_config16(ctrl->d1f0, 0x3e, 0x0008);
> -	
> -	/* CPU to PCI flow control 1 */
> -	pci_write_config8(ctrl->d1f0, 0x40, 0x83);
> -	pci_write_config8(ctrl->d1f0, 0x41, 0xc3);//clear reset error, set to 43
> -	pci_write_config8(ctrl->d1f0, 0x42, 0xe2);
> -	pci_write_config8(ctrl->d1f0, 0x43, 0x44);
> -	pci_write_config8(ctrl->d1f0, 0x44, 0x34);
> -	pci_write_config8(ctrl->d1f0, 0x45, 0x72);
> +	/* SMM and APIC deocoding, we donot use SMM */
> +	pci_write_config8(ctrl->d0f3, 0x86, 0x29);
> +	/* SMM and APIC decoding mirror */
> +	pci_write_config8(ctrl->d0f7, 0xe6, 0x29);
>
> -	/* Disable cross bank/multi page mode */
> -	pci_write_config8(ctrl->d0f3, DDR_PAGE_CTL, 0x80);
> -	pci_write_config8(ctrl->d0f3, DRAM_REFRESH_COUNTER, 0x00);
> -
> -	/* Set WR=5 and RFC */
> -	pci_write_config8(ctrl->d0f3, 0x61, 0xc7);
> -	/* Set CAS=5 */
> -	pci_write_config8(ctrl->d0f3, 0x62, 0xaf);
> -	pci_write_config8(ctrl->d0f3, 0x63, 0xca);
> -	/* Set to DDR2 sdram, BL=8 (0xc8, 0xc0 for bl=4) */
> -	pci_write_config8(ctrl->d0f3, 0x6c, 0xc8);
> -	/* Allow manual dll reset */
> -	pci_write_config8(ctrl->d0f3, 0x6b, 0x10);
> -	
> +	/* dram module configuration */
>   pci_write_config8(ctrl->d0f3, 0x6e, 0x89);
> -	pci_write_config8(ctrl->d0f3, 0x67, 0x50);
> -	pci_write_config8(ctrl->d0f3, 0x65, 0xd9);
> -	
> -	/* Only enable bank 1, for now */
> -	/* TODO: Multiple, dynamically controlled bank enables */
> -	pci_write_config8(ctrl->d0f3, 0x54, 0x80);
> -	pci_write_config8(ctrl->d0f3, 0x55, 0x00);
> -	
> -	/* Set to 2T, MA Map type 1. 
> -	 * TODO: Needs to become dynamic */
> -	pci_write_config16(ctrl->d0f3, 0x50, 0x0020);
> +}
>
> -	/* BA0-2 Selection. Don't mess with */
> -	pci_write_config8(ctrl->d0f3, 0x52, 0x33);
> -	pci_write_config8(ctrl->d0f3, 0x53, 0x3f);
> -	
> -	/* Disable bank interleaving. This feature seems useless anyways */
> -	pci_write_config32(ctrl->d0f3, 0x58, 0x00000000);
> -	pci_write_config8(ctrl->d0f3, 0x88, 0x08);
> +static void sdram_set_post(const struct mem_controller *ctrl)
> +{
> +	device_t dev = ctrl->d0f3;
> +	/* Enable multipage mode. */
> +	pci_write_config8(dev, 0x69, 0x03);
>
> -	/* Some DQS control stuffs */
> -	pci_write_config8(ctrl->d0f3, 0x74, 0x04);
> -	pci_write_config8(ctrl->d0f3, 0x75, 0x04);
> -	pci_write_config8(ctrl->d0f3, 0x76, 0x00);
> -}
> +	/* Enable refresh. */
> +	pci_write_config8(dev, 0x6a, 0x32);
>
> -/**
> - * Set up dram size according to spd data. Eventually, DRAM timings should be 
> - * done in a similar manner.
> - *
> - * @param ctrl The northbridge devices and spd addresses.
> - */
> -static void sdram_set_spd_registers(const struct mem_controller *ctrl)
> -{
> -	u8 spd_data, spd_data2;
> +	// vga device
> +	pci_write_config16(dev, 0xa0, (1 <<15));
> +	pci_write_config16(dev, 0xa4, 0x0010);
>
> -	/* DRAM Bank Size */
> -	spd_data = spd_read_byte(ctrl->channel0[0],
> -					SPD_DENSITY_OF_EACH_ROW_ON_MODULE);
> -	/* I know this seems weird. Blame JEDEC/Via. */
> -	if(spd_data >= 0x10)
> -		spd_data = spd_data >> 1;
> -	else
> -		spd_data = spd_data << 1;
> -
> -	/* Check for double sided dimm and adjust size accordingly */
> -	spd_data2 = spd_read_byte(ctrl->channel0[0], SPD_NUM_BANKS_PER_SDRAM);
> -	/* There should be 4 banks on a single sided dimm, 
> -	 * or 8 on a dual sided one */
> -	spd_data = spd_data * (spd_data2 / 4);
> -	pci_write_config8(ctrl->d0f3, 0x40, spd_data);
> -	/* TODO: The rest of the DIMMs */
>  }
>
> -static void sdram_enable(device_t dev)
> +static void sdram_enable(device_t dev, unsigned long rank_address)
>  {
> -	int i;
> +	u8 i;
>
> /* 1. Apply NOP. */
>   PRINT_DEBUG_MEM("RAM Enable 1: Apply NOP\r\n");
> -	do_ram_command(dev, RAM_COMMAND_NOP, 0);
> -	udelay(200);
> +	do_ram_command(dev, RAM_COMMAND_NOP, 0x100);
> +	udelay(100);
> +	read32(rank_address + 0x10);
>
> /* 2. Precharge all. */
> +	udelay(400);
>   PRINT_DEBUG_MEM("RAM Enable 2: Precharge all\r\n");
> -	do_ram_command(dev, RAM_COMMAND_PRECHARGE, 0);
> +	do_ram_command(dev, RAM_COMMAND_PRECHARGE, 0x200);
> +	read32(rank_address + 0x10);
>
> /* 3. Mode register set. */
>   PRINT_DEBUG_MEM("RAM Enable 4: Mode register set\r\n");
> -	do_ram_command(dev, RAM_COMMAND_MRS, 0x2000);//enable dll
> -	do_ram_command(dev, RAM_COMMAND_MRS, 0x800);//reset dll
> +	do_ram_command(dev, RAM_COMMAND_MRS, 0x120020);//enable dll
> +	read32(rank_address + 0x120000);
> +	read32(rank_address + 0x800);
>
> /* 4. Precharge all again. */
>   PRINT_DEBUG_MEM("RAM Enable 2: Precharge all\r\n");
> -	do_ram_command(dev, RAM_COMMAND_PRECHARGE, 0);
> +	do_ram_command(dev, RAM_COMMAND_PRECHARGE, 0x300);
> +	read32(rank_address + 0x0);
>
> /* 5. Perform 8 refresh cycles. Wait tRC each time. */
>   PRINT_DEBUG_MEM("RAM Enable 3: CBR\r\n");
> -	do_ram_command(dev, RAM_COMMAND_CBR, 0);
> +	do_ram_command(dev, RAM_COMMAND_CBR, 0x400);
>   /* First read is actually done by do_ram_command */
> -	for(i = 0; i < 7; i++) {
> +	for(i = 0; i < 8; i++) {
> +		read32(rank_address + 0x20);
>       udelay(100);
> -		read32(0);
>   }
>
> /* 6. Mode register set. */
> @@ -343,31 +432,53 @@
>   /* (E)MRS values are from the BPG. No direct explanation is given, but 
>    * they should somehow conform to the JEDEC DDR2 SDRAM Specification
>    * (JESD79-2C). */
> -	do_ram_command(dev, RAM_COMMAND_MRS, 0x0022d8);
> +	do_ram_command(dev, RAM_COMMAND_MRS, 0x0011d8);
> +	read32(rank_address + 0x002258);
> +	read32(rank_address + 0x121c20);
> +	read32(rank_address + 0x120020);
>
> -	/* 7. Mode register set. */
> -	PRINT_DEBUG_MEM("RAM Enable 4: Mode register set\r\n");
> -	do_ram_command(dev, RAM_COMMAND_MRS, 0x21c20);//default OCD calibration
> -	do_ram_command(dev, RAM_COMMAND_MRS, 0x20020);//exit calibration mode
> -	
>   /* 8. Normal operation */
>   PRINT_DEBUG_MEM("RAM Enable 5: Normal operation\r\n");
>   do_ram_command(dev, RAM_COMMAND_NORMAL, 0);
> -	
> -	/* Enable multipage mode. */
> -	pci_write_config8(dev, DDR_PAGE_CTL, 0x83);
> -	/* Enable refresh. */
> -	pci_write_config8(dev, DRAM_REFRESH_COUNTER, 0x32);
> -	
> -	/* DQS Tuning: testing on a couple different boards has shown this is
> -	 * static, or close enough that it can be. Which is good, because the
> -	 * tuning function used too many registers. */
> -	pci_write_config8(dev, CH_A_DQS_OUTPUT_DELAY, 0x00);
> -	pci_write_config8(dev, CH_A_MD_OUTPUT_DELAY, 0x03);
> +	read32(rank_address + 0x30);
> +}
>
> -	/* Enable VGA device with no memory, add memory later. We need this
> -	 * here to enable the actual device, otherwise it won't show up until
> -	 * later and LB will have a fit. */
> -	pci_write_config16(dev, 0xa0, (1 << 15));
> -	pci_write_config16(dev, 0xa4, 0x0010);
> +static void dumpnorth(void)
> +{
> +	u16 i, j;
> +	device_t dev;
> +	for (dev = PCI_DEV(0, 0, 0); dev <= PCI_DEV(0, 1, 0); dev += PCI_DEV(0, 0, 1)) {
> +		if (dev == PCI_DEV(0, 0, 5) || dev == PCI_DEV(0, 0, 6))
> +			continue;
> +		print_debug("dumping device ");
> +		print_debug_hex32(dev);
> +		print_debug("\r\n");
> +		for (i = 0; i < 0xFF; i += 0x10) {
> +			print_debug_hex8(i);
> +			print_debug(": ");
> +			for (j = 0; j < 0x10; j++) {
> +				print_debug_hex8(pci_read_config8(dev, i + j));
> +				print_debug(" ");
> +			}
> +			print_debug("\r\n");
> +		}
> +	}

Not sure we need this in here, I think there's a similar function in various
other debug.c files spread all over the repo. Can one of them be re-used?
> }
> +/*
> + * Support one dimm with up to 2 ranks
> + */
> +static void ddr_ram_setup(const struct mem_controller *ctrl)
> +{
> +	u8 reg;
> +
> +	c7_cpu_setup(ctrl->d0f2);
> +	sdram_set_registers(ctrl);
> +	sdram_set_size(ctrl);
> +	sdram_enable(ctrl->d0f3, 0);
> +	reg = pci_read_config8(ctrl->d0f3, 0x41);
> +	if (reg != 0)
> +		sdram_enable(ctrl->d0f3, pci_read_config8(ctrl->d0f3, 0x40) << 26);
> +	sdram_set_post(ctrl);
> +//	dumpnorth();
> +}
> +
> Index: src/northbridge/via/cn700/northbridge.c
> ===================================================================
> --- src/northbridge/via/cn700/northbridge.c	(revision 3305)
> +++ src/northbridge/via/cn700/northbridge.c	(working copy)
> @@ -1,6 +1,8 @@
>  /*
>   * This file is part of the coreboot project.
>   *
> + * Copyright (C) 2008 VIA Technologies, Inc.
> + * Copyright (C) 2008 Aaron Lwe <[e-mail]>

See above.
> * Copyright (C) 2007 Corey Osgood <[e-mail]>
>   *
>   * This program is free software; you can redistribute it and/or modify
> @@ -35,9 +37,8 @@
>  static void memctrl_init(device_t dev)
>  {
>   u16 reg16;
> +	u8 ranks;
>
> -	pci_write_config8(dev, 0x86, 0x2d);
> -	
>   /* Set up the vga framebuffer size */
>   reg16 = (log2(CONFIG_VIDEO_MB) << 12) | (1 << 15);
>   pci_write_config16(dev, 0xa0, reg16);
> @@ -45,8 +46,41 @@
>   /* Set up VGA timers */
>   pci_write_config8(dev, 0xa2, 0x44);
>
> -	pci_write_config16(dev, 0xb0, 0xaa60);
> +	for (ranks = 0x4b; ranks >= 0x48; ranks--) {
> +		if (pci_read_config8(dev, ranks)) {
> +			ranks -= 0x48;
> +			break;
> +		}
> +	}
> +	if (ranks == 0x47)
> +		ranks = 0x00;
> +	reg16 = 0xaae0;
> +	reg16 |= ranks;
> +	/* GMINT Misc. FrameBuffer rank */
> +	pci_write_config16(dev, 0xb0, reg16);
> +	/* AGPCINT Misc. */
>   pci_write_config8(dev, 0xb8, 0x08);
> +
> +	/* shadown ram */
> +	u8 shadowreg;
> +	shadowreg = pci_read_config8(dev, 0x83);
> +	/* 0xf0000-0xfffff */
> +	shadowreg |= 0x30;
> +	pci_write_config8(dev, 0x83, shadowreg);
> +	/* PAGE C, D, E are all read write enable */
> +	pci_write_config8(dev, 0x80, 0xff);
> +	pci_write_config8(dev, 0x81, 0xff);
> +	pci_write_config8(dev, 0x82, 0xff);
> +	/* vlink mirror */
> +	dev = dev_find_device(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_CN700_VLINK, 0);
> +	if (dev) {
> +		pci_write_config8(dev, 0x61, 0xff);
> +		pci_write_config8(dev, 0x62, 0xff);
> +		pci_write_config8(dev, 0x64, 0xff);
> +		shadowreg = pci_read_config8(dev, 0x63);
> +		shadowreg |= 0x30;
> +		pci_write_config8(dev, 0x63, shadowreg);
> +	}
>  }
>
> static const struct device_operations memctrl_operations = {
> @@ -124,7 +158,7 @@
>
> static void pci_domain_set_resources(device_t dev)
>  {
> -	static const u8 ramregs[] = {0x40, 0x41, 0x42, 0x43};
> +	static const u8 ramregs[] = {0x43, 0x42, 0x41, 0x40};

Does the order matter? If yes, please add a comment saying why.
> device_t mc_dev;
>          u32 pci_tolm;
>
> @@ -133,7 +167,7 @@
>          pci_tolm = find_pci_tolm(&dev->link[0]);
>   mc_dev = dev_find_device(PCI_VENDOR_ID_VIA, 
>       		PCI_DEVICE_ID_VIA_CN700_MEMCTRL, 0);
> -	
> +
>   if (mc_dev) {
>       unsigned long tomk, tolmk;
>       unsigned char rambits;
> @@ -141,11 +175,13 @@
>
> for(rambits = 0, i = 0; i < ARRAY_SIZE(ramregs); i++) {
>       	unsigned char reg;
> -			reg = pci_read_config8(mc_dev, ramregs[i]);
> -			rambits += reg;
> +			rambits = pci_read_config8(mc_dev, ramregs[i]);
> +			if (rambits != 0)
> +				break;
>       }
>
> -		tomk = rambits;
> +		tomk = rambits * 64 * 1024;
> +		printk_spew("tomk is 0x%x\n", tomk);
>       /* Compute the Top Of Low Memory, in Kb */
>       tolmk = pci_tolm >> 10;
>       if (tolmk >= tomk) {
> @@ -156,9 +192,8 @@
>       idx = 10;
>       /* TODO: Hole needed? */
>       ram_resource(dev, idx++, 0, 640); /* first 640k */
> -		/* Leave a hole for vga */
> -		ram_resource(dev, idx++, 768, (tolmk - 768 -
> -						(CONFIG_VIDEO_MB * 1024)));
> +		/* Leave a hole for vga, 0xa0000 - 0xc0000 */
> +		ram_resource(dev, idx++, 768, (tolmk - 768 - CONFIG_VIDEO_MB * 1024));
>   }
>   assign_resources(&dev->link[0]);
>  }
> Index: targets/via/epia-cn/Config.lb
> ===================================================================
> --- targets/via/epia-cn/Config.lb	(revision 0)
> +++ targets/via/epia-cn/Config.lb	(revision 0)

Add the usual license header please.
> @@ -0,0 +1,45 @@
> +# Sample config file for EPIA-CN
> +# This will make a target directory of ./epia-CN

Drop these comments, not needed.
> +
> +target via_epia_cn
> +
> +mainboard via/epia-cn
> +
> +option  MAXIMUM_CONSOLE_LOGLEVEL=8
> +option  DEFAULT_CONSOLE_LOGLEVEL=8
> +option  CONFIG_CONSOLE_SERIAL8250=1
> +#
> +# generate the final rom like this: cat vgabios bochsbios coreboot.rom > coreboot.rom.final
> +#
> +option ROM_SIZE=512*1024 - 64 * 1024 - 64 * 1024
> +option HAVE_OPTION_TABLE=1
> +option CONFIG_ROM_PAYLOAD=1
> +option HAVE_FALLBACK_BOOT=1

Are these needed here? I think src/.../Options.lb should set their
defaults properly.
> +
> +###
> +### Compute the location and size of where this firmware image
> +### (coreboot plus bootloader) will live in the boot rom chip.
> +###
> +option FALLBACK_SIZE=ROM_SIZE
> +
> +## coreboot C code runs at this location in RAM
> +option _RAMBASE=0x00004000

Ditto, should be in Options.lb.
> +
> +#
> +###
> +### Compute the start location and size size of
> +### The coreboot bootloader.
> +###
> +
> +#
> +# EPIA-CN
> +#
> +romimage "fallback"

Just cosmetics (the name doesn't matter), but if you're not using a
normal/fallback mechanism, maybe it's better to name the romimage
name "image" or similar, "fallback" might be confusing in that case.
> +	option USE_FALLBACK_IMAGE=1
> +	option ROM_IMAGE_SIZE=0x20000
> +	option COREBOOT_EXTRA_VERSION=".0Fallback"
> +	payload $(HOME)/src/filo-0.5/filo.elf

Make it "payload ../payload.elf" please, I think we should do this
consistently for all v2 boards, if only to make the buildrom work easier.
> +end
> +
> +buildrom ./coreboot.rom ROM_SIZE "fallback"

Great work Aaron! Thanks a lot for the patch!


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

[bari]

It looks good!

I'll give my full ack after I have tested the new patch on actual 
hardware today.

I hope we can tackle the issue with SATA after that.

Thanks again Aaron.

-Bari

aaron lwe wrote:
> Hi all,
>
> I've cleaned up the code, adding back the comments and use defined
> macro to replace magic values. I've also removed acpi related files
> since I'm not sure of its ownership. Thank you all for your comments.
>
> -Aaron

[aaron lwe]

Hi,

I've added these headers and did some clean ups. Thanks for your comments.
Signed-off-by: Aaron Lwe <[e-mail]>

-Aaron

[Rudolf Marek]

Hi AAron

I just checked the stuff I know from other VIA chipsets, the mem init and 
configs I skipped, perhaps it is OK.
> Index: src/include/device/pci_ids.h
> ===================================================================
> --- src/include/device/pci_ids.h	(revision 3321)
> +++ src/include/device/pci_ids.h	(working copy)
> @@ -1160,6 +1160,7 @@
>  #define PCI_DEVICE_ID_VIA_K8T890CE_BR	0xb188
>  #define PCI_DEVICE_ID_VIA_VT6420_SATA	0x3149
>  #define PCI_DEVICE_ID_VIA_VT8237R_LPC	0x3227
> +#define PCI_DEVICE_ID_VIA_VT8237R_SATA	0x3149

Why not use PCI_DEVICE_ID_VIA_VT6420_SATA	as I did?
> #define PCI_DEVICE_ID_VIA_CN700_AGP	0x0314
>  #define PCI_DEVICE_ID_VIA_CN700_ERR	0x1314
>  #define PCI_DEVICE_ID_VIA_CN700_HOST	0x2314
> +}
> +
> +#include "northbridge/via/cn700/raminit.c"
> +
> +static void enable_mainboard_devices(void) 
> +{
> +	device_t dev;
> +	u8 reg;
> + 
> +       /*
> +	* If I enbale sata, filo will not find the ide disk, so I'll disable sata here
> +	* To not conflict with pci sepc, I'll move ide device from 00:0f.1 to 00:0f.0

Please try attached patch. Thanks. I'm booting from first SATA at filo with hd0 
as first SATA and second sata as hd2 imho the first ide disk will be hd4.
> +	*/	
> +	dev = pci_locate_device(PCI_ID(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_VT8237R_SATA), 0);
> +	if (dev != PCI_DEV_INVALID) {
> +		/* enable backdoor */
> +		reg = pci_read_config8(dev, 0xd1);
> +		reg |= 0x08;
> +		pci_write_config8(dev, 0xd1, reg);
> +		/* enable pata */
> +		reg = pci_read_config8(dev, 0x49);
> +		reg |= 0x80;
> +		pci_write_config8(dev, 0x49, reg);
> +	}
> +	else

snip
> /* AGTL+ Compensation Status */
> @@ -135,206 +133,297 @@
>   pci_write_config8(dev, 0x79, 0xaa);
>   /* Address Strobe Input Delay Control */
>   pci_write_config8(dev, 0x7a, 0x24);
> -	/* Address CKG Rising/Falling Time Control */
> -	pci_write_config8(dev, 0x7b, 0x00);
> +	// Address CKG Rising/Falling Time Control
> +	pci_write_config8(dev, 0x7b, 0xaa);
>   /* Address CKG Clock Rising/Falling Time Control */
>   pci_write_config8(dev, 0x7c, 0x00);
>   /* Undefined (can't remember why I did this) */

lol ;)
> -	pci_write_config8(dev, 0x7d, 0x6d);	
> +	pci_write_config8(dev, 0x7d, 0x6d);
> +	pci_write_config8(dev, 0x7e, 0x00);
> +	pci_write_config8(dev, 0x7f, 0x00);
> +	pci_write_config8(dev, 0x80, 0x1b);
> +	pci_write_config8(dev, 0x81, 0x0a);
> +	pci_write_config8(dev, 0x82, 0x0a);
> +	pci_write_config8(dev, 0x83, 0x0a);
> -	/* VLink Control */
> -	pci_write_config8(ctrl->d0f7, 0xb0, 0x05);
> -	pci_write_config8(ctrl->d0f7, 0xb1, 0x01);

Just a question do you have in plan to support also some other pairs like VT8237A/S?
> +	pci_write_config8(ctrl->d0f3, 0x86, 0x29);
> +	/* SMM and APIC decoding mirror */
> +	pci_write_config8(ctrl->d0f7, 0xe6, 0x29);

Hmm too dangerous please can you copy 0x86 reg to 0xe6?
> -	/* Disable cross bank/multi page mode */
> -	pci_write_config8(ctrl->d0f3, DDR_PAGE_CTL, 0x80);
> +
> Index: src/northbridge/via/cn700/northbridge.c
> ===================================================================
> --- src/northbridge/via/cn700/northbridge.c	(revision 3321)
> +++ src/northbridge/via/cn700/northbridge.c	(working copy)
> @@ -1,6 +1,8 @@
>  /*
>   * This file is part of the coreboot project.
>   *
> + * Copyright (C) 2008 VIA Technologies, Inc.
> + * (Written by Aaron Lwe <[e-mail]> for VIA)
>   * Copyright (C) 2007 Corey Osgood <[e-mail]>
>   *
>   * This program is free software; you can redistribute it and/or modify
> @@ -35,9 +37,8 @@
>  static void memctrl_init(device_t dev)
>  {
>   u16 reg16;
> +	u8 ranks;
>
> -	pci_write_config8(dev, 0x86, 0x2d);
> -	
>   /* Set up the vga framebuffer size */
>   reg16 = (log2(CONFIG_VIDEO_MB) << 12) | (1 << 15);
>   pci_write_config16(dev, 0xa0, reg16);
> @@ -45,8 +46,41 @@
>   /* Set up VGA timers */
>   pci_write_config8(dev, 0xa2, 0x44);
>
> -	pci_write_config16(dev, 0xb0, 0xaa60);
> +	for (ranks = 0x4b; ranks >= 0x48; ranks--) {
> +		if (pci_read_config8(dev, ranks)) {
> +			ranks -= 0x48;
> +			break;
> +		}
> +	}
> +	if (ranks == 0x47)
> +		ranks = 0x00;
> +	reg16 = 0xaae0;
> +	reg16 |= ranks;
> +	/* GMINT Misc. FrameBuffer rank */
> +	pci_write_config16(dev, 0xb0, reg16);
> +	/* AGPCINT Misc. */
>   pci_write_config8(dev, 0xb8, 0x08);
> +
> +	/* shadown ram */
> +	u8 shadowreg;
> +	shadowreg = pci_read_config8(dev, 0x83);
> +	/* 0xf0000-0xfffff */
> +	shadowreg |= 0x30;
> +	pci_write_config8(dev, 0x83, shadowreg);
> +	/* PAGE C, D, E are all read write enable */
> +	pci_write_config8(dev, 0x80, 0xff);
> +	pci_write_config8(dev, 0x81, 0xff);
> +	pci_write_config8(dev, 0x82, 0xff);
> +	/* vlink mirror */

Again here please can you do the copy of registers or someone else will forgot 
that below ff MUST be same as above ;)
> +	dev = dev_find_device(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_CN700_VLINK, 0);
> +	if (dev) {
> +		pci_write_config8(dev, 0x61, 0xff);
> +		pci_write_config8(dev, 0x62, 0xff);
> +		pci_write_config8(dev, 0x64, 0xff);
> +		shadowreg = pci_read_config8(dev, 0x63);
> +		shadowreg |= 0x30;
> +		pci_write_config8(dev, 0x63, shadowreg);

This is tricky with other then VT8237R there is new registers for whole range. 
Please can you check if it is OK I mean that only two bits are for the whole 
region? Typical symptoms are random disk read lockup and controller reset - when 
DMA goes to the region of 0xE0000-0xEFFFF.

Here is my note from: 
http://tracker.coreboot.org/trac/coreboot/browser/trunk/coreboot-v2/src/sout...

/*
  	         * All access bits for 0xE0000-0xEFFFF encode as just 2 bits!
  	         * So the NB reg is quite inconsistent, we expect there only 0xff or 
0x00,
  	         * and write them to 0x63 7-6 but! VIA 8237A has the mirror at 0x64!
  	         */

Rudolf

[Uwe Hermann]

On Thu, May 15, 2008 at 12:05:02PM +0800, aaron lwe wrote:
> I've added these headers and did some clean ups. Thanks for your comments.
> Signed-off-by: Aaron Lwe <[e-mail]>

Great, thanks. I build-tested it here and it works fine, some more
comments inline.
> Index: src/mainboard/via/epia-cn/Config.lb
> ===================================================================
> --- src/mainboard/via/epia-cn/Config.lb	(revision 0)
> +++ src/mainboard/via/epia-cn/Config.lb	(revision 0)
> @@ -0,0 +1,216 @@
> +##
> +## This file is part of the coreboot project.
> +## 
> +## Copyright (C) 2008 VIA Technologies, Inc.

Feel free to also add the 

 (Written by Aaron Lwe <[e-mail]> for VIA)

in each file (but that's optional I guess).
> +chip northbridge/via/cn700
> +	device pci_domain 0 on
> +		device pci 0.0 on end # AGP Bridge
> +		device pci 0.1 on end # Error Reporting
> +		device pci 0.2 on end # Host Bus Control
> +		device pci 0.3 on end # Memory Controller
> +		device pci 0.4 on end # Power Management
> +		device pci 0.7 on end # V-Link Controller
> +		device pci 1.0 on # PCI Bridge
> +		end # PCI Bridge

This looks confusing, please put the "end" on the same line.
> Index: src/mainboard/via/epia-cn/irq_tables.c
> ===================================================================
> --- src/mainboard/via/epia-cn/irq_tables.c	(revision 0)
> +++ src/mainboard/via/epia-cn/irq_tables.c	(revision 0)
> @@ -0,0 +1,53 @@
> +/*
> + * This file is part of the coreboot project.

Please add

  Copyright (C) 2008 VIA Technologies, Inc.
  (Written by Aaron Lwe <[e-mail]> for VIA)
> + *
> + * 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
> + */
> Index: src/mainboard/via/epia-cn/Options.lb
> ===================================================================
> --- src/mainboard/via/epia-cn/Options.lb	(revision 0)
> +++ src/mainboard/via/epia-cn/Options.lb	(revision 0)
> @@ -0,0 +1,172 @@
> +##
> +## This file is part of the coreboot project.
> +## 
> +## Copyright (C) 2008 VIA Technologies, Inc.

See above.
> +###
> +### coreboot layout values
> +###
> +
> +## ROM_IMAGE_SIZE is the amount of space to allow coreboot to occupy.
> +default ROM_IMAGE_SIZE = 0x20000

Change this to 0x10000, or rather:

  default ROM_IMAGE_SIZE = 64 * 1024

to make the actual coreboot code only half the size. I verified locally
that the epia-cn target will still build fine with the change.
> +       /*
> +	* If I enbale sata, filo will not find the ide disk, so I'll disable sata here
> +	* To not conflict with pci sepc, I'll move ide device from 00:0f.1 to 00:0f.0
> +	*/	
> +	dev = pci_locate_device(PCI_ID(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_VT8237R_SATA), 0);
> +	if (dev != PCI_DEV_INVALID) {
> +		/* enable backdoor */
> +		reg = pci_read_config8(dev, 0xd1);
> +		reg |= 0x08;
> +		pci_write_config8(dev, 0xd1, reg);

Can you add a more detailed comment about what exactly this
does and why? It sure sounds scary for someone who hasn't read
the datasheets (i.e. someone like me).
> +	do_ram_command(dev, RAM_COMMAND_MRS, 0x0011d8);
> +	read32(rank_address + 0x002258);
> +	read32(rank_address + 0x121c20);
> +	read32(rank_address + 0x120020);

Stuff like this could use a comment as it's highly non-obvious what is
going on, especially without datasheets.
> Index: targets/via/epia-cn/Config.lb
> ===================================================================
> --- targets/via/epia-cn/Config.lb	(revision 0)
> +++ targets/via/epia-cn/Config.lb	(revision 0)
> @@ -0,0 +1,45 @@
> +##
> +## This file is part of the coreboot project.
> +## 
> +## Copyright (C) 2008 VIA Technologies, Inc.

See above.
> +##
> +## 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
> +##
> +
> +target via_epia_cn
> +
> +mainboard via/epia-cn
> +option  CONFIG_CONSOLE_SERIAL8250=1

This should also go into Options.lb.
> +#
> +# generate the final rom like this: cat vgabios bochsbios coreboot.rom > coreboot.rom.final
> +#
> +option ROM_SIZE=512*1024 - 64 * 1024 - 64 * 1024
>
> +#
> +###
> +### Compute the start location and size size of
> +### The coreboot bootloader.
> +###
> +
> +#
> +# EPIA-CN
> +#
> +romimage "image" 
> +	option COREBOOT_EXTRA_VERSION="-epiacn"
> +	payload ../payload.elf
> +end
> +
> +buildrom ./coreboot.rom ROM_SIZE "image"

Very nice stuff otherwise. I cannot test on actual hardware though, I'll
leave that to others.


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

[ron minnich]

Aaron, thanks very much for this fine work!

I am wondering what a good mainboard for this chipset is. Any recommendations?

Thanks again, we've really been hoping to get the -cn into coreboot.

ron

[Peter Stuge]

On Thu, May 15, 2008 at 07:26:14PM -0700, ron minnich wrote:
> I am wondering what a good mainboard for this chipset is.
> Any recommendations?

I have a http://www.mini-box.com/VIA-EPIA but haven't yet tried the
patch.

I use their 120W picoPSU for my M57SLI btw.


//Peter

[bari]

ron minnich wrote:
> Aaron, thanks very much for this fine work!
>
> I am wondering what a good mainboard for this chipset is. Any recommendations?
>
> Thanks again, we've really been hoping to get the -cn into coreboot.
>
> ron

Aaron's coreboot port is for the epia-cn and uses: C7 + CN700 + VT8237R 
Plus + VT1211

VIA EPIA CN13000G
VIA EPIA CN10000EG
http://www.via.com.tw/en/products/mainboards/motherboards.jsp?motherboard_id...

Other epia boards that use the C7 or C7D + CN700 + VT8237R Plus and are 
compatible with the epia-cn patch with only changes to raminit.c:

VIA EPIA LN10000EG
VIA EPIA LN10000EAG (no TV-out)
http://www.via.com.tw/en/products/mainboards/motherboards.jsp?motherboard_id...

VIA NAB 7400
VIA NAB 7410
http://www.via.com.tw/en/products/mainboards/motherboards.jsp?motherboard_id...

PC2500E
http://www.via.com.tw/en/initiatives/empowered/pc2500_platform/index.jsp

http://www.clubit.com/product_detail.cfm?itemno=A4842001
$59.99 !

VIA VB7001G
VIA VB7001AG (no TV-out)
http://www.via.com.tw/en/products/mainboards/motherboards.jsp?motherboard_id...

There are also several Jetway boards that use the same chipset except 
for the VIA VT1211 super io. The Jetway boards tend to use the Fintek 
F71805F/FG super io that Corey has a patch for.
http://www.jetway.com.tw/jw/ipcboard_socket.asp?platid=16

-Bari

[bari]

bari wrote:
> ron minnich wrote:
> > Aaron, thanks very much for this fine work!
> >
> > I am wondering what a good mainboard for this chipset is. Any recommendations?
> >
> > Thanks again, we've really been hoping to get the -cn into coreboot.
> >
> > ron
>
> Aaron's coreboot port is for the epia-cn and uses: C7 + CN700 + VT8237R 
> Plus + VT1211
>
> VIA EPIA CN13000G
> VIA EPIA CN10000EG
> http://www.via.com.tw/en/products/mainboards/motherboards.jsp?motherboard_id...
>
> Other epia boards that use the C7 or C7D + CN700 + VT8237R Plus and are 
> compatible with the epia-cn patch with only changes to raminit.c:
>
> VIA EPIA LN10000EG
> VIA EPIA LN10000EAG (no TV-out)
> http://www.via.com.tw/en/products/mainboards/motherboards.jsp?motherboard_id...
>
> VIA NAB 7400
> VIA NAB 7410
> http://www.via.com.tw/en/products/mainboards/motherboards.jsp?motherboard_id...
>
> PC2500E
> http://www.via.com.tw/en/initiatives/empowered/pc2500_platform/index.jsp
>
> http://www.clubit.com/product_detail.cfm?itemno=A4842001
> $59.99 !
>
> VIA VB7001G
> VIA VB7001AG (no TV-out)
> http://www.via.com.tw/en/products/mainboards/motherboards.jsp?motherboard_id...
>
> There are also several Jetway boards that use the same chipset except 
> for the VIA VT1211 super io. The Jetway boards tend to use the Fintek 
> F71805F/FG super io that Corey has a patch for.
> http://www.jetway.com.tw/jw/ipcboard_socket.asp?platid=16

Forgot to add that MSI also makes a board with the same chipset, except 
they use a Winbond W83627EHG super io.
http://www.msicomputer.com/product/p_spec.asp?model=Fuzzy_CN700&class=ipc

-Bari

[jtd]

On Saturday 17 May 2008 01:23, bari wrote:
> > There are also several Jetway boards that use the same chipset
> > except for the VIA VT1211 super io. The Jetway boards tend to use
> > the Fintek F71805F/FG super io that Corey has a patch for.
> > http://www.jetway.com.tw/jw/ipcboard_socket.asp?platid=16

http://www.idot.com.tw/en/products/mb-pc2500e/

with VT6103 for the network.

-- 
Rgds
JTD

[aaron lwe]

> Why not use PCI_DEVICE_ID_VIA_VT6420_SATA       as I did?

Oh...I only did a string search for VT8237R_SATA, I really should did
a value search ;-)
> Please try attached patch. Thanks. I'm booting from first SATA at filo with
> hd0 as first SATA and second sata as hd2 imho the first ide disk will be
> hd4.

I still cannot make it work with this patch, filo cannot find the IDE
disk. I've tried hd0 - hd7 and then give up...
> Just a question do you have in plan to support also some other pairs like
> VT8237A/S?
> I do not have such a plan.
> Again here please can you do the copy of registers or someone else will
> forgot that below ff MUST be same as above ;)

Good idea, thanks.
> This is tricky with other then VT8237R there is new registers for whole
> range. Please can you check if it is OK I mean that only two bits are for
> the whole region? Typical symptoms are random disk read lockup and
> controller reset - when DMA goes to the region of 0xE0000-0xEFFFF.
> The datasheet said so, these two bits control f region. I didn't have
> problems with disk and dma under linux.
> Can you add a more detailed comment about what exactly this
> does and why? It sure sounds scary for someone who hasn't read
> the datasheets (i.e. someone like me).
> Sorry but I just followed the steps the programming guide gave on how
> to disable sata and move pata from 00:0f.1 to 00:0f.0. I'm not clear
> what happened either...
> I am wondering what a good mainboard for this chipset is. Any recommendations?
> I'm mostly using via's internal demo boards, I'm not clear what boards
> are selling so I cannot make any recommendations, sorry.

Signed-off-by: Aaron Lwe <[e-mail]>

-Aaron

[Rudolf Marek]

> I still cannot make it work with this patch, filo cannot find the IDE
> disk. I've tried hd0 - hd7 and then give up...

Does it work from Linux? Can you boot from SATA? (hd0, hd2) ?

Can you give me lspci -vvvxxx

Rudolf

[Rudolf Marek]

I forgot.
> > I still cannot make it work with this patch, filo cannot find the IDE
> > disk. I've tried hd0 - hd7 and then give up...
>
> Does it work from Linux? Can you boot from SATA? (hd0, hd2) ?

Maybe compiled FILO with my patch and debugging (pci debug etc etc) will be good 
idea we can then compare outputs.

Thanks,
Rudolf

[Uwe Hermann]

> Signed-off-by: Aaron Lwe <[e-mail]>

Thanks, looks good to me. Committed in r3334 with smaller cosmetic
changes (no actual code changes though).

Thanks a lot for your work!


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

[bari]

aaron lwe wrote:
> I'm mostly using via's internal demo boards, I'm not clear what boards
> are selling so I cannot make any recommendations, sorry.
>
> Signed-off-by: Aaron Lwe <[e-mail]>
>
> -Aaron

Thank you for the cn patch!

Were you able to get usb keyboard / mouse or network working with your 
coreboot patch with vgabios and bochs loaded on your development boards?

I currently have your patch working headless without vgabios or bochs. 
USB keyboard and networking work fine this way.

When vgabios and bochs are loaded I get working video, but lose usb and 
network functions. PS2 keyboard and mouse are fine.

Did you see the same problem?

Do you have any suggestions for video + USB and networking?

Thank you,

-Bari

[aaron lwe]

> Do you have any suggestions for video + USB and networking?

Hi,

I've such problem too, and I think it's vga that caused this problem.
In vga.c the irq number is hard coded  to 0xa which will conflict with
other devices. so removing these code solved this problem here ;-)

Signed-off-by: Aaron Lwe <[e-mail]>

-Aaron

[Peter Stuge]

Hi Aaron,

On Tue, May 20, 2008 at 10:43:27AM +0800, aaron lwe wrote:
> > Do you have any suggestions for video + USB and networking?
>
> I've such problem too, and I think it's vga that caused this problem.
> In vga.c the irq number is hard coded  to 0xa which will conflict with
> other devices. so removing these code solved this problem here ;-)

Which interrupt does the VGA use then?


//Peter

[bari]

aaron lwe wrote:
> > Do you have any suggestions for video + USB and networking?
>
> Hi,
>
> I've such problem too, and I think it's vga that caused this problem.
> In vga.c the irq number is hard coded  to 0xa which will conflict with
> other devices. so removing these code solved this problem here ;-)
>
> Signed-off-by: Aaron Lwe <[e-mail]>
>
> -Aaron

Acked-by: Bari Ari <[e-mail]>