linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/7] This is the 1st version of suspend for RK3288.
@ 2014-10-29 11:51 Chris Zhong
  2014-10-29 11:51 ` [PATCH v6 1/7] pinctrl: rockchip: add suspend/resume functions Chris Zhong
                   ` (6 more replies)
  0 siblings, 7 replies; 20+ messages in thread
From: Chris Zhong @ 2014-10-29 11:51 UTC (permalink / raw)
  To: linux-arm-kernel

RK3288 can shut down the cpu, gpu and other device controllers in suspend,
and it will pull the GLOBAL_PWROFF pin to high in the final stage of the
process of suspend, pull the pin to low again when resume.

Changes in v6:
- modify comments
- get rid of the save/restore of SRAM
- doing the copy of resume code once at init time
- remove ROCKCHIP_ARM_OFF_LOGIC_DEEP from rk3288_fill_in_bootram
- add of_platform_populate in rockchip_dt_init
- change pmu_intmem at ff720000 to sram at ff720000
- change pmu_intmem at ff720000 to sram at ff720000

Changes in v5:
- reset-author
- use "__maybe_unused" annotation
- add pinctrl_force_default() in the error case
- modify comments
- use rk3288_bootram_sz for memcpy size
- fixed error of sram save and restore
- change the size of sram in example
- change size to 4k

Changes in v4:
- use SIMPLE_DEV_PM_OPS for suspend/resume struct
- remove grf regmap

Changes in v3:
- move the pinmux of gpio6_c6 save and restore to pinctrl-rockchip

Changes in v2:
- __raw_readl/__raw_writel replaced by readl_relaxed/writel_relaxed
- add the regulator calls in prepare and finish.
- add the pinmux of gpio6_c6 save and restore
- put "rockchip,rk3288-pmu-sram" to first

Chris Zhong (7):
  pinctrl: rockchip: add suspend/resume functions
  pinctrl: rockchip: save and restore gpio6_c6 pinmux in suspend/resume
  clk: rockchip: RK3288: add suspend and resume
  ARM: rockchip: add suspend and resume for RK3288
  ARM: rockchip: Add pmu-sram binding
  ARM: dts: add RK3288 suspend support
  ARM: dts: add suspend voltage setting for RK808

 .../devicetree/bindings/arm/rockchip/pmu-sram.txt  |   16 ++
 arch/arm/boot/dts/rk3288-evb-rk808.dts             |   16 +-
 arch/arm/boot/dts/rk3288.dtsi                      |   11 +
 arch/arm/mach-rockchip/Makefile                    |    1 +
 arch/arm/mach-rockchip/pm.c                        |  278 ++++++++++++++++++++
 arch/arm/mach-rockchip/pm.h                        |  103 ++++++++
 arch/arm/mach-rockchip/rockchip.c                  |    8 +
 arch/arm/mach-rockchip/sleep.S                     |   90 +++++++
 drivers/clk/rockchip/clk-rk3288.c                  |   60 +++++
 drivers/pinctrl/pinctrl-rockchip.c                 |   46 ++++
 10 files changed, 628 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/arm/rockchip/pmu-sram.txt
 create mode 100644 arch/arm/mach-rockchip/pm.c
 create mode 100644 arch/arm/mach-rockchip/pm.h
 create mode 100644 arch/arm/mach-rockchip/sleep.S

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v6 1/7] pinctrl: rockchip: add suspend/resume functions
  2014-10-29 11:51 [PATCH v6 0/7] This is the 1st version of suspend for RK3288 Chris Zhong
@ 2014-10-29 11:51 ` Chris Zhong
  2014-10-31 20:54   ` Linus Walleij
  2014-11-02 20:47   ` Heiko Stübner
  2014-10-29 11:52 ` [PATCH v6 2/7] pinctrl: rockchip: save and restore gpio6_c6 pinmux in suspend/resume Chris Zhong
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 20+ messages in thread
From: Chris Zhong @ 2014-10-29 11:51 UTC (permalink / raw)
  To: linux-arm-kernel

support suspend/resume of pinctrl, it allows handling sleep mode
for hogged pins in pinctrl

Signed-off-by: Chris Zhong <zyw@rock-chips.com>
Tested-by: Doug Anderson <dianders@chromium.org>
Reviewed-by: Doug Anderson <dianders@chromium.org>
Tested-by: Heiko Stuebner <heiko@sntech.de>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>

---

Changes in v6: None
Changes in v5:
- reset-author
- use "__maybe_unused" annotation

Changes in v4:
- use SIMPLE_DEV_PM_OPS for suspend/resume struct

Changes in v3: None
Changes in v2: None

 drivers/pinctrl/pinctrl-rockchip.c |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index 016f457..172ad1d 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -1770,6 +1770,23 @@ static struct rockchip_pin_ctrl *rockchip_pinctrl_get_soc_data(
 	return ctrl;
 }
 
+static int __maybe_unused rockchip_pinctrl_suspend(struct device *dev)
+{
+	struct rockchip_pinctrl *info = dev_get_drvdata(dev);
+
+	return pinctrl_force_sleep(info->pctl_dev);
+}
+
+static int __maybe_unused rockchip_pinctrl_resume(struct device *dev)
+{
+	struct rockchip_pinctrl *info = dev_get_drvdata(dev);
+
+	return pinctrl_force_default(info->pctl_dev);
+}
+
+static SIMPLE_DEV_PM_OPS(rockchip_pinctrl_dev_pm_ops, rockchip_pinctrl_suspend,
+			 rockchip_pinctrl_resume);
+
 static int rockchip_pinctrl_probe(struct platform_device *pdev)
 {
 	struct rockchip_pinctrl *info;
@@ -1983,6 +2000,7 @@ static struct platform_driver rockchip_pinctrl_driver = {
 	.driver = {
 		.name	= "rockchip-pinctrl",
 		.owner	= THIS_MODULE,
+		.pm = &rockchip_pinctrl_dev_pm_ops,
 		.of_match_table = rockchip_pinctrl_dt_match,
 	},
 };
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v6 2/7] pinctrl: rockchip: save and restore gpio6_c6 pinmux in suspend/resume
  2014-10-29 11:51 [PATCH v6 0/7] This is the 1st version of suspend for RK3288 Chris Zhong
  2014-10-29 11:51 ` [PATCH v6 1/7] pinctrl: rockchip: add suspend/resume functions Chris Zhong
@ 2014-10-29 11:52 ` Chris Zhong
  2014-10-29 11:52 ` [PATCH v6 3/7] clk: rockchip: RK3288: add suspend and resume Chris Zhong
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 20+ messages in thread
From: Chris Zhong @ 2014-10-29 11:52 UTC (permalink / raw)
  To: linux-arm-kernel

Save and restore the gpio6_c6 pinmux setting, since Maskrom of RK3288
would modify it to sdmmc0_det, so it need to be restored to the correct
setting after resume from Maskrom.

Signed-off-by: Chris Zhong <zyw@rock-chips.com>
Tested-by: Doug Anderson <dianders@chromium.org>
Reviewed-by: Doug Anderson <dianders@chromium.org>
Tested-by: Heiko Stuebner <heiko@sntech.de>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>

---

Changes in v6: None
Changes in v5:
- add pinctrl_force_default() in the error case

Changes in v4: None
Changes in v3: None
Changes in v2: None

 drivers/pinctrl/pinctrl-rockchip.c |   30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index 172ad1d..49ca40c 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -1770,16 +1770,44 @@ static struct rockchip_pin_ctrl *rockchip_pinctrl_get_soc_data(
 	return ctrl;
 }
 
+#define RK3288_GRF_GPIO6C_IOMUX		0x64
+#define GPIO6C6_SEL_WRITE_ENABLE	BIT(28)
+
+static u32 rk3288_grf_gpio6c_iomux;
+
 static int __maybe_unused rockchip_pinctrl_suspend(struct device *dev)
 {
 	struct rockchip_pinctrl *info = dev_get_drvdata(dev);
+	int ret = pinctrl_force_sleep(info->pctl_dev);
+
+	if (ret)
+		return ret;
+
+	/*
+	 * RK3288 GPIO6_C6 mux would be modified by Maskrom when resume, so save
+	 * the setting here, and restore it at resume.
+	 */
+	if (info->ctrl->type == RK3288) {
+		ret = regmap_read(info->regmap_base, RK3288_GRF_GPIO6C_IOMUX,
+				  &rk3288_grf_gpio6c_iomux);
+		if (ret) {
+			pinctrl_force_default(info->pctl_dev);
+			return ret;
+		}
+	}
 
-	return pinctrl_force_sleep(info->pctl_dev);
+	return 0;
 }
 
 static int __maybe_unused rockchip_pinctrl_resume(struct device *dev)
 {
 	struct rockchip_pinctrl *info = dev_get_drvdata(dev);
+	int ret = regmap_write(info->regmap_base, RK3288_GRF_GPIO6C_IOMUX,
+			       rk3288_grf_gpio6c_iomux |
+			       GPIO6C6_SEL_WRITE_ENABLE);
+
+	if (ret)
+		return ret;
 
 	return pinctrl_force_default(info->pctl_dev);
 }
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v6 3/7] clk: rockchip: RK3288: add suspend and resume
  2014-10-29 11:51 [PATCH v6 0/7] This is the 1st version of suspend for RK3288 Chris Zhong
  2014-10-29 11:51 ` [PATCH v6 1/7] pinctrl: rockchip: add suspend/resume functions Chris Zhong
  2014-10-29 11:52 ` [PATCH v6 2/7] pinctrl: rockchip: save and restore gpio6_c6 pinmux in suspend/resume Chris Zhong
@ 2014-10-29 11:52 ` Chris Zhong
  2014-10-29 11:52 ` [PATCH v6 4/7] ARM: rockchip: add suspend and resume for RK3288 Chris Zhong
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 20+ messages in thread
From: Chris Zhong @ 2014-10-29 11:52 UTC (permalink / raw)
  To: linux-arm-kernel

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>
Reviewed-by: Doug Anderson <dianders@chromium.org>
Tested-by: Doug Anderson <dianders@chromium.org>

---

Changes in v6:
- modify comments

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..66a9cb0 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;
+
+/* Some CRU registers will be reset in maskrom when the system
+ * wakes up from fastboot.
+ * So save them before suspend, restore them after resume.
+ */
+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)];
+
+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);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v6 4/7] ARM: rockchip: add suspend and resume for RK3288
  2014-10-29 11:51 [PATCH v6 0/7] This is the 1st version of suspend for RK3288 Chris Zhong
                   ` (2 preceding siblings ...)
  2014-10-29 11:52 ` [PATCH v6 3/7] clk: rockchip: RK3288: add suspend and resume Chris Zhong
@ 2014-10-29 11:52 ` Chris Zhong
  2014-10-29 19:01   ` Kevin Hilman
  2014-10-29 11:52 ` [PATCH v6 6/7] ARM: dts: add RK3288 suspend support Chris Zhong
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Chris Zhong @ 2014-10-29 11:52 UTC (permalink / raw)
  To: linux-arm-kernel

It's a basic version of suspend and resume for rockchip,
it only support RK3288 now.

Signed-off-by: Tony Xie <xxx@rock-chips.com>
Signed-off-by: Chris Zhong <zyw@rock-chips.com>
Tested-by: Doug Anderson <dianders@chromium.org>
Reviewed-by: Doug Anderson <dianders@chromium.org>

---

Changes in v6:
- get rid of the save/restore of SRAM
- doing the copy of resume code once at init time
- remove ROCKCHIP_ARM_OFF_LOGIC_DEEP from rk3288_fill_in_bootram
- add of_platform_populate in rockchip_dt_init

Changes in v5:
- use rk3288_bootram_sz for memcpy size
- fixed error of sram save and restore

Changes in v4:
- remove grf regmap

Changes in v3:
- move the pinmux of gpio6_c6 save and restore to pinctrl-rockchip

Changes in v2:
- add the regulator calls in prepare and finish.
- add the pinmux of gpio6_c6 save and restore

 arch/arm/mach-rockchip/Makefile   |    1 +
 arch/arm/mach-rockchip/pm.c       |  278 +++++++++++++++++++++++++++++++++++++
 arch/arm/mach-rockchip/pm.h       |  103 ++++++++++++++
 arch/arm/mach-rockchip/rockchip.c |    8 ++
 arch/arm/mach-rockchip/sleep.S    |   90 ++++++++++++
 5 files changed, 480 insertions(+)
 create mode 100644 arch/arm/mach-rockchip/pm.c
 create mode 100644 arch/arm/mach-rockchip/pm.h
 create mode 100644 arch/arm/mach-rockchip/sleep.S

diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile
index b29d8ea..5c3a9b2 100644
--- a/arch/arm/mach-rockchip/Makefile
+++ b/arch/arm/mach-rockchip/Makefile
@@ -1,4 +1,5 @@
 CFLAGS_platsmp.o := -march=armv7-a
 
 obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip.o
+obj-$(CONFIG_PM_SLEEP) += pm.o sleep.o
 obj-$(CONFIG_SMP) += headsmp.o platsmp.o
diff --git a/arch/arm/mach-rockchip/pm.c b/arch/arm/mach-rockchip/pm.c
new file mode 100644
index 0000000..0cf3128
--- /dev/null
+++ b/arch/arm/mach-rockchip/pm.c
@@ -0,0 +1,278 @@
+/*
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ * Author: Tony Xie <tony.xie@rock-chips.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/regmap.h>
+#include <linux/suspend.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regulator/machine.h>
+
+#include <asm/cacheflush.h>
+#include <asm/tlbflush.h>
+#include <asm/suspend.h>
+
+#include "pm.h"
+
+struct rockchip_pm_device_id {
+	const char *compatible;
+	const struct platform_suspend_ops *ops;
+	int (*init)(void);
+};
+
+static void __iomem *rk3288_bootram_base;
+static phys_addr_t rk3288_bootram_phy;
+
+static struct regmap *pmu_regmap;
+static struct regmap *sgrf_regmap;
+
+static inline u32 rk3288_l2_config(void)
+{
+	u32 l2ctlr;
+
+	asm("mrc p15, 1, %0, c9, c0, 2" : "=r" (l2ctlr));
+	return l2ctlr;
+}
+
+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();
+
+	rkpm_bootdata_ddr_code = 0;
+}
+
+static u32 rk3288_pmu_pwr_mode_con;
+static u32 rk3288_sgrf_soc_con0;
+
+static void rk3288_slp_mode_set(int level)
+{
+	u32 mode_set, mode_set1;
+
+	regmap_read(sgrf_regmap, RK3288_SGRF_SOC_CON0, &rk3288_sgrf_soc_con0);
+
+	regmap_read(pmu_regmap, RK3288_PMU_PWRMODE_CON,
+		    &rk3288_pmu_pwr_mode_con);
+
+	/* set bit 8 so that system will resume to FAST_BOOT_ADDR */
+	regmap_write(sgrf_regmap, RK3288_SGRF_SOC_CON0,
+		     BIT(SGRF_FAST_BOOT_EN) | BIT(SGRF_FAST_BOOT_EN + 16));
+
+	/* booting address of resuming system is from this register value */
+	regmap_write(sgrf_regmap, RK3288_SGRF_FAST_BOOT_ADDR,
+		     rk3288_bootram_phy);
+
+	regmap_write(pmu_regmap, RK3288_PMU_WAKEUP_CFG1,
+		     PMU_ARMINT_WAKEUP_EN);
+
+	mode_set = BIT(PMU_GLOBAL_INT_DISABLE) | BIT(PMU_L2FLUSH_EN) |
+		   BIT(PMU_SREF0_ENTER_EN) | BIT(PMU_SREF1_ENTER_EN) |
+		   BIT(PMU_DDR0_GATING_EN) | BIT(PMU_DDR1_GATING_EN) |
+		   BIT(PMU_PWR_MODE_EN) | BIT(PMU_CHIP_PD_EN) |
+		   BIT(PMU_SCU_EN);
+
+	mode_set1 = BIT(PMU_CLR_CORE) | BIT(PMU_CLR_CPUP);
+
+	if (level == ROCKCHIP_ARM_OFF_LOGIC_DEEP) {
+		/* arm off, logic deep sleep */
+		mode_set |= BIT(PMU_BUS_PD_EN) |
+			    BIT(PMU_DDR1IO_RET_EN) | BIT(PMU_DDR0IO_RET_EN) |
+			    BIT(PMU_OSC_24M_DIS) | BIT(PMU_PMU_USE_LF) |
+			    BIT(PMU_ALIVE_USE_LF) | BIT(PMU_PLL_PD_EN);
+
+		mode_set1 |= BIT(PMU_CLR_ALIVE) | BIT(PMU_CLR_BUS) |
+			     BIT(PMU_CLR_PERI) | BIT(PMU_CLR_DMA);
+
+	} else {
+		/*
+		 * arm off, logic normal
+		 * if pmu_clk_core_src_gate_en is not set,
+		 * wakeup will be error
+		 */
+		mode_set |= BIT(PMU_CLK_CORE_SRC_GATE_EN);
+	}
+
+	regmap_write(pmu_regmap, RK3288_PMU_PWRMODE_CON, mode_set);
+
+	regmap_write(pmu_regmap, RK3288_PMU_PWRMODE_CON1, mode_set1);
+}
+
+static void rk3288_slp_mode_set_resume(void)
+{
+	regmap_write(pmu_regmap, RK3288_PMU_PWRMODE_CON,
+		     rk3288_pmu_pwr_mode_con);
+
+	regmap_write(sgrf_regmap, RK3288_SGRF_SOC_CON0,
+		     rk3288_sgrf_soc_con0 | BIT(SGRF_FAST_BOOT_EN + 16));
+}
+
+/*
+ * level: used to control the level of sleep mode.
+ * ROCKCHIP_ARM_OFF_LOGIC_NORMAL : arm is off and logic is in normal
+ * sleep mode.
+ * ROCKCHIP_ARM_OFF_LOGIC_DEEP : arm is off and logic is in deep sleep mode
+ */
+static void rk3288_save_settings(u32 level)
+{
+	rk3288_fill_in_bootram(level);
+
+	rk3288_slp_mode_set(level);
+}
+
+static void rk3288_restore_settings(void)
+{
+	rk3288_slp_mode_set_resume();
+}
+
+static int rockchip_lpmode_enter(unsigned long arg)
+{
+	flush_cache_all();
+
+	cpu_do_idle();
+
+	pr_info("Failed to suspend the system\n");
+
+	return 1;
+}
+
+static int rk3288_suspend_enter(suspend_state_t state)
+{
+	local_fiq_disable();
+
+	rk3288_save_settings(ROCKCHIP_ARM_OFF_LOGIC_NORMAL);
+
+	cpu_suspend(0, rockchip_lpmode_enter);
+
+	rk3288_restore_settings();
+
+	local_fiq_enable();
+
+	return 0;
+}
+
+static int rk3288_suspend_prepare(void)
+{
+	return regulator_suspend_prepare(PM_SUSPEND_MEM);
+}
+
+static void rk3288_suspend_finish(void)
+{
+	if (regulator_suspend_finish())
+		pr_warn("suspend finish failed\n");
+}
+
+static int rk3288_suspend_iomap(void)
+{
+	struct device_node *node;
+	struct resource res;
+
+	node = of_find_compatible_node(NULL, NULL, "rockchip,rk3288-pmu-sram");
+	if (!node) {
+		pr_err("%s: could not find bootram dt node\n", __func__);
+		return -1;
+	}
+
+	rk3288_bootram_base = of_iomap(node, 0);
+	if (!rk3288_bootram_base) {
+		pr_err("%s: could not map bootram base\n", __func__);
+		return -1;
+	}
+
+	if (of_address_to_resource(node, 0, &res)) {
+		pr_err("%s: could not get bootram phy addr\n", __func__);
+		return -1;
+	}
+
+	rk3288_bootram_phy = res.start;
+
+	/* copy resume code and data to bootsram */
+	memcpy(rk3288_bootram_base, rockchip_slp_cpu_resume,
+	       rk3288_bootram_sz);
+
+	return 0;
+}
+
+static int rk3288_suspend_init(void)
+{
+	int ret;
+
+	pmu_regmap = syscon_regmap_lookup_by_compatible(
+				"rockchip,rk3288-pmu");
+
+	if (IS_ERR(pmu_regmap)) {
+		pr_err("%s: could not find pmu regmap\n", __func__);
+		return -1;
+	}
+
+	sgrf_regmap = syscon_regmap_lookup_by_compatible(
+				"rockchip,rk3288-sgrf");
+
+	if (IS_ERR(sgrf_regmap)) {
+		pr_err("%s: could not find sgrf regmap\n", __func__);
+		return -1;
+	}
+
+	ret = rk3288_suspend_iomap();
+
+	return ret;
+}
+
+static const struct platform_suspend_ops rk3288_suspend_ops = {
+	.enter   = rk3288_suspend_enter,
+	.valid   = suspend_valid_only_mem,
+	.prepare = rk3288_suspend_prepare,
+	.finish  = rk3288_suspend_finish,
+};
+
+static const struct rockchip_pm_device_id rockchip_pm_dt_match[] __initconst = {
+	{
+		.compatible = "rockchip,rk3288",
+		.ops = &rk3288_suspend_ops,
+		.init = rk3288_suspend_init,
+	},
+	{ /* sentinel */ },
+};
+
+void __init rockchip_suspend_init(void)
+{
+	const struct rockchip_pm_device_id *matches =
+		rockchip_pm_dt_match;
+
+	while (matches->compatible && matches->ops) {
+		if (of_machine_is_compatible(matches->compatible))
+			break;
+		matches++;
+	}
+
+	if (!matches->compatible || !matches->ops) {
+		pr_err("%s:there is not a machine matched\n", __func__);
+		return;
+	}
+
+	if (matches->init) {
+		if (matches->init()) {
+			pr_err("%s: matches init error\n", __func__);
+			return;
+		}
+	}
+
+	suspend_set_ops(matches->ops);
+}
diff --git a/arch/arm/mach-rockchip/pm.h b/arch/arm/mach-rockchip/pm.h
new file mode 100644
index 0000000..781ea9d
--- /dev/null
+++ b/arch/arm/mach-rockchip/pm.h
@@ -0,0 +1,103 @@
+/*
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ * Author: Tony Xie <tony.xie@rock-chips.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#ifndef __MACH_ROCKCHIP_PM_H
+#define __MACH_ROCKCHIP_PM_H
+
+extern unsigned long rkpm_bootdata_cpusp;
+extern unsigned long rkpm_bootdata_cpu_code;
+extern unsigned long rkpm_bootdata_l2ctlr_f;
+extern unsigned long rkpm_bootdata_l2ctlr;
+extern unsigned long rkpm_bootdata_ddr_code;
+extern unsigned long rkpm_bootdata_ddr_data;
+extern unsigned long rk3288_bootram_sz;
+/* The function selction of low power mode */
+/* arm is off and logic is in normal sleep mode */
+#define ROCKCHIP_ARM_OFF_LOGIC_NORMAL (0)
+/* arm is off and logic is in deep sleep mode */
+#define ROCKCHIP_ARM_OFF_LOGIC_DEEP (1)
+
+void rockchip_slp_cpu_resume(void);
+void __init rockchip_suspend_init(void);
+
+/****** following is rk3288 defined **********/
+#define RK3288_PMU_WAKEUP_CFG0          0x00
+#define RK3288_PMU_WAKEUP_CFG1          0x04
+#define RK3288_PMU_PWRMODE_CON          0x18
+#define RK3288_PMU_OSC_CNT              0x20
+#define RK3288_PMU_PLL_CNT              0x24
+#define RK3288_PMU_STABL_CNT            0x28
+#define RK3288_PMU_DDR0IO_PWRON_CNT     0x2c
+#define RK3288_PMU_DDR1IO_PWRON_CNT     0x30
+#define RK3288_PMU_CORE_PWRDWN_CNT      0x34
+#define RK3288_PMU_CORE_PWRUP_CNT       0x38
+#define RK3288_PMU_GPU_PWRDWN_CNT       0x3c
+#define RK3288_PMU_GPU_PWRUP_CNT        0x40
+#define RK3288_PMU_WAKEUP_RST_CLR_CNT   0x44
+#define RK3288_PMU_PWRMODE_CON1         0x90
+
+#define RK3288_SGRF_SOC_CON0            (0x0000)
+#define RK3288_SGRF_FAST_BOOT_ADDR      (0x0120)
+#define SGRF_FAST_BOOT_EN (8)
+
+#define RK3288_CRU_MODE_CON (0x50)
+#define RK3288_CRU_SEL0_CON (0x60)
+#define RK3288_CRU_SEL1_CON (0x64)
+#define RK3288_CRU_SEL10_CON (0x88)
+#define RK3288_CRU_SEL33_CON (0xe4)
+#define RK3288_CRU_SEL37_CON (0xf4)
+
+/* PMU_WAKEUP_CFG1 bits */
+#define PMU_ARMINT_WAKEUP_EN BIT(0)
+
+enum rk3288_pwr_mode_con {
+	PMU_PWR_MODE_EN = 0,
+	PMU_CLK_CORE_SRC_GATE_EN,
+	PMU_GLOBAL_INT_DISABLE,
+	PMU_L2FLUSH_EN,
+	PMU_BUS_PD_EN,
+	PMU_A12_0_PD_EN,
+	PMU_SCU_EN,
+	PMU_PLL_PD_EN,
+	PMU_CHIP_PD_EN, /* POWER OFF PIN ENABLE */
+	PMU_PWROFF_COMB,
+	PMU_ALIVE_USE_LF,
+	PMU_PMU_USE_LF,
+	PMU_OSC_24M_DIS,
+	PMU_INPUT_CLAMP_EN,
+	PMU_WAKEUP_RESET_EN,
+	PMU_SREF0_ENTER_EN,
+	PMU_SREF1_ENTER_EN,
+	PMU_DDR0IO_RET_EN,
+	PMU_DDR1IO_RET_EN,
+	PMU_DDR0_GATING_EN,
+	PMU_DDR1_GATING_EN,
+	PMU_DDR0IO_RET_DE_REQ,
+	PMU_DDR1IO_RET_DE_REQ
+};
+
+enum rk3288_pwr_mode_con1 {
+	PMU_CLR_BUS = 0,
+	PMU_CLR_CORE,
+	PMU_CLR_CPUP,
+	PMU_CLR_ALIVE,
+	PMU_CLR_DMA,
+	PMU_CLR_PERI,
+	PMU_CLR_GPU,
+	PMU_CLR_VIDEO,
+	PMU_CLR_HEVC,
+	PMU_CLR_VIO,
+};
+
+#endif /* __MACH_ROCKCHIP_PM_H */
diff --git a/arch/arm/mach-rockchip/rockchip.c b/arch/arm/mach-rockchip/rockchip.c
index 8ab9e0e..03ca842 100644
--- a/arch/arm/mach-rockchip/rockchip.c
+++ b/arch/arm/mach-rockchip/rockchip.c
@@ -23,6 +23,13 @@
 #include <asm/mach/map.h>
 #include <asm/hardware/cache-l2x0.h>
 #include "core.h"
