Change in coreboot[master]: mb/google/guybrush: PCIe GPIOs - enable enables, disable resets
Attention is currently required from: Martin Roth. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52115 )
Change subject: mb/google/guybrush: PCIe GPIOs - enable enables, disable resets ......................................................................
Patch Set 2:
(2 comments)
File src/mainboard/google/guybrush/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/52115/comment/f0624b14_956e526e PS1, Line 54: HIGH
Until we handle the timings, which is not a part of this patch, this is the best way to go. […]
Wouldn't you run into issues during the bringup if you are not ensuring that the timings are met? It is going to cause random behaviors on different boards and more issues to debug. Also, the bug you raised for "handle timings" (b/184598323), does not capture any of these steps that need to be performed.
https://review.coreboot.org/c/coreboot/+/52115/comment/54c3d5ce_418bee74 PS1, Line 169: /* EN_PP3300_WLAN */
The early gpio init is getting moved from bootblock to psp_verstage.
I think we should have two separate tables - one for verstage on PSP and other for bootblock. The PCIe related GPIOs don't really need to be configured in verstage. Those are required only before FSP-M runs and so doing it in bootblock should be perfectly fine. Also, ensures that we are able to update this in field if required.
-
Furquan Shaikh (Code Review)