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

[Shunxi Zhang (Code Review)]

Attention is currently required from: Hung-Te Lin, Jarried Lin, Paul Menzel, Yidi Lin, Yu-Ping Wu.

Shunxi Zhang 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 5: Code-Review+1

(24 comments)

File src/soc/mediatek/mt8196/include/soc/mt6685_rtc.h:

https://review.coreboot.org/c/coreboot/+/85978/comment/b5131e66_7fdcc18d?usp... :
PS2, Line 84: /*
            :  * Complete the RTC initialization process,
            :  * setting of related registers
            :  */
> `/* Complete the RTC initialization process and register settings. […]

will update

File src/soc/mediatek/mt8196/include/soc/mt6685_rtc_hw.h:

PS1:
> keep comment unresolved

The new version(patchset 3) of header file mt6685_rtc_hw.h deletes the same definition as rtc_common.h

File src/soc/mediatek/mt8196/include/soc/rtc.h:

https://review.coreboot.org/c/coreboot/+/85978/comment/2982039b_0855560c?usp... :
PS2, Line 58: WATCH_INIT
> If this means "stopwatch_init", please use a more descriptive name. […]

will update

https://review.coreboot.org/c/coreboot/+/85978/comment/19f29831_673884ed?usp... :
PS2, Line 59: WAIT_EOSC_CHECK_CLK_RDY_TIME_US
> `EOSC_CHECK_CLK_TIMEOUT_US`

will update

https://review.coreboot.org/c/coreboot/+/85978/comment/25535ee9_f1de19fa?usp... :
PS2, Line 60: RETRY_MAX_TIME
> `RECOVERY_RETRY_COUNT`

will update

File src/soc/mediatek/mt8196/include/soc/rtc.h:

https://review.coreboot.org/c/coreboot/+/85978/comment/13f562fb_26ff0c07?usp... :
PS1, Line 18: 	RTC_BBPU_PWREN		= BIT(0),
> Keep this open until resolved.

The new version of the header file mt6685_RTC_HW.H deletes the same macro definition as rtc_common.h

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

https://review.coreboot.org/c/coreboot/+/85978/comment/37a8dd61_e5f424c4?usp... :
PS1, Line 68: 			     RG_BANK_FQMTR_RST_SHIFT);
> keep comment unresolved

will update

https://review.coreboot.org/c/coreboot/+/85978/comment/acd91690_0fafed9b?usp... :
PS1, Line 148: freq[2]
> Please answer Yidi's question here in the Gerrit comment.

freq[0] is: select 26M as fixed clock

freq[2] is: select 26M as target clock

https://review.coreboot.org/c/coreboot/+/85978/comment/59b95a92_c99ce384?usp... :
PS1, Line 167: 	printk(BIOS_INFO, "EOSC cali val = %#x\n", val);
> Then use log level debug?

will update

https://review.coreboot.org/c/coreboot/+/85978/comment/0e3ee24a_ce1661ab?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())
> keep comment unresolved

will update

https://review.coreboot.org/c/coreboot/+/85978/comment/b637d771_fe300474?usp... :
PS1, Line 417:
> not done

will update

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

https://review.coreboot.org/c/coreboot/+/85978/comment/a847d39e_065a781a?usp... :
PS2, Line 101: ERROR: get frequency
> `FQMTR read`

will update

https://review.coreboot.org/c/coreboot/+/85978/comment/efa2907d_1d142248?usp... :
PS2, Line 105: 26M/32K = 794
> `26MHz / 23K = (26 * 10^6) / (23 * 1024) ~= 794`

will update

https://review.coreboot.org/c/coreboot/+/85978/comment/9222fee6_089a3903?usp... :
PS2, Line 139:
> You removed the wrong blank line. […]

will update

https://review.coreboot.org/c/coreboot/+/85978/comment/fe951a16_308452f4?usp... :
PS2, Line 193: RTC_CON_GPU
> align

will update

https://review.coreboot.org/c/coreboot/+/85978/comment/f72a7bc7_fdb2f984?usp... :
PS2, Line 325: rtc_powerkey_init();
             : 
             : 	if (!rtc_write_trigger())
             : 		return false;
> `rtc_powerkey_init` already contains `rtc_write_trigger`. Change this to […]

will update

https://review.coreboot.org/c/coreboot/+/85978/comment/250e71ff_5844372c?usp... :
PS2, Line 340: rtc_powerkey_init();
             : 	if (!rtc_write_trigger())
             : 		return false;
>

will update

https://review.coreboot.org/c/coreboot/+/85978/comment/d98a441f_e27041c3?usp... :
PS2, Line 375: wrritten
> written

will update

https://review.coreboot.org/c/coreboot/+/85978/comment/89f8b190_1b91193f?usp... :
PS2, Line 388: 	rtc_powerkey_init();
> By changing the 2 `rtc_write` calls to `rtc_powerkey_init`, there will be an extra `rtc_write_trigge […]

Done

https://review.coreboot.org/c/coreboot/+/85978/comment/fa2bc204_2c9cab59?usp... :
PS2, Line 414: need
> Done

done

https://review.coreboot.org/c/coreboot/+/85978/comment/41684237_ff0cada8?usp... :
PS2, Line 415: rtc_powerkey_init();
             : 
             : 	if (!rtc_write_trigger()) {
> `if (!rtc_powerkey_init()) {`

will update

https://review.coreboot.org/c/coreboot/+/85978/comment/462e7b0b_df97311b?usp... :
PS2, Line 419: rtc_write_trigger
> `rtc_powerkey_init failed`

will update

https://review.coreboot.org/c/coreboot/+/85978/comment/89c0765e_491ed506?usp... :
PS2, Line 425:
> failed

will update

https://review.coreboot.org/c/coreboot/+/85978/comment/40c726de_a903eceb?usp... :
PS2, Line 504: Check clock source are match with 32K exist
> `Check if clock sources match existing 32K` […]

will update