+#include "pm.h"
+
+static void __init rockchip_dt_init(void)
+{
+	rockchip_suspend_init();
+	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
+}
 
 static const char * const rockchip_board_dt_compat[] = {
 	"rockchip,rk2928",
@@ -37,4 +44,5 @@ DT_MACHINE_START(ROCKCHIP_DT, "Rockchip Cortex-A9 (Device Tree)")
 	.l2c_aux_val	= 0,
 	.l2c_aux_mask	= ~0,
 	.dt_compat	= rockchip_board_dt_compat,
+	.init_machine	= rockchip_dt_init,
 MACHINE_END
diff --git a/arch/arm/mach-rockchip/sleep.S b/arch/arm/mach-rockchip/sleep.S
new file mode 100644
index 0000000..e1c5b8c
--- /dev/null
+++ b/arch/arm/mach-rockchip/sleep.S
@@ -0,0 +1,90 @@
+/*
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ * Author: Tony Xie <tony.xie@rock-chips.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#include <linux/linkage.h>
+#include <asm/assembler.h>
+#include <asm/memory.h>
+
+.data
+/*
+ * this code will be copied from
+ * ddr to sram for system resumeing.
+ * so it is ".data section".
+ */
+.align
+
+ENTRY(rockchip_slp_cpu_resume)
+	setmode	PSR_I_BIT | PSR_F_BIT | SVC_MODE, r1  @ set svc, irqs off
+	mrc	p15, 0, r1, c0, c0, 5
+	and	r1, r1, #0xf
+	cmp	r1, #0
+	/* olny cpu0 can continue to run, the others is halt here */
+	beq	cpu0run
+secondary_loop:
+	wfe
+	b	secondary_loop
+cpu0run:
+	ldr	r3, rkpm_bootdata_l2ctlr_f
+	cmp	r3, #0
+	beq	sp_set
+	ldr	r3, rkpm_bootdata_l2ctlr
+	mcr	p15, 1, r3, c9, c0, 2
+sp_set:
+	ldr	sp, rkpm_bootdata_cpusp
+
+	ldr	r1, rkpm_bootdata_ddr_code
+	cmp	r1, #0
+	beq	res
+	ldr	r0, rkpm_bootdata_ddr_data
+	blx	r1
+res:
+	ldr	r1, rkpm_bootdata_cpu_code
+	bx	r1
+ENDPROC(rockchip_slp_cpu_resume)
+
+/* Parameters filled in by the kernel */
+
+/* Code to jump to for DDR resume code, or NULL */
+	.global rkpm_bootdata_ddr_code
+rkpm_bootdata_ddr_code:
+	.long 0
+
+/* Data to pass to DDR resume data */
+	.global rkpm_bootdata_ddr_data
+rkpm_bootdata_ddr_data:
+	.long 0
+
+/* Flag for whether to restore L2CTLR on resume */
+	.global rkpm_bootdata_l2ctlr_f
+rkpm_bootdata_l2ctlr_f:
+	.long 0
+
+/* Saved L2CTLR to restore on resume */
+	.global rkpm_bootdata_l2ctlr
+rkpm_bootdata_l2ctlr:
+	.long 0
+
+/* CPU resume SP addr */
+	.globl rkpm_bootdata_cpusp
+rkpm_bootdata_cpusp:
+	.long 0
+
+/* CPU resume function (physical address) */
+	.globl rkpm_bootdata_cpu_code
+rkpm_bootdata_cpu_code:
+	.long 0
+
+ENTRY(rk3288_bootram_sz)
+        .word   . - rockchip_slp_cpu_resume
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v6 6/7] ARM: dts: add RK3288 suspend support
  2014-10-29 11:51 [PATCH v6 0/7] This is the 1st version of suspend for RK3288 Chris Zhong
                   ` (3 preceding siblings ...)
  2014-10-29 11:52 ` [PATCH v6 4/7] ARM: rockchip: add suspend and resume for RK3288 Chris Zhong
@ 2014-10-29 11:52 ` Chris Zhong
  2014-10-29 11:52 ` [PATCH v6 7/7] ARM: dts: add suspend voltage setting for RK808 Chris Zhong
  2014-10-29 20:06 ` [PATCH v6 0/7] This is the 1st version of suspend for RK3288 Kevin Hilman
  6 siblings, 0 replies; 20+ messages in thread
From: Chris Zhong @ 2014-10-29 11:52 UTC (permalink / raw)
  To: linux-arm-kernel

add pmu sram node for suspend, add global_pwroff pinctrl.
The pmu sram is used to store the resume code.
global_pwroff is held low level at work, it would be pull to high
when entering suspend. reference this in the board DTS file since
some boards need it.

Signed-off-by: Tony Xie <xxx@rock-chips.com>
Signed-off-by: Chris Zhong <zyw@rock-chips.com>
Reviewed-by: Doug Anderson <dianders@chromium.org>
Tested-by: Doug Anderson <dianders@chromium.org>

---

Changes in v6:
- change pmu_intmem at ff720000 to sram at ff720000

Changes in v5:
- change size to 4k

Changes in v4: None
Changes in v3: None
Changes in v2:
- put "rockchip,rk3288-pmu-sram" to first

 arch/arm/boot/dts/rk3288.dtsi |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 874e66d..248c349 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -439,6 +439,11 @@
 		status = "disabled";
 	};
 
+	sram at ff720000 {
+		compatible = "rockchip,rk3288-pmu-sram", "mmio-sram";
+		reg = <0xff720000 0x1000>;
+	};
+
 	pmu: power-management at ff730000 {
 		compatible = "rockchip,rk3288-pmu", "syscon";
 		reg = <0xff730000 0x100>;
@@ -634,6 +639,12 @@
 			bias-disable;
 		};
 
+		sleep {
+			global_pwroff: global-pwroff {
+				rockchip,pins = <0 0 RK_FUNC_1 &pcfg_pull_none>;
+			};
+		};
+
 		i2c0 {
 			i2c0_xfer: i2c0-xfer {
 				rockchip,pins = <0 15 RK_FUNC_1 &pcfg_pull_none>,
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v6 7/7] ARM: dts: add suspend voltage setting for RK808
  2014-10-29 11:51 [PATCH v6 0/7] This is the 1st version of suspend for RK3288 Chris Zhong
                   ` (4 preceding siblings ...)
  2014-10-29 11:52 ` [PATCH v6 6/7] ARM: dts: add RK3288 suspend support Chris Zhong
@ 2014-10-29 11:52 ` Chris Zhong
  2014-10-29 15:40   ` Doug Anderson
  2014-10-29 20:06 ` [PATCH v6 0/7] This is the 1st version of suspend for RK3288 Kevin Hilman
  6 siblings, 1 reply; 20+ messages in thread
From: Chris Zhong @ 2014-10-29 11:52 UTC (permalink / raw)
  To: linux-arm-kernel

global_pwroff would be pull to high when RK3288 entering suspend,
this pin is a sleep signal for RK808, so RK808 could goto sleep
mode, and some regulators would be disable.

Signed-off-by: Chris Zhong <zyw@rock-chips.com>

---

Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

 arch/arm/boot/dts/rk3288-evb-rk808.dts |   16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/rk3288-evb-rk808.dts b/arch/arm/boot/dts/rk3288-evb-rk808.dts
index ff522f8..319fcb6 100644
--- a/arch/arm/boot/dts/rk3288-evb-rk808.dts
+++ b/arch/arm/boot/dts/rk3288-evb-rk808.dts
@@ -27,7 +27,7 @@
 		interrupt-parent = <&gpio0>;
 		interrupts = <4 IRQ_TYPE_LEVEL_LOW>;
 		pinctrl-names = "default";
-		pinctrl-0 = <&pmic_int>;
+		pinctrl-0 = <&pmic_int &global_pwroff>;
 		rockchip,system-power-controller;
 		wakeup-source;
 		#clock-cells = <1>;
@@ -46,6 +46,7 @@
 				regulator-min-microvolt = <750000>;
 				regulator-max-microvolt = <1300000>;
 				regulator-name = "vdd_arm";
