Few comments, please resend with these fixes and I'll ack, I need this to get in as well (so I can figure out how to patch against it without breaking things on your end).
Index: src/southbridge/via/vt8237r/TODO
--- src/southbridge/via/vt8237r/TODO (revision 0) +++ src/southbridge/via/vt8237r/TODO (revision 0) @@ -0,0 +1,26 @@ +NB & SB
+Fix DMA to 0xe0000-0xeffff - done +Move NB APIC init to NB code - done +MMCONFIG for PCI is reserved but not propagated (0xe0000000) - done +HPET is disabled - fixed
+Motherboard code: +Fix reboot -done
+K8 CODE:
+Fix the 2 dimms memory - done +Fix the resource typo - done?
+Fix enable more than 512KB flash chip
Hrm, not sure about this. vt8237r supports 4Mb to 16Mb flash, but only if it's strapped correctly. I'm not sure why they did that, but it's extremely annoying. So unless your chip is strapped correctly (or you change the strapping) you can't use any larger than the motherboard manufacturer decided to (here, it's 4MB :( )
+Fix HPET acpi record to match the ACPI reported table +Fix the e820 reserved region for MMCONFIG +Fix PCIe startup - done
+Fix error conditiion when PCIe Link is not trained correctly. +Disable GART in NB?
I have no objection to a TODO list, I rather like the idea. But please have separate lists for each part.
Index: src/southbridge/via/vt8237r/romstrap.inc
--- src/southbridge/via/vt8237r/romstrap.inc (revision 0) +++ src/southbridge/via/vt8237r/romstrap.inc (revision 0) Index: src/southbridge/via/vt8237r/romstrap.lds =================================================================== --- src/southbridge/via/vt8237r/romstrap.lds (revision 0) +++ src/southbridge/via/vt8237r/romstrap.lds (revision 0)
What are these files all about? Do they really belong in vt8237r?
+void set_led() +{
- // set power led to steady now that lxbios has virtually done its job
- //device_t dev;
- //dev = dev_find_device(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_8237, 0);
- //pci_write_config8(dev, 0x94, 0xb0);
+}
Please just remove this function and all calls to it.
Index: src/southbridge/via/vt8237r/vt8237r.h
--- src/southbridge/via/vt8237r/vt8237r.h (revision 0) +++ src/southbridge/via/vt8237r/vt8237r.h (revision 0) @@ -0,0 +1,2 @@ +//GPL here
Same here, no need for an empty file.
+static unsigned int get_spd_data(unsigned int dimm, unsigned int offset) +{
- unsigned int val;
- print_debug("DIMM ");
- print_debug_hex8(dimm);
- print_debug(" OFFSET ");
- print_debug_hex8(offset);
- print_debug("\r\n");
- smbus_reset();
- /* clear host data port */
- outb(0x00, SMBHSTDAT0);
- SMBUS_DELAY();
- smbus_wait_until_ready();
- /* Do some mathmatic magic */
- dimm = (dimm << 1);
+// dimm &= 0x0E; +// dimm |= 0xA1;
- dimm |= 1;
- outb(dimm, SMBXMITADD);
- outb(offset, SMBHSTCMD);
- outb(0x48, SMBHSTCTL);
- SMBUS_DELAY();
- smbus_wait_until_ready();
- val = inb(SMBHSTDAT0);
- print_debug("Read: ");
- print_debug_hex8(val);
- print_debug("\r\n");
- smbus_reset(); /* probably don't have to do this, but it
can't hurt */
- return val; /* can I just "return inb(SMBHSTDAT0)"? */
+}
+static unsigned int smbus_read_byte(unsigned int dimm, unsigned int offset) +{
- unsigned int data;
- data = get_spd_data(dimm, offset);
- return data;
+}
Please just rename get_spd_data and remove the commented lines entirely. I know I said otherwise in the past. And nuke that line about mathmatic magic, I think I must have been high when I wrote that.
Index: src/southbridge/via/vt8237r/vt8237_bridge.c
--- src/southbridge/via/vt8237r/vt8237_bridge.c (revision 0) +++ src/southbridge/via/vt8237r/vt8237_bridge.c (revision 0) @@ -0,0 +1,60 @@ +/*
- This file is part of the LinuxBIOS project.
- Copyright (C) 2007 Rudolf Marek r.marek@assembler.cz
- 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
- */
+#include <device/device.h> +#include <device/pci.h> +#include <device/pci_ops.h> +#include <device/pci_ids.h> +#include <console/console.h>
+static void smth_enable(struct device *dev) +{
+// print_err("B188 device dump\n"); +// VIA recommends this, sorry no known info
- writeback(dev, 0x40, 0x91);
- writeback(dev, 0x41, 0x40);
- writeback(dev, 0x43, 0x44);
- writeback(dev, 0x44, 0x31);
- writeback(dev, 0x45, 0x3a);
- writeback(dev, 0x46, 0x88); //PCI ID
- writeback(dev, 0x47, 0xb1); //PCI ID
- writeback(dev, 0x3e, 0x16); //bridge control????
+// dump_south(dev);
+}
+static struct device_operations smth_ops = {
- .read_resources = pci_bus_read_resources,
- .set_resources = pci_dev_set_resources,
- .enable_resources = pci_bus_enable_resources,
- .enable = smth_enable,
- .scan_bus = pci_scan_bridge,
- .reset_bus = pci_bus_reset,
- .ops_pci = 0,
+};
+static struct pci_driver northbridge_driver __pci_driver = {
- .ops = &smth_ops,
- .vendor = PCI_VENDOR_ID_VIA,
- .device = 0xb188,
+};
I don't think this is really part of the southbridge, rather the northbridge. Biggest justification for that idea is that the device ID is different from mine (0xb198), and mine is listed in the northbridge docs. I know lspci lists it as part of the southbridge, but it's probably wrong.
Index: src/cpu/x86/lapic/lapic_cpu_init.c
--- src/cpu/x86/lapic/lapic_cpu_init.c (revision 2776) +++ src/cpu/x86/lapic/lapic_cpu_init.c (working copy) @@ -60,7 +60,7 @@ * Starting actual IPI sequence... */
- printk_spew("Asserting INIT.\n");
printk_err("Asserting INIT. on %x\n", apicid);
/*
- Turn INIT on target chip
Yes to the change in the message, no to the change in printk level. Please leave at spew or debug.
Index: src/mainboard/asus/a8v/acpi-dsdt.aml
Cannot display: file marked as a binary type. svn:mime-type = application/octet-stream
Property changes on: src/mainboard/asus/a8v/acpi-dsdt.aml ___________________________________________________________________ Name: svn:mime-type
- application/octet-stream
Remove this from the patch please ;)
Aside from that, there's a bunch of whitespace issues, etc, which can be fixed later.
-Corey