[coreboot] [PATCH 6/6] Intel EP80579 Development Board mainboard
Uwe Hermann
uwe at hermann-uwe.de
Mon Aug 25 22:49:51 CEST 2008
Comments below, but this should be the last commit anyway (after all the
NB/SB code is committed; I'll try to review that ASAP).
On Wed, Aug 20, 2008 at 09:19:35AM -0700, Ed Swierk wrote:
> Index: coreboot-v2-3363/src/mainboard/intel/truxton/Config.lb
> ===================================================================
> --- /dev/null
[...]
> +chip northbridge/intel/i3100
> + device pci_domain 0 on
> + device pci 00.0 on end # IMCH
> + device pci 00.1 on end # IMCH error status
> + device pci 01.0 on end # IMCH EDMA engine
> + device pci 02.0 on end # PCIe port A/A0
> + device pci 03.0 on end # PCIe port A1
> + device pci 04.0 on end
> + device pci 08.0 off end
> + device pci 0d.0 off end
> + device pci 0d.1 off end
What are these? Please add comments. Why are three of them disabled?
Not available on this board?
> + device pci 1f.2 on end # SATA
> + device pci 1f.3 on end # SMBus
> + device pci 1f.4 on end
Same here.
> + end
> + end
> + device apic_cluster 0 on
> + chip cpu/intel/ep80579
> + device apic 0 on end
> + end
> + end
> +end
> Index: coreboot-v2-3363/src/mainboard/intel/truxton/auto.c
> ===================================================================
> --- /dev/null
> +++ coreboot-v2-3363/src/mainboard/intel/truxton/auto.c
> @@ -0,0 +1,114 @@
[...]
> +static void main(unsigned long bist)
> +{
> + msr_t msr;
> + u16 perf;
> + static const struct mem_controller mch[] = {
> + {
> + .node_id = 0,
> + .f0 = PCI_DEV(0, 0x00, 0),
> + .channel0 = { (0xa<<3)|2, (0xa<<3)|3 },
> + }
> + };
> +
> + if (bist == 0) {
> + /* Skip this if there was a built in self test failure */
> + early_mtrr_init();
> + if (memory_initialized()) {
> + asm volatile ("jmp __cpu_reset");
> + }
> + }
> + /* Set up the console */
> + i3100_enable_superio();
> + i3100_enable_serial(0x4e, I3100_SP1, TTYS0_BASE);
Maybe make the 0x4e a #define (e.g. I3100_SUPERIO_CONFIG_PORT) for
better readability.
> + uart_init();
> + console_init();
> +
> + /* Prevent the TCO timer from rebooting us */
> + i3100_halt_tco_timer();
> +
> + /* Halt if there was a built in self test failure */
> + report_bist_failure(bist);
> +
> +#if 0
> + print_pci_devices();
> +#endif
> + enable_smbus();
> +#if 1
> + dump_spd_registers();
> +#endif
> +
> + sdram_initialize(ARRAY_SIZE(mch), mch);
> +#if 1
> + dump_pci_devices();
> +#endif
> + dump_pci_device(PCI_DEV(0, 0x00, 0));
> +#if 0
> + dump_bar14(PCI_DEV(0, 0x00, 0));
> +#endif
> +
> +#if 0
> + ram_fill(0x00000000, 0x20000000);
> + ram_verify(0x00000000, 0x20000000);
> +#endif
Remove the "#if 1" please, and turn "#if 0" into regular
/*foo*/-style comments if the functions are needed/wanted.
> Index: coreboot-v2-3363/src/mainboard/intel/truxton/irq_tables.c
> ===================================================================
> --- /dev/null
> +++ coreboot-v2-3363/src/mainboard/intel/truxton/irq_tables.c
> @@ -0,0 +1,44 @@
> +/*
> + * This file is part of the coreboot project.
> + *
> + * Copyright (C) 2008 Arastra, Inc.
> + *
> + * 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 <arch/pirq_routing.h>
> +
> +const struct irq_routing_table intel_irq_routing_table = {
> + PIRQ_SIGNATURE, /* u32 signature */
> + PIRQ_VERSION, /* u16 version */
> + 32+16*IRQ_SLOT_COUNT, /* u16 Table size 32+(16*devices) */
> + 0x00, /* u8 Bus 0 */
> + (0x1f << 3) | 0x0, /* u8 Device 1f, Function 0 */
> + 0x0000, /* u16 reserve IRQ for PCI */
> + 0x8086, /* u16 Vendor */
> + 0x5031, /* Device ID */
> + 0x00000000, /* u32 miniport_data */
> + { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, /* u8 rfu[11] */
> + 0x5e, /* u8 checksum - mod 256 checksum must give zero */
> + { /* bus, devfn, {link, bitmap}, {link, bitmap}, {link, bitmap}, {link, bitmap}, slot, rfu */
> + {0x00, 0xf8, {{0x62, 0xdc78}, {0x61, 0xdcf8}, {0x00, 0x0000}, {0x00, 0x0000}}, 0x00, 0x00},
Please add a coment what device/slot this is.
> Index: coreboot-v2-3363/src/mainboard/intel/truxton/mptable.c
> ===================================================================
> --- /dev/null
> +++ coreboot-v2-3363/src/mainboard/intel/truxton/mptable.c
> @@ -0,0 +1,199 @@
[...]
> +void *smp_write_config_table(void *v)
> +{
> + static const char sig[4] = "PCMP";
> + static const char oem[8] = "Intel ";
> + static const char productid[12] = "Truxton ";
> + struct mp_config_table *mc;
> + u8 bus_num;
> + u8 bus_isa;
> + u8 bus_pea0 = 0;
> + u8 bus_pea1 = 0;
> + u8 bus_aioc;
> +
> + mc = (void *)(((char *)v) + SMP_FLOATING_TABLE_LEN);
> + memset(mc, 0, sizeof(*mc));
> +
> + memcpy(mc->mpc_signature, sig, sizeof(sig));
> + mc->mpc_length = sizeof(*mc); /* initially just the header */
> + mc->mpc_spec = 0x04;
> + mc->mpc_checksum = 0; /* not yet computed */
> + memcpy(mc->mpc_oem, oem, sizeof(oem));
> + memcpy(mc->mpc_productid, productid, sizeof(productid));
> + mc->mpc_oemptr = 0;
> + mc->mpc_oemsize = 0;
> + mc->mpc_entry_count = 0; /* No entries yet... */
> + mc->mpc_lapic = LAPIC_ADDR;
> + mc->mpe_length = 0;
> + mc->mpe_checksum = 0;
> + mc->reserved = 0;
> +
> + smp_write_processors(mc);
> +
> + {
> + device_t dev;
> +
> + /* AIOC bridge */
> + dev = dev_find_slot(0, PCI_DEVFN(0x04,0));
> + if (dev) {
> + bus_aioc = pci_read_config8(dev, PCI_SECONDARY_BUS);
> + bus_isa = pci_read_config8(dev, PCI_SUBORDINATE_BUS);
> + bus_isa++;
> + }
> + else {
> + printk_debug("ERROR - could not find PCI 0:04.0\n");
> + bus_aioc = 0;
> + bus_isa = 9;
> + }
> + /* PCIe A0 */
> + dev = dev_find_slot(0, PCI_DEVFN(0x02,0));
> + if (dev) {
> + bus_pea0 = pci_read_config8(dev, PCI_SECONDARY_BUS);
> + }
> + else {
> + printk_debug("ERROR - could not find PCI 0:02.0\n");
> + bus_pea0 = 0;
> + }
> + /* PCIe A1 */
> + dev = dev_find_slot(0, PCI_DEVFN(0x03,0));
> + if (dev) {
> + bus_pea1 = pci_read_config8(dev, PCI_SECONDARY_BUS);
> + }
> + else {
> + printk_debug("ERROR - could not find PCI 0:03.0\n");
> + bus_pea1 = 0;
> + }
> + }
Thy is this an extra block? Any technical reason? If yes, it should be
documented in a comment, or the block be "inlined".
Uwe.
--
http://www.hermann-uwe.de | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
More information about the coreboot
mailing list