Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44196 )
Change subject: mrc_cache: Update mrc_cache data in romstage ......................................................................
Patch Set 8:
(6 comments)
So, just updated mrc_cache.c for now. I need to think about how to do the config options properly.
https://review.coreboot.org/c/coreboot/+/44196/7/src/drivers/mrc_cache/mrc_c... File src/drivers/mrc_cache/mrc_cache.c:
https://review.coreboot.org/c/coreboot/+/44196/7/src/drivers/mrc_cache/mrc_c... PS7, Line 562: update_mrc_cache_by_type(MRC_TRAINING_DATA, md, data, size);
I don't think you're treating `type` quite right throughout this. […]
Done
https://review.coreboot.org/c/coreboot/+/44196/7/src/drivers/mrc_cache/mrc_c... PS7, Line 569: invalidate_normal_cache();
This and the protect_mrc_region() should probably just go in finalize, then.
This function has been removed.
https://review.coreboot.org/c/coreboot/+/44196/7/src/drivers/mrc_cache/mrc_c... PS7, Line 589: printk(BIOS_ERR, "MRC: No data in cbmem for '%s'.\n",
Should this really be an ERR... […]
Done
https://review.coreboot.org/c/coreboot/+/44196/7/src/drivers/mrc_cache/mrc_c... PS7, Line 605: update_mrc_cache_from_cbmem(MRC_VARIABLE_DATA);
Pretty sure this should also only happen for NV_LATE.
Done
https://review.coreboot.org/c/coreboot/+/44196/7/src/drivers/mrc_cache/mrc_c... PS7, Line 644: md->header_checksum = compute_ip_checksum(md, sizeof(*md));
Let's try to factor out some of the code duplication here. For example: […]
Done
https://review.coreboot.org/c/coreboot/+/44196/7/src/drivers/mrc_cache/mrc_c... PS7, Line 668: #if CONFIG(MRC_WRITE_NV_LATE)
Oh gotcha. Sorry, I misunderstood your previous comment.
removed the else condition for now and still using the conditional NV_LATE for now unless Furquan says otherwise.