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 12:
(4 comments)
https://review.coreboot.org/c/coreboot/+/34188/11//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34188/11//COMMIT_MSG@8 PS11, Line 8:
Can you please also add details that this CL adds 2 new variant APIs: […]
Referring to other practice, I split CLs for adding each variant APIs above.
https://review.coreboot.org/c/coreboot/+/34188/11/src/mainboard/google/octop... File src/mainboard/google/octopus/smihandler.c:
https://review.coreboot.org/c/coreboot/+/34188/11/src/mainboard/google/octop... PS11, Line 22: #include <gpio.h>
Not required?
Done
https://review.coreboot.org/c/coreboot/+/34188/11/src/mainboard/google/octop... File src/mainboard/google/octopus/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/34188/11/src/mainboard/google/octop... PS11, Line 346: UP_20K
Why is PU required here when this pad is being actively driven?
Just copy the original GPIO configuration from line 231 in gpio_table as the defautl stage.
https://review.coreboot.org/c/coreboot/+/34188/11/src/mainboard/google/octop... File src/mainboard/google/octopus/variants/garg/gpio.c:
https://review.coreboot.org/c/coreboot/+/34188/11/src/mainboard/google/octop... PS11, Line 110: lte_early_override_table
I believe you already confirmed that it is okay to configure the GPIOs this way on non-LTE SKUs?
Yes, as my previous comment to Karthik. I confirmed with kuten@ about it is fine to configure GPIOs for LTE SKU in non-LTE SKUs.