[v3] VT8237R support

[Corey Osgood]

See attached. Not perfect, but enough to get started with.

[Corey Osgood]

And here with the actual patch.

On Mon, Mar 24, 2008 at 2:07 AM, Corey Osgood <corey.osgood(a)gmail.com>
wrote:

  See attached. Not perfect, but enough to get started
with.

[Carl-Daniel Hailfinger]

Hi Corey,

this seems to have slipped through the cracks.

On 24.03.2008 07:08, Corey Osgood wrote:
  Add preliminary, as yet untested support for the
vt8237r, based on the code in 
 coreboot v2. This will need some fixes, but at the very least it compiles and 
 should do smbus operations.

 Signed-off-by: Corey Osgood <corey.osgood(a)gmail.com>
Can you repost an updated patch? V3 has changed quite a bit in the last
few months, especially considering PCI access functions and other stuff.

Review follows:

  Index: southbridge/via/vt8237r/vt8237r.c
 ===================================================================
 --- southbridge/via/vt8237r/vt8237r.c	(revision 0)
 +++ southbridge/via/vt8237r/vt8237r.c	(revision 0)
 @@ -0,0 +1,465 @@
 +/*
 + * This file is part of the coreboot project.
 + *
 + * Copyright (C) 2007 Rudolf Marek <r.marek(a)assembler.cz>
 + * Copyright (C) 2008 Corey Osgood <corey.osgood(a)gmail.com>
 + *
 + * 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 <types.h>
 +#include <console.h>
 +#include <device/device.h>
 +#include <device/pci.h>
 +#include <mc146818rtc.h>
 +#include <string.h>
 +#include <keyboard.h>
 +#include <io.h>
 +#include <legacy.h>
 +#include <statictree.h>
 +#include "vt8237r.h"
 +
 +#define ALL		(0xff << 24)
 +#define NONE		(0)
 +#define DISABLED	(1 << 16)
 +#define ENABLED		(0 << 16)
 +#define TRIGGER_EDGE	(0 << 15)
 +#define TRIGGER_LEVEL	(1 << 15)
 +#define POLARITY_HIGH	(0 << 13)
 +#define POLARITY_LOW	(1 << 13)
 +#define PHYSICAL_DEST	(0 << 11)
 +#define LOGICAL_DEST	(1 << 11)
 +#define ExtINT		(7 << 8)
 +#define NMI		(4 << 8)
 +#define SMI		(2 << 8)
 +#define INT		(1 << 8)
 +
 +static struct ioapicreg {
 +	u32 reg;
 +	u32 value_low;
 +	u32 value_high;
 +} ioapic_table[] = {
 +	/* IO-APIC virtual wire mode configuration. */
 +	/* mask, trigger, polarity, destination, delivery, vector */
 +	{0,  ENABLED | TRIGGER_EDGE | POLARITY_HIGH | PHYSICAL_DEST |
 +	     ExtINT, NONE},
 +	{1,  DISABLED, NONE},
 +	{2,  DISABLED, NONE},
 +	{3,  DISABLED, NONE},
 +	{4,  DISABLED, NONE},
 +	{5,  DISABLED, NONE},
 +	{6,  DISABLED, NONE},
 +	{7,  DISABLED, NONE},
 +	{8,  DISABLED, NONE},
 +	{9,  DISABLED, NONE},
 +	{10, DISABLED, NONE},
 +	{11, DISABLED, NONE},
 +	{12, DISABLED, NONE},
 +	{13, DISABLED, NONE},
 +	{14, DISABLED, NONE},
 +	{15, DISABLED, NONE},
 +	{16, DISABLED, NONE},
 +	{17, DISABLED, NONE},
 +	{18, DISABLED, NONE},
 +	{19, DISABLED, NONE},
 +	{20, DISABLED, NONE},
 +	{21, DISABLED, NONE},
 +	{22, DISABLED, NONE},
 +	{23, DISABLED, NONE},
 +};
 +
 +static void setup_ioapic(u32 ioapic_base)
 +{
 +#if 0  /* Commented because lapicid() doesn't exist, yet */
Can you add a #warning before the #if 0? That way, we won't forget it.

  +	u32 value_low, value_high, val;
 +	volatile u32 *l;
There has been an effort to use readX/writeX functions instead of
volatile accesses. You may want to change that. Then again, I don't have
a strong opinion either way.

  +	int i;
 +
 +	/* All delivered to CPU0. */
 +	ioapic_table[0].value_high = (lapicid()) << (56 - 32);
Can you use a #define for the magic 56?

  +	l = (unsigned long *)ioapic_base;
 +
 +	/* Set APIC to FSB message bus. */
 +	l[0] = 0x3;
Magic values...

  +	val = l[4];
 +	l[4] = (val & 0xFFFFFE) | 1;
The line above is equivalent to
l[4] = (val & 0xffffff) | 1;

You decide which one is clearer. Anyway, the 0xFFFFFE is magic.

  +
 +	/* Set APIC ADDR - this will be VT8237R_APIC_ID. */
 +	l[0] = 0;
 +	val = l[4];
 +	l[4] = (val & 0xF0FFFF) | (VT8237R_APIC_ID << 24);
 +
 +	for (i = 0; i < ARRAY_SIZE(ioapic_table); i++) {
 +		l[0] = (ioapic_table[i].reg * 2) + 0x10;
 +		l[4] = ioapic_table[i].value_low;
 +		value_low = l[4];
 +		l[0] = (ioapic_table[i].reg * 2) + 0x11;
 +		l[4] = ioapic_table[i].value_high;
 +		value_high = l[4];
 +
 +		if ((i == 0) && (value_low == 0xffffffff)) {
 +			printk_warning("IO APIC not responding.\n");
 +			return;
 +		}
 +	}
Reading the code sequence above hurts. You set l[0] and l[4] always in
combination. What about helper functions like set_apic() and get_apic()?

  +#endif
 +}
 +
 +/** Set up PCI IRQ routing, route everything through APIC. */
 +static void pci_routing_fixup(struct device *dev)
 +{
 +	/* PCI PNP Interrupt Routing INTE/F - disable */
 +	pci_write_config8(dev, 0x44, 0x00);
 +
 +	/* PCI PNP Interrupt Routing INTG/H - disable */
 +	pci_write_config8(dev, 0x45, 0x00);
 +
 +	/* Route INTE-INTH through registers above, no map to INTA-INTD. */
 +	pci_write_config8(dev, 0x46, 0x10);
 +
 +	/* PCI Interrupt Polarity */
 +	pci_write_config8(dev, 0x54, 0x00);
 +
 +	/* PCI INTA# Routing */
 +	pci_write_config8(dev, 0x55, 0x00);
 +
 +	/* PCI INTB#/C# Routing */
 +	pci_write_config8(dev, 0x56, 0x00);
 +
 +	/* PCI INTD# Routing */
 +	pci_write_config8(dev, 0x57, 0x00);
Well-commented, but I'd appreciate #defines for the magic locations as
well. Your call, though.

  +}
 +
 +/**
 + * Set up the power management capabilities directly into ACPI mode.
 + * This avoids having to handle any System Management Interrupts (SMIs).
 + */
 +static void setup_pm(struct device *dev)
