Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 32:
(13 comments)
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... File src/soc/mediatek/mt8192/dpm.c:
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 27: Just one tab, or
dm_file_bytes = cbfs_boot_load_file( dm_file_name, dpm_dm_bin, sizeof(dpm_dm_bin), CBFS_TYPE_RAW);
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 29: binary %s not found Failed to load %s
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 34: sizeof(dpm_pm_bin), CBFS_TYPE_RAW); Same.
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 36: binary %s not found Failed to load %s
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 39: %d Use "%zu" (or "%#zx" if you prefer hex), and no need to cast it to "int".
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 40: (int) Please align with "BIOS_INFO".
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 43: write32(&mtk_dpm->sw_rstn, read32(&mtk_dpm->sw_rstn) | 0x10000000); Use clrsetbits32.
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 43: 0x10000000 Please define a macro for this.
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 54: 0x1 Please define a macro for this.
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/dp... PS32, Line 54: write32(&mtk_dpm->sw_rstn, read32(&mtk_dpm->sw_rstn) | 0x1); Use clrsetbits32.
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/in... File src/soc/mediatek/mt8192/include/soc/addressmap.h:
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/in... PS32, Line 30: AUXADC_BASE = IO_PHYS + 0x01001000, : DPM_PM_SRAM_BASE = IO_PHYS + 0x00900000, : DPM_DM_SRAM_BASE = IO_PHYS + 0x00920000, : DPM_CFG_BASE = IO_PHYS + 0x00940000, Please sort these by values.
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/in... File src/soc/mediatek/mt8192/include/soc/dpm.h:
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/in... PS32, Line 10: 0x8000 Use KiB?
https://review.coreboot.org/c/coreboot/+/46393/32/src/soc/mediatek/mt8192/in... PS32, Line 44: #define mtk_dpm ((struct dpm_regs *)DPM_CFG_BASE) Use global variable to be consistent with others.