Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48583 )
Change subject: soc/intel: hook up new gpio device in the soc chips ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48583/7/src/soc/intel/alderlake/chi... File src/soc/intel/alderlake/chip.c:
https://review.coreboot.org/c/coreboot/+/48583/7/src/soc/intel/alderlake/chi... PS7, Line 202: else if (dev->path.type == DEVICE_PATH_GPIO) : dev->ops = &soc_gpio_ops;
With your code the block gpio driver will get assigned to both gpio devices which is obviously wrong.
You will have to decorate the gpio device with chip soc/intel/common/block/gpio. Then `intel_gpio_enable()` gets called only for that specific device. Similarly for the other chip_gpio, it's own enable_dev will be called. Thus, there should not be any problem with the above proposal I made.
I agree on what Nico says. It's not a chip and we shouldn't fake it. I will try to move the struct.
The `struct gpio_operations` could definitely move to `common/block/`, that should be easy. Without adding another "chip", you could either make it `extern` and hook it up here, or call such a intel_gpio_enable() from here (and each soc).
Easy? :'D You have no idea how many different no-working variants I tested to get this moved to block :'D :P ... calling `intel_gpio_enabe` sounds like a good idea, though.