[L] Change in coreboot[main]: soc/mediatek/mt8196: Add srclken_rc drivers

[Yidi Lin (Code Review)]

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

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

Change subject: soc/mediatek/mt8196: Add srclken_rc drivers
......................................................................

Patch Set 1:

(14 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/85842/comment/122078bb_d774b180?usp... :
PS1, Line 9: Mt8196
MT8196

https://review.coreboot.org/c/coreboot/+/85842/comment/3f6f082a_b4748832?usp... :
PS1, Line 8: 
           : Mt8196 use new RC mode with clk_buf driver, and need srclken_rc to send
           : cmd.
Can you elaborate more ? What is RC mode ? What is this driver for ?

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

https://review.coreboot.org/c/coreboot/+/85842/comment/d42ce3d5_eae44a9f?usp... :
PS1, Line 6: /***********************************************************************************
           : ** Definitions
           : ************************************************************************************/
remove

https://review.coreboot.org/c/coreboot/+/85842/comment/3305315d_9c55b956?usp... :
PS1, Line 192: 	
remove one tab

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

https://review.coreboot.org/c/coreboot/+/85842/comment/85957d5b_39f4fe91?usp... :
PS1, Line 69: #define SW_BBLPM_HIGH		1
            : #define SW_BBLPM_LOW		0
            : #define SW_FPM_HIGH		1
            : #define SW_FPM_LOW		0
use enum ?

https://review.coreboot.org/c/coreboot/+/85842/comment/1c7770b6_1b57a511?usp... :
PS1, Line 149: | 
move `|` to above line

https://review.coreboot.org/c/coreboot/+/85842/comment/2f17b35e_020173e4?usp... :
PS1, Line 278: DCXO_SETTLE_BLK_EN
align with first param

https://review.coreboot.org/c/coreboot/+/85842/comment/dbb1e742_d0da4092?usp... :
PS1, Line 312: else
             : 		return;
```
if (mode == ... )
    ....
else
    ....
```
We can assume the parameter is correct if the function is not visible to other files.

https://review.coreboot.org/c/coreboot/+/85842/comment/4ff9ae6a_b905639b?usp... :
PS1, Line 318: id, read32(&rc_regs->rc_mxx_srclken_cfg[id]));
move to above line

https://review.coreboot.org/c/coreboot/+/85842/comment/5ac57d2a_b595363e?usp... :
PS1, Line 320: 
remove one blank line

https://review.coreboot.org/c/coreboot/+/85842/comment/34a24b90_bc456ca6?usp... :
PS1, Line 359: retry(ACK_DELAY_TIMES, (read32(&rc_status_regs->rc_mxx_req_sta_0[chn]) & 0xa) == state, udelay(ACK_DELAY_US))
```
if (!retry(ACK_DELAY_TIMES,
           (read32(&rc_status_regs->rc_mxx_req_sta_0[chn]) & 0xa) == state,
           udelay(ACK_DELAY_US))) {
```

https://review.coreboot.org/c/coreboot/+/85842/comment/97f8b272_1e4782ed?usp... :
PS1, Line 360: 0x%x
%#x

https://review.coreboot.org/c/coreboot/+/85842/comment/0b8ac618_a65a557f?usp... :
PS1, Line 362: chn,
move to the above line

https://review.coreboot.org/c/coreboot/+/85842/comment/f16dbbe6_90880309?usp... :
PS1, Line 385: dsb
coreboot read/write APIs already include `dmb()`. Are you sure dsb is necessary ?