Attention is currently required from: Raul Rangel, Martin Roth, Felix Held. Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57317 )
Change subject: mb/google/guybrush: Initialize WWAN GPIOs the same for PCI vs USB ......................................................................
Patch Set 3:
(3 comments)
Patchset:
PS3: With too many tables, it is hard for me to get the complete picture. Here is what I understood. Can you please confirm if it is correct?
1) In PSP verstage, WWAN is disabled and put into reset 2) In bootblock, WWAN is enabled and reset line is de-asserted. But the WWAN still is in reset since WWAN_AUX_RESET_L is still asserted. 3) The previous CL in this stack ensures that PCIe training is done in romstage when the system has PCIe WWAN. Otherwise no PCIe training. 4) In ramstage, WWAN is brought of reset because WWAN_AUX_RESET_L is de-asserted.
I believe all of this is done to meet the power sequence requirements of USB and PCIe WWAN modules. If so, we should plan to leverage the ACPI power resource to do this. Both USB and PCIe Root ports have the support to define power resources. Once defined, kernel will exercise the power sequencing as defined in power resources. Again this proposal is not for this change, but as a future improvement. Thoughts/concerns?
File src/mainboard/google/guybrush/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/57317/comment/d4e326b1_07829a04 PS3, Line 172: /* Assert all AUX reset lines */ Delayed question. But better late than never.
Why do we need to do this in PSP verstage? This is more of a question. So I dont want this comment to block the CL. Same for turning on power to WLAN and WWAN.
File src/mainboard/google/guybrush/variants/guybrush/gpio.c:
https://review.coreboot.org/c/coreboot/+/57317/comment/6ca9e431_64376673 PS3, Line 50: /* : * This code is run before the EC is available to check the board ID : * since this is needed to work on Board ID 1 and is unused on other : * versions of guybrush, just enable it. : * : * Guybrush BID>1: Unused TP27; BID==1: SD_AUX_RESET_L : */ Nice catch :)