Attention is currently required from: Raul Rangel, Jason Nien, Jon Murphy, Martin Roth, Tim Van Patten, Karthik Ramasubramanian, Mark Hasemeyer.
Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74592 )
Change subject: mb/google/skyrim: rename baseboard gpios in gpio.c ......................................................................
Patch Set 10:
(1 comment)
File src/mainboard/google/skyrim/variants/baseboard/gpio.c:
https://review.coreboot.org/c/coreboot/+/74592/comment/243a6401_4c4a6819 PS2, Line 10: SYS_RST_ODL
We moved netnames between build phases themselves - eg. GSC_SOC_INT moved from GPIO_X to GPIO_Y within guybrush phase1 and pahse2 itself. Not between guybrush and nipperkin. So phase specific overrides will have GPIO_NN still.
I actually think this is an argument *for* using the net names. We can rename them from GSC_SOC_INT to GSC_SOC_INT_PHASE1 and GSC_SOC_INT_PHASE2 or something. That makes what's going on with this very clear.
One of the nice things about using the numbers is that we can quickly glance at the table to make sure we defined all the GPIOs in the ramstage table.
I *still* feel that we need a runtime check to verify this - doing it manually is always going to be more error-prone. Doing something at runtime gives us a number of features. We can capture the time of the change, the IOMUX settings, what if anything got changed in the FSP calls, and allows us to verify that everything was set correctly. If something gets updated in bootblock, but we forget to update it in ramstage, that would be a lot more obvious in a diff between the two GPIO init logs than in the code.