Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31490 )
Change subject: soc/amd/common: Refactor S3 helpers ......................................................................
Patch Set 5:
(1 comment)
Does Stoney currently update the S3DataBlock in SPI on every cold boot? I remember discussion before that MRC cache fastboot is not supported.
Maybe it would be weird/risky to resume from S3 with a different set of parameters compared to what the cold boot used though. Nevertheless, those who have no need for S3 might prefer the cold boots with SPI part write-protected, skipping AMD_INIT_RTB.
https://review.coreboot.org/#/c/31490/5/src/soc/amd/common/block/pi/agesawra... File src/soc/amd/common/block/pi/agesawrapper.c:
https://review.coreboot.org/#/c/31490/5/src/soc/amd/common/block/pi/agesawra... PS5, Line 315: Status = OemS3Save(&RtbParams->S3DataBlock);
I believe most codes consider 0 as success, because it leaves a wide range for different errors. […]
Right, OemS3Save() should only be called if amd_dispatch() does not return errors so we know S3DataBlock is valid
In an odd case where OemS3Save fails (unable to write SPI) it would be preferred to runtime disable S3 feature in ACPI. But we don't have the necessary logic ready for that (has been on my TODO for long time).