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+coreboot@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?
static acpi_rsdp_t *valid_rsdp(acpi_rsdp_t *rsdp) @@ -567,9 +567,11 @@ return wake_vec; }
+#if CONFIG_SMP == 1
I think you can write most of the kconfig CONFIG_* variable checks as:
#if CONFIG_SMP
The "== 1" part is _usally_ not needed (there may be exceptions).
+#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.
+/*
- Intel i82371eb (piix4e) datasheet, section 7.2.3, page 142:
Intel 82371EB (PIIX4E)
(cosmetics)
- 000b / 0x0: soft off/suspend to disk (soff/std) S5
- 001b / 0x1: suspend to ram (str) S3
- 010b / 0x2: powered on suspend, context lost (poscl) S2
- Note: 'context lost' menas the cpu restarts at the reset vector
- 011b / 0x3: powered on suspend, cpu context lost (posccl) S1
- Note: Looks like 'cpu context lost' does _not_ mean the cpu
cpu -> CPU
+int acpi_is_wakeup_early(void) +{
- u16 tmp, result;
- print_debug(__func__);
- print_debug("\n");
Please use printk() in general, unless there's a technical reason to resort to print_debug().
Uwe.