Damien Zammit <dam...@zamaudio.com> writes: > + /* Enable ACPI */ > + outb(acpi_en, smi_cmd); > + for (i = 0; i < 300; i++) > + { > + if ( (inw(pm1a_ctl) & SCI_EN) == SCI_EN) > + break; > + }
Perhaps this code should first check whether the SCI_EN bit is already set, as described in ACPI 5.0 section 4.8.2.5 "Legacy/ACPI Select and the SCI Interrupt", and skip the outb if so. Section 5.2.9 "Fixed ACPI Description Table (FADT)" says the SMI_CMD field of FADT can be zero. > + /* try sleep state 5 first */ > + outw(SLP_TYP5 | SLP_EN, pm1a_ctl); > + > + /* if we reach here then above did not work */ > + outw(SLP_TYP0 | SLP_EN, pm1a_ctl); More about these below. > +#define SLP_TYP0 (0x0 << 10) > +#define SLP_TYP5 (0x5 << 10) This hardcodes QEMU-specific values. ACPI 5.0 section 7.3.4 says the values for PM1a_CNT.SLP_TYP must be read from the \S0-\S5 objects. The DSDT of my laptop has: Name (_S5, Package (0x04) // _S5_: S5 System State { 0x07, Zero, Zero, Zero }) so PM1a_CNT.SLP_TYP on this system should be 7, not 5. However, implementing this properly would require an AML interpreter; the DSDT of my laptop also has: If (LEqual (S3DS, One)) { Name (_S3, Package (0x04) // _S3_: S3 System State { 0x05, Zero, Zero, Zero }) } and defines S3DS as a field in an OperationRegion, so OSPM would have to understand those concepts in order to know whether \_S3 exists. I assume a similar construct is possible for \_S5. Maybe it's good enough if S5 works on QEMU. In that case though, I would like to see a comment warning about this limitation.