Julius Werner 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 1:
(8 comments)
Jenkins says you forgot src/drivers/intel/fsp1_1/romstage.c
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
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?
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.
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
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
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
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
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)