Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47895 )
Change subject: WIP: mediatek/common/mtlib.c: refactor load_blob_file API ......................................................................
Patch Set 6:
(8 comments)
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/common/mtl... File src/soc/mediatek/common/mtlib.c:
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/common/mtl... PS6, Line 10: mcu Is mcu a general term, suitable for sspm?
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/common/mtl... PS6, Line 10: fw_name blob_name (since you're using 'blob_bytes')
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/common/mtl... PS6, Line 13: blob_bytes I'd prefer 'blob_size'.
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/common/mtl... PS6, Line 21: . Remove this
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/common/mtl... PS6, Line 33: , size %zd bytes Either "(%zd bytes)" or "(size = %zd)"
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/mt8192/dpm... File src/soc/mediatek/mt8192/dpm.c:
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/mt8192/dpm... PS6, Line 36: printk(BIOS_ERR, "%s() failed (%d)\n", __func__, ret); Same. Maybe no need to repeat the error message here.
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/mt8192/mcu... File src/soc/mediatek/mt8192/mcupm.c:
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/mt8192/mcu... PS6, Line 23: die("%s() failed\n", __func__); Why do we die here, but only print an error message in sspm_init()? Is this failure more severe than the other?
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/mt8192/ssp... File src/soc/mediatek/mt8192/sspm.c:
https://review.coreboot.org/c/coreboot/+/47895/6/src/soc/mediatek/mt8192/ssp... PS6, Line 18: printk(BIOS_ERR, "%s() failed\n", __func__); An error message should have been printed in mtlib_load_and_run(). Maybe no need to repeat it here.