+				regulator-suspend-mem-disabled;
 			};
 
 			vdd_gpu: DCDC_REG2 {
@@ -54,12 +55,14 @@
 				regulator-min-microvolt = <850000>;
 				regulator-max-microvolt = <1250000>;
 				regulator-name = "vdd_gpu";
+				regulator-suspend-mem-disabled;
 			};
 
 			vcc_ddr: DCDC_REG3 {
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-name = "vcc_ddr";
+				regulator-suspend-mem-enabled;
 			};
 
 			vcc_io: DCDC_REG4 {
@@ -68,6 +71,7 @@
 				regulator-min-microvolt = <3300000>;
 				regulator-max-microvolt = <3300000>;
 				regulator-name = "vcc_io";
+				regulator-suspend-mem-microvolt = <3300000>;
 			};
 
 			vccio_pmu: LDO_REG1 {
@@ -76,6 +80,7 @@
 				regulator-min-microvolt = <3300000>;
 				regulator-max-microvolt = <3300000>;
 				regulator-name = "vccio_pmu";
+				regulator-suspend-mem-microvolt = <3300000>;
 			};
 
 			vcc_tp: LDO_REG2 {
@@ -84,6 +89,7 @@
 				regulator-min-microvolt = <3300000>;
 				regulator-max-microvolt = <3300000>;
 				regulator-name = "vcc_tp";
+				regulator-suspend-mem-disabled;
 			};
 
 			vdd_10: LDO_REG3 {
@@ -92,6 +98,7 @@
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-name = "vdd_10";
+				regulator-suspend-mem-microvolt = <1000000>;
 			};
 
 			vcc18_lcd: LDO_REG4 {
@@ -100,6 +107,7 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-name = "vcc18_lcd";
+				regulator-suspend-mem-disabled;
 			};
 
 			vccio_sd: LDO_REG5 {
@@ -108,6 +116,7 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <3300000>;
 				regulator-name = "vccio_sd";
+				regulator-suspend-mem-disabled;
 			};
 
 			vdd10_lcd: LDO_REG6 {
@@ -116,6 +125,7 @@
 				regulator-min-microvolt = <1000000>;
 				regulator-max-microvolt = <1000000>;
 				regulator-name = "vdd10_lcd";
+				regulator-suspend-mem-disabled;
 			};
 
 			vcc_18: LDO_REG7 {
@@ -124,6 +134,7 @@
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <1800000>;
 				regulator-name = "vcc_18";
+				regulator-suspend-mem-microvolt = <1800000>;
 			};
 
 			vcca_codec: LDO_REG8 {
@@ -132,18 +143,21 @@
 				regulator-min-microvolt = <3300000>;
 				regulator-max-microvolt = <3300000>;
 				regulator-name = "vcca_codec";
+				regulator-suspend-mem-disabled;
 			};
 
 			vcc_wl: SWITCH_REG1 {
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-name = "vcc_wl";
+				regulator-suspend-mem-enabled;
 			};
 
 			vcc_lcd: SWITCH_REG2 {
 				regulator-always-on;
 				regulator-boot-on;
 				regulator-name = "vcc_lcd";
+				regulator-suspend-mem-disabled;
 			};
 		};
 	};
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v6 7/7] ARM: dts: add suspend voltage setting for RK808
  2014-10-29 11:52 ` [PATCH v6 7/7] ARM: dts: add suspend voltage setting for RK808 Chris Zhong
@ 2014-10-29 15:40   ` Doug Anderson
  2014-10-29 16:29     ` Javier Martinez Canillas
  0 siblings, 1 reply; 20+ messages in thread
From: Doug Anderson @ 2014-10-29 15:40 UTC (permalink / raw)
  To: linux-arm-kernel

Chris,

On Wed, Oct 29, 2014 at 4:52 AM, Chris Zhong <zyw@rock-chips.com> wrote:
> global_pwroff would be pull to high when RK3288 entering suspend,
> this pin is a sleep signal for RK808, so RK808 could goto sleep
> mode, and some regulators would be disable.
>
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
>
> ---
>
> Changes in v6: None
> Changes in v5: None
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
>
>  arch/arm/boot/dts/rk3288-evb-rk808.dts |   16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/rk3288-evb-rk808.dts b/arch/arm/boot/dts/rk3288-evb-rk808.dts
> index ff522f8..319fcb6 100644
> --- a/arch/arm/boot/dts/rk3288-evb-rk808.dts
> +++ b/arch/arm/boot/dts/rk3288-evb-rk808.dts
> @@ -27,7 +27,7 @@
>                 interrupt-parent = <&gpio0>;
>                 interrupts = <4 IRQ_TYPE_LEVEL_LOW>;
>                 pinctrl-names = "default";
> -               pinctrl-0 = <&pmic_int>;
> +               pinctrl-0 = <&pmic_int &global_pwroff>;
>                 rockchip,system-power-controller;
>                 wakeup-source;
>                 #clock-cells = <1>;
> @@ -46,6 +46,7 @@
>                                 regulator-min-microvolt = <750000>;
>                                 regulator-max-microvolt = <1300000>;
>                                 regulator-name = "vdd_arm";
> +                               regulator-suspend-mem-disabled;

NAK.  "regulator-suspend-mem-disabled" is a local property added by
local Chrome OS patches and doesn't belong in an upstream submission.

You should using the new patches from Chanwoo.

40e20d6 regulator: of: Add support for parsing regulator_state for suspend state
291d761 regulator: Document binding for regulator suspend state for PM state

In your case, it would look like:

regulator-state-mem {
  regulator-off-in-suspend;
};

> @@ -76,6 +80,7 @@
>                                 regulator-min-microvolt = <3300000>;
>                                 regulator-max-microvolt = <3300000>;
>                                 regulator-name = "vccio_pmu";
> +                               regulator-suspend-mem-microvolt = <3300000>;

Similarly this property isn't upstream.  You can see Javier's work on
this in <https://patchwork.kernel.org/patch/5106351/> and I think
you'll need an rk808-specific patch just like he needs an max77802
patch.  You probably want to wait for him to spin it first, though,
since Mark had feedback on his last patch.

-Doug

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v6 7/7] ARM: dts: add suspend voltage setting for RK808
  2014-10-29 15:40   ` Doug Anderson
@ 2014-10-29 16:29     ` Javier Martinez Canillas
  2014-10-29 16:51       ` Doug Anderson
  0 siblings, 1 reply; 20+ messages in thread
From: Javier Martinez Canillas @ 2014-10-29 16:29 UTC (permalink / raw)
  To: linux-arm-kernel

Hello Chris,

On 10/29/2014 04:40 PM, Doug Anderson wrote:
>>                                 regulator-min-microvolt = <750000>;
>>                                 regulator-max-microvolt = <1300000>;
>>                                 regulator-name = "vdd_arm";
>> +                               regulator-suspend-mem-disabled;
> 
> NAK.  "regulator-suspend-mem-disabled" is a local property added by
> local Chrome OS patches and doesn't belong in an upstream submission.
> 
> You should using the new patches from Chanwoo.
> 
> 40e20d6 regulator: of: Add support for parsing regulator_state for suspend state
> 291d761 regulator: Document binding for regulator suspend state for PM state
> 
> In your case, it would look like:
> 
> regulator-state-mem {
>   regulator-off-in-suspend;
> };

Agreed.

> 
>> @@ -76,6 +80,7 @@
>>                                 regulator-min-microvolt = <3300000>;
>>                                 regulator-max-microvolt = <3300000>;
>>                                 regulator-name = "vccio_pmu";
>> +                               regulator-suspend-mem-microvolt = <3300000>;
> 
> Similarly this property isn't upstream.  You can see Javier's work on
> this in <https://patchwork.kernel.org/patch/5106351/> and I think
> you'll need an rk808-specific patch just like he needs an max77802
> patch.  You probably want to wait for him to spin it first, though,
> since Mark had feedback on his last patch.
>

I'm working on adding support to configure the regulator mode on startup
and when the system enters in a suspend state.

As Doug said I've to re-spin since Mark wanted things to be more integrated
with the core. So I'm doing some refactoring to pass the static regulator
descriptor to the function extracting the regulator initial data so drivers
should only define a function handler that does the modes translation.
I believe this is the more sensible place to add the mapping function since
the modes translation should be a non-varying property of the regulator.
 
Having said that, I see a different use case here. You want to set a voltage
on system suspend. But the value is the same that is set to your fixed
regulator so I wonder if what you want here is to enable the regulator on
suspend instead?

In other words, do you want the core to call rk808_set_suspend_voltage() or
rk808_set_suspend_enable()? If the later then you can use Chanwoo's bindings:

regulator-state-mem {
   regulator-on-in-suspend;
};

If you want the former then you should look at Chanwoo's previous series that
had support to define the voltage for regulators during a suspend state [0]

Mark had some questions about how that should play with the regulator voltage
range [1] but was not against the idea AFAIU.

Best regards,
Javier

[0]: https://lkml.org/lkml/2014/8/18/22
[1]: https://lkml.org/lkml/2014/9/4/651

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v6 7/7] ARM: dts: add suspend voltage setting for RK808
  2014-10-29 16:29     ` Javier Martinez Canillas
@ 2014-10-29 16:51       ` Doug Anderson
  2014-10-30  0:32         ` Javier Martinez Canillas
  0 siblings, 1 reply; 20+ messages in thread
From: Doug Anderson @ 2014-10-29 16:51 UTC (permalink / raw)
  To: linux-arm-kernel

Javier,

On Wed, Oct 29, 2014 at 9:29 AM, Javier Martinez Canillas
<javier.martinez@collabora.co.uk> wrote:
>>> @@ -76,6 +80,7 @@
>>>                                 regulator-min-microvolt = <3300000>;
>>>                                 regulator-max-microvolt = <3300000>;
>>>                                 regulator-name = "vccio_pmu";
>>> +                               regulator-suspend-mem-microvolt = <3300000>;
>>
>> Similarly this property isn't upstream.  You can see Javier's work on
>> this in <https://patchwork.kernel.org/patch/5106351/> and I think
>> you'll need an rk808-specific patch just like he needs an max77802
>> patch.  You probably want to wait for him to spin it first, though,
>> since Mark had feedback on his last patch.
>>
>
> I'm working on adding support to configure the regulator mode on startup
> and when the system enters in a suspend state.
>
> As Doug said I've to re-spin since Mark wanted things to be more integrated
> with the core. So I'm doing some refactoring to pass the static regulator
> descriptor to the function extracting the regulator initial data so drivers
> should only define a function handler that does the modes translation.
> I believe this is the more sensible place to add the mapping function since
> the modes translation should be a non-varying property of the regulator.
>
> Having said that, I see a different use case here. You want to set a voltage
> on system suspend. But the value is the same that is set to your fixed
> regulator so I wonder if what you want here is to enable the regulator on
> suspend instead?
>
> In other words, do you want the core to call rk808_set_suspend_voltage() or
> rk808_set_suspend_enable()? If the later then you can use Chanwoo's bindings:
>
> regulator-state-mem {
>    regulator-on-in-suspend;
> };

The rk808 has a separate register for storing the sleep voltage,
though.  ...so just using "regulator-on-in-suspend" without any
changes to the rk808 driver won't get us this "for free".  I'd expect
to need

 regulator-state-mem {
    regulator-on-in-suspend;
    regulator-suspend-microvolts = <3300000>;
 };

IMHO it wouldn't be insane to say that if someone has
"regulator-on-in-suspend" but no suspend voltage defined that it
should carry forward the runtime voltage.  I actually argued for that
earlier and Mark Brown said "no" at
<http://marc.info/?l=linux-kernel&m=141277262106368&w=2>.  More at
<http://marc.info/?l=linux-kernel&m=141279245027919&w=2>.  It's
entirely possible I wasn't making myself clear, though.

-Doug

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v6 4/7] ARM: rockchip: add suspend and resume for RK3288
  2014-10-29 11:52 ` [PATCH v6 4/7] ARM: rockchip: add suspend and resume for RK3288 Chris Zhong
