Attention is currently required from: Hung-Te Lin, Paul Menzel, Rex-BC Chen. Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52668 )
Change subject: soc/mediatek/mt8195: add pmif/spmi/pmic driver ......................................................................
Patch Set 3:
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/52668/comment/d2d651c9_15f64db5 PS3, Line 9: MT8195
What is the difference to `mediatek/mt8192`? The code looks similar.
The IO pins, clock setting and PMIC setting related to SoC are difference from MT8192. The common parts are moved to common/ folder.
File src/soc/mediatek/mt8195/mt6315.c:
https://review.coreboot.org/c/coreboot/+/52668/comment/8e7a5f45_ab8a954d PS3, Line 6: /* disable magic key protection */
What does that mean? Please reference the datasheet name and section.
The setting is the PMIC internal setting. The values are simulation results provided by IC designer. The setting is unique to different MediaTek SoC and can't be customized. We have datasheets for MT6315 and MT6359p. But those settings won't be disclosed.
File src/soc/mediatek/mt8195/pmif_clk.c:
PS3:
This looks very similar to `src/soc/mediatek/mt8192/pmif_clk.c`. Please do not duplicate code.
The clock register and bit definition are different to mt8192 even though it looks very similar to `mt8192/pmif_clk.c`. We prefer keeping a complete pmif_clk.c in mt8195/ folder for maintenance.
-
Yidi Lin (Code Review)