Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46393 )
Change subject: soc/mediatek/mt8192: Add dpm loader ......................................................................
Patch Set 5:
(6 comments)
https://review.coreboot.org/c/coreboot/+/46393/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46393/5//COMMIT_MSG@8 PS5, Line 8: Please elaborate, what DPM is, what it is used for, and where it’s documented.
https://review.coreboot.org/c/coreboot/+/46393/5//COMMIT_MSG@10 PS5, Line 10: TEST=Boots correctly on Asurada Tested with what firmware version, and how much time did it add? Maybe add the related coreboot log messages.
https://review.coreboot.org/c/coreboot/+/46393/5/src/soc/mediatek/mt8192/dpm... File src/soc/mediatek/mt8192/dpm.c:
https://review.coreboot.org/c/coreboot/+/46393/5/src/soc/mediatek/mt8192/dpm... PS5, Line 18: Document the return values.
https://review.coreboot.org/c/coreboot/+/46393/5/src/soc/mediatek/mt8192/dpm... PS5, Line 23: size_t dm_file_size, pm_file_size; Is the unit bytes? Add it to the variable name?
https://review.coreboot.org/c/coreboot/+/46393/5/src/soc/mediatek/mt8192/dpm... PS5, Line 38: printk(BIOS_INFO, "dm_file_size:%d, pm_file_size:%d\n", Does the blob contain a version? Please print it out too.
https://review.coreboot.org/c/coreboot/+/46393/5/src/soc/mediatek/mt8192/dpm... PS5, Line 38: dm_file_size:%d, pm_file_size:%d Please add a space after the colon and the units.