Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47659 )
Change subject: soc/intel/common/fast_spi: Add custom boot media device ......................................................................
Patch Set 9:
(3 comments)
https://review.coreboot.org/c/coreboot/+/47659/9/src/soc/intel/common/block/... File src/soc/intel/common/block/fast_spi/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/47659/9/src/soc/intel/common/block/... PS9, Line 25: mmap_boot
mmap_boot. […]
I had named it mmap_boot.c because the file that provides the boot media device definition is named as mmap_boot.c in: src/arch/x86/mmap_boot.c and src/soc/intel/apollolake/mmap_boot.c
Would ext_bios_mmap_boot.c be a better name?
https://review.coreboot.org/c/coreboot/+/47659/9/src/soc/intel/common/block/... PS9, Line 35: check-fmap-16mib-crossing
this seems like it could be done easier in fmaptool? but it would probably need to be behind an arg […]
Yeah, I went back and forth on that a bit. Adding an option to fmaptool works, but it seems to be required for a very specific use case. Hence, I ended up keeping it here.
https://review.coreboot.org/c/coreboot/+/47659/9/src/soc/intel/common/block/... File src/soc/intel/common/block/fast_spi/mmap_boot.c:
https://review.coreboot.org/c/coreboot/+/47659/9/src/soc/intel/common/block/... PS9, Line 103: CONFIG_EXT_BIOS_WIN_BASE
could these be checked at compile time?
Yes, _Static_assert ensures that this gets checked at compile time.