Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/27276 )
Change subject: soc/amd/common: Remove redundant ACPI S3 test ......................................................................
Patch Set 4: Code-Review+2
(2 comments)
https://review.coreboot.org/#/c/27276/4/src/soc/amd/stoneyridge/Kconfig File src/soc/amd/stoneyridge/Kconfig:
https://review.coreboot.org/#/c/27276/4/src/soc/amd/stoneyridge/Kconfig@a53 PS4, Line 53:
Why remove? True, if no S3 enabled than the linker will not add these files. […]
I'm OK with this change. No change in compile time (see the makefile).
https://review.coreboot.org/#/c/27276/4/src/soc/amd/stoneyridge/chip.c File src/soc/amd/stoneyridge/chip.c:
https://review.coreboot.org/#/c/27276/4/src/soc/amd/stoneyridge/chip.c@158 PS4, Line 158: acpi_s3_resume_allowed()
Not sure why this more complicated test. If it's on a resume path, then certainly S3 is allowed.
Richard, look at acpi_s3_resume_allowed(). This will compile to if (0 && romstage_handoff_is_()) which avoids the 2nd check with all its cbmem stuff when resume is unsupported.
-
Marshall Dawson (Code Review)