@ 2014-10-29 19:01   ` Kevin Hilman
  2014-10-29 19:26     ` Doug Anderson
  2014-11-11  7:33     ` Chris Zhong
  0 siblings, 2 replies; 20+ messages in thread
From: Kevin Hilman @ 2014-10-29 19:01 UTC (permalink / raw)
  To: linux-arm-kernel

Chris Zhong <zyw@rock-chips.com> writes:

> It's a basic version of suspend and resume for rockchip,
> it only support RK3288 now.
>
> Signed-off-by: Tony Xie <xxx@rock-chips.com>
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> Tested-by: Doug Anderson <dianders@chromium.org>
> Reviewed-by: Doug Anderson <dianders@chromium.org>
>
> ---
>
> Changes in v6:
> - get rid of the save/restore of SRAM
> - doing the copy of resume code once at init time
> - remove ROCKCHIP_ARM_OFF_LOGIC_DEEP from rk3288_fill_in_bootram
> - add of_platform_populate in rockchip_dt_init
>
> Changes in v5:
> - use rk3288_bootram_sz for memcpy size
> - fixed error of sram save and restore
>
> Changes in v4:
> - remove grf regmap
>
> Changes in v3:
> - move the pinmux of gpio6_c6 save and restore to pinctrl-rockchip
>
> Changes in v2:
> - add the regulator calls in prepare and finish.
> - add the pinmux of gpio6_c6 save and restore
>
>  arch/arm/mach-rockchip/Makefile   |    1 +
>  arch/arm/mach-rockchip/pm.c       |  278 +++++++++++++++++++++++++++++++++++++
>  arch/arm/mach-rockchip/pm.h       |  103 ++++++++++++++
>  arch/arm/mach-rockchip/rockchip.c |    8 ++
>  arch/arm/mach-rockchip/sleep.S    |   90 ++++++++++++
>  5 files changed, 480 insertions(+)
>  create mode 100644 arch/arm/mach-rockchip/pm.c
>  create mode 100644 arch/arm/mach-rockchip/pm.h
>  create mode 100644 arch/arm/mach-rockchip/sleep.S
>
> diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile
> index b29d8ea..5c3a9b2 100644
> --- a/arch/arm/mach-rockchip/Makefile
> +++ b/arch/arm/mach-rockchip/Makefile
> @@ -1,4 +1,5 @@
>  CFLAGS_platsmp.o := -march=armv7-a
>  
>  obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip.o
> +obj-$(CONFIG_PM_SLEEP) += pm.o sleep.o
>  obj-$(CONFIG_SMP) += headsmp.o platsmp.o
> diff --git a/arch/arm/mach-rockchip/pm.c b/arch/arm/mach-rockchip/pm.c
> new file mode 100644
> index 0000000..0cf3128
> --- /dev/null
> +++ b/arch/arm/mach-rockchip/pm.c
> @@ -0,0 +1,278 @@
> +/*
> + * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
> + * Author: Tony Xie <tony.xie@rock-chips.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + */
> +
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/regmap.h>
> +#include <linux/suspend.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/regulator/machine.h>
> +
> +#include <asm/cacheflush.h>
> +#include <asm/tlbflush.h>
> +#include <asm/suspend.h>
> +
> +#include "pm.h"
> +
> +struct rockchip_pm_device_id {
> +	const char *compatible;
> +	const struct platform_suspend_ops *ops;
> +	int (*init)(void);
> +};
> +
> +static void __iomem *rk3288_bootram_base;
> +static phys_addr_t rk3288_bootram_phy;
> +
> +static struct regmap *pmu_regmap;
> +static struct regmap *sgrf_regmap;
> +
> +static inline u32 rk3288_l2_config(void)
> +{
> +	u32 l2ctlr;
> +
> +	asm("mrc p15, 1, %0, c9, c0, 2" : "=r" (l2ctlr));
> +	return l2ctlr;
> +}
> +

> +static void rk3288_fill_in_bootram(u32 level)

This function name no longer reflects what it does...

> +{
> +	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();
> +
> +	rkpm_bootdata_ddr_code = 0;

Already initialized to 0 in the resume code.

> +}

In addition, you're no longer copying the resume code to SRAM, but this
function writes values into the version of resume code that's in memory.
IOW, this function is being called for every suspend cycle, but has no
effect since the resume code has already been copied to SRAM.

Seems to me like this whole 'fill_in_bootram()' needs to go away, and be
replaced by a single setup at init time.

> +static u32 rk3288_pmu_pwr_mode_con;
> +static u32 rk3288_sgrf_soc_con0;

Minor nit, and it's up to Heiko's preference here, but it's generally
preferred to have global variables like  this at the top of the file,
not interspersed with the code.

Also, I'm not sure if you expect the number of registers that need
saving to grow here, but regmap provides a regcache facility for keeping
copies of things that may be useful.  Probably not worth the over head
for a single register in each of these regmaps, but worth exploring if
this is going to grow.

> +static void rk3288_slp_mode_set(int level)
> +{
> +	u32 mode_set, mode_set1;
> +
> +	regmap_read(sgrf_regmap, RK3288_SGRF_SOC_CON0, &rk3288_sgrf_soc_con0);
> +
> +	regmap_read(pmu_regmap, RK3288_PMU_PWRMODE_CON,
> +		    &rk3288_pmu_pwr_mode_con);
> +
> +	/* set bit 8 so that system will resume to FAST_BOOT_ADDR */
> +	regmap_write(sgrf_regmap, RK3288_SGRF_SOC_CON0,
> +		     BIT(SGRF_FAST_BOOT_EN) | BIT(SGRF_FAST_BOOT_EN + 16));

Comment says "bit 8", but code says bit 8 and bit 24, and if bit 24 is
needed, it should probably get its own #define in the header.

> +
> +	/* booting address of resuming system is from this register value */
> +	regmap_write(sgrf_regmap, RK3288_SGRF_FAST_BOOT_ADDR,
> +		     rk3288_bootram_phy);
> +
> +	regmap_write(pmu_regmap, RK3288_PMU_WAKEUP_CFG1,
> +		     PMU_ARMINT_WAKEUP_EN);
> +
> +	mode_set = BIT(PMU_GLOBAL_INT_DISABLE) | BIT(PMU_L2FLUSH_EN) |
> +		   BIT(PMU_SREF0_ENTER_EN) | BIT(PMU_SREF1_ENTER_EN) |
> +		   BIT(PMU_DDR0_GATING_EN) | BIT(PMU_DDR1_GATING_EN) |
> +		   BIT(PMU_PWR_MODE_EN) | BIT(PMU_CHIP_PD_EN) |
> +		   BIT(PMU_SCU_EN);
> +
> +	mode_set1 = BIT(PMU_CLR_CORE) | BIT(PMU_CLR_CPUP);
> +
> +	if (level == ROCKCHIP_ARM_OFF_LOGIC_DEEP) {
> +		/* arm off, logic deep sleep */
> +		mode_set |= BIT(PMU_BUS_PD_EN) |
> +			    BIT(PMU_DDR1IO_RET_EN) | BIT(PMU_DDR0IO_RET_EN) |
> +			    BIT(PMU_OSC_24M_DIS) | BIT(PMU_PMU_USE_LF) |
> +			    BIT(PMU_ALIVE_USE_LF) | BIT(PMU_PLL_PD_EN);
> +
> +		mode_set1 |= BIT(PMU_CLR_ALIVE) | BIT(PMU_CLR_BUS) |
> +			     BIT(PMU_CLR_PERI) | BIT(PMU_CLR_DMA);
> +
> +	} else {
> +		/*
> +		 * arm off, logic normal
> +		 * if pmu_clk_core_src_gate_en is not set,
> +		 * wakeup will be error
> +		 */
> +		mode_set |= BIT(PMU_CLK_CORE_SRC_GATE_EN);
> +	}
> +
> +	regmap_write(pmu_regmap, RK3288_PMU_PWRMODE_CON, mode_set);
> +

extra blank line not needed

> +	regmap_write(pmu_regmap, RK3288_PMU_PWRMODE_CON1, mode_set1);
> +}
> +
> +static void rk3288_slp_mode_set_resume(void)
> +{
> +	regmap_write(pmu_regmap, RK3288_PMU_PWRMODE_CON,
> +		     rk3288_pmu_pwr_mode_con);

extra blank line

> +	regmap_write(sgrf_regmap, RK3288_SGRF_SOC_CON0,
> +		     rk3288_sgrf_soc_con0 | BIT(SGRF_FAST_BOOT_EN + 16));

here is magic bit 24 again.

> +}
> +
> +/*
> + * level: used to control the level of sleep mode.
> + * ROCKCHIP_ARM_OFF_LOGIC_NORMAL : arm is off and logic is in normal
> + * sleep mode.
> + * ROCKCHIP_ARM_OFF_LOGIC_DEEP : arm is off and logic is in deep sleep mode

IMO, these descriptions don't add any value over the already descriptive
names of the #defines

> + */
> +static void rk3288_save_settings(u32 level)
> +{
> +	rk3288_fill_in_bootram(level);
> +
> +	rk3288_slp_mode_set(level);
> +}
> +
> +static void rk3288_restore_settings(void)
> +{
> +	rk3288_slp_mode_set_resume();
> +}

These 'save' and 'restore' functions seem like unnecessary levels of
redirection, especially if 'fill_in_bootram' goes away.  

IMO, probably best to just move the calls into _suspend_enter().

> +static int rockchip_lpmode_enter(unsigned long arg)
> +{
> +	flush_cache_all();
> +
> +	cpu_do_idle();
> +
> +	pr_info("Failed to suspend the system\n");
> +
> +	return 1;
> +}
> +
> +static int rk3288_suspend_enter(suspend_state_t state)
> +{
> +	local_fiq_disable();
> +
> +	rk3288_save_settings(ROCKCHIP_ARM_OFF_LOGIC_NORMAL);
> +
> +	cpu_suspend(0, rockchip_lpmode_enter);
> +
> +	rk3288_restore_settings();
> +
> +	local_fiq_enable();
> +
> +	return 0;
> +}
> +
> +static int rk3288_suspend_prepare(void)
> +{
> +	return regulator_suspend_prepare(PM_SUSPEND_MEM);
> +}
> +
> +static void rk3288_suspend_finish(void)
> +{
> +	if (regulator_suspend_finish())
> +		pr_warn("suspend finish failed\n");
> +}
> +
> +static int rk3288_suspend_iomap(void)
> +{
> +	struct device_node *node;
> +	struct resource res;
> +
> +	node = of_find_compatible_node(NULL, NULL, "rockchip,rk3288-pmu-sram");
> +	if (!node) {
> +		pr_err("%s: could not find bootram dt node\n", __func__);
> +		return -1;
> +	}
> +
> +	rk3288_bootram_base = of_iomap(node, 0);
> +	if (!rk3288_bootram_base) {
> +		pr_err("%s: could not map bootram base\n", __func__);
> +		return -1;
> +	}
> +
> +	if (of_address_to_resource(node, 0, &res)) {
> +		pr_err("%s: could not get bootram phy addr\n", __func__);
> +		return -1;
> +	}
> +
> +	rk3288_bootram_phy = res.start;
> +
> +	/* copy resume code and data to bootsram */
> +	memcpy(rk3288_bootram_base, rockchip_slp_cpu_resume,
> +	       rk3288_bootram_sz);
> +
> +	return 0;
> +}
> +
> +static int rk3288_suspend_init(void)
> +{
> +	int ret;
> +
> +	pmu_regmap = syscon_regmap_lookup_by_compatible(
> +				"rockchip,rk3288-pmu");
> +
> +	if (IS_ERR(pmu_regmap)) {
> +		pr_err("%s: could not find pmu regmap\n", __func__);
> +		return -1;
> +	}
> +
> +	sgrf_regmap = syscon_regmap_lookup_by_compatible(
> +				"rockchip,rk3288-sgrf");
> +
> +	if (IS_ERR(sgrf_regmap)) {
> +		pr_err("%s: could not find sgrf regmap\n", __func__);
> +		return -1;
> +	}
> +
> +	ret = rk3288_suspend_iomap();
> +
> +	return ret;
> +}
> +
> +static const struct platform_suspend_ops rk3288_suspend_ops = {
> +	.enter   = rk3288_suspend_enter,
> +	.valid   = suspend_valid_only_mem,
> +	.prepare = rk3288_suspend_prepare,
> +	.finish  = rk3288_suspend_finish,
> +};
> +
> +static const struct rockchip_pm_device_id rockchip_pm_dt_match[] __initconst = {
> +	{
> +		.compatible = "rockchip,rk3288",
> +		.ops = &rk3288_suspend_ops,
> +		.init = rk3288_suspend_init,
> +	},
> +	{ /* sentinel */ },
> +};
> +
> +void __init rockchip_suspend_init(void)
> +{
> +	const struct rockchip_pm_device_id *matches =
> +		rockchip_pm_dt_match;
> +
> +	while (matches->compatible && matches->ops) {
> +		if (of_machine_is_compatible(matches->compatible))
> +			break;
> +		matches++;
> +	}
> +
> +	if (!matches->compatible || !matches->ops) {
> +		pr_err("%s:there is not a machine matched\n", __func__);
> +		return;
> +	}
> +
> +	if (matches->init) {
> +		if (matches->init()) {
> +			pr_err("%s: matches init error\n", __func__);
> +			return;
> +		}
> +	}
> +
> +	suspend_set_ops(matches->ops);
> +}
> diff --git a/arch/arm/mach-rockchip/pm.h b/arch/arm/mach-rockchip/pm.h
> new file mode 100644
> index 0000000..781ea9d
> --- /dev/null
> +++ b/arch/arm/mach-rockchip/pm.h
> @@ -0,0 +1,103 @@
> +/*
> + * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
> + * Author: Tony Xie <tony.xie@rock-chips.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + */
> +
> +#ifndef __MACH_ROCKCHIP_PM_H
> +#define __MACH_ROCKCHIP_PM_H
> +
> +extern unsigned long rkpm_bootdata_cpusp;
> +extern unsigned long rkpm_bootdata_cpu_code;
> +extern unsigned long rkpm_bootdata_l2ctlr_f;
> +extern unsigned long rkpm_bootdata_l2ctlr;
> +extern unsigned long rkpm_bootdata_ddr_code;
> +extern unsigned long rkpm_bootdata_ddr_data;
> +extern unsigned long rk3288_bootram_sz;
> +/* The function selction of low power mode */
> +/* arm is off and logic is in normal sleep mode */
> +#define ROCKCHIP_ARM_OFF_LOGIC_NORMAL (0)
> +/* arm is off and logic is in deep sleep mode */
> +#define ROCKCHIP_ARM_OFF_LOGIC_DEEP (1)

These defines only seem to be used inside pm.c, so should be moved there
(and maybe turned into an enum?)

> +void rockchip_slp_cpu_resume(void);
> +void __init rockchip_suspend_init(void);
> +
> +/****** following is rk3288 defined **********/
> +#define RK3288_PMU_WAKEUP_CFG0          0x00
> +#define RK3288_PMU_WAKEUP_CFG1          0x04
> +#define RK3288_PMU_PWRMODE_CON          0x18
> +#define RK3288_PMU_OSC_CNT              0x20
> +#define RK3288_PMU_PLL_CNT              0x24
> +#define RK3288_PMU_STABL_CNT            0x28
> +#define RK3288_PMU_DDR0IO_PWRON_CNT     0x2c
> +#define RK3288_PMU_DDR1IO_PWRON_CNT     0x30
> +#define RK3288_PMU_CORE_PWRDWN_CNT      0x34
> +#define RK3288_PMU_CORE_PWRUP_CNT       0x38
> +#define RK3288_PMU_GPU_PWRDWN_CNT       0x3c
> +#define RK3288_PMU_GPU_PWRUP_CNT        0x40
> +#define RK3288_PMU_WAKEUP_RST_CLR_CNT   0x44
> +#define RK3288_PMU_PWRMODE_CON1         0x90
> +
> +#define RK3288_SGRF_SOC_CON0            (0x0000)
> +#define RK3288_SGRF_FAST_BOOT_ADDR      (0x0120)
> +#define SGRF_FAST_BOOT_EN (8)
> +
> +#define RK3288_CRU_MODE_CON (0x50)
> +#define RK3288_CRU_SEL0_CON (0x60)
> +#define RK3288_CRU_SEL1_CON (0x64)
> +#define RK3288_CRU_SEL10_CON (0x88)
> +#define RK3288_CRU_SEL33_CON (0xe4)
> +#define RK3288_CRU_SEL37_CON (0xf4)
> +
> +/* PMU_WAKEUP_CFG1 bits */
> +#define PMU_ARMINT_WAKEUP_EN BIT(0)
> +
> +enum rk3288_pwr_mode_con {
> +	PMU_PWR_MODE_EN = 0,
> +	PMU_CLK_CORE_SRC_GATE_EN,
> +	PMU_GLOBAL_INT_DISABLE,
> +	PMU_L2FLUSH_EN,
> +	PMU_BUS_PD_EN,
> +	PMU_A12_0_PD_EN,
> +	PMU_SCU_EN,
> +	PMU_PLL_PD_EN,
> +	PMU_CHIP_PD_EN, /* POWER OFF PIN ENABLE */
> +	PMU_PWROFF_COMB,
> +	PMU_ALIVE_USE_LF,
> +	PMU_PMU_USE_LF,
> +	PMU_OSC_24M_DIS,
> +	PMU_INPUT_CLAMP_EN,
> +	PMU_WAKEUP_RESET_EN,
> +	PMU_SREF0_ENTER_EN,
> +	PMU_SREF1_ENTER_EN,
> +	PMU_DDR0IO_RET_EN,
> +	PMU_DDR1IO_RET_EN,
> +	PMU_DDR0_GATING_EN,
> +	PMU_DDR1_GATING_EN,
> +	PMU_DDR0IO_RET_DE_REQ,
> +	PMU_DDR1IO_RET_DE_REQ
> +};
> +
> +enum rk3288_pwr_mode_con1 {
> +	PMU_CLR_BUS = 0,
> +	PMU_CLR_CORE,
> +	PMU_CLR_CPUP,
> +	PMU_CLR_ALIVE,
> +	PMU_CLR_DMA,
> +	PMU_CLR_PERI,
> +	PMU_CLR_GPU,
> +	PMU_CLR_VIDEO,
> +	PMU_CLR_HEVC,
> +	PMU_CLR_VIO,
> +};
> +
> +#endif /* __MACH_ROCKCHIP_PM_H */
> diff --git a/arch/arm/mach-rockchip/rockchip.c b/arch/arm/mach-rockchip/rockchip.c
> index 8ab9e0e..03ca842 100644
> --- a/arch/arm/mach-rockchip/rockchip.c
> +++ b/arch/arm/mach-rockchip/rockchip.c
> @@ -23,6 +23,13 @@
>  #include <asm/mach/map.h>
>  #include <asm/hardware/cache-l2x0.h>
>  #include "core.h"
> +#include "pm.h"
> +
> +static void __init rockchip_dt_init(void)
> +{
> +	rockchip_suspend_init();
> +	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> +}
>  
>  static const char * const rockchip_board_dt_compat[] = {
>  	"rockchip,rk2928",
> @@ -37,4 +44,5 @@ DT_MACHINE_START(ROCKCHIP_DT, "Rockchip Cortex-A9 (Device Tree)")
>  	.l2c_aux_val	= 0,
>  	.l2c_aux_mask	= ~0,
>  	.dt_compat	= rockchip_board_dt_compat,
> +	.init_machine	= rockchip_dt_init,
>  MACHINE_END
> diff --git a/arch/arm/mach-rockchip/sleep.S b/arch/arm/mach-rockchip/sleep.S
> new file mode 100644
> index 0000000..e1c5b8c
> --- /dev/null
> +++ b/arch/arm/mach-rockchip/sleep.S
> @@ -0,0 +1,90 @@
> +/*
> + * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
> + * Author: Tony Xie <tony.xie@rock-chips.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + */
> +
> +#include <linux/linkage.h>
> +#include <asm/assembler.h>
> +#include <asm/memory.h>
> +
> +.data
> +/*
> + * this code will be copied from
> + * ddr to sram for system resumeing.
> + * so it is ".data section".
> + */
> +.align
> +
> +ENTRY(rockchip_slp_cpu_resume)
> +	setmode	PSR_I_BIT | PSR_F_BIT | SVC_MODE, r1  @ set svc, irqs off
> +	mrc	p15, 0, r1, c0, c0, 5
> +	and	r1, r1, #0xf
> +	cmp	r1, #0
> +	/* olny cpu0 can continue to run, the others is halt here */
> +	beq	cpu0run
> +secondary_loop:
> +	wfe
> +	b	secondary_loop
> +cpu0run:
> +	ldr	r3, rkpm_bootdata_l2ctlr_f
> +	cmp	r3, #0
> +	beq	sp_set
> +	ldr	r3, rkpm_bootdata_l2ctlr
> +	mcr	p15, 1, r3, c9, c0, 2
> +sp_set:
> +	ldr	sp, rkpm_bootdata_cpusp
> +
> +	ldr	r1, rkpm_bootdata_ddr_code
> +	cmp	r1, #0
> +	beq	res

In this series, 'ddr_code' is always zero, and othrewise unused.  I'd
suggest leaving it out of this series entirely for simplicity (and
readability) and add it back in a patch/series where it's actually used.

> +	ldr	r0, rkpm_bootdata_ddr_data
> +	blx	r1

And 'ddr_data' is entirely unused.

> +res:
> +	ldr	r1, rkpm_bootdata_cpu_code
> +	bx	r1
> +ENDPROC(rockchip_slp_cpu_resume)
> +
> +/* Parameters filled in by the kernel */
> +
> +/* Code to jump to for DDR resume code, or NULL */
> +	.global rkpm_bootdata_ddr_code
> +rkpm_bootdata_ddr_code:
> +	.long 0
> +
> +/* Data to pass to DDR resume data */
> +	.global rkpm_bootdata_ddr_data
> +rkpm_bootdata_ddr_data:
> +	.long 0
> +
> +/* Flag for whether to restore L2CTLR on resume */
> +	.global rkpm_bootdata_l2ctlr_f
> +rkpm_bootdata_l2ctlr_f:
> +	.long 0
> +
> +/* Saved L2CTLR to restore on resume */
> +	.global rkpm_bootdata_l2ctlr
> +rkpm_bootdata_l2ctlr:
> +	.long 0
> +
> +/* CPU resume SP addr */
> +	.globl rkpm_bootdata_cpusp
> +rkpm_bootdata_cpusp:
> +	.long 0
> +
> +/* CPU resume function (physical address) */
> +	.globl rkpm_bootdata_cpu_code
> +rkpm_bootdata_cpu_code:
> +	.long 0
> +
> +ENTRY(rk3288_bootram_sz)
> +        .word   . - rockchip_slp_cpu_resume

Kevin

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v6 4/7] ARM: rockchip: add suspend and resume for RK3288
  2014-10-29 19:01   ` Kevin Hilman
