[coreboot-gerrit] [M] Change in coreboot[main]: soc/mediatek/mt8196: Add MMinfra driver support

[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/+/85188?usp=email )

Change subject: soc/mediatek/mt8196: Add MMinfra driver support
......................................................................

Patch Set 5:

(3 comments)

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

https://review.coreboot.org/c/coreboot/+/85188/comment/cc580518_48e6c4f9?usp... :
PS5, Line 19: mm_pre_init
Why is this called `pre_init`? This only registers callbacks for 3 IDs. Looking at the code, I don't think we need to register them before `mt_pll_post_init`. The `disp_vcore_cb` callback in mtcmos.c is also registered in `mtcmos_post_init`, not in `pre_init` or `init` function.

I wonder if we can rename this function to `mminfra_post_init`, and move the 3 `mtcmos_ctrl` calls there:

```
void mminfra_post_init(void)
{
    mtcmos_cb_register(...);
    mtcmos_cb_register(...);
    mtcmos_cb_register(...);

mtcmos_ctrl(MTCMOS_ID_MM_INFRA_AO, MTCMOS_POWER_ON);
    mtcmos_ctrl(MTCMOS_ID_MM_INFRA0, MTCMOS_POWER_ON);
    mtcmos_ctrl(MTCMOS_ID_MM_INFRA1, MTCMOS_POWER_ON);
}
```

Then the flow here would be:

```
mtcmos_init();
mt_pll_post_init();
mminfra_post_init();
mtcmos_post_init();
```

Does that make sense to you?

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

https://review.coreboot.org/c/coreboot/+/85188/comment/5f27db01_8ff7fe78?usp... :
PS5, Line 33: mm
mminfra

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

PS5: 
If there's no other `mminfra_*.c` files, can we rename this to `mminfra.c`?