Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34545 )
Change subject: mediatek/mt8183: Init SPM driver ......................................................................
Patch Set 16:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34545/16/src/soc/mediatek/mt8183/in... File src/soc/mediatek/mt8183/include/soc/spm.h:
https://review.coreboot.org/c/coreboot/+/34545/16/src/soc/mediatek/mt8183/in... PS16, Line 462: heck_member(mtk_spm_regs, spm_power_on_val0, 0x0004); : check_member(mtk_spm_regs, spm_power_on_val1, 0x0008); : check_member(mtk_spm_regs, spm_clk_con, 0x000c); : check_member(mtk_spm_regs, pcm_con0, 0x0018); : check_member(mtk_spm_regs, pcm_con1, 0x001c); : check_member(mtk_spm_regs, pcm_im_ptr, 0x0020); : check_member(mtk_spm_regs, pcm_im_len, 0x0024); : check_member(mtk_spm_regs, pcm_pwr_io_en, 0x002c); : check_member(mtk_spm_regs, pcm_im_host_rw_dat, 0x003c); : check_member(mtk_spm_regs, pcm_event_vector[15], 0x007c); : check_member(mtk_spm_regs, spm_swint_clr, 0x0094); : check_member(mtk_spm_regs, spm_cpu_wakeup_event, 0x00b0); : check_member(mtk_spm_regs, spm_irq_mask, 0x00b4); : check_member(mtk_spm_regs, spm_wakeup_event_mask, 0x00c4); : check_member(mtk_spm_regs, ddr_en_dbc_len, 0x00d8); : check_member(mtk_spm_regs, pcm_reg0_data, 0x0100); : check_member(mtk_spm_regs, pcm_reg15_data, 0x013c); : check_member(mtk_spm_regs, spm_irq_sta, 0x0158); : check_member(mtk_spm_regs, pcm_fsm_sta, 0x0178); : check_member(mtk_spm_regs, src_ddren_sta, 0x01e0); : check_member(mtk_spm_regs, mcu_pwr_con, 0x0200); : check_member(mtk_spm_regs, mp0_cputop_l2_pdn, 0x0240); : check_member(mtk_spm_regs, cpu_ext_buck_iso, 0x0290); : check_member(mtk_spm_regs, dummy1_pwr_con, 0x02b0); : check_member(mtk_spm_regs, vde_pwr_con, 0x0300); : check_member(mtk_spm_regs, sysrom_con, 0x0354); : check_member(mtk_spm_regs, ufs_sram_con, 0x036c); : check_member(mtk_spm_regs, dummy_sram_con, 0x0380); : check_member(mtk_spm_regs, md_ext_buck_iso_con, 0x0390); : check_member(mtk_spm_regs, mbist_efuse_repair_ack_sta, 0x03d0); : check_member(mtk_spm_regs, spm_dvfs_con, 0x0400); : check_member(mtk_spm_regs, spm_mas_pause2_mask_b, 0x040c); : check_member(mtk_spm_regs, mp0_cpu0_wfi_en, 0x0530); : check_member(mtk_spm_regs, root_cputop_addr, 0x0570); : check_member(mtk_spm_regs, cpu_spare_con, 0x0580); : check_member(mtk_spm_regs, spm2sw_mailbox_0, 0x05d0); : check_member(mtk_spm_regs, spare_ack_mask, 0x06f4); : check_member(mtk_spm_regs, spm_sw_rsv_18, 0x067c); : check_member(mtk_spm_regs, dvfsrc_event_mask_con, 0x0690); : check_member(mtk_spm_regs, spare_src_req_mask, 0x06c0); : check_member(mtk_spm_regs, spare_ack_sta, 0x06f0); : check_member(mtk_spm_regs, spm_dvfs_con1, 0x0700); : check_member(mtk_spm_regs, spm_dvfs_cmd0, 0x0710); : check_member(mtk_spm_regs, wdt_latch_spare0_fix, 0x0780); : check_member(mtk_spm_regs, pcm_wdt_latch_0, 0x0800); : check_member(mtk_spm_regs, spm_pc_trace_con, 0x08c0); : check_member(mtk_spm_regs, spm_ack_chk_con, 0x0900); : check_member(mtk_spm_regs, spm_ack_chk_con2, 0x0920); : check_member(mtk_spm_regs, spm_ack_chk_con3, 0x0940); : check_member(mtk_spm_regs, spm_ack_chk_con4, 0x0960); I think we can delete all these. just keep first/last elements should be good enough.
https://review.coreboot.org/c/coreboot/+/34545/14/src/soc/mediatek/mt8183/in... File src/soc/mediatek/mt8183/include/soc/spm_init.h:
https://review.coreboot.org/c/coreboot/+/34545/14/src/soc/mediatek/mt8183/in... PS14, Line 16: #ifndef __SPM_INIT__
Actually, it's up to you and reviewer's comment, I could move to spm.h. […]
Moving everything to spm.h sounds ok to me.
Since the only thing we want to do for SPM inside firmware is simply to init, I see no reason having two different .c/.h files.