[coreboot] [PATCH] Add ACPI generic resume support

Stefan Reinauer stepan at coresystems.de
Sat Apr 11 20:43:01 CEST 2009


Wonderful!



On 11.04.2009 17:50 Uhr, Rudolf Marek wrote:

> Index: src/arch/i386/boot/Config.lb
> ===================================================================
> --- src/arch/i386/boot/Config.lb	(revision 4093)
> +++ src/arch/i386/boot/Config.lb	(working copy)
> @@ -14,4 +14,5 @@
>  if HAVE_ACPI_TABLES
>  object acpi.o
>  object acpigen.o
> +object wakeup.S
>  end
>   
Can you please guard the code with CONFIG_RESUME_SUPPORT? Not everyone
wants the code included.

> Index: src/arch/i386/boot/acpi.c
> ===================================================================
> --- src/arch/i386/boot/acpi.c	(revision 4093)
> +++ src/arch/i386/boot/acpi.c	(working copy)
> @@ -27,6 +27,9 @@
>   


> +int acpi_is_wakeup(void) {
> +	return (acpi_slp_type == 3);
> +}
>   

Please, for functions, put the { on a new line.


> +static acpi_rsdp_t *valid_rsdp(acpi_rsdp_t *rsdp) {
> +
> +	if (strncmp((char *)rsdp, RSDP_SIG, sizeof(RSDP_SIG) - 1) != 0)
> +		return NULL;
>   
Maybe you can compare a 32bit number here instead of doing a strncmp?
Not sure if this is any timing critical, but it looks like a lot of byte
wise memory access can be spared here.

> +extern char *lowmem_backup;
> +extern char *lowmem_backup_ptr;
> +extern int lowmem_backup_size;
>
>   

Please also put stuff like that in CONFIG_RESUME_SUPPORT



-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: info at coresystems.dehttp://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866





More information about the coreboot mailing list