Attention is currently required from: Guangjie Song, Hung-Te Lin, Jarried Lin.
Yu-Ping Wu has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/84497?usp=email )
Change subject: soc/mediatek/mt8196: Add mtcmos init support ......................................................................
Patch Set 34:
(2 comments)
File src/soc/mediatek/mt8196/mtcmos.c:
https://review.coreboot.org/c/coreboot/+/84497/comment/d293607f_9a5c4b58?usp... : PS34, Line 390: if (!strcmp(item->name, cb->name)) { Using a string as the unique identifier is inefficient. If we really worry about having register duplicate callbacks, please use numeric IDs. However I'm not sure if the check is needed here.
https://review.coreboot.org/c/coreboot/+/84497/comment/41651479_d3492d78?usp... : PS34, Line 836: 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); : : mtcmos_ctrl(MTCMOS_ID_DISP_VCORE, MTCMOS_POWER_ON); So, the whole callback code is only for these 4 `mtcmos_ctrl` calls? If there are no other calls that need callbacks, then we can remove the callback infrastructure altogether.
``` // spm_mtcmos.h struct mtcmos_ops { int (*pre_on)(void); int (*pre_off)(void); ... int (*post_on_sram)(void); };
// mtcmos.c void mtcmos_ctrl_ops(enum mtcmos_id id, enum mtcmos_state state, const struct mtcmos_ops *ops) { ...; }
void mtcmos_ctrl(enum mtcmos_id id, enum mtcmos_state state) { mtcmos_ctrl_ops(id, state, NULL); }
static struct mtcmos_ops disp_vcore_ops { ...; };
void mtcmos_post_init(void) { ... mtcmos_ctrl_ops(MTCMOS_ID_DISP_VCORE, MTCMOS_POWER_ON, &disp_vcore_ops); ... } ```