On 11/28/10 12:36 PM, Uwe Hermann wrote: > On Sat, Nov 27, 2010 at 12:02:46PM +0100, Tobias Diedrich wrote: >> Add acpi_is_wakeup_early to i82371eb and P2B. >> Build fix for src/arch/i386/boot/acpi.c if !CONFIG_SMP >> Also check for acpi_slp_type 2 in acpi_is_wakeup, since S2 >> uses the same acpi wakeup vector as S3. >> Other chipsets so far only ever set acpi_slp_type to 0 and 3. >> >> Abuild-tested. >> >> Signed-off-by: Tobias Diedrich <ranma+coreb...@tdiedrich.de> > Hm, not sure I feel confident enough to ack this, esp. as it might > affect non-P2B boards, but for now here's a quick review below. > > >> Index: coreboot-svn-p2b/src/arch/i386/boot/acpi.c >> =================================================================== >> --- coreboot-svn-p2b.orig/src/arch/i386/boot/acpi.c 2010-11-27 >> 11:48:28.000000000 +0100 >> +++ coreboot-svn-p2b/src/arch/i386/boot/acpi.c 2010-11-27 >> 11:48:41.000000000 +0100 >> @@ -481,7 +481,7 @@ >> >> static int acpi_is_wakeup(void) >> { >> - return (acpi_slp_type == 3); >> + return (acpi_slp_type == 3 || acpi_slp_type == 2); > Can this have negative effects for other ACPI-enabled chipsets/boards?
Oh yeah this is not good. acpi_slp_type must be 3 for wakeup. >> +#if CONFIG_HAVE_ACPI_RESUME == 1 >> + reg = (inw(DEFAULT_PMBASE + PMCNTRL) >> 10) & 7; >> + switch (reg) { >> + case 1: >> + acpi_slp_type = 3; >> + break; >> + case 2: >> + case 3: >> + acpi_slp_type = 2; >> + break; >> + default: >> + acpi_slp_type = 5; >> + break; >> + } >> + printk(BIOS_INFO, >> + "%s: acpi_slp_type=%d!\n", __func__, acpi_slp_type); > I'd drop __func__ and make this a bit more userfriendly by wording it > something like "ACPI sleep type = %d" or similar. why do you need 2,3 and 5 for sleep types anyways? Stefan -- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot