[coreboot] epia-cn patch

[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 <aaron.lwe at gmail.com>

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 <aaron.lwe at gmail.com> 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 <aaron.lwe at gmail.com> 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