A comment about which device this function is called with would be great.

  +{
 +	/* Debounce LID and PWRBTN# Inputs for 16ms. */
 +	pci_write_config8(dev, 0x80, 0x20);
 +
 +	/* Set ACPI base address to I/O VT8237R_ACPI_IO_BASE. */
 +	pci_write_config16(dev, 0x88, VT8237R_ACPI_IO_BASE | 0x1);
 +
 +	/* Set ACPI to 9, must set IRQ 9 override to level! Set PSON gating. */
 +	pci_write_config8(dev, 0x82, 0x40 | VT8237R_ACPI_IRQ);
 +
 +	/* Primary interupt channel, define wake events 0=IRQ0 15=IRQ15 1=en. */
 +	pci_write_config16(dev, 0x84, 0x30b2);
 +
 +	/* SMI output level to low, 7.5us throttle clock */
 +	pci_write_config8(dev, 0x8d, 0x18);
 +
 +	/* GP Timer Control 1s */
 +	pci_write_config8(dev, 0x93, 0x88);
 +
 +	/* 7 = SMBus clock from RTC 32.768KHz
 +	 * 5 = Internal PLL reset from susp
 +	 * 2 = GPO2 is GPIO
 +	 */
 +	pci_write_config8(dev, 0x94, 0xa4);
 +
 +	/* 7 = stp to sust delay 1msec
 +	 * 6 = SUSST# Deasserted Before PWRGD for STD
 +	 * 3 = GPO26/GPO27 is GPO 
 +	 * 2 = Disable Alert on Lan
 +	 */
 +	pci_write_config8(dev, 0x95, 0xcc);
 +
 +	/* Disable GP3 timer. */
 +	pci_write_config8(dev, 0x98, 0);
 +
 +	/* Enable SATA LED, disable special CPU Frequency Change -
 +	 * GPIO28 GPIO22 GPIO29 GPIO23 are GPIOs.
 +	 */
 +	pci_write_config8(dev, 0xe5, 0x9);
 +
 +	/* REQ5 as PCI request input - should be together with INTE-INTH. */
 +	pci_write_config8(dev, 0xe4, 0x4);
 +
 +	/* Enable ACPI accessm RTC signal gated with PSON. */
 +	pci_write_config8(dev, 0x81, 0x84);
 +
 +	/* Clear status events. */
 +	outw(0xffff, VT8237R_ACPI_IO_BASE + 0x00);
 +	outw(0xffff, VT8237R_ACPI_IO_BASE + 0x20);
 +	outw(0xffff, VT8237R_ACPI_IO_BASE + 0x28);
 +	outl(0xffffffff, VT8237R_ACPI_IO_BASE + 0x30);
 +
 +	/* Disable SCI on GPIO. */
 +	outw(0x0, VT8237R_ACPI_IO_BASE + 0x22);
 +
 +	/* Disable SMI on GPIO. */
 +	outw(0x0, VT8237R_ACPI_IO_BASE + 0x24);
 +
 +	/* Disable all global enable SMIs. */
 +	outw(0x0, VT8237R_ACPI_IO_BASE + 0x2a);
 +
 +	/* All SMI off, both IDE buses ON, PSON rising edge. */
 +	outw(0x0, VT8237R_ACPI_IO_BASE + 0x2c);
 +
 +	/* Primary activity SMI disable. */
 +	outl(0x0, VT8237R_ACPI_IO_BASE + 0x34);
 +
 +	/* GP timer reload on none. */
 +	outl(0x0, VT8237R_ACPI_IO_BASE + 0x38);
 +
 +	/* Disable extended IO traps. */
 +	outb(0x0, VT8237R_ACPI_IO_BASE + 0x42);
 +
 +	/* SCI is generated for RTC/pwrBtn/slpBtn. */
 +	outw(0x001, VT8237R_ACPI_IO_BASE + 0x04);
 +
 +	/* FIXME: Intel needs more bit set for C2/C3. */
 +
 +	/* Allow SLP# signal to assert LDTSTOP_L.
 +	 * Will work for C3 and for FID/VID change.
 +	 */
 +	outb(0x1, VT8237R_ACPI_IO_BASE + 0x11);
 +}
 +
 +static void vt8237r_init(struct device *dev)
