Yuchen Huang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46395 )
Change subject: soc/mediatek/mt8192: add rtc MT6359P driver ......................................................................
Patch Set 41:
(5 comments)
https://review.coreboot.org/c/coreboot/+/46395/40/src/soc/mediatek/mt8192/rt... File src/soc/mediatek/mt8192/rtc.c:
https://review.coreboot.org/c/coreboot/+/46395/40/src/soc/mediatek/mt8192/rt... PS40, Line 88: static u16 rtc_get_frequency_meter
what about make this function into common function and extract FQMTR enabling part to the SOC-depend […]
Designer provided programming guide of frequency meter flow according to different PMIC. The shift and name of register bit may be quiet different, it is not recommended to use this part as a common part. In future PMICs, this one may be more different
https://review.coreboot.org/c/coreboot/+/46395/40/src/soc/mediatek/mt8192/rt... PS40, Line 165: static u16 rtc_eosc_cali(void)
I think this function can be shared with 83.
Done
https://review.coreboot.org/c/coreboot/+/46395/40/src/soc/mediatek/mt8192/rt... PS40, Line 221: void rtc_osc_init(void)
I think this function can be shared with 83.
Done
https://review.coreboot.org/c/coreboot/+/46395/40/src/soc/mediatek/mt8192/rt... PS40, Line 237: static int rtc_lpen(u16 con)
ditto
Done
https://review.coreboot.org/c/coreboot/+/46395/40/src/soc/mediatek/mt8192/rt... PS40, Line 325: rtc_powerkey_init
ditto
Done