Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32788 )
Change subject: soc/intel/common/block/gpio: Add gpio_pm_configure() function ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/#/c/32788/5/src/soc/intel/common/block/gpio/gpio... File src/soc/intel/common/block/gpio/gpio.c:
https://review.coreboot.org/#/c/32788/5/src/soc/intel/common/block/gpio/gpio... PS5, Line 73: /* no-op */ no-op means `mask` and `value` stay unitialized and registers are filled with garbage? I think there is simply no need for a weak implementation here: If gpio_pm_configure() gets called and there is no proper soc_fill_gpio_pm_ configuration(), then it's called by error.
But actually, I would prefer not to have soc_fill_gpio_pm_configuration(), see below.
https://review.coreboot.org/#/c/32788/5/src/soc/intel/common/block/gpio/gpio... PS5, Line 633: soc_fill_gpio_pm_configuration(misccfg_pm_mask, misccfg_pm_value); I don't understand why you want to call back to soc code. As Furquan mentioned, we could pass all necessary information in the call to gpio_ pm_configure():
soc common | mask, value | |------------>| |<------------|
should be much simpler than
soc common |------------>| | | |<------------| | mask, value | |------------>| | | |<------------|
unless I miss why the latter would be necessary.