Doxygen comment, please. Including a hint which PCI device this is
called with.

  +{
 +	u8 enables, byte;
 +
 +	/* Enable addr/data stepping. */
 +	byte = pci_read_config8(dev, PCI_COMMAND);
 +	byte |= PCI_COMMAND_WAIT;
 +	pci_write_config8(dev, PCI_COMMAND, byte);
 +
 +	/* Enable the internal I/O decode. */
 +	enables = pci_read_config8(dev, 0x6C);
 +	enables |= 0x80;
 +	pci_write_config8(dev, 0x6C, enables);
 +
 +	/* FIXME: Map 4MB of flash into the address space,
 +	 * this should be in CAR call.
Not really. This belongs in hardware_stage1() or vt8237_stage1().

  +	 */
 +	/* pci_write_config8(dev, 0x41, 0x7f); */
 +
 +	/* Set bit 6 of 0x40 (I/O recovery time).
 +	 * IMPORTANT FIX - EISA = ECLR reg at 0x4d0! Decoding must be on so
 +	 * that PCI interrupts can be properly marked as level triggered.
 +	 */
 +	enables = pci_read_config8(dev, 0x40);
 +	enables |= 0x44;
 +	pci_write_config8(dev, 0x40, enables);
 +
 +	/* Line buffer control */
 +	enables = pci_read_config8(dev, 0x42);
 +	enables |= 0xf8;
 +	pci_write_config8(dev, 0x42, enables);
 +
 +	/* Delay transaction control */
 +	pci_write_config8(dev, 0x43, 0xb);
 +
 +	/* I/O recovery time */
 +	pci_write_config8(dev, 0x4c, 0x44);
 +
 +	/* ROM memory cycles go to LPC. */
 +	pci_write_config8(dev, 0x59, 0x80);
 +
 +	/* Bypass APIC De-Assert Message, INTE#, INTF#, INTG#, INTH# as PCI. */
 +	pci_write_config8(dev, 0x5B, 0xb);
 +
 +	/* Set Read Pass Write Control Enable (force A2 from APIC FSB to low). */
 +	pci_write_config8(dev, 0x48, 0x8c);
 +
 +	/* Set 0x58 to 0x43 APIC and RTC. */
 +	pci_write_config8(dev, 0x58, 0x43);
 +
 +	/* Set bit 3 of 0x4f (use INIT# as CPU reset). */
 +	enables = pci_read_config8(dev, 0x4f);
 +	enables |= 0x08;
 +	pci_write_config8(dev, 0x4f, enables);
 +
 +	/* Enable serial IRQ, 6PCI clocks. */
 +	pci_write_config8(dev, 0x52, 0x9);
 +
 +	/* Enable HPET at VT8237R_HPET_ADDR. */
 +	pci_write_config32(dev, 0x68, (VT8237R_HPET_ADDR | 0x80));
 +
 +	/* Power management setup */
 +	setup_pm(dev);
 +
 +	/* Start the RTC. */
 +	rtc_init(0);
 +}
 +
 +static void vt8237r_read_resources(struct device *dev)
 +{
 +	struct resource *res;
 +
 +	pci_dev_read_resources(dev);
 +	/* Fixed APIC resource */
 +	res = new_resource(dev, 0x44);
 +	res->base = VT8237R_APIC_BASE;
 +	res->size = 256;
 +	res->limit = res->base + res->size - 1;
 +	res->align = 8;
 +	res->gran = 8;
 +	res->flags = IORESOURCE_MEM | IORESOURCE_FIXED |
 +		     IORESOURCE_STORED | IORESOURCE_ASSIGNED;
 +}
 +
 +/**
 + * The VT8237R is not a PCI bridge and has no resources of its own (other
 + * than standard PC I/O addresses), however it does control the ISA bus
 + * and so we need to manually call enable childrens resources on that bus.
 + */
 +static void vt8237r_enable_resources(struct device *dev)
 +{
 +	pci_dev_enable_resources(dev);
 +	enable_childrens_resources(dev);
 +}
 +
 +static void init_keyboard(struct device *dev)
 +{
 +	u8 regval = pci_read_config8(dev, 0x51);
 +	if (regval & 0x1)
 +		init_pc_keyboard(0x60, 0x64, 0);
 +}
 +
 +static void vt8237r_lpc_init(struct device *dev)
 +{
 +	vt8237r_init(dev);
 +	pci_routing_fixup(dev);
 +	setup_ioapic(VT8237R_APIC_BASE);
 +	setup_i8259();
 +	init_keyboard(dev);
 +}
 +
 +static void vt8237r_ide_init(struct device *dev)
 +{
 +	struct southbridge_via_vt8237r_ide_config *sb =
 +	    (struct southbridge_via_vt8237r_ide_config *)dev->device_configuration;
 +
 +	u8 enables;
 +	u32 cablesel;
 +
 +	printk(BIOS_INFO, "%s IDE interface %s\n", "Primary",
 +		    sb->ide0_enable ? "enabled" : "disabled");
 +	printk(BIOS_INFO, "%s IDE interface %s\n", "Secondary",
 +		    sb->ide1_enable ? "enabled" : "disabled");
 +	enables = pci_read_config8(dev, IDE_CS) & ~0x3;
 +	enables |= (sb->ide0_enable << 1) | sb->ide1_enable;
 +	pci_write_config8(dev, IDE_CS, enables);
 +	enables = pci_read_config8(dev, IDE_CS);
 +	printk(BIOS_DEBUG, "Enables in reg 0x40 read back as 0x%x\n", enables);
 +
 +	/* Enable only compatibility mode. */
 +	enables = pci_read_config8(dev, IDE_CONF_II);
 +	enables &= ~0xc0;
Magic 0xc0.

  +	pci_write_config8(dev, IDE_CONF_II, enables);
 +	enables = pci_read_config8(dev, IDE_CONF_II);
 +	printk(BIOS_DEBUG, "Enables in reg 0x42 read back as 0x%x\n", enables);
 +
 +	/* Enable prefetch buffers. */
 +	enables = pci_read_config8(dev, IDE_CONF_I);
 +	enables |= 0xf0;
Magic 0xf0.

  +	pci_write_config8(dev, IDE_CONF_I, enables);
 +
 +	/* Flush FIFOs at half. */
 +	enables = pci_read_config8(dev, IDE_CONF_FIFO);
 +	enables &= 0xf0;
 +	enables |= (1 << 2) | (1 << 0);
 +	pci_write_config8(dev, IDE_CONF_FIFO, enables);
 +
 +	/* PIO read prefetch counter, Bus Master IDE Status Reg. Read Retry. */
 +	enables = pci_read_config8(dev, IDE_MISC_I);
 +	enables &= 0xe2;
 +	enables |= (1 << 4) | (1 << 3);
 +	pci_write_config8(dev, IDE_MISC_I, enables);
 +
 +	/* Use memory read multiple, Memory-Write-and-Invalidate. */
 +	enables = pci_read_config8(dev, IDE_MISC_II);
 +	enables |= (1 << 2) | (1 << 3);
 +	pci_write_config8(dev, IDE_MISC_II, enables);
 +
 +	/* Force interrupts to use compat mode. */
 +	pci_write_config8(dev, PCI_INTERRUPT_PIN, 0x0);
 +	pci_write_config8(dev, PCI_INTERRUPT_LINE, 0xff);
 +
 +	/* Cable guy... */
 +	cablesel = pci_read_config32(dev, IDE_UDMA);
 +	cablesel &= ~((1 << 28) | (1 << 20) | (1 << 12) | (1 <<
4));
 +	cablesel |= (sb->ide0_80pin_cable << 28) |
 +		    (sb->ide0_80pin_cable << 20) |
 +		    (sb->ide1_80pin_cable << 12) |
 +		    (sb->ide1_80pin_cable << 4);
 +	pci_write_config32(dev, IDE_UDMA, cablesel);
 +}
 +
 +static void vt8237r_sata_init(struct device *dev)
 +{
 +	u8 reg;
 +
 +	printk(BIOS_DEBUG, "Configuring VIA SATA controller\n");
 +
 +	/* Class IDE Disk */
 +	reg = pci_read_config8(dev, SATA_MISC_CTRL);
 +	reg &= 0x7f;		/* Sub Class Write Protect off */
 +	pci_write_config8(dev, SATA_MISC_CTRL, reg);
 +
 +	/* Change the device class to SATA from RAID. */
 +	pci_write_config8(dev, PCI_CLASS_DEVICE, 0x1);
 +	reg |= 0x80;		/* Sub Class Write Protect on */
 +	pci_write_config8(dev, SATA_MISC_CTRL, reg);
 +
 +}
 +
 +/* You can override or extend each operation as needed for the device. */
 +struct device_operations vt8237r_lpc = {
 +	.id = {.type = DEVICE_ID_PCI,
 +		.u = {.pci = {	.vendor = PCI_VENDOR_ID_VIA,
Remove the ".u =" because this is now an anonymous union.

  +				.device = PCI_DEVICE_ID_VIA_VT8237R_LPC}}},
 +	.constructor		 = default_device_constructor,
 +	//.phase3_scan		 = 0,
 +	.phase4_read_resources	 = pci_dev_read_resources,
 +	.phase4_set_resources	 = pci_dev_set_resources,
 +	.phase4_enable_disable	 = 0,
 +	.phase5_enable_resources = pci_dev_enable_resources,
 +	.phase6_init		 = vt8237r_lpc_init,
 +	.ops_pci		 = &pci_dev_ops_pci,
 +};
 +
 +struct device_operations vt8237r_ide = {
 +	.id = {.type = DEVICE_ID_PCI,
 +		.u = {.pci = {	.vendor = PCI_VENDOR_ID_VIA, 
.u removal

  +				.device = PCI_DEVICE_ID_VIA_VT82XXX_IDE}}},
 +	.constructor		 = default_device_constructor,
 +	.phase3_scan		 = 0,
 +	.phase4_read_resources	 = pci_dev_read_resources,
 +	.phase4_set_resources	 = pci_dev_set_resources,
 +	.phase4_enable_disable	 = 0,
 +	.phase5_enable_resources = pci_dev_enable_resources,
 +	.phase6_init		 = vt8237r_ide_init,
 +	.ops_pci		 = &pci_dev_ops_pci,
 +};
 +
 +struct device_operations vt8237r_sata = {
 +	.id = {.type = DEVICE_ID_PCI,
 +		.u = {.pci = {	.vendor = PCI_VENDOR_ID_VIA,
.u removal

  +				.device = PCI_DEVICE_ID_VIA_VT8237R_SATA}}},
 +	.constructor		 = default_device_constructor,
 +	.phase3_scan		 = 0,
 +	.phase4_read_resources	 = pci_dev_read_resources,
 +	.phase4_set_resources	 = pci_dev_set_resources,
 +	.phase4_enable_disable	 = 0,
 +	.phase5_enable_resources = pci_dev_enable_resources,
 +	.phase6_init		 = vt8237r_sata_init,
 +	.ops_pci		 = &pci_dev_ops_pci,
 +};
 Index: southbridge/via/vt8237r/ide
 ===================================================================
 --- southbridge/via/vt8237r/ide	(revision 0)
 +++ southbridge/via/vt8237r/ide	(revision 0)
 @@ -0,0 +1,27 @@
 +/*
 + * This file is part of the coreboot project.
 + *
 + * Copyright (C) 2008 Corey Osgood &lt;corey.osgood(a)gmail.com&gt;
 + *
 + * 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
 + */
 +
 +{
 +	device_operations = "vt8237r_ide";
 +	ide0_enable = "1";
 +	ide0_80pin_cable = "0";
 +	ide1_enable = "0";
 +	ide1_80pin_cable = "0";
 +};
 Index: southbridge/via/vt8237r/vt8237r.h
 ===================================================================
 --- southbridge/via/vt8237r/vt8237r.h	(revision 0)
 +++ southbridge/via/vt8237r/vt8237r.h	(revision 0)
 @@ -0,0 +1,74 @@
 +/*
 + * This file is part of the coreboot project.
 + *
 + * Copyright (C) 2007 Rudolf Marek &lt;r.marek(a)assembler.cz&gt;
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License v2 as published by
 + * the Free Software Foundation.
 + *
 + * 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
 + */
 +
 +#ifndef SOUTHBRIDGE_VIA_VT8237R_VT8237R_H
 +#define SOUTHBRIDGE_VIA_VT8237R_VT8237R_H
 +
 +/* Static resources for the VT8237R southbridge */
 +
 +#define VT8237R_APIC_ID			0x2
 +#define VT8237R_ACPI_IO_BASE		0x500
 +#define VT8237R_SMBUS_IO_BASE		0x400
 +/* 0x0 disabled, 0x2 reserved, 0xf = IRQ15 */
 +#define VT8237R_ACPI_IRQ		0x9
 +#define VT8237R_HPET_ADDR		0xfed00000ULL
 +#define VT8237R_APIC_BASE		0xfec00000ULL
 +
 +/* IDE */
 +#define IDE_CS				0x40
 +#define IDE_CONF_I			0x41
 +#define IDE_CONF_II			0x42
 +#define IDE_CONF_FIFO			0x43
 +#define IDE_MISC_I			0x44
 +#define IDE_MISC_II			0x45
 +#define IDE_UDMA			0x50
 +#define SATA_MISC_CTRL			0x45
 +
 +/* SMBus */
 +#define VT8237R_POWER_WELL		0x94
 +#define VT8237R_SMBUS_IO_BASE_REG	0xd0
 +#define VT8237R_SMBUS_HOST_CONF		0xd2
 +
 +#define SMBHSTSTAT			(VT8237R_SMBUS_IO_BASE + 0x0)
 +#define SMBSLVSTAT			(VT8237R_SMBUS_IO_BASE + 0x1)
 +#define SMBHSTCTL			(VT8237R_SMBUS_IO_BASE + 0x2)
 +#define SMBHSTCMD			(VT8237R_SMBUS_IO_BASE + 0x3)
 +#define SMBXMITADD			(VT8237R_SMBUS_IO_BASE + 0x4)
 +#define SMBHSTDAT0			(VT8237R_SMBUS_IO_BASE + 0x5)
 +
 +#define HOST_RESET 			0xff
 +/* 1 in the 0 bit of SMBHSTADD states to READ. */
 +#define READ_CMD			0x01
 +#define SMBUS_TIMEOUT			(100 * 1000 * 10)
 +#define I2C_TRANS_CMD			0x40
 +#define CLOCK_SLAVE_ADDRESS		0x69
 +
 +#if DEBUG_SMBUS == 1
 +#define PRINT_DEBUG(x)		print_debug(x)
 +#define PRINT_DEBUG_HEX16(x)	print_debug_hex16(x)
 +#else
 +#define PRINT_DEBUG(x)
 +#define PRINT_DEBUG_HEX16(x)
 +#endif
 +
 +#define SMBUS_DELAY() inb(0x80)
 +
 +void vt8237r_stage1(void);
 +
 +#endif
 Index: southbridge/via/vt8237r/stage1.c
 ===================================================================
 --- southbridge/via/vt8237r/stage1.c	(revision 0)
 +++ southbridge/via/vt8237r/stage1.c	(revision 0)
 @@ -0,0 +1,36 @@
 +/*
 + * This file is part of the coreboot project.
 + *
 + * Copyright (C) 2008 Corey Osgood &lt;corey.osgood(a)gmail.com&gt;
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 + *
 + * 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 <device/pci.h>
 +#include <statictree.h>
 +#include "vt8237r.h"
 +
 +void vt8237r_stage1(void)
 +{
 +/*	struct device *dev = dev_find_pci_device(PCI_VENDOR_ID_VIA,
 +				       PCI_DEVICE_ID_VIA_VT8237R_LPC, 0);
 +	struct southbridge_via_vt8237r_dts_config *sb =
 +	    (struct southbridge_via_vt8237r_dts_config *)dev->device_operations;
 +
 +	pci_write_config8(dev, 0x50, sb->fn_ctrl_lo);
 +	pci_write_config8(dev, 0x51, sb->fn_ctrl_hi);
 +*/
 +}
 +
 Index: southbridge/via/vt8237r/dts
 ===================================================================
 --- southbridge/via/vt8237r/dts	(revision 0)
 +++ southbridge/via/vt8237r/dts	(revision 0)
 @@ -0,0 +1,25 @@
 +/*
 + * This file is part of the coreboot project.
 + *
 + * Copyright (C) 2008 Corey Osgood &lt;corey.osgood(a)gmail.com&gt;
 + *
 + * 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
 + */
 +
 +{
 +	fn_ctrl_lo = "0x00"
 +	fn_ctrl_hi = "0x00"
 +	device_operations = "vt8237r_lpc";
 +};
 Index: southbridge/via/vt8237r/Makefile
 ===================================================================
 --- southbridge/via/vt8237r/Makefile	(revision 0)
 +++ southbridge/via/vt8237r/Makefile	(revision 0)
 @@ -0,0 +1,26 @@
 +##
 +## This file is part of the coreboot project.
 +##
 +## Copyright (C) 2008 Corey Osgood &lt;corey.osgood(a)gmail.com&gt;
 +##
 +## 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
 +##
 +
 +ifeq ($(CONFIG_SOUTHBRIDGE_VIA_VT8237R),y)
 +
 +STAGE2_CHIPSET_OBJ += $(obj)/southbridge/via/vt8237r/vt8237r.o
 +STAGE0_CHIPSET_OBJ += $(obj)/southbridge/via/vt8237r/stage1.o
 +
 +endif
 Index: southbridge/via/vt8237r/smbus_initram.c
 ===================================================================
 --- southbridge/via/vt8237r/smbus_initram.c	(revision 0)
 +++ southbridge/via/vt8237r/smbus_initram.c	(revision 0)
 @@ -0,0 +1,208 @@
 +/*
 + * This file is part of the coreboot project.
 + *
 + * Copyright (C) 2007-2008 Corey Osgood &lt;corey.osgood(a)gmail.com&gt;
 + * Copyright (C) 2007 Rudolf Marek &lt;r.marek(a)assembler.cz&gt;
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 + *
 + * 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 <types.h>
 +#include <console.h>
 +#include <io.h>
 +#include <device/pci_ids.h>
 +#include "vt8237r.h"
 +
 +/**
 + * Print an error, should it occur. If no error, just exit.
 + *
 + * @param host_status The data returned on the host status register after
 + *		      a transaction is processed.
 + * @param loops The number of times a transaction was attempted.
 + */
 +static void smbus_print_error(u8 host_status, int loops)
 +{
 +	/* Check if there actually was an error. */
 +	if ((host_status == 0x00 || host_status == 0x40 ||
 +	     host_status == 0x42) && (loops < SMBUS_TIMEOUT))
 +		return;
 +
 +	if (loops >= SMBUS_TIMEOUT)
 +		printk(BIOS_ERR, "SMBus timeout\r\n");
 +	if (host_status & (1 << 4))
 +		printk(BIOS_ERR, "Interrupt/SMI# was Failed Bus Transaction\r\n");
 +	if (host_status & (1 << 3))
 +		printk(BIOS_ERR, "Bus error\r\n");
 +	if (host_status & (1 << 2))
 +		printk(BIOS_ERR, "Device error\r\n");
 +	if (host_status & (1 << 1))
 +		printk(BIOS_DEBUG, "Interrupt/SMI# completed successfully\r\n");
 +	if (host_status & (1 << 0))
 +		printk(BIOS_ERR, "Host busy\r\n");
 +}
 +
 +/**
 + * Wait for the SMBus to become ready to process the next transaction.
 + */
 +static void smbus_wait_until_ready(void)
 +{
 +	int loops;
 +
 +	loops = 0;
 +	/* Yes, this is a mess, but it's the easiest way to do it. */
 +	while ((inb(SMBHSTSTAT) & 1) == 1 && loops < SMBUS_TIMEOUT)
 +		++loops;
 +
 +	smbus_print_error(inb(SMBHSTSTAT), loops);
 +}
 +
 +/**
 + * Reset and take ownership of the SMBus.
 + */
 +static void smbus_reset(void)
 +{
 +	outb(HOST_RESET, SMBHSTSTAT);
 +
 +	/* Datasheet says we have to read it to take ownership of SMBus. */
 +	inb(SMBHSTSTAT);
 +}
 +
 +/**
 + * Read a byte from the SMBus.
 + *
 + * @param dimm The address location of the DIMM on the SMBus.
 + * @param offset The offset the data is located at.
 + */
 +u8 smbus_read_byte(u8 dimm, u8 offset)
 +{
 +	u8 val;
 +
 +	smbus_reset();
 +
 +	/* Clear host data port. */
 +	outb(0x00, SMBHSTDAT0);
 +	SMBUS_DELAY();
 +	smbus_wait_until_ready();
 +
 +	/* Actual addr to reg format. */
 +	dimm = (dimm << 1);
 +	dimm |= 1;
 +	outb(dimm, SMBXMITADD);
 +	outb(offset, SMBHSTCMD);
 +
 +	/* Start transaction, byte data read. */
 +	outb(0x48, SMBHSTCTL);
 +	SMBUS_DELAY();
 +	smbus_wait_until_ready();
 +
 +	val = inb(SMBHSTDAT0);
 +	/* Probably don't have to do this, but it can't hurt. */
 +	smbus_reset();
 +
 +	return val;
 +}
 +static void smbus_enable(void)
 +{
 +	struct device *dev;
 +
 +	/* Power management controller */
 +	dev = dev_find_pci_device(PCI_VENDOR_ID_VIA,
 +				       PCI_DEVICE_ID_VIA_VT8237R_LPC, 0);
 +
 +	if (dev == PCI_DEV_INVALID)
 +		die("Power management controller not found\r\n");
 +
 +	/* 7 = SMBus Clock from RTC 32.768KHz
 +	 * 5 = Internal PLL reset from susp
 +	 */
 +	pci_write_config8(dev, VT8237R_POWER_WELL, 0xa0);
 +
 +	/* Enable SMBus. */
 +	pci_write_config16(dev, VT8237R_SMBUS_IO_BASE_REG,
 +			   VT8237R_SMBUS_IO_BASE | 0x1);
 +
 +	/* SMBus Host Configuration, enable. */
 +	pci_write_config8(dev, VT8237R_SMBUS_HOST_CONF, 0x01);
 +
 +	/* Make it work for I/O. */
 +	pci_write_config16(dev, PCI_COMMAND, PCI_COMMAND_IO);
 +
 +	smbus_reset();
 +
 +	/* Reset the internal pointer. */
 +	inb(SMBHSTCTL);}
