Marco Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34188 )
Change subject: mb/google/octopus/variants/garg: support LTE power sequence. ......................................................................
Patch Set 6:
(3 comments)
https://review.coreboot.org/c/coreboot/+/34188/6/src/mainboard/google/octopu... File src/mainboard/google/octopus/smihandler.c:
https://review.coreboot.org/c/coreboot/+/34188/6/src/mainboard/google/octopu... PS6, Line 37: gpio_get_output
Just realized that this is in the mainboard and checking for sku will not work. […]
Like codes for CNVi module, we check some values in runtime to determine whether it exists. Here, I think RESET pin is the hint for us to judge or is there any issue here?
https://review.coreboot.org/c/coreboot/+/34188/6/src/mainboard/google/octopu... File src/mainboard/google/octopus/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/34188/6/src/mainboard/google/octopu... PS6, Line 97:
I think we should not remove the configuration for GPIO_67, GPIO_117 for the benefit of non-LTE skus […]
The previous comment mentioned about after confirming with HW eng - kuten@, these pins can be remained to 1 no matter LTE or non-LTE skus in garg. As a result, the CL didn't drive them to 0 in ram stage when in non-LTE skus.
Otherwise, if we want to play it safe then I can add this logic. thought?
https://review.coreboot.org/c/coreboot/+/34188/6/src/mainboard/google/octopu... File src/mainboard/google/octopus/variants/garg/gpio.c:
https://review.coreboot.org/c/coreboot/+/34188/6/src/mainboard/google/octopu... PS6, Line 82: PAD_CFG_GPO(GPIO_161, 0, DEEP), By the way, the reason for setting 67/117 to PWROK and 161 to DEEP is for power sequence of warm boot, where we need RESET (161) to toggle but keep others the same (hight). Will add a comment for whole power sequence requirement we consider to.