Attention is currently required from: Raul Rangel, Andrey Pronin, Kangheui Won, Julius Werner, Karthik Ramasubramanian. Rob Barnes 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:
(5 comments)
File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/58870/comment/a7ef8e0d_4c39f748 PS5, Line 645: config RESUME_PATH_SAME_AS_BOOT
+1. […]
I'm checking CHROMEOS_DISABLE_PLATFORM_HIERARCHY_ON_RESUME and it depends on RESUME_PATH_SAME_AS_BOOT. As pointed out above, resume in the S0i3 context is fundamentally different than the S3 resume. So how about CHROMEOS_DISABLE_PLATFORM_HIERARCHY_ON_RESUME and RESUME_PATH_SAME_AS_BOOT get renamed to CHROMEOS_DISABLE_PLATFORM_HIERARCHY_ON_S3_RESUME and S3_RESUME_PATH_SAME_AS_BOOT and I remove the checks on these configs from this CL. I think this will mitigate most confusion.
File src/mainboard/google/guybrush/Kconfig:
https://review.coreboot.org/c/coreboot/+/58870/comment/73501b35_95ccd9f2 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 […]
Not if HAVE_ACPI_RESUME is false. In any case, I plan to remove the check for this config from this CL.
File src/mainboard/google/guybrush/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/58870/comment/8473173b_6a580f70 PS7, Line 381: PAD_INT(GPIO_3, PULL_NONE, EDGE_LOW, STATUS_DELIVERY),
This doesn't seem to match the normal GPIO table?
Good catch. The GPIO for GSC_SOC_INT_L moved from GPIO_3 to GPIO_85 in the latest schematic. So the board version will need to be checked before configuring this GPIO.
File src/soc/amd/common/psp_verstage/psp_verstage.c:
https://review.coreboot.org/c/coreboot/+/58870/comment/f933c642_c3ac1280 PS5, Line 230: verstage_mainboard_s0i3_init();
I think some additional GPIOs(eg. WLAN_AUX_REST_L) are re-initialized in ramstage. […]
Yes it can hurt. WLAN_AUX_RESE is asserted in verstage_mainboard_early_init and deasserted in ramstage. So following the same initialization path here will result in a bad state for this GPIO. There may be other examples.
I could move eSPI and TPM gpios out of the early_gpio_table and add verstage_mainboard_tpm_init and verstage_mainboard_espi_init to both the normal verstage boot flow. This removes some of the duplication.
File src/vendorcode/google/chromeos/Kconfig:
https://review.coreboot.org/c/coreboot/+/58870/comment/3c70396c_0d4660c5 PS3, Line 47: depends on TPM2 && RESUME_PATH_SAME_AS_BOOT
BTW, didn't see this discussion earlier, but as I understand it you're not running through ramstage […]
OK. Let's not depend on RESUME_PATH_SAME_AS_BOOT for this CL.