Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44006 )
Change subject: mrc_cache: mrc_cache fetch functions to support non-x86 platforms ......................................................................
Patch Set 8:
(6 comments)
https://review.coreboot.org/c/coreboot/+/44006/7/src/drivers/intel/fsp1_1/ro... File src/drivers/intel/fsp1_1/romstage.c:
https://review.coreboot.org/c/coreboot/+/44006/7/src/drivers/intel/fsp1_1/ro... PS7, Line 53: (CONFIG(CACHE_MRC_SETTINGS)
But, I mean, we can't just leave it like this, right? That's just wrong. […]
Ok, this is what Angel was trying to tell me earlier but I didn't understand. Sorry about that. Fixed.
https://review.coreboot.org/c/coreboot/+/44006/7/src/drivers/intel/fsp1_1/ro... PS7, Line 292: __weak void *mrc_cache_current_mmap_leak(int type, uint32_t version,
I... am very confused why this is here. […]
Are we really not using this? I just changed it to make it consistent with the new definition, but I figure that it was there for a reason...
https://review.coreboot.org/c/coreboot/+/44006/7/src/northbridge/intel/ironl... File src/northbridge/intel/ironlake/raminit.c:
https://review.coreboot.org/c/coreboot/+/44006/7/src/northbridge/intel/ironl... PS7, Line 1626: &mrc_size
Are you not allowed to pass NULL here if you don't need it? We should write the function so it's leg […]
Done
https://review.coreboot.org/c/coreboot/+/44006/7/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/raminit.c:
https://review.coreboot.org/c/coreboot/+/44006/7/src/northbridge/intel/sandy... PS7, Line 278:
This is missing the size check?
Done
https://review.coreboot.org/c/coreboot/+/44006/7/src/soc/amd/common/block/s3... File src/soc/amd/common/block/s3/s3_resume.c:
https://review.coreboot.org/c/coreboot/+/44006/7/src/soc/amd/common/block/s3... PS7, Line 36: size = mrc_size;
Just pass `size` in directly?
Done
https://review.coreboot.org/c/coreboot/+/44006/7/src/soc/intel/apollolake/ro... File src/soc/intel/apollolake/romstage.c:
https://review.coreboot.org/c/coreboot/+/44006/7/src/soc/intel/apollolake/ro... PS7, Line 319: assert(CONFIG(BOOT_DEVICE_MEMORY_MAPPED));
There was never a reason for these assert()s to be guarded, I'm not sure why a lot of this code is o […]
Done