Re: [coreboot] [PATCH] s289x ACPI fix

[Peter Stuge]

Myles Watson wrote:
> Initialize the interrupts even if you don't generate the MP_TABLE.  I
> just copied the values from mptable.c.
> Also set IRQ 9 to be edge-triggered to make Linux stop complaining.
>
> Signed-off-by: Myles Watson

Acked-by: Peter Stuge

But some comments:
> +++ cbv2/src/mainboard/tyan/s2891/acpi_tables.c
> @@ -42,6 +42,19 @@ unsigned long acpi_fill_madt(unsigned lo
>    	apic_addr = pci_read_config32(dev, PCI_BASE_ADDRESS_1) & ~0xf;
>    	current += acpi_create_madt_ioapic((acpi_madt_ioapic_t *) current, 4,
>    					   apic_addr, 0);
> #if !CONFIG_HAVE_MP_TABLE /* Initialize interrupt mapping. */
> {
> 	u32 dword;
> 	dword = 0x0120d218;
> 	pci_write_config32(dev, 0x7c, dword);

I don't think this is how Ron meant he likes CONFIG_ values in code.
More like:

if(!CONFIG_HAVE_MP_TABLE) {
}

Personally I favor letting the preprocessor do preprocessing, rather
than pushing this type of code removal to the compiler.
> @@ -61,7 +74,7 @@ unsigned long acpi_fill_madt(unsigned lo
>
> /* IRQ9 ACPI active low. */
>    current += acpi_create_madt_irqoverride((acpi_madt_irqoverride_t *)
> current, 0, 9, 9, MP_IRQ_TRIGGER_LEVEL | MP_IRQ_POLARITY_LOW);
> current, 0, 9, 9, MP_IRQ_TRIGGER_EDGE | MP_IRQ_POLARITY_LOW);

This is definately something that should not be unique per board. I
expect not just Tyan boards will have this code. Could it be moved
out to a common place?

//Peter