[PATCH v5 4/6] ARM: rockchip: add suspend and resume for RK3288

Doug Anderson dianders at chromium.org
Mon Oct 27 10:44:45 PDT 2014


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.



More information about the Linux-rockchip mailing list