@ 2014-10-29 19:26     ` Doug Anderson
  2014-10-29 20:00       ` Kevin Hilman
  2014-11-11  7:33     ` Chris Zhong
  1 sibling, 1 reply; 20+ messages in thread
From: Doug Anderson @ 2014-10-29 19:26 UTC (permalink / raw)
  To: linux-arm-kernel

Kevin,

On Wed, Oct 29, 2014 at 12:01 PM, Kevin Hilman <khilman@kernel.org> wrote:
>> +     /* set bit 8 so that system will resume to FAST_BOOT_ADDR */
>> +     regmap_write(sgrf_regmap, RK3288_SGRF_SOC_CON0,
>> +                  BIT(SGRF_FAST_BOOT_EN) | BIT(SGRF_FAST_BOOT_EN + 16));
>
> Comment says "bit 8", but code says bit 8 and bit 24, and if bit 24 is
> needed, it should probably get its own #define in the header.

This is Rockchip's "write enable" feature hitting us again.  The upper
16-bits are "write enables" for the lower 16-bits.  So to set bit 8
you set bit 8 to whatever value you want and then 'write enable' by
setting (1 << 8 << 16).  It allows you to avoid read-modify-write.
See this in Rockchip's clk.h:

#define HIWORD_UPDATE(val, mask, shift) \
                ((val) << (shift) | (mask) << ((shift) + 16))

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v6 4/7] ARM: rockchip: add suspend and resume for RK3288
  2014-10-29 19:26     ` Doug Anderson
@ 2014-10-29 20:00       ` Kevin Hilman
  0 siblings, 0 replies; 20+ messages in thread
From: Kevin Hilman @ 2014-10-29 20:00 UTC (permalink / raw)
  To: linux-arm-kernel

Doug Anderson <dianders@chromium.org> writes:

> Kevin,
>
> On Wed, Oct 29, 2014 at 12:01 PM, Kevin Hilman <khilman@kernel.org> wrote:
>>> +     /* set bit 8 so that system will resume to FAST_BOOT_ADDR */
>>> +     regmap_write(sgrf_regmap, RK3288_SGRF_SOC_CON0,
>>> +                  BIT(SGRF_FAST_BOOT_EN) | BIT(SGRF_FAST_BOOT_EN + 16));
>>
>> Comment says "bit 8", but code says bit 8 and bit 24, and if bit 24 is
>> needed, it should probably get its own #define in the header.
>
> This is Rockchip's "write enable" feature hitting us again.  The upper
> 16-bits are "write enables" for the lower 16-bits.  So to set bit 8
> you set bit 8 to whatever value you want and then 'write enable' by
> setting (1 << 8 << 16).  It allows you to avoid read-modify-write.
> See this in Rockchip's clk.h:
>
> #define HIWORD_UPDATE(val, mask, shift) \
>                 ((val) << (shift) | (mask) << ((shift) + 16))

OK, sounds like a macro is needed in the core code as well (with a
comment for those of us who will forget why it's needed)

Kevin

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v6 0/7] This is the 1st version of suspend for RK3288.
  2014-10-29 11:51 [PATCH v6 0/7] This is the 1st version of suspend for RK3288 Chris Zhong
                   ` (5 preceding siblings ...)
  2014-10-29 11:52 ` [PATCH v6 7/7] ARM: dts: add suspend voltage setting for RK808 Chris Zhong
@ 2014-10-29 20:06 ` Kevin Hilman
  2014-10-29 21:50   ` Heiko Stübner
  6 siblings, 1 reply; 20+ messages in thread
From: Kevin Hilman @ 2014-10-29 20:06 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Chris,

Chris Zhong <zyw@rock-chips.com> writes:

> RK3288 can shut down the cpu, gpu and other device controllers in suspend,
> and it will pull the GLOBAL_PWROFF pin to high in the final stage of the
> process of suspend, pull the pin to low again when resume.

I tried to test this on top of linux-next (next-20141029) and it doesn't
wake up from serial port activity.

Can you describe how to test this, as well as describe dependencies on
other out-of-tree patches, including pointers to where they've been
posted.

Also, please describe how you tested this and on which hardware
platforms.  It's a big help to reviewers to know how it's been tested,
and for anyone with similar hardware to know what else it's been tested
on.

Thanks,

Kevin

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v6 0/7] This is the 1st version of suspend for RK3288.
  2014-10-29 20:06 ` [PATCH v6 0/7] This is the 1st version of suspend for RK3288 Kevin Hilman
@ 2014-10-29 21:50   ` Heiko Stübner
  2014-10-29 22:26     ` Doug Anderson
  0 siblings, 1 reply; 20+ messages in thread
From: Heiko Stübner @ 2014-10-29 21:50 UTC (permalink / raw)
  To: linux-arm-kernel

