Attention is currently required from: Hung-Te Lin, Jarried Lin.
Yu-Ping Wu has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85774?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: soc/mediatek/mt8196: Add dynamic power-saving for peripheral clocks ......................................................................
Patch Set 3:
(3 comments)
File src/soc/mediatek/mt8196/include/soc/addressmap.h:
https://review.coreboot.org/c/coreboot/+/85774/comment/1fea172f_52d38df3?usp... : PS3, Line 137: PERI_MASK_IDLE_TO_CKSYS = IO_PHYS + 0x06630270, I think this file is supposed to have only base registers, not all the registers we need. Is `PERICFG_AO_SEC_BASE` the correct base of `PERI_MASK_IDLE_TO_CKSYS`? If so, please write something like `PERICFG_AO_SEC_BASE + CKSYS_OFFSET`, where `CKSYS_OFFSET` can be defined in cksys.c.
File src/soc/mediatek/mt8196/mtk_cksys.c:
PS3: No need to add mtk_ to the file name. We don't use that prefix for other files.
The function name can remain the same.
https://review.coreboot.org/c/coreboot/+/85774/comment/e3ed72d5_66f34d75?usp... : PS3, Line 6: mtk_cksys Same for the header file name. Can we use cksys.h?