Chris, On Mon, Oct 27, 2014 at 6:47 AM, Chris Zhong <zyw at rock-chips.com> wrote: > It's a basic version of suspend and resume for rockchip, it only support RK3288 > now. > > Signed-off-by: Tony Xie <xxx at rock-chips.com> > Signed-off-by: Chris Zhong <zyw at rock-chips.com> > > Reviewed-by: Doug Anderson <dianders at chromium.org> > Tested-by: Doug Anderson <dianders at chromium.org> Please no blank lines in tags. > +static inline void rk3288_copy_data_to_sram(void) > +{ > + /* save boot sram data in ddr mem */ > + memcpy(bootram_save_data, rk3288_bootram_base, rk3288_bootram_sz); I'm going to suggest again to get rid of the save/restore of SRAM. I think when I asked before you argued that the save / restore is important because on some Rockchip variants the same region of SRAM is used for SMP processor bring up and for resume. On those variants it's important to save and restore. Fine. ...but here you're in an rk3288 specific function. There's no reason to do the save/restore there. NOTE: I think Kevin Hilman suggested only doing the copy once at init time. That's not a terrible suggestion unless someone is trashing this region of SRAM. ...in my proposed adaptation of the suspend/resume to C code <https://chromium-review.googlesource.com/#/c/225492/> I have removed the save / restore but still copy it every time... > +static void rk3288_fill_in_bootram(u32 level) > +{ > + rkpm_bootdata_cpusp = rk3288_bootram_phy + (SZ_4K - 8); > + rkpm_bootdata_cpu_code = virt_to_phys(cpu_resume); > + > + rkpm_bootdata_l2ctlr_f = 1; > + rkpm_bootdata_l2ctlr = rk3288_l2_config(); > + > + if (level == ROCKCHIP_ARM_OFF_LOGIC_DEEP) { > + /* > + * In this mode the SDRAM power domain will be off, > + * so it need to be resumed, > + * but now the sdram resume code is not ready. > + * i have to set "rkpm_bootdata_ddr_code" 0. > + */ > + rkpm_bootdata_ddr_code = 0; > + } else { > + rkpm_bootdata_ddr_code = 0; > + } Keven requested to just remove the SDRAM stubs for now. We can readd once we have SDRAM code.