[coreboot-gerrit] [M] Change in coreboot[main]: ec/dell/mec5035: Add S3 suspend SMI handler

[Nicholas Sudsgaard (Code Review)]

Attention is currently required from: Angel Pons, Nicholas Chin, Paul Menzel.

Nicholas Sudsgaard has posted comments on this change by Nicholas Chin. ( https://review.coreboot.org/c/coreboot/+/82170?usp=email )

Change subject: ec/dell/mec5035: Add S3 suspend SMI handler
......................................................................

Patch Set 7:

(2 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/82170/comment/7809d6b0_9d8a7ae3?usp... :
PS7, Line 10: If a command isn't sent, the EC cuts power to the DIMMs,
            : preventing the system from resuming.
> the system immediately locks up and the screen goes dark

Maybe the system doesn't appreciate the EC unexpectedly going to sleep. 😊

This sounds similar to how my device reacts if I suddenly put the EC into a sleeping state. However, it seems to work fine when done through the ACPI `WAK`/`PTS` methods. Maybe you could try that?

File src/ec/dell/mec5035/smihandler.c:

https://review.coreboot.org/c/coreboot/+/82170/comment/2cc68b2c_2c2f771e?usp... :
PS7, Line 13: mec5035_change_wake(EC_ACPI_WAKE_LID, WAKE_OFF);
Does this need to be called every time the system goes into S3?