Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42224 )
Change subject: soc/amd/common/blocks/gpio: Introduce gpio_get_bar ......................................................................
Patch Set 3:
(6 comments)
https://review.coreboot.org/c/coreboot/+/42224/3/src/soc/amd/common/block/gp... File src/soc/amd/common/block/gpio_banks/gpio.c:
https://review.coreboot.org/c/coreboot/+/42224/3/src/soc/amd/common/block/gp... PS3, Line 153: uint8_t gpio_t?
https://review.coreboot.org/c/coreboot/+/42224/3/src/soc/amd/common/block/in... File src/soc/amd/common/block/include/amdblocks/gpio_banks.h:
https://review.coreboot.org/c/coreboot/+/42224/3/src/soc/amd/common/block/in... PS3, Line 54: #define GPIO_OUTPUT_VALUE_SHIFT 22 : #define GPIO_OUTPUT_VALUE_MASK (1 << GPIO_OUTPUT_VALUE_SHIFT) : #define GPIO_OUTPUT_ENABLE (1 << 23) those are already defined in lines 60, 61, 63
https://review.coreboot.org/c/coreboot/+/42224/3/src/soc/amd/common/block/in... PS3, Line 57: 0 (0 << 23) to make this more consistent?
https://review.coreboot.org/c/coreboot/+/42224/3/src/soc/amd/common/block/in... PS3, Line 301: /* : * Gets the raw memory address of the control register of a particular pin. : */ nit: this comment is only 1 line long, so no need for a multi-line-style comment
https://review.coreboot.org/c/coreboot/+/42224/3/src/soc/amd/picasso/i2c.c File src/soc/amd/picasso/i2c.c:
https://review.coreboot.org/c/coreboot/+/42224/3/src/soc/amd/picasso/i2c.c@1... PS3, Line 184: [] i'd use [I2C_MASTER_DEV_COUNT] here
https://review.coreboot.org/c/coreboot/+/42224/3/src/soc/amd/stoneyridge/i2c... File src/soc/amd/stoneyridge/i2c.c:
https://review.coreboot.org/c/coreboot/+/42224/3/src/soc/amd/stoneyridge/i2c... PS3, Line 166: [] [I2C_DEVICE_COUNT] maybe?