[PATCH v5 3/6] clk: rockchip: RK3288: add suspend and resume

[Doug Anderson]

Chris,

On Mon, Oct 27, 2014 at 8:15 PM, Chris Zhong <zyw at rock-chips.com> wrote:
>
> On 10/28/2014 01:27 AM, Doug Anderson wrote:
>
> Chris,
>
> On Mon, Oct 27, 2014 at 6:47 AM, Chris Zhong <zyw at rock-chips.com> wrote:
>
> save and restore some clks, which might be changed in suspend.
>
> Signed-off-by: Tony Xie <xxx at rock-chips.com>
> Signed-off-by: Chris Zhong <zyw at rock-chips.com>
>
> ---
>
> Changes in v5:
> - modify comments
>
> Changes in v4: None
> Changes in v3: None
> Changes in v2:
> - __raw_readl/__raw_writel replaced by readl_relaxed/writel_relaxed
>
>  drivers/clk/rockchip/clk-rk3288.c |   60
> +++++++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
>
> diff --git a/drivers/clk/rockchip/clk-rk3288.c
> b/drivers/clk/rockchip/clk-rk3288.c
> index 2327829..94da06c 100644
> --- a/drivers/clk/rockchip/clk-rk3288.c
> +++ b/drivers/clk/rockchip/clk-rk3288.c
> @@ -16,6 +16,7 @@
>  #include <linux/clk-provider.h>
>  #include <linux/of.h>
>  #include <linux/of_address.h>
> +#include <linux/syscore_ops.h>
>  #include <dt-bindings/clock/rk3288-cru.h>
>  #include "clk.h"
>
> @@ -762,6 +763,64 @@ static const char *rk3288_critical_clocks[] __initconst
> = {
>         "hclk_peri",
>  };
>
> +#ifdef CONFIG_PM_SLEEP
> +static void __iomem *rk3288_cru_base;
> +static const int rk3288_saved_cru_reg_ids[] = {
> +       RK3288_MODE_CON,
> +       RK3288_CLKSEL_CON(0),
> +       RK3288_CLKSEL_CON(1),
> +       RK3288_CLKSEL_CON(10),
> +       RK3288_CLKSEL_CON(33),
> +       RK3288_CLKSEL_CON(37),
> +};
>
> I still haven't had time to figure out exactly why these registers and
> not others...
>
> ...but I think that the code here looks reasonable and I have verified
> that saving at least some of these registers does matter.  I'd vote to
> land these and if we have to add/remove registers later we can do it.
> Thus:
>
> Reviewed-by: Doug Anderson <dianders at chromium.org>
> Tested-by: Doug Anderson <dianders at chromium.org>
>
>
> These CLKSEL  registers would be changed in maskrom.  so we need save and
> restore them.
>
> code in maskrom ：
> ...
> cruReg->CRU_CLKSEL_CON[0] = (0x9fff<<16)
> | (0x0<<15) // CORE select ARM PLL
> | (0x0<<8) // core div
> | (0x0<<4) // aclk_core_mp_div
> | (0x0); // aclk_core_m0_dive
> cruReg->CRU_CLKSEL_CON[1] = (0xf3ff<<16)
> | (1<<15) // bus_aclk_pll_sel= GPLL
> | (0x0<<12) // pd_bus_pclk_div
> | (0x0<<8) // pd_bus_hclk_div=1:1
> | (0x0<<3) // pd_bus_aclk_div
> | (0x0); // pd_bus_clk_div
> cruReg->CRU_CLKSEL_CON[10] = (((0x1<<15)|(0x3<<12)|(0x3<<8)|(0x1f))<<16)
> | (0x1<<15) // peri_pll_sel = GPLL
> | (0x0<<12) // aclk_periph:pclk_periph = 1:1
> | (0x0<<8) // aclk_periph:hclk_periph = 1:1
> | (0x0); // GPLL:aclk_periph = 1:1
> cruReg->CRU_CLKSEL_CON[33] = (((0x1f<<8)|(0x1f))<<16)
> | (0x0<<8) // alive_pclk
> | (0x0); // pmu_pclk
> cruReg->CRU_CLKSEL_CON[37] = (((0x1f<<9)|(0x1f<<4)|(0x7))<<16)
> | (0<<9) //pclk_core_dbg
> | (0x0<<4) // atclk_core
> | (0x0); // clk_l2ram
> ...

OK, now it makes sense!  When I read:

+ * Cru will be reset in maskrom when system wake up from fastboot.
+ * The apll/cpll/gpll will be set into slow mode in maskrom.
+ * So save them before suspend, restore them after resume.

I thought that all of the CRU was being reset.  ...but it's only some
registers.  I guess I would have expected:

+ * Some CRU registers will be reset in maskrom when the system
+ * wakes up from fastboot.
+ * So save them before suspend, restore them after resume.


When you go into deep sleep do we need to save and restore more of the
registers?  I think your current patch series can only want up from
the very lightest sleep mode, right?