What's that trailing "}"?

  +
 +}
 +
 +/**
 + * A fixup for some systems that need time for the SMBus to "warm up". This is

 + * needed on some VT823x based systems, where the SMBus spurts out bad data for 
 + * a short time after power on. This has been seen on the VIA Epia series and 
 + * Jetway J7F2-series. It reads the ID byte from SMBus, looking for 
 + * known-good data from a slot/address. Exits on either good data or a timeout.
 + *
 + * TODO: This should probably go into some global file, but one would need to
 + *       be created just for it. If some other chip needs/wants it, we can
 + *       worry about it then.
 + *
 + * @param ctrl The memory controller and SMBus addresses.
 + */
 +void smbus_fixup(const struct mem_controller *ctrl)
 +{
 +	int i, ram_slots, current_slot = 0;
 +	u8 result = 0;
 +
 +	ram_slots = ARRAY_SIZE(ctrl->channel0);
 +	if (!ram_slots) {
 +		printk(BIOS_ERR, "smbus_fixup() thinks there are no RAM slots!\r\n");
 +		return;
 +	}
 +
 +	/*
 +	 * Bad SPD data should be either 0 or 0xff, but YMMV. So we look for
 +	 * the ID bytes of SDRAM, DDR, DDR2, and DDR3 (and anything in between).
 +	 * VT8237R has only been seen on DDR and DDR2 based systems, so far.
 +	 */
 +	for (i = 0; (i < SMBUS_TIMEOUT && ((result < SPD_MEMORY_TYPE_SDRAM) ||
 +				(result > SPD_MEMORY_TYPE_SDRAM_DDR3))); i++) {
 +
 +		if (current_slot > ram_slots)
 +			current_slot = 0;
 +
 +		result = smbus_read_byte(ctrl->channel0[current_slot],
 +					 SPD_MEMORY_TYPE);
 +		current_slot++;
 +		PRINT_DEBUG(".");
printk() please.

  +	}
 +
 +	if (i >= SMBUS_TIMEOUT)
 +		print_err("SMBus timed out while warming up\r\n");
printk() please.

  +	else
 +		PRINT_DEBUG("Done\r\n");
printk() please.

  +}
 +
 +void enable_rom_decode(void)
 +{
 +	struct device *dev;
 +
 +	/* Bus Control and Power Management  */
 +	dev = dev_find_pci_device(PCI_VENDOR_ID_VIA,
 +				       PCI_DEVICE_ID_VIA_VT8237R_LPC, 0);
 +
 +	if (dev == PCI_DEV_INVALID)
 +		die("SB not found\r\n");
 +
 +	/* ROM decode last 1MB FFC00000 - FFFFFFFF */
 +	pci_write_config8(dev, 0x41, 0x7f);
 +}
 Index: Kconfig
 ===================================================================
 --- Kconfig	(revision 644)
 +++ Kconfig	(working copy)
 @@ -80,6 +80,8 @@
  	boolean
  config SOUTHBRIDGE_INTEL_I82371EB
  	boolean
 +config SOUTHBRIDGE_VIA_VT8237R
 +	boolean
  
  # Super I/Os:
  config SUPERIO_WINBOND_W83627HF
 Index: include/device/pci_ids.h
 ===================================================================
 --- include/device/pci_ids.h	(revision 644)
 +++ include/device/pci_ids.h	(working copy)
 @@ -155,4 +155,10 @@
  #define PCI_VENDOR_ID_CIRRUS			0x1013
  #define PCI_DEVICE_ID_CIRRUS_5446		0x00b8	/* Used by QEMU */
  
 +/* Need to check these */
 +#define PCI_VENDOR_ID_VIA			0x1106
 +#define PCI_DEVICE_ID_VIA_VT8237R_LPC		0x3074
 +#define PCI_DEVICE_ID_VIA_VT82XXX_IDE		0x0571
 +#define PCI_DEVICE_ID_VIA_VT8237R_SATA		0x3149
 +
  #endif /* DEVICE_PCI_IDS_H */


