Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46395 )
Change subject: soc/mediatek/mt8192: add rtc MT6359P driver ......................................................................
Patch Set 40:
(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 function.
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.
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.
https://review.coreboot.org/c/coreboot/+/46395/40/src/soc/mediatek/mt8192/rt... PS40, Line 237: static int rtc_lpen(u16 con) ditto
https://review.coreboot.org/c/coreboot/+/46395/40/src/soc/mediatek/mt8192/rt... PS40, Line 325: rtc_powerkey_init ditto