Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47800 )
Change subject: WIP: soc/mediatek/mt8192: add i2c driver support ......................................................................
Patch Set 3:
(5 comments)
https://review.coreboot.org/c/coreboot/+/47800/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47800/3//COMMIT_MSG@10 PS3, Line 10: Please note the datasheet name und revision. What is different from the already supported devices, so common code cannot be used?
https://review.coreboot.org/c/coreboot/+/47800/3/src/soc/mediatek/mt8192/i2c... File src/soc/mediatek/mt8192/i2c.c:
https://review.coreboot.org/c/coreboot/+/47800/3/src/soc/mediatek/mt8192/i2c... PS3, Line 145: static void mtk_i2c_speed_init(uint8_t bus) Why can’t this be common code?
https://review.coreboot.org/c/coreboot/+/47800/3/src/soc/mediatek/mt8192/i2c... PS3, Line 156: (400 * KHz * sample_div * 2) * clock_div); Should fit into 96 characters.
https://review.coreboot.org/c/coreboot/+/47800/3/src/soc/mediatek/mt8192/i2c... PS3, Line 161: Timing timing
https://review.coreboot.org/c/coreboot/+/47800/3/src/soc/mediatek/mt8192/i2c... PS3, Line 169: clock_div - 1); Fits into one line.