Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35594 )
Change subject: mb/siemens/mc_bdx1: Enable VBOOT ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35594/1/src/mainboard/siemens/mc_bd... File src/mainboard/siemens/mc_bdx1/Kconfig:
https://review.coreboot.org/c/coreboot/+/35594/1/src/mainboard/siemens/mc_bd... PS1, Line 18: select BOOT_DEVICE_SPI_FLASH_RW_NOMMAP_EARLY : select BOOT_DEVICE_SUPPORTS_WRITES
I wonder why this is needed here as SOUTHBRIDGE_INTEL_COMMON_SPI selects BOOT_DEVICE_SUPPORTS_WRITES […]
I don't see it on SoC level for now but I might miss something. These switches are needed to enable VBOOT writing its NV data into the SPI flash. For every board which does not have VBOOT enabled it makes no sense to have this featureset included in the binary (though the linker might drop the functions). If I now drop just BOOT_DEVICE_SPI_FLASH_RW_NOMMAP_EARLY in the current config, I get the error: src/lib/boot_device.c:55: undefined reference to `boot_device_rw'
in src/drivers/spi/Makefile.inc you can find the following:
$(1)-$(CONFIG_BOOT_DEVICE_SPI_FLASH_RW_NOMMAP$(2)) += boot_device_rw_nommap.c
So to be able to write NV data in romstage (which is needed for VBOOT as it start in romstage for fsp_broadwell_de) you need the switch CONFIG_BOOT_DEVICE_SPI_FLASH_RW_NOMMAP.
Or do I mix up things here?
https://review.coreboot.org/c/coreboot/+/35594/1/src/mainboard/siemens/mc_bd... PS1, Line 32: default "src/mainboard/$(CONFIG_MAINBOARD_DIR)/mc_bdx1.fmd"
You could make this depend on "if VBOOT". […]
Sounds nice, will try.