Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45400 )
Change subject: soc/mediatek/mt8192: add pmic MT6315 driver ......................................................................
Patch Set 1:
(11 comments)
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/mt6... File src/soc/mediatek/mt8192/mt6315.c:
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/mt6... PS1, Line 23: A lowercase 'a'
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/mt6... PS1, Line 23: static static const
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/mt6... PS1, Line 118: B b
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/mt6... PS1, Line 118: static static const
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/mt6... PS1, Line 212: if (pmif_arb == NULL) : pmif_arb = get_pmif_controller(PMIF_SPMI, 0); Similar to MT6359P, I think there's no need for this.
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/mt6... PS1, Line 262: size_t int
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/mt6... PS1, Line 267: size_t int
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/mt6... PS1, Line 273: buck_uV buck_uv
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/mt6... PS1, Line 287: assert(0); die()?
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/mt6... PS1, Line 311: assert(0); die()?
https://review.coreboot.org/c/coreboot/+/45400/1/src/soc/mediatek/mt8192/mt6... PS1, Line 324: pmif_arb What if this is NULL?