<br><br><div class="gmail_quote">On Thu, Oct 15, 2009 at 6:24 PM, Peter Stuge <span dir="ltr"><<a href="mailto:peter@stuge.se">peter@stuge.se</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div><div></div><div class="h5">Myles Watson wrote:<br>
> Initialize the interrupts even if you don't generate the MP_TABLE.  I<br>
> just copied the values from mptable.c.<br>
> Also set IRQ 9 to be edge-triggered to make Linux stop complaining.<br>
><br>
> Signed-off-by: Myles Watson <<a href="mailto:mylesgw@gmail.com">mylesgw@gmail.com</a>><br>
<br>
</div></div>Acked-by: Peter Stuge <<a href="mailto:peter@stuge.se">peter@stuge.se</a>><br></blockquote><div>Rev 4787. <br></div><div> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

But some comments:<br></blockquote><div>Thanks for reviewing!<br> <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
> +++ cbv2/src/mainboard/tyan/s2891/acpi_tables.c<br>
> @@ -42,6 +42,19 @@ unsigned long acpi_fill_madt(unsigned lo<br>
>               apic_addr = pci_read_config32(dev, PCI_BASE_ADDRESS_1) & ~0xf;<br>
>               current += acpi_create_madt_ioapic((acpi_madt_ioapic_t *) current, 4,<br>
>                                                  apic_addr, 0);<br>
> +             #if !CONFIG_HAVE_MP_TABLE /* Initialize interrupt mapping. */<br>
> +             {<br>
<br>
I don't think this is how Ron meant he likes CONFIG_ values in code.<br>
More like:<br>
<br>
if(!CONFIG_HAVE_MP_TABLE) {<br>
}<br></blockquote><div>The problem with this style is that it doesn't support conditional compilation.  In this case it doesn't matter, but if you wanted to put in:<br><br>if(CONFIG_HAVE_MP_TABLE) {<br>     use_some_MP_defines();<br>
}<br><br>It would break, wouldn't it?  Since CONFIG_ variables are used to control what gets included in the image, I think we should stick with #if. <br></div><div> </div><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
> @@ -61,7 +74,7 @@ unsigned long acpi_fill_madt(unsigned lo<br>
><br>
>       /* IRQ9 ACPI active low. */<br>
>       current += acpi_create_madt_irqoverride((acpi_madt_irqoverride_t *)<br>
> -             current, 0, 9, 9, MP_IRQ_TRIGGER_LEVEL | MP_IRQ_POLARITY_LOW);<br>
> +             current, 0, 9, 9, MP_IRQ_TRIGGER_EDGE | MP_IRQ_POLARITY_LOW);<br>
<br>
This is definately something that should not be unique per board. I<br>
expect not just Tyan boards will have this code. Could it be moved<br>
out to a common place?<br></blockquote><div>Actually, the factory BIOS and asus/m2v-mx_2e have it level triggered, which is why I had it wrong.  It's a southbridge setting.  The other parts of acpi_tables are fairly generic, but acpi_fill_madt has very little duplication.<br>
<br>Thanks,<br>Myles<br></div></div><br>