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 2:
(8 comments)
Patch Set 1:
(8 comments)
Jenkins says you forgot src/drivers/intel/fsp1_1/romstage.c
Jenkins is always right.
I'm temporarily using Jenkins for building all the boards to make sure that I didn't miss anything else, so please hold up on reviewing this change for now...
https://review.coreboot.org/c/coreboot/+/44006/1/src/drivers/intel/fsp2_0/me... File src/drivers/intel/fsp2_0/memory_init.c:
https://review.coreboot.org/c/coreboot/+/44006/1/src/drivers/intel/fsp2_0/me... PS1, Line 123:
nit: double blank line
Done
https://review.coreboot.org/c/coreboot/+/44006/1/src/drivers/intel/fsp2_0/me... PS1, Line 133: mrc_size);
nit: does this fit one line now?
Done
https://review.coreboot.org/c/coreboot/+/44006/1/src/drivers/mrc_cache/mrc_c... File src/drivers/mrc_cache/mrc_cache.c:
https://review.coreboot.org/c/coreboot/+/44006/1/src/drivers/mrc_cache/mrc_c... PS1, Line 299:
This looks like it's trying to align to the parenthesis with spaces, but it's off the mark.
Done
https://review.coreboot.org/c/coreboot/+/44006/1/src/drivers/mrc_cache/mrc_c... PS1, Line 315: buffer_size
This must be data_size, not buffer_size
Done
https://review.coreboot.org/c/coreboot/+/44006/1/src/drivers/mrc_cache/mrc_c... PS1, Line 335: data = rdev_mmap(&rdev, 0, region_device_size);
nit: rdev_mmap_full(&rdev) is a shorthand for this
Done
https://review.coreboot.org/c/coreboot/+/44006/1/src/drivers/mrc_cache/mrc_c... PS1, Line 342: *data_size
This also needs to be region_device_size
Done
https://review.coreboot.org/c/coreboot/+/44006/1/src/include/mrc_cache.h File src/include/mrc_cache.h:
https://review.coreboot.org/c/coreboot/+/44006/1/src/include/mrc_cache.h@37 PS1, Line 37:
nit: missing period
Done
https://review.coreboot.org/c/coreboot/+/44006/1/src/include/mrc_cache.h@38 PS1, Line 38: unmmap
nit: "unmap" (or, if you want to reference the exact name of the function, munmap)
Done