Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32460 )
Change subject: mediatek/mt8183: Add SPI GPIO driving setting ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/#/c/32460/2/src/soc/mediatek/mt8183/gpio.c File src/soc/mediatek/mt8183/gpio.c:
https://review.coreboot.org/#/c/32460/2/src/soc/mediatek/mt8183/gpio.c@140 PS2, Line 140: 0x4 Where does this 0x4 come from? Is this a drive strength? Or a bit field? A comment about what exactly this does and why would be nice. (If it's a drive strength, are we sure 0x4 will always be the right value for all boards, or should this be passed in from mainboard code?)
https://review.coreboot.org/#/c/32460/2/src/soc/mediatek/mt8183/mt8183.c File src/soc/mediatek/mt8183/mt8183.c:
https://review.coreboot.org/#/c/32460/2/src/soc/mediatek/mt8183/mt8183.c@23 PS2, Line 23: gpio_set_spi_driving(); If this is required for SPI flash access, don't we need to set it in the bootblock? This function runs in verstage, we've already read stuff from SPI flash by this point.