Attention is currently required from: Raul Rangel, Tim Van Patten, Karthik Ramasubramanian, Mark Hasemeyer.
Jon Murphy has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74107 )
Change subject: mb/google/myst: Log mainboard events to elog ......................................................................
Patch Set 5:
(3 comments)
File src/mainboard/google/myst/variants/baseboard/smihandler.c:
https://review.coreboot.org/c/coreboot/+/74107/comment/b83882cf_6b15c5ad PS5, Line 16: MAINBOARD_EC_S3_WAKE_EVENTS
Why S0IX below, and S3 here?
The prototype specifically calls out S3. All other boards are configured this way as well.
``` /* * Set wake masks according to sleep type, clear SCI and SMI masks, * and clear any pending events. */ void chromeec_smi_sleep(int slp_type, uint64_t s3_mask, uint64_t s5_mask); ```
We do set the S0IX wake events equal to S3, so for clarity we should probably update it accordingly. https://source.corp.google.com/chromeos_public/src/third_party/coreboot/src/...
https://review.coreboot.org/c/coreboot/+/74107/comment/b2ec5c3e_a059a7a1 PS5, Line 26: void elog_gsmi_cb_mainboard_log_wake_source(void)
This isn't implemented for any AMD boards. […]
Looks like it was removed and I was unaware of it https://review.coreboot.org/c/coreboot/+/67184 https://review.coreboot.org/c/coreboot/+/63280
Should be removed here as well.
https://review.coreboot.org/c/coreboot/+/74107/comment/f9b4aa5b_4a0fc571 PS5, Line 28: MAINBOARD_EC_S0IX_WAKE_EVENTS
Should this call `acpi_is_wakeup_s3()`, so it uses either `MAINBOARD_EC_S0IX_WAKE_EVENTS` or `MAINBO […]
OBE