Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42522 )
Change subject: soc/amd/common: Drop ACPIMMIO GPIO bank separation ......................................................................
Patch Set 8:
(3 comments)
https://review.coreboot.org/c/coreboot/+/42522/6/src/soc/amd/common/block/gp... File src/soc/amd/common/block/gpio_banks/gpio.c:
https://review.coreboot.org/c/coreboot/+/42522/6/src/soc/amd/common/block/gp... PS6, Line 88: static void __gpio_update32(gpio_t gpio_num, uint32_t mask, uint32_t or) : { : uint32_t reg; : : reg = gpio_read32(gpio_num); : reg &= mask; : reg |= or; : gpio_write32(gpio_num, reg); : }
In a different patch I had replaced all the invocations with mem_read_write32: https://chromium-revi […]
Ack
https://review.coreboot.org/c/coreboot/+/42522/6/src/soc/amd/common/block/gp... PS6, Line 98: __
why the __? See https://stackoverflow.com/questions/25090635/use-and-in-c-programs […]
Propose another prefix that gets us out of polluting global gpio_. Maybe I have understood incorrectly that __xxx would always be a safe name choice for user implementations, and never defined by compiler or standard includes.
https://review.coreboot.org/c/coreboot/+/42522/6/src/soc/amd/common/block/in... File src/soc/amd/common/block/include/amdblocks/acpimmio.h:
https://review.coreboot.org/c/coreboot/+/42522/6/src/soc/amd/common/block/in... PS6, Line 372: static inline void gpio_write32_rb(uint8_t gpio_num, uint32_t value) : { : write32(gpio_ctrl_ptr(gpio_num), value); : read32(gpio_ctrl_ptr(gpio_num)); : } :
I don't think it's worth while to pollute the public API with this. […]
I agree, see CB:42825. There is also an open question in the follow-ups if this MMIO region now has posted writes disabled.