Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34545 )
Change subject: mediatek/mt8183: Init SPM driver ......................................................................
Patch Set 55:
(5 comments)
https://review.coreboot.org/c/coreboot/+/34545/53/src/soc/mediatek/mt8183/in... File src/soc/mediatek/mt8183/include/soc/spm.h:
https://review.coreboot.org/c/coreboot/+/34545/53/src/soc/mediatek/mt8183/in... PS53, Line 591: char path[128];
Why is this here? You don't seem to be using it.
@dawei
https://review.coreboot.org/c/coreboot/+/34545/53/src/soc/mediatek/mt8183/sp... File src/soc/mediatek/mt8183/spm.c:
https://review.coreboot.org/c/coreboot/+/34545/53/src/soc/mediatek/mt8183/sp... PS53, Line 268: dyna_load_pcm[index].buf);
It is just for debug purpose only. […]
Debugging message removed.
https://review.coreboot.org/c/coreboot/+/34545/53/src/soc/mediatek/mt8183/sp... PS53, Line 270: /* start of pcm_desc */
Do you mean if we could use the other format to package this FW? If yes, sorry that please understan […]
I think that's understandable.
https://review.coreboot.org/c/coreboot/+/34545/53/src/soc/mediatek/mt8183/sp... PS53, Line 277: snprintf(dyna_load_pcm[index].version,
Something needs to make sure the input string is terminated here. […]
@dawei Do we really need 'version'?
https://review.coreboot.org/c/coreboot/+/34545/55/src/soc/mediatek/mt8183/sp... File src/soc/mediatek/mt8183/spm.c:
https://review.coreboot.org/c/coreboot/+/34545/55/src/soc/mediatek/mt8183/sp... PS55, Line 286: assert(offset <= file_size); @jwerner @hungte I'm not sure if this is correct.