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

[Yu-Ping Wu (Code Review)]

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

Yu-Ping Wu 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 2:

(21 comments)

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

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

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

https://review.coreboot.org/c/coreboot/+/85978/comment/89890d8f_e85f3a62?usp... :
PS2, Line 58: WATCH_INIT
If this means "stopwatch_init", please use a more descriptive name. How about `BBPU_RELOAD_TIMEOUT_US`?

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

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

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

https://review.coreboot.org/c/coreboot/+/85978/comment/73f3250a_ecc615d5?usp... :
PS1, Line 18: 	RTC_BBPU_PWREN		= BIT(0),
> Regarding the problem of the header file RTC_COMMON. […]

Keep this open until resolved.

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

https://review.coreboot.org/c/coreboot/+/85978/comment/e6a1ea0a_121c683b?usp... :
PS1, Line 148: freq[2]
> will update

Please answer Yidi's question here in the Gerrit comment.

https://review.coreboot.org/c/coreboot/+/85978/comment/73d35939_817f4c1d?usp... :
PS1, Line 417:
> Done

not done

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

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

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

https://review.coreboot.org/c/coreboot/+/85978/comment/bd221d13_f7821e07?usp... :
PS2, Line 139: 
You removed the wrong blank line. I think this function should have 3 groups separated by blank lines. Each group contains

```
config_interface(...);
udelay(100);
/* Comment */
result-xxx = ...;
```

Does that make sense to you?

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

```
if (!rtc_powerkey_init())
    return false;
```

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

https://review.coreboot.org/c/coreboot/+/85978/comment/1122e8c2_c445c08b?usp... :
PS2, Line 358: rtc_init_after_recovery
> align or move to above line

move to above line

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

https://review.coreboot.org/c/coreboot/+/85978/comment/3fcf476d_1062d182?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_trigger` call. Please confirm if the behavior is correct.

https://review.coreboot.org/c/coreboot/+/85978/comment/5b05d356_1bc5460e?usp... :
PS2, Line 391: wrritten
written

https://review.coreboot.org/c/coreboot/+/85978/comment/d37b1979_98fcb057?usp... :
PS2, Line 414: need
`needs to`

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

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

https://review.coreboot.org/c/coreboot/+/85978/comment/78e09049_db29c39b?usp... :
PS2, Line 425:  
failed

https://review.coreboot.org/c/coreboot/+/85978/comment/088d679a_c2dd58f6?usp... :
PS2, Line 504: Check clock source are match with 32K exist
`Check if clock sources match existing 32K`

Does my rewording make sense to you?