Aaron Durbin 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 5:
(4 comments)
https://review.coreboot.org/c/coreboot/+/42522/5/src/soc/amd/common/block/gp... File src/soc/amd/common/block/gpio_banks/gpio.c:
https://review.coreboot.org/c/coreboot/+/42522/5/src/soc/amd/common/block/gp... PS5, Line 98: __gpio_and32 There's only one user of this. Just use __gpio_update32 in the gpio_input() func?
https://review.coreboot.org/c/coreboot/+/42522/5/src/soc/amd/common/block/gp... PS5, Line 105: -1UL dirty ;)
https://review.coreboot.org/c/coreboot/+/42522/5/src/soc/amd/common/block/gp... PS5, Line 139: gpio_set(gpio_num, value); I know this is the same ordering as previous code, but should we set the value prior to enabling output to reduce glitches? It can be in a follow up, but I absolutely think we shouldn't have this weird state where output value that is not expected is driven once output enable is set.
https://review.coreboot.org/c/coreboot/+/42522/5/src/soc/amd/picasso/i2c.c File src/soc/amd/picasso/i2c.c:
https://review.coreboot.org/c/coreboot/+/42522/5/src/soc/amd/picasso/i2c.c@1... PS5, Line 196: gpio_read32(0); /* Flush posted write */ Is it worth coalescing the posted writes? Or just use gpio_write32_rb() for both?