Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Karthik Ramasubramanian, Mark Hasemeyer, Matt DeVillier.
Tim Van Patten has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79155?usp=email )
Change subject: soc/amd/common/psp_verstage: Make SPI ROM mapping configurable ......................................................................
Patch Set 1:
(4 comments)
File src/soc/amd/common/psp_verstage/Kconfig:
https://review.coreboot.org/c/coreboot/+/79155/comment/fadc3a86_69177da0 : PS1, Line 49: default y if SOC_AMD_CEZANNE Have you tried any earlier boards as well (e.g., Grunt and Zork)?
I'm wondering if this is exclusive to just Cezanne boards, or if other boards need it as well.
File src/soc/amd/common/psp_verstage/boot_dev.c:
https://review.coreboot.org/c/coreboot/+/79155/comment/37dfb085_f9d3f1dd : PS1, Line 40: uint32_t ret; This can be moved inside the `CONFIG` conditional block.
File src/soc/amd/common/psp_verstage/fch.c:
https://review.coreboot.org/c/coreboot/+/79155/comment/874585b8_ff779839 : PS1, Line 101: if (!CONFIG(PSP_VERSTAGE_MAP_ENTIRE_SPIROM)) : return spi.SpiBiosSmnBase; : : if (spi.SpiBiosSmnBase != 0) : if (svc_map_spi_rom(spi.SpiBiosSmnBase, CONFIG_ROM_SIZE, (void **)&addr)) : printk(BIOS_DEBUG, "Error mapping SPI ROM to address.\n"); : : return addr; Positive logic is easier to read, so I think this should be inverted:
``` if (CONFIG(PSP_VERSTAGE_MAP_ENTIRE_SPIROM)) { if (spi.SpiBiosSmnBase != 0) if (svc_map_spi_rom(spi.SpiBiosSmnBase, CONFIG_ROM_SIZE, (void **)&addr)) printk(BIOS_DEBUG, "Error mapping SPI ROM to address.\n");
return addr; } return spi.SpiBiosSmnBase; ```
File src/soc/amd/common/psp_verstage/psp_verstage.c:
https://review.coreboot.org/c/coreboot/+/79155/comment/1cf50ba1_c7b04e3a : PS1, Line 355: boot_dev_base = rdev_mmap_full(boot_device_ro()); Missing the `post_code(POSTCODE_UNMAP_SPI_ROM);` line as well:
https://review.coreboot.org/c/coreboot/+/74606/5/src/soc/amd/common/psp_vers...