[coreboot] ACPI: Implementation of the _WAK method in e. g. Google Butterfly

[Paul Menzel]

Dear coreboot folks,


looking into the patch »Persimmon FWTS update: Fix FWTS S3 test.« [1] I
read up on the ACPI `_WAK` (System Wake) method (section 7.3.7 in ACPI
4.0a specification [2]).

        ### Return Value:
        A Package containing two Integers containing status and the power supply S-state
        
        ### Return Value Information
        _WAK returns a package with the following format:
        
        • Element 0 – An Integer containing a bitfield that represents conditions that occurred during sleep.
        0x00000000 – Wake was signaled and was successful
        0x00000001 – Wake was signaled but failed due to lack of power
        0x00000002 – Wake was signaled but failed due to thermal condition
        Other values – Reserved
        
        • Element 1 – An Integer containing the power supply S-state.
        If non-zero, this is the effective S-state the power supply that was actually entered. This value is used
        to detect when the targeted S-state was not entered because of too much current being drawn from the
        power supply. For example, this might occur when some active device’s current consumption pushes
        the system’s power requirements over the low power supply mark, thus preventing the lower power
        mode from being entered as desired.

So first question is, what is, when Element 1 is set to 0?

I was surprised to see, that the recent-Intel chip based boards like
Google Parrot or Butterfly just return a package with both elements set
to zero without any further checks. The AMD boards did the same(?), but
according to the above patch [1] this causes issues with Ubuntu’s
Firmware Test Suite [3].

        $ more src/mainboard/google/butterfly/acpi/platform.asl
        […]
        /* The _WAK method is called on system wake-up */

        Method(_WAK,1)
        {
        	/* Update in case state changed while asleep */
        	/* Update AC status */
        	Store (\_SB.PCI0.LPCB.EC0.ADPT, Local0)
        	if (LNotEqual (Local0, \PWRS)) {
        		Store (Local0, \PWRS)
        		Notify (\_SB.PCI0.LPCB.EC0.AC, 0x80)
        	}

        	/* Update LID status */
        	Store (\_SB.PCI0.LPCB.EC0.LIDF, Local0)
        	if (LNotEqual (Local0, \LIDS)) {
        		Store (Local0, \LIDS)
        		Notify (\_SB.LID0, 0x80)
        	}

        	Return(Package(){0,0})
        }

If I am not mistaken that contradicts the ACPI specification. Errors
during resume (Element 0) are not considered and neither is the power
supply.

I guess it is not deemed to be worth it to implement that. But could
such assumptions (and consequences(?)) please be documented in the code?

Or what am I missing?


Thanks,

Paul


[1] http://review.coreboot.org/#/c/2444/
[2] http://www.acpi.info/spec40a.htm
[3] https://wiki.ubuntu.com/Kernel/Reference/fwts/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20130219/c950a107/attachment-0001.sig>