Martin Roth 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)
Will push an update shortly.
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?
Done
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
Removed those in favor of these.
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?
Done
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
Done
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
Done
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?
Done