Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30974 )
Change subject: mediatek/mt8183: Support gpio eh and rsel setting for I2C ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/#/c/30974/1/src/soc/mediatek/mt8183/gpio.c File src/soc/mediatek/mt8183/gpio.c:
https://review.coreboot.org/#/c/30974/1/src/soc/mediatek/mt8183/gpio.c@87 PS1, Line 87: MTK_I2C_PIN_SPEC(PAD_SCL5_ID, 20, 18), nit: There's no need for this macro, if you want to initialize by position you can just write
{PAD_SDA5_ID, 15, 13}
(of course, initializing by position only is hard to read, so you really should write { .id = PAD_SDA5_ID, .eh_bit = 15, .rsel_bit = 13 } explicitly on every line).
https://review.coreboot.org/#/c/30974/1/src/soc/mediatek/mt8183/gpio.c@98 PS1, Line 98: MTK_I2C_PIN_SPEC(PAD_SDA4_ID, 17, 12), What do these numbers do and where do they come from? Are they specific to the board layout? Why didn't we need this on 8173?