Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Mohan Viswanathan, Name of user not set #1004133, Al Hirani, Avinash Alevoor, Rob Barnes, Fred Reitberger. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61462 )
Change subject: soc/amd/common/block/psp: add PSP command ......................................................................
Patch Set 5:
(4 comments)
File src/soc/amd/common/block/psp/Kconfig:
https://review.coreboot.org/c/coreboot/+/61462/comment/b4f42f98_2ed87b9f PS5, Line 34: select SOC_AMD_COMMON_BLOCK_PSP this shouldn't be selected here. since this new option depends on SOC_AMD_COMMON_BLOCK_PSP_GEN2, a dependency should be added here instead
File src/soc/amd/common/block/psp/psp.c:
https://review.coreboot.org/c/coreboot/+/61462/comment/0257bb9b_ef57821a PS5, Line 80: BOOT_STATE_INIT_ENTRY(BS_PAYLOAD_BOOT, BS_ON_ENTRY, : psp_set_spl_fuse, NULL); this should be moved to the psp_gen2 file which also avoids having to make this function visible outside of the psp_gen2 file
File src/soc/amd/common/block/psp/psp_def.h:
https://review.coreboot.org/c/coreboot/+/61462/comment/fc113136_45596763 PS5, Line 30: #define CORE_2_PSP_MSG_38_FUSE_SPL 0x00001000 at least for me this would be easier to read if it was in the BIT(x) or (1 << x) form
File src/soc/amd/common/block/psp/psp_gen2.c:
https://review.coreboot.org/c/coreboot/+/61462/comment/ecbd5c67_3742a2a5 PS5, Line 124: static u32 soc_read_c2p38(void) commented on https://review.coreboot.org/c/coreboot/+/60968/11/src/soc/amd/common/block/p... ; would be good if you could coordinate this and have one implementation here with a prototype in psp_def.h. a follow-up that does this would be fine too