23 comments:
File src/soc/mediatek/mt8183/include/soc/spm.h:
u32 pcm_event_vector0;
u32 pcm_event_vector1;
u32 pcm_event_vector2;
u32 pcm_event_vector3;
u32 pcm_event_vector4;
u32 pcm_event_vector5;
u32 pcm_event_vector6;
u32 pcm_event_vector7;
u32 pcm_event_vector8;
u32 pcm_event_vector9;
u32 pcm_event_vector10;
u32 pcm_event_vector11;
u32 pcm_event_vector12;
u32 pcm_event_vector13;
u32 pcm_event_vector14;
u32 pcm_event_vector15;
should we change these to pcm_event_vector[16];
it is ok to me
check_member(mtk_spm_regs, poweron_config_set, 0x0000);
check_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_reg_data_ini, 0x0028);
check_member(mtk_spm_regs, pcm_pwr_io_en, 0x002c);
check_member(mtk_spm_regs, pcm_im_host_rw_ptr, 0x0038);
check_member(mtk_spm_regs, pcm_im_host_rw_dat, 0x003c);
check_member(mtk_spm_regs, pcm_event_vector0, 0x0040);
check_member(mtk_spm_regs, pcm_event_vector1, 0x0044);
check_member(mtk_spm_regs, pcm_event_vector2, 0x0048);
check_member(mtk_spm_regs, pcm_event_vector3, 0x004c);
check_member(mtk_spm_regs, pcm_event_vector4, 0x0050);
check_member(mtk_spm_regs, pcm_event_vector5, 0x0054);
check_member(mtk_spm_regs, pcm_event_vector6, 0x0058);
check_member(mtk_spm_regs, pcm_event_vector7, 0x005c);
check_member(mtk_spm_regs, pcm_event_vector8, 0x0060);
check_member(mtk_spm_regs, pcm_event_vector9, 0x0064);
check_member(mtk_spm_regs, pcm_event_vector10, 0x0068);
check_member(mtk_spm_regs, pcm_event_vector11, 0x006c);
check_member(mtk_spm_regs, pcm_event_vector12, 0x0070);
check_member(mtk_spm_regs, pcm_event_vector13, 0x0074);
check_member(mtk_spm_regs, pcm_event_vector14, 0x0078);
check_member(mtk_spm_regs, pcm_event_vector15, 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);
I don't think these are needed. […]
Do you mean that we don't need to check every register what we use, just check the few critical register which is enough, especially the last one for close number, right?
File src/soc/mediatek/mt8183/include/soc/spm_init.h:
Patch Set #14, Line 16: #ifndef __SPM_INIT__
I think we can move whole spm_init.h into spm.h. […]
Actually, it's up to you and reviewer's comment, I could move to spm.h.
In my opinion, since these are for SPM initial setting, so why don't add a new spm_init.
How do you think
Patch Set #14, Line 137: WAKE_SRC_R12_PCM_TIMER = (1U << 0),
for enum you don't need () […]
ok, I would change it on next version.
Patch Set #14, Line 178: u32 vec0;
u32 vector[16];
ok, I would change it on next version.
Patch Set #14, Line 196: pwr_ctrl
this seems not used anywhere. […]
Yes, I would remove these on next version.
Patch Set #14, Line 301: wake_status
this seems not used anywhere. […]
Yes, I would remove these on next version.
File src/soc/mediatek/mt8183/spm.c:
if ((pcm_fsm_sta & PCM_FSM_STA_MASK) != PCM_FSM_STA_DEF)
printk(BIOS_ERR, "PCM reset failed\n");
should we abort and return?
ok, I would update it on next version.
Patch Set #14, Line 94: unsigned int
u32
ok, I would update it on next version.
Patch Set #14, Line 102: wait_us
should we abort and return on timeout?
ok, I would update it on next version.
Should we say something like timeout? this message seems obfuscated
ok, I would update it on next version.
Patch Set #14, Line 117: unsigned int
since we'll use these for write/read32 they should be u32.
ok, I would update it on next version.
Patch Set #14, Line 128: write32
sounds like you want clrsetbits32
Yes, but I need write project code for high 16 bit to clear such register at the same time.
pcm_con1 = pcm con1 | (0xb16 << 16) & ~PCM_TIMER_EN_LSB
How do you suggest
%p
ok, I would update it on next version.
Patch Set #14, Line 162: unsigned int
u32
ok, I would update it on next version.
Patch Set #14, Line 174: nsigned int
u32
ok, I would update it on next version.
Patch Set #14, Line 184: ptr = (unsigned int)(unsigned long)(pcmdesc->base);
so you only want to print base? […]
ok, I would update it on next version.
Patch Set #14, Line 185: len = pcmdesc->size - 1;
just put that in printk.
ok, I would update it on next version.
Patch Set #14, Line 214: write32
change the regs to array and do a for-loop
ok, I would try to update it on next version.
sizeof(firmware_size)
ok, I would update it on next version.
int
ok, I would update it on next version.
if CONFIG(MT8183_DRAM_EMCP)
index = DYNA_LOAD_PCM_SUSPEND_LP4_3733;
#else
index = DYNA_LOAD_PCM_SUSPEND_LP4_3200;
#endif
Use real code. […]
ok, I would update it on next version.
Patch Set #14, Line 346: return
return 1 or return -1 to indicate failure
ok, I would update it on next version.
To view, visit change 34545. To unsubscribe, or for help writing mail filters, visit settings.