Attention is currently required from: Jason Glenesk, Raul Rangel, Martin L Roth, Marshall Dawson, Matt DeVillier, Paul Menzel, Arthur Heymans, Martin Roth, Fred Reitberger, Felix Held.
ritul guru has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/65523 )
Change subject: soc/amd/common/psp: Support PSP SMI handler ......................................................................
Patch Set 27:
(6 comments)
File src/soc/amd/common/block/psp/psp_smi.c:
https://review.coreboot.org/c/coreboot/+/65523/comment/34f6efe3_076e43f2 PS24, Line 68: static enum mbox_p2c_status find_psp_spi_flash_device_region(const struct spi_flash **flash)
the code won't compile, since both the store and id variable is used inside the function, but not av […]
Ack
https://review.coreboot.org/c/coreboot/+/65523/comment/55e4fe10_8ce835b8 PS24, Line 100: find_spi_flash_device_region
because right now this code doesn't compile when selecting this feature in a mainboard due to no fun […]
Ack
File src/soc/amd/common/block/psp/psp_smi.c:
https://review.coreboot.org/c/coreboot/+/65523/comment/5f491533_7223b561 PS26, Line 211: /* : * Assume PSP NVRAM region address 0 writes indicate : * initialization due to PSP NVRAM region being empty/erased. : * Ensure alternative SPI flash is in similar state in order : * to keep PSP NVRAM data synchronized. : */ : if (addr == 0) { : printk(BIOS_DEBUG, "PSP: Alt SPI erase NVRAM\n"); : : if (rdev_eraseat(&store, 0, region_device_sz(&store)) : != region_device_sz(&store)) { : printk(BIOS_ERR, "PSP: Failed to erase Alt SPI NVRAM\n"); : ret = MBOX_PSP_COMMAND_PROCESS_ERROR; : goto out; : } : }
this doesn't look like the behavior i'd assume the PSP would expect. […]
It is based on chip select, operation happens only on one spirom.
https://review.coreboot.org/c/coreboot/+/65523/comment/53781d67_7e7e35ca PS26, Line 280:
in the CONFIG(SOC_AMD_COMMON_BLOCK_SPI_ALT) case only the pages in the secondary spi flash on SPI_CS […]
yes, based on chip select.
File src/soc/amd/common/block/spi/Kconfig:
https://review.coreboot.org/c/coreboot/+/65523/comment/7e3b4213_a12c0c4a PS23, Line 104: SOC_AMD_COMMON_BLOCK_SPI_ALT
this looks to me to be a feature where the system has two separate spi flash chips on the two chip s […]
This is when there is alternative SPIROM (dual SPIROM), which contains a backup of original SPI or any additional write to SPIROM will happen only in ALT SPIROM(based on chip select). This is independent SPIROM implementation, so should not rely on other configs.
File src/soc/amd/common/block/spi/Kconfig:
https://review.coreboot.org/c/coreboot/+/65523/comment/f43e8098_bb8220a0 PS26, Line 108: a SPI flash
should this be "a second SPI flash"? i'm not sure if i understand correctly what this does exactly, […]
Ack