Attention is currently required from: Jason Glenesk, Felix Held.
Fred Reitberger has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69411 )
Change subject: mb/amd/birman/gpio: Configure birman GPIOs ......................................................................
Patch Set 3:
(6 comments)
File src/mainboard/amd/birman/gpio.c:
https://review.coreboot.org/c/coreboot/+/69411/comment/2ac5f3a3_90a742e3 PS2, Line 19: PAD_GPI(GPIO_3, PULL_UP),
this should probably be PAD_SCI instead of PAD_GPI
Done
https://review.coreboot.org/c/coreboot/+/69411/comment/48ac6f86_dbc61120 PS2, Line 31: PAD_SCI(GPIO_9, PULL_UP, EDGE_LOW),
i'm not 100% sure what this does and if it needs the SCI, but since it's a GEVENT capable pin, i'd g […]
I'm not 100% sure here either. The reference implementation isn't much help either - it looks like it might only be an input.
https://review.coreboot.org/c/coreboot/+/69411/comment/0572fb90_14656035 PS2, Line 33: PAD_GPO(GPIO_10, HIGH),
have you checked this with the reference implementation? not sure what's the correct default here
Verified against the reference implementation.
https://review.coreboot.org/c/coreboot/+/69411/comment/402c35e3_e532175b PS2, Line 37: PAD_GPO(GPIO_12, HIGH),
not sure what this one does and if this is the correct output level. […]
Verified against the reference implementation.
https://review.coreboot.org/c/coreboot/+/69411/comment/645cb41d_5b1ed4ee PS2, Line 80: PAD_GPO(GPIO_42, HIGH),
have you checked this with the reference implementation? not sure what's the correct default here
Verified against the reference implementation.
https://review.coreboot.org/c/coreboot/+/69411/comment/91a86032_2589faaf PS2, Line 91: /* SPI1_CS1_L */ : PAD_NF(GPIO_74, SPI1_CS1_L, PULL_NONE),
this is used as GPO and not as SPI1 #CS1
Ack
-
Fred Reitberger (Code Review)