[LinuxBIOS] [PATCH] Add support for the Asus A8V-E SE motherboard
Stefan Reinauer
stepan at coresystems.de
Wed Oct 31 11:29:56 CET 2007
* Uwe Hermann <uwe at hermann-uwe.de> [071030 15:18]:
> > +
> > +void acpi_create_fadt(acpi_fadt_t * fadt, acpi_facs_t * facs, void *dsdt)
> > +{
> > + acpi_header_t *header = &(fadt->header);
> > +
> > + memset((void *) fadt, 0, sizeof(acpi_fadt_t));
> > + memcpy(header->signature, "FACP", 4);
> > + header->length = 244;
> > + header->revision = 1;
> > + memcpy(header->oem_id, "LXBIOS", 6);
>
> "LinuxBIOS" as string? How long can the ID become?
>
> > + memcpy(header->oem_table_id, "LXBACPI ", 8);
> > + memcpy(header->asl_compiler_id, "LXB", 8);
> Same here. How long?
How about a little RTFS in the reviews? ;-) SCNR
from src/arch/i386/include/arch/acpi.h:
typedef struct acpi_table_header /* ACPI common table header */
{
char signature [4]; /* ACPI signature (4 ASCII characters) */\
u32 length; /* Length of table, in bytes, including header */\
u8 revision; /* ACPI Specification minor version # */\
u8 checksum; /* To make sum of entire table == 0 */\
char oem_id [6]; /* OEM identification */\
char oem_table_id [8]; /* OEM table identification */\
u32 oem_revision; /* OEM revision number */\
char asl_compiler_id [4]; /* ASL compiler vendor ID */\
u32 asl_compiler_revision; /* ASL compiler revision number */
} __attribute__ ((packed)) acpi_header_t;
> > + fadt->firmware_ctrl = facs;
> > + fadt->dsdt = dsdt;
> > + fadt->preferred_pm_profile = 0;
> > + fadt->sci_int = 9;
> > + fadt->smi_cmd = 0;
> > + fadt->acpi_enable = 0;
> > + fadt->acpi_disable = 0;
> > + fadt->s4bios_req = 0x0;
> > + fadt->pstate_cnt = 0x0;
[..]
> > + fadt->x_gpe1_blk.space_id = 1;
> > + fadt->x_gpe1_blk.bit_width = 0;
> > + fadt->x_gpe1_blk.bit_offset = 0;
> > + fadt->x_gpe1_blk.resv = 0;
> > + fadt->x_gpe1_blk.addrl = 0x0;
> > + fadt->x_gpe1_blk.addrh = 0x0;
> > +
> > + header->checksum =
> > + acpi_checksum((void *) fadt, sizeof(acpi_fadt_t));
>
> Let's commit this, but can it be changed later?
> This all looks very similar, maybe a 'static const struct ...' table
> with all the numbers, and a small loop to set the entries would
> be possible?
This table would be struct acpi_fadt? And copying it to ram ("the loop")
would work with memcpy?
> > Index: src/mainboard/asus/a8v-e_se/acpi_tables.c
> > ===================================================================
> > --- src/mainboard/asus/a8v-e_se/acpi_tables.c (revision 0)
> > +++ src/mainboard/asus/a8v-e_se/acpi_tables.c (revision 0)
> > @@ -0,0 +1,170 @@
> > +/*
> > + * This file is part of the LinuxBIOS project.
> > + * 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
> > + *
> > + * Copyright (C) 2007 Rudolf Marek <r.marek at assembler.cz>
>
> Hm, please add
>
> Copyright (C) xxxx Stefan Reinauer <stepan at openbios.org>
> Copyright (C) xxxx Nick Barker <nick.barker9 at btinternet.com>
>
> then. I think this is not done in the original file either, and should
> be fixed there, too. Listing all copyright owners explicitly is important.
Let's fix this issue in another patch. It is pretty irrelevant for this review.
Stefan
--
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: info at coresystems.de • http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866
More information about the coreboot
mailing list