Re: [coreboot] unifying `acpi_tables.c`

[shawn Bai]

Hi,folks,  
I searched for 'acpi_tables.c' in directory 'mainboard', and 49 entries come.They are as follows, but not all listed here,just for your reference.
coreboot\src\mainboard\advansus\a785e-i\acpi_tables.ccoreboot\src\mainboard\amd\bimini_fam10\acpi_tables.ccoreboot\src\mainboard\amd\dbm690t\acpi_tables.ccoreboot\src\mainboard\amd\inagua\acpi_tables.ccoreboot\src\mainboard\amd\mahogany\acpi_tables.ccoreboot\src\mainboard\amd\mahogany_fam10\acpi_tables.ccoreboot\src\mainboard\amd\persimmon\acpi_tables.ccoreboot\src\mainboard\amd\pistachio\acpi_tables.c 
I don't know ACPI and don't know the reason why there are so many files with the same name scattering in different subdirectories. 
For different mainboards, should the implementation in 'acpi_tables.c' be so different? Does it depend on ACPI spec or the mainboard details, or both? 
Maybe the answer is quite clear; I know little, so forgive me have this problem. 
I find the contents of some 'acpi_tables.c' among the 49 entries are the same，or just different with a conditional preprocessor macros,such as '#if CONFIG_BOARD_HAS_FADT == 1'.
Definitely, the 49 files with the same name 'acpi_tables.c' can be combined into one or more files, but the number must be less than 49, and this can reduce the chance of errors.

----------------------------------------
> Message: 5
> Date: Thu, 19 May 2011 00:14:49 +0200
> From: Paul Menzel 
> To: [e-mail]
> Subject: [coreboot] unifying `acpi_tables.c` (was: [PATCH] workaround
> for "An unexpected error (805262864) occurred at line 1768 of
> d:\xpclient\base\boot\setup\arcdisp.c")
> Message-ID: 
> Content-Type: text/plain; charset="utf-8"
>
> Dear Scott and coreboot folks,
>
> Am Mittwoch, den 18.05.2011, 01:40 -0500 schrieb Scott Duplichan:
>
> [?]
>
> looking at the change
> > Index: src/mainboard/amd/bimini_fam10/acpi_tables.c
> >
> > --- src/mainboard/amd/bimini_fam10/acpi_tables.c (revision 6599)
> > +++ src/mainboard/amd/bimini_fam10/acpi_tables.c (working copy)
> > @@ -127,6 +127,31 @@
> > acpi_write_rsdp(rsdp, rsdt, NULL);
> > acpi_write_rsdt(rsdt);
> > /* DSDT */
> > current = ( current + 0x07) & -0x08;
> > printk(BIOS_DEBUG, "ACPI: * DSDT at %lx\n", current);
> > dsdt = (acpi_header_t *)current; // it will used by fadt
> > memcpy(dsdt, &AmlCode, sizeof(acpi_header_t));
> > current += dsdt->length;
> > memcpy(dsdt, &AmlCode, dsdt->length);
> > printk(BIOS_DEBUG, "ACPI: * DSDT @ %p Length %x\n",dsdt,dsdt->length);
> > /* FACS */ // it needs 64 bit alignment
> > current = ( current + 0x07) & -0x08;
> > printk(BIOS_DEBUG, "ACPI: * FACS at %lx\n", current);
> > facs = (acpi_facs_t *) current; // it will be used by fadt
> > current += sizeof(acpi_facs_t);
> > acpi_create_facs(facs);
> > /* FADT */
> > current = ( current + 0x07) & -0x08;
> > printk(BIOS_DEBUG, "ACPI: * FADT at %lx\n", current);
> > fadt = (acpi_fadt_t *) current;
> > current += sizeof(acpi_fadt_t);
> > acpi_create_fadt(fadt, facs, dsdt);
> > acpi_add_table(rsdp, fadt);
>
> [?]
>
> the contents seems to very similar. Besides some comments even
> identical.
>
> Can `acpi_tables.c` be shared between certain families or even all new
> boards?
>
> If yes, maybe that could be put on a todo list.
>
> Thanks,
>
> Paul
> -------------- next part --------------
> A non-text attachment was scrubbed...
> Name: not available
> Type: application/pgp-signature
> Size: 198 bytes
> Desc: This is a digitally signed message part
> URL:
>
> Message: 6
> Date: Thu, 19 May 2011 00:21:35 +0200
> From: Peter Stuge 
> To: [e-mail]
> Subject: Re: [coreboot] unifying `acpi_tables.c`
> Message-ID: 
> Content-Type: text/plain; charset="us-ascii"
>
> Paul Menzel wrote:
> > Can `acpi_tables.c` be shared between certain families or even all
> > new boards?
> >
> > If yes, maybe that could be put on a todo list.
>
> Yeah, I ranted about this a bit on IRC. Will take a look in Prague.
>
> //Peter
> -------------- next part --------------
> A non-text attachment was scrubbed...
> Name: not available
> Type: application/pgp-signature
> Size: 190 bytes
> Desc: not available
> URL:
>
> coreboot mailing list
> [e-mail]
> http://www.coreboot.org/mailman/listinfo/coreboot
>
> End of coreboot Digest, Vol 75, Issue 55

unifying `acpi_tables.c`

[Stefan Reinauer]

* shawn Bai  [110522 06:28]:
> Hi,folks,  
> I searched for 'acpi_tables.c' in directory 'mainboard', and 49
> entries come.They are as follows, but not all listed here,just for
> your reference.

[..]
> I don't know ACPI and don't know the reason why there are so many
> files with the same name scattering in different subdirectories. 
> For different mainboards, should the implementation in 'acpi_tables.c'
> be so different? Does it depend on ACPI spec or the mainboard details,
> or both? 
> Maybe the answer is quite clear; I know little, so forgive me have
> this problem. 
> I find the contents of some 'acpi_tables.c' among the 49 entries are
> the same，or just different with a conditional preprocessor
> macros,such as '#if CONFIG_BOARD_HAS_FADT == 1'.
> Definitely, the 49 files with the same name 'acpi_tables.c' can be
> combined into one or more files, but the number must be less than 49,
> and this can reduce the chance of errors.

They mostly call functions that have been unified in
src/arch/x86/boot/acpi.c

And yes, we should try to unify them and get rid of as much code as
possible and reasonable from the mainboard directories.

Stefan