Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47804 )
Change subject: soc/intel/elkhartlake: Fix EHL mainboard build fail errors ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47804/3/src/soc/intel/elkhartlake/i... File src/soc/intel/elkhartlake/include/soc/gpio.h:
https://review.coreboot.org/c/coreboot/+/47804/3/src/soc/intel/elkhartlake/i... PS3, Line 6: #include <soc/gpio_defs.h> : #include <intelblocks/gpio.h> From the first look it seems like soc/gpio_defs.h defines "GPIO_NUM_PAD_CFG_REGS" which is then used in intelblocks/gpio.h for struct pad_config. The other candidate is "NUM_GPI_STATUS_REGS", which is defined in soc/gpio_defs.h and used in intelblocks/gpio.h. It is kind of strange that we rely on a proper include order in a file in order to satisfy the needs in a common block path. From a first look this #includes would be better placed directly there where they are needed. Is there something that prevents us to include soc/gpio_dfefs.h directly in intelblocks/gpio.h as already done with soc/gpio.h? You could then (I haven't tried yet) include intelblocks/gpio.h directly in baseboard/gpio.h and get rid of these two lines completely. This would, in my point of view, make these inclusion dependency much more clear. As a follow up, we could clean that on other socs, too as at least jasperlake follows the same scheme.
What are your thoughs about it?