Attention is currently required from: Arthur Heymans, Felix Held, Fred Reitberger, Jason Glenesk, Marshall Dawson, Martin L Roth, Matt DeVillier, Paul Menzel, Raul Rangel, ritul guru.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/65523?usp=email )
Change subject: soc/amd/common/psp: Support PSP SMI handler ......................................................................
Patch Set 31:
(7 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/65523/comment/1b38cd4c_f044a9d4 : PS31, Line 16: See AMD document 55570 revision 3.16 for additional details. Is this public or NDA? Please mention this both here and below.
File src/soc/amd/common/block/psp/Kconfig:
https://review.coreboot.org/c/coreboot/+/65523/comment/e5442879_baf0fa17 : PS31, Line 23: default n This can probably be left out. By adding it, the default gets set, and nothing loaded after this can change it. If this line is not included, other Kconfig files loaded afterwards have the opportunity to set the default. If nothing sets it, the default value is 'n'.
Typically, if you want a default of 'n' it's best just to leave out the default line.
File src/soc/amd/common/block/psp/psp_def.h:
https://review.coreboot.org/c/coreboot/+/65523/comment/1ce20564_ca8ff616 : PS31, Line 32: Nit: Add a couple more tabs?
File src/soc/amd/common/block/psp/psp_gen2.c:
https://review.coreboot.org/c/coreboot/+/65523/comment/495fcd38_104025a7 : PS31, Line 119: static bool is_valid_psp_spi_id(u64 id) Put these in a separate file that's only pulled in when we're using PSP SPI code?
File src/soc/amd/common/block/psp/psp_p2c.c:
https://review.coreboot.org/c/coreboot/+/65523/comment/f29d84b0_9d3b4b3d : PS31, Line 2: #include <device/mmio.h> Nit: add an additional line under the SPDX header?
File src/soc/amd/common/block/spi/Kconfig:
https://review.coreboot.org/c/coreboot/+/65523/comment/d9309e5e_b0430e22 : PS23, Line 104: SOC_AMD_COMMON_BLOCK_SPI_ALT
This is when there is alternative SPIROM (dual SPIROM), which contains a backup of original SPI or a […]
So this isn't SMM specific? Can it go into a separate commit?
File src/soc/amd/stoneyridge/include/soc/smi.h:
https://review.coreboot.org/c/coreboot/+/65523/comment/fb44a4b9_4d6bc038 : PS31, Line 74: Align