[v5,3/6] clk: rockchip: RK3288: add suspend and resume
diff mbox

Message ID 1414417650-19402-4-git-send-email-zyw@rock-chips.com
State New, archived
Headers show

Commit Message

Chris Zhong Oct. 27, 2014, 1:47 p.m. UTC
save and restore some clks, which might be changed in suspend.

Signed-off-by: Tony Xie <xxx@rock-chips.com>
Signed-off-by: Chris Zhong <zyw@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(+)

Comments

Douglas Anderson Oct. 27, 2014, 5:27 p.m. UTC | #1
Chris,

On Mon, Oct 27, 2014 at 6:47 AM, Chris Zhong <zyw@rock-chips.com> wrote:
> save and restore some clks, which might be changed in suspend.
>
> Signed-off-by: Tony Xie <xxx@rock-chips.com>
> Signed-off-by: Chris Zhong <zyw@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@chromium.org>
Tested-by: Doug Anderson <dianders@chromium.org>
Douglas Anderson Oct. 28, 2014, 3:31 a.m. UTC | #2
Chris,

On Mon, Oct 27, 2014 at 8:15 PM, Chris Zhong <zyw@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@rock-chips.com> wrote:
>
> save and restore some clks, which might be changed in suspend.
>
> Signed-off-by: Tony Xie <xxx@rock-chips.com>
> Signed-off-by: Chris Zhong <zyw@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@chromium.org>
> Tested-by: Doug Anderson <dianders@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?
Chris Zhong Oct. 28, 2014, 7:02 a.m. UTC | #3
On 10/28/2014 11:31 AM, Doug Anderson wrote:
> Chris,
>
> On Mon, Oct 27, 2014 at 8:15 PM, Chris Zhong <zyw@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@rock-chips.com> wrote:
>>
>> save and restore some clks, which might be changed in suspend.
>>
>> Signed-off-by: Tony Xie <xxx@rock-chips.com>
>> Signed-off-by: Chris Zhong <zyw@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@chromium.org>
>> Tested-by: Doug Anderson <dianders@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.
TKS, I will add it to the next version patch.
>
> 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?
the power supply of CRU is always on, unless power off mode.
so don't worry about this issue.
Heiko Stübner Oct. 28, 2014, 2:32 p.m. UTC | #4
Hi Chris,

Am Dienstag, 28. Oktober 2014, 11:15:39 schrieb Chris Zhong:
> On 10/28/2014 01:27 AM, Doug Anderson wrote:
> > Chris,
> > 
> > On Mon, Oct 27, 2014 at 6:47 AM, Chris Zhong <zyw@rock-chips.com> wrote:
> >> save and restore some clks, which might be changed in suspend.
> >> 
> >> Signed-off-by: Tony Xie <xxx@rock-chips.com>
> >> Signed-off-by: Chris Zhong <zyw@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@chromium.org>
> > Tested-by: Doug Anderson <dianders@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

does the maskrom of rk3066a and rk3188 do similar things?

No need to add it in the current series, but it would be nice if we had this 
list already anyway so I can make sure older and newer SoCs don't diverge for 
to long.


Heiko

Patch
diff mbox

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),
+};
+
+static u32 rk3288_saved_cru_regs[ARRAY_SIZE(rk3288_saved_cru_reg_ids)];
+
+/*
+ * 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.
+ */
+static int rk3288_clk_suspend(void)
+{
+	int i, reg_id;
+
+	for (i = 0; i < ARRAY_SIZE(rk3288_saved_cru_reg_ids); i++) {
+		reg_id = rk3288_saved_cru_reg_ids[i];
+
+		rk3288_saved_cru_regs[i] =
+				readl_relaxed(rk3288_cru_base + reg_id);
+	}
+	return 0;
+}
+
+static void rk3288_clk_resume(void)
+{
+	int i, reg_id;
+
+	for (i = ARRAY_SIZE(rk3288_saved_cru_reg_ids) - 1; i >= 0; i--) {
+		reg_id = rk3288_saved_cru_reg_ids[i];
+
+		writel_relaxed(rk3288_saved_cru_regs[i] | 0xffff0000,
+			       rk3288_cru_base + reg_id);
+	}
+}
+
+static struct syscore_ops rk3288_clk_syscore_ops = {
+	.suspend = rk3288_clk_suspend,
+	.resume = rk3288_clk_resume,
+};
+
+static void rk3288_clk_sleep_init(void __iomem *reg_base)
+{
+	rk3288_cru_base = reg_base;
+	register_syscore_ops(&rk3288_clk_syscore_ops);
+}
+
+#else /* CONFIG_PM_SLEEP */
+static void rk3288_clk_sleep_init(void __iomem *reg_base) {}
+#endif
+
 static void __init rk3288_clk_init(struct device_node *np)
 {
 	void __iomem *reg_base;
@@ -810,5 +869,6 @@  static void __init rk3288_clk_init(struct device_node *np)
 				  ROCKCHIP_SOFTRST_HIWORD_MASK);
 
 	rockchip_register_restart_notifier(RK3288_GLB_SRST_FST);
+	rk3288_clk_sleep_init(reg_base);
 }
 CLK_OF_DECLARE(rk3288_cru, "rockchip,rk3288-cru", rk3288_clk_init);