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 6:
(4 comments)
https://review.coreboot.org/c/coreboot/+/31490/6/src/soc/amd/common/block/pi... File src/soc/amd/common/block/pi/agesawrapper.c:
https://review.coreboot.org/c/coreboot/+/31490/6/src/soc/amd/common/block/pi... PS6, Line 318: if (OemS3Save(&RtbParams->S3DataBlock) != AGESA_SUCCESS) While it is called OemS3Save, this step saves memory training data to SPI and is not strictly about HAVE_ACPI_RESUME=y and definetly not only about stage_cache.
(Yes, PI build may be limited for us to be able to use it, we should still follow the specs.)
https://review.coreboot.org/c/coreboot/+/31490/6/src/soc/amd/common/block/s3... File src/soc/amd/common/block/s3/s3_resume.c:
https://review.coreboot.org/c/coreboot/+/31490/6/src/soc/amd/common/block/s3... PS6, Line 72: stage_cache_get_raw(STAGE_S3_DATA, &base, &size); Never reached with HAVE_ACPI_RESUME=n, ahould link just fine with empty stubs in stage_cache.h.
https://review.coreboot.org/c/coreboot/+/31490/6/src/soc/amd/common/block/s3... PS6, Line 88: if (mrc_cache_stash_data(MRC_TRAINING_DATA, DEFAULT_MRC_VERSION, This part is about SPI flash, for MRC cache. Maybe someone did poor decisions and accepted AMD not to support this with stoneyridge, but it is part of the specs regardless.
https://review.coreboot.org/c/coreboot/+/31490/6/src/soc/amd/common/block/s3... PS6, Line 94: stage_cache_add_raw(STAGE_S3_DATA, dataBlock->VolatileStorage, Empty stub function in stage_cache.h so this becomes no-op, just fine as-is.