I think with another iteration this will be golden.
Bonus points if you fix this in v2 as well.

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/

[ron minnich]

I understand the complaints about magic numbers but here's a principal
I'd like to stick to:
if the driver comes from v2, and there was magic number in it, it was
accepted in v2.
SO:
If we want to ask for this for each magic
#warning please fix this magic number

That's fine.

BUT:
we should not require that we fix each and every magic # when they've
been in v2 for 6 years in some cases. We can't fix every v2 flaw on
the first try.

Plus, sometimes the magic numbers are magic at the request of the vendor ..

Just my $.02 :=)

And, corey, thanks. I am sensing some increasing v3 action on this
list. It's nice to see.

ron

[Rudolf Marek]

Hi all,

Maybe someone with more time can take mine wip2.patch with VT8237S support and 
then port it to v3 all-together. Most of the code VT8237R is bit redone, so 
perhaps it is waste of time to change the code right now. Maybe commit it as-is 
and then add support for rev S (port from v2?)

Please check the latest patch wip2.patch in thread Re: [coreboot] K8M890 VT8237S 
status update

Ideas how to proceed?

Thanks,
Rudolf

[ron minnich]

On Sun, Aug 10, 2008 at 1:32 PM, Rudolf Marek <r.marek(a)assembler.cz> wrote:
  Hi all,

 Maybe someone with more time can take mine wip2.patch with VT8237S support
 and then port it to v3 all-together. Most of the code VT8237R is bit redone,
 so perhaps it is waste of time to change the code right now. Maybe commit it
 as-is and then add support for rev S (port from v2?)

 Please check the latest patch wip2.patch in thread Re: [coreboot] K8M890
 VT8237S status update

 Ideas how to proceed?
