Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46389 )
Change subject: soc/mediatek/mt8192: add spmfw loader ......................................................................
Patch Set 36:
(3 comments)
https://review.coreboot.org/c/coreboot/+/46389/35/src/soc/mediatek/mt8192/sp... File src/soc/mediatek/mt8192/spm.c:
https://review.coreboot.org/c/coreboot/+/46389/35/src/soc/mediatek/mt8192/sp... PS35, Line 436: 0x40000000 please use DRAM_START instead of hard-coded values. (or another name so it's easier to figure out what this value means, especially if we change DRAM_START address someday)
https://review.coreboot.org/c/coreboot/+/46389/35/src/soc/mediatek/mt8192/sp... PS35, Line 443: BIOS_INFO more like DEBUG?
https://review.coreboot.org/c/coreboot/+/46389/35/src/soc/mediatek/mt8192/sp... PS35, Line 453: if (read32(&mtk_spm->md32pcm_dma0_src) != ptr || : read32(&mtk_spm->md32pcm_dma0_dst) != pmem_start || : read32(&mtk_spm->md32pcm_dma0_wppt) != pmem_words || : read32(&mtk_spm->md32pcm_dma0_wpto) != dmem_start || : read32(&mtk_spm->md32pcm_dma0_count) != total_words || : read32(&mtk_spm->md32pcm_dma0_con) != MD32PCM_DMA0_CON_VAL) { why do we need to do slave mode if the regs look correct? how can we be sure if the IM has correct data, since there's no checksum?
what about we simply always kick IM to re-fetch?