[coreboot-gerrit] Change in coreboot[master]: mediatek/mt8192: add spmfw loader

[Yidi Lin (Code Review)]

Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46389 )

Change subject: mediatek/mt8192: add spmfw loader
......................................................................

Patch Set 29:

(2 comments)

https://review.coreboot.org/c/coreboot/+/46389/26/src/soc/mediatek/mt8192/sp... 
File src/soc/mediatek/mt8192/spm.c:

https://review.coreboot.org/c/coreboot/+/46389/26/src/soc/mediatek/mt8192/sp... 
PS26, Line 566: write32(&mtk_spm->pcm_con1, SPM_REGWR_CFG_KEY |
              : 		(read32(&mtk_spm->pcm_con1) & ~RG_PCM_TIMER_EN_LSB));
> Forgot this?

Done

https://review.coreboot.org/c/coreboot/+/46389/29/src/soc/mediatek/mt8192/sp... 
File src/soc/mediatek/mt8192/spm.c:

https://review.coreboot.org/c/coreboot/+/46389/29/src/soc/mediatek/mt8192/sp... 
PS29, Line 421: con1 = read32(&mtk_spm->pcm_con1) & RG_PCM_WDT_WAKE_LSB;
              : 	write32(&mtk_spm->pcm_con1,
              : 		con1 | SPM_REGWR_CFG_KEY | REG_EVENT_LOCK_EN_LSB |
              : 		REG_SPM_SRAM_ISOINT_B_LSB | RG_AHBMIF_APBEN_LSB |
              : 		REG_MD32_APB_INTERNAL_EN_LSB);
> Isn't this equivalent to the following? […]

Hi Yu-Ping,
con1 value could be 0 or (1U << 9).

Roger,
Please consult with DE if bit 9 of pcm_con1 could be always set to 0 or 1 at boot time.