Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45402 )
Change subject: soc/mediatek: Add function to get clock frequency of MT8192 ......................................................................
Patch Set 2:
(7 comments)
https://review.coreboot.org/c/coreboot/+/45402/2/src/soc/mediatek/common/inc... File src/soc/mediatek/common/include/soc/pll_common.h:
https://review.coreboot.org/c/coreboot/+/45402/2/src/soc/mediatek/common/inc... PS2, Line 65: N Please add a trailing "," for consistency.
https://review.coreboot.org/c/coreboot/+/45402/2/src/soc/mediatek/common/inc... PS2, Line 67: unsigned int u32
https://review.coreboot.org/c/coreboot/+/45402/2/src/soc/mediatek/mt8192/pll... File src/soc/mediatek/mt8192/pll.c:
https://review.coreboot.org/c/coreboot/+/45402/2/src/soc/mediatek/mt8192/pll... PS2, Line 488: return 0; Do we want to die() in this case?
https://review.coreboot.org/c/coreboot/+/45402/2/src/soc/mediatek/mt8192/pll... PS2, Line 497: setbits32(&mtk_topckgen->clk26cali_0, CLK26CALI_0_TRIGGER); Could also we use the SET32_BITFIELDS API for this?
https://review.coreboot.org/c/coreboot/+/45402/2/src/soc/mediatek/mt8192/pll... PS2, Line 503: else Also add {} for the else block.
https://review.coreboot.org/c/coreboot/+/45402/2/src/soc/mediatek/mt8192/pll... PS2, Line 504: output = 0; If this is an error case, perhaps at least print an error message here?
https://review.coreboot.org/c/coreboot/+/45402/2/src/soc/mediatek/mt8192/pll... PS2, Line 514: ( No need for the parentheses.