Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45399 )
Change subject: soc/mediatek/mt8192: add pmic MT6359P driver ......................................................................
Patch Set 24:
(5 comments)
https://review.coreboot.org/c/coreboot/+/45399/24/src/soc/mediatek/mt8192/mt... File src/soc/mediatek/mt8192/mt6359p.c:
https://review.coreboot.org/c/coreboot/+/45399/24/src/soc/mediatek/mt8192/mt... PS24, Line 336: { Check pmif_arb for these 4 functions.
if (pmif_arb == NULL) die("ERROR: pmif_arb not initialized");
https://review.coreboot.org/c/coreboot/+/45399/24/src/soc/mediatek/mt8192/mt... PS24, Line 344: No need for this extra blank line.
https://review.coreboot.org/c/coreboot/+/45399/24/src/soc/mediatek/mt8192/mt... PS24, Line 374: Same.
https://review.coreboot.org/c/coreboot/+/45399/24/src/soc/mediatek/mt8192/mt... PS24, Line 414: ( Remove the parentheses.
https://review.coreboot.org/c/coreboot/+/45399/24/src/soc/mediatek/mt8192/mt... PS24, Line 419: if (pmif_arb == NULL) { : pmif_arb = get_pmif_controller(PMIF_SPI, 0); : if (pmif_arb == NULL) : die("ERROR - Failed to get pmif controller\n"); : } Can we extract this to a static function init_pmif_arb()?