[LinuxBIOS] [PATCH] Add support for the Asus A8V-E SE motherboard

[Stefan Reinauer]

* 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