Am Mittwoch, 29. Oktober 2014, 13:06:05 schrieb Kevin Hilman:
> Hi Chris,
> 
> Chris Zhong <zyw@rock-chips.com> writes:
> > RK3288 can shut down the cpu, gpu and other device controllers in suspend,
> > and it will pull the GLOBAL_PWROFF pin to high in the final stage of the
> > process of suspend, pull the pin to low again when resume.
> 
> I tried to test this on top of linux-next (next-20141029) and it doesn't
> wake up from serial port activity.
> 
> Can you describe how to test this, as well as describe dependencies on
> other out-of-tree patches, including pointers to where they've been
> posted.
> 
> Also, please describe how you tested this and on which hardware
> platforms.  It's a big help to reviewers to know how it's been tested,
> and for anyone with similar hardware to know what else it's been tested
> on.

When testing this series it did go to sleep with

/ # echo mem > /sys/power/state 
PM: Syncing filesystems ... done.
Freezing user space processes ... (elapsed 0.010 seconds) done.
Freezing remaining freezable tasks ... (elapsed 0.010 seconds) done.
PM: suspend of devices complete after 0.001 msecs
PM: late suspend of devices complete after 0.001 msecs
PM: noirq suspend of devices complete after 0.001 msecs
Disabling non-boot CPUs ...
CPU1: shutdown
CPU2: shutdown
CPU3: shutdown

and the change in pmic-noise lets me assume it's really asleep.


But I'm not exactly sure how to wake it up again. I even hard-wired the gpio-
keys to always enable the irq wake, but so far it didn't wake again when 
pressing the power-key on the evb.

If anyone wants to peek, the collected patches (Doug's and Chris') can be 
found on [0].


Heiko


[0] https://github.com/mmind/linux-rockchip/tree/wip/rk3288-suspend

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v6 0/7] This is the 1st version of suspend for RK3288.
  2014-10-29 21:50   ` Heiko Stübner
@ 2014-10-29 22:26     ` Doug Anderson
  0 siblings, 0 replies; 20+ messages in thread
From: Doug Anderson @ 2014-10-29 22:26 UTC (permalink / raw)
  To: linux-arm-kernel

Heiko,

On Wed, Oct 29, 2014 at 2:50 PM, Heiko St?bner <heiko@sntech.de> wrote:
> Am Mittwoch, 29. Oktober 2014, 13:06:05 schrieb Kevin Hilman:
>> Hi Chris,
>>
>> Chris Zhong <zyw@rock-chips.com> writes:
>> > RK3288 can shut down the cpu, gpu and other device controllers in suspend,
>> > and it will pull the GLOBAL_PWROFF pin to high in the final stage of the
>> > process of suspend, pull the pin to low again when resume.
>>
>> I tried to test this on top of linux-next (next-20141029) and it doesn't
>> wake up from serial port activity.
>>
>> Can you describe how to test this, as well as describe dependencies on
>> other out-of-tree patches, including pointers to where they've been
>> posted.
>>
>> Also, please describe how you tested this and on which hardware
>> platforms.  It's a big help to reviewers to know how it's been tested,
>> and for anyone with similar hardware to know what else it's been tested
>> on.
>
> When testing this series it did go to sleep with
>
> / # echo mem > /sys/power/state
> PM: Syncing filesystems ... done.
> Freezing user space processes ... (elapsed 0.010 seconds) done.
> Freezing remaining freezable tasks ... (elapsed 0.010 seconds) done.
> PM: suspend of devices complete after 0.001 msecs
> PM: late suspend of devices complete after 0.001 msecs
> PM: noirq suspend of devices complete after 0.001 msecs
> Disabling non-boot CPUs ...
> CPU1: shutdown
> CPU2: shutdown
> CPU3: shutdown
>
> and the change in pmic-noise lets me assume it's really asleep.
>
>
> But I'm not exactly sure how to wake it up again. I even hard-wired the gpio-
> keys to always enable the irq wake, but so far it didn't wake again when
> pressing the power-key on the evb.
>
> If anyone wants to peek, the collected patches (Doug's and Chris') can be
> found on [0].

Unless you get a device tree that sets up regulator states I think
you're going to be SOL.  In other words one thing that will definitely
bite you is (7592100 ARM: dts: add suspend voltage setting for RK808).

That's referencing the "global_pwroff" which means we'll be telling
the PMIC when we suspend.  That's good but the PMIC hasn't been
programmed with any reasonable voltages because all the device tree
properties are ignored.  That's not good.  You can see my comments
about this at <https://patchwork.kernel.org/patch/5186951/>

Also: I think that I remember the GPU being pretty unhappy if you
turned its voltage off but didn't gate its power domain.  At the
moment I've got all the power domain patches sitting in my tree plus
<https://chromium-review.googlesource.com/#/c/223464/>.  You might
also just be able to change the GPU not to be powered off in suspend
(but then you need to program a good voltage to it).


At the moment all of my testing is happening in the chromeos-3.14 tree
(which has a ton of backports and thus isn't that different than
mainline as far as Rockchip is concerned).  You could theoretically
try jamming Chris's stuff into the top of the chromeos-3.14 tree and
see what happens on EVB.  I may be able to give that a shot
tomorrow...

-Doug

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v6 7/7] ARM: dts: add suspend voltage setting for RK808
  2014-10-29 16:51       ` Doug Anderson
@ 2014-10-30  0:32         ` Javier Martinez Canillas
  0 siblings, 0 replies; 20+ messages in thread
From: Javier Martinez Canillas @ 2014-10-30  0:32 UTC (permalink / raw)
  To: linux-arm-kernel

Hello Doug,

On 10/29/2014 05:51 PM, Doug Anderson wrote:
>>
>> In other words, do you want the core to call rk808_set_suspend_voltage() or
>> rk808_set_suspend_enable()? If the later then you can use Chanwoo's bindings:
>>
>> regulator-state-mem {
>>    regulator-on-in-suspend;
>> };
> 
> The rk808 has a separate register for storing the sleep voltage,
> though.  ...so just using "regulator-on-in-suspend" without any
> changes to the rk808 driver won't get us this "for free".  I'd expect
> to need
> 
>  regulator-state-mem {
>     regulator-on-in-suspend;
>     regulator-suspend-microvolts = <3300000>;
>  };
>

I see, then as you said the voltage has to be defined in the DT for the suspend
state.

As I mentioned in my previous email, Chanwoo's original suspend state series had
a "regulator-volt" DT property that was used to fill the struct regulator_state
.uV field so the core could call the .set_suspend_voltage function handler.

AFAIU Mark wanted the property to be renamed to "rebulator-microvolt" [0] to be
consistent with the rest of the binding and also he was not sure if the suspend
voltage needed be in the runtime voltage range specified by the properties
"regulator-{min,max}-microvolt" since Chanwoo's patch had a check for that [1].

Chanwoo decided to drop setting the suspend voltage in a following version, but
given that there is a use case for this feature, I think that those two issues
could be addressed and added that property to the generic regulator DT binding.
 
> IMHO it wouldn't be insane to say that if someone has
> "regulator-on-in-suspend" but no suspend voltage defined that it
> should carry forward the runtime voltage.  I actually argued for that
> earlier and Mark Brown said "no" at
> <http://marc.info/?l=linux-kernel&m=141277262106368&w=2>.  More at
> <http://marc.info/?l=linux-kernel&m=141279245027919&w=2>.  It's
> entirely possible I wasn't making myself clear, though.
> 

What I learned from previous Mark's explanations is that there is always a
reason why the regulator API split different operations. So if there is both
a .set_voltage and .set_suspend_voltage, then that is an indication for me
that if someone wants a particular voltage during suspend then it has to be
explicitly set and should not rely on the runtime voltage previously set.

The fact that the rk808 has different registers to configure the runtime
and suspend voltage seems to confirm that the API split makes sense.

Best regards,
Javier

[0]: https://lkml.org/lkml/2014/9/4/652
[1]: https://lkml.org/lkml/2014/9/4/651

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v6 1/7] pinctrl: rockchip: add suspend/resume functions
  2014-10-29 11:51 ` [PATCH v6 1/7] pinctrl: rockchip: add suspend/resume functions Chris Zhong
@ 2014-10-31 20:54   ` Linus Walleij
  2014-11-02 20:47   ` Heiko Stübner
  1 sibling, 0 replies; 20+ messages in thread
From: Linus Walleij @ 2014-10-31 20:54 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Oct 29, 2014 at 12:51 PM, Chris Zhong <zyw@rock-chips.com> wrote:

> support suspend/resume of pinctrl, it allows handling sleep mode
> for hogged pins in pinctrl
>
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> Tested-by: Doug Anderson <dianders@chromium.org>
> Reviewed-by: Doug Anderson <dianders@chromium.org>
> Tested-by: Heiko Stuebner <heiko@sntech.de>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>

As mentioned I expect Heiko to queue this patch with the
other stuff and send the result to me.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v6 1/7] pinctrl: rockchip: add suspend/resume functions
  2014-10-29 11:51 ` [PATCH v6 1/7] pinctrl: rockchip: add suspend/resume functions Chris Zhong
  2014-10-31 20:54   ` Linus Walleij
@ 2014-11-02 20:47   ` Heiko Stübner
  1 sibling, 0 replies; 20+ messages in thread
From: Heiko Stübner @ 2014-11-02 20:47 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Chris,

Am Mittwoch, 29. Oktober 2014, 19:51:59 schrieb Chris Zhong:
> support suspend/resume of pinctrl, it allows handling sleep mode
> for hogged pins in pinctrl

I've now applied patches 1 and 2 of your series to my v3.19-pinctrl/next 
branch and will be sending a pull request to LinusW shortly.

So _if_ a v7 becomes necessary you don't need to include these two patches 
anymore.


Heiko


> 
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> Tested-by: Doug Anderson <dianders@chromium.org>
> Reviewed-by: Doug Anderson <dianders@chromium.org>
> Tested-by: Heiko Stuebner <heiko@sntech.de>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> 
> ---
> 
> Changes in v6: None
> Changes in v5:
> - reset-author
> - use "__maybe_unused" annotation
> 
> Changes in v4:
> - use SIMPLE_DEV_PM_OPS for suspend/resume struct
> 
> Changes in v3: None
> Changes in v2: None
> 
>  drivers/pinctrl/pinctrl-rockchip.c |   18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/pinctrl/pinctrl-rockchip.c
> b/drivers/pinctrl/pinctrl-rockchip.c index 016f457..172ad1d 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c
> @@ -1770,6 +1770,23 @@ static struct rockchip_pin_ctrl
> *rockchip_pinctrl_get_soc_data( return ctrl;
>  }
> 
> +static int __maybe_unused rockchip_pinctrl_suspend(struct device *dev)
> +{
> +	struct rockchip_pinctrl *info = dev_get_drvdata(dev);
> +
> +	return pinctrl_force_sleep(info->pctl_dev);
> +}
> +
> +static int __maybe_unused rockchip_pinctrl_resume(struct device *dev)
> +{
> +	struct rockchip_pinctrl *info = dev_get_drvdata(dev);
> +
> +	return pinctrl_force_default(info->pctl_dev);
> +}
> +
> +static SIMPLE_DEV_PM_OPS(rockchip_pinctrl_dev_pm_ops,
> rockchip_pinctrl_suspend, +			 rockchip_pinctrl_resume);
> +
>  static int rockchip_pinctrl_probe(struct platform_device *pdev)
>  {
>  	struct rockchip_pinctrl *info;
> @@ -1983,6 +2000,7 @@ static struct platform_driver rockchip_pinctrl_driver
> = { .driver = {
>  		.name	= "rockchip-pinctrl",
>  		.owner	= THIS_MODULE,
> +		.pm = &rockchip_pinctrl_dev_pm_ops,
>  		.of_match_table = rockchip_pinctrl_dt_match,
>  	},
>  };

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v6 4/7] ARM: rockchip: add suspend and resume for RK3288
  2014-10-29 19:01   ` Kevin Hilman
  2014-10-29 19:26     ` Doug Anderson