I appreciate the idea but right now ... if there is a patch here it
wins. I would love to see your code at some point as well if someone
does find the time.

Many thanks

ron

[Corey Osgood]

On Sun, Aug 10, 2008 at 3:30 PM, Carl-Daniel Hailfinger
<c-d.hailfinger.devel.2006(a)gmx.net> wrote:
  Hi Corey,

 this seems to have slipped through the cracks.

 On 24.03.2008 07:08, Corey Osgood wrote:
  Add preliminary, as yet untested support for the
vt8237r, based on the code in
 coreboot v2. This will need some fixes, but at the very least it compiles and
 should do smbus operations.

 Signed-off-by: Corey Osgood <corey.osgood(a)gmail.com>
 Can you repost an updated patch? V3 has changed quite a bit in the last
 few months, especially considering PCI access functions and other stuff.
I haven't been following v3 development lately, been too busy. I'll
try to bring this up as soon as I get a chance.

---snip---
  +     l =
(unsigned long *)ioapic_base;
 +
 +     /* Set APIC to FSB message bus. */
 +     l[0] = 0x3;
 Magic values...

  +     val = l[4];
 +     l[4] = (val & 0xFFFFFE) | 1;
 The line above is equivalent to
 l[4] = (val & 0xffffff) | 1;

 You decide which one is clearer. Anyway, the 0xFFFFFE is magic.

  +
 +     /* Set APIC ADDR - this will be VT8237R_APIC_ID. */
 +     l[0] = 0;
 +     val = l[4];
 +     l[4] = (val & 0xF0FFFF) | (VT8237R_APIC_ID << 24);
 +
 +     for (i = 0; i < ARRAY_SIZE(ioapic_table); i++) {
 +             l[0] = (ioapic_table[i].reg * 2) + 0x10;
 +             l[4] = ioapic_table[i].value_low;
 +             value_low = l[4];
 +             l[0] = (ioapic_table[i].reg * 2) + 0x11;
 +             l[4] = ioapic_table[i].value_high;
 +             value_high = l[4];
 +
 +             if ((i == 0) && (value_low == 0xffffffff)) {
 +                     printk_warning("IO APIC not responding.\n");
 +                     return;
 +             }
 +     }
 Reading the code sequence above hurts. You set l[0] and l[4] always in
 combination. What about helper functions like set_apic() and get_apic()?
