Re: [coreboot] [PATCH 6/6] Intel EP80579 Development Board mainboard

[Carl-Daniel Hailfinger]

Hi Ed,

thanks for the reworked patch.

One thing that caught my eye is the new romcc style which hasn't been
adopted in your code yet. Basically, we now build only one romcc copy
and use that for fallback and normal images.

On 14.10.2008 00:49, Ed Swierk wrote:
> At long last, here is a new version of the code for the EP80579
> development mainboard (codename Truxton). It has been tested on real
> hardware, booting a Linux kernel payload and running memtest86+ with
> no errors. This is the final part of the EP80579 (Tolapai) patch set.
>
> I tried to address all your comments from the first version; see
> inline for exceptions.
>
> Signed-off-by: Ed Swierk

With the romcc changes as indicated above and outlined below, this is
Acked-by: Carl-Daniel Hailfinger
> On Mon, Aug 25, 2008 at 1:49 PM, Uwe Hermann  wrote:
> > 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?
>
> I don't know what these are, but I found that the latter three have to
> be set to "off", else the PCI bus setup hangs.

With your comments that code is good to go.
> > >                    device pci 1f.2 on end  # SATA
> > >                    device pci 1f.3 on end  # SMBus
> > >
> > >                    device pci 1f.4 on end
> >
> > Same here.
>
> I don't know what this is, either, and the datasheet doesn't say. But
> I figured since this is a development board there's no harm in leaving
> it on. Perhaps someone else will figure out that it's a built-in MP3
> player or something.

Heh.
> > >            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/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.
>
> I have no idea--my understanding of the IRQ routing business is pretty
> fuzzy. But Linux wasn't happy booting without it, if I recall.

OK for me.
> Index: coreboot-v2/src/mainboard/intel/truxton/Config.lb
>
> --- coreboot-v2/src/mainboard/intel/truxton/Config.lb	(revision 0)
> +++ coreboot-v2/src/mainboard/intel/truxton/Config.lb	(revision 0)
> @@ -0,0 +1,187 @@
> +##
> +## 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
> +##
>
> +##
> +## 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=131072
> +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_MP_TABLE object mptable.o end
> +if HAVE_PIRQ_TABLE object irq_tables.o end
>
> +##
> +## Romcc output
> +##
> +makerule ./failover.E
>    depends "$(MAINBOARD)/../../../arch/i386/lib/failover.c ./romcc"
>    action "./romcc -E -O --label-prefix=failover -I$(TOP)/src -I. $(CPPFLAGS) $(MAINBOARD)/../../../arch/i386/lib/failover.c -o $@"

Here would be the changed romcc location
> +end
>
> +makerule ./failover.inc
>    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 $@"

Same here
> +end
>
> +makerule ./auto.E
>    depends "$(MAINBOARD)/auto.c ./romcc"
>    action "./romcc -E -mcpu=p4 -O2 -I$(TOP)/src -I. $(CPPFLAGS) $(MAINBOARD)/auto.c -o $@"

And here
> +end
> +makerule ./auto.inc
>    depends "$(MAINBOARD)/auto.c ./romcc"
>    action "./romcc -mcpu=p4 -fno-simplify-phi -O2 -I$(TOP)/src -I. $(CPPFLAGS) $(MAINBOARD)/auto.c -o $@"

And here
> +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 cpu/x86/sse/enable_sse.inc
> +mainboardinit ./auto.inc
> +mainboardinit cpu/x86/sse/disable_sse.inc
> +mainboardinit cpu/x86/mmx/disable_mmx.inc
>
> +##
> +## Include the secondary Configuration files
> +##
> +dir /pc80
> +config chip.h
>
> +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 # must be off to boot
>            device pci 0d.0 off end # must be off to boot
>            device pci 0d.1 off end # must be off to boot
>            chip southbridge/intel/i3100
>                    # PIRQ line -> legacy IRQ mappings
>                    register "pirq_a_d" = "0x0b070a05"
>                    register "pirq_e_h" = "0x0a808080"
>                    device pci 1d.0 on end  # USB (UHCI)
>                    device pci 1d.7 on end  # USB (EHCI)
>                    device pci 1f.0 on      # LPC bridge
>                            chip superio/intel/i3100
>                                    device pnp 4e.4 on # Com1
>                                             io 0x60 = 0x3f8
>                                            irq 0x70 = 4
>                                    end
>                                    device pnp 4e.5 on # Com2
>                                             io 0x60 = 0x2f8
>                                            irq 0x70 = 3
>                                    end
>                            end
>                    end
>                    device pci 1f.2 on end  # SATA
>                    device pci 1f.3 on end  # SMBus
>                    device pci 1f.4 on end  # ?
>            end
>    end
>    device apic_cluster 0 on
>            chip cpu/intel/ep80579
>                    device apic 0 on end
>            end
>    end
>
> +end

Regards,
Carl-Daniel