@ 2014-11-11  7:33     ` Chris Zhong
  1 sibling, 0 replies; 20+ messages in thread
From: Chris Zhong @ 2014-11-11  7:33 UTC (permalink / raw)
  To: linux-arm-kernel


On 10/30/2014 03:01 AM, Kevin Hilman wrote:
> Chris Zhong <zyw@rock-chips.com> writes:
>
>> It's a basic version of suspend and resume for rockchip,
>> it only support RK3288 now.
>>
>> Signed-off-by: Tony Xie <xxx@rock-chips.com>
>> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
>> Tested-by: Doug Anderson <dianders@chromium.org>
>> Reviewed-by: Doug Anderson <dianders@chromium.org>
>>
>> ---
>> +static void rk3288_fill_in_bootram(u32 level)
> This function name no longer reflects what it does...
have removed in v7.
>
>> +{
>> +	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();
>> +
>> +	rkpm_bootdata_ddr_code = 0;
> Already initialized to 0 in the resume code.
have removed in v7
>
>> +}
> In addition, you're no longer copying the resume code to SRAM, but this
> function writes values into the version of resume code that's in memory.
> IOW, this function is being called for every suspend cycle, but has no
> effect since the resume code has already been copied to SRAM.
>
> Seems to me like this whole 'fill_in_bootram()' needs to go away, and be
> replaced by a single setup at init time.
Done in v7
>
>> +static u32 rk3288_pmu_pwr_mode_con;
>> +static u32 rk3288_sgrf_soc_con0;
> Minor nit, and it's up to Heiko's preference here, but it's generally
> preferred to have global variables like  this at the top of the file,
> not interspersed with the code.
>
> Also, I'm not sure if you expect the number of registers that need
> saving to grow here, but regmap provides a regcache facility for keeping
> copies of things that may be useful.  Probably not worth the over head
> for a single register in each of these regmaps, but worth exploring if
> this is going to grow.
have moved them to the top of file in v7
>
>> +static void rk3288_slp_mode_set(int level)
>> +{
>> +	u32 mode_set, mode_set1;
>> +
>> +	regmap_read(sgrf_regmap, RK3288_SGRF_SOC_CON0, &rk3288_sgrf_soc_con0);
>> +
>> +	regmap_read(pmu_regmap, RK3288_PMU_PWRMODE_CON,
>> +		    &rk3288_pmu_pwr_mode_con);
>> +
>> +	/* set bit 8 so that system will resume to FAST_BOOT_ADDR */
>> +	regmap_write(sgrf_regmap, RK3288_SGRF_SOC_CON0,
>> +		     BIT(SGRF_FAST_BOOT_EN) | BIT(SGRF_FAST_BOOT_EN + 16));
> Comment says "bit 8", but code says bit 8 and bit 24, and if bit 24 is
> needed, it should probably get its own #define in the header.
Done in v7
>
>> +
>> +	/* booting address of resuming system is from this register value */
>> +	regmap_write(sgrf_regmap, RK3288_SGRF_FAST_BOOT_ADDR,
>> +		     rk3288_bootram_phy);
>> +
>> +	regmap_write(pmu_regmap, RK3288_PMU_WAKEUP_CFG1,
>> +		     PMU_ARMINT_WAKEUP_EN);
>> +
>> +	mode_set = BIT(PMU_GLOBAL_INT_DISABLE) | BIT(PMU_L2FLUSH_EN) |
>> +		   BIT(PMU_SREF0_ENTER_EN) | BIT(PMU_SREF1_ENTER_EN) |
>> +		   BIT(PMU_DDR0_GATING_EN) | BIT(PMU_DDR1_GATING_EN) |
>> +		   BIT(PMU_PWR_MODE_EN) | BIT(PMU_CHIP_PD_EN) |
>> +		   BIT(PMU_SCU_EN);
>> +
>> +	mode_set1 = BIT(PMU_CLR_CORE) | BIT(PMU_CLR_CPUP);
>> +
>> +	if (level == ROCKCHIP_ARM_OFF_LOGIC_DEEP) {
>> +		/* arm off, logic deep sleep */
>> +		mode_set |= BIT(PMU_BUS_PD_EN) |
>> +			    BIT(PMU_DDR1IO_RET_EN) | BIT(PMU_DDR0IO_RET_EN) |
>> +			    BIT(PMU_OSC_24M_DIS) | BIT(PMU_PMU_USE_LF) |
>> +			    BIT(PMU_ALIVE_USE_LF) | BIT(PMU_PLL_PD_EN);
>> +
>> +		mode_set1 |= BIT(PMU_CLR_ALIVE) | BIT(PMU_CLR_BUS) |
>> +			     BIT(PMU_CLR_PERI) | BIT(PMU_CLR_DMA);
>> +
>> +	} else {
>> +		/*
>> +		 * arm off, logic normal
>> +		 * if pmu_clk_core_src_gate_en is not set,
>> +		 * wakeup will be error
>> +		 */
>> +		mode_set |= BIT(PMU_CLK_CORE_SRC_GATE_EN);
>> +	}
>> +
>> +	regmap_write(pmu_regmap, RK3288_PMU_PWRMODE_CON, mode_set);
>> +
> extra blank line not needed
Done in v7
>
>> +	regmap_write(pmu_regmap, RK3288_PMU_PWRMODE_CON1, mode_set1);
>> +}
>> +
>> +static void rk3288_slp_mode_set_resume(void)
>> +{
>> +	regmap_write(pmu_regmap, RK3288_PMU_PWRMODE_CON,
>> +		     rk3288_pmu_pwr_mode_con);
> extra blank line
Done in v7
>
>> +	regmap_write(sgrf_regmap, RK3288_SGRF_SOC_CON0,
>> +		     rk3288_sgrf_soc_con0 | BIT(SGRF_FAST_BOOT_EN + 16));
> here is magic bit 24 again.
Done in v7
>
>> +}
>> +
>> +/*
>> + * level: used to control the level of sleep mode.
>> + * ROCKCHIP_ARM_OFF_LOGIC_NORMAL : arm is off and logic is in normal
>> + * sleep mode.
>> + * ROCKCHIP_ARM_OFF_LOGIC_DEEP : arm is off and logic is in deep sleep mode
> IMO, these descriptions don't add any value over the already descriptive
> names of the #defines
have removed it in v7
>
>> + */
>> +static void rk3288_save_settings(u32 level)
>> +{
>> +	rk3288_fill_in_bootram(level);
>> +
>> +	rk3288_slp_mode_set(level);
>> +}
>> +
>> +static void rk3288_restore_settings(void)
>> +{
>> +	rk3288_slp_mode_set_resume();
>> +}
> These 'save' and 'restore' functions seem like unnecessary levels of
> redirection, especially if 'fill_in_bootram' goes away.
>
> IMO, probably best to just move the calls into _suspend_enter().
Done in v7
.
.
.
>> diff --git a/arch/arm/mach-rockchip/pm.h b/arch/arm/mach-rockchip/pm.h
>> new file mode 100644
>> index 0000000..781ea9d
>> --- /dev/null
>> +++ b/arch/arm/mach-rockchip/pm.h
>> @@ -0,0 +1,103 @@
>> +/*
>> + * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
>> + * Author: Tony Xie <tony.xie@rock-chips.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>> + * more details.
>> + */
>> +
>> +#ifndef __MACH_ROCKCHIP_PM_H
>> +#define __MACH_ROCKCHIP_PM_H
>> +
>> +extern unsigned long rkpm_bootdata_cpusp;
>> +extern unsigned long rkpm_bootdata_cpu_code;
>> +extern unsigned long rkpm_bootdata_l2ctlr_f;
>> +extern unsigned long rkpm_bootdata_l2ctlr;
>> +extern unsigned long rkpm_bootdata_ddr_code;
>> +extern unsigned long rkpm_bootdata_ddr_data;
>> +extern unsigned long rk3288_bootram_sz;
>> +/* The function selction of low power mode */
>> +/* arm is off and logic is in normal sleep mode */
>> +#define ROCKCHIP_ARM_OFF_LOGIC_NORMAL (0)
>> +/* arm is off and logic is in deep sleep mode */
>> +#define ROCKCHIP_ARM_OFF_LOGIC_DEEP (1)
> These defines only seem to be used inside pm.c, so should be moved there
> (and maybe turned into an enum?)
have removed them to pm.c, but still Macros, I will use enum in v8.

.
.
> + * ddr to sram for system resumeing.
> + * so it is ".data section".
> + */
> +.align
> +
> +ENTRY(rockchip_slp_cpu_resume)
> +	setmode	PSR_I_BIT | PSR_F_BIT | SVC_MODE, r1  @ set svc, irqs off
> +	mrc	p15, 0, r1, c0, c0, 5
> +	and	r1, r1, #0xf
> +	cmp	r1, #0
> +	/* olny cpu0 can continue to run, the others is halt here */
> +	beq	cpu0run
> +secondary_loop:
> +	wfe
> +	b	secondary_loop
> +cpu0run:
> +	ldr	r3, rkpm_bootdata_l2ctlr_f
> +	cmp	r3, #0
> +	beq	sp_set
> +	ldr	r3, rkpm_bootdata_l2ctlr
> +	mcr	p15, 1, r3, c9, c0, 2
> +sp_set:
> +	ldr	sp, rkpm_bootdata_cpusp
> +
> +	ldr	r1, rkpm_bootdata_ddr_code
> +	cmp	r1, #0
> +	beq	res
> In this series, 'ddr_code' is always zero, and othrewise unused.  I'd
> suggest leaving it out of this series entirely for simplicity (and
> readability) and add it back in a patch/series where it's actually used.
>
>> +	ldr	r0, rkpm_bootdata_ddr_data
>> +	blx	r1
> And 'ddr_data' is entirely unused.
Yes, the ddr_ have been removed in v7
>
>> +res:
>> +	ldr	r1, rkpm_bootdata_cpu_code
>> +	bx	r1
>> +ENDPROC(rockchip_slp_cpu_resume)
>> +
>> +/* Parameters filled in by the kernel */
>> +
>> +/* Code to jump to for DDR resume code, or NULL */
>> +	.global rkpm_bootdata_ddr_code
>> +rkpm_bootdata_ddr_code:
>> +	.long 0
>> +
>> +/* Data to pass to DDR resume data */
>> +	.global rkpm_bootdata_ddr_data
>> +rkpm_bootdata_ddr_data:
>> +	.long 0
>> +
>> +/* Flag for whether to restore L2CTLR on resume */
>> +	.global rkpm_bootdata_l2ctlr_f
>> +rkpm_bootdata_l2ctlr_f:
>> +	.long 0
>> +
>> +/* Saved L2CTLR to restore on resume */
>> +	.global rkpm_bootdata_l2ctlr
>> +rkpm_bootdata_l2ctlr:
>> +	.long 0
>> +
>> +/* CPU resume SP addr */
>> +	.globl rkpm_bootdata_cpusp
>> +rkpm_bootdata_cpusp:
>> +	.long 0
>> +
>> +/* CPU resume function (physical address) */
>> +	.globl rkpm_bootdata_cpu_code
>> +rkpm_bootdata_cpu_code:
>> +	.long 0
>> +
>> +ENTRY(rk3288_bootram_sz)
>> +        .word   . - rockchip_slp_cpu_resume
> Kevin
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
>
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2014-11-11  7:33 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-29 11:51 [PATCH v6 0/7] This is the 1st version of suspend for RK3288 Chris Zhong
2014-10-29 11:51 ` [PATCH v6 1/7] pinctrl: rockchip: add suspend/resume functions Chris Zhong
2014-10-31 20:54   ` Linus Walleij
2014-11-02 20:47   ` Heiko Stübner
2014-10-29 11:52 ` [PATCH v6 2/7] pinctrl: rockchip: save and restore gpio6_c6 pinmux in suspend/resume Chris Zhong
2014-10-29 11:52 ` [PATCH v6 3/7] clk: rockchip: RK3288: add suspend and resume Chris Zhong
2014-10-29 11:52 ` [PATCH v6 4/7] ARM: rockchip: add suspend and resume for RK3288 Chris Zhong
2014-10-29 19:01   ` Kevin Hilman
2014-10-29 19:26     ` Doug Anderson
2014-10-29 20:00       ` Kevin Hilman
2014-11-11  7:33     ` Chris Zhong
2014-10-29 11:52 ` [PATCH v6 6/7] ARM: dts: add RK3288 suspend support Chris Zhong
2014-10-29 11:52 ` [PATCH v6 7/7] ARM: dts: add suspend voltage setting for RK808 Chris Zhong
2014-10-29 15:40   ` Doug Anderson
2014-10-29 16:29     ` Javier Martinez Canillas
2014-10-29 16:51       ` Doug Anderson
2014-10-30  0:32         ` Javier Martinez Canillas
2014-10-29 20:06 ` [PATCH v6 0/7] This is the 1st version of suspend for RK3288 Kevin Hilman
2014-10-29 21:50   ` Heiko Stübner
2014-10-29 22:26     ` Doug Anderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).