That would be Rudolf's migraine :p I frankly don't really understand
APIC, so I honestly have no idea what's going on there, that was
pulled right out of v2. I'll try to fix everything else up though.

-Corey

[Corey Osgood]

On Sun, Aug 10, 2008 at 3:30 PM, Carl-Daniel Hailfinger
<c-d.hailfinger.devel.2006(a)gmx.net> wrote:
  Hi Corey,

 this seems to have slipped through the cracks.

 On 24.03.2008 07:08, Corey Osgood wrote:
  Add preliminary, as yet untested support for the
vt8237r, based on the code in
 coreboot v2. This will need some fixes, but at the very least it compiles and
 should do smbus operations.

 Signed-off-by: Corey Osgood <corey.osgood(a)gmail.com>
 Can you repost an updated patch? V3 has changed quite a bit in the last
 few months, especially considering PCI access functions and other stuff.
I haven't been following v3 development lately, been too busy. I'll
try to bring this up as soon as I get a chance.

---snip---
  +     l =
(unsigned long *)ioapic_base;
 +
 +     /* Set APIC to FSB message bus. */
 +     l[0] = 0x3;
 Magic values...

  +     val = l[4];
 +     l[4] = (val & 0xFFFFFE) | 1;
 The line above is equivalent to
 l[4] = (val & 0xffffff) | 1;

 You decide which one is clearer. Anyway, the 0xFFFFFE is magic.

  +
 +     /* Set APIC ADDR - this will be VT8237R_APIC_ID. */
 +     l[0] = 0;
 +     val = l[4];
 +     l[4] = (val & 0xF0FFFF) | (VT8237R_APIC_ID << 24);
 +
 +     for (i = 0; i < ARRAY_SIZE(ioapic_table); i++) {
 +             l[0] = (ioapic_table[i].reg * 2) + 0x10;
 +             l[4] = ioapic_table[i].value_low;
 +             value_low = l[4];
 +             l[0] = (ioapic_table[i].reg * 2) + 0x11;
 +             l[4] = ioapic_table[i].value_high;
 +             value_high = l[4];
 +
 +             if ((i == 0) && (value_low == 0xffffffff)) {
 +                     printk_warning("IO APIC not responding.\n");
 +                     return;
 +             }
 +     }
 Reading the code sequence above hurts. You set l[0] and l[4] always in
 combination. What about helper functions like set_apic() and get_apic()?
