Change in coreboot[master]: fsp_broadwell_de: Enable early write access to the SPI flash
Werner Zeh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35598 )
Change subject: fsp_broadwell_de: Enable early write access to the SPI flash ......................................................................
fsp_broadwell_de: Enable early write access to the SPI flash
If VBOOT is used on a mainboard based on fsp_broadwell_de then VBOOT needs to be able to write to its NV data which may be stored on the SPI flash. Enable write access to the SPI flash on SoC level. If the mainboard does not use VBOOT the linker will drop the extra code. The benefit is that this code is at least compiled and therefore build tested with fsp_broadwell_de.
Change-Id: I90a2d30f5749c75df2b286dce6779f10dde62632 Signed-off-by: Werner Zeh werner.zeh@siemens.com --- M src/soc/intel/fsp_broadwell_de/Kconfig 1 file changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/35598/1
diff --git a/src/soc/intel/fsp_broadwell_de/Kconfig b/src/soc/intel/fsp_broadwell_de/Kconfig index 9c91d7c..94eff07 100644 --- a/src/soc/intel/fsp_broadwell_de/Kconfig +++ b/src/soc/intel/fsp_broadwell_de/Kconfig @@ -29,6 +29,8 @@ select SOC_INTEL_COMMON select SOC_INTEL_COMMON_BLOCK select SOC_INTEL_COMMON_BLOCK_IMC + select BOOT_DEVICE_SUPPORTS_WRITES + select BOOT_DEVICE_SPI_FLASH_RW_NOMMAP_EARLY
config VBOOT select VBOOT_STARTS_IN_ROMSTAGE
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35598 )
Change subject: fsp_broadwell_de: Enable early write access to the SPI flash ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/35598/1/src/soc/intel/fsp_broadwell... File src/soc/intel/fsp_broadwell_de/Kconfig:
https://review.coreboot.org/c/coreboot/+/35598/1/src/soc/intel/fsp_broadwell... PS1, Line 33: BOOT_DEVICE_SPI_FLASH_RW_NOMMAP_EARLY if BOOT_DEVICE_SPI_FLASH. This is y unless BOOT_DEVICE_NOT_SPI_FLASH is selected
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35598 )
Change subject: fsp_broadwell_de: Enable early write access to the SPI flash ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35598/1/src/soc/intel/fsp_broadwell... File src/soc/intel/fsp_broadwell_de/Kconfig:
https://review.coreboot.org/c/coreboot/+/35598/1/src/soc/intel/fsp_broadwell... PS1, Line 33: BOOT_DEVICE_SPI_FLASH_RW_NOMMAP_EARLY
if BOOT_DEVICE_SPI_FLASH. […]
Is there a chance that fsp_broadwell_de will not use a SPI flash as boot device?
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35598 )
Change subject: fsp_broadwell_de: Enable early write access to the SPI flash ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35598/1/src/soc/intel/fsp_broadwell... File src/soc/intel/fsp_broadwell_de/Kconfig:
https://review.coreboot.org/c/coreboot/+/35598/1/src/soc/intel/fsp_broadwell... PS1, Line 33: BOOT_DEVICE_SPI_FLASH_RW_NOMMAP_EARLY
Is there a chance that fsp_broadwell_de will not use a SPI flash as boot device?
And if we would follow this path then we would need to add this to select SOUTHBRIDGE_INTEL_COMMON_SPI a few lines above as well...and not just for fsp_broadwell_de.
David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35598 )
Change subject: fsp_broadwell_de: Enable early write access to the SPI flash ......................................................................
Patch Set 1: Code-Review+2
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35598 )
Change subject: fsp_broadwell_de: Enable early write access to the SPI flash ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35598/1/src/soc/intel/fsp_broadwell... File src/soc/intel/fsp_broadwell_de/Kconfig:
https://review.coreboot.org/c/coreboot/+/35598/1/src/soc/intel/fsp_broadwell... PS1, Line 33: BOOT_DEVICE_SPI_FLASH_RW_NOMMAP_EARLY
And if we would follow this path then we would need to add this to […]
Ack
Andrey Petrov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35598 )
Change subject: fsp_broadwell_de: Enable early write access to the SPI flash ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35598/1/src/soc/intel/fsp_broadwell... File src/soc/intel/fsp_broadwell_de/Kconfig:
https://review.coreboot.org/c/coreboot/+/35598/1/src/soc/intel/fsp_broadwell... PS1, Line 32: select BOOT_DEVICE_SUPPORTS_WRITES shouldn't this depend on VBOOT?
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35598 )
Change subject: fsp_broadwell_de: Enable early write access to the SPI flash ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/35598/1/src/soc/intel/fsp_broadwell... File src/soc/intel/fsp_broadwell_de/Kconfig:
https://review.coreboot.org/c/coreboot/+/35598/1/src/soc/intel/fsp_broadwell... PS1, Line 32: select BOOT_DEVICE_SUPPORTS_WRITES
shouldn't this depend on VBOOT?
It's the opposite. VBOOT_VBNV_FLASH depends on BOOT_DEVICE_SUPPORTS_WRITES.
Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35598 )
Change subject: fsp_broadwell_de: Enable early write access to the SPI flash ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35598/1/src/soc/intel/fsp_broadwell... File src/soc/intel/fsp_broadwell_de/Kconfig:
https://review.coreboot.org/c/coreboot/+/35598/1/src/soc/intel/fsp_broadwell... PS1, Line 32: select BOOT_DEVICE_SUPPORTS_WRITES
It's the opposite. VBOOT_VBNV_FLASH depends on BOOT_DEVICE_SUPPORTS_WRITES.
In the first run I had these switches in mainboards Kconfig. Arthur mentioned that it would be nice to have them on SoC level. This way the features will be compiled for any fsp_broadwell_de based mainboard though the linker will drop the code. One could add here a
if VBOOT
but then Arthurs goal (compile the code for mainboards even without VBOOT enabled) would be missed.
Werner Zeh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35598 )
Change subject: fsp_broadwell_de: Enable early write access to the SPI flash ......................................................................
fsp_broadwell_de: Enable early write access to the SPI flash
If VBOOT is used on a mainboard based on fsp_broadwell_de then VBOOT needs to be able to write to its NV data which may be stored on the SPI flash. Enable write access to the SPI flash on SoC level. If the mainboard does not use VBOOT the linker will drop the extra code. The benefit is that this code is at least compiled and therefore build tested with fsp_broadwell_de.
Change-Id: I90a2d30f5749c75df2b286dce6779f10dde62632 Signed-off-by: Werner Zeh werner.zeh@siemens.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/35598 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: David Hendricks david.hendricks@gmail.com Reviewed-by: Arthur Heymans arthur@aheymans.xyz --- M src/soc/intel/fsp_broadwell_de/Kconfig 1 file changed, 2 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified David Hendricks: Looks good to me, approved Arthur Heymans: Looks good to me, approved
diff --git a/src/soc/intel/fsp_broadwell_de/Kconfig b/src/soc/intel/fsp_broadwell_de/Kconfig index 9c91d7c..94eff07 100644 --- a/src/soc/intel/fsp_broadwell_de/Kconfig +++ b/src/soc/intel/fsp_broadwell_de/Kconfig @@ -29,6 +29,8 @@ select SOC_INTEL_COMMON select SOC_INTEL_COMMON_BLOCK select SOC_INTEL_COMMON_BLOCK_IMC + select BOOT_DEVICE_SUPPORTS_WRITES + select BOOT_DEVICE_SPI_FLASH_RW_NOMMAP_EARLY
config VBOOT select VBOOT_STARTS_IN_ROMSTAGE
-
Andrey Petrov (Code Review) -
Arthur Heymans (Code Review)
-
David Hendricks (Code Review)
-
Werner Zeh (Code Review)