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

Reply via email to