No subject

>> +
>> +
>> +	// 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
>> +	pci_write_config8(dev, 0x41, 0x0);
>>     
>
> Should be the maximum possible, does that make sense? Or it could be
> a config option.
>   

I'll test this soon. Do as you wish with it, I can patch it up later if
needed.

>> +
>> +	// Set bit 6 of 0x40, because Award does it (IO recovery time)
>> +	// IMPORTANT FIX - EISA 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);
>> +
>> +	// Set 0x42 to 0xf8 to match Award bios
>>     
>
> Nah, please don't try to blindly match what other BIOSes do, only if
> there's a good reason (functionality or performance). We want to
> know exactly _why_ this is done (or not).
>   

It's interrupt options, explaning would be a pain without just copy and
pasting the datasheet. Perhaps a "refer to the datasheet" would be best.

>> +	enables = pci_read_config8(dev, 0x42);
>> +	enables |= 0xf8;
>> +	pci_write_config8(dev, 0x42, enables);
>> +
>> +
>> +	/* Delay Transaction Control */
>> +	pci_write_config8(dev, 0x43, 0xb);
>> +
>> +	/* IO Recovery time */
>> +	pci_write_config8(dev, 0x4c, 0x44);
>> +
>> +	/* ROM Memory Cycles Go To LPC */
>> +	pci_write_config8(dev, 0x59, 0x80);
>> +
>> +	/* bypass 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 to match award (use INIT# as cpu reset) */
>> +	enables = pci_read_config8(dev, 0x4f);
>> +	enables |= 0x08;
>> +	pci_write_config8(dev, 0x4f, enables);
>> +
>> +	/* enable serial irq */
>> +	pci_write_config8(dev, 0x52, 0x9);
>> +
>> +
>> +
>> +	/* dma */
>> +	//pci_write_config8(dev, 0x53, 0x00);
>> +
>> +	//enable HPET, ACPI has define to fixed addr
>> +#define HPET_ADDR  0xfed00000ULL
>>     
>
> Put this at the top of the file or in the *.h file. Does it make sense
> to make a config option out of it?
>   

No config option, it's a fixed address. I'm writing this from the bottom
up, so the lower posts explain a bit more. Should be at the top of the
file though, or else hardcoded (or perhaps even defined in acpi files,
since these seem to be generic values). It might already be defined in
some acpi file.

>> +void vt8237r_read_resources(device_t dev)
>> +{
>> +	struct resource *res;
>> +
>> +	pci_dev_read_resources(dev);
>> +
>> +	/* fixed APIC resource */
>> +	res = new_resource(dev, 0x44);
>> +	res->base = 0xfec00000;
>>     
>
> Is this different from HPET_ADDR? Should it?
>   

Yes, this is ioapic not hpet.

>> +	vt8237r_init(dev);
>> +	pci_routing_fixup(dev);
>> +	setup_ioapic(0xfec00000);
>>     
>
> Why hardcode this number here again?
>   

This is a non-configurable address, messing around with it will bork
things up.

>> +	if (resource) {
>> +		report_resource_stored(dev, resource, "<mmconfig>");
>> +
>> +		/* Remember this resource has been stored */
>> +		resource->flags |= IORESOURCE_STORED;
>> +		pci_write_config8(dev, 0x61, (resource->base >> 28));
>> +		reg = pci_read_config8(dev, 0x60);
>> +		reg |= 0x3;
>> +		/* enable MMCONFIG decoding */
>> +		pci_write_config8(dev, 0x60, reg);
>> +	}
>> +
>> +	pci_dev_set_resources(dev);
>> +}
>> +
>> +
>> +
>> +static void apic_mmconfig_read_resources(device_t dev)
>> +{
>> +	struct resource *res;
>> +	pci_dev_read_resources(dev);
>> +
>> +	res = new_resource(dev, 0x40);
>> +	/* NB APIC fixed to this addr */
>> +	res->base = 0xfecc0000;
>>     
>
> Hardcoded again. I think this should be a #define in *.h or even a
> config option, rather than hardcoding it in all places.
>   

Another fixed address, I think this one is LAPIC. Config options are not
an option, these aren't configurable.

>> +
>> +##
>> +## Build code to export an x86 MP table
>> +## Useful for specifying IRQ routing values
>> +##
>> +default HAVE_MP_TABLE=1
>> +
>> +##
>> +## Build code to export a CMOS option table
>> +##
>> +##default HAVE_OPTION_TABLE=1
>>     
>
> Either enable it, or drop cmos.layout below.
>   

Huh? afaik it is enabled (with default USE_OPTION_TABLE !=
USE_FALLBACK_IMAGE below).

>> Index: src/mainboard/asus/a8v/acpi_tables.c
>> ===================================================================
>> --- src/mainboard/asus/a8v/acpi_tables.c	(revision 0)
>> +++ src/mainboard/asus/a8v/acpi_tables.c	(revision 0)
>> @@ -0,0 +1,155 @@
>> +/*
>> + * LinuxBIOS ACPI Table support
>> + * written by Stefan Reinauer <stepan at openbios.org>
>> + * ACPI FADT, FACS, and DSDT table support added by 
>> + * Nick Barker <nick.barker9 at btinternet.com>, and those portions
>> + * (C) Copyright 2004 Nick Barker
>> + * (C) Copyright 2005 Stefan Reinauer
>> + */
>>     
>
> Missing license. Please check back with Stefan/Nick, but I guess it's
> GPL (v2? v2-or-later?).
>
> How similar is this file to the original one? Does it make sense to
> drop this file and make the original one a generic version which can be
> used by multiple chipsets?
>   

Nope, different boards have different tables. A uniprocessor board has
fewer tables than a multiprocessor board, and different archs also have
different/other tables. I think acpi is as generic as it's going to get.

>> Index: src/mainboard/asus/a8v/fadt.c
>> ===================================================================
>> --- src/mainboard/asus/a8v/fadt.c	(revision 0)
>> +++ src/mainboard/asus/a8v/fadt.c	(revision 0)
>> @@ -0,0 +1,157 @@
>> +/*
>> + * ACPI - create the Fixed ACPI Description Tables (FADT)
>> + * (C) Copyright 2004 Nick Barker <nick.barker9 at btinternet.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 <string.h>
>> +#include <arch/acpi.h>
>> +
>> +void acpi_create_fadt(acpi_fadt_t * fadt, acpi_facs_t * facs, void *dsdt)
>> +{
>>     
>
> Did you modify this file? How? Should probably be a common global file,
> not copied in each target?

The FADT is/should be mainboard-specific. So please don't try to
generic-ize it. My fadt is somewhat different, FWIW.

-Corey