Attention is currently required from: Martin Roth. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52763 )
Change subject: soc/amd/common: Add placeholder GPIO macro, PAD_UNCHANGED ......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/52763/comment/6c63d68a_21a3f2c3 PS1, Line 9: GPIOs can only be updated in gpio_configure_pads_with_override() if they : are present in the base table. If they are not there, the override : does not work. This allows them to be in the base table so that they can : be overridden without changing the existing configuration.
Why must a mainboard configure all pads in ramstage?
ramstage is the place in coreboot where all the platform initialization happens in order to prepare the platform for payload or OS usage. All the stages prior to that are primarily to be able to boot to ramstage by doing whatever is essential - verified boot, DRAM init, etc. For these cases, yes, certain GPIOs might have to be configured. But, ramstage is the point where we are preparing the platform to jump to OS/payload and hence all subsystems are initialized,resources are allocated, etc. Hence, my comment that all pads must be initialized in ramstage.
I understand that we have also been using early stages to drive GPIOs certain way to meet power sequencing requirements and hence some GPIOs might be configured early on before ramstage. My point about always configuring the pads in ramstage is to avoid the same issue that you mentioned about adding in one place and forgetting ramstage. If all pads are always added to ramstage, we do not need to worry about whether the pad is configured in early stage or not, or if it is missed in ramstage whether that was an intentional or just a missed configuration. I think it keeps things simpler for the author as well as the reviewer.
If that's a bad plan, and we really want to update every gpio pad in ramstage, whether they've already been set or not I can do that, but I honestly believe that it will lead to bugs down the line.
What kind of bugs are you worried about? I don't think it is a bad plan, but would like to understand what bugs you are thinking about.