Attention is currently required from: Raul Rangel, Andrey Pronin, Kangheui Won, Rob Barnes, Karthik Ramasubramanian. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58870 )
Change subject: soc/amd/psp_verstage: Init TPM on S0i3 resume ......................................................................
Patch Set 7:
(4 comments)
File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/58870/comment/ab4c4a6e_8ff7cabf PS5, Line 645: config RESUME_PATH_SAME_AS_BOOT I'm not sure you should really have this set. I think this option is really a bit misnamed (and misplaced) at this point... if you look at what it controls, it really just decides whether you run through vboot verification again on resume, and that's not the case for this platform. Maybe we should move this to src/security/vboot/Kconfig and rename it accordingly (or maybe we should just get rid of it and check HAVE_ACPI_RESUME directly in the respective places, there doesn't really seem to be a good reason for the two to be separate).
File src/mainboard/google/guybrush/Kconfig:
https://review.coreboot.org/c/coreboot/+/58870/comment/ef6e2e31_77940a09 PS5, Line 12: select CHROMEOS_DISABLE_PLATFORM_HIERARCHY_ON_RESUME Unnecessary since it's default y? (Also, it seems like the effect of this won't end up mattering for you anyway...)
File src/mainboard/google/guybrush/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/58870/comment/8fd447f9_4e3873d4 PS7, Line 381: PAD_INT(GPIO_3, PULL_NONE, EDGE_LOW, STATUS_DELIVERY), This doesn't seem to match the normal GPIO table?
File src/soc/amd/common/psp_verstage/psp_verstage.c:
https://review.coreboot.org/c/coreboot/+/58870/comment/f185ea1f_4454b605 PS5, Line 230: verstage_mainboard_s0i3_init(); Is there a good reason the normal verstage_mainboard_early_init() won't work for this? I think it would be better to avoid having to duplicate functionality for this as much as possible.
-
Julius Werner (Code Review)