That would be Rudolf's migraine :p I frankly don't really understand
APIC, so I honestly have no idea what's going on there, that was
pulled right out of v2. I'll try to fix everything else up though.

-Corey

[Corey Osgood]

On Sun, Aug 10, 2008 at 3:30 PM, Carl-Daniel Hailfinger
<c-d.hailfinger.devel.2006(a)gmx.net> wrote:
  Hi Corey,

 this seems to have slipped through the cracks.

 On 24.03.2008 07:08, Corey Osgood wrote:
  Add preliminary, as yet untested support for the
vt8237r, based on the code in
 coreboot v2. This will need some fixes, but at the very least it compiles and
 should do smbus operations.

 Signed-off-by: Corey Osgood <corey.osgood(a)gmail.com>
 Can you repost an updated patch? V3 has changed quite a bit in the last
 few months, especially considering PCI access functions and other stuff.
I haven't been following v3 development lately, been too busy. I'll
try to bring this up as soon as I get a chance.

---snip---
  +     l =
(unsigned long *)ioapic_base;
 +
 +     /* Set APIC to FSB message bus. */
 +     l[0] = 0x3;
 Magic values...

  +     val = l[4];
 +     l[4] = (val & 0xFFFFFE) | 1;
 The line above is equivalent to
 l[4] = (val & 0xffffff) | 1;

 You decide which one is clearer. Anyway, the 0xFFFFFE is magic.

  +
 +     /* Set APIC ADDR - this will be VT8237R_APIC_ID. */
 +     l[0] = 0;
 +     val = l[4];
 +     l[4] = (val & 0xF0FFFF) | (VT8237R_APIC_ID << 24);
 +
 +     for (i = 0; i < ARRAY_SIZE(ioapic_table); i++) {
 +             l[0] = (ioapic_table[i].reg * 2) + 0x10;
 +             l[4] = ioapic_table[i].value_low;
 +             value_low = l[4];
 +             l[0] = (ioapic_table[i].reg * 2) + 0x11;
 +             l[4] = ioapic_table[i].value_high;
 +             value_high = l[4];
 +
 +             if ((i == 0) && (value_low == 0xffffffff)) {
 +                     printk_warning("IO APIC not responding.\n");
 +                     return;
 +             }
 +     }
 Reading the code sequence above hurts. You set l[0] and l[4] always in
 combination. What about helper functions like set_apic() and get_apic()?
That would be Rudolf's migraine :p I frankly don't really understand
APIC, so I honestly have no idea what's going on there, that was
pulled right out of v2. I'll try to fix everything else up though.

-Corey

PS: sorry if this gets sent more then once, gmail is acting up today :(

[bari]

Corey Osgood wrote:
  On Sun, Aug 10, 2008 at 3:30 PM, Carl-Daniel
Hailfinger
 <c-d.hailfinger.devel.2006(a)gmx.net> wrote:
   
  Hi Corey,

 this seems to have slipped through the cracks.

 On 24.03.2008 07:08, Corey Osgood wrote:
     
  Add preliminary, as yet untested support for the
vt8237r, based on the code in
 coreboot v2. This will need some fixes, but at the very least it compiles and
 should do smbus operations.

 Signed-off-by: Corey Osgood <corey.osgood(a)gmail.com>

 Can you repost an updated patch? V3 has changed quite a bit in the last
 few months, especially considering PCI access functions and other stuff.

 I haven't been following v3 development lately, been too busy. I'll
 try to bring this up as soon as I get a chance.

The current vt8237x tree has changed quite a bit (vt8237s is being added 
as well) and will have a few more changes. Let us know when/if you have 
time and we will hopefully have a nice working V2 by then.

-Bari

[Corey Osgood]

Quick question: what southbridge should I be looking at as a good
example of how trying to do things? Does amd/8111 have a correct
dts/Kconfig setup?

Thanks,
Corey

On Tue, Aug 12, 2008 at 11:22 PM, bari <bari(a)onelabs.com> wrote:
  Corey Osgood wrote:
 
 On Sun, Aug 10, 2008 at 3:30 PM, Carl-Daniel Hailfinger
 <c-d.hailfinger.devel.2006(a)gmx.net> wrote:

 
 Hi Corey,

 this seems to have slipped through the cracks.

 On 24.03.2008 07:08, Corey Osgood wrote:

 
 Add preliminary, as yet untested support for the vt8237r, based on the
 code in
 coreboot v2. This will need some fixes, but at the very least it
 compiles and
 should do smbus operations.

 Signed-off-by: Corey Osgood <corey.osgood(a)gmail.com>

 Can you repost an updated patch? V3 has changed quite a bit in the last
 few months, especially considering PCI access functions and other stuff.

 I haven't been following v3 development lately, been too busy. I'll
 try to bring this up as soon as I get a chance.

 The current vt8237x tree has changed quite a bit (vt8237s is being added as
 well) and will have a few more changes. Let us know when/if you have time
 and we will hopefully have a nice working V2 by then.

 -Bari

[ron minnich]

On Thu, Aug 28, 2008 at 12:21 AM, Corey Osgood <corey.osgood(a)gmail.com> wrote:
  Quick question: what southbridge should I be looking
at as a good
 example of how trying to do things? Does amd/8111 have a correct
 dts/Kconfig setup?

yeah, 8111 I think is the best example for now.

ron