Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38593 )
Change subject: sc7180: GPIO: Add I2S configuration for sc7180 ......................................................................
Patch Set 16:
(3 comments)
https://review.coreboot.org/c/coreboot/+/38593/16/src/mainboard/google/trogd... File src/mainboard/google/trogdor/board.h:
https://review.coreboot.org/c/coreboot/+/38593/16/src/mainboard/google/trogd... PS16, Line 26: board_id() < 1 Actually, let's please change this into a CONFIG(TROGDOR_REV0) check instead. The problem is that we now also have Lazor boards and they are starting board IDs at 0 again, but their schematics are based off Trogdor rev1.
https://review.coreboot.org/c/coreboot/+/38593/16/src/mainboard/google/trogd... File src/mainboard/google/trogdor/chromeos.c:
https://review.coreboot.org/c/coreboot/+/38593/16/src/mainboard/google/trogd... PS16, Line 32: gpio_output(GPIO_AMP_ENABLE, 1); Can we just merge the line from CB:40534 into this patch as well?
https://review.coreboot.org/c/coreboot/+/38593/16/src/mainboard/google/trogd... File src/mainboard/google/trogdor/mainboard.c:
https://review.coreboot.org/c/coreboot/+/38593/16/src/mainboard/google/trogd... PS16, Line 37: GPIO_PULL_UP Can you please double-check that this is correct? Why would we have a pull-up on an output pin? According to my (limited) electrical understanding, that's basically never the right thing to do.