Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45571 )
Change subject: soc/intel/alderlake: Add GPIOs for Alder Lake SOC ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45571/2/src/soc/intel/alderlake/inc... File src/soc/intel/alderlake/include/soc/gpio_soc_defs.h:
https://review.coreboot.org/c/coreboot/+/45571/2/src/soc/intel/alderlake/inc... PS2, Line 309:
Not sure if you have ADL Chapter 18 GPIO section access, i don't see any public document for this, i […]
I was asked to comment here, but I don't have the time to dive deep into documentation. So I'll try to point out some potential communi- cation issue: I think the biggest problem is that people still refer to this as "GPIO config". But it's much bigger today. What we do here is "pad configuration". It's not anymore about GPIOs only (which were the first pins that are configurable and to have muxes, but that was ten years ago), there are many more configurable pads today even if not used as GPIO.
I believe, coreboot has to configure all pads that need configu- ration even if it's just choosing one NF over another or just to enable the only NF instead of configuring some pull. If coreboot needs to set any FSP UPD to configure a pad, then we failed to do our job. The reason is simple: there is an incredible amount of configurable pads today and somebody needs to review their confi- guration. Reviewers are humans and humans fail from time to time. If the configuration is spread across many places, it is much more likely that errors sneak in, quality gets lower and time-to-market is longer. Currently, Intel seems to lower the coreboot quality with their "this is BIOS, this is not" policies, but I don't think they want to. It's an accident, accidents happen. It's our job now to fix that for the future and make it clear that all pad confi- guration has to happen in coreboot.