[coreboot-gerrit] [XL] Change in coreboot[main]: soc/mediatek/mt8196: Add RTC driver

[Yidi Lin (Code Review)]

Attention is currently required from: Hung-Te Lin, Jarried Lin, Paul Menzel.

Yidi Lin has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85978?usp=email )

Change subject: soc/mediatek/mt8196: Add RTC driver
......................................................................

Patch Set 1:

(16 comments)

File src/soc/mediatek/mt8196/mt6685_rtc.c:

https://review.coreboot.org/c/coreboot/+/85978/comment/2d51cc77_7021d863?usp... :
PS1, Line 6: 
           : #include <commonlib/bsd/bcd.h>
           : #include <console/console.h>
           : #include <delay.h>
           : #include <soc/rtc.h>
           : #include <soc/mt6685.h>
           : #include <soc/mt6685_rtc.h>
           : #include <soc/mt6685_hw.h>
           : #include <soc/mt6685_rtc_hw.h>
           : #include <soc/mt6685.h>
           : #include <soc/pmif.h>
           : #include <soc/spmi.h>
           : #include <stdbool.h>
           : #include <timer.h>
sort

https://review.coreboot.org/c/coreboot/+/85978/comment/4c16c7c2_04ab8c0d?usp... :
PS1, Line 79: RG_FQMTR_CLK_CK_PDN_SHIFT
align and fix similar issues.

https://review.coreboot.org/c/coreboot/+/85978/comment/e97cf240_aa1f6841?usp... :
PS1, Line 148: freq[2]
What is the difference from freq[0] ?

https://review.coreboot.org/c/coreboot/+/85978/comment/761c563e_abab95d2?usp... :
PS1, Line 173: u32
u16

https://review.coreboot.org/c/coreboot/+/85978/comment/a0fc5fc4_fbe50442?usp... :
PS1, Line 173: rtc_clr_clrset_trigger
rtc_clrset_trigger

https://review.coreboot.org/c/coreboot/+/85978/comment/a1d0358a_c719b02d?usp... :
PS1, Line 173: u32
u16

https://review.coreboot.org/c/coreboot/+/85978/comment/616b1803_047a87c5?usp... :
PS1, Line 233: rtc_read(RTC_AL_SEC, &rdata);
             : 
             : 	rtc_write(RTC_AL_SEC, (rdata & (~RTC_LPD_OPT_MASK)) | RTC_LPD_OPT_EOSC_LPD);
             : 	if (!rtc_write_trigger())
rtc_clrset_trigger ?

https://review.coreboot.org/c/coreboot/+/85978/comment/0efb6102_4b1098d3?usp... :
PS1, Line 239: rtc_read(RTC_CON, &rdata);
             : 	con = rdata | RTC_XOSC32_LPEN;
             : 	con &= ~RTC_CON_LPRST;
             : 	rtc_write(RTC_CON, con);
             : 	if (!rtc_write_trigger())
rtc_clrset_trigger ?

https://review.coreboot.org/c/coreboot/+/85978/comment/89dc07da_40b45a68?usp... :
PS1, Line 315: RTC_SEC_DSN_REV0, rtc_sec_dsn_rev0);
move to above line

https://review.coreboot.org/c/coreboot/+/85978/comment/7b06e585_b2933c81?usp... :
PS1, Line 341: rtc_write(RTC_POWERKEY1, RTC_POWERKEY1_KEY);
             : 	rtc_write(RTC_POWERKEY2, RTC_POWERKEY2_KEY);
             : 
             : 	if (!rtc_write_trigger())
`rtc_powerkey_init`

https://review.coreboot.org/c/coreboot/+/85978/comment/3b982afe_b0031b98?usp... :
PS1, Line 357: rtc_write(RTC_POWERKEY1, RTC_POWERKEY1_KEY);
             : 	rtc_write(RTC_POWERKEY2, RTC_POWERKEY2_KEY);
             : 	if (!rtc_write_trigger())
rtc_powerkey_init

https://review.coreboot.org/c/coreboot/+/85978/comment/fd27df3c_80c1e2c5?usp... :
PS1, Line 374: rtc_frequency_meter_check() 
move to the second and align with RETRY_MAX_TIME

https://review.coreboot.org/c/coreboot/+/85978/comment/69d9a440_834ec4a3?usp... :
PS1, Line 404: | RTC_K_EOSC_RSV_7 | RTC_K_EOSC_RSV_6);
move to above line

https://review.coreboot.org/c/coreboot/+/85978/comment/55a843b0_8f229869?usp... :
PS1, Line 433: MT6351
Is this comment correct ?

https://review.coreboot.org/c/coreboot/+/85978/comment/9b4f199d_8226c64c?usp... :
PS1, Line 434: 	rtc_write(RTC_POWERKEY1, RTC_POWERKEY1_KEY);
             : 	rtc_write(RTC_POWERKEY2, RTC_POWERKEY2_KEY);
             : 
             : 	if (!rtc_write_trigger()) {
             : 		printk(BIOS_ERR,
             : 		       "%s rtc_write_trigger after writing POWERKEY\n", __func__);
             : 		return false;
             : 	}
             : 
             : 	if (!rtc_writeif_unlock()) {
             : 		printk(BIOS_ERR,
             : 		       "%s rtc_writeif_unlock after writing POWERKEY\n", __func__);
             : 		return false;
             : 	}
What is the difference between line 406~412 and here ? (additional call to `rtc_write_trigger`)

https://review.coreboot.org/c/coreboot/+/85978/comment/2b05b3d0_3eb4da8a?usp... :
PS1, Line 585: RG_OCT1_RTC32K_1V8_0_MASK
align