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 7:
(5 comments)
https://review.coreboot.org/c/coreboot/+/31490/5/src/soc/amd/common/block/pi... File src/soc/amd/common/block/pi/agesawrapper.c:
https://review.coreboot.org/c/coreboot/+/31490/5/src/soc/amd/common/block/pi... PS5, Line 315: Status = OemS3Save(&RtbParams->S3DataBlock);
That would be acceptable. […]
Done
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)
Unfortunately, stoney had no way of loading the timing data back into one of the memory components o […]
The code here should replace AGESA OemS3Save() which also works for fastboot aka MRC cache with fam16kb.
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);
I believe the linker will not include it on the image if it's never called, with end result similar […]
There is no change here. My comment was for Subrata to tell that there is no need to add any if (CONFIG(HAVE_ACPI_RESUME)) condition here.
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. […]
Done
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.
Comment for Subrata.