wang qii has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47392 )
Change subject: WIP: soc/mediatek/mt8192: modify i2c and dma reset flow ......................................................................
Patch Set 6:
(7 comments)
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... File src/soc/mediatek/common/i2c.c:
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... PS3, Line 13: inline
I think we don't necessarily need this to be inline.
Done
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... PS3, Line 15: struct
const struct
In order to maintain consistency with other references, const is not used in particular, and it may be possible to add another patch for uniform modification.
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... PS3, Line 16: struct
const struct
In order to maintain consistency with other references, const is not used in particular, and it may be possible to add another patch for uniform modification.
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... PS3, Line 17:
Check bus < ARRAY_SIZE(mtk_i2c_bus_controller)
mtk_i2c_bus_init has used it.
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... PS3, Line 26: (
No need for parentheses.
Done
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... PS3, Line 27: (
No need for parentheses.
Done
https://review.coreboot.org/c/coreboot/+/47392/3/src/soc/mediatek/common/i2c... PS3, Line 35: udelay(50);
Why do we need 50us delay here, but only 10us for I2C_APDMA_ASYNC?
That’s only required by the HW designers