Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46878 )
Change subject: soc/mediatek/mt8192: add clkbuf and srclken_rc MT6359P driver ......................................................................
Patch Set 41:
(10 comments)
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/cl... File src/soc/mediatek/mt8192/clkbuf.c:
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/cl... PS41, Line 23: static void buf_read(u32 addr, u32 *rdata) can we reuse rtc_read/rtc_write ?
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/cl... PS41, Line 66: pmic_cw00 = buf_read_field(PMIC_RG_DCXO_CW00, 0xffff, 0); use PMIC_REG_MASK and PMIC_REG_SHIFT ?
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/cl... PS41, Line 77: buf_info("DCXO_CW00/09/12/13/15/19=0x%x %x %x %x %x %x\n", %#x
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/cl... PS41, Line 95: buf_write_field(PMIC_TOP_TMA_KEY, 0x9CA6, 0xFFFF, 0); please define this number
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/in... File src/soc/mediatek/mt8192/include/soc/clkbuf.h:
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/in... PS41, Line 17: remove extra blank line
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/in... File src/soc/mediatek/mt8192/include/soc/pmif.h:
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/in... PS41, Line 176: #define PMIFSPI_INF_EN_SRCLKEN_RC_HW_MSK 0x1 use tab for indent
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/in... File src/soc/mediatek/mt8192/include/soc/rtc.h:
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/in... PS41, Line 151: #if 0 just remove
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/pm... File src/soc/mediatek/mt8192/pmif.c:
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/pm... PS41, Line 12: #include <soc/srclken_rc.h> is this header necessary ?
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/rt... File src/soc/mediatek/mt8192/rtc.c:
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/rt... PS41, Line 5: #include <soc/clkbuf.h> remove ?
https://review.coreboot.org/c/coreboot/+/46878/41/src/soc/mediatek/mt8192/rt... PS41, Line 316: #if 0 just